--- docs/schemas/domaincommon.rng | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 10b017f..14b6700 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4119,17 +4119,17 @@ <interleave> <optional> <element name="relaxed"> - <ref name="hypervtristate"/> + <ref name="featurestate"/> </element> </optional> <optional> <element name="vapic"> - <ref name="hypervtristate"/> + <ref name="featurestate"/> </element> </optional> <optional> <element name="spinlocks"> - <ref name="hypervtristate"/> + <ref name="featurestate"/> <optional> <attribute name="retries"> <data type="unsignedInt"/> @@ -4141,7 +4141,7 @@ </element> </define> - <define name="hypervtristate"> + <define name="featurestate"> <attribute name="state"> <choice> <value>on</value> -- 1.8.3.2

--- src/conf/domain_conf.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6bf2b30..365de77 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11408,8 +11408,7 @@ virDomainDefParseXML(xmlDocPtr xml, int val = virDomainFeatureTypeFromString((const char *)nodes[i]->name); if (val < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected feature %s"), - nodes[i]->name); + _("unexpected feature '%s'"), nodes[i]->name); goto error; } def->features |= (1 << val); @@ -11419,7 +11418,7 @@ virDomainDefParseXML(xmlDocPtr xml, int eoi; if ((eoi = virDomainFeatureStateTypeFromString(tmp)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown value for attribute eoi: %s"), + _("unknown value for attribute eoi: '%s'"), tmp); goto error; } -- 1.8.3.2

--- src/qemu/qemu_command.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9f0e3da..abb62e9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -11125,7 +11125,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps, #define WANT_VALUE() \ const char *val = progargv[++i]; \ if (!val) { \ - virReportError(VIR_ERR_INTERNAL_ERROR, \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ _("missing value for %s argument"), arg); \ goto error; \ } -- 1.8.3.2

From: Jiri Denemark <jdenemar(a)redhat.com&gt; This patch adds cpuDataFormat and cpuDataParse APIs to be used in unit tests for testing APIs that deal with virCPUData. In the x86 world, this means we can now store/load arbitrary CPUID data in the test suite to check correctness of CPU related APIs that could not be tested before. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/cpu/cpu.c | 41 ++++++++++++++ src/cpu/cpu.h | 14 +++++ src/cpu/cpu_x86.c | 135 +++++++++++++++++++++++++++++++++++++++-------- src/libvirt_private.syms | 2 + 4 files changed, 171 insertions(+), 21 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 9eb7df5..548edc7 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -442,6 +442,47 @@ cpuHasFeature(const virCPUData *data, return driver->hasFeature(data, feature); } +char * +cpuDataFormat(const virCPUData *data) +{ + struct cpuArchDriver *driver; + + VIR_DEBUG("data=%p", data); + + if (!(driver = cpuGetSubDriver(data->arch))) + return NULL; + + if (!driver->dataFormat) { + virReportError(VIR_ERR_NO_SUPPORT, + _("cannot format %s CPU data"), + virArchToString(data->arch)); + return NULL; + } + + return driver->dataFormat(data); +} + +virCPUDataPtr +cpuDataParse(virArch arch, + const char *xmlStr) +{ + struct cpuArchDriver *driver; + + VIR_DEBUG("arch=%s, xmlStr=%s", virArchToString(arch), xmlStr); + + if (!(driver = cpuGetSubDriver(arch))) + return NULL; + + if (!driver->dataParse) { + virReportError(VIR_ERR_NO_SUPPORT, + _("cannot parse %s CPU data"), + virArchToString(arch)); + return NULL; + } + + return driver->dataParse(xmlStr); +} + bool cpuModelIsAllowed(const char *model, const char **models, diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 967b481..85b768b 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -93,6 +93,11 @@ typedef int (*cpuArchHasFeature) (const virCPUData *data, const char *feature); +typedef char * +(*cpuArchDataFormat)(const virCPUData *data); + +typedef virCPUDataPtr +(*cpuArchDataParse) (const char *xmlStr); struct cpuArchDriver { const char *name; @@ -107,6 +112,8 @@ struct cpuArchDriver { cpuArchBaseline baseline; cpuArchUpdate update; cpuArchHasFeature hasFeature; + cpuArchDataFormat dataFormat; + cpuArchDataParse dataParse; }; @@ -178,4 +185,11 @@ cpuModelIsAllowed(const char *model, extern int cpuGetModels(const char *arch, char ***models); +/* cpuDataFormat and cpuDataParse are implemented for unit tests only and + * have no real-life usage + */ +char *cpuDataFormat(const virCPUData *data); +virCPUDataPtr cpuDataParse(virArch arch, + const char *xmlStr); + #endif /* __VIR_CPU_H__ */ diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 7486757..ba21bf1 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -666,12 +666,42 @@ x86FeatureNames(const struct x86_map *map, static int +x86ParseCPUID(xmlXPathContextPtr ctxt, + struct cpuX86cpuid *cpuid) +{ + unsigned long fun, eax, ebx, ecx, edx; + int ret_fun, ret_eax, ret_ebx, ret_ecx, ret_edx; + + memset(cpuid, 0, sizeof(*cpuid)); + + fun = eax = ebx = ecx = edx = 0; + ret_fun = virXPathULongHex("string(@function)", ctxt, &fun); + ret_eax = virXPathULongHex("string(@eax)", ctxt, &eax); + ret_ebx = virXPathULongHex("string(@ebx)", ctxt, &ebx); + ret_ecx = virXPathULongHex("string(@ecx)", ctxt, &ecx); + ret_edx = virXPathULongHex("string(@edx)", ctxt, &edx); + + if (ret_fun < 0 || ret_eax == -2 || ret_ebx == -2 + || ret_ecx == -2 || ret_edx == -2) + return -1; + + cpuid->function = fun; + cpuid->eax = eax; + cpuid->ebx = ebx; + cpuid->ecx = ecx; + cpuid->edx = edx; + return 0; +} + + +static int x86FeatureLoad(xmlXPathContextPtr ctxt, struct x86_map *map) { xmlNodePtr *nodes = NULL; xmlNodePtr ctxt_node = ctxt->node; struct x86_feature *feature; + struct cpuX86cpuid cpuid; int ret = 0; size_t i; int n; @@ -697,31 +727,13 @@ x86FeatureLoad(xmlXPathContextPtr ctxt, goto ignore; for (i = 0; i < n; i++) { - struct cpuX86cpuid cpuid; - unsigned long fun, eax, ebx, ecx, edx; - int ret_fun, ret_eax, ret_ebx, ret_ecx, ret_edx; - ctxt->node = nodes[i]; - fun = eax = ebx = ecx = edx = 0; - ret_fun = virXPathULongHex("string(@function)", ctxt, &fun); - ret_eax = virXPathULongHex("string(@eax)", ctxt, &eax); - ret_ebx = virXPathULongHex("string(@ebx)", ctxt, &ebx); - ret_ecx = virXPathULongHex("string(@ecx)", ctxt, &ecx); - ret_edx = virXPathULongHex("string(@edx)", ctxt, &edx); - - if (ret_fun < 0 || ret_eax == -2 || ret_ebx == -2 - || ret_ecx == -2 || ret_edx == -2) { + if (x86ParseCPUID(ctxt, &cpuid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid cpuid[%zu] in %s feature"), i, feature->name); + _("Invalid cpuid[%zu] in %s feature"), + i, feature->name); goto ignore; } - - cpuid.function = fun; - cpuid.eax = eax; - cpuid.ebx = ebx; - cpuid.ecx = ecx; - cpuid.edx = edx; - if (x86DataAddCpuid(feature->data, &cpuid)) goto error; } @@ -1124,6 +1136,85 @@ error: } +static char * +x86CPUDataFormat(const virCPUData *data) +{ + struct data_iterator iter = DATA_ITERATOR_INIT(data->data.x86); + struct cpuX86cpuid *cpuid; + virBuffer buf = VIR_BUFFER_INITIALIZER; + + virBufferAddLit(&buf, "<cpudata arch='x86'>\n"); + while ((cpuid = x86DataCpuidNext(&iter))) { + virBufferAsprintf(&buf, + " <cpuid function='0x%08x'" + " eax='0x%08x' ebx='0x%08x'" + " ecx='0x%08x' edx='0x%08x'/>\n", + cpuid->function, + cpuid->eax, cpuid->ebx, cpuid->ecx, cpuid->edx); + } + virBufferAddLit(&buf, "</cpudata>\n"); + + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + virReportOOMError(); + return NULL; + } + + return virBufferContentAndReset(&buf); +} + + +static virCPUDataPtr +x86CPUDataParse(const char *xmlStr) +{ + xmlDocPtr xml = NULL; + xmlXPathContextPtr ctxt = NULL; + xmlNodePtr *nodes = NULL; + virCPUDataPtr cpuData = NULL; + struct cpuX86Data *data = NULL; + struct cpuX86cpuid cpuid; + size_t i; + int n; + + if (VIR_ALLOC(data) < 0) + goto cleanup; + + if (!(xml = virXMLParseStringCtxt(xmlStr, _("CPU data"), &ctxt))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot parse CPU data")); + goto cleanup; + } + ctxt->node = xmlDocGetRootElement(xml); + + n = virXPathNodeSet("/cpudata[@arch='x86']/data", ctxt, &nodes); + if (n < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("no x86 CPU data found")); + goto cleanup; + } + + for (i = 0; i < n; i++) { + ctxt->node = nodes[i]; + if (x86ParseCPUID(ctxt, &cpuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse cpuid[%zu]"), i); + goto cleanup; + } + if (x86DataAddCpuid(data, &cpuid) < 0) + goto cleanup; + } + + cpuData = x86MakeCPUData(VIR_ARCH_X86_64, &data); + +cleanup: + VIR_FREE(nodes); + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xml); + x86DataFree(data); + return cpuData; +} + + /* A helper macro to exit the cpu computation function without writing * redundant code: * MSG: error message @@ -1956,4 +2047,6 @@ struct cpuArchDriver cpuDriverX86 = { .baseline = x86Baseline, .update = x86Update, .hasFeature = x86HasFeature, + .dataFormat = x86CPUDataFormat, + .dataParse = x86CPUDataParse, }; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7ee538c..84c1c28 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -720,7 +720,9 @@ cpuBaseline; cpuBaselineXML; cpuCompare; cpuCompareXML; +cpuDataFormat; cpuDataFree; +cpuDataParse; cpuDecode; cpuEncode; cpuGetModels; -- 1.8.3.2

From: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/cpu/cpu_x86.c | 64 +++++++++++++++++++++++++------------------------- src/cpu/cpu_x86_data.h | 7 +++--- 2 files changed, 36 insertions(+), 35 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index ba21bf1..4c1e745 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -38,13 +38,13 @@ #define VENDOR_STRING_LENGTH 12 -static const struct cpuX86cpuid cpuidNull = { 0, 0, 0, 0, 0 }; +static const virCPUx86CPUID cpuidNull = { 0, 0, 0, 0, 0 }; static const virArch archs[] = { VIR_ARCH_I686, VIR_ARCH_X86_64 }; struct x86_vendor { char *name; - struct cpuX86cpuid cpuid; + virCPUx86CPUID cpuid; struct x86_vendor *next; }; @@ -91,8 +91,8 @@ struct data_iterator { static int -x86cpuidMatch(const struct cpuX86cpuid *cpuid1, - const struct cpuX86cpuid *cpuid2) +x86cpuidMatch(const virCPUx86CPUID *cpuid1, + const virCPUx86CPUID *cpuid2) { return (cpuid1->eax == cpuid2->eax && cpuid1->ebx == cpuid2->ebx && @@ -102,8 +102,8 @@ x86cpuidMatch(const struct cpuX86cpuid *cpuid1, static int -x86cpuidMatchMasked(const struct cpuX86cpuid *cpuid, - const struct cpuX86cpuid *mask) +x86cpuidMatchMasked(const virCPUx86CPUID *cpuid, + const virCPUx86CPUID *mask) { return ((cpuid->eax & mask->eax) == mask->eax && (cpuid->ebx & mask->ebx) == mask->ebx && @@ -113,8 +113,8 @@ x86cpuidMatchMasked(const struct cpuX86cpuid *cpuid, static void -x86cpuidSetBits(struct cpuX86cpuid *cpuid, - const struct cpuX86cpuid *mask) +x86cpuidSetBits(virCPUx86CPUID *cpuid, + const virCPUx86CPUID *mask) { cpuid->eax |= mask->eax; cpuid->ebx |= mask->ebx; @@ -124,8 +124,8 @@ x86cpuidSetBits(struct cpuX86cpuid *cpuid, static void -x86cpuidClearBits(struct cpuX86cpuid *cpuid, - const struct cpuX86cpuid *mask) +x86cpuidClearBits(virCPUx86CPUID *cpuid, + const virCPUx86CPUID *mask) { cpuid->eax &= ~mask->eax; cpuid->ebx &= ~mask->ebx; @@ -135,8 +135,8 @@ x86cpuidClearBits(struct cpuX86cpuid *cpuid, static void -x86cpuidAndBits(struct cpuX86cpuid *cpuid, - const struct cpuX86cpuid *mask) +x86cpuidAndBits(virCPUx86CPUID *cpuid, + const virCPUx86CPUID *mask) { cpuid->eax &= mask->eax; cpuid->ebx &= mask->ebx; @@ -146,10 +146,10 @@ x86cpuidAndBits(struct cpuX86cpuid *cpuid, /* skips all zero CPUID leafs */ -static struct cpuX86cpuid * +static virCPUx86CPUID * x86DataCpuidNext(struct data_iterator *iterator) { - struct cpuX86cpuid *ret; + virCPUx86CPUID *ret; struct cpuX86Data *data = iterator->data; if (!data) @@ -177,11 +177,11 @@ x86DataCpuidNext(struct data_iterator *iterator) } -static struct cpuX86cpuid * +static virCPUx86CPUID * x86DataCpuid(const struct cpuX86Data *data, uint32_t function) { - struct cpuX86cpuid *cpuids; + virCPUx86CPUID *cpuids; int len; size_t i; @@ -297,11 +297,11 @@ x86DataExpand(struct cpuX86Data *data, static int x86DataAddCpuid(struct cpuX86Data *data, - const struct cpuX86cpuid *cpuid) + const virCPUx86CPUID *cpuid) { unsigned int basic_by = 0; unsigned int extended_by = 0; - struct cpuX86cpuid **cpuids; + virCPUx86CPUID **cpuids; unsigned int pos; if (cpuid->function < CPUX86_EXTENDED) { @@ -374,8 +374,8 @@ x86DataIntersect(struct cpuX86Data *data1, const struct cpuX86Data *data2) { struct data_iterator iter = DATA_ITERATOR_INIT(data1); - struct cpuX86cpuid *cpuid1; - struct cpuX86cpuid *cpuid2; + virCPUx86CPUID *cpuid1; + virCPUx86CPUID *cpuid2; while ((cpuid1 = x86DataCpuidNext(&iter))) { cpuid2 = x86DataCpuid(data2, cpuid1->function); @@ -402,8 +402,8 @@ x86DataIsSubset(const struct cpuX86Data *data, { struct data_iterator iter = DATA_ITERATOR_INIT((struct cpuX86Data *)subset); - const struct cpuX86cpuid *cpuid; - const struct cpuX86cpuid *cpuidSubset; + const virCPUx86CPUID *cpuid; + const virCPUx86CPUID *cpuidSubset; while ((cpuidSubset = x86DataCpuidNext(&iter))) { if (!(cpuid = x86DataCpuid(data, cpuidSubset->function)) || @@ -443,7 +443,7 @@ x86DataToVendor(struct cpuX86Data *data, const struct x86_map *map) { const struct x86_vendor *vendor = map->vendors; - struct cpuX86cpuid *cpuid; + virCPUx86CPUID *cpuid; while (vendor) { if ((cpuid = x86DataCpuid(data, vendor->cpuid.function)) && @@ -667,7 +667,7 @@ x86FeatureNames(const struct x86_map *map, static int x86ParseCPUID(xmlXPathContextPtr ctxt, - struct cpuX86cpuid *cpuid) + virCPUx86CPUID *cpuid) { unsigned long fun, eax, ebx, ecx, edx; int ret_fun, ret_eax, ret_ebx, ret_ecx, ret_edx; @@ -701,7 +701,7 @@ x86FeatureLoad(xmlXPathContextPtr ctxt, xmlNodePtr *nodes = NULL; xmlNodePtr ctxt_node = ctxt->node; struct x86_feature *feature; - struct cpuX86cpuid cpuid; + virCPUx86CPUID cpuid; int ret = 0; size_t i; int n; @@ -914,8 +914,8 @@ x86ModelCompare(const struct x86_model *model1, enum compare_result result = EQUAL; struct data_iterator iter1 = DATA_ITERATOR_INIT(model1->data); struct data_iterator iter2 = DATA_ITERATOR_INIT(model2->data); - struct cpuX86cpuid *cpuid1; - struct cpuX86cpuid *cpuid2; + virCPUx86CPUID *cpuid1; + virCPUx86CPUID *cpuid2; while ((cpuid1 = x86DataCpuidNext(&iter1))) { enum compare_result match = SUPERSET; @@ -1140,7 +1140,7 @@ static char * x86CPUDataFormat(const virCPUData *data) { struct data_iterator iter = DATA_ITERATOR_INIT(data->data.x86); - struct cpuX86cpuid *cpuid; + virCPUx86CPUID *cpuid; virBuffer buf = VIR_BUFFER_INITIALIZER; virBufferAddLit(&buf, "<cpudata arch='x86'>\n"); @@ -1172,7 +1172,7 @@ x86CPUDataParse(const char *xmlStr) xmlNodePtr *nodes = NULL; virCPUDataPtr cpuData = NULL; struct cpuX86Data *data = NULL; - struct cpuX86cpuid cpuid; + virCPUx86CPUID cpuid; size_t i; int n; @@ -1709,7 +1709,7 @@ error: #if HAVE_CPUID static inline void -cpuidCall(struct cpuX86cpuid *cpuid) +cpuidCall(virCPUx86CPUID *cpuid) { # if __x86_64__ asm("xor %%ebx, %%ebx;" /* clear the other registers as some cpuid */ @@ -1743,11 +1743,11 @@ cpuidCall(struct cpuX86cpuid *cpuid) static int -cpuidSet(uint32_t base, struct cpuX86cpuid **set) +cpuidSet(uint32_t base, virCPUx86CPUID **set) { uint32_t max; uint32_t i; - struct cpuX86cpuid cpuid = { base, 0, 0, 0, 0 }; + virCPUx86CPUID cpuid = { base, 0, 0, 0, 0 }; cpuidCall(&cpuid); max = cpuid.eax - base; diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h index dc972a6..acb7c32 100644 --- a/src/cpu/cpu_x86_data.h +++ b/src/cpu/cpu_x86_data.h @@ -26,7 +26,8 @@ # include <stdint.h> -struct cpuX86cpuid { +typedef struct _virCPUx86CPUID virCPUx86CPUID; +struct _virCPUx86CPUID { uint32_t function; uint32_t eax; uint32_t ebx; @@ -39,9 +40,9 @@ struct cpuX86cpuid { struct cpuX86Data { size_t basic_len; - struct cpuX86cpuid *basic; + virCPUx86CPUID *basic; size_t extended_len; - struct cpuX86cpuid *extended; + virCPUx86CPUID *extended; }; #endif /* __VIR_CPU_X86_DATA_H__ */ -- 1.8.3.2

From: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/cpu/cpu.h | 2 +- src/cpu/cpu_x86.c | 82 +++++++++++++++++++++++++------------------------- src/cpu/cpu_x86_data.h | 3 +- 3 files changed, 44 insertions(+), 43 deletions(-) diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 85b768b..27169fe 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -37,7 +37,7 @@ typedef virCPUData *virCPUDataPtr; struct _virCPUData { virArch arch; union { - struct cpuX86Data *x86; + virCPUx86Data *x86; struct cpuPPCData ppc; /* generic driver needs no data */ } data; diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 4c1e745..b8b9a07 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -51,7 +51,7 @@ struct x86_vendor { struct x86_feature { char *name; - struct cpuX86Data *data; + virCPUx86Data *data; struct x86_feature *next; }; @@ -59,7 +59,7 @@ struct x86_feature { struct x86_model { char *name; const struct x86_vendor *vendor; - struct cpuX86Data *data; + virCPUx86Data *data; struct x86_model *next; }; @@ -80,7 +80,7 @@ enum compare_result { struct data_iterator { - struct cpuX86Data *data; + virCPUx86Data *data; int pos; bool extended; }; @@ -150,7 +150,7 @@ static virCPUx86CPUID * x86DataCpuidNext(struct data_iterator *iterator) { virCPUx86CPUID *ret; - struct cpuX86Data *data = iterator->data; + virCPUx86Data *data = iterator->data; if (!data) return NULL; @@ -178,7 +178,7 @@ x86DataCpuidNext(struct data_iterator *iterator) static virCPUx86CPUID * -x86DataCpuid(const struct cpuX86Data *data, +x86DataCpuid(const virCPUx86Data *data, uint32_t function) { virCPUx86CPUID *cpuids; @@ -204,7 +204,7 @@ x86DataCpuid(const struct cpuX86Data *data, static void -x86DataFree(struct cpuX86Data *data) +x86DataFree(virCPUx86Data *data) { if (data == NULL) return; @@ -216,7 +216,7 @@ x86DataFree(struct cpuX86Data *data) static virCPUDataPtr -x86MakeCPUData(virArch arch, struct cpuX86Data **data) +x86MakeCPUData(virArch arch, virCPUx86Data **data) { virCPUDataPtr cpuData; @@ -241,10 +241,10 @@ x86FreeCPUData(virCPUDataPtr data) } -static struct cpuX86Data * -x86DataCopy(const struct cpuX86Data *data) +static virCPUx86Data * +x86DataCopy(const virCPUx86Data *data) { - struct cpuX86Data *copy = NULL; + virCPUx86Data *copy = NULL; size_t i; if (VIR_ALLOC(copy) < 0 @@ -267,7 +267,7 @@ x86DataCopy(const struct cpuX86Data *data) static int -x86DataExpand(struct cpuX86Data *data, +x86DataExpand(virCPUx86Data *data, int basic_by, int extended_by) { @@ -296,7 +296,7 @@ x86DataExpand(struct cpuX86Data *data, static int -x86DataAddCpuid(struct cpuX86Data *data, +x86DataAddCpuid(virCPUx86Data *data, const virCPUx86CPUID *cpuid) { unsigned int basic_by = 0; @@ -324,8 +324,8 @@ x86DataAddCpuid(struct cpuX86Data *data, static int -x86DataAdd(struct cpuX86Data *data1, - const struct cpuX86Data *data2) +x86DataAdd(virCPUx86Data *data1, + const virCPUx86Data *data2) { size_t i; @@ -349,8 +349,8 @@ x86DataAdd(struct cpuX86Data *data1, static void -x86DataSubtract(struct cpuX86Data *data1, - const struct cpuX86Data *data2) +x86DataSubtract(virCPUx86Data *data1, + const virCPUx86Data *data2) { size_t i; unsigned int len; @@ -370,8 +370,8 @@ x86DataSubtract(struct cpuX86Data *data1, static void -x86DataIntersect(struct cpuX86Data *data1, - const struct cpuX86Data *data2) +x86DataIntersect(virCPUx86Data *data1, + const virCPUx86Data *data2) { struct data_iterator iter = DATA_ITERATOR_INIT(data1); virCPUx86CPUID *cpuid1; @@ -388,7 +388,7 @@ x86DataIntersect(struct cpuX86Data *data1, static bool -x86DataIsEmpty(struct cpuX86Data *data) +x86DataIsEmpty(virCPUx86Data *data) { struct data_iterator iter = DATA_ITERATOR_INIT(data); @@ -397,11 +397,11 @@ x86DataIsEmpty(struct cpuX86Data *data) static bool -x86DataIsSubset(const struct cpuX86Data *data, - const struct cpuX86Data *subset) +x86DataIsSubset(const virCPUx86Data *data, + const virCPUx86Data *subset) { - struct data_iterator iter = DATA_ITERATOR_INIT((struct cpuX86Data *)subset); + struct data_iterator iter = DATA_ITERATOR_INIT((virCPUx86Data *)subset); const virCPUx86CPUID *cpuid; const virCPUx86CPUID *cpuidSubset; @@ -419,7 +419,7 @@ x86DataIsSubset(const struct cpuX86Data *data, static int x86DataToCPUFeatures(virCPUDefPtr cpu, int policy, - struct cpuX86Data *data, + virCPUx86Data *data, const struct x86_map *map) { const struct x86_feature *feature = map->features; @@ -439,7 +439,7 @@ x86DataToCPUFeatures(virCPUDefPtr cpu, /* also removes bits corresponding to vendor string from data */ static const struct x86_vendor * -x86DataToVendor(struct cpuX86Data *data, +x86DataToVendor(virCPUx86Data *data, const struct x86_map *map) { const struct x86_vendor *vendor = map->vendors; @@ -459,13 +459,13 @@ x86DataToVendor(struct cpuX86Data *data, static virCPUDefPtr -x86DataToCPU(const struct cpuX86Data *data, +x86DataToCPU(const virCPUx86Data *data, const struct x86_model *model, const struct x86_map *map) { virCPUDefPtr cpu; - struct cpuX86Data *copy = NULL; - struct cpuX86Data *modelData = NULL; + virCPUx86Data *copy = NULL; + virCPUx86Data *modelData = NULL; const struct x86_vendor *vendor; if (VIR_ALLOC(cpu) < 0 || @@ -640,7 +640,7 @@ x86FeatureFind(const struct x86_map *map, static char * x86FeatureNames(const struct x86_map *map, const char *separator, - struct cpuX86Data *data) + virCPUx86Data *data) { virBuffer ret = VIR_BUFFER_INITIALIZER; bool first = true; @@ -1171,7 +1171,7 @@ x86CPUDataParse(const char *xmlStr) xmlXPathContextPtr ctxt = NULL; xmlNodePtr *nodes = NULL; virCPUDataPtr cpuData = NULL; - struct cpuX86Data *data = NULL; + virCPUx86Data *data = NULL; virCPUx86CPUID cpuid; size_t i; int n; @@ -1218,7 +1218,7 @@ cleanup: /* A helper macro to exit the cpu computation function without writing * redundant code: * MSG: error message - * CPU_DEF: a struct cpuX86Data pointer with flags that are conflicting + * CPU_DEF: a virCPUx86Data pointer with flags that are conflicting * RET: return code to set * * This macro generates the error string outputs it into logs. @@ -1352,7 +1352,7 @@ x86Compute(virCPUDefPtr host, } if (guest != NULL) { - struct cpuX86Data *guestData; + virCPUx86Data *guestData; if ((guest_model = x86ModelCopy(host_model)) == NULL) goto error; @@ -1441,7 +1441,7 @@ x86AddFeatures(virCPUDefPtr cpu, static int x86Decode(virCPUDefPtr cpu, - const struct cpuX86Data *data, + const virCPUx86Data *data, const char **models, unsigned int nmodels, const char *preferred, @@ -1558,13 +1558,13 @@ x86DecodeCPUData(virCPUDefPtr cpu, } -static struct cpuX86Data * +static virCPUx86Data * x86EncodePolicy(const virCPUDef *cpu, const struct x86_map *map, enum virCPUFeaturePolicy policy) { struct x86_model *model; - struct cpuX86Data *data = NULL; + virCPUx86Data *data = NULL; if (!(model = x86ModelFromCPU(cpu, map, policy))) return NULL; @@ -1588,12 +1588,12 @@ x86Encode(virArch arch, virCPUDataPtr *vendor) { struct x86_map *map = NULL; - struct cpuX86Data *data_forced = NULL; - struct cpuX86Data *data_required = NULL; - struct cpuX86Data *data_optional = NULL; - struct cpuX86Data *data_disabled = NULL; - struct cpuX86Data *data_forbidden = NULL; - struct cpuX86Data *data_vendor = NULL; + virCPUx86Data *data_forced = NULL; + virCPUx86Data *data_required = NULL; + virCPUx86Data *data_optional = NULL; + virCPUx86Data *data_disabled = NULL; + virCPUx86Data *data_forbidden = NULL; + virCPUx86Data *data_vendor = NULL; int ret = -1; if (forced) @@ -1769,7 +1769,7 @@ static virCPUDataPtr x86NodeData(virArch arch) { virCPUDataPtr cpuData = NULL; - struct cpuX86Data *data; + virCPUx86Data *data; int ret; if (VIR_ALLOC(data) < 0) diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h index acb7c32..5910ea9 100644 --- a/src/cpu/cpu_x86_data.h +++ b/src/cpu/cpu_x86_data.h @@ -38,7 +38,8 @@ struct _virCPUx86CPUID { # define CPUX86_BASIC 0x0 # define CPUX86_EXTENDED 0x80000000 -struct cpuX86Data { +typedef struct _virCPUx86Data virCPUx86Data; +struct _virCPUx86Data { size_t basic_len; virCPUx86CPUID *basic; size_t extended_len; -- 1.8.3.2

From: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/cpu/cpu_x86.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index b8b9a07..3e58e35 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -204,7 +204,7 @@ x86DataCpuid(const virCPUx86Data *data, static void -x86DataFree(virCPUx86Data *data) +virCPUx86DataFree(virCPUx86Data *data) { if (data == NULL) return; @@ -236,7 +236,7 @@ x86FreeCPUData(virCPUDataPtr data) if (!data) return; - x86DataFree(data->data.x86); + virCPUx86DataFree(data->data.x86); VIR_FREE(data); } @@ -250,7 +250,7 @@ x86DataCopy(const virCPUx86Data *data) if (VIR_ALLOC(copy) < 0 || VIR_ALLOC_N(copy->basic, data->basic_len) < 0 || VIR_ALLOC_N(copy->extended, data->extended_len) < 0) { - x86DataFree(copy); + virCPUx86DataFree(copy); return NULL; } @@ -489,8 +489,8 @@ x86DataToCPU(const virCPUx86Data *data, goto error; cleanup: - x86DataFree(modelData); - x86DataFree(copy); + virCPUx86DataFree(modelData); + virCPUx86DataFree(copy); return cpu; error: @@ -614,7 +614,7 @@ x86FeatureFree(struct x86_feature *feature) return; VIR_FREE(feature->name); - x86DataFree(feature->data); + virCPUx86DataFree(feature->data); VIR_FREE(feature); } @@ -784,7 +784,7 @@ x86ModelFree(struct x86_model *model) return; VIR_FREE(model->name); - x86DataFree(model->data); + virCPUx86DataFree(model->data); VIR_FREE(model); } @@ -997,7 +997,7 @@ x86ModelLoad(xmlXPathContextPtr ctxt, VIR_FREE(name); model->vendor = ancestor->vendor; - x86DataFree(model->data); + virCPUx86DataFree(model->data); if (!(model->data = x86DataCopy(ancestor->data))) goto error; } @@ -1210,7 +1210,7 @@ cleanup: VIR_FREE(nodes); xmlXPathFreeContext(ctxt); xmlFreeDoc(xml); - x86DataFree(data); + virCPUx86DataFree(data); return cpuData; } @@ -1368,7 +1368,7 @@ x86Compute(virCPUDefPtr host, if (!(guestData = x86DataCopy(guest_model->data)) || !(*guest = x86MakeCPUData(arch, &guestData))) { - x86DataFree(guestData); + virCPUx86DataFree(guestData); goto error; } } @@ -1685,12 +1685,12 @@ cleanup: return ret; error: - x86DataFree(data_forced); - x86DataFree(data_required); - x86DataFree(data_optional); - x86DataFree(data_disabled); - x86DataFree(data_forbidden); - x86DataFree(data_vendor); + virCPUx86DataFree(data_forced); + virCPUx86DataFree(data_required); + virCPUx86DataFree(data_optional); + virCPUx86DataFree(data_disabled); + virCPUx86DataFree(data_forbidden); + virCPUx86DataFree(data_vendor); if (forced) x86FreeCPUData(*forced); if (required) @@ -1789,7 +1789,7 @@ x86NodeData(virArch arch) return cpuData; error: - x86DataFree(data); + virCPUx86DataFree(data); return NULL; } -- 1.8.3.2

From: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/cpu/cpu_x86.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 3e58e35..aed19b4 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -216,7 +216,7 @@ virCPUx86DataFree(virCPUx86Data *data) static virCPUDataPtr -x86MakeCPUData(virArch arch, virCPUx86Data **data) +virCPUx86MakeData(virArch arch, virCPUx86Data **data) { virCPUDataPtr cpuData; @@ -1204,7 +1204,7 @@ x86CPUDataParse(const char *xmlStr) goto cleanup; } - cpuData = x86MakeCPUData(VIR_ARCH_X86_64, &data); + cpuData = virCPUx86MakeData(VIR_ARCH_X86_64, &data); cleanup: VIR_FREE(nodes); @@ -1367,7 +1367,7 @@ x86Compute(virCPUDefPtr host, x86DataSubtract(guest_model->data, cpu_disable->data); if (!(guestData = x86DataCopy(guest_model->data)) || - !(*guest = x86MakeCPUData(arch, &guestData))) { + !(*guest = virCPUx86MakeData(arch, &guestData))) { virCPUx86DataFree(guestData); goto error; } @@ -1659,22 +1659,22 @@ x86Encode(virArch arch, } if (forced && - !(*forced = x86MakeCPUData(arch, &data_forced))) + !(*forced = virCPUx86MakeData(arch, &data_forced))) goto error; if (required && - !(*required = x86MakeCPUData(arch, &data_required))) + !(*required = virCPUx86MakeData(arch, &data_required))) goto error; if (optional && - !(*optional = x86MakeCPUData(arch, &data_optional))) + !(*optional = virCPUx86MakeData(arch, &data_optional))) goto error; if (disabled && - !(*disabled = x86MakeCPUData(arch, &data_disabled))) + !(*disabled = virCPUx86MakeData(arch, &data_disabled))) goto error; if (forbidden && - !(*forbidden = x86MakeCPUData(arch, &data_forbidden))) + !(*forbidden = virCPUx86MakeData(arch, &data_forbidden))) goto error; if (vendor && - !(*vendor = x86MakeCPUData(arch, &data_vendor))) + !(*vendor = virCPUx86MakeData(arch, &data_vendor))) goto error; ret = 0; @@ -1783,7 +1783,7 @@ x86NodeData(virArch arch) goto error; data->extended_len = ret; - if (!(cpuData = x86MakeCPUData(arch, &data))) + if (!(cpuData = virCPUx86MakeData(arch, &data))) goto error; return cpuData; -- 1.8.3.2

From: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/cpu/cpu_x86.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index aed19b4..1d70c55 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -296,8 +296,8 @@ x86DataExpand(virCPUx86Data *data, static int -x86DataAddCpuid(virCPUx86Data *data, - const virCPUx86CPUID *cpuid) +virCPUx86DataAddCPUID(virCPUx86Data *data, + const virCPUx86CPUID *cpuid) { unsigned int basic_by = 0; unsigned int extended_by = 0; @@ -734,7 +734,7 @@ x86FeatureLoad(xmlXPathContextPtr ctxt, i, feature->name); goto ignore; } - if (x86DataAddCpuid(feature->data, &cpuid)) + if (virCPUx86DataAddCPUID(feature->data, &cpuid)) goto error; } @@ -1200,7 +1200,7 @@ x86CPUDataParse(const char *xmlStr) _("failed to parse cpuid[%zu]"), i); goto cleanup; } - if (x86DataAddCpuid(data, &cpuid) < 0) + if (virCPUx86DataAddCPUID(data, &cpuid) < 0) goto cleanup; } @@ -1653,7 +1653,7 @@ x86Encode(virArch arch, if (v && (VIR_ALLOC(data_vendor) < 0 || - x86DataAddCpuid(data_vendor, &v->cpuid) < 0)) { + virCPUx86DataAddCPUID(data_vendor, &v->cpuid) < 0)) { goto error; } } @@ -1879,7 +1879,7 @@ x86Baseline(virCPUDefPtr *cpus, goto error; } - if (vendor && x86DataAddCpuid(base_model->data, &vendor->cpuid) < 0) + if (vendor && virCPUx86DataAddCPUID(base_model->data, &vendor->cpuid) < 0) goto error; if (x86Decode(cpu, base_model->data, models, nmodels, NULL, flags) < 0) -- 1.8.3.2

Use virCPUx86DataIterator and virCPUx86DataIteratorInit. --- src/cpu/cpu_x86.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 1d70c55..c9a9e3e 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -79,14 +79,14 @@ enum compare_result { }; -struct data_iterator { +struct virCPUx86DataIterator { virCPUx86Data *data; int pos; bool extended; }; -#define DATA_ITERATOR_INIT(data) \ +#define virCPUx86DataIteratorInit(data) \ { data, -1, false } @@ -147,7 +147,7 @@ x86cpuidAndBits(virCPUx86CPUID *cpuid, /* skips all zero CPUID leafs */ static virCPUx86CPUID * -x86DataCpuidNext(struct data_iterator *iterator) +x86DataCpuidNext(struct virCPUx86DataIterator *iterator) { virCPUx86CPUID *ret; virCPUx86Data *data = iterator->data; @@ -373,7 +373,7 @@ static void x86DataIntersect(virCPUx86Data *data1, const virCPUx86Data *data2) { - struct data_iterator iter = DATA_ITERATOR_INIT(data1); + struct virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data1); virCPUx86CPUID *cpuid1; virCPUx86CPUID *cpuid2; @@ -390,7 +390,7 @@ x86DataIntersect(virCPUx86Data *data1, static bool x86DataIsEmpty(virCPUx86Data *data) { - struct data_iterator iter = DATA_ITERATOR_INIT(data); + struct virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data); return x86DataCpuidNext(&iter) == NULL; } @@ -401,7 +401,7 @@ x86DataIsSubset(const virCPUx86Data *data, const virCPUx86Data *subset) { - struct data_iterator iter = DATA_ITERATOR_INIT((virCPUx86Data *)subset); + struct virCPUx86DataIterator iter = virCPUx86DataIteratorInit((virCPUx86Data *)subset); const virCPUx86CPUID *cpuid; const virCPUx86CPUID *cpuidSubset; @@ -912,8 +912,8 @@ x86ModelCompare(const struct x86_model *model1, const struct x86_model *model2) { enum compare_result result = EQUAL; - struct data_iterator iter1 = DATA_ITERATOR_INIT(model1->data); - struct data_iterator iter2 = DATA_ITERATOR_INIT(model2->data); + struct virCPUx86DataIterator iter1 = virCPUx86DataIteratorInit(model1->data); + struct virCPUx86DataIterator iter2 = virCPUx86DataIteratorInit(model2->data); virCPUx86CPUID *cpuid1; virCPUx86CPUID *cpuid2; @@ -1139,7 +1139,7 @@ error: static char * x86CPUDataFormat(const virCPUData *data) { - struct data_iterator iter = DATA_ITERATOR_INIT(data->data.x86); + struct virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data->data.x86); virCPUx86CPUID *cpuid; virBuffer buf = VIR_BUFFER_INITIALIZER; -- 1.8.3.2

These return boolean results. --- src/cpu/cpu_x86.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index c9a9e3e..3ef079e 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -90,7 +90,7 @@ struct virCPUx86DataIterator { { data, -1, false } -static int +static bool x86cpuidMatch(const virCPUx86CPUID *cpuid1, const virCPUx86CPUID *cpuid2) { @@ -101,7 +101,7 @@ x86cpuidMatch(const virCPUx86CPUID *cpuid1, } -static int +static bool x86cpuidMatchMasked(const virCPUx86CPUID *cpuid, const virCPUx86CPUID *mask) { -- 1.8.3.2

--- src/cpu/cpu_x86.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 3ef079e..e6ff591 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1240,6 +1240,7 @@ cleanup: ret = VIR_CPU_COMPARE_INCOMPATIBLE; \ } while (0) + static virCPUCompareResult x86Compute(virCPUDefPtr host, virCPUDefPtr cpu, @@ -1410,6 +1411,7 @@ x86GuestData(virCPUDefPtr host, return x86Compute(host, guest, data, message); } + static int x86AddFeatures(virCPUDefPtr cpu, struct x86_map *map) @@ -2010,8 +2012,10 @@ x86Update(virCPUDefPtr guest, return -1; } -static int x86HasFeature(const virCPUData *data, - const char *name) + +static int +x86HasFeature(const virCPUData *data, + const char *name) { struct x86_map *map; struct x86_feature *feature; @@ -2030,6 +2034,7 @@ cleanup: return ret; } + struct cpuArchDriver cpuDriverX86 = { .name = "x86", .arch = archs, -- 1.8.3.2

--- src/cpu/cpu_x86.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index e6ff591..5e787fa 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1296,6 +1296,7 @@ x86Compute(virCPUDefPtr host, "CPU vendor %s"), cpu->vendor) < 0) goto error; + return VIR_CPU_COMPARE_INCOMPATIBLE; } @@ -1312,7 +1313,7 @@ x86Compute(virCPUDefPtr host, if (!x86DataIsEmpty(cpu_forbid->data)) { virX86CpuIncompatible(N_("Host CPU provides forbidden features"), cpu_forbid->data); - goto out; + goto cleanup; } /* first remove features that were inherited from the CPU model and were @@ -1327,7 +1328,7 @@ x86Compute(virCPUDefPtr host, virX86CpuIncompatible(N_("Host CPU does not provide required " "features"), cpu_require->data); - goto out; + goto cleanup; } ret = VIR_CPU_COMPARE_IDENTICAL; @@ -1349,7 +1350,7 @@ x86Compute(virCPUDefPtr host, virX86CpuIncompatible(N_("Host CPU does not strictly match guest CPU: " "Extra features"), diff->data); - goto out; + goto cleanup; } if (guest != NULL) { @@ -1374,7 +1375,7 @@ x86Compute(virCPUDefPtr host, } } -out: +cleanup: x86MapFree(map); x86ModelFree(host_model); x86ModelFree(diff); @@ -1389,7 +1390,7 @@ out: error: ret = VIR_CPU_COMPARE_ERROR; - goto out; + goto cleanup; } #undef virX86CpuIncompatible -- 1.8.3.2

Avoid a line exceeding 80 characters and change argument alignment in two error messages. --- src/cpu/cpu_x86.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 5e787fa..8427555 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -542,8 +542,8 @@ x86VendorLoad(xmlXPathContextPtr ctxt, vendor->name = virXPathString("string(@name)", ctxt); if (!vendor->name) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Missing CPU vendor name")); + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing CPU vendor name")); goto ignore; } @@ -556,7 +556,8 @@ x86VendorLoad(xmlXPathContextPtr ctxt, string = virXPathString("string(@string)", ctxt); if (!string) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Missing vendor string for CPU vendor %s"), vendor->name); + _("Missing vendor string for CPU vendor %s"), + vendor->name); goto ignore; } if (strlen(string) != VENDOR_STRING_LENGTH) { -- 1.8.3.2

On Tue, Oct 15, 2013 at 02:30:44PM +0200, Peter Krempa wrote: To be honest I'm not really a fan of making these kind of changes. IMHO 80 column limit is something that shouldn't be strictly enforced when it causes splitting of messages which are only marginally over. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

From: Jiri Denemark <jdenemar(a)redhat.com&gt; This makes virCPUx86DataAddCPUID, virCPUx86DataFree, and virCPUx86MakeData available for direct usage outside of cpu driver. --- src/cpu/cpu_x86.c | 6 +++--- src/cpu/cpu_x86.h | 9 +++++++++ src/libvirt_private.syms | 6 ++++++ 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 8427555..539d8b2 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -203,7 +203,7 @@ x86DataCpuid(const virCPUx86Data *data, } -static void +void virCPUx86DataFree(virCPUx86Data *data) { if (data == NULL) @@ -215,7 +215,7 @@ virCPUx86DataFree(virCPUx86Data *data) } -static virCPUDataPtr +virCPUDataPtr virCPUx86MakeData(virArch arch, virCPUx86Data **data) { virCPUDataPtr cpuData; @@ -295,7 +295,7 @@ x86DataExpand(virCPUx86Data *data, } -static int +int virCPUx86DataAddCPUID(virCPUx86Data *data, const virCPUx86CPUID *cpuid) { diff --git a/src/cpu/cpu_x86.h b/src/cpu/cpu_x86.h index 77965b7..af0fa23 100644 --- a/src/cpu/cpu_x86.h +++ b/src/cpu/cpu_x86.h @@ -25,7 +25,16 @@ # define __VIR_CPU_X86_H__ # include "cpu.h" +# include "cpu_x86_data.h" extern struct cpuArchDriver cpuDriverX86; +int virCPUx86DataAddCPUID(virCPUx86Data *data, + const virCPUx86CPUID *cpuid); + +void virCPUx86DataFree(virCPUx86Data *data); + +virCPUDataPtr virCPUx86MakeData(virArch arch, + virCPUx86Data **data); + #endif /* __VIR_CPU_X86_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 84c1c28..cb80700 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -733,6 +733,12 @@ cpuNodeData; cpuUpdate; +# cpu/cpu_x86.h +virCPUx86DataAddCPUID; +virCPUx86DataFree; +virCPUx86MakeData; + + # datatypes.h virConnectClass; virDomainClass; -- 1.8.3.2

From: Jiri Denemark <jdenemar(a)redhat.com&gt; The qemu monitor supports retrieval of actual CPUID bits presented to the guest using QMP monitor. Add APIs to extract these information and tests for them. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_monitor.c | 21 ++++ src/qemu/qemu_monitor.h | 3 + src/qemu/qemu_monitor_json.c | 107 +++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 + tests/Makefile.am | 1 + .../qemumonitorjson-getcpu-empty.data | 2 + .../qemumonitorjson-getcpu-empty.json | 46 +++++++++ .../qemumonitorjson-getcpu-filtered.data | 4 + .../qemumonitorjson-getcpu-filtered.json | 46 +++++++++ .../qemumonitorjson-getcpu-full.data | 5 + .../qemumonitorjson-getcpu-full.json | 46 +++++++++ .../qemumonitorjson-getcpu-host.data | 6 ++ .../qemumonitorjson-getcpu-host.json | 45 +++++++++ tests/qemumonitorjsontest.c | 76 +++++++++++++++ 14 files changed, 411 insertions(+) create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.data create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.data create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.data create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.data create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.json diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 2bafe28..f1556d8 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3926,3 +3926,24 @@ qemuMonitorSetDomainLog(qemuMonitorPtr mon, int logfd) return 0; } + + +virCPUDataPtr +qemuMonitorGetGuestCPU(qemuMonitorPtr mon) +{ + VIR_DEBUG("mon=%p", mon); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return NULL; + } + + if (!mon->json) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return NULL; + } + + return qemuMonitorJSONGetCPUData(mon, "feature-words"); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 06ba7e8..65ade56 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -32,6 +32,7 @@ # include "virhash.h" # include "virjson.h" # include "device_conf.h" +# include "cpu/cpu.h" typedef struct _qemuMonitor qemuMonitor; typedef qemuMonitor *qemuMonitorPtr; @@ -763,6 +764,8 @@ int qemuMonitorGetDeviceAliases(qemuMonitorPtr mon, int qemuMonitorSetDomainLog(qemuMonitorPtr mon, int logfd); +virCPUDataPtr qemuMonitorGetGuestCPU(qemuMonitorPtr mon); + /** * When running two dd process and using <> redirection, we need a * shell that will not truncate files. These two strings serve that diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 05f8aa6..3a5c2ad 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -42,6 +42,7 @@ #include "virerror.h" #include "virjson.h" #include "virstring.h" +#include "cpu/cpu_x86.h" #ifdef WITH_DTRACE_PROBES # include "libvirt_qemu_probes.h" @@ -49,6 +50,7 @@ #define VIR_FROM_THIS VIR_FROM_QEMU +#define QOM_CPU_PATH "/machine/unattached/device[0]" #define LINE_ENDING "\r\n" @@ -5454,3 +5456,108 @@ cleanup: VIR_FREE(paths); return ret; } + + +static int +qemuMonitorJSONParseCPUFeatureWord(virJSONValuePtr data, + virCPUx86CPUID *cpuid) +{ + const char *reg; + unsigned long long fun; + unsigned long long features; + + memset(cpuid, 0, sizeof(*cpuid)); + + if (!(reg = virJSONValueObjectGetString(data, "cpuid-register"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing cpuid-register in CPU data")); + return -1; + } + if (virJSONValueObjectGetNumberUlong(data, "cpuid-input-eax", &fun)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing or invalid cpuid-input-eax in CPU data")); + return -1; + } + if (virJSONValueObjectGetNumberUlong(data, "features", &features) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing or invalid features in CPU data")); + return -1; + } + + cpuid->function = fun; + if (STREQ(reg, "EAX")) { + cpuid->eax = features; + } else if (STREQ(reg, "EBX")) { + cpuid->ebx = features; + } else if (STREQ(reg, "ECX")) { + cpuid->ecx = features; + } else if (STREQ(reg, "EDX")) { + cpuid->edx = features; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown CPU register '%s'"), reg); + return -1; + } + + return 0; +} + +virCPUDataPtr +qemuMonitorJSONGetCPUData(qemuMonitorPtr mon, + const char *property) +{ + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + virCPUDataPtr cpuData = NULL; + virCPUx86Data *x86Data = NULL; + virCPUx86CPUID cpuid; + size_t i; + int ret; + int n; + + if (!(cmd = qemuMonitorJSONMakeCommand("qom-get", + "s:path", QOM_CPU_PATH, + "s:property", property, + NULL))) + return NULL; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + if (ret < 0) + goto cleanup; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qom-get reply was missing return data")); + goto cleanup; + } + + if ((n = virJSONValueArraySize(data)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s CPU property did not return an array"), + property); + goto cleanup; + } + + if (VIR_ALLOC(x86Data) < 0) + goto cleanup; + + for (i = 0; i < n; i++) { + if (qemuMonitorJSONParseCPUFeatureWord(virJSONValueArrayGet(data, i), + &cpuid) < 0 || + virCPUx86DataAddCPUID(x86Data, &cpuid) < 0) + goto cleanup; + } + + cpuData = virCPUx86MakeData(VIR_ARCH_X86_64, &x86Data); + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + virCPUx86DataFree(x86Data); + return cpuData; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 51cf19c..25f1fe6 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -29,6 +29,7 @@ # include "qemu_monitor.h" # include "virbitmap.h" +# include "cpu/cpu.h" int qemuMonitorJSONIOProcess(qemuMonitorPtr mon, const char *data, @@ -426,4 +427,6 @@ int qemuMonitorJSONDetachCharDev(qemuMonitorPtr mon, int qemuMonitorJSONGetDeviceAliases(qemuMonitorPtr mon, char ***aliases); +virCPUDataPtr qemuMonitorJSONGetCPUData(qemuMonitorPtr mon, + const char *property); #endif /* QEMU_MONITOR_JSON_H */ diff --git a/tests/Makefile.am b/tests/Makefile.am index 250cd8c..9ec4fcb 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -85,6 +85,7 @@ EXTRA_DIST = \ qemucapabilitiesdata \ qemuhelpdata \ qemuhotplugtestdata \ + qemumonitorjsondata \ qemuxml2argvdata \ qemuxml2xmloutdata \ qemuxmlnsdata \ diff --git a/tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.data b/tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.data new file mode 100644 index 0000000..2b3c111 --- /dev/null +++ b/tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.data @@ -0,0 +1,2 @@ +<cpudata arch='x86'> +</cpudata> diff --git a/tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.json b/tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.json new file mode 100644 index 0000000..36db555 --- /dev/null +++ b/tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.json @@ -0,0 +1,46 @@ +{ + "return": [ + { + "cpuid-register": "EDX", + "cpuid-input-eax": 2147483658, + "features": 0 + }, + { + "cpuid-register": "EAX", + "cpuid-input-eax": 1073741825, + "features": 0 + }, + { + "cpuid-register": "EDX", + "cpuid-input-eax": 3221225473, + "features": 0 + }, + { + "cpuid-register": "ECX", + "cpuid-input-eax": 2147483649, + "features": 0 + }, + { + "cpuid-register": "EDX", + "cpuid-input-eax": 2147483649, + "features": 0 + }, + { + "cpuid-register": "EBX", + "cpuid-input-ecx": 0, + "cpuid-input-eax": 7, + "features": 0 + }, + { + "cpuid-register": "ECX", + "cpuid-input-eax": 1, + "features": 0 + }, + { + "cpuid-register": "EDX", + "cpuid-input-eax": 1, + "features": 0 + } + ], + "id": "libvirt-6" +} diff --git a/tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.data b/tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.data new file mode 100644 index 0000000..57431e2 --- /dev/null +++ b/tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.data @@ -0,0 +1,4 @@ +<cpudata arch='x86'> + <cpuid function='0x00000001' eax='0x00000000' ebx='0x00000000' ecx='0x00401000' edx='0x00000000'/> + <cpuid function='0x00000007' eax='0x00000000' ebx='0x00000fb9' ecx='0x00000000' edx='0x00000000'/> +</cpudata> diff --git a/tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.json b/tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.json new file mode 100644 index 0000000..13352f9 --- /dev/null +++ b/tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.json @@ -0,0 +1,46 @@ +{ + "return": [ + { + "cpuid-register": "EDX", + "cpuid-input-eax": 2147483658, + "features": 0 + }, + { + "cpuid-register": "EAX", + "cpuid-input-eax": 1073741825, + "features": 0 + }, + { + "cpuid-register": "EDX", + "cpuid-input-eax": 3221225473, + "features": 0 + }, + { + "cpuid-register": "ECX", + "cpuid-input-eax": 2147483649, + "features": 0 + }, + { + "cpuid-register": "EDX", + "cpuid-input-eax": 2147483649, + "features": 0 + }, + { + "cpuid-register": "EBX", + "cpuid-input-ecx": 0, + "cpuid-input-eax": 7, + "features": 4025 + }, + { + "cpuid-register": "ECX", + "cpuid-input-eax": 1, + "features": 4198400 + }, + { + "cpuid-register": "EDX", + "cpuid-input-eax": 1, + "features": 0 + } + ], + "id": "libvirt-7" +} diff --git a/tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.data b/tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.data new file mode 100644 index 0000000..bba8d31 --- /dev/null +++ b/tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.data @@ -0,0 +1,5 @@ +<cpudata arch='x86'> + <cpuid function='0x00000001' eax='0x00000000' ebx='0x00000000' ecx='0x97ba2223' edx='0x078bfbfd'/> + <cpuid function='0x40000001' eax='0x0100003b' ebx='0x00000000' ecx='0x00000000' edx='0x00000000'/> + <cpuid function='0x80000001' eax='0x00000000' ebx='0x00000000' ecx='0x00000001' edx='0x28100800'/> +</cpudata> diff --git a/tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.json b/tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.json new file mode 100644 index 0000000..29c00b4 --- /dev/null +++ b/tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.json @@ -0,0 +1,46 @@ +{ + "return": [ + { + "cpuid-register": "EDX", + "cpuid-input-eax": 2147483658, + "features": 0 + }, + { + "cpuid-register": "EAX", + "cpuid-input-eax": 1073741825, + "features": 16777275 + }, + { + "cpuid-register": "EDX", + "cpuid-input-eax": 3221225473, + "features": 0 + }, + { + "cpuid-register": "ECX", + "cpuid-input-eax": 2147483649, + "features": 1 + }, + { + "cpuid-register": "EDX", + "cpuid-input-eax": 2147483649, + "features": 672139264 + }, + { + "cpuid-register": "EBX", + "cpuid-input-ecx": 0, + "cpuid-input-eax": 7, + "features": 0 + }, + { + "cpuid-register": "ECX", + "cpuid-input-eax": 1, + "features": 2545558051 + }, + { + "cpuid-register": "EDX", + "cpuid-input-eax": 1, + "features": 126614525 + } + ], + "id": "libvirt-6" +} diff --git a/tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.data b/tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.data new file mode 100644 index 0000000..98b9e7c --- /dev/null +++ b/tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.data @@ -0,0 +1,6 @@ +<cpudata arch='x86'> + <cpuid function='0x00000001' eax='0x00000000' ebx='0x00000000' ecx='0x97ba2223' edx='0x0f8bfbff'/> + <cpuid function='0x00000007' eax='0x00000000' ebx='0x00000002' ecx='0x00000000' edx='0x00000000'/> + <cpuid function='0x40000001' eax='0x0100007b' ebx='0x00000000' ecx='0x00000000' edx='0x00000000'/> + <cpuid function='0x80000001' eax='0x00000000' ebx='0x00000000' ecx='0x00000001' edx='0x2993fbff'/> +</cpudata> diff --git a/tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.json b/tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.json new file mode 100644 index 0000000..b5fb9f3 --- /dev/null +++ b/tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.json @@ -0,0 +1,45 @@ +{ + "return": [ + { + "cpuid-register": "EDX", + "cpuid-input-eax": 2147483658, + "features": 0 + }, + { + "cpuid-register": "EAX", + "cpuid-input-eax": 1073741825, + "features": 16777339 + }, + { + "cpuid-register": "EDX", + "cpuid-input-eax": 3221225473, + "features": 0 + }, + { + "cpuid-register": "ECX", + "cpuid-input-eax": 2147483649, + "features": 1 + }, + { + "cpuid-register": "EDX", + "cpuid-input-eax": 2147483649, + "features": 697564159 + }, + { + "cpuid-register": "EBX", + "cpuid-input-ecx": 0, + "cpuid-input-eax": 7, + "features": 2 + }, + { + "cpuid-register": "ECX", + "cpuid-input-eax": 1, + "features": 2545558051 + }, + { + "cpuid-register": "EDX", + "cpuid-input-eax": 1, + "features": 260832255 + } + ] +} diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index de907ae..7ca3fb0 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -27,6 +27,7 @@ #include "virthread.h" #include "virerror.h" #include "virstring.h" +#include "cpu/cpu.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -1958,6 +1959,68 @@ cleanup: return ret; } + +struct testCPUData { + const char *name; + const char *property; + virDomainXMLOptionPtr xmlopt; +}; + + +static int +testQemuMonitorJSONGetCPUData(const void *opaque) +{ + const struct testCPUData *data = opaque; + qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, data->xmlopt); + virCPUDataPtr cpuData = NULL; + char *jsonFile = NULL; + char *dataFile = NULL; + char *jsonStr = NULL; + char *expected = NULL; + char *actual = NULL; + int ret = -1; + + if (!test) + return -1; + + if (virAsprintf(&jsonFile, + "%s/qemumonitorjsondata/qemumonitorjson-getcpu-%s.json", + abs_srcdir, data->name) < 0 || + virAsprintf(&dataFile, + "%s/qemumonitorjsondata/qemumonitorjson-getcpu-%s.data", + abs_srcdir, data->name) < 0) + goto cleanup; + + if (virtTestLoadFile(jsonFile, &jsonStr) < 0 || + virtTestLoadFile(dataFile, &expected) < 0) + goto cleanup; + + if (qemuMonitorTestAddItem(test, "qom-get", jsonStr) < 0) + goto cleanup; + + cpuData = qemuMonitorJSONGetCPUData(qemuMonitorTestGetMonitor(test), + data->property); + if (!cpuData || !(actual = cpuDataFormat(cpuData))) + goto cleanup; + + if (STRNEQ(expected, actual)) { + virtTestDifference(stderr, expected, actual); + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FREE(jsonFile); + VIR_FREE(dataFile); + VIR_FREE(jsonStr); + VIR_FREE(expected); + VIR_FREE(actual); + cpuDataFree(cpuData); + qemuMonitorTestFree(test); + return ret; +} + + static int mymain(void) { @@ -1991,6 +2054,14 @@ mymain(void) if (virtTestRun(# name, testQemuMonitorJSON ## name, &simpleFunc) < 0) \ ret = -1 +#define DO_TEST_CPU_DATA(name, property) \ + do { \ + struct testCPUData data = { name, property, xmlopt }; \ + const char *label = "GetCPUData(" property ", " name ")"; \ + if (virtTestRun(label, testQemuMonitorJSONGetCPUData, &data) < 0) \ + ret = -1; \ + } while (0) + DO_TEST(GetStatus); DO_TEST(GetVersion); DO_TEST(GetMachines); @@ -2055,6 +2126,11 @@ mymain(void) DO_TEST(qemuMonitorJSONGetVirtType); DO_TEST(qemuMonitorJSONSendKey); + DO_TEST_CPU_DATA("host", "feature-words"); + DO_TEST_CPU_DATA("full", "feature-words"); + DO_TEST_CPU_DATA("filtered", "filtered-features"); + DO_TEST_CPU_DATA("empty", "filtered-features"); + virObjectUnref(xmlopt); return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; -- 1.8.3.2

Until now the map was loaded from the XML definition file every time a operation on the flags was requested. With the introduciton of one shot initializers we can store the definition forever (as it will never change) instead of parsing it over and over again. --- src/cpu/cpu_x86.c | 67 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 36 insertions(+), 31 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 1b1f2b4..e91cf49 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -70,6 +70,10 @@ struct x86_map { struct x86_model *models; }; +static struct x86_map* virCPUx86Map = NULL; +int virCPUx86MapOnceInit(void); +VIR_ONCE_GLOBAL_INIT(virCPUx86Map); + enum compare_result { SUBSET, @@ -1064,22 +1068,34 @@ x86MapLoadCallback(enum cpuMapElement element, } -static struct x86_map * -x86LoadMap(void) +int +virCPUx86MapOnceInit(void) { struct x86_map *map; if (VIR_ALLOC(map) < 0) - return NULL; + return -1; if (cpuMapLoad("x86", x86MapLoadCallback, map) < 0) goto error; - return map; + virCPUx86Map = map; + + return 0; error: x86MapFree(map); - return NULL; + return -1; +} + + +static const struct x86_map * +virCPUx86GetMap(void) +{ + if (virCPUx86MapInitialize() < 0) + return NULL; + + return virCPUx86Map; } @@ -1194,7 +1210,7 @@ x86Compute(virCPUDefPtr host, virCPUDataPtr *guest, char **message) { - struct x86_map *map = NULL; + const struct x86_map *map = NULL; struct x86_model *host_model = NULL; struct x86_model *cpu_force = NULL; struct x86_model *cpu_require = NULL; @@ -1247,7 +1263,7 @@ x86Compute(virCPUDefPtr host, return VIR_CPU_COMPARE_INCOMPATIBLE; } - if (!(map = x86LoadMap()) || + if (!(map = virCPUx86GetMap()) || !(host_model = x86ModelFromCPU(host, map, VIR_CPU_FEATURE_REQUIRE)) || !(cpu_force = x86ModelFromCPU(cpu, map, VIR_CPU_FEATURE_FORCE)) || !(cpu_require = x86ModelFromCPU(cpu, map, VIR_CPU_FEATURE_REQUIRE)) || @@ -1323,7 +1339,6 @@ x86Compute(virCPUDefPtr host, } cleanup: - x86MapFree(map); x86ModelFree(host_model); x86ModelFree(diff); x86ModelFree(cpu_force); @@ -1362,7 +1377,7 @@ x86GuestData(virCPUDefPtr host, static int x86AddFeatures(virCPUDefPtr cpu, - struct x86_map *map) + const struct x86_map *map) { const struct x86_model *candidate; const struct x86_feature *feature = map->features; @@ -1398,7 +1413,7 @@ x86Decode(virCPUDefPtr cpu, unsigned int flags) { int ret = -1; - struct x86_map *map; + const struct x86_map *map; const struct x86_model *candidate; virCPUDefPtr cpuCandidate; virCPUDefPtr cpuModel = NULL; @@ -1406,7 +1421,7 @@ x86Decode(virCPUDefPtr cpu, virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); - if (data == NULL || (map = x86LoadMap()) == NULL) + if (!data || !(map = virCPUx86GetMap())) return -1; candidate = map->models; @@ -1490,7 +1505,6 @@ x86Decode(virCPUDefPtr cpu, ret = 0; out: - x86MapFree(map); virCPUDefFree(cpuModel); return ret; @@ -1537,14 +1551,13 @@ x86Encode(virArch arch, virCPUDataPtr *forbidden, virCPUDataPtr *vendor) { - struct x86_map *map = NULL; + const struct x86_map *map = NULL; virCPUx86Data *data_forced = NULL; virCPUx86Data *data_required = NULL; virCPUx86Data *data_optional = NULL; virCPUx86Data *data_disabled = NULL; virCPUx86Data *data_forbidden = NULL; virCPUx86Data *data_vendor = NULL; - int ret = -1; if (forced) *forced = NULL; @@ -1559,7 +1572,7 @@ x86Encode(virArch arch, if (vendor) *vendor = NULL; - if ((map = x86LoadMap()) == NULL) + if ((map = virCPUx86GetMap()) == NULL) goto error; if (forced) { @@ -1627,12 +1640,7 @@ x86Encode(virArch arch, !(*vendor = virCPUx86MakeData(arch, &data_vendor))) goto error; - ret = 0; - -cleanup: - x86MapFree(map); - - return ret; + return 0; error: virCPUx86DataFree(data_forced); @@ -1653,7 +1661,7 @@ error: x86FreeCPUData(*forbidden); if (vendor) x86FreeCPUData(*vendor); - goto cleanup; + return -1; } @@ -1748,7 +1756,7 @@ x86Baseline(virCPUDefPtr *cpus, unsigned int nmodels, unsigned int flags) { - struct x86_map *map = NULL; + const struct x86_map *map = NULL; struct x86_model *base_model = NULL; virCPUDefPtr cpu = NULL; size_t i; @@ -1756,7 +1764,7 @@ x86Baseline(virCPUDefPtr *cpus, struct x86_model *model = NULL; bool outputVendor = true; - if (!(map = x86LoadMap())) + if (!(map = virCPUx86GetMap())) goto error; if (!(base_model = x86ModelFromCPU(cpus[0], map, VIR_CPU_FEATURE_REQUIRE))) @@ -1837,7 +1845,6 @@ x86Baseline(virCPUDefPtr *cpus, cleanup: x86ModelFree(base_model); - x86MapFree(map); return cpu; @@ -1855,10 +1862,10 @@ x86UpdateCustom(virCPUDefPtr guest, { int ret = -1; size_t i; - struct x86_map *map; + const struct x86_map *map; struct x86_model *host_model = NULL; - if (!(map = x86LoadMap()) || + if (!(map = virCPUx86GetMap()) || !(host_model = x86ModelFromCPU(host, map, VIR_CPU_FEATURE_REQUIRE))) goto cleanup; @@ -1890,7 +1897,6 @@ x86UpdateCustom(virCPUDefPtr guest, ret = 0; cleanup: - x86MapFree(map); x86ModelFree(host_model); return ret; } @@ -1960,11 +1966,11 @@ static int x86HasFeature(const virCPUData *data, const char *name) { - struct x86_map *map; + const struct x86_map *map; struct x86_feature *feature; int ret = -1; - if (!(map = x86LoadMap())) + if (!(map = virCPUx86GetMap())) return -1; if (!(feature = x86FeatureFind(map, name))) @@ -1973,7 +1979,6 @@ x86HasFeature(const virCPUData *data, ret = x86DataIsSubset(data->data.x86, feature->data) ? 1 : 0; cleanup: - x86MapFree(map); return ret; } -- 1.8.3.2

Some of the emulator features are presented in the <features> element in the domain XML although they are virtual CPUID feature bits when presented to the guest. To avoid confusing the users with these features, as they are not configurable via the <cpu> element, this patch adds an internal array where those can be stored privately instead of exposing them in the XML. Additionaly KVM feature bits are added as example usage of this code. --- src/cpu/cpu_x86.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/cpu/cpu_x86_data.h | 12 ++++++++++ 2 files changed, 75 insertions(+) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index e91cf49..c972709 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -56,6 +56,25 @@ struct x86_feature { struct x86_feature *next; }; +struct x86_kvm_feature { + const char *name; + const virCPUx86CPUID cpuid; +}; + +static const struct x86_kvm_feature x86_kvm_features[] = +{ + {VIR_CPU_x86_KVM_CLOCKSOURCE, { .function = 0x40000001, .eax = 0x00000001 }}, + {VIR_CPU_x86_KVM_NOP_IO_DELAY, { .function = 0x40000001, .eax = 0x00000002 }}, + {VIR_CPU_x86_KVM_MMU_OP, { .function = 0x40000001, .eax = 0x00000004 }}, + {VIR_CPU_x86_KVM_CLOCKSOURCE2, { .function = 0x40000001, .eax = 0x00000008 }}, + {VIR_CPU_x86_KVM_ASYNC_PF, { .function = 0x40000001, .eax = 0x00000010 }}, + {VIR_CPU_x86_KVM_STEAL_TIME, { .function = 0x40000001, .eax = 0x00000020 }}, + {VIR_CPU_x86_KVM_PV_EOI, { .function = 0x40000001, .eax = 0x00000040 }}, + {VIR_CPU_x86_KVM_PV_UNHALT, { .function = 0x40000001, .eax = 0x00000080 }}, + {VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT, + { .function = 0x40000001, .eax = 0x01000000 }}, +}; + struct x86_model { char *name; const struct x86_vendor *vendor; @@ -1068,6 +1087,47 @@ x86MapLoadCallback(enum cpuMapElement element, } +static int +x86MapLoadInternalFeatures(struct x86_map *map) +{ + size_t i; + struct x86_feature *feature = NULL; + + for (i = 0; i < ARRAY_CARDINALITY(x86_kvm_features); i++) { + const char *name = x86_kvm_features[i].name; + + if (x86FeatureFind(map, name)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("CPU feature %s already defined"), name); + return -1; + } + + if (!(feature = x86FeatureNew())) + goto error; + + if (VIR_STRDUP(feature->name, name) < 0) + goto error; + + if (virCPUx86DataAddCPUID(feature->data, &x86_kvm_features[i].cpuid)) + goto error; + + if (map->features == NULL) { + map->features = feature; + } else { + feature->next = map->features; + map->features = feature; + } + + feature = NULL; + } + + return 0; + +error: + x86FeatureFree(feature); + return -1; +} + int virCPUx86MapOnceInit(void) { @@ -1079,6 +1139,9 @@ virCPUx86MapOnceInit(void) if (cpuMapLoad("x86", x86MapLoadCallback, map) < 0) goto error; + if (x86MapLoadInternalFeatures(map) < 0) + goto error; + virCPUx86Map = map; return 0; diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h index 69066f1..88dccf6 100644 --- a/src/cpu/cpu_x86_data.h +++ b/src/cpu/cpu_x86_data.h @@ -36,8 +36,20 @@ struct _virCPUx86CPUID { }; # define CPUX86_BASIC 0x0 +# define CPUX86_KVM 0x40000000 # define CPUX86_EXTENDED 0x80000000 +# define VIR_CPU_x86_KVM_CLOCKSOURCE "__kvm_clocksource" +# define VIR_CPU_x86_KVM_NOP_IO_DELAY "__kvm_no_io_delay" +# define VIR_CPU_x86_KVM_MMU_OP "__kvm_mmu_op" +# define VIR_CPU_x86_KVM_CLOCKSOURCE2 "__kvm_clocksource2" +# define VIR_CPU_x86_KVM_ASYNC_PF "__kvm_async_pf" +# define VIR_CPU_x86_KVM_STEAL_TIME "__kvm_steal_time" +# define VIR_CPU_x86_KVM_PV_EOI "__kvm_pv_eoi" +# define VIR_CPU_x86_KVM_PV_UNHALT "__kvm_pv_unhalt" +# define VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT "__kvm_clocksource_stable" + + typedef struct _virCPUx86Data virCPUx86Data; struct _virCPUx86Data { size_t len; -- 1.8.3.2

To allow a finer control for some future flags, refactor the code to store them in an array instead of a bitmap. --- src/conf/domain_conf.c | 159 ++++++++++++++++++++++++++++++--------------- src/conf/domain_conf.h | 2 +- src/libxl/libxl_conf.c | 9 ++- src/lxc/lxc_container.c | 6 +- src/qemu/qemu_command.c | 15 ++--- src/vbox/vbox_tmpl.c | 45 +++++++------ src/xenapi/xenapi_driver.c | 10 +-- src/xenapi/xenapi_utils.c | 22 +++---- src/xenxs/xen_sxpr.c | 20 +++--- src/xenxs/xen_xm.c | 30 ++++----- 10 files changed, 187 insertions(+), 131 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 365de77..58afdbf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11411,10 +11411,10 @@ virDomainDefParseXML(xmlDocPtr xml, _("unexpected feature '%s'"), nodes[i]->name); goto error; } - def->features |= (1 << val); - if (val == VIR_DOMAIN_FEATURE_APIC) { - tmp = virXPathString("string(./features/apic/@eoi)", ctxt); - if (tmp) { + + switch ((enum virDomainFeature) val) { + case VIR_DOMAIN_FEATURE_APIC: + if ((tmp = virXPathString("string(./features/apic/@eoi)", ctxt))) { int eoi; if ((eoi = virDomainFeatureStateTypeFromString(tmp)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -11425,11 +11425,23 @@ virDomainDefParseXML(xmlDocPtr xml, def->apic_eoi = eoi; VIR_FREE(tmp); } + /* fallthrough */ + case VIR_DOMAIN_FEATURE_ACPI: + case VIR_DOMAIN_FEATURE_PAE: + case VIR_DOMAIN_FEATURE_HAP: + case VIR_DOMAIN_FEATURE_VIRIDIAN: + case VIR_DOMAIN_FEATURE_PRIVNET: + case VIR_DOMAIN_FEATURE_HYPERV: + def->features[val] = VIR_DOMAIN_FEATURE_STATE_ON; + break; + + case VIR_DOMAIN_FEATURE_LAST: + break; } } VIR_FREE(nodes); - if (def->features & (1 << VIR_DOMAIN_FEATURE_HYPERV)) { + if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_DOMAIN_FEATURE_STATE_ON) { int feature; int value; node = ctxt->node; @@ -13401,12 +13413,16 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, { size_t i; - /* basic check */ - if (src->features != dst->features) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain features %d does not match source %d"), - dst->features, src->features); - return false; + for (i = 0; i < VIR_DOMAIN_FEATURE_LAST; i++) { + if (src->features[i] != dst->features[i]) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("State of feature '%s' differs: " + "source: '%s', destination: '%s'"), + virDomainFeatureTypeToString(i), + virDomainFeatureStateTypeToString(src->features[i]), + virDomainFeatureStateTypeToString(dst->features[i])); + return false; + } } /* APIC EOI */ @@ -13420,7 +13436,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, } /* hyperv */ - if (src->features & (1 << VIR_DOMAIN_FEATURE_HYPERV)) { + if (src->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_DOMAIN_FEATURE_STATE_ON) { for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) { switch ((enum virDomainHyperv) i) { case VIR_DOMAIN_HYPERV_RELAXED: @@ -16743,58 +16759,99 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, " </idmap>\n"); } + for (i = 0; i < VIR_DOMAIN_FEATURE_LAST; i++) { + if (def->features[i] != VIR_DOMAIN_FEATURE_STATE_DEFAULT) + break; + } - if (def->features) { + if (i != VIR_DOMAIN_FEATURE_LAST) { virBufferAddLit(buf, " <features>\n"); + for (i = 0; i < VIR_DOMAIN_FEATURE_LAST; i++) { - if (def->features & (1 << i) && i != VIR_DOMAIN_FEATURE_HYPERV) { - const char *name = virDomainFeatureTypeToString(i); - if (!name) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected feature %zu"), i); - goto error; - } - virBufferAsprintf(buf, " <%s", name); - if (i == VIR_DOMAIN_FEATURE_APIC && def->apic_eoi) { - virBufferAsprintf(buf, - " eoi='%s'", - virDomainFeatureStateTypeToString(def->apic_eoi)); - } - virBufferAddLit(buf, "/>\n"); + const char *name = virDomainFeatureTypeToString(i); + size_t j; + + if (!name) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected feature %zu"), i); + goto error; } - } - if (def->features & (1 << VIR_DOMAIN_FEATURE_HYPERV)) { - virBufferAddLit(buf, " <hyperv>\n"); - for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) { - switch ((enum virDomainHyperv) i) { - case VIR_DOMAIN_HYPERV_RELAXED: - case VIR_DOMAIN_HYPERV_VAPIC: - if (def->hyperv_features[i]) - virBufferAsprintf(buf, " <%s state='%s'/>\n", - virDomainHypervTypeToString(i), - virDomainFeatureStateTypeToString( - def->hyperv_features[i])); + switch ((enum virDomainFeature) i) { + case VIR_DOMAIN_FEATURE_ACPI: + case VIR_DOMAIN_FEATURE_PAE: + case VIR_DOMAIN_FEATURE_HAP: + case VIR_DOMAIN_FEATURE_VIRIDIAN: + case VIR_DOMAIN_FEATURE_PRIVNET: + switch ((enum virDomainFeatureState) def->features[i]) { + case VIR_DOMAIN_FEATURE_STATE_DEFAULT: break; - case VIR_DOMAIN_HYPERV_SPINLOCKS: - if (def->hyperv_features[i] == 0) - break; + case VIR_DOMAIN_FEATURE_STATE_ON: + virBufferAsprintf(buf, " <%s/>\n", name); + break; + + case VIR_DOMAIN_FEATURE_STATE_LAST: + case VIR_DOMAIN_FEATURE_STATE_OFF: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected state of feature '%s'"), name); + + goto error; + break; + } + + break; - virBufferAsprintf(buf, " <spinlocks state='%s'", - virDomainFeatureStateTypeToString( - def->hyperv_features[i])); - if (def->hyperv_features[i] == VIR_DOMAIN_FEATURE_STATE_ON) - virBufferAsprintf(buf, " retries='%d'", - def->hyperv_spinlocks); + case VIR_DOMAIN_FEATURE_APIC: + if (def->features[i] == VIR_DOMAIN_FEATURE_STATE_ON) { + virBufferAddLit(buf, " <apic"); + if (def->apic_eoi) { + virBufferAsprintf(buf, " eoi='%s'", + virDomainFeatureStateTypeToString(def->apic_eoi)); + } virBufferAddLit(buf, "/>\n"); - break; + } + break; - case VIR_DOMAIN_HYPERV_LAST: + case VIR_DOMAIN_FEATURE_HYPERV: + if (def->features[i] != VIR_DOMAIN_FEATURE_STATE_ON) break; + + virBufferAddLit(buf, " <hyperv>\n"); + for (j = 0; j < VIR_DOMAIN_HYPERV_LAST; j++) { + switch ((enum virDomainHyperv) j) { + case VIR_DOMAIN_HYPERV_RELAXED: + case VIR_DOMAIN_HYPERV_VAPIC: + if (def->hyperv_features[j]) + virBufferAsprintf(buf, " <%s state='%s'/>\n", + virDomainHypervTypeToString(j), + virDomainFeatureStateTypeToString( + def->hyperv_features[j])); + break; + + case VIR_DOMAIN_HYPERV_SPINLOCKS: + if (def->hyperv_features[j] == 0) + break; + + virBufferAsprintf(buf, " <spinlocks state='%s'", + virDomainFeatureStateTypeToString( + def->hyperv_features[j])); + if (def->hyperv_features[j] == VIR_DOMAIN_FEATURE_STATE_ON) + virBufferAsprintf(buf, " retries='%d'", + def->hyperv_spinlocks); + virBufferAddLit(buf, "/>\n"); + break; + + case VIR_DOMAIN_HYPERV_LAST: + break; + } } + virBufferAddLit(buf, " </hyperv>\n"); + break; + + case VIR_DOMAIN_FEATURE_LAST: + break; } - virBufferAddLit(buf, " </hyperv>\n"); } virBufferAddLit(buf, " </features>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4520ae4..ffd7e9d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1988,7 +1988,7 @@ struct _virDomainDef { virDomainOSDef os; char *emulator; - int features; + int features[VIR_DOMAIN_FEATURE_LAST]; /* enum virDomainFeatureState */ int apic_eoi; /* These options are of type virDomainFeatureState */ diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index d4226b8..79cf2b6 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -603,11 +603,14 @@ libxlMakeDomBuildInfo(virDomainObjPtr vm, libxl_domain_config *d_config) char bootorder[VIR_DOMAIN_BOOT_LAST + 1]; libxl_defbool_set(&b_info->u.hvm.pae, - def->features & (1 << VIR_DOMAIN_FEATURE_PAE)); + def->features[VIR_DOMAIN_FEATURE_PAE] == + VIR_DOMAIN_FEATURE_STATE_ON); libxl_defbool_set(&b_info->u.hvm.apic, - def->features & (1 << VIR_DOMAIN_FEATURE_APIC)); + def->features[VIR_DOMAIN_FEATURE_APIC] == + VIR_DOMAIN_FEATURE_STATE_ON); libxl_defbool_set(&b_info->u.hvm.acpi, - def->features & (1 << VIR_DOMAIN_FEATURE_ACPI)); + def->features[VIR_DOMAIN_FEATURE_ACPI] == + VIR_DOMAIN_FEATURE_STATE_ON); for (i = 0; i < def->clock.ntimers; i++) { if (def->clock.timers[i]->name == VIR_DOMAIN_TIMER_NAME_HPET && def->clock.timers[i]->present == 1) { diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 56df69e..610d131 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1867,8 +1867,8 @@ static int lxcContainerChild(void *data) } /* rename and enable interfaces */ - if (lxcContainerRenameAndEnableInterfaces(!!(vmDef->features & - (1 << VIR_DOMAIN_FEATURE_PRIVNET)), + if (lxcContainerRenameAndEnableInterfaces(vmDef->features[VIR_DOMAIN_FEATURE_PRIVNET] == + VIR_DOMAIN_FEATURE_STATE_ON, argv->nveths, argv->veths) < 0) { goto cleanup; @@ -1958,7 +1958,7 @@ lxcNeedNetworkNamespace(virDomainDefPtr def) size_t i; if (def->nets != NULL) return true; - if (def->features & (1 << VIR_DOMAIN_FEATURE_PRIVNET)) + if (def->features[VIR_DOMAIN_FEATURE_PRIVNET] == VIR_DOMAIN_FEATURE_STATE_ON) return true; for (i = 0; i < def->nhostdevs; i++) { if (def->hostdevs[i]->mode == VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES && diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index abb62e9..91ffacc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6718,7 +6718,7 @@ qemuBuildCpuArgStr(virQEMUDriverPtr driver, have_cpu = true; } - if (def->features & (1 << VIR_DOMAIN_FEATURE_HYPERV)) { + if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_DOMAIN_FEATURE_STATE_ON) { if (!have_cpu) { virBufferAdd(&buf, default_model, -1); have_cpu = true; @@ -8053,7 +8053,7 @@ qemuBuildCommandLine(virConnectPtr conn, } if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_ACPI)) { - if (!(def->features & (1 << VIR_DOMAIN_FEATURE_ACPI))) + if (def->features[VIR_DOMAIN_FEATURE_ACPI] != VIR_DOMAIN_FEATURE_STATE_ON) virCommandAddArg(cmd, "-no-acpi"); } @@ -10852,7 +10852,7 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, if (*feature == '\0') goto syntax; - dom->features |= (1 << VIR_DOMAIN_FEATURE_HYPERV); + dom->features[VIR_DOMAIN_FEATURE_HYPERV] = VIR_DOMAIN_FEATURE_STATE_ON; if ((f = virDomainHypervTypeFromString(feature)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -11106,7 +11106,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps, goto error; if (strstr(path, "kvm")) { def->virtType = VIR_DOMAIN_VIRT_KVM; - def->features |= (1 << VIR_DOMAIN_FEATURE_PAE); + def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_DOMAIN_FEATURE_STATE_ON; } } @@ -11119,8 +11119,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps, if ((def->os.arch == VIR_ARCH_I686) || (def->os.arch == VIR_ARCH_X86_64)) - def->features |= (1 << VIR_DOMAIN_FEATURE_ACPI) - /*| (1 << VIR_DOMAIN_FEATURE_APIC)*/; + def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_ON; #define WANT_VALUE() \ const char *val = progargv[++i]; \ @@ -11414,7 +11413,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps, def->disks[def->ndisks++] = disk; disk = NULL; } else if (STREQ(arg, "-no-acpi")) { - def->features &= ~(1 << VIR_DOMAIN_FEATURE_ACPI); + def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_DEFAULT; } else if (STREQ(arg, "-no-reboot")) { def->onReboot = VIR_DOMAIN_LIFECYCLE_DESTROY; } else if (STREQ(arg, "-no-kvm")) { @@ -11533,7 +11532,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps, def->mem.nosharepages = true; } else if (STRPREFIX(param, "accel=kvm")) { def->virtType = VIR_DOMAIN_VIRT_KVM; - def->features |= (1 << VIR_DOMAIN_FEATURE_PAE); + def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_DOMAIN_FEATURE_STATE_ON; } } virStringFreeList(list); diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index cf34f5c..187bb6d 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -2338,7 +2338,6 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { } } - def->features = 0; #if VBOX_API_VERSION < 3001 machine->vtbl->GetPAEEnabled(machine, &PAEEnabled); #elif VBOX_API_VERSION == 3001 @@ -2346,21 +2345,18 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { #elif VBOX_API_VERSION >= 3002 machine->vtbl->GetCPUProperty(machine, CPUPropertyType_PAE, &PAEEnabled); #endif - if (PAEEnabled) { - def->features = def->features | (1 << VIR_DOMAIN_FEATURE_PAE); - } + if (PAEEnabled) + def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_DOMAIN_FEATURE_STATE_ON; machine->vtbl->GetBIOSSettings(machine, &bios); if (bios) { bios->vtbl->GetACPIEnabled(bios, &ACPIEnabled); - if (ACPIEnabled) { - def->features = def->features | (1 << VIR_DOMAIN_FEATURE_ACPI); - } + if (ACPIEnabled) + def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_ON; bios->vtbl->GetIOAPICEnabled(bios, &IOAPICEnabled); - if (IOAPICEnabled) { - def->features = def->features | (1 << VIR_DOMAIN_FEATURE_APIC); - } + if (IOAPICEnabled) + def->features[VIR_DOMAIN_FEATURE_APIC] = VIR_DOMAIN_FEATURE_STATE_ON; VBOX_RELEASE(bios); } @@ -5076,40 +5072,43 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) { } #if VBOX_API_VERSION < 3001 - rc = machine->vtbl->SetPAEEnabled(machine, (def->features) & - (1 << VIR_DOMAIN_FEATURE_PAE)); + rc = machine->vtbl->SetPAEEnabled(machine, + def->features[VIR_DOMAIN_FEATURE_PAE] == + VIR_DOMAIN_FEATURE_STATE_ON); #elif VBOX_API_VERSION == 3001 rc = machine->vtbl->SetCpuProperty(machine, CpuPropertyType_PAE, - (def->features) & - (1 << VIR_DOMAIN_FEATURE_PAE)); + def->features[VIR_DOMAIN_FEATURE_PAE] == + VIR_DOMAIN_FEATURE_STATE_ON); #elif VBOX_API_VERSION >= 3002 rc = machine->vtbl->SetCPUProperty(machine, CPUPropertyType_PAE, - (def->features) & - (1 << VIR_DOMAIN_FEATURE_PAE)); + def->features[VIR_DOMAIN_FEATURE_PAE] == + VIR_DOMAIN_FEATURE_STATE_ON); #endif if (NS_FAILED(rc)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("could not change PAE status to: %s, rc=%08x"), - ((def->features) & (1 << VIR_DOMAIN_FEATURE_PAE)) + (def->features[VIR_DOMAIN_FEATURE_PAE] == VIR_DOMAIN_FEATURE_STATE_ON) ? _("Enabled") : _("Disabled"), (unsigned)rc); } machine->vtbl->GetBIOSSettings(machine, &bios); if (bios) { - rc = bios->vtbl->SetACPIEnabled(bios, (def->features) & - (1 << VIR_DOMAIN_FEATURE_ACPI)); + rc = bios->vtbl->SetACPIEnabled(bios, + def->features[VIR_DOMAIN_FEATURE_ACPI] == + VIR_DOMAIN_FEATURE_STATE_ON); if (NS_FAILED(rc)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("could not change ACPI status to: %s, rc=%08x"), - ((def->features) & (1 << VIR_DOMAIN_FEATURE_ACPI)) + (def->features[VIR_DOMAIN_FEATURE_ACPI] == VIR_DOMAIN_FEATURE_STATE_ON) ? _("Enabled") : _("Disabled"), (unsigned)rc); } - rc = bios->vtbl->SetIOAPICEnabled(bios, (def->features) & - (1 << VIR_DOMAIN_FEATURE_APIC)); + rc = bios->vtbl->SetIOAPICEnabled(bios, + def->features[VIR_DOMAIN_FEATURE_APIC] == + VIR_DOMAIN_FEATURE_STATE_ON); if (NS_FAILED(rc)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("could not change APIC status to: %s, rc=%08x"), - ((def->features) & (1 << VIR_DOMAIN_FEATURE_APIC)) + (def->features[VIR_DOMAIN_FEATURE_APIC] == VIR_DOMAIN_FEATURE_STATE_ON) ? _("Enabled") : _("Disabled"), (unsigned)rc); } VBOX_RELEASE(bios); diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 4b522c0..df926a6 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1474,15 +1474,15 @@ xenapiDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) for (i = 0; i < result->size; i++) { if (STREQ(result->contents[i].val, "true")) { if (STREQ(result->contents[i].key, "acpi")) - defPtr->features = defPtr->features | (1<<VIR_DOMAIN_FEATURE_ACPI); + defPtr->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_ON; else if (STREQ(result->contents[i].key, "apic")) - defPtr->features = defPtr->features | (1<<VIR_DOMAIN_FEATURE_APIC); + defPtr->features[VIR_DOMAIN_FEATURE_APIC] = VIR_DOMAIN_FEATURE_STATE_ON; else if (STREQ(result->contents[i].key, "pae")) - defPtr->features = defPtr->features | (1<<VIR_DOMAIN_FEATURE_PAE); + defPtr->features[VIR_DOMAIN_FEATURE_PAE] = VIR_DOMAIN_FEATURE_STATE_ON; else if (STREQ(result->contents[i].key, "hap")) - defPtr->features = defPtr->features | (1<<VIR_DOMAIN_FEATURE_HAP); + defPtr->features[VIR_DOMAIN_FEATURE_HAP] = VIR_DOMAIN_FEATURE_STATE_ON; else if (STREQ(result->contents[i].key, "viridian")) - defPtr->features = defPtr->features | (1<<VIR_DOMAIN_FEATURE_VIRIDIAN); + defPtr->features[VIR_DOMAIN_FEATURE_VIRIDIAN] = VIR_DOMAIN_FEATURE_STATE_ON; } } xen_string_string_map_free(result); diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c index 91ae54b..02d4885 100644 --- a/src/xenapi/xenapi_utils.c +++ b/src/xenapi/xenapi_utils.c @@ -511,18 +511,16 @@ createVMRecordFromXml(virConnectPtr conn, virDomainDefPtr def, if (def->onCrash) (*record)->actions_after_crash = actionCrashLibvirt2XenapiEnum(def->onCrash); - if (def->features) { - if (def->features & (1 << VIR_DOMAIN_FEATURE_ACPI)) - allocStringMap(&strings, (char *)"acpi", (char *)"true"); - if (def->features & (1 << VIR_DOMAIN_FEATURE_APIC)) - allocStringMap(&strings, (char *)"apic", (char *)"true"); - if (def->features & (1 << VIR_DOMAIN_FEATURE_PAE)) - allocStringMap(&strings, (char *)"pae", (char *)"true"); - if (def->features & (1 << VIR_DOMAIN_FEATURE_HAP)) - allocStringMap(&strings, (char *)"hap", (char *)"true"); - if (def->features & (1 << VIR_DOMAIN_FEATURE_VIRIDIAN)) - allocStringMap(&strings, (char *)"viridian", (char *)"true"); - } + if (def->features[VIR_DOMAIN_FEATURE_ACPI] == VIR_DOMAIN_FEATURE_STATE_ON) + allocStringMap(&strings, (char *)"acpi", (char *)"true"); + if (def->features[VIR_DOMAIN_FEATURE_APIC] == VIR_DOMAIN_FEATURE_STATE_ON) + allocStringMap(&strings, (char *)"apic", (char *)"true"); + if (def->features[VIR_DOMAIN_FEATURE_PAE] == VIR_DOMAIN_FEATURE_STATE_ON) + allocStringMap(&strings, (char *)"pae", (char *)"true"); + if (def->features[VIR_DOMAIN_FEATURE_HAP] == VIR_DOMAIN_FEATURE_STATE_ON) + allocStringMap(&strings, (char *)"hap", (char *)"true"); + if (def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] == VIR_DOMAIN_FEATURE_STATE_ON) + allocStringMap(&strings, (char *)"viridian", (char *)"true"); if (strings != NULL) (*record)->platform = strings; diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index 3cbe958..d23a3ca 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1202,15 +1202,15 @@ xenParseSxpr(const struct sexpr *root, if (hvm) { if (sexpr_int(root, "domain/image/hvm/acpi")) - def->features |= (1 << VIR_DOMAIN_FEATURE_ACPI); + def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_ON; if (sexpr_int(root, "domain/image/hvm/apic")) - def->features |= (1 << VIR_DOMAIN_FEATURE_APIC); + def->features[VIR_DOMAIN_FEATURE_APIC] = VIR_DOMAIN_FEATURE_STATE_ON; if (sexpr_int(root, "domain/image/hvm/pae")) - def->features |= (1 << VIR_DOMAIN_FEATURE_PAE); + def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_DOMAIN_FEATURE_STATE_ON; if (sexpr_int(root, "domain/image/hvm/hap")) - def->features |= (1 << VIR_DOMAIN_FEATURE_HAP); + def->features[VIR_DOMAIN_FEATURE_HAP] = VIR_DOMAIN_FEATURE_STATE_ON; if (sexpr_int(root, "domain/image/hvm/viridian")) - def->features |= (1 << VIR_DOMAIN_FEATURE_VIRIDIAN); + def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] = VIR_DOMAIN_FEATURE_STATE_ON; } /* 12aaf4a2486b (3.0.3) added a second low-priority 'localtime' setting */ @@ -2333,15 +2333,15 @@ xenFormatSxpr(virConnectPtr conn, } } - if (def->features & (1 << VIR_DOMAIN_FEATURE_ACPI)) + if (def->features[VIR_DOMAIN_FEATURE_ACPI] == VIR_DOMAIN_FEATURE_STATE_ON) virBufferAddLit(&buf, "(acpi 1)"); - if (def->features & (1 << VIR_DOMAIN_FEATURE_APIC)) + if (def->features[VIR_DOMAIN_FEATURE_APIC] == VIR_DOMAIN_FEATURE_STATE_ON) virBufferAddLit(&buf, "(apic 1)"); - if (def->features & (1 << VIR_DOMAIN_FEATURE_PAE)) + if (def->features[VIR_DOMAIN_FEATURE_PAE] == VIR_DOMAIN_FEATURE_STATE_ON) virBufferAddLit(&buf, "(pae 1)"); - if (def->features & (1 << VIR_DOMAIN_FEATURE_HAP)) + if (def->features[VIR_DOMAIN_FEATURE_HAP] == VIR_DOMAIN_FEATURE_STATE_ON) virBufferAddLit(&buf, "(hap 1)"); - if (def->features & (1 << VIR_DOMAIN_FEATURE_VIRIDIAN)) + if (def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] == VIR_DOMAIN_FEATURE_STATE_ON) virBufferAddLit(&buf, "(viridian 1)"); virBufferAddLit(&buf, "(usb 1)"); diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index 9e07f95..88374f4 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -398,23 +398,23 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, if (xenXMConfigGetBool(conf, "pae", &val, 0) < 0) goto cleanup; else if (val) - def->features |= (1 << VIR_DOMAIN_FEATURE_PAE); + def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_DOMAIN_FEATURE_STATE_ON; if (xenXMConfigGetBool(conf, "acpi", &val, 0) < 0) goto cleanup; else if (val) - def->features |= (1 << VIR_DOMAIN_FEATURE_ACPI); + def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_ON; if (xenXMConfigGetBool(conf, "apic", &val, 0) < 0) goto cleanup; else if (val) - def->features |= (1 << VIR_DOMAIN_FEATURE_APIC); + def->features[VIR_DOMAIN_FEATURE_APIC] = VIR_DOMAIN_FEATURE_STATE_ON; if (xenXMConfigGetBool(conf, "hap", &val, 0) < 0) goto cleanup; else if (val) - def->features |= (1 << VIR_DOMAIN_FEATURE_HAP); + def->features[VIR_DOMAIN_FEATURE_HAP] = VIR_DOMAIN_FEATURE_STATE_ON; if (xenXMConfigGetBool(conf, "viridian", &val, 0) < 0) goto cleanup; else if (val) - def->features |= (1 << VIR_DOMAIN_FEATURE_VIRIDIAN); + def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] = VIR_DOMAIN_FEATURE_STATE_ON; if (xenXMConfigGetBool(conf, "hpet", &val, -1) < 0) goto cleanup; @@ -1571,29 +1571,29 @@ virConfPtr xenFormatXM(virConnectPtr conn, goto cleanup; if (xenXMConfigSetInt(conf, "pae", - (def->features & - (1 << VIR_DOMAIN_FEATURE_PAE)) ? 1 : 0) < 0) + (def->features[VIR_DOMAIN_FEATURE_PAE] == + VIR_DOMAIN_FEATURE_STATE_ON) ? 1 : 0) < 0) goto cleanup; if (xenXMConfigSetInt(conf, "acpi", - (def->features & - (1 << VIR_DOMAIN_FEATURE_ACPI)) ? 1 : 0) < 0) + (def->features[VIR_DOMAIN_FEATURE_ACPI] == + VIR_DOMAIN_FEATURE_STATE_ON) ? 1 : 0) < 0) goto cleanup; if (xenXMConfigSetInt(conf, "apic", - (def->features & - (1 << VIR_DOMAIN_FEATURE_APIC)) ? 1 : 0) < 0) + (def->features[VIR_DOMAIN_FEATURE_APIC] == + VIR_DOMAIN_FEATURE_STATE_ON) ? 1 : 0) < 0) goto cleanup; if (xendConfigVersion >= XEND_CONFIG_VERSION_3_0_4) { if (xenXMConfigSetInt(conf, "hap", - (def->features & - (1 << VIR_DOMAIN_FEATURE_HAP)) ? 1 : 0) < 0) + (def->features[VIR_DOMAIN_FEATURE_HAP] == + VIR_DOMAIN_FEATURE_STATE_ON) ? 1 : 0) < 0) goto cleanup; if (xenXMConfigSetInt(conf, "viridian", - (def->features & - (1 << VIR_DOMAIN_FEATURE_VIRIDIAN)) ? 1 : 0) < 0) + (def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] == + VIR_DOMAIN_FEATURE_STATE_ON) ? 1 : 0) < 0) goto cleanup; } -- 1.8.3.2

The linux kernel recently added support for paravirtual spinlock handling to avoid performance regressions on overcomitted hosts. This feature needs to be turned in the hypervisor so that the guest OS is notified about the possible support. This patch adds a new feature "paravirt-spinlock" to the XML and supporting code to enable the "kvm_pv_unhalt" pseudoCPU feature in qemu. https://bugzilla.redhat.com/show_bug.cgi?id=1008989 --- docs/formatdomain.html.in | 7 +++++ docs/schemas/domaincommon.rng | 10 +++++- src/conf/domain_conf.c | 36 +++++++++++++++++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 14 +++++++++ .../qemuxml2argv-pv-spinlock-disabled.args | 5 +++ .../qemuxml2argv-pv-spinlock-disabled.xml | 26 ++++++++++++++++ .../qemuxml2argv-pv-spinlock-enabled.args | 5 +++ .../qemuxml2argv-pv-spinlock-enabled.xml | 26 ++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ tests/qemuxml2xmltest.c | 2 ++ 11 files changed, 132 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c5a3fa8..04cb546 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1184,6 +1184,7 @@ &lt;vapic state='on'/&gt; &lt;spinlocks state='on' retries='4096'/&gt; &lt;/hyperv&gt; + &lt;paravirt-spinlock/&gt; &lt;/features&gt; ...</pre> @@ -1255,6 +1256,12 @@ </tr> </table> </dd> + <dt><code>paravirt-spinlock</code></dt> + <dd>Notify the guest that the host supports paravirtual spinlocks + for example by exposing the pvticketlocks mechanism. This feature + can be forced of by using <code>state='off'</code> attribute. + </dd> + </dl> <h3><a name="elementsTime">Time keeping</a></h3> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 14b6700..43a0fc8 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3561,7 +3561,7 @@ </define> <!-- A set of optional features: PAE, APIC, ACPI, - HyperV Enlightenment and HAP support + HyperV Enlightenment, paravirtual spinlocks and HAP support --> <define name="features"> <optional> @@ -3607,6 +3607,14 @@ <empty/> </element> </optional> + <optional> + <element name="paravirt-spinlock"> + <optional> + <ref name="featurestate"/> + </optional> + <empty/> + </element> + </optional> </interleave> </element> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 58afdbf..c3874a6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -142,7 +142,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, "hap", "viridian", "privnet", - "hyperv") + "hyperv", + "paravirt-spinlock") VIR_ENUM_IMPL(virDomainFeatureState, VIR_DOMAIN_FEATURE_STATE_LAST, "default", @@ -11435,6 +11436,22 @@ virDomainDefParseXML(xmlDocPtr xml, def->features[val] = VIR_DOMAIN_FEATURE_STATE_ON; break; + case VIR_DOMAIN_FEATURE_PARAVIRT_SPINLOCK: + node = ctxt->node; + ctxt->node = nodes[i]; + if ((tmp = virXPathString("string(./@state)", ctxt))) { + if ((def->features[val] = virDomainFeatureStateTypeFromString(tmp)) == -1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown state atribute '%s' of feature '%s'"), + tmp, virDomainFeatureTypeToString(val)); + goto error; + } + } else { + def->features[val] = VIR_DOMAIN_FEATURE_STATE_ON; + } + ctxt->node = node; + break; + case VIR_DOMAIN_FEATURE_LAST: break; } @@ -16802,6 +16819,23 @@ virDomainDefFormatInternal(virDomainDefPtr def, break; + case VIR_DOMAIN_FEATURE_PARAVIRT_SPINLOCK: + switch ((enum virDomainFeatureState) def->features[i]) { + case VIR_DOMAIN_FEATURE_STATE_LAST: + case VIR_DOMAIN_FEATURE_STATE_DEFAULT: + break; + + case VIR_DOMAIN_FEATURE_STATE_ON: + virBufferAsprintf(buf, " <%s/>\n", name); + break; + + case VIR_DOMAIN_FEATURE_STATE_OFF: + virBufferAsprintf(buf, " <%s state='off'/>\n", name); + break; + } + + break; + case VIR_DOMAIN_FEATURE_APIC: if (def->features[i] == VIR_DOMAIN_FEATURE_STATE_ON) { virBufferAddLit(buf, " <apic"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ffd7e9d..b1ee737 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1619,6 +1619,7 @@ enum virDomainFeature { VIR_DOMAIN_FEATURE_VIRIDIAN, VIR_DOMAIN_FEATURE_PRIVNET, VIR_DOMAIN_FEATURE_HYPERV, + VIR_DOMAIN_FEATURE_PARAVIRT_SPINLOCK, VIR_DOMAIN_FEATURE_LAST }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 91ffacc..7e56547 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6718,6 +6718,20 @@ qemuBuildCpuArgStr(virQEMUDriverPtr driver, have_cpu = true; } + if (def->features[VIR_DOMAIN_FEATURE_PARAVIRT_SPINLOCK]) { + char sign; + if (def->features[VIR_DOMAIN_FEATURE_PARAVIRT_SPINLOCK] == + VIR_DOMAIN_FEATURE_STATE_ON) + sign = '+'; + else + sign = '-'; + + virBufferAsprintf(&buf, "%s,%ckvm_pv_unhalt", + have_cpu ? "" : default_model, + sign); + have_cpu = true; + } + if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_DOMAIN_FEATURE_STATE_ON) { if (!have_cpu) { virBufferAdd(&buf, default_model, -1); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.args b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.args new file mode 100644 index 0000000..80047f9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.args @@ -0,0 +1,5 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc \ +-cpu qemu32,-kvm_pv_unhalt -m 214 -smp 6 -nographic -monitor \ +unix:/tmp/test-monitor,server,nowait -boot n -usb -net none -serial \ +none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.xml b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.xml new file mode 100644 index 0000000..afb0b41 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.xml @@ -0,0 +1,26 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>6</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <features> + <acpi/> + <pae/> + <paravirt-spinlock state='off'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.args b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.args new file mode 100644 index 0000000..70db173 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.args @@ -0,0 +1,5 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc \ +-cpu qemu32,+kvm_pv_unhalt -m 214 -smp 6 -nographic -monitor \ +unix:/tmp/test-monitor,server,nowait -boot n -usb -net none -serial \ +none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.xml b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.xml new file mode 100644 index 0000000..f29cade --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.xml @@ -0,0 +1,26 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>6</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <features> + <acpi/> + <pae/> + <paravirt-spinlock/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 060acf2..d716c58 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -448,6 +448,8 @@ mymain(void) QEMU_CAPS_CHARDEV_SPICEVMC, QEMU_CAPS_SPICE, QEMU_CAPS_HDA_DUPLEX); DO_TEST("eoi-disabled", NONE); DO_TEST("eoi-enabled", NONE); + DO_TEST("pv-spinlock-disabled", NONE); + DO_TEST("pv-spinlock-enabled", NONE); DO_TEST("kvmclock+eoi-disabled", QEMU_CAPS_ENABLE_KVM); DO_TEST("hyperv", NONE); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 4e308b4..ffff3b5 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -159,6 +159,8 @@ mymain(void) DO_TEST("cpu-eoi-enabled"); DO_TEST("eoi-disabled"); DO_TEST("eoi-enabled"); + DO_TEST("pv-spinlock-disabled"); + DO_TEST("pv-spinlock-enabled"); DO_TEST("hyperv"); DO_TEST("hyperv-off"); -- 1.8.3.2

When starting a VM the qemu process may filter out some requested features of a domain as it's not supported either by the host or by qemu. Libvirt didn't check if this happened which might end up in changing of the guest ABI when migrating. The proof of concept implementation adds the check for the recently introduced kvm_pv_unhalt cpuid feature bit. This feature depends on both qemu and host kernel support and thus increase the possibility of guest ABI breakage. --- src/qemu/qemu_process.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 20d8394..b7d7cff 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -44,6 +44,7 @@ #include "qemu_bridge_filter.h" #include "qemu_migration.h" +#include "cpu/cpu.h" #include "datatypes.h" #include "virlog.h" #include "virerror.h" @@ -3473,6 +3474,43 @@ qemuValidateCpuMax(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) return true; } + +static bool +qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver, virDomainObjPtr vm) +{ + virDomainDefPtr def = vm->def; + virArch arch = def->os.arch; + virCPUDataPtr guestcpu = NULL; + qemuDomainObjPrivatePtr priv = vm->privateData; + bool ret = false; + + if (arch == VIR_ARCH_I686 || arch == VIR_ARCH_X86_64) { + qemuDomainObjEnterMonitor(driver, vm); + guestcpu = qemuMonitorGetGuestCPU(priv->mon); + qemuDomainObjExitMonitor(driver, vm); + + if (!guestcpu) { + virResetLastError(); + return true; + } + + if (def->features[VIR_DOMAIN_FEATURE_PARAVIRT_SPINLOCK] == VIR_DOMAIN_FEATURE_STATE_ON) { + if (!cpuHasFeature(guestcpu, VIR_CPU_x86_KVM_PV_UNHALT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("host doesn't support paravirtual spinlocks")); + goto cleanup; + } + } + } + + ret = true; + +cleanup: + cpuDataFree(guestcpu); + return ret; +} + + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -3940,6 +3978,10 @@ int qemuProcessStart(virConnectPtr conn, priv->agentError = true; } + VIR_DEBUG("Detecting if required emulator features are present"); + if (!qemuProcessVerifyGuestCPU(driver, vm)) + goto cleanup; + VIR_DEBUG("Detecting VCPU PIDs"); if (qemuProcessDetectVcpuPIDs(driver, vm) < 0) goto cleanup; -- 1.8.3.2