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

Re: [libvirt] [PATCH] Added the attribute vendor_id to the cpu model



On 06/27/2012 09:14 AM, Michal Privoznik wrote:
> On 21.06.2012 16:58, Hendrik Schwartke wrote:
>> Introducing the attribute vendor_id to force the CPUID instruction
>> in a kvm guest to return the specified vendor.
>> ---

>> +      <optional>
>> +        <attribute name="vendor_id">
>> +          <data type="string">
>> +            <param name='pattern'>.{12}</param>

Rather than using '.', use [a-zA-Z...] to limit the RNG to the same set
of characters as the C code (I'm not sure what the official set is,
though).  I can understand rejecting a too-long name, but do we really
also want to reject a too-short name?


>> +        def->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
>> +
>> +           if(virXPathBoolean("boolean(./model[1]/@fallback)", ctxt)) {

Style: space after 'if'


>> +           }
>> +
>> +           if(virXPathBoolean("boolean(./model[1]/@vendor_id)", ctxt)) {
>> +               char *vendor_id;
>> +
>> +               vendor_id = virXPathString("string(./model[1]/@vendor_id)", ctxt);
>> +               if(!vendor_id || strlen(vendor_id)!=VIR_CPU_VENDOR_ID_LENGTH) {

two more instances.  Also, space on both sides of '!='


>> +               }
>> +               for(i=0; i<strlen(vendor_id); i++) {

space after 'for'

>> +                   if(!isprint(vendor_id[i]) || isspace(vendor_id[i])) {

and 'if'


>>  
>> +#define VIR_CPU_VENDOR_ID_LENGTH 12
>> +
> 
> Insert one space between '#' and define so this is indented properly.

'make syntax-check' with 'cppi' installed will flag this for you.


>> @@ -3924,6 +3926,8 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver,
>>                  goto cleanup;
>>  
>>              virBufferAdd(&buf, guest->model, -1);
>> +            if(guest->vendor_id)

space after 'if'

> 
> Otherwise looking good. Can somebody chime in and provide more light on
> allowed characters in vendor id?

Sorry, I can't help there.  But I think we've pointed out enough issues
to warrant a v2 submission.

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