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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list