[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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.  tests/qemuxml2xmltest.c is a good
place to tweak to make sure we can round trip canonical XML.  For that,
I think you need a v3.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]