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