[libvirt] [PATCH v4] qemu: Allow the user to specify vendor and product for disk

Osier Yang jyang at redhat.com
Mon Nov 26 15:25:32 UTC 2012


ping

On 2012年11月22日 15:55, Osier Yang wrote:
> QEMU supports setting vendor and product strings for disk since
> 1.2.0 (only scsi-disk, scsi-hd, scsi-cd support it), this patch
> exposes it with new XML elements<vendor>  and<product>  of disk
> device.
>
> v3 - v4:
>
>    * Per Paolo's feedback, allows all printable chars.
> ---
>   docs/formatdomain.html.in                          |   11 +++++
>   docs/schemas/domaincommon.rng                      |   14 ++++++
>   src/conf/domain_conf.c                             |   44 ++++++++++++++++++++
>   src/conf/domain_conf.h                             |    2 +
>   src/libvirt_private.syms                           |    1 +
>   src/qemu/qemu_command.c                            |   29 +++++++++++++
>   src/util/util.c                                    |   12 +++++
>   src/util/util.h                                    |    1 +
>   ...qemuxml2argv-disk-scsi-disk-vpd-build-error.xml |   35 ++++++++++++++++
>   .../qemuxml2argv-disk-scsi-disk-vpd.args           |   13 ++++++
>   .../qemuxml2argv-disk-scsi-disk-vpd.xml            |   38 +++++++++++++++++
>   tests/qemuxml2argvtest.c                           |    8 ++++
>   tests/qemuxml2xmltest.c                            |    2 +
>   13 files changed, 210 insertions(+), 0 deletions(-)
>   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd-build-error.xml
>   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args
>   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 6a3b976..903c069 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1657,6 +1657,17 @@
>           of 16 hexadecimal digits.
>           <span class='since'>Since 0.10.1</span>
>         </dd>
> +<dt><code>vendor</code></dt>
> +<dd>If present, this element specifies the vendor of a virtual hard
> +        disk or CD-ROM device. It must not be longer than 8 printable
> +        characters.
> +<span class='since'>Since 1.0.1</span>
> +</dd>
> +<dt><code>product</code></dt>
> +<dd>If present, this element specifies the product of a virtual hard
> +        disk or CD-ROM device. It must not be longer than 16 printable
> +<span class='since'>Since 1.0.1</span>
> +</dd>
>         <dt><code>host</code></dt>
>         <dd>The<code>host</code>  element has two attributes "name" and "port",
>           which specify the hostname and the port number. The meaning of this
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 02ad477..19bf597 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -905,6 +905,20 @@
>             <ref name="wwn"/>
>           </element>
>         </optional>
> +<optional>
> +<element name="vendor">
> +<data type="string">
> +<param name="pattern">[x20-x7E]{0,8}</param>
> +</data>
> +</element>
> +</optional>
> +<optional>
> +<element name="product">
> +<data type="string">
> +<param name="pattern">[x20-x7E]{0,16}</param>
> +</data>
> +</element>
> +</optional>
>       </interleave>
>     </define>
>     <define name="snapshot">
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 047c4fc..e975f74 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -979,6 +979,8 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
>       VIR_FREE(def->mirror);
>       VIR_FREE(def->auth.username);
>       VIR_FREE(def->wwn);
> +    VIR_FREE(def->vendor);
> +    VIR_FREE(def->product);
>       if (def->auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE)
>           VIR_FREE(def->auth.secret.usage);
>       virStorageEncryptionFree(def->encryption);
> @@ -3490,6 +3492,8 @@ cleanup:
>       goto cleanup;
>   }
>
> +#define VENDOR_LEN  8
> +#define PRODUCT_LEN 16
>
>   /* Parse the XML definition for a disk
>    * @param node XML nodeset to parse for disk definition
> @@ -3542,6 +3546,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
>       char *logical_block_size = NULL;
>       char *physical_block_size = NULL;
>       char *wwn = NULL;
> +    char *vendor = NULL;
> +    char *product = NULL;
>
>       if (VIR_ALLOC(def)<  0) {
>           virReportOOMError();
> @@ -3880,6 +3886,36 @@ virDomainDiskDefParseXML(virCapsPtr caps,
>
>                   if (!virValidateWWN(wwn))
>                       goto error;
> +            } else if (!vendor&&
> +                       xmlStrEqual(cur->name, BAD_CAST "vendor")) {
> +                vendor = (char *)xmlNodeGetContent(cur);
> +
> +                if (strlen(vendor)>  VENDOR_LEN) {
> +                    virReportError(VIR_ERR_XML_ERROR, "%s",
> +                                   _("disk vendor is more than 8 characters"));
> +                    goto error;
> +                }
> +
> +                if (!virStrIsPrint(vendor)) {
> +                    virReportError(VIR_ERR_XML_ERROR, "%s",
> +                                   _("disk vendor is not printable string"));
> +                    goto error;
> +                }
> +            } else if (!product&&
> +                       xmlStrEqual(cur->name, BAD_CAST "product")) {
> +                product = (char *)xmlNodeGetContent(cur);
> +
> +                if (strlen(vendor)>  PRODUCT_LEN) {
> +                    virReportError(VIR_ERR_XML_ERROR, "%s",
> +                                   _("disk product is more than 16 characters"));
> +                    goto error;
> +                }
> +
> +                if (!virStrIsPrint(product)) {
> +                    virReportError(VIR_ERR_XML_ERROR, "%s",
> +                                   _("disk product is not printable string"));
> +                    goto error;
> +                }
>               } else if (xmlStrEqual(cur->name, BAD_CAST "boot")) {
>                   /* boot is parsed as part of virDomainDeviceInfoParseXML */
>               }
> @@ -4176,6 +4212,10 @@ virDomainDiskDefParseXML(virCapsPtr caps,
>       serial = NULL;
>       def->wwn = wwn;
>       wwn = NULL;
> +    def->vendor = vendor;
> +    vendor = NULL;
> +    def->product = product;
> +    product = NULL;
>
>       if (driverType) {
>           def->format = virStorageFileFormatTypeFromString(driverType);
> @@ -4249,6 +4289,8 @@ cleanup:
>       VIR_FREE(logical_block_size);
>       VIR_FREE(physical_block_size);
>       VIR_FREE(wwn);
> +    VIR_FREE(vendor);
> +    VIR_FREE(product);
>
>       ctxt->node = save_ctxt;
>       return def;
> @@ -12085,6 +12127,8 @@ virDomainDiskDefFormat(virBufferPtr buf,
>           virBufferAddLit(buf, "<transient/>\n");
>       virBufferEscapeString(buf, "<serial>%s</serial>\n", def->serial);
>       virBufferEscapeString(buf, "<wwn>%s</wwn>\n", def->wwn);
> +    virBufferEscapeString(buf, "<vendor>%s</vendor>\n", def->vendor);
> +    virBufferEscapeString(buf, "<product>%s</product>\n", def->product);
>       if (def->encryption) {
>           virBufferAdjustIndent(buf, 6);
>           if (virStorageEncryptionFormat(buf, def->encryption)<  0)
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index c3e8c16..e3522a5 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -591,6 +591,8 @@ struct _virDomainDiskDef {
>
>       char *serial;
>       char *wwn;
> +    char *vendor;
> +    char *product;
>       int cachemode;
>       int error_policy;  /* enum virDomainDiskErrorPolicy */
>       int rerror_policy; /* enum virDomainDiskErrorPolicy */
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 756d7bd..8a1f9df 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1280,6 +1280,7 @@ virSetUIDGID;
>   virSkipSpaces;
>   virSkipSpacesAndBackslash;
>   virSkipSpacesBackwards;
> +virStrIsPrint;
>   virStrToDouble;
>   virStrToLong_i;
>   virStrToLong_l;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 02e105d..2217ccd 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2428,6 +2428,13 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
>           }
>       }
>
> +    if ((disk->vendor || disk->product)&&
> +        disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("Only scsi disk supports vendor and product"));
> +            goto error;
> +    }
> +
>       if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
>           /* make sure that both the bus and the qemu binary support
>            *  type='lun' (SG_IO).
> @@ -2455,6 +2462,11 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
>                              _("Setting wwn is not supported for lun device"));
>               goto error;
>           }
> +        if (disk->vendor || disk->product) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("Setting vendor or product is not supported for lun device"));
> +            goto error;
> +        }
>       }
>
>       switch (disk->bus) {
> @@ -2504,6 +2516,17 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
>               goto error;
>           }
>
> +        /* Properties wwn, vendor and product were introduced in the
> +         * same QEMU release (1.2.0).
> +         */
> +        if ((disk->vendor || disk->product)&&
> +            !qemuCapsGet(caps, QEMU_CAPS_SCSI_DISK_WWN)) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("Setting vendor or product for scsi disk is not "
> +                             "supported by this QEMU"));
> +            goto error;
> +        }
> +
>           controllerModel =
>               virDomainDiskFindControllerModel(def, disk,
>                                                VIR_DOMAIN_CONTROLLER_TYPE_SCSI);
> @@ -2649,6 +2672,12 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
>       if (disk->wwn)
>           virBufferAsprintf(&opt, ",wwn=%s", disk->wwn);
>
> +    if (disk->vendor)
> +        virBufferAsprintf(&opt, ",vendor=%s", disk->vendor);
> +
> +    if (disk->product)
> +        virBufferAsprintf(&opt, ",product=%s", disk->product);
> +
>       if (virBufferError(&opt)) {
>           virReportOOMError();
>           goto error;
> diff --git a/src/util/util.c b/src/util/util.c
> index 75b18c1..ea5d12e 100644
> --- a/src/util/util.c
> +++ b/src/util/util.c
> @@ -3114,3 +3114,15 @@ virValidateWWN(const char *wwn) {
>
>       return true;
>   }
> +
> +bool
> +virStrIsPrint(const char *str)
> +{
> +    int i;
> +
> +    for (i = 0; str[i]; i++)
> +        if (!c_isprint(str[i]))
> +            return false;
> +
> +    return true;
> +}
> diff --git a/src/util/util.h b/src/util/util.h
> index 4316ab1..6d5dd03 100644
> --- a/src/util/util.h
> +++ b/src/util/util.h
> @@ -280,4 +280,5 @@ bool virIsDevMapperDevice(const char *dev_name) ATTRIBUTE_NONNULL(1);
>
>   bool virValidateWWN(const char *wwn);
>
> +bool virStrIsPrint(const char *str);
>   #endif /* __VIR_UTIL_H__ */
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd-build-error.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd-build-error.xml
> new file mode 100644
> index 0000000..ca68275
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd-build-error.xml
> @@ -0,0 +1,35 @@
> +<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'>1</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</emulator>
> +<disk type='block' device='cdrom'>
> +<source dev='/dev/HostVG/QEMUGuest1'/>
> +<target dev='sda' bus='virtio'/>
> +<vendor>SEAGATE</vendor>
> +<product>ST3146707LC</product>
> +</disk>
> +<disk type='block' device='disk'>
> +<source dev='/dev/HostVG/QEMUGuest2'/>
> +<target dev='sdb' bus='scsi'/>
> +<vendor>SEAGATE</vendor>
> +<product>ST3567807GD</product>
> +<address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +</disk>
> +<controller type='usb' index='0'/>
> +<controller type='scsi' index='0' model='virtio-scsi'/>
> +<controller type='scsi' index='1' model='lsilogic'/>
> +<memballoon model='virtio'/>
> +</devices>
> +</domain>
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args
> new file mode 100644
> index 0000000..f5c1999
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args
> @@ -0,0 +1,13 @@
> +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \
> +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \
> +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
> +-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \
> +-device lsi,id=scsi1,bus=pci.0,addr=0x4 \
> +-usb \
> +-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-scsi0-0-0-0 \
> +-device scsi-cd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,\
> +id=scsi0-0-0-0,vendor=SEAGATE,product=ST3146707LC \
> +-drive file=/dev/HostVG/QEMUGuest2,if=none,id=drive-scsi1-0-0 \
> +-device scsi-hd,bus=scsi1.0,scsi-id=0,drive=drive-scsi1-0-0,\
> +id=scsi1-0-0,vendor=SEAGATE,product=ST3567807GD \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.xml
> new file mode 100644
> index 0000000..96786e3
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.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'>1</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</emulator>
> +<disk type='block' device='cdrom'>
> +<source dev='/dev/HostVG/QEMUGuest1'/>
> +<target dev='sda' bus='scsi'/>
> +<readonly/>
> +<vendor>SEAGATE</vendor>
> +<product>ST3146707LC</product>
> +<address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +</disk>
> +<disk type='block' device='disk'>
> +<source dev='/dev/HostVG/QEMUGuest2'/>
> +<target dev='sdb' bus='scsi'/>
> +<readonly/>
> +<vendor>SEAGATE</vendor>
> +<product>ST3567807GD</product>
> +<address type='drive' controller='1' bus='0' target='0' unit='0'/>
> +</disk>
> +<controller type='usb' index='0'/>
> +<controller type='scsi' index='0' model='virtio-scsi'/>
> +<controller type='scsi' index='1' model='lsilogic'/>
> +<memballoon model='virtio'/>
> +</devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 4cccbb0..1f69b6c 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -505,6 +505,14 @@ mymain(void)
>               QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
>               QEMU_CAPS_SCSI_CD, QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRTIO_SCSI_PCI,
>               QEMU_CAPS_SCSI_DISK_WWN);
> +    DO_TEST("disk-scsi-disk-vpd",
> +            QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
> +            QEMU_CAPS_SCSI_CD, QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRTIO_SCSI_PCI,
> +            QEMU_CAPS_SCSI_DISK_WWN);
> +    DO_TEST_FAILURE("disk-scsi-disk-vpd-build-error",
> +            QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
> +            QEMU_CAPS_SCSI_CD, QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRTIO_SCSI_PCI,
> +            QEMU_CAPS_SCSI_DISK_WWN);
>       DO_TEST("disk-scsi-vscsi",
>               QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
>       DO_TEST("disk-scsi-virtio-scsi",
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 1d366f1..88ef050 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -239,6 +239,8 @@ mymain(void)
>       DO_TEST("seclabel-none");
>       DO_TEST("numad-static-vcpu-no-numatune");
>
> +    DO_TEST("disk-scsi-disk-vpd");
> +
>       /* These tests generate different XML */
>       DO_TEST_DIFFERENT("balloon-device-auto");
>       DO_TEST_DIFFERENT("channel-virtio-auto");




More information about the libvir-list mailing list