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

On 11/14/2012 06:54 AM, Osier Yang wrote: > QEMU supports to set vendor and product strings for disk since s/to set/setting/ > 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. > --- > +++ b/docs/schemas/domaincommon.rng > @@ -905,6 +905,16 @@ > <ref name="wwn"/> > </element> > </optional> > +<optional> > +<element name="vendor"> > +<text/> We could use something stricter than<text/> so that RNG validation would reject the same names our code rejects, such as: <data type='string'> <param name='pattern'>[a-zA-Z0-9]{0,8}</param> </data>

> + } > + > + if (!virStrIsAlpha(vendor)) { > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("disk vendor is not alphanemuric string")); s/alphanemuric/alphanumeric/ > + 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 (!virStrIsAlpha(product)) { > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("disk product is not alphanemuric string")); and again > +++ b/src/libvirt_private.syms > @@ -1279,6 +1279,7 @@ virSetUIDGID; > virSkipSpaces; > virSkipSpacesAndBackslash; > virSkipSpacesBackwards; > +virStrIsAlpha; This name is misleading; I'd use virStrIsAlnum, to match the<ctype.h> difference between alpha and alnum.

Bigger problem - I see where you parse the new XML, but not where you generate it back out when formatting.

place to tweak to make sure we can round trip canonical XML. For that, I think you need a v3.