[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