On 28.06.2012 12:21, Hendrik Schwartke wrote:
Introducing the attribute vendor_id to force the CPUID instruction in a kvm guest to return the specified vendor. --- docs/schemas/domaincommon.rng | 7 +++++ src/conf/cpu_conf.c | 64 +++++++++++++++++++++++++++++++++-------- src/conf/cpu_conf.h | 3 ++ src/qemu/qemu_command.c | 6 +++- tests/testutilsqemu.c | 1 + 5 files changed, 68 insertions(+), 13 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 46e539d..5c55269 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2820,6 +2820,13 @@ </choice> </attribute> </optional> +<optional> +<attribute name="vendor_id"> +<data type="string"> +<param name='pattern'>[^,]{12}</param> +</data> +</attribute> +</optional> <choice> <text/> <empty/> diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index b520f7c..b3098d8 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -22,6 +22,7 @@ */
#include<config.h> +#include<c-ctype.h> This is useless. Oops! I overlooked that.
#include "virterror_internal.h" #include "memory.h" @@ -68,6 +69,7 @@ virCPUDefFreeModel(virCPUDefPtr def)
VIR_FREE(def->model); VIR_FREE(def->vendor); + VIR_FREE(def->vendor_id);
for (i = 0; i< def->nfeatures; i++) VIR_FREE(def->features[i].name); @@ -104,6 +106,7 @@ virCPUDefCopyModel(virCPUDefPtr dst,
if ((src->model&& !(dst->model = strdup(src->model))) || (src->vendor&& !(dst->vendor = strdup(src->vendor))) + || (src->vendor_id&& !(dst->vendor_id = strdup(src->vendor_id))) || VIR_ALLOC_N(dst->features, src->nfeatures)< 0) goto no_memory; dst->nfeatures_max = dst->nfeatures = src->nfeatures; @@ -288,18 +291,46 @@ virCPUDefParseXML(const xmlNodePtr node, }
if (def->type == VIR_CPU_TYPE_GUEST&& - def->mode != VIR_CPU_MODE_HOST_PASSTHROUGH&& - virXPathBoolean("boolean(./model[1]/@fallback)", ctxt)) { - const char *fallback; - - fallback = virXPathString("string(./model[1]/@fallback)", ctxt); - if (fallback) { - def->fallback = virCPUFallbackTypeFromString(fallback); - VIR_FREE(fallback); - if (def->fallback< 0) { - virCPUReportError(VIR_ERR_XML_ERROR, "%s", - _("Invalid fallback attribute")); - goto error; + def->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) { + + if (virXPathBoolean("boolean(./model[1]/@fallback)", ctxt)) { + const char *fallback; + + fallback = + virXPathString("string(./model[1]/@fallback)", ctxt); Why this change? Well, Eric suggested to use 'make syntax-check'. And it recommended to use two lines. + if (fallback) { + def->fallback = virCPUFallbackTypeFromString(fallback); + VIR_FREE(fallback); + if (def->fallback< 0) { + virCPUReportError(VIR_ERR_XML_ERROR, "%s", + _("Invalid fallback attribute")); + goto error; + } + } + + if (virXPathBoolean("boolean(./model[1]/@vendor_id)", ctxt)) { + char *vendor_id; + + vendor_id = + virXPathString("string(./model[1]/@vendor_id)", ctxt); If we are dealing with long lines we tend to split them rather at ',' than this. I think make syntax-check didn't like the long line here. + if (!vendor_id + || strlen(vendor_id) != VIR_CPU_VENDOR_ID_LENGTH) { And || operator needs to be on the preceding line. Again the syntax check recommended something else. There are also a lot of other places in the code where operators are at the beginning of a
On 03.07.2012 12:06, Michal Privoznik wrote: line, see above. (IMHO it's easier to read.)
+ virCPUReportError(VIR_ERR_XML_ERROR, + _("vendor_id must be exactly %d characters long"), Long line. Yep, your're right + VIR_CPU_VENDOR_ID_LENGTH); + VIR_FREE(vendor_id); + goto error; + } + /* ensure that the string can be passed to qemu*/ + for (i = 0; i< strlen(vendor_id); i++) { + if (vendor_id[i]==',') { + virCPUReportError(VIR_ERR_XML_ERROR, "%s", + _("vendor id is invalid")); + VIR_FREE(vendor_id); + goto error; + } + } + def->vendor_id = vendor_id; } } } @@ -588,6 +619,8 @@ virCPUDefFormatBuf(virBufferPtr buf, return -1; } virBufferAsprintf(buf, " fallback='%s'", fallback); + if (def->vendor_id) + virBufferAsprintf(buf, " vendor_id='%s'", def->vendor_id); } if (formatModel&& def->model) { virBufferAsprintf(buf, ">%s</model>\n", def->model); @@ -738,6 +771,13 @@ virCPUDefIsEqual(virCPUDefPtr src, goto cleanup; }
+ if (STRNEQ_NULLABLE(src->vendor_id, dst->vendor_id)) { + virCPUReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target CPU model %s does not match source %s"), + NULLSTR(dst->vendor_id), NULLSTR(src->vendor_id)); + goto cleanup; + } + if (src->sockets != dst->sockets) { virCPUReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target CPU sockets %d does not match source %d"), diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index f8b7bf9..2df0a50 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -28,6 +28,8 @@ # include "buf.h" # include "xml.h"
+# define VIR_CPU_VENDOR_ID_LENGTH 12 + enum virCPUType { VIR_CPU_TYPE_HOST, VIR_CPU_TYPE_GUEST, @@ -103,6 +105,7 @@ struct _virCPUDef { int match; /* enum virCPUMatch */ char *arch; char *model; + char *vendor_id; /* vendor id returned by CPUID in the guest */ int fallback; /* enum virCPUFallback */ char *vendor; unsigned int sockets; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bd4f96a..d8d0220 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3910,7 +3910,9 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, } virBufferAddLit(&buf, "host"); } else { - if (VIR_ALLOC(guest)< 0 || !(guest->arch = strdup(host->arch))) + if (VIR_ALLOC(guest)< 0 + || !(guest->arch = strdup(host->arch)) + || (cpu->vendor_id&& !(guest->vendor_id = strdup(cpu->vendor_id)))) goto no_memory; Again operators on the begining of line. Ok, get that.
if (cpu->match == VIR_CPU_MATCH_MINIMUM) @@ -3924,6 +3926,8 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, goto cleanup;
virBufferAdd(&buf, guest->model, -1); + if (guest->vendor_id) + virBufferAsprintf(&buf, ",vendor=%s", guest->vendor_id); for (i = 0; i< guest->nfeatures; i++) { char sign; if (guest->features[i].policy == VIR_CPU_FEATURE_DISABLE) diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 8d5a3bf..8b7cb33 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -116,6 +116,7 @@ virCapsPtr testQemuCapsInit(void) { 0, /* match */ (char *) "x86_64", /* arch */ (char *) "core2duo", /* model */ + NULL, /* vendor_id */ 0, /* fallback */ (char *) "Intel", /* vendor */ 1, /* sockets */
So squashing this in:
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index b3098d8..7fe3c1e 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -22,7 +22,6 @@ */
#include<config.h> -#include<c-ctype.h>
#include "virterror_internal.h" #include "memory.h" @@ -296,8 +295,7 @@ virCPUDefParseXML(const xmlNodePtr node, if (virXPathBoolean("boolean(./model[1]/@fallback)", ctxt)) { const char *fallback;
- fallback = - virXPathString("string(./model[1]/@fallback)", ctxt); + fallback = virXPathString("string(./model[1]/@fallback)", ctxt); if (fallback) { def->fallback = virCPUFallbackTypeFromString(fallback); VIR_FREE(fallback); @@ -311,12 +309,13 @@ virCPUDefParseXML(const xmlNodePtr node, 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) { + vendor_id = virXPathString("string(./model[1]/@vendor_id)", + ctxt); + if (!vendor_id || + strlen(vendor_id) != VIR_CPU_VENDOR_ID_LENGTH) { virCPUReportError(VIR_ERR_XML_ERROR, - _("vendor_id must be exactly %d characters long"), + _("vendor_id must be exactly" + " %d characters long"), VIR_CPU_VENDOR_ID_LENGTH); VIR_FREE(vendor_id); goto error; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0b5d894..528b189 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3913,9 +3913,9 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, } virBufferAddLit(&buf, "host"); } else { - if (VIR_ALLOC(guest)< 0 - || !(guest->arch = strdup(host->arch)) - || (cpu->vendor_id&& !(guest->vendor_id = strdup(cpu->vendor_id)))) + if (VIR_ALLOC(guest)< 0 || + !(guest->arch = strdup(host->arch)) || + (cpu->vendor_id&& !(guest->vendor_id = strdup(cpu->vendor_id)))) goto no_memory;
if (cpu->match == VIR_CPU_MATCH_MINIMUM)
And pushed now.
Michal Thanks a lot Michal!
Hendrik