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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list