[libvirt] [PATCH 01/14] virstoragefile: Introduce virStoragePRDef

John Ferlan jferlan at redhat.com
Fri Jan 26 03:07:16 UTC 2018



On 01/18/2018 11:04 AM, Michal Privoznik wrote:
> This is a definition that holds information on SCSI persistent
> reservation settings. The XML part looks like this:
> 
>   <reservations enabled='yes' managed='no'>
>     <source type='unix' path='/path/to/qemu-pr-helper.sock' mode='client'/>
>   </reservations>
> 
> If @managed is set to 'yes' then the <source/> is not parsed.
> This design was agreed on here:
> 
> https://www.redhat.com/archives/libvir-list/2017-November/msg01005.html
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  docs/formatdomain.html.in                          |  25 +++-
>  docs/schemas/domaincommon.rng                      |  19 +--
>  docs/schemas/storagecommon.rng                     |  34 +++++
>  src/conf/domain_conf.c                             |  36 +++++
>  src/libvirt_private.syms                           |   3 +
>  src/util/virstoragefile.c                          | 148 +++++++++++++++++++++
>  src/util/virstoragefile.h                          |  15 +++
>  .../disk-virtio-scsi-reservations-not-managed.xml  |  40 ++++++
>  .../disk-virtio-scsi-reservations.xml              |  38 ++++++
>  .../disk-virtio-scsi-reservations-not-managed.xml  |   1 +
>  .../disk-virtio-scsi-reservations.xml              |   1 +
>  tests/qemuxml2xmltest.c                            |   4 +
>  12 files changed, 348 insertions(+), 16 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-reservations-not-managed.xml
>  create mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-reservations.xml
>  create mode 120000 tests/qemuxml2xmloutdata/disk-virtio-scsi-reservations-not-managed.xml
>  create mode 120000 tests/qemuxml2xmloutdata/disk-virtio-scsi-reservations.xml
> 

Before digging too deep into this...

 - I assume we're avoiding <disk> iSCSI mainly because those
reservations would take place elsewhere, safe assumption?

 - What about using lun's from a storage pool (and what could become
your favorite, NPIV devices ;-))

   <disk type='volume' device='lun'>
     <driver name='qemu' type='raw'/>
     <source pool='sourcepool' volume='unit:0:4:0'/>
     <target dev='sda' bus='scsi'/>
   </disk>

 - What about <hostdev>'s?

   <hostdev mode='subsystem' type='scsi'>

   but not iSCSI or vHost hostdev's. I think that creates the SCSI
generic LUN, but it's been a while since I've thought about the
terminology used for hostdev's...

   I also have this faint recollection of PR related to sgio filtering
and perhaps even rawio, but dredging that back up could send me down the
path of some "downstream only" type bz's. Although searching on just
VIR_DOMAIN_DISK_DEVICE_LUN does bring up qemuSetUnprivSGIO.

And finally... I assume there is one qemu-pr-manager (qemu.conf changes
eventually)... Eventually there's magic that allows/adds per domain
*and* per LUN some socket path. If libvirt provided it's generated via
the domain temporary directory; however, what's not really clear is how
that unmanaged path really works.  Need a virtual whiteboard...

I only commented at this first patch for now. I did briefly scan the
others, but too many questions here to dig deep into those. I did run
the entire series through my Coverity checker - there's 3 things that
get tagged in later patches.

John

> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index d272cc1ba..23c5f071e 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -2457,7 +2457,10 @@
>    </disk>
>    <disk type='block' device='lun'>
>      <driver name='qemu' type='raw'/>
> -    <source dev='/dev/sda'/>
> +    <source dev='/dev/sda'>
> +      <reservations enabled='yes' managed='no'>
> +        <source type='unix' path='/path/to/qemu-pr-helper' mode='client'/>
> +      </reservations>
>      <target dev='sda' bus='scsi'/>
>      <address type='drive' controller='0' bus='0' target='3' unit='0'/>
>    </disk>
> @@ -2819,6 +2822,26 @@
>              <a href="formatstorageencryption.html">Storage Encryption</a>
>              page for more information.
>            </dd>
> +          <dt><code>reservations</code></dt>
> +          <dd><span class="since">Since libvirt 3.10.0</span>, the

Will be 4.1.0 at least...

> +            <code>reservations</code> can be a sub-element of the
> +            <code>source</code> element for storage sources (QEMU driver only).
> +            If present (and enabled) it enabled persistent reservations for

Doesn't read well - even with the second enabled changed to enables.

Furthermore what's the purpose of enabled=no? Isn't that just like
removing <reservations>? If so, then I think enabled just gets removed
and the "managed" is the only required attribute. Besides you don't have
a "enabled='no'" example.  And, no, sorry I didn't go back through the
entire "New QEMU daemon for persistent reservations" discussion that
started in August to see if this was discussed there.

Looking at virStoragePRDefFormat when enabled='no' anything anyone set
for managed would be lost (e.g. not printed). IOW: by changing from
enabled=yes and managed=no to enabled=no, then everything in the
<source...> subelement is lost.

> +            SCSI based disks. The element has one mandatory attribute
> +            <code>enabled</code> with accepted values <code>yes</code> and

NB: Other uses go with "yes" and "no" not the code wrapped tag.

> +            <code>no</code>. If the feature is enabled, then there's another
> +            mandatory attribute <code>managed</code> (accepted values are the
> +            same as for <code>enabled</code>) that enables or disables libvirt
> +            spawning any helper process (should one be needed). However, if

Things get a bit confusing from here on in...

> +            libvirt is not enabled spawning helper process (i.e. hypervisor
> +            should just use one which is already running), path to its socket
> +            must be provided in child element <code>source</code>, which
> +            currently accepts only the following attributes: <code>type</code>
> +            with one value <code>unix</code>, <code>path</code> with path the
> +            socket, and finally <code>mode</code> which accepts either
> +            <code>server</code> or <code>client</code> and specifies the role
> +            of hypervisor.

Let's see if I can suggest something...

Since libvirt 4.1.0, when the disk is a SCSI based lun, then this
element may be used to enable persistent reservations on the storage
source (QEMU driver only). The required attribute <code>managed</code>
can be either "yes" or "no" indicating whether libvirt should spawn the
helper process to communicate with QEMU via a <code>source</code>
sub-element.

[ok, I'm kind of lost, but hopefully there's something that can be
expounded up that can help clear my vision...]

> +          </dd>
>          </dl>
>  
>          <p>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index f22c932f6..884081bd7 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1498,6 +1498,9 @@
>          <optional>
>            <ref name="encryption"/>
>          </optional>
> +        <optional>
> +          <ref name="reservations"/>
> +        </optional>

domainschema fails syntax-check with your added XML

You need to add this hunk to diskSourceBlock instead of diskSourceFile
because this is only supported for 'lun' which as your XML examples
exhibit have the following syntax:

+    <disk type='block' device='lun'>
+      <source dev='/dev/HostVG/QEMUGuest1'>
+        <reservations enabled='yes' managed='no'>

and

+    <disk type='block' device='lun'>
+      <source dev='/dev/HostVG/QEMUGuest1'>
+        <reservations enabled='yes' managed='yes'/>

BTW: Since you're going w/ 'lun' (which I believe is the same as a
hostdev, then having a hostdev example and support probably is needed.

>          <zeroOrMore>
>            <ref name='devSeclabel'/>
>          </zeroOrMore>
> @@ -2447,21 +2450,7 @@
>              <value>vhostuser</value>
>            </attribute>
>            <interleave>
> -            <element name="source">
> -              <attribute name="type">
> -                <value>unix</value>
> -              </attribute>
> -              <attribute name="path">
> -                <ref name="absFilePath"/>
> -              </attribute>
> -              <attribute name="mode">
> -                <choice>
> -                  <value>server</value>
> -                  <value>client</value>
> -                </choice>
> -              </attribute>
> -              <empty/>
> -            </element>
> +            <ref name="unixSocketSource"/>
>              <ref name="interface-options"/>
>            </interleave>
>          </group>
> diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
> index edee1b084..9071667b0 100644
> --- a/docs/schemas/storagecommon.rng
> +++ b/docs/schemas/storagecommon.rng
> @@ -39,6 +39,40 @@
>      </element>
>    </define>
>  
> +  <define name='unixSocketSource'>
> +    <element name="source">
> +      <attribute name="type">
> +        <value>unix</value>
> +      </attribute>
> +      <attribute name="path">
> +        <ref name="absFilePath"/>
> +      </attribute>
> +      <attribute name="mode">
> +        <choice>
> +          <value>server</value>
> +          <value>client</value>
> +        </choice>
> +      </attribute>
> +      <empty/>
> +    </element>
> +  </define>
> +
> +  <define name='reservations'>
> +    <element name='reservations'>
> +      <attribute name='enabled'>
> +        <ref name='virYesNo'/>
> +      </attribute>
> +      <optional>
> +        <attribute name='managed'>
> +          <ref name='virYesNo'/>
> +        </attribute>
> +      </optional>
> +      <optional>
> +        <ref name='unixSocketSource'/>
> +      </optional>
> +    </element>
> +  </define>
> +
>    <define name='secret'>
>      <element name='secret'>
>        <attribute name='type'>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a1c25060f..222c6dd15 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -5163,6 +5163,13 @@ virDomainDiskDefValidate(const virDomainDiskDef *disk)
>          }
>      }
>  
> +    if (disk->src->pr &&
> +        disk->device != VIR_DOMAIN_DISK_DEVICE_LUN) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("<reservations/> allowed only for lun disks"));

Do you really need the < >/?  IOW: why not just "reservations allowed
only..."

And what about other things that support having LUN such as VOLUME and
NETWORK/ISCSI (e.g. above).

> +        return -1;
> +    }
> +
>      /* Reject disks with a bus type that is not compatible with the
>       * given address type. The function considers only buses that are
>       * handled in common code. For other bus types it's not possible
> @@ -8579,6 +8586,29 @@ virDomainDiskSourcePrivateDataParse(xmlXPathContextPtr ctxt,
>  }
>  
>  
> +static int
> +virDomainDiskSourcePRParse(xmlNodePtr node,
> +                           virStoragePRDefPtr *prsrc)
> +{
> +    xmlNodePtr child;
> +    virStoragePRDefPtr pr = NULL;
> +
> +    for (child = node->children; child; child = child->next) {
> +        if (child->type == XML_ELEMENT_NODE &&
> +            virXMLNodeNameEqual(child, "reservations")) {
> +
> +            if (!(pr = virStoragePRDefParseNode(node->doc, child)))
> +                return -1;
> +
> +            *prsrc = pr;
> +            return 0;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +
>  int
>  virDomainDiskSourceParse(xmlNodePtr node,
>                           xmlXPathContextPtr ctxt,
> @@ -8623,6 +8653,9 @@ virDomainDiskSourceParse(xmlNodePtr node,
>      if (virDomainDiskSourceEncryptionParse(node, &src->encryption) < 0)
>          goto cleanup;
>  
> +    if (virDomainDiskSourcePRParse(node, &src->pr) < 0)
> +        goto cleanup;
> +
>      if (virDomainDiskSourcePrivateDataParse(ctxt, src, flags, xmlopt) < 0)
>          goto cleanup;
>  
> @@ -22519,6 +22552,9 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
>              virStorageEncryptionFormat(&childBuf, src->encryption) < 0)
>              return -1;
>  
> +        if (src->pr)
> +            virStoragePRDefFormat(&childBuf, src->pr);
> +
>          if (virDomainDiskSourceFormatPrivateData(&childBuf, src, flags, xmlopt) < 0)
>              return -1;
>  
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index bc8cc1fba..b0dc76b91 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2717,6 +2717,9 @@ virStorageNetHostDefFree;
>  virStorageNetHostTransportTypeFromString;
>  virStorageNetHostTransportTypeToString;
>  virStorageNetProtocolTypeToString;
> +virStoragePRDefFormat;
> +virStoragePRDefFree;
> +virStoragePRDefParseNode;
>  virStorageSourceBackingStoreClear;
>  virStorageSourceClear;
>  virStorageSourceCopy;
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index 5780180a9..fda6617f7 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -1913,6 +1913,153 @@ virStorageAuthDefFormat(virBufferPtr buf,
>  }
>  
>  
> +void
> +virStoragePRDefFree(virStoragePRDefPtr prd)
> +{
> +    if (!prd)
> +        return;
> +
> +    VIR_FREE(prd->path);
> +    VIR_FREE(prd);
> +}
> +
> +
> +static virStoragePRDefPtr
> +virStoragePRDefParseXML(xmlXPathContextPtr ctxt)
> +{
> +    virStoragePRDefPtr prd, ret = NULL;
> +    char *enabled = NULL;
> +    char *managed = NULL;
> +    char *type = NULL;
> +    char *path = NULL;
> +    char *mode = NULL;
> +
> +    if (VIR_ALLOC(prd) < 0)
> +        return NULL;
> +
> +    if (!(enabled = virXPathString("string(./@enabled)", ctxt))) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("missing @enabled attribute for <reservations/>"));
> +        goto cleanup;
> +    }
> +
> +    if ((prd->enabled = virTristateBoolTypeFromString(enabled)) <= 0) {
> +        virReportError(VIR_ERR_XML_ERROR,
> +                       _("invalid value for 'enabled': %s"), NULLSTR(enabled));
> +        goto cleanup;
> +    }
> +
> +    if (prd->enabled == VIR_TRISTATE_BOOL_YES) {

As noted before, I see no purpose for enabled=no

> +        managed = virXPathString("string(./@managed)", ctxt);
> +        if ((prd->managed = virTristateBoolTypeFromString(managed)) <= 0) {
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("invalid value for 'managed': %s"), NULLSTR(managed));
> +            goto cleanup;
> +        }
> +
> +        if (prd->managed == VIR_TRISTATE_BOOL_NO) {
> +            type = virXPathString("string(./source[1]/@type)", ctxt);
> +            path = virXPathString("string(./source[1]/@path)", ctxt);
> +            mode = virXPathString("string(./source[1]/@mode)", ctxt);
> +
> +            if (!type) {
> +                virReportError(VIR_ERR_XML_ERROR, "%s",
> +                               _("missing connection type"));
> +                goto cleanup;
> +            }
> +
> +            if (!path) {
> +                virReportError(VIR_ERR_XML_ERROR, "%s",
> +                               _("missing path"));
> +                goto cleanup;
> +            }
> +
> +            if (!mode) {
> +                virReportError(VIR_ERR_XML_ERROR, "%s",
> +                               _("missing connection mode"));
> +                goto cleanup;
> +            }
> +
> +            if (STRNEQ(type, "unix")) {
> +                virReportError(VIR_ERR_XML_ERROR,
> +                               _("unsupported type: %s"), type);
> +                goto cleanup;
> +            }
> +
> +            if (STRNEQ(mode, "client")) {
> +                virReportError(VIR_ERR_XML_ERROR,
> +                               _("unsupported mode: %s"),  mode);
> +                goto cleanup;
> +            }

But docs say client or server... If only going to be client, then no
sense adding this yet.

> +
> +            prd->path = path;
> +            path = NULL;
> +
> +        }
> +    }
> +
> +    ret = prd;
> +    prd = NULL;
> +
> + cleanup:
> +    VIR_FREE(mode);
> +    VIR_FREE(path);
> +    VIR_FREE(type);
> +    VIR_FREE(managed);
> +    VIR_FREE(enabled);
> +    virStoragePRDefFree(prd);
> +    return ret;
> +}
> +
> +
> +virStoragePRDefPtr
> +virStoragePRDefParseNode(xmlDocPtr xml,
> +                         xmlNodePtr root)
> +{
> +    xmlXPathContextPtr ctxt = NULL;
> +    virStoragePRDefPtr prd = NULL;
> +
> +    ctxt = xmlXPathNewContext(xml);
> +    if (ctxt == NULL) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }
> +
> +    ctxt->node = root;
> +    prd = virStoragePRDefParseXML(ctxt);
> +
> + cleanup:
> +    xmlXPathFreeContext(ctxt);
> +    return prd;
> +}
> +
> +
> +void
> +virStoragePRDefFormat(virBufferPtr buf,
> +                      virStoragePRDefPtr prd)
> +{
> +    virBufferAsprintf(buf, "<reservations enabled='%s'",
> +                      virTristateBoolTypeToString(prd->enabled));
> +    if (prd->enabled == VIR_TRISTATE_BOOL_YES) {
> +        virBufferAsprintf(buf, " managed='%s'",
> +                          virTristateBoolTypeToString(prd->managed));
> +        if (prd->managed == VIR_TRISTATE_BOOL_NO) {
> +            virBufferAddLit(buf, ">\n");
> +            virBufferAdjustIndent(buf, 2);
> +            virBufferAddLit(buf, "<source type='unix'");
> +            virBufferEscapeString(buf, " path='%s'", prd->path);
> +            virBufferAddLit(buf, " mode='client'/>\n");

What about mode='server'?

> +            virBufferAdjustIndent(buf, -2);
> +            virBufferAddLit(buf, "</reservations>\n");
> +        } else {
> +            virBufferAddLit(buf, "/>\n");
> +        }
> +    } else {
> +        virBufferAddLit(buf, "/>\n");
> +    }
> +}
> +
> +
>  virSecurityDeviceLabelDefPtr
>  virStorageSourceGetSecurityLabelDef(virStorageSourcePtr src,
>                                      const char *model)
> @@ -2291,6 +2438,7 @@ virStorageSourceClear(virStorageSourcePtr def)
>      virBitmapFree(def->features);
>      VIR_FREE(def->compat);
>      virStorageEncryptionFree(def->encryption);
> +    virStoragePRDefFree(def->pr);
>      virStorageSourceSeclabelsClear(def);
>      virStoragePermsFree(def->perms);
>      VIR_FREE(def->timestamps);
> diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
> index ecd806c93..f82c20cf4 100644
> --- a/src/util/virstoragefile.h
> +++ b/src/util/virstoragefile.h
> @@ -216,6 +216,14 @@ struct _virStorageAuthDef {
>      virSecretLookupTypeDef seclookupdef;
>  };
>  
> +typedef struct _virStoragePRDef virStoragePRDef;
> +typedef virStoragePRDef *virStoragePRDefPtr;
> +struct _virStoragePRDef {
> +    int enabled; /* enum virTristateBool */
> +    int managed; /* enum virTristateBool */
> +    char *path;
> +};
> +
>  typedef struct _virStorageDriverData virStorageDriverData;
>  typedef virStorageDriverData *virStorageDriverDataPtr;
>  
> @@ -243,6 +251,7 @@ struct _virStorageSource {
>      bool authInherited;
>      virStorageEncryptionPtr encryption;
>      bool encryptionInherited;

Add an empty line here - if only to separate auth/encryption from
this... Auth and Encryption could probably be separated to. It's just a
visual thing.

> +    virStoragePRDefPtr pr;

Not a fan of pr, but persistentReservation would be far worse and pR is
no better.  I find I mess up typing encryption often, so either expanded
word would be tough ;-)

>  
>      virObjectPtr privateData;
>  
> @@ -369,6 +378,12 @@ virStorageAuthDefPtr virStorageAuthDefCopy(const virStorageAuthDef *src);
>  virStorageAuthDefPtr virStorageAuthDefParse(xmlDocPtr xml, xmlNodePtr root);
>  int virStorageAuthDefFormat(virBufferPtr buf, virStorageAuthDefPtr authdef);
>  
> +void virStoragePRDefFree(virStoragePRDefPtr prd);
> +virStoragePRDefPtr virStoragePRDefParseNode(xmlDocPtr xml,
> +                                            xmlNodePtr root);
> +void virStoragePRDefFormat(virBufferPtr buf,
> +                           virStoragePRDefPtr prd);
> +
>  virSecurityDeviceLabelDefPtr
>  virStorageSourceGetSecurityLabelDef(virStorageSourcePtr src,
>                                      const char *model);
> diff --git a/tests/qemuxml2argvdata/disk-virtio-scsi-reservations-not-managed.xml b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations-not-managed.xml
> new file mode 100644
> index 000000000..8ee623a70
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations-not-managed.xml
> @@ -0,0 +1,40 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219136</memory>
> +  <currentMemory unit='KiB'>219136</currentMemory>
> +  <vcpu placement='static'>8</vcpu>
> +  <os>
> +    <type arch='i686' machine='pc'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-i686</emulator>
> +    <disk type='block' device='lun'>
> +      <source dev='/dev/HostVG/QEMUGuest1'>
> +        <reservations enabled='yes' managed='no'>
> +          <source type='unix' path='/path/to/qemu-pr-helper.sock' mode='client'/>

Not that it was parsed, but 'server' was described so if supported there
should be an example.

> +        </reservations>
> +      </source>
> +      <target dev='sdb' bus='scsi'/>
> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <controller type='usb' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
> +    </controller>
> +    <controller type='scsi' index='0' model='virtio-scsi'>
> +      <driver queues='8'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> +    </controller>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <memballoon model='virtio'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
> +    </memballoon>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.xml b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.xml
> new file mode 100644
> index 000000000..874446f7b
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.xml
> @@ -0,0 +1,38 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219136</memory>
> +  <currentMemory unit='KiB'>219136</currentMemory>
> +  <vcpu placement='static'>8</vcpu>
> +  <os>
> +    <type arch='i686' machine='pc'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-i686</emulator>
> +    <disk type='block' device='lun'>
> +      <source dev='/dev/HostVG/QEMUGuest1'>
> +        <reservations enabled='yes' managed='yes'/>
> +      </source>
> +      <target dev='sdb' bus='scsi'/>

If enabled='no' is important, then a second one should be added here
without it just so we parse it.

> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <controller type='usb' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
> +    </controller>
> +    <controller type='scsi' index='0' model='virtio-scsi'>
> +      <driver queues='8'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> +    </controller>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <memballoon model='virtio'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
> +    </memballoon>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmloutdata/disk-virtio-scsi-reservations-not-managed.xml b/tests/qemuxml2xmloutdata/disk-virtio-scsi-reservations-not-managed.xml
> new file mode 120000
> index 000000000..f96d62cb6
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/disk-virtio-scsi-reservations-not-managed.xml
> @@ -0,0 +1 @@
> +../qemuxml2argvdata/disk-virtio-scsi-reservations-not-managed.xml
> \ No newline at end of file
> diff --git a/tests/qemuxml2xmloutdata/disk-virtio-scsi-reservations.xml b/tests/qemuxml2xmloutdata/disk-virtio-scsi-reservations.xml
> new file mode 120000
> index 000000000..dde52fd1d
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/disk-virtio-scsi-reservations.xml
> @@ -0,0 +1 @@
> +../qemuxml2argvdata/disk-virtio-scsi-reservations.xml
> \ No newline at end of file
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 2be8eb2c1..3d70f13c8 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -531,6 +531,10 @@ mymain(void)
>              QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_SCSI);
>      DO_TEST("disk-virtio-scsi-num_queues",
>              QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_SCSI);
> +    DO_TEST("disk-virtio-scsi-reservations",
> +            QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_SCSI);
> +    DO_TEST("disk-virtio-scsi-reservations-not-managed",
> +            QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_SCSI);
>      DO_TEST("disk-virtio-scsi-cmd_per_lun",
>              QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_SCSI);
>      DO_TEST("disk-virtio-scsi-max_sectors",
> 




More information about the libvir-list mailing list