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

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(a)linux.vnet.ibm.com&gt; > Reviewed-by: Matthew Rosato <mjrosato(a)linux.vnet.ibm.com&gt; > Reviewed-by: Stefan Zimmermann <stzi(a)linux.vnet.ibm.com&gt; > Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com&gt; > --- > 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..

> <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)