9:58 a.m.
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 | 61 ++++++++++++++++++++++++++++++++---------
src/conf/cpu_conf.h | 3 ++
src/qemu/qemu_command.c | 6 +++-
tests/testutilsqemu.c | 1 +
5 files changed, 64 insertions(+), 14 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 46e539d..a246e8b 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..b8ca7a5 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -22,6 +22,7 @@
*/
#include <config.h>
+#include <ctype.h>
#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,19 +291,42 @@ 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);
+ 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(!vendor_id || strlen(vendor_id)!=VIR_CPU_VENDOR_ID_LENGTH) {
+ virCPUReportError(VIR_ERR_XML_ERROR, "%s",
+ _("vendor id must be 12 characters long"));
+ VIR_FREE(vendor_id);
+ goto error;
+ }
+ for(i=0; i<strlen(vendor_id); i++) {
+ if(!isprint(vendor_id[i]) || isspace(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 +614,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 +766,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..3daadbd 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..56ecefb 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;
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 */
--
1.7.9.5
5:27 a.m.
Hi,
last week I posted a patch to fake the vendor_id of the guest cpu.
I'm writing an application which depends on this feature so I'm
wondering if anyone has had a look at it?
Best regards
Hendrik
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.
---
docs/schemas/domaincommon.rng | 7 +++++
src/conf/cpu_conf.c | 61 ++++++++++++++++++++++++++++++++---------
src/conf/cpu_conf.h | 3 ++
src/qemu/qemu_command.c | 6 +++-
tests/testutilsqemu.c | 1 +
5 files changed, 64 insertions(+), 14 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 46e539d..a246e8b 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..b8ca7a5 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -22,6 +22,7 @@
*/
#include<config.h>
+#include<ctype.h>
#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,19 +291,42 @@ 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);
+ 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(!vendor_id || strlen(vendor_id)!=VIR_CPU_VENDOR_ID_LENGTH) {
+ virCPUReportError(VIR_ERR_XML_ERROR, "%s",
+ _("vendor id must be 12 characters long"));
+ VIR_FREE(vendor_id);
+ goto error;
+ }
+ for(i=0; i<strlen(vendor_id); i++) {
+ if(!isprint(vendor_id[i]) || isspace(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 +614,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 +766,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..3daadbd 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..56ecefb 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;
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 */
10:14 a.m.
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.
---
docs/schemas/domaincommon.rng | 7 +++++
src/conf/cpu_conf.c | 61 ++++++++++++++++++++++++++++++++---------
src/conf/cpu_conf.h | 3 ++
src/qemu/qemu_command.c | 6 +++-
tests/testutilsqemu.c | 1 +
5 files changed, 64 insertions(+), 14 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 46e539d..a246e8b 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..b8ca7a5 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -22,6 +22,7 @@
*/
#include <config.h>
+#include <ctype.h>
#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,19 +291,42 @@ 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);
+ 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(!vendor_id || strlen(vendor_id)!=VIR_CPU_VENDOR_ID_LENGTH) {
+ virCPUReportError(VIR_ERR_XML_ERROR, "%s",
+ _("vendor id must be 12 characters long"));
I'd reword this: virCPUReportError(VIR_ERR_XML_ERROR, _("vendor_id must
be exactly %d characters long"), VIR_CPU_VENDOR_ID_LENGTH);
I know vendor_id length is unlikely to change but nobody knows ...
+ VIR_FREE(vendor_id);
+ goto error;
+ }
+ for(i=0; i<strlen(vendor_id); i++) {
+ if(!isprint(vendor_id[i]) || isspace(vendor_id[i])) {
This should be c_isprint() and c_isspace(); however, I am not sure but I
think vendor id can contain spaces, doesn't it? Anyway, then you should
not #include <ctype.h> but #include "c-ctype.h"
+ virCPUReportError(VIR_ERR_XML_ERROR,
"%s",
+ _("vendor id is invalid"));
+ VIR_FREE(vendor_id);
+ goto error;
+ }
+ }
+ def->vendor_id = vendor_id;
+ }
}
}
@@ -588,6 +614,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 +766,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..3daadbd 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
+
Insert one space between '#' and define so this is indented properly.
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..56ecefb 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;
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 */
Otherwise looking good. Can somebody chime in and provide more light on
allowed characters in vendor id?
Michal
10:22 a.m.
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
11:09 a.m.
Thank you Michal and Eric for fast
On 27.06.2012 17:22, Eric Blake wrote:
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?
Ok, that right. I will fix the pattern. The
reason for rejecting
too-short names is that the CPUID instruction returns the vendor id in
EBX, ECX and EDX, thus the vendor id must be exactly 12 characters long.
So it would be necessary to pad short names in some way. I think the
best way to handle them is to reject them.
>> + 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.
Thanks a lot, that's a very good hint!
>> @@ -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?
Well, the Intel instruction manual doesn't
make any statement about the
allowed characters.
There are two reasons why I check for printable characters and spaces.
The first is that I think the CPUID instruction is intended that way.
The second is that I want to prevent problems calling qemu. It's for
example not possible to pass spaces in the vendor id to qemu.
Sorry, I can't help there. But I think we've pointed out
enough issues
to warrant a v2 submission.
I will rework the patch and post it tomorrow.
11:29 a.m.
On 27.06.2012 18:09, Hendrik Schwartke wrote:
Thank you Michal and Eric for fast
On 27.06.2012 17:22, Eric Blake wrote:
> 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?
Ok, that right. I will fix the pattern. The reason for rejecting
too-short names is that the CPUID instruction returns the vendor id in
EBX, ECX and EDX, thus the vendor id must be exactly 12 characters long.
So it would be necessary to pad short names in some way. I think the
best way to handle them is to reject them.
>
>>> + 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.
Thanks a lot, that's a very good hint!
>
>>> @@ -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?
Well, the Intel instruction manual doesn't make any statement about the
allowed characters.
There are two reasons why I check for printable characters and spaces.
The first is that I think the CPUID instruction is intended that way.
The second is that I want to prevent problems calling qemu. It's for
example not possible to pass spaces in the vendor id to qemu.
> Sorry, I can't help there. But I think we've pointed out enough issues
> to warrant a v2 submission.
>
I will rework the patch and post it tomorrow.
According to Wikipedia, it's possible to have spaces in vendor ID:
http://en.wikipedia.org/wiki/CPUID#EAX.3D0:_Get_vendor_ID
e.g. "VIA VIA VIA ". In that case we need to find a way of telling this
to qemu. What about simple argument closing into quotation marks?
Michal
5:20 a.m.
On 27.06.2012 18:29, Michal Privoznik wrote:
On 27.06.2012 18:09, Hendrik Schwartke wrote:
> Thank you Michal and Eric for fast
>
> On 27.06.2012 17:22, Eric Blake wrote:
>> 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?
> Ok, that right. I will fix the pattern. The reason for rejecting
> too-short names is that the CPUID instruction returns the vendor id in
> EBX, ECX and EDX, thus the vendor id must be exactly 12 characters long.
> So it would be necessary to pad short names in some way. I think the
> best way to handle them is to reject them.
>
>>>> + 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.
> Thanks a lot, that's a very good hint!
>>>> @@ -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?
> Well, the Intel instruction manual doesn't make any statement about the
> allowed characters.
> There are two reasons why I check for printable characters and spaces.
> The first is that I think the CPUID instruction is intended that way.
> The second is that I want to prevent problems calling qemu. It's for
> example not possible to pass spaces in the vendor id to qemu.
>
>> Sorry, I can't help there. But I think we've pointed out enough issues
>> to warrant a v2 submission.
>>
> I will rework the patch and post it tomorrow.
>
According to Wikipedia, it's possible to have spaces in vendor ID:
http://en.wikipedia.org/wiki/CPUID#EAX.3D0:_Get_vendor_ID
e.g. "VIA VIA VIA ". In that case we need to find a way of telling this
to qemu. What about simple argument closing into quotation marks?
Michal
Yes, you're right. I tested again. The reason for my problem with spaces
was the shell. I had a look at the qemu code and the only character
which is not usable is ','.
I will post the patch shortly.
Hendrik
5:21 a.m.
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>
#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);
+ 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 (!vendor_id
+ || strlen(vendor_id) != VIR_CPU_VENDOR_ID_LENGTH) {
+ virCPUReportError(VIR_ERR_XML_ERROR,
+ _("vendor_id must be exactly %d characters
long"),
+ 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;
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 */
--
1.7.9.5
5:06 a.m.
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.
#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?
+ 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.
+ if (!vendor_id
+ || strlen(vendor_id) != VIR_CPU_VENDOR_ID_LENGTH) {
And || operator needs to be on the preceding line.
+ virCPUReportError(VIR_ERR_XML_ERROR,
+ _("vendor_id must be exactly %d characters
long"),
Long line.
+ 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.
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
5:37 a.m.
On 03.07.2012 12:06, Michal Privoznik wrote:
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
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
4525
days inactive
4537
days old
9 comments
3 participants
participants (3)
-
Eric Blake -
Hendrik Schwartke -
Michal Privoznik