[libvirt] [PATCH 4/5] docs: Fix XML schema handling of LUN address in hostdev tag

[Eric Farman]

On 06/18/2015 02:38 PM, John Ferlan wrote:
>
> On 06/16/2015 11:29 PM, Eric Farman wrote:
>> Defining a domain with a SCSI disk attached via a hostdev
>> tag and a source address unit value longer than two digits
>> causes an error when editing the domain with virsh edit,
>> even if no changes are made to the domain definition.
>> The error suggests invalid XML, somewhere:
>>
>>    # virsh edit lmb_guest
>>    error: XML document failed to validate against schema:
>>    Unable to validate doc against /usr/local/share/libvirt/schemas/domain.rng
>>    Extra element devices in interleave
>>    Element domain failed to validate content
>>
>> The virt-xml-validate tool fails with a similar error:
>>
>>    # virt-xml-validate lmb_guest.xml
>>    Relax-NG validity error : Extra element devices in interleave
>>    lmb_guest.xml:17: element devices: Relax-NG validity error :
>>    Element domain failed to validate content
>>    lmb_guest.xml fails to validate
>>
>> The hostdev tag requires a source address to be specified,
>> which includes bus, target, and unit address attributes.
>> According to the SCSI Architecture Model spec (section
>> 4.9 of SAM-2), a LUN address is 64 bits and thus could be
>> up to 20 decimal digits long.  Unfortunately, the XML
>> schema limits this string to just two digits.  Similarly,
>> the target field can be up to 32 bits in length, which
>> would be 10 decimal digits.
>>
>>    # lsscsi -xx
>>    [0:0:19:0x4022401100000000]  disk    IBM      2107900          3.44 /dev/sda
>>    # lsscsi
>>    [0:0:19:1074872354]disk    IBM      2107900          3.44  /dev/sda
>>    # cat lmb_guest.xml
>>    <domain type='kvm'>
>>      <name>lmb_guest</name>
>>      <memory unit='MiB'>1024</memory>
>>    ...trimmed...
>>      <devices>
>>        <controller type='scsi' model='virtio-scsi' index='0'/>
>>        <hostdev mode='subsystem' type='scsi'>
>>          <source>
>>            <adapter name='scsi_host0'/>
>>            <address bus='0' target='19' unit='1074872354'/>
>>          </source>
>>        </hostdev>
>>    ...trimmed...
>>
>> Since the reference unit and target fields are used in
>> several places in the XML schema, create a separate one
>> specific for SCSI Logical Units that will permit the
>> greater length.  This permits both the validation utility
>> and the virsh edit command to succeed when a hostdev
>> tag is included.
>>
>> Signed-off-by: Eric Farman <farman at linux.vnet.ibm.com>
>> Reviewed-by: Matthew Rosato <mjrosato at linux.vnet.ibm.com>
>> Reviewed-by: Stefan Zimmermann <stzi at linux.vnet.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy at linux.vnet.ibm.com>
>> ---
>>   docs/formatdomain.html.in     |  6 +++++-
>>   docs/schemas/domaincommon.rng | 14 ++++++++++++--
>>   2 files changed, 17 insertions(+), 3 deletions(-)
>>
>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index 4e85b51..c88c4a6 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -3256,7 +3256,11 @@
>>             </dd>
>>             <dt>scsi</dt>
>>             <dd>SCSI devices are described by both the <code>adapter</code>
>> -            and <code>address</code> elements.
>> +            and <code>address</code> elements. The <code>address</code>
>> +            element includes a <code>bus</code> attribute (a 2-digit bus
>> +            number), a <code>target</code> attribute (a 10-digit target
>> +            number), and a <code>unit</code> attribute (a 20-digit unit
>> +            number on the bus).
> Since we know from the v1 comments that qemu has a max of 16384 units, add:
>
> Not all hypervisors support larger <code>unit</code> values. It is up to
> each hypervisor to determine the maximum value supported for the adapter.
>
> I could add "<code>target</code> and " if you think that would be better..

The text with both target and unit sounds good to me.

>
>
>>               <p>
>>               <span class="since">Since 1.2.8</span>, the <code>source</code>
>>               element of a SCSI device may contain the <code>protocol</code>
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index f0f7400..b3c5cb8 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -3833,10 +3833,10 @@
>>         <ref name="driveBus"/>
>>       </attribute>
>>       <attribute name="target">
>> -      <ref name="driveTarget"/>
>> +      <ref name="driveSCSITarget"/>
>>       </attribute>
>>       <attribute name="unit">
>> -      <ref name="driveUnit"/>
>> +      <ref name="driveSCSIUnit"/>
>>       </attribute>
>>     </define>
>>     <define name="usbportaddress">
>> @@ -5129,11 +5129,21 @@
>>         <param name="pattern">[0-9]{1,2}</param>
>>       </data>
>>     </define>
>> +  <define name="driveSCSITarget">
>> +    <data type="string">
>> +      <param name="pattern">[0-9]{1,10}</param>
>> +    </data>
>> +  </define>
>>     <define name="driveUnit">
>>       <data type="string">
>>         <param name="pattern">[0-9]{1,2}</param>
>>       </data>
>>     </define>
>> +  <define name="driveSCSIUnit">
>> +    <data type="string">
>> +      <param name="pattern">[0-9]{1,20}</param>
>> +    </data>
>> +  </define>
>>     <define name="featureName">
>>       <data type="string">
>>         <param name='pattern'>[a-zA-Z0-9\-_\.]+</param>
>>
> I'm going to add a test as well. It's essentially a copy of
> tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-scsi.xml
> adjusted to have a larger unit value (it'll be attached)
>
Ok.  If you push it as a separate patch, feel free to add:

Reviewed-by: Eric Farman <farman at linux.vnet.ibm.com>

Thanks,
   Eric