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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)