[libvirt] [PATCH v2 8/8] hostdev: Add iSCSI hostdev XML

[John Ferlan]

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