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

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."

> +<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().