Re: [libvirt] [PATCH v2 8/8] hostdev: Add iSCSI hostdev XML
On 08/04/2014 02:21 PM, Eric Blake wrote:
<snip>
> +++ b/docs/schemas/domaincommon.rng
> @@ -3563,13 +3563,47 @@
> </choice>
> </attribute>
> </optional>
> + <optional>
> + <ref name='diskAuth'/>
> + </optional>
Does diskAuth work for all configurations, or only for the new iscsi
configuration? By putting it here, you are allowing it for all users,...
> <element name="source">
> - <interleave>
> - <ref name="sourceinfoadapter"/>
> - <element name="address">
> - <ref name="scsiaddress"/>
> - </element>
> - </interleave>
> + <optional>
> + <attribute name="protocol">
> + <choice>
> + <value>iscsi</value>
> + </choice>
...whereas using some <group> magic could make it valid only when
protocol is iscsi.
> + </attribute>
> + <attribute name="name">
> + <text/>
> + </attribute>
> + </optional>
> + <choice>
> + <group>
> + <interleave>
> + <ref name="sourceinfoadapter"/>
> + <element name="address">
> + <ref name="scsiaddress"/>
> + </element>
> + </interleave>
> + </group>
> + <group>
> + <interleave>
> + <oneOrMore>
> + <element name='host'>
> + <attribute name='name'>
> + <text/>
> + </attribute>
> + <optional>
> + <attribute name='port'>
> + <ref name="PortNumber"/>
> + </attribute>
> + </optional>
> + <empty/>
> + </element>
> + </oneOrMore>
> + </interleave>
This <interleave> adds nothing. It doesn't hurt to leave it in, but
when there is only one possible (repetition of a) child "host"
<element>, there is nothing to be interleaved.
Maybe perhaps sleep helped me figure this out... Is the following what
you had in mind? (and it passes make check too)
<define name="hostdevsubsysscsi">
<attribute name="type">
<value>scsi</value>
</attribute>
<optional>
<attribute name="sgio">
<choice>
<value>filtered</value>
<value>unfiltered</value>
</choice>
</attribute>
</optional>
<choice>
<group>
<element name="source">
<interleave>
<ref name="sourceinfoadapter"/>
<element name="address">
<ref name="scsiaddress"/>
</element>
</interleave>
</element>
</group>
<group>
<optional>
<ref name='diskAuth'/>
</optional>
<element name="source">
<attribute name="protocol">
<choice>
<value>iscsi</value>
</choice>
</attribute>
<attribute name="name">
<text/>
</attribute>
<interleave>
<oneOrMore>
<element name='host'>
<attribute name='name'>
<text/>
</attribute>
<optional>
<attribute name='port'>
<ref name="PortNumber"/>
</attribute>
</optional>
<empty/>
</element>
</oneOrMore>
</interleave>
</element>
</group>
</choice>
</define>
I suppose I could/should follow the disk and put auth after the source...
Would you like to see a v3? also including the extraneous spacing
change from the initial review of 5/8 and I had created a separate patch
on (which wasn't technically acked, so I didn't push):
http://www.redhat.com/archives/libvir-list/2014-July/msg01268.html
John