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

[Osier Yang]

On 2012年11月15日 10:01, Eric Blake wrote:
> 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>

Agreed.

>
>> +                }
>> +
>> +                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.

Agreed.

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

Oops, I missed that when rebasing.

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

Okay.