[libvirt] [PATCHv3 0/6] Introduce paravirtual spinlock support
Version 3 is fixed according to the review feedback. Please see individual patches for changelog. Jiri Denemark (1): qemu: Add monitor APIs to fetch CPUID data from QEMU Peter Krempa (5): cpu_x86: Refactor storage of CPUID data to add support for KVM features cpu: x86: Add internal CPUID features support and KVM feature bits conf: Refactor storing and usage of feature flags qemu: Add support for paravirtual spinlocks in the guest qemu: process: Validate specific CPUID flags of a guest docs/formatdomain.html.in | 8 + docs/schemas/domaincommon.rng | 10 +- src/conf/domain_conf.c | 195 +++++++++++---- src/conf/domain_conf.h | 3 +- src/cpu/cpu_x86.c | 277 +++++++++++---------- src/cpu/cpu_x86_data.h | 20 +- src/libxl/libxl_conf.c | 9 +- src/lxc/lxc_container.c | 6 +- src/qemu/qemu_command.c | 28 ++- src/qemu/qemu_monitor.c | 31 +++ src/qemu/qemu_monitor.h | 4 + src/qemu/qemu_monitor_json.c | 133 ++++++++++ src/qemu/qemu_monitor_json.h | 2 + src/qemu/qemu_process.c | 46 ++++ 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 +-- tests/Makefile.am | 1 + .../qemumonitorjson-getcpu-full.data | 5 + .../qemumonitorjson-getcpu-full.json | 46 ++++ .../qemumonitorjson-getcpu-host.data | 6 + .../qemumonitorjson-getcpu-host.json | 45 ++++ tests/qemumonitorjsontest.c | 75 ++++++ .../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 + 31 files changed, 869 insertions(+), 274 deletions(-) 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 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 -- 1.8.4.2
The CPUID functions were stored in multiple arrays according to a specified prefix of those. This made it very hard to add another prefix to store KVM CPUID features (0x40000000). Instead of hardcoding a third array this patch changes the approach used: The code is refactored to use a single array where the CPUID functions are stored ordered by the cpuid function so that they don't depend on the specific prefix and don't waste memory. The code is also less complex using this approach. A trateoff to this is the change from O(N) complexity to O(N^2) in x86DataAdd and x86DataSubtract. The rest of the functions were already using O(N^2) algorithms. --- Notes: Version 3: - no change, still waiting on an additional review Version 2: - no change, weak ACK from Daniel src/cpu/cpu_x86.c | 213 ++++++++++++++++++------------------------------- src/cpu/cpu_x86_data.h | 8 +- 2 files changed, 80 insertions(+), 141 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 1785665..ba6a2b0 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -84,14 +84,13 @@ enum compare_result { struct virCPUx86DataIterator { - virCPUx86Data *data; + const virCPUx86Data *data; int pos; - bool extended; }; #define virCPUx86DataIteratorInit(data) \ - { data, -1, false } + { data, -1 } static bool @@ -120,6 +119,9 @@ static void x86cpuidSetBits(virCPUx86CPUID *cpuid, const virCPUx86CPUID *mask) { + if (!mask) + return; + cpuid->eax |= mask->eax; cpuid->ebx |= mask->ebx; cpuid->ecx |= mask->ecx; @@ -131,6 +133,9 @@ static void x86cpuidClearBits(virCPUx86CPUID *cpuid, const virCPUx86CPUID *mask) { + if (!mask) + return; + cpuid->eax &= ~mask->eax; cpuid->ebx &= ~mask->ebx; cpuid->ecx &= ~mask->ecx; @@ -142,42 +147,45 @@ static void x86cpuidAndBits(virCPUx86CPUID *cpuid, const virCPUx86CPUID *mask) { + if (!mask) + return; + cpuid->eax &= mask->eax; cpuid->ebx &= mask->ebx; cpuid->ecx &= mask->ecx; cpuid->edx &= mask->edx; } +static int +virCPUx86CPUIDSorter(const void *a, const void *b) +{ + virCPUx86CPUID *da = (virCPUx86CPUID *) a; + virCPUx86CPUID *db = (virCPUx86CPUID *) b; + + if (da->function > db->function) + return 1; + else if (da->function < db->function) + return -1; + + return 0; +} + /* skips all zero CPUID leafs */ static virCPUx86CPUID * x86DataCpuidNext(struct virCPUx86DataIterator *iterator) { - virCPUx86CPUID *ret; - virCPUx86Data *data = iterator->data; + const virCPUx86Data *data = iterator->data; if (!data) return NULL; - do { - ret = NULL; - iterator->pos++; - - if (!iterator->extended) { - if (iterator->pos < data->basic_len) - ret = data->basic + iterator->pos; - else { - iterator->extended = true; - iterator->pos = 0; - } - } - - if (iterator->extended && iterator->pos < data->extended_len) { - ret = data->extended + iterator->pos; - } - } while (ret && x86cpuidMatch(ret, &cpuidNull)); + while (++iterator->pos < data->len) { + if (!x86cpuidMatch(data->data + iterator->pos, &cpuidNull)) + return data->data + iterator->pos; + } - return ret; + return NULL; } @@ -185,36 +193,23 @@ static virCPUx86CPUID * x86DataCpuid(const virCPUx86Data *data, uint32_t function) { - virCPUx86CPUID *cpuids; - int len; size_t i; - if (function < CPUX86_EXTENDED) { - cpuids = data->basic; - len = data->basic_len; - i = function; - } - else { - cpuids = data->extended; - len = data->extended_len; - i = function - CPUX86_EXTENDED; + for (i = 0; i < data->len; i++) { + if (data->data[i].function == function) + return data->data + i; } - if (i < len && !x86cpuidMatch(cpuids + i, &cpuidNull)) - return cpuids + i; - else - return NULL; + return NULL; } - void virCPUx86DataFree(virCPUx86Data *data) { if (data == NULL) return; - VIR_FREE(data->basic); - VIR_FREE(data->extended); + VIR_FREE(data->data); VIR_FREE(data); } @@ -251,77 +246,36 @@ x86DataCopy(const virCPUx86Data *data) virCPUx86Data *copy = NULL; size_t i; - if (VIR_ALLOC(copy) < 0 - || VIR_ALLOC_N(copy->basic, data->basic_len) < 0 - || VIR_ALLOC_N(copy->extended, data->extended_len) < 0) { + if (VIR_ALLOC(copy) < 0 || + VIR_ALLOC_N(copy->data, data->len) < 0) { virCPUx86DataFree(copy); return NULL; } - copy->basic_len = data->basic_len; - for (i = 0; i < data->basic_len; i++) - copy->basic[i] = data->basic[i]; - - copy->extended_len = data->extended_len; - for (i = 0; i < data->extended_len; i++) - copy->extended[i] = data->extended[i]; + copy->len = data->len; + for (i = 0; i < data->len; i++) + copy->data[i] = data->data[i]; return copy; } -static int -x86DataExpand(virCPUx86Data *data, - int basic_by, - int extended_by) -{ - size_t i; - - if (basic_by > 0) { - size_t len = data->basic_len; - if (VIR_EXPAND_N(data->basic, data->basic_len, basic_by) < 0) - return -1; - - for (i = 0; i < basic_by; i++) - data->basic[len + i].function = len + i; - } - - if (extended_by > 0) { - size_t len = data->extended_len; - if (VIR_EXPAND_N(data->extended, data->extended_len, extended_by) < 0) - return -1; - - for (i = 0; i < extended_by; i++) - data->extended[len + i].function = len + i + CPUX86_EXTENDED; - } - - return 0; -} - - int virCPUx86DataAddCPUID(virCPUx86Data *data, const virCPUx86CPUID *cpuid) { - unsigned int basic_by = 0; - unsigned int extended_by = 0; - virCPUx86CPUID **cpuids; - unsigned int pos; - - if (cpuid->function < CPUX86_EXTENDED) { - pos = cpuid->function; - basic_by = pos + 1 - data->basic_len; - cpuids = &data->basic; - } else { - pos = cpuid->function - CPUX86_EXTENDED; - extended_by = pos + 1 - data->extended_len; - cpuids = &data->extended; - } + virCPUx86CPUID *existing; - if (x86DataExpand(data, basic_by, extended_by) < 0) - return -1; + if ((existing = x86DataCpuid(data, cpuid->function))) { + x86cpuidSetBits(existing, cpuid); + } else { + if (VIR_APPEND_ELEMENT_COPY(data->data, data->len, + *((virCPUx86CPUID *)cpuid)) < 0) + return -1; - x86cpuidSetBits((*cpuids) + pos, cpuid); + qsort(data->data, data->len, + sizeof(virCPUx86CPUID), virCPUx86CPUIDSorter); + } return 0; } @@ -331,21 +285,19 @@ static int x86DataAdd(virCPUx86Data *data1, const virCPUx86Data *data2) { - size_t i; - - if (x86DataExpand(data1, - data2->basic_len - data1->basic_len, - data2->extended_len - data1->extended_len) < 0) - return -1; + struct virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data2); + virCPUx86CPUID *cpuid1; + virCPUx86CPUID *cpuid2; - for (i = 0; i < data2->basic_len; i++) { - x86cpuidSetBits(data1->basic + i, - data2->basic + i); - } + while ((cpuid2 = x86DataCpuidNext(&iter))) { + cpuid1 = x86DataCpuid(data1, cpuid2->function); - for (i = 0; i < data2->extended_len; i++) { - x86cpuidSetBits(data1->extended + i, - data2->extended + i); + if (cpuid1) { + x86cpuidSetBits(cpuid1, cpuid2); + } else { + if (virCPUx86DataAddCPUID(data1, cpuid2) < 0) + return -1; + } } return 0; @@ -356,19 +308,13 @@ static void x86DataSubtract(virCPUx86Data *data1, const virCPUx86Data *data2) { - size_t i; - unsigned int len; - - len = MIN(data1->basic_len, data2->basic_len); - for (i = 0; i < len; i++) { - x86cpuidClearBits(data1->basic + i, - data2->basic + i); - } + struct virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data1); + virCPUx86CPUID *cpuid1; + virCPUx86CPUID *cpuid2; - len = MIN(data1->extended_len, data2->extended_len); - for (i = 0; i < len; i++) { - x86cpuidClearBits(data1->extended + i, - data2->extended + i); + while ((cpuid1 = x86DataCpuidNext(&iter))) { + cpuid2 = x86DataCpuid(data2, cpuid1->function); + x86cpuidClearBits(cpuid1, cpuid2); } } @@ -1763,25 +1709,23 @@ cpuidCall(virCPUx86CPUID *cpuid) static int -cpuidSet(uint32_t base, virCPUx86CPUID **set) +cpuidSet(uint32_t base, virCPUx86Data *data) { uint32_t max; uint32_t i; virCPUx86CPUID cpuid = { base, 0, 0, 0, 0 }; cpuidCall(&cpuid); - max = cpuid.eax - base; + max = cpuid.eax; - if (VIR_ALLOC_N(*set, max + 1) < 0) - return -1; - - for (i = 0; i <= max; i++) { - cpuid.function = base | i; + for (i = base; i <= max; i++) { + cpuid.function = i; cpuidCall(&cpuid); - (*set)[i] = cpuid; + if (virCPUx86DataAddCPUID(data, &cpuid) < 0) + return -1; } - return max + 1; + return 0; } @@ -1790,18 +1734,15 @@ x86NodeData(virArch arch) { virCPUDataPtr cpuData = NULL; virCPUx86Data *data; - int ret; if (VIR_ALLOC(data) < 0) return NULL; - if ((ret = cpuidSet(CPUX86_BASIC, &data->basic)) < 0) + if (cpuidSet(CPUX86_BASIC, data) < 0) goto error; - data->basic_len = ret; - if ((ret = cpuidSet(CPUX86_EXTENDED, &data->extended)) < 0) + if (cpuidSet(CPUX86_EXTENDED, data) < 0) goto error; - data->extended_len = ret; if (!(cpuData = virCPUx86MakeData(arch, &data))) goto error; diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h index 5910ea9..69066f1 100644 --- a/src/cpu/cpu_x86_data.h +++ b/src/cpu/cpu_x86_data.h @@ -1,7 +1,7 @@ /* * cpu_x86_data.h: x86 specific CPU data * - * Copyright (C) 2009-2010 Red Hat, Inc. + * Copyright (C) 2009-2010, 2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -40,10 +40,8 @@ struct _virCPUx86CPUID { typedef struct _virCPUx86Data virCPUx86Data; struct _virCPUx86Data { - size_t basic_len; - virCPUx86CPUID *basic; - size_t extended_len; - virCPUx86CPUID *extended; + size_t len; + virCPUx86CPUID *data; }; #endif /* __VIR_CPU_X86_DATA_H__ */ -- 1.8.4.2
On Mon, Nov 04, 2013 at 14:55:02 +0100, Peter Krempa wrote:
The CPUID functions were stored in multiple arrays according to a specified prefix of those. This made it very hard to add another prefix to store KVM CPUID features (0x40000000). Instead of hardcoding a third array this patch changes the approach used:
The code is refactored to use a single array where the CPUID functions are stored ordered by the cpuid function so that they don't depend on the specific prefix and don't waste memory. The code is also less complex using this approach. A trateoff to this is the change from O(N) complexity to O(N^2) in x86DataAdd and x86DataSubtract. The rest of the functions were already using O(N^2) algorithms.
It's always fun reviewing patches that try to remove relics of my first libvirt commit made 4 years ago :-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 1785665..ba6a2b0 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -84,14 +84,13 @@ enum compare_result {
struct virCPUx86DataIterator { - virCPUx86Data *data; + const virCPUx86Data *data; int pos; - bool extended; };
Why do we need to keep the iterator at all when everything is now stored in a single array? But that's mostly an additional cleanup to be done. ... Good thing is the cputest passes after this series. ACK Jirka
From: Jiri Denemark <jdenemar@redhat.com> 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@redhat.com> --- Notes: Version 3: - removed unneeded error state - fixed typo in error message Version 2: - unified global and JSON monitor func signatures - changed return values so that errors can be differenitated from lack of support - code is conditionally run only when architecture matches src/qemu/qemu_monitor.c | 31 +++++ src/qemu/qemu_monitor.h | 4 + src/qemu/qemu_monitor_json.c | 133 +++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 2 + tests/Makefile.am | 1 + .../qemumonitorjson-getcpu-full.data | 5 + .../qemumonitorjson-getcpu-full.json | 46 +++++++ .../qemumonitorjson-getcpu-host.data | 6 + .../qemumonitorjson-getcpu-host.json | 45 +++++++ tests/qemumonitorjsontest.c | 75 ++++++++++++ 10 files changed, 348 insertions(+) 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..e865808 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3926,3 +3926,34 @@ qemuMonitorSetDomainLog(qemuMonitorPtr mon, int logfd) return 0; } + + +/** + * qemuMonitorJSONGetGuestCPU: + * @mon: Pointer to the monitor + * @arch: arch of the guest + * + * Retrieve the definition of the guest CPU from a running qemu instance. + * + * Returns the cpu definition object. On error returns NULL. + */ +virCPUDataPtr +qemuMonitorGetGuestCPU(qemuMonitorPtr mon, + virArch arch) +{ + VIR_DEBUG("mon=%p, arch='%s'", mon, virArchToString(arch)); + + 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 qemuMonitorJSONGetGuestCPU(mon, arch); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 06ba7e8..ecc6d7b 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,9 @@ int qemuMonitorGetDeviceAliases(qemuMonitorPtr mon, int qemuMonitorSetDomainLog(qemuMonitorPtr mon, int logfd); +virCPUDataPtr qemuMonitorGetGuestCPU(qemuMonitorPtr mon, + virArch arch); + /** * 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..e738fe3 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,134 @@ cleanup: VIR_FREE(paths); return ret; } + + +static int +qemuMonitorJSONParseCPUx86FeatureWord(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; +} + + +static virCPUDataPtr +qemuMonitorJSONGetCPUx86Data(qemuMonitorPtr mon, + const char *property) +{ + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + virCPUx86Data *x86Data = NULL; + virCPUx86CPUID cpuid; + size_t i; + virCPUDataPtr ret = NULL; + int n; + + if (!(cmd = qemuMonitorJSONMakeCommand("qom-get", + "s:path", QOM_CPU_PATH, + "s:property", property, + NULL))) + return NULL; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply)) + 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 (qemuMonitorJSONParseCPUx86FeatureWord(virJSONValueArrayGet(data, i), + &cpuid) < 0 || + virCPUx86DataAddCPUID(x86Data, &cpuid) < 0) + goto cleanup; + } + + if (!(ret = virCPUx86MakeData(VIR_ARCH_X86_64, &x86Data))) + goto cleanup; + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + virCPUx86DataFree(x86Data); + return ret; +} + + +/** + * qemuMonitorJSONGetGuestCPU: + * @mon: Pointer to the monitor + * @arch: arch of the guest + * + * Retrieve the definition of the guest CPU from a running qemu instance. + * + * Returns the cpu definition object. On error returns NULL. + */ +virCPUDataPtr +qemuMonitorJSONGetGuestCPU(qemuMonitorPtr mon, + virArch arch) +{ + switch (arch) { + case VIR_ARCH_X86_64: + case VIR_ARCH_I686: + return qemuMonitorJSONGetCPUx86Data(mon, "feature-words"); + + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("CPU definition retrieval isn't supported for '%s'"), + virArchToString(arch)); + return NULL; + } +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index a1a7548..9e0a0c9 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,5 @@ int qemuMonitorJSONDetachCharDev(qemuMonitorPtr mon, int qemuMonitorJSONGetDeviceAliases(qemuMonitorPtr mon, char ***aliases); +virCPUDataPtr qemuMonitorJSONGetGuestCPU(qemuMonitorPtr mon, virArch arch); #endif /* QEMU_MONITOR_JSON_H */ diff --git a/tests/Makefile.am b/tests/Makefile.am index 866ecd4..85f3f9a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -86,6 +86,7 @@ EXTRA_DIST = \ qemucapabilitiesdata \ qemuhelpdata \ qemuhotplugtestdata \ + qemumonitorjsondata \ qemuxml2argvdata \ qemuxml2xmloutdata \ qemuxmlnsdata \ 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..a6bd346 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,69 @@ cleanup: return ret; } + +struct testCPUData { + const char *name; + 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; + + if (!(cpuData = qemuMonitorJSONGetGuestCPU(qemuMonitorTestGetMonitor(test), + VIR_ARCH_X86_64))) + goto cleanup; + + if (!(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 +2055,14 @@ mymain(void) if (virtTestRun(# name, testQemuMonitorJSON ## name, &simpleFunc) < 0) \ ret = -1 +#define DO_TEST_CPU_DATA(name) \ + do { \ + struct testCPUData data = { name, xmlopt }; \ + const char *label = "GetCPUData(" name ")"; \ + if (virtTestRun(label, testQemuMonitorJSONGetCPUData, &data) < 0) \ + ret = -1; \ + } while (0) + DO_TEST(GetStatus); DO_TEST(GetVersion); DO_TEST(GetMachines); @@ -2055,6 +2127,9 @@ mymain(void) DO_TEST(qemuMonitorJSONGetVirtType); DO_TEST(qemuMonitorJSONSendKey); + DO_TEST_CPU_DATA("host"); + DO_TEST_CPU_DATA("full"); + virObjectUnref(xmlopt); return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; -- 1.8.4.2
On Mon, Nov 04, 2013 at 14:55:03 +0100, Peter Krempa wrote:
From: Jiri Denemark <jdenemar@redhat.com>
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@redhat.com> ---
Notes: Version 3: - removed unneeded error state - fixed typo in error message
Version 2: - unified global and JSON monitor func signatures - changed return values so that errors can be differenitated from lack of support - code is conditionally run only when architecture matches
src/qemu/qemu_monitor.c | 31 +++++ src/qemu/qemu_monitor.h | 4 + src/qemu/qemu_monitor_json.c | 133 +++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 2 + tests/Makefile.am | 1 + .../qemumonitorjson-getcpu-full.data | 5 + .../qemumonitorjson-getcpu-full.json | 46 +++++++ .../qemumonitorjson-getcpu-host.data | 6 + .../qemumonitorjson-getcpu-host.json | 45 +++++++ tests/qemumonitorjsontest.c | 75 ++++++++++++ 10 files changed, 348 insertions(+) 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..e865808 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3926,3 +3926,34 @@ qemuMonitorSetDomainLog(qemuMonitorPtr mon, int logfd)
return 0; } + + +/** + * qemuMonitorJSONGetGuestCPU: + * @mon: Pointer to the monitor + * @arch: arch of the guest + * + * Retrieve the definition of the guest CPU from a running qemu instance. + * + * Returns the cpu definition object. On error returns NULL. + */ +virCPUDataPtr +qemuMonitorGetGuestCPU(qemuMonitorPtr mon, + virArch arch) +{ + VIR_DEBUG("mon=%p, arch='%s'", mon, virArchToString(arch)); + + 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 qemuMonitorJSONGetGuestCPU(mon, arch); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 06ba7e8..ecc6d7b 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,9 @@ int qemuMonitorGetDeviceAliases(qemuMonitorPtr mon,
int qemuMonitorSetDomainLog(qemuMonitorPtr mon, int logfd);
+virCPUDataPtr qemuMonitorGetGuestCPU(qemuMonitorPtr mon, + virArch arch); + /** * 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..e738fe3 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,134 @@ cleanup: VIR_FREE(paths); return ret; } + + +static int +qemuMonitorJSONParseCPUx86FeatureWord(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)) {
I guess this should have been "if (... < 0)", right?
+ 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; + } ...
ACK Jirka
On 11/04/2013 08:55 AM, Peter Krempa wrote:
From: Jiri Denemark <jdenemar@redhat.com>
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@redhat.com> ---
<...snip...>
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 05f8aa6..e738fe3 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,134 @@ cleanup: VIR_FREE(paths); return ret; } + + +static int +qemuMonitorJSONParseCPUx86FeatureWord(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; +} + + +static virCPUDataPtr +qemuMonitorJSONGetCPUx86Data(qemuMonitorPtr mon, + const char *property) +{ + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + virCPUx86Data *x86Data = NULL; + virCPUx86CPUID cpuid; + size_t i; + virCPUDataPtr ret = NULL; + int n; + + if (!(cmd = qemuMonitorJSONMakeCommand("qom-get", + "s:path", QOM_CPU_PATH, + "s:property", property, + NULL))) + return NULL; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply)) + 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 (qemuMonitorJSONParseCPUx86FeatureWord(virJSONValueArrayGet(data, i), + &cpuid) < 0 || + virCPUx86DataAddCPUID(x86Data, &cpuid) < 0) + goto cleanup; + } + + if (!(ret = virCPUx86MakeData(VIR_ARCH_X86_64, &x86Data))) + goto cleanup; + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + virCPUx86DataFree(x86Data); + return ret; +} + + +/** + * qemuMonitorJSONGetGuestCPU: + * @mon: Pointer to the monitor + * @arch: arch of the guest + * + * Retrieve the definition of the guest CPU from a running qemu instance. + * + * Returns the cpu definition object. On error returns NULL. + */ +virCPUDataPtr +qemuMonitorJSONGetGuestCPU(qemuMonitorPtr mon, + virArch arch) +{ + switch (arch) { + case VIR_ARCH_X86_64: + case VIR_ARCH_I686: + return qemuMonitorJSONGetCPUx86Data(mon, "feature-words");
According to: http://wiki.qemu.org/Features/CPUModels This was added in qemu 1.5, I have qemu 1.4.2 installed right now and VM startups fail: error: Failed to start domain f18 error: internal error: unable to execute QEMU command 'qom-get': Property '.feature-words' not found John
+ + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("CPU definition retrieval isn't supported for '%s'"), + virArchToString(arch)); + return NULL; + } +}
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. --- Notes: Version 3: - no changes to previous version - ACKed by DanPB Version 2: - Rebased to changes in previous patch. src/cpu/cpu_x86.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/cpu/cpu_x86_data.h | 12 ++++++++++ 2 files changed, 76 insertions(+) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index ba6a2b0..ce75562 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,48 @@ 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; +} + + static struct x86_map * virCPUx86LoadMap(void) { @@ -1079,6 +1140,9 @@ virCPUx86LoadMap(void) if (cpuMapLoad("x86", x86MapLoadCallback, map) < 0) goto error; + if (x86MapLoadInternalFeatures(map) < 0) + goto error; + return map; error: 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.4.2
On Mon, Nov 04, 2013 at 14:55:04 +0100, Peter Krempa wrote:
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. ---
Notes: Version 3: - no changes to previous version - ACKed by DanPB
Version 2: - Rebased to changes in previous patch.
src/cpu/cpu_x86.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/cpu/cpu_x86_data.h | 12 ++++++++++ 2 files changed, 76 insertions(+)
ACK Jirka
Currently we were storing domain feature flags in a bit field as the they were either enabled or disabled. New features such as paravirtual spinlocks however can be tri-state as the default option may depend on hypervisor version. To allow storing tri-state feature state in the same place instead of having to declare dedicated variables for each feature this patch refactors the bit field to an array. --- Notes: Version 3: - no changes - second review requested (weakly ACKed) Version 2: - extracted unrelated changes to separate patches - added error on features that don't support state=off - removed spurious comment 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 7849693..f6f2e64 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11417,10 +11417,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, @@ -11431,11 +11431,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; @@ -13407,12 +13419,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 */ @@ -13426,7 +13442,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: @@ -16749,58 +16765,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 255c711..a350e90 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1869,8 +1869,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; @@ -1960,7 +1960,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 63e235d..8092844 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; @@ -8063,7 +8063,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"); } @@ -10862,7 +10862,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, @@ -11116,7 +11116,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; } } @@ -11129,8 +11129,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]; \ @@ -11424,7 +11423,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")) { @@ -11543,7 +11542,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 37ca651..3807a6d 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -2337,7 +2337,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 @@ -2345,21 +2344,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 c5b8d8f..5a798e7 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1489,15 +1489,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.4.2
On Mon, Nov 04, 2013 at 14:55:05 +0100, Peter Krempa wrote:
Currently we were storing domain feature flags in a bit field as the they were either enabled or disabled. New features such as paravirtual spinlocks however can be tri-state as the default option may depend on hypervisor version.
To allow storing tri-state feature state in the same place instead of having to declare dedicated variables for each feature this patch refactors the bit field to an array. ---
Notes: Version 3: - no changes - second review requested (weakly ACKed)
Do two weak ACKs make a normal ACK? If so, weak ACK :-) Jirka
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" pseudo CPU feature in qemu. https://bugzilla.redhat.com/show_bug.cgi?id=1008989 --- Notes: Version 3: - no change - ACKed by Daniel Version 2: - fixed docs mistake - changed name of the feature to pvspinlock docs/formatdomain.html.in | 8 +++++ docs/schemas/domaincommon.rng | 10 +++++- src/conf/domain_conf.c | 36 +++++++++++++++++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 13 ++++++++ .../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 aa90701..096e2a3 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1193,6 +1193,7 @@ <vapic state='on'/> <spinlocks state='on' retries='4096'/> </hyperv> + <pvspinlock/> </features> ...</pre> @@ -1264,6 +1265,13 @@ </tr> </table> </dd> + <dt><code>pvspinlock</code></dt> + <dd>Notify the guest that the host supports paravirtual spinlocks + for example by exposing the pvticketlocks mechanism. This feature + can be explicitly disabled 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..cc3096d 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="pvspinlock"> + <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 f6f2e64..8779517 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", + "pvspinlock") VIR_ENUM_IMPL(virDomainFeatureState, VIR_DOMAIN_FEATURE_STATE_LAST, "default", @@ -11441,6 +11442,22 @@ virDomainDefParseXML(xmlDocPtr xml, def->features[val] = VIR_DOMAIN_FEATURE_STATE_ON; break; + case VIR_DOMAIN_FEATURE_PVSPINLOCK: + 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; } @@ -16808,6 +16825,23 @@ virDomainDefFormatInternal(virDomainDefPtr def, break; + case VIR_DOMAIN_FEATURE_PVSPINLOCK: + 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 state='on'/>\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..05c2a84 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_PVSPINLOCK, VIR_DOMAIN_FEATURE_LAST }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8092844..fb7145a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6718,6 +6718,19 @@ qemuBuildCpuArgStr(virQEMUDriverPtr driver, have_cpu = true; } + if (def->features[VIR_DOMAIN_FEATURE_PVSPINLOCK]) { + char sign; + if (def->features[VIR_DOMAIN_FEATURE_PVSPINLOCK] == 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..4820476 --- /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/> + <pvspinlock 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..ac8781b --- /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/> + <pvspinlock state='on'/> + </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 b14e713..07c3386 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.4.2
On Mon, Nov 04, 2013 at 14:55:06 +0100, Peter Krempa wrote:
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" pseudo CPU feature in qemu.
ACK Jirka
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. --- Notes: Version 3: - monitor code is now called only on the correct arch to avoid spurious errors in log Version 2: - adapted to previous changes src/qemu/qemu_process.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7c23eb4..95b6d1d 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,47 @@ 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; + + switch (arch) { + case VIR_ARCH_I686: + case VIR_ARCH_X86_64: + qemuDomainObjEnterMonitor(driver, vm); + guestcpu = qemuMonitorGetGuestCPU(priv->mon, arch); + qemuDomainObjExitMonitor(driver, vm); + + if (!(guestcpu)) + goto cleanup; + + if (def->features[VIR_DOMAIN_FEATURE_PVSPINLOCK] == 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; + } + } + break; + + default: + break; + } + + ret = true; + +cleanup: + cpuDataFree(guestcpu); + return ret; +} + + int qemuProcessStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -3932,6 +3974,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.4.2
On Mon, Nov 04, 2013 at 14:55:07 +0100, Peter Krempa wrote:
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. ---
Notes: Version 3: - monitor code is now called only on the correct arch to avoid spurious errors in log
Version 2: - adapted to previous changes
src/qemu/qemu_process.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)
ACK Jirka
On 11/07/13 23:22, Jiri Denemark wrote:
On Mon, Nov 04, 2013 at 14:55:07 +0100, Peter Krempa wrote:
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. ---
Notes: Version 3: - monitor code is now called only on the correct arch to avoid spurious errors in log
Version 2: - adapted to previous changes
src/qemu/qemu_process.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+)
ACK
Jirka
Thanks; I've fixed 2/6 and pushed the series. Peter
participants (3)
-
Jiri Denemark -
John Ferlan -
Peter Krempa