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

Osier Yang jyang at redhat.com
Wed Nov 7 15:24:43 UTC 2012


On 2012年11月05日 21:34, Martin Kletzander wrote:
> On 11/05/2012 08:04 AM, Osier Yang wrote:
>> QEMU supports to set 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.
>> ---
>>   docs/formatdomain.html.in                          |   10 +++++
>>   docs/schemas/domaincommon.rng                      |   10 +++++
>>   src/conf/domain_conf.c                             |   30 ++++++++++++++++
>>   src/conf/domain_conf.h                             |    2 +
>>   src/qemu/qemu_command.c                            |   29 ++++++++++++++++
>>   .../qemuxml2argv-disk-scsi-disk-vpd.args           |   13 +++++++
>>   .../qemuxml2argv-disk-scsi-disk-vpd.xml            |   36 ++++++++++++++++++++
>>   tests/qemuxml2argvtest.c                           |    4 ++
>>   8 files changed, 134 insertions(+), 0 deletions(-)
>>   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 c8da33d..cc9e871 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -1657,6 +1657,16 @@
>>           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's a not more than 8 bytes alphanumeric string.
>
> Last sentence doesn't make sense, I suggest changing it to either: "It
> can't be longer than 8 alphanumeric characters." or simply "Maximum 8
> alphanumeric characters."

Okay, honestly, I wasn't comfortable with the sentence, but can't
get a better one at that time, :-) I will change your advise a bit:

It must not be longer than 8 alphanumeric 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's a not more than 16 bytes alphanumeric string.
>
> dtto
>
>> +<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 2beb035..ed7d1d0 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -905,6 +905,16 @@
>>             <ref name="wwn"/>
>>           </element>
>>         </optional>
>> +<optional>
>> +<element name="vendor">
>> +<text/>
>> +</element>
>> +</optional>
>> +<optional>
>> +<element name="product">
>> +<text/>
>> +</element>
>> +</optional>
>
> This is little bit different than the other specifications around in the
> code and could be made better, but looking at the schema I've found more
> inconsistencies, so it's ok for now.  I'll send a cleanup patch later
> for these and the others as well.
>
>>       </interleave>
>>     </define>
>>     <define name="snapshot">
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 0575fcd..db6608e 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);
>> @@ -3498,6 +3500,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
>> @@ -3550,6 +3554,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();
>> @@ -3888,6 +3894,24 @@ 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 bytes string"));
>
> This should be "disk vendor name is more than 8 characters long" or "..
> is longer than 8 characters", also there is no check these are
> alphanumeric although it is mentioned in the documentation.  I believe
> we can use something similar to virValidateWWN().

Okay, since I already tried to validate the strings, further checking
makes sense.

Sorry for the late response, will send v2 soon.

Regards,
Osier




More information about the libvir-list mailing list