[libvirt] [PATCH] Add flag to BaselineCPU API to return detailed CPU features
Currently the virConnectBaselineCPU API does not expose the CPU features that are part of the CPU's model. This patch adds a new flag, VIR_CONNECT_BASELINE_SHOW_MODEL, that causes the API to explictly list all features that are part of that model. Signed-off-by: Don Dugger <donald.d.dugger@intel.com> --- include/libvirt/libvirt.h.in | 9 +++++++ src/cpu/cpu.c | 12 +++++---- src/cpu/cpu.h | 12 ++++++--- src/cpu/cpu_arm.c | 3 ++- src/cpu/cpu_generic.c | 3 ++- src/cpu/cpu_powerpc.c | 6 +++-- src/cpu/cpu_s390.c | 3 ++- src/cpu/cpu_x86.c | 39 ++++++++++++++++++++++++--- src/qemu/qemu_driver.c | 4 +-- tests/cputest.c | 30 +++++++++++---------- tests/cputestdata/x86-baseline-3-result.xml | 35 ++++++++++++++++++++++++ tests/cputestdata/x86-baseline-3.xml | 7 +++++ tools/virsh-domain.c | 11 +++++++- 13 files changed, 140 insertions(+), 34 deletions(-) create mode 100644 tests/cputestdata/x86-baseline-3-result.xml create mode 100644 tests/cputestdata/x86-baseline-3.xml diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 1804c93..d0afc65 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3890,6 +3890,15 @@ int virConnectCompareCPU(virConnectPtr conn, /** + * virConnectBaselineCPUFlags + * + * Flags when getting XML description of a computed CPU + */ +typedef enum { + VIR_CONNECT_BASELINE_SHOW_MODEL = (1 << 0), /* show model features*/ +} virConnectBaselineCPUFlags; + +/** * virConnectBaselineCPU: * * @conn: virConnect connection diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 68125a5..c96f669 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -167,7 +167,7 @@ cpuDecode(virCPUDefPtr cpu, return -1; } - return driver->decode(cpu, data, models, nmodels, preferred); + return driver->decode(cpu, data, models, nmodels, preferred, 0); } @@ -277,7 +277,8 @@ char * cpuBaselineXML(const char **xmlCPUs, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { xmlDocPtr doc = NULL; xmlXPathContextPtr ctxt = NULL; @@ -324,7 +325,7 @@ cpuBaselineXML(const char **xmlCPUs, doc = NULL; } - if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels))) + if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels, flags))) goto error; cpustr = virCPUDefFormat(cpu, 0); @@ -353,7 +354,8 @@ virCPUDefPtr cpuBaseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { struct cpuArchDriver *driver; unsigned int i; @@ -395,7 +397,7 @@ cpuBaseline(virCPUDefPtr *cpus, return NULL; } - return driver->baseline(cpus, ncpus, models, nmodels); + return driver->baseline(cpus, ncpus, models, nmodels, flags); } diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index cba7149..9148871 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -49,7 +49,8 @@ typedef int const union cpuData *data, const char **models, unsigned int nmodels, - const char *preferred); + const char *preferred, + unsigned int flags); typedef int (*cpuArchEncode) (const virCPUDefPtr cpu, @@ -76,7 +77,8 @@ typedef virCPUDefPtr (*cpuArchBaseline) (virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels); + unsigned int nmodels, + unsigned int /* flags */); typedef int (*cpuArchUpdate) (virCPUDefPtr guest, @@ -145,13 +147,15 @@ extern char * cpuBaselineXML(const char **xmlCPUs, unsigned int ncpus, const char **models, - unsigned int nmodels); + unsigned int nmodels, + unsigned int /* flags */); extern virCPUDefPtr cpuBaseline (virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels); + unsigned int nmodels, + unsigned int /* flags */); extern int cpuUpdate (virCPUDefPtr guest, diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index cfe1a23..af1309c 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -48,7 +48,8 @@ ArmDecode(virCPUDefPtr cpu ATTRIBUTE_UNUSED, const union cpuData *data ATTRIBUTE_UNUSED, const char **models ATTRIBUTE_UNUSED, unsigned int nmodels ATTRIBUTE_UNUSED, - const char *preferred ATTRIBUTE_UNUSED) + const char *preferred ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) { return 0; } diff --git a/src/cpu/cpu_generic.c b/src/cpu/cpu_generic.c index 7e3eda2..cad1782 100644 --- a/src/cpu/cpu_generic.c +++ b/src/cpu/cpu_generic.c @@ -115,7 +115,8 @@ static virCPUDefPtr genericBaseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags ATTRIBUTE_UNUSED) { virCPUDefPtr cpu = NULL; virCPUFeatureDefPtr features = NULL; diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c index d17f9ca..cd468dd 100644 --- a/src/cpu/cpu_powerpc.c +++ b/src/cpu/cpu_powerpc.c @@ -310,7 +310,8 @@ ppcDecode(virCPUDefPtr cpu, const union cpuData *data, const char **models, unsigned int nmodels, - const char *preferred ATTRIBUTE_UNUSED) + const char *preferred ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) { int ret = -1; struct ppc_map *map; @@ -385,7 +386,8 @@ static virCPUDefPtr ppcBaseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags ATTRIBUTE_UNUSED) { struct ppc_map *map = NULL; const struct ppc_model *model; diff --git a/src/cpu/cpu_s390.c b/src/cpu/cpu_s390.c index 998197c..32ab2d9 100644 --- a/src/cpu/cpu_s390.c +++ b/src/cpu/cpu_s390.c @@ -50,7 +50,8 @@ s390Decode(virCPUDefPtr cpu ATTRIBUTE_UNUSED, const union cpuData *data ATTRIBUTE_UNUSED, const char **models ATTRIBUTE_UNUSED, unsigned int nmodels ATTRIBUTE_UNUSED, - const char *preferred ATTRIBUTE_UNUSED) + const char *preferred ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) { return 0; } diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 5d479c2..6cb8263 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1296,13 +1296,43 @@ x86GuestData(virCPUDefPtr host, return x86Compute(host, guest, data, message); } +static void +x86AddFeatures(virCPUDefPtr cpu, + struct x86_map *map) +{ + const struct x86_model *candidate; + const struct x86_feature *feature = map->features; + + candidate = map->models; + while (candidate != NULL) { + if (STREQ(cpu->model, candidate->name)) + break; + candidate = candidate->next; + } + if (!candidate) { + VIR_WARN("Odd, %s not a known CPU model\n", cpu->model); + return; + } + while (feature != NULL) { + if (x86DataIsSubset(candidate->data, feature->data)) { + if (virCPUDefAddFeature(cpu, feature->name, VIR_CPU_FEATURE_REQUIRE) < 0) { + VIR_WARN("CPU model %s, no room for feature %s", cpu->model, feature->name); + return; + } + } + feature = feature->next; + } + return; +} + static int x86Decode(virCPUDefPtr cpu, const union cpuData *data, const char **models, unsigned int nmodels, - const char *preferred) + const char *preferred, + unsigned int flags) { int ret = -1; struct x86_map *map; @@ -1383,6 +1413,8 @@ x86Decode(virCPUDefPtr cpu, goto out; } + if (flags & VIR_CONNECT_BASELINE_SHOW_MODEL) + x86AddFeatures(cpuModel, map); cpu->model = cpuModel->model; cpu->vendor = cpuModel->vendor; cpu->nfeatures = cpuModel->nfeatures; @@ -1610,7 +1642,8 @@ static virCPUDefPtr x86Baseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { struct x86_map *map = NULL; struct x86_model *base_model = NULL; @@ -1691,7 +1724,7 @@ x86Baseline(virCPUDefPtr *cpus, if (vendor && x86DataAddCpuid(base_model->data, &vendor->cpuid) < 0) goto no_memory; - if (x86Decode(cpu, base_model->data, models, nmodels, NULL) < 0) + if (x86Decode(cpu, base_model->data, models, nmodels, NULL, flags) < 0) goto error; if (!outputVendor) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4a76f14..b87c73a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10260,9 +10260,9 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, { char *cpu; - virCheckFlags(0, NULL); + virCheckFlags(VIR_CONNECT_BASELINE_SHOW_MODEL, NULL); - cpu = cpuBaselineXML(xmlCPUs, ncpus, NULL, 0); + cpu = cpuBaselineXML(xmlCPUs, ncpus, NULL, 0, flags); return cpu; } diff --git a/tests/cputest.c b/tests/cputest.c index 0105440..227faa0 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -75,6 +75,7 @@ struct data { const char *modelsName; unsigned int nmodels; const char *preferred; + int flags; int result; }; @@ -331,7 +332,7 @@ cpuTestBaseline(const void *arg) if (!(cpus = cpuTestLoadMultiXML(data->arch, data->name, &ncpus))) goto cleanup; - baseline = cpuBaseline(cpus, ncpus, NULL, 0); + baseline = cpuBaseline(cpus, ncpus, NULL, 0, data->flags); if (data->result < 0) { virResetLastError(); if (!baseline) @@ -512,12 +513,12 @@ mymain(void) } #define DO_TEST(arch, api, name, host, cpu, \ - models, nmodels, preferred, result) \ + models, nmodels, preferred, flags, result) \ do { \ static struct data data = { \ arch, api, host, cpu, models, \ models == NULL ? NULL : #models, \ - nmodels, preferred, result \ + nmodels, preferred, flags, result \ }; \ if (cpuTestRun(name, &data) < 0) \ ret = -1; \ @@ -526,31 +527,31 @@ mymain(void) #define DO_TEST_COMPARE(arch, host, cpu, result) \ DO_TEST(arch, API_COMPARE, \ host "/" cpu " (" #result ")", \ - host, cpu, NULL, 0, NULL, result) + host, cpu, NULL, 0, NULL, 0, result) #define DO_TEST_UPDATE(arch, host, cpu, result) \ do { \ DO_TEST(arch, API_UPDATE, \ cpu " on " host, \ - host, cpu, NULL, 0, NULL, 0); \ + host, cpu, NULL, 0, NULL, 0, 0); \ DO_TEST_COMPARE(arch, host, host "+" cpu, result); \ } while (0) -#define DO_TEST_BASELINE(arch, name, result) \ +#define DO_TEST_BASELINE(arch, name, flags, result) \ DO_TEST(arch, API_BASELINE, name, NULL, "baseline-" name, \ - NULL, 0, NULL, result) + NULL, 0, NULL, flags, result) #define DO_TEST_HASFEATURE(arch, host, feature, result) \ DO_TEST(arch, API_HAS_FEATURE, \ host "/" feature " (" #result ")", \ - host, feature, NULL, 0, NULL, result) + host, feature, NULL, 0, NULL, 0, result) #define DO_TEST_GUESTDATA(arch, host, cpu, models, preferred, result) \ DO_TEST(arch, API_GUEST_DATA, \ host "/" cpu " (" #models ", pref=" #preferred ")", \ host, cpu, models, \ models == NULL ? 0 : sizeof(models) / sizeof(char *), \ - preferred, result) + preferred, 0, result) /* host to host comparison */ DO_TEST_COMPARE("x86", "host", "host", VIR_CPU_COMPARE_IDENTICAL); @@ -594,11 +595,12 @@ mymain(void) DO_TEST_UPDATE("x86", "host", "host-passthrough", VIR_CPU_COMPARE_IDENTICAL); /* computing baseline CPUs */ - DO_TEST_BASELINE("x86", "incompatible-vendors", -1); - DO_TEST_BASELINE("x86", "no-vendor", 0); - DO_TEST_BASELINE("x86", "some-vendors", 0); - DO_TEST_BASELINE("x86", "1", 0); - DO_TEST_BASELINE("x86", "2", 0); + DO_TEST_BASELINE("x86", "incompatible-vendors", 0, -1); + DO_TEST_BASELINE("x86", "no-vendor", 0, 0); + DO_TEST_BASELINE("x86", "some-vendors", 0, 0); + DO_TEST_BASELINE("x86", "1", 0, 0); + DO_TEST_BASELINE("x86", "2", 0, 0); + DO_TEST_BASELINE("x86", "3", VIR_CONNECT_BASELINE_SHOW_MODEL, 0); /* CPU features */ DO_TEST_HASFEATURE("x86", "host", "vmx", YES); diff --git a/tests/cputestdata/x86-baseline-3-result.xml b/tests/cputestdata/x86-baseline-3-result.xml new file mode 100644 index 0000000..d196112 --- /dev/null +++ b/tests/cputestdata/x86-baseline-3-result.xml @@ -0,0 +1,35 @@ +<cpu mode='custom' match='exact'> + <model fallback='allow'>Westmere</model> + <feature policy='require' name='lahf_lm'/> + <feature policy='require' name='lm'/> + <feature policy='require' name='nx'/> + <feature policy='require' name='syscall'/> + <feature policy='require' name='aes'/> + <feature policy='require' name='popcnt'/> + <feature policy='require' name='sse4.2'/> + <feature policy='require' name='sse4.1'/> + <feature policy='require' name='cx16'/> + <feature policy='require' name='ssse3'/> + <feature policy='require' name='pni'/> + <feature policy='require' name='sse2'/> + <feature policy='require' name='sse'/> + <feature policy='require' name='fxsr'/> + <feature policy='require' name='mmx'/> + <feature policy='require' name='clflush'/> + <feature policy='require' name='pse36'/> + <feature policy='require' name='pat'/> + <feature policy='require' name='cmov'/> + <feature policy='require' name='mca'/> + <feature policy='require' name='pge'/> + <feature policy='require' name='mtrr'/> + <feature policy='require' name='sep'/> + <feature policy='require' name='apic'/> + <feature policy='require' name='cx8'/> + <feature policy='require' name='mce'/> + <feature policy='require' name='pae'/> + <feature policy='require' name='msr'/> + <feature policy='require' name='tsc'/> + <feature policy='require' name='pse'/> + <feature policy='require' name='de'/> + <feature policy='require' name='fpu'/> +</cpu> diff --git a/tests/cputestdata/x86-baseline-3.xml b/tests/cputestdata/x86-baseline-3.xml new file mode 100644 index 0000000..7654a1d --- /dev/null +++ b/tests/cputestdata/x86-baseline-3.xml @@ -0,0 +1,7 @@ +<cpuTest> +<cpu> + <arch>x86_64</arch> + <model>Westmere</model> + <topology sockets='1' cores='2' threads='1'/> +</cpu> +</cpuTest> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 0402aef..0aa8b60 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5986,6 +5986,10 @@ static const vshCmdOptDef opts_cpu_baseline[] = { .flags = VSH_OFLAG_REQ, .help = N_("file containing XML CPU descriptions") }, + {.name = "model_features", + .type = VSH_OT_BOOL, + .help = N_("Show features that are part of the CPU model type") + }, {.name = NULL} }; @@ -5997,6 +6001,7 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) char *buffer; char *result = NULL; const char **list = NULL; + unsigned int flags = 0; int count = 0; xmlDocPtr xml = NULL; @@ -6006,6 +6011,9 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) virBuffer buf = VIR_BUFFER_INITIALIZER; int i; + if (vshCommandOptBool(cmd, "model_features")) + flags |= VIR_CONNECT_BASELINE_SHOW_MODEL; + if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) return false; @@ -6049,7 +6057,8 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) list[i] = vshStrdup(ctl, (const char *)xmlBufferContent(xml_buf)); } - result = virConnectBaselineCPU(ctl->conn, list, count, 0); + result = virConnectBaselineCPU(ctl->conn, list, count, flags); +vshPrint(ctl, "result - %p\n", result); if (result) { vshPrint(ctl, "%s", result); -- 1.7.10.4 -- Don Dugger "Censeo Toto nos in Kansa esse decisse." - D. Gale n0ano@n0ano.com Ph: 303/443-3786
On Mon, Jul 29, 2013 at 05:31:19PM -0600, Don Dugger wrote:
Currently the virConnectBaselineCPU API does not expose the CPU features that are part of the CPU's model. This patch adds a new flag, VIR_CONNECT_BASELINE_SHOW_MODEL, that causes the API to explictly list all features that are part of that model.
Nit-pick: I'd prefer the constant to be named VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE so that it is more precisely describing what it is doing, and includes the word "CPU" there to match the API name.
+static void +x86AddFeatures(virCPUDefPtr cpu, + struct x86_map *map) +{ + const struct x86_model *candidate; + const struct x86_feature *feature = map->features; + + candidate = map->models; + while (candidate != NULL) { + if (STREQ(cpu->model, candidate->name)) + break; + candidate = candidate->next; + } + if (!candidate) { + VIR_WARN("Odd, %s not a known CPU model\n", cpu->model);
VIR_WARN shouldn't be used for errors which can occur in public API call paths. It should virReportError() and return an error code '-1' to the caller of the API
+ return; + } + while (feature != NULL) { + if (x86DataIsSubset(candidate->data, feature->data)) { + if (virCPUDefAddFeature(cpu, feature->name, VIR_CPU_FEATURE_REQUIRE) < 0) { + VIR_WARN("CPU model %s, no room for feature %s", cpu->model, feature->name); + return;
Again this should be reporting an error & propagating it back to the caller Regards, 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 :|
[V2 - per review comments change the flag name to be more descriptive and change errors from warnings to propogated errors.] Currently the virConnectBaselineCPU API does not expose the CPU features that are part of the CPU's model. This patch adds a new flag, VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE, that causes the API to explictly list all features that are part of that model. Signed-off-by: Don Dugger <donald.d.dugger@intel.com> --- include/libvirt/libvirt.h.in | 9 ++++++ src/cpu/cpu.c | 12 ++++---- src/cpu/cpu.h | 12 +++++--- src/cpu/cpu_arm.c | 3 +- src/cpu/cpu_generic.c | 3 +- src/cpu/cpu_powerpc.c | 6 ++-- src/cpu/cpu_s390.c | 3 +- src/cpu/cpu_x86.c | 43 +++++++++++++++++++++++++-- src/qemu/qemu_driver.c | 4 +-- tests/cputest.c | 30 ++++++++++--------- tests/cputestdata/x86-baseline-3-result.xml | 35 ++++++++++++++++++++++ tests/cputestdata/x86-baseline-3.xml | 7 +++++ tools/virsh-domain.c | 11 ++++++- 13 files changed, 144 insertions(+), 34 deletions(-) create mode 100644 tests/cputestdata/x86-baseline-3-result.xml create mode 100644 tests/cputestdata/x86-baseline-3.xml diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 1804c93..b377707 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3890,6 +3890,15 @@ int virConnectCompareCPU(virConnectPtr conn, /** + * virConnectBaselineCPUFlags + * + * Flags when getting XML description of a computed CPU + */ +typedef enum { + VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE = (1 << 0), /* show model features*/ +} virConnectBaselineCPUFlags; + +/** * virConnectBaselineCPU: * * @conn: virConnect connection diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 68125a5..c96f669 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -167,7 +167,7 @@ cpuDecode(virCPUDefPtr cpu, return -1; } - return driver->decode(cpu, data, models, nmodels, preferred); + return driver->decode(cpu, data, models, nmodels, preferred, 0); } @@ -277,7 +277,8 @@ char * cpuBaselineXML(const char **xmlCPUs, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { xmlDocPtr doc = NULL; xmlXPathContextPtr ctxt = NULL; @@ -324,7 +325,7 @@ cpuBaselineXML(const char **xmlCPUs, doc = NULL; } - if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels))) + if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels, flags))) goto error; cpustr = virCPUDefFormat(cpu, 0); @@ -353,7 +354,8 @@ virCPUDefPtr cpuBaseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { struct cpuArchDriver *driver; unsigned int i; @@ -395,7 +397,7 @@ cpuBaseline(virCPUDefPtr *cpus, return NULL; } - return driver->baseline(cpus, ncpus, models, nmodels); + return driver->baseline(cpus, ncpus, models, nmodels, flags); } diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index cba7149..9148871 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -49,7 +49,8 @@ typedef int const union cpuData *data, const char **models, unsigned int nmodels, - const char *preferred); + const char *preferred, + unsigned int flags); typedef int (*cpuArchEncode) (const virCPUDefPtr cpu, @@ -76,7 +77,8 @@ typedef virCPUDefPtr (*cpuArchBaseline) (virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels); + unsigned int nmodels, + unsigned int /* flags */); typedef int (*cpuArchUpdate) (virCPUDefPtr guest, @@ -145,13 +147,15 @@ extern char * cpuBaselineXML(const char **xmlCPUs, unsigned int ncpus, const char **models, - unsigned int nmodels); + unsigned int nmodels, + unsigned int /* flags */); extern virCPUDefPtr cpuBaseline (virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels); + unsigned int nmodels, + unsigned int /* flags */); extern int cpuUpdate (virCPUDefPtr guest, diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index cfe1a23..af1309c 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -48,7 +48,8 @@ ArmDecode(virCPUDefPtr cpu ATTRIBUTE_UNUSED, const union cpuData *data ATTRIBUTE_UNUSED, const char **models ATTRIBUTE_UNUSED, unsigned int nmodels ATTRIBUTE_UNUSED, - const char *preferred ATTRIBUTE_UNUSED) + const char *preferred ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) { return 0; } diff --git a/src/cpu/cpu_generic.c b/src/cpu/cpu_generic.c index 7e3eda2..cad1782 100644 --- a/src/cpu/cpu_generic.c +++ b/src/cpu/cpu_generic.c @@ -115,7 +115,8 @@ static virCPUDefPtr genericBaseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags ATTRIBUTE_UNUSED) { virCPUDefPtr cpu = NULL; virCPUFeatureDefPtr features = NULL; diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c index d17f9ca..cd468dd 100644 --- a/src/cpu/cpu_powerpc.c +++ b/src/cpu/cpu_powerpc.c @@ -310,7 +310,8 @@ ppcDecode(virCPUDefPtr cpu, const union cpuData *data, const char **models, unsigned int nmodels, - const char *preferred ATTRIBUTE_UNUSED) + const char *preferred ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) { int ret = -1; struct ppc_map *map; @@ -385,7 +386,8 @@ static virCPUDefPtr ppcBaseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags ATTRIBUTE_UNUSED) { struct ppc_map *map = NULL; const struct ppc_model *model; diff --git a/src/cpu/cpu_s390.c b/src/cpu/cpu_s390.c index 998197c..32ab2d9 100644 --- a/src/cpu/cpu_s390.c +++ b/src/cpu/cpu_s390.c @@ -50,7 +50,8 @@ s390Decode(virCPUDefPtr cpu ATTRIBUTE_UNUSED, const union cpuData *data ATTRIBUTE_UNUSED, const char **models ATTRIBUTE_UNUSED, unsigned int nmodels ATTRIBUTE_UNUSED, - const char *preferred ATTRIBUTE_UNUSED) + const char *preferred ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) { return 0; } diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 5d479c2..f0fa2d1 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1296,13 +1296,46 @@ x86GuestData(virCPUDefPtr host, return x86Compute(host, guest, data, message); } +static int +x86AddFeatures(virCPUDefPtr cpu, + struct x86_map *map) +{ + const struct x86_model *candidate; + const struct x86_feature *feature = map->features; + + candidate = map->models; + while (candidate != NULL) { + if (STREQ(cpu->model, candidate->name)) + break; + candidate = candidate->next; + } + if (!candidate) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s not a known CPU model\n", cpu->model); + return -1; + } + while (feature != NULL) { + if (x86DataIsSubset(candidate->data, feature->data)) { + if (virCPUDefAddFeature(cpu, feature->name, VIR_CPU_FEATURE_REQUIRE) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "CPU model %s, no room for feature %s", + cpu->model, feature->name); + return -1; + } + } + feature = feature->next; + } + return 0; +} + static int x86Decode(virCPUDefPtr cpu, const union cpuData *data, const char **models, unsigned int nmodels, - const char *preferred) + const char *preferred, + unsigned int flags) { int ret = -1; struct x86_map *map; @@ -1383,6 +1416,9 @@ x86Decode(virCPUDefPtr cpu, goto out; } + if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE) + if (x86AddFeatures(cpuModel, map) < 0) + goto out; cpu->model = cpuModel->model; cpu->vendor = cpuModel->vendor; cpu->nfeatures = cpuModel->nfeatures; @@ -1610,7 +1646,8 @@ static virCPUDefPtr x86Baseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { struct x86_map *map = NULL; struct x86_model *base_model = NULL; @@ -1691,7 +1728,7 @@ x86Baseline(virCPUDefPtr *cpus, if (vendor && x86DataAddCpuid(base_model->data, &vendor->cpuid) < 0) goto no_memory; - if (x86Decode(cpu, base_model->data, models, nmodels, NULL) < 0) + if (x86Decode(cpu, base_model->data, models, nmodels, NULL, flags) < 0) goto error; if (!outputVendor) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4a76f14..f5d565b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10260,9 +10260,9 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, { char *cpu; - virCheckFlags(0, NULL); + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE, NULL); - cpu = cpuBaselineXML(xmlCPUs, ncpus, NULL, 0); + cpu = cpuBaselineXML(xmlCPUs, ncpus, NULL, 0, flags); return cpu; } diff --git a/tests/cputest.c b/tests/cputest.c index 0105440..618469e 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -75,6 +75,7 @@ struct data { const char *modelsName; unsigned int nmodels; const char *preferred; + int flags; int result; }; @@ -331,7 +332,7 @@ cpuTestBaseline(const void *arg) if (!(cpus = cpuTestLoadMultiXML(data->arch, data->name, &ncpus))) goto cleanup; - baseline = cpuBaseline(cpus, ncpus, NULL, 0); + baseline = cpuBaseline(cpus, ncpus, NULL, 0, data->flags); if (data->result < 0) { virResetLastError(); if (!baseline) @@ -512,12 +513,12 @@ mymain(void) } #define DO_TEST(arch, api, name, host, cpu, \ - models, nmodels, preferred, result) \ + models, nmodels, preferred, flags, result) \ do { \ static struct data data = { \ arch, api, host, cpu, models, \ models == NULL ? NULL : #models, \ - nmodels, preferred, result \ + nmodels, preferred, flags, result \ }; \ if (cpuTestRun(name, &data) < 0) \ ret = -1; \ @@ -526,31 +527,31 @@ mymain(void) #define DO_TEST_COMPARE(arch, host, cpu, result) \ DO_TEST(arch, API_COMPARE, \ host "/" cpu " (" #result ")", \ - host, cpu, NULL, 0, NULL, result) + host, cpu, NULL, 0, NULL, 0, result) #define DO_TEST_UPDATE(arch, host, cpu, result) \ do { \ DO_TEST(arch, API_UPDATE, \ cpu " on " host, \ - host, cpu, NULL, 0, NULL, 0); \ + host, cpu, NULL, 0, NULL, 0, 0); \ DO_TEST_COMPARE(arch, host, host "+" cpu, result); \ } while (0) -#define DO_TEST_BASELINE(arch, name, result) \ +#define DO_TEST_BASELINE(arch, name, flags, result) \ DO_TEST(arch, API_BASELINE, name, NULL, "baseline-" name, \ - NULL, 0, NULL, result) + NULL, 0, NULL, flags, result) #define DO_TEST_HASFEATURE(arch, host, feature, result) \ DO_TEST(arch, API_HAS_FEATURE, \ host "/" feature " (" #result ")", \ - host, feature, NULL, 0, NULL, result) + host, feature, NULL, 0, NULL, 0, result) #define DO_TEST_GUESTDATA(arch, host, cpu, models, preferred, result) \ DO_TEST(arch, API_GUEST_DATA, \ host "/" cpu " (" #models ", pref=" #preferred ")", \ host, cpu, models, \ models == NULL ? 0 : sizeof(models) / sizeof(char *), \ - preferred, result) + preferred, 0, result) /* host to host comparison */ DO_TEST_COMPARE("x86", "host", "host", VIR_CPU_COMPARE_IDENTICAL); @@ -594,11 +595,12 @@ mymain(void) DO_TEST_UPDATE("x86", "host", "host-passthrough", VIR_CPU_COMPARE_IDENTICAL); /* computing baseline CPUs */ - DO_TEST_BASELINE("x86", "incompatible-vendors", -1); - DO_TEST_BASELINE("x86", "no-vendor", 0); - DO_TEST_BASELINE("x86", "some-vendors", 0); - DO_TEST_BASELINE("x86", "1", 0); - DO_TEST_BASELINE("x86", "2", 0); + DO_TEST_BASELINE("x86", "incompatible-vendors", 0, -1); + DO_TEST_BASELINE("x86", "no-vendor", 0, 0); + DO_TEST_BASELINE("x86", "some-vendors", 0, 0); + DO_TEST_BASELINE("x86", "1", 0, 0); + DO_TEST_BASELINE("x86", "2", 0, 0); + DO_TEST_BASELINE("x86", "3", VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE, 0); /* CPU features */ DO_TEST_HASFEATURE("x86", "host", "vmx", YES); diff --git a/tests/cputestdata/x86-baseline-3-result.xml b/tests/cputestdata/x86-baseline-3-result.xml new file mode 100644 index 0000000..d196112 --- /dev/null +++ b/tests/cputestdata/x86-baseline-3-result.xml @@ -0,0 +1,35 @@ +<cpu mode='custom' match='exact'> + <model fallback='allow'>Westmere</model> + <feature policy='require' name='lahf_lm'/> + <feature policy='require' name='lm'/> + <feature policy='require' name='nx'/> + <feature policy='require' name='syscall'/> + <feature policy='require' name='aes'/> + <feature policy='require' name='popcnt'/> + <feature policy='require' name='sse4.2'/> + <feature policy='require' name='sse4.1'/> + <feature policy='require' name='cx16'/> + <feature policy='require' name='ssse3'/> + <feature policy='require' name='pni'/> + <feature policy='require' name='sse2'/> + <feature policy='require' name='sse'/> + <feature policy='require' name='fxsr'/> + <feature policy='require' name='mmx'/> + <feature policy='require' name='clflush'/> + <feature policy='require' name='pse36'/> + <feature policy='require' name='pat'/> + <feature policy='require' name='cmov'/> + <feature policy='require' name='mca'/> + <feature policy='require' name='pge'/> + <feature policy='require' name='mtrr'/> + <feature policy='require' name='sep'/> + <feature policy='require' name='apic'/> + <feature policy='require' name='cx8'/> + <feature policy='require' name='mce'/> + <feature policy='require' name='pae'/> + <feature policy='require' name='msr'/> + <feature policy='require' name='tsc'/> + <feature policy='require' name='pse'/> + <feature policy='require' name='de'/> + <feature policy='require' name='fpu'/> +</cpu> diff --git a/tests/cputestdata/x86-baseline-3.xml b/tests/cputestdata/x86-baseline-3.xml new file mode 100644 index 0000000..7654a1d --- /dev/null +++ b/tests/cputestdata/x86-baseline-3.xml @@ -0,0 +1,7 @@ +<cpuTest> +<cpu> + <arch>x86_64</arch> + <model>Westmere</model> + <topology sockets='1' cores='2' threads='1'/> +</cpu> +</cpuTest> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 0402aef..8912a64 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5986,6 +5986,10 @@ static const vshCmdOptDef opts_cpu_baseline[] = { .flags = VSH_OFLAG_REQ, .help = N_("file containing XML CPU descriptions") }, + {.name = "model_features", + .type = VSH_OT_BOOL, + .help = N_("Show features that are part of the CPU model type") + }, {.name = NULL} }; @@ -5997,6 +6001,7 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) char *buffer; char *result = NULL; const char **list = NULL; + unsigned int flags = 0; int count = 0; xmlDocPtr xml = NULL; @@ -6006,6 +6011,9 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) virBuffer buf = VIR_BUFFER_INITIALIZER; int i; + if (vshCommandOptBool(cmd, "model_features")) + flags |= VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE; + if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) return false; @@ -6049,7 +6057,8 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) list[i] = vshStrdup(ctl, (const char *)xmlBufferContent(xml_buf)); } - result = virConnectBaselineCPU(ctl->conn, list, count, 0); + result = virConnectBaselineCPU(ctl->conn, list, count, flags); +vshPrint(ctl, "result - %p\n", result); if (result) { vshPrint(ctl, "%s", result); -- 1.7.10.4 -- Don Dugger "Censeo Toto nos in Kansa esse decisse." - D. Gale n0ano@n0ano.com Ph: 303/443-3786
On 07/30/2013 03:09 PM, Don Dugger wrote:
[V2 - per review comments change the flag name to be more descriptive and change errors from warnings to propogated errors.]
This aside belongs...
Currently the virConnectBaselineCPU API does not expose the CPU features that are part of the CPU's model. This patch adds a new flag, VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE, that causes the API to explictly list all features that are part of that model.
Signed-off-by: Don Dugger <donald.d.dugger@intel.com> ---
...here, after the '---', so that 'git am' won't turn it into a permanent part of libvirt.git history. Your patch didn't apply as is (are you properly rebasing to the latest libvirt.git?), but the conflict resolution in qemu_driver.c seemed pretty trivial. You also failed 'make syntax-check': flags_usage src/cpu/cpu_arm.c:48: unsigned int flags ATTRIBUTE_UNUSED) src/cpu/cpu_generic.c:117: unsigned int flags ATTRIBUTE_UNUSED) src/cpu/cpu_powerpc.c:308: unsigned int flags ATTRIBUTE_UNUSED) src/cpu/cpu_powerpc.c:382: unsigned int flags ATTRIBUTE_UNUSED) src/cpu/cpu_s390.c:52: unsigned int flags ATTRIBUTE_UNUSED) maint.mk: flags should be checked with virCheckFlags When adding a flags parameter, we intentionally update callers to explicitly check for a 0 argument if they are unprepared to handle non-zero flags; this leaves the door open for extensions that use new flag bits, rather than being stuck with undefined behavior when passing a new bit to an old binary. So, all of these sites need to use either virCheckFlags(0, NULL) to state they don't handle the flag, or virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE, NULL) to state that the flag is valid (and does not impact the output for that architecture).
include/libvirt/libvirt.h.in | 9 ++++++
Needs a corresponding doc patch in src/libvirt.c that mentions use of the new flag.
tools/virsh-domain.c | 11 ++++++-
Likewise, this needs a corresponding patch to tools/virsh.pod to update the man page to mention the new virsh option.
+++ b/include/libvirt/libvirt.h.in @@ -3890,6 +3890,15 @@ int virConnectCompareCPU(virConnectPtr conn,
/** + * virConnectBaselineCPUFlags + * + * Flags when getting XML description of a computed CPU + */ +typedef enum { + VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE = (1 << 0), /* show model features*/
Space before */ I know this is the name Dan suggested, but I can't help but wonder if _EXPAND_FEATURES sounds nicer than EXPAND_FEATURE, since pretty much every cpu name has more than one feature listed when expanded.
+++ b/src/cpu/cpu.c @@ -167,7 +167,7 @@ cpuDecode(virCPUDefPtr cpu, return -1; }
- return driver->decode(cpu, data, models, nmodels, preferred); + return driver->decode(cpu, data, models, nmodels, preferred, 0); }
@@ -277,7 +277,8 @@ char * cpuBaselineXML(const char **xmlCPUs, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) {
Needs to make sure we reject invalid flags: virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE, -1)
+++ b/src/cpu/cpu.h @@ -49,7 +49,8 @@ typedef int const union cpuData *data, const char **models, unsigned int nmodels, - const char *preferred); + const char *preferred, + unsigned int flags);
typedef int (*cpuArchEncode) (const virCPUDefPtr cpu, @@ -76,7 +77,8 @@ typedef virCPUDefPtr (*cpuArchBaseline) (virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels); + unsigned int nmodels, + unsigned int /* flags */);
No need to comment out the parameter name (multiple instances */.
+++ b/src/cpu/cpu_x86.c @@ -1296,13 +1296,46 @@ x86GuestData(virCPUDefPtr host, return x86Compute(host, guest, data, message); }
+static int +x86AddFeatures(virCPUDefPtr cpu, + struct x86_map *map) +{ + const struct x86_model *candidate; + const struct x86_feature *feature = map->features; + + candidate = map->models; + while (candidate != NULL) { + if (STREQ(cpu->model, candidate->name)) + break; + candidate = candidate->next; + } + if (!candidate) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s not a known CPU model\n", cpu->model); + return -1; + } + while (feature != NULL) { + if (x86DataIsSubset(candidate->data, feature->data)) { + if (virCPUDefAddFeature(cpu, feature->name, VIR_CPU_FEATURE_REQUIRE) < 0) {
This function already outputs a decent error message on failure...
+ virReportError(VIR_ERR_INTERNAL_ERROR, + "CPU model %s, no room for feature %s", + cpu->model, feature->name);
...but this message overwrites that error. Drop this line.
@@ -1383,6 +1416,9 @@ x86Decode(virCPUDefPtr cpu, goto out; }
+ if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE) + if (x86AddFeatures(cpuModel, map) < 0) + goto out;
You could write this with fewer nested if: if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE) && x86AddFeatures(cpuModel, map) < 0) goto out;
+++ b/tests/cputest.c
#define DO_TEST(arch, api, name, host, cpu, \ - models, nmodels, preferred, result) \ + models, nmodels, preferred, flags, result) \ do { \ static struct data data = { \ arch, api, host, cpu, models, \ models == NULL ? NULL : #models, \ - nmodels, preferred, result \ + nmodels, preferred, flags, result \
Might as well fix the alignment of the \ while touching this.
+++ b/tools/virsh-domain.c @@ -5986,6 +5986,10 @@ static const vshCmdOptDef opts_cpu_baseline[] = { .flags = VSH_OFLAG_REQ, .help = N_("file containing XML CPU descriptions") }, + {.name = "model_features",
virsh flags should favor - over _. But rather than name it --model-features, why not just go with the shorter --features? Furthermore, the fact that you named it "features" rather than "feature" adds weight to my argument that the flag name in libvirt.h.in needs to use _FEATURES.
@@ -6049,7 +6057,8 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) list[i] = vshStrdup(ctl, (const char *)xmlBufferContent(xml_buf)); }
- result = virConnectBaselineCPU(ctl->conn, list, count, 0); + result = virConnectBaselineCPU(ctl->conn, list, count, flags); +vshPrint(ctl, "result - %p\n", result);
Leftover debugging?
if (result) { vshPrint(ctl, "%s", result);
Getting closer; looking forward to v3. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 07/30/2013 03:59 PM, Eric Blake wrote:
On 07/30/2013 03:09 PM, Don Dugger wrote:
[V2 - per review comments change the flag name to be more descriptive and change errors from warnings to propogated errors.]
This aside belongs...
Currently the virConnectBaselineCPU API does not expose the CPU features that are part of the CPU's model. This patch adds a new flag, VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE, that causes the API to explictly list all features that are part of that model.
Signed-off-by: Don Dugger <donald.d.dugger@intel.com> ---
...here, after the '---', so that 'git am' won't turn it into a permanent part of libvirt.git history. Your patch didn't apply as is (are you properly rebasing to the latest libvirt.git?), but the conflict resolution in qemu_driver.c seemed pretty trivial. You also failed 'make syntax-check':
You also failed regular 'make': CC libvirt_cpu_la-cpu_x86.lo cpu/cpu_x86.c: In function 'x86DecodeCPUData': cpu/cpu_x86.c:1467:5: error: too few arguments to function 'x86Decode' return x86Decode(cpu, data->data.x86, models, nmodels, preferred); ^ cpu/cpu_x86.c:1356:1: note: declared here x86Decode(virCPUDefPtr cpu, ^ cpu/cpu_x86.c: At top level: cpu/cpu_x86.c:1948:5: error: initialization from incompatible pointer type [-Werror] .decode = x86DecodeCPUData, ^ cpu/cpu_x86.c:1948:5: error: (near initialization for 'cpuDriverX86.decode') [-Werror] cpu/cpu_x86.c: In function 'x86DecodeCPUData': cpu/cpu_x86.c:1468:1: error: control reaches end of non-void function [-Werror=return-type] } ^ -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Sigh. Rebase, I've heard of that, maybe I should remember to do it :-( The failures are related to the rebasing, I'm working on that and I'll incorporate your other fixes (with a couple of comments in line). Only open issue is the flag name, I have no problems with _EXPAND_FEATURES, unless there's an objection I'll do that. On Tue, Jul 30, 2013 at 03:59:38PM -0600, Eric Blake wrote: ...
+++ b/src/cpu/cpu.h @@ -49,7 +49,8 @@ typedef int const union cpuData *data, const char **models, unsigned int nmodels, - const char *preferred); + const char *preferred, + unsigned int flags);
typedef int (*cpuArchEncode) (const virCPUDefPtr cpu, @@ -76,7 +77,8 @@ typedef virCPUDefPtr (*cpuArchBaseline) (virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels); + unsigned int nmodels, + unsigned int /* flags */);
No need to comment out the parameter name (multiple instances */.
This was to avoid compile failures with unused parameters but this becomes moot when I put in the code to explicitly check that flag. ...
@@ -1383,6 +1416,9 @@ x86Decode(virCPUDefPtr cpu, goto out; }
+ if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE) + if (x86AddFeatures(cpuModel, map) < 0) + goto out;
You could write this with fewer nested if:
if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE) && x86AddFeatures(cpuModel, map) < 0) goto out;
I actually prefer the nested ifs over the logical expression (to me that's more obvious) but I don't mind changing if that's your preferred coding style. ...
@@ -6049,7 +6057,8 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) list[i] = vshStrdup(ctl, (const char *)xmlBufferContent(xml_buf)); }
- result = virConnectBaselineCPU(ctl->conn, list, count, 0); + result = virConnectBaselineCPU(ctl->conn, list, count, flags); +vshPrint(ctl, "result - %p\n", result);
Leftover debugging?
Yep, shoot me, I should know better. Sorry about that. -- Don Dugger "Censeo Toto nos in Kansa esse decisse." - D. Gale n0ano@n0ano.com Ph: 303/443-3786
Currently the virConnectBaselineCPU API does not expose the CPU features that are part of the CPU's model. This patch adds a new flag, VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, that causes the API to explictly list all features that are part of that model. Signed-off-by: Don Dugger <donald.d.dugger@intel.com> --- [V2 - per review comments change the flag name to be more descriptive and change errors from warnings to propogated errors.] [V3 - Incorporate review suggestions to better handle flags and errors. Also add documentation about the API change.] include/libvirt/libvirt.h.in | 9 ++++++ src/cpu/cpu.c | 12 ++++--- src/cpu/cpu.h | 12 ++++--- src/cpu/cpu_arm.c | 6 +++- src/cpu/cpu_generic.c | 5 ++- src/cpu/cpu_powerpc.c | 10 ++++-- src/cpu/cpu_s390.c | 6 +++- src/cpu/cpu_x86.c | 45 ++++++++++++++++++++++++--- src/libvirt.c | 7 ++++- src/qemu/qemu_driver.c | 4 +-- tests/cputest.c | 30 +++++++++--------- tests/cputestdata/x86-baseline-3-result.xml | 35 +++++++++++++++++++++ tests/cputestdata/x86-baseline-3.xml | 7 +++++ tools/virsh-domain.c | 10 +++++- tools/virsh.pod | 7 +++-- 15 files changed, 166 insertions(+), 39 deletions(-) create mode 100644 tests/cputestdata/x86-baseline-3-result.xml create mode 100644 tests/cputestdata/x86-baseline-3.xml diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 7bd3559..b843355 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4008,6 +4008,15 @@ int virConnectCompareCPU(virConnectPtr conn, /** + * virConnectBaselineCPUFlags + * + * Flags when getting XML description of a computed CPU + */ +typedef enum { + VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES = (1 << 0), /* show all features */ +} virConnectBaselineCPUFlags; + +/** * virConnectBaselineCPU: * * @conn: virConnect connection diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 4124354..023ce26 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -167,7 +167,7 @@ cpuDecode(virCPUDefPtr cpu, return -1; } - return driver->decode(cpu, data, models, nmodels, preferred); + return driver->decode(cpu, data, models, nmodels, preferred, 0); } @@ -276,7 +276,8 @@ char * cpuBaselineXML(const char **xmlCPUs, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { xmlDocPtr doc = NULL; xmlXPathContextPtr ctxt = NULL; @@ -323,7 +324,7 @@ cpuBaselineXML(const char **xmlCPUs, doc = NULL; } - if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels))) + if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels, flags))) goto error; cpustr = virCPUDefFormat(cpu, 0); @@ -350,7 +351,8 @@ virCPUDefPtr cpuBaseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { struct cpuArchDriver *driver; size_t i; @@ -392,7 +394,7 @@ cpuBaseline(virCPUDefPtr *cpus, return NULL; } - return driver->baseline(cpus, ncpus, models, nmodels); + return driver->baseline(cpus, ncpus, models, nmodels, flags); } diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 4003435..7f1d4bd 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -53,7 +53,8 @@ typedef int const virCPUDataPtr data, const char **models, unsigned int nmodels, - const char *preferred); + const char *preferred, + unsigned int flags); typedef int (*cpuArchEncode) (virArch arch, @@ -81,7 +82,8 @@ typedef virCPUDefPtr (*cpuArchBaseline) (virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels); + unsigned int nmodels, + unsigned int flags); typedef int (*cpuArchUpdate) (virCPUDefPtr guest, @@ -149,13 +151,15 @@ extern char * cpuBaselineXML(const char **xmlCPUs, unsigned int ncpus, const char **models, - unsigned int nmodels); + unsigned int nmodels, + unsigned int flags); extern virCPUDefPtr cpuBaseline (virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels); + unsigned int nmodels, + unsigned int flags); extern int cpuUpdate (virCPUDefPtr guest, diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index 25e25ba..d1b2a99 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -44,8 +44,12 @@ ArmDecode(virCPUDefPtr cpu ATTRIBUTE_UNUSED, const virCPUDataPtr data ATTRIBUTE_UNUSED, const char **models ATTRIBUTE_UNUSED, unsigned int nmodels ATTRIBUTE_UNUSED, - const char *preferred ATTRIBUTE_UNUSED) + const char *preferred ATTRIBUTE_UNUSED, + unsigned int flags) { + + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); + return 0; } diff --git a/src/cpu/cpu_generic.c b/src/cpu/cpu_generic.c index 2fe792f..1264da4 100644 --- a/src/cpu/cpu_generic.c +++ b/src/cpu/cpu_generic.c @@ -113,7 +113,8 @@ static virCPUDefPtr genericBaseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { virCPUDefPtr cpu = NULL; virCPUFeatureDefPtr features = NULL; @@ -121,6 +122,8 @@ genericBaseline(virCPUDefPtr *cpus, unsigned int count; size_t i, j; + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); + if (!cpuModelIsAllowed(cpus[0]->model, models, nmodels)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("CPU model %s is not supported by hypervisor"), diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c index 55a4153..647a8a1 100644 --- a/src/cpu/cpu_powerpc.c +++ b/src/cpu/cpu_powerpc.c @@ -304,12 +304,15 @@ ppcDecode(virCPUDefPtr cpu, const virCPUDataPtr data, const char **models, unsigned int nmodels, - const char *preferred ATTRIBUTE_UNUSED) + const char *preferred ATTRIBUTE_UNUSED, + unsigned int flags) { int ret = -1; struct ppc_map *map; const struct ppc_model *model; + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); + if (data == NULL || (map = ppcLoadMap()) == NULL) return -1; @@ -377,7 +380,8 @@ static virCPUDefPtr ppcBaseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { struct ppc_map *map = NULL; const struct ppc_model *model; @@ -385,6 +389,8 @@ ppcBaseline(virCPUDefPtr *cpus, virCPUDefPtr cpu = NULL; size_t i; + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); + if (!(map = ppcLoadMap())) goto error; diff --git a/src/cpu/cpu_s390.c b/src/cpu/cpu_s390.c index cbfae42..d997e06 100644 --- a/src/cpu/cpu_s390.c +++ b/src/cpu/cpu_s390.c @@ -48,8 +48,12 @@ s390Decode(virCPUDefPtr cpu ATTRIBUTE_UNUSED, const virCPUDataPtr data ATTRIBUTE_UNUSED, const char **models ATTRIBUTE_UNUSED, unsigned int nmodels ATTRIBUTE_UNUSED, - const char *preferred ATTRIBUTE_UNUSED) + const char *preferred ATTRIBUTE_UNUSED, + unsigned int flags) { + + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); + return 0; } diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index a388f0f..f16a3cb 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1319,13 +1319,41 @@ x86GuestData(virCPUDefPtr host, return x86Compute(host, guest, data, message); } +static int +x86AddFeatures(virCPUDefPtr cpu, + struct x86_map *map) +{ + const struct x86_model *candidate; + const struct x86_feature *feature = map->features; + + candidate = map->models; + while (candidate != NULL) { + if (STREQ(cpu->model, candidate->name)) + break; + candidate = candidate->next; + } + if (!candidate) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s not a known CPU model"), cpu->model); + return -1; + } + while (feature != NULL) { + if (x86DataIsSubset(candidate->data, feature->data) && + (virCPUDefAddFeature(cpu, feature->name, VIR_CPU_FEATURE_REQUIRE) < 0)) + return -1; + feature = feature->next; + } + return 0; +} + static int x86Decode(virCPUDefPtr cpu, const struct cpuX86Data *data, const char **models, unsigned int nmodels, - const char *preferred) + const char *preferred, + unsigned int flags) { int ret = -1; struct x86_map *map; @@ -1334,6 +1362,8 @@ x86Decode(virCPUDefPtr cpu, virCPUDefPtr cpuModel = NULL; size_t i; + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); + if (data == NULL || (map = x86LoadMap()) == NULL) return -1; @@ -1406,6 +1436,9 @@ x86Decode(virCPUDefPtr cpu, goto out; } + if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES && + (x86AddFeatures(cpuModel, map) < 0)) + goto out; cpu->model = cpuModel->model; cpu->vendor = cpuModel->vendor; cpu->nfeatures = cpuModel->nfeatures; @@ -1426,9 +1459,10 @@ x86DecodeCPUData(virCPUDefPtr cpu, const virCPUDataPtr data, const char **models, unsigned int nmodels, - const char *preferred) + const char *preferred, + unsigned int flags) { - return x86Decode(cpu, data->data.x86, models, nmodels, preferred); + return x86Decode(cpu, data->data.x86, models, nmodels, preferred, flags); } @@ -1674,7 +1708,8 @@ static virCPUDefPtr x86Baseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { struct x86_map *map = NULL; struct x86_model *base_model = NULL; @@ -1755,7 +1790,7 @@ x86Baseline(virCPUDefPtr *cpus, if (vendor && x86DataAddCpuid(base_model->data, &vendor->cpuid) < 0) goto error; - if (x86Decode(cpu, base_model->data, models, nmodels, NULL) < 0) + if (x86Decode(cpu, base_model->data, models, nmodels, NULL, flags) < 0) goto error; if (!outputVendor) diff --git a/src/libvirt.c b/src/libvirt.c index 8157488..6ce6c32 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18522,11 +18522,16 @@ error: * @conn: virConnect connection * @xmlCPUs: array of XML descriptions of host CPUs * @ncpus: number of CPUs in xmlCPUs - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virConnectBaselineCPUFlags * * Computes the most feature-rich CPU which is compatible with all given * host CPUs. * + * If @flags includes VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES then libvirt + * will explicitly list all CPU features that are part of the host CPU, + * without this flag features that are part of the CPU model will not be + * listed. + * * Returns XML description of the computed CPU or NULL on error. */ char * diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2daafa8..2ad236e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11056,12 +11056,12 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, { char *cpu = NULL; - virCheckFlags(0, NULL); + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); if (virConnectBaselineCPUEnsureACL(conn) < 0) goto cleanup; - cpu = cpuBaselineXML(xmlCPUs, ncpus, NULL, 0); + cpu = cpuBaselineXML(xmlCPUs, ncpus, NULL, 0, flags); cleanup: return cpu; diff --git a/tests/cputest.c b/tests/cputest.c index 2e5f0cd..959cb9f 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -75,6 +75,7 @@ struct data { const char *modelsName; unsigned int nmodels; const char *preferred; + unsigned int flags; int result; }; @@ -330,7 +331,7 @@ cpuTestBaseline(const void *arg) if (!(cpus = cpuTestLoadMultiXML(data->arch, data->name, &ncpus))) goto cleanup; - baseline = cpuBaseline(cpus, ncpus, NULL, 0); + baseline = cpuBaseline(cpus, ncpus, NULL, 0, data->flags); if (data->result < 0) { virResetLastError(); if (!baseline) @@ -510,12 +511,12 @@ mymain(void) } #define DO_TEST(arch, api, name, host, cpu, \ - models, nmodels, preferred, result) \ + models, nmodels, preferred, flags, result) \ do { \ static struct data data = { \ arch, api, host, cpu, models, \ models == NULL ? NULL : #models, \ - nmodels, preferred, result \ + nmodels, preferred, flags, result \ }; \ if (cpuTestRun(name, &data) < 0) \ ret = -1; \ @@ -524,31 +525,31 @@ mymain(void) #define DO_TEST_COMPARE(arch, host, cpu, result) \ DO_TEST(arch, API_COMPARE, \ host "/" cpu " (" #result ")", \ - host, cpu, NULL, 0, NULL, result) + host, cpu, NULL, 0, NULL, 0, result) #define DO_TEST_UPDATE(arch, host, cpu, result) \ do { \ DO_TEST(arch, API_UPDATE, \ cpu " on " host, \ - host, cpu, NULL, 0, NULL, 0); \ + host, cpu, NULL, 0, NULL, 0, 0); \ DO_TEST_COMPARE(arch, host, host "+" cpu, result); \ } while (0) -#define DO_TEST_BASELINE(arch, name, result) \ +#define DO_TEST_BASELINE(arch, name, flags, result) \ DO_TEST(arch, API_BASELINE, name, NULL, "baseline-" name, \ - NULL, 0, NULL, result) + NULL, 0, NULL, flags, result) #define DO_TEST_HASFEATURE(arch, host, feature, result) \ DO_TEST(arch, API_HAS_FEATURE, \ host "/" feature " (" #result ")", \ - host, feature, NULL, 0, NULL, result) + host, feature, NULL, 0, NULL, 0, result) #define DO_TEST_GUESTDATA(arch, host, cpu, models, preferred, result) \ DO_TEST(arch, API_GUEST_DATA, \ host "/" cpu " (" #models ", pref=" #preferred ")", \ host, cpu, models, \ models == NULL ? 0 : sizeof(models) / sizeof(char *), \ - preferred, result) + preferred, 0, result) /* host to host comparison */ DO_TEST_COMPARE("x86", "host", "host", VIR_CPU_COMPARE_IDENTICAL); @@ -593,11 +594,12 @@ mymain(void) DO_TEST_UPDATE("x86", "host", "host-passthrough", VIR_CPU_COMPARE_IDENTICAL); /* computing baseline CPUs */ - DO_TEST_BASELINE("x86", "incompatible-vendors", -1); - DO_TEST_BASELINE("x86", "no-vendor", 0); - DO_TEST_BASELINE("x86", "some-vendors", 0); - DO_TEST_BASELINE("x86", "1", 0); - DO_TEST_BASELINE("x86", "2", 0); + DO_TEST_BASELINE("x86", "incompatible-vendors", 0, -1); + DO_TEST_BASELINE("x86", "no-vendor", 0, 0); + DO_TEST_BASELINE("x86", "some-vendors", 0, 0); + DO_TEST_BASELINE("x86", "1", 0, 0); + DO_TEST_BASELINE("x86", "2", 0, 0); + DO_TEST_BASELINE("x86", "3", VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, 0); /* CPU features */ DO_TEST_HASFEATURE("x86", "host", "vmx", YES); diff --git a/tests/cputestdata/x86-baseline-3-result.xml b/tests/cputestdata/x86-baseline-3-result.xml new file mode 100644 index 0000000..d196112 --- /dev/null +++ b/tests/cputestdata/x86-baseline-3-result.xml @@ -0,0 +1,35 @@ +<cpu mode='custom' match='exact'> + <model fallback='allow'>Westmere</model> + <feature policy='require' name='lahf_lm'/> + <feature policy='require' name='lm'/> + <feature policy='require' name='nx'/> + <feature policy='require' name='syscall'/> + <feature policy='require' name='aes'/> + <feature policy='require' name='popcnt'/> + <feature policy='require' name='sse4.2'/> + <feature policy='require' name='sse4.1'/> + <feature policy='require' name='cx16'/> + <feature policy='require' name='ssse3'/> + <feature policy='require' name='pni'/> + <feature policy='require' name='sse2'/> + <feature policy='require' name='sse'/> + <feature policy='require' name='fxsr'/> + <feature policy='require' name='mmx'/> + <feature policy='require' name='clflush'/> + <feature policy='require' name='pse36'/> + <feature policy='require' name='pat'/> + <feature policy='require' name='cmov'/> + <feature policy='require' name='mca'/> + <feature policy='require' name='pge'/> + <feature policy='require' name='mtrr'/> + <feature policy='require' name='sep'/> + <feature policy='require' name='apic'/> + <feature policy='require' name='cx8'/> + <feature policy='require' name='mce'/> + <feature policy='require' name='pae'/> + <feature policy='require' name='msr'/> + <feature policy='require' name='tsc'/> + <feature policy='require' name='pse'/> + <feature policy='require' name='de'/> + <feature policy='require' name='fpu'/> +</cpu> diff --git a/tests/cputestdata/x86-baseline-3.xml b/tests/cputestdata/x86-baseline-3.xml new file mode 100644 index 0000000..7654a1d --- /dev/null +++ b/tests/cputestdata/x86-baseline-3.xml @@ -0,0 +1,7 @@ +<cpuTest> +<cpu> + <arch>x86_64</arch> + <model>Westmere</model> + <topology sockets='1' cores='2' threads='1'/> +</cpu> +</cpuTest> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 8cafce4..a1d5ab4 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6148,6 +6148,10 @@ static const vshCmdOptDef opts_cpu_baseline[] = { .flags = VSH_OFLAG_REQ, .help = N_("file containing XML CPU descriptions") }, + {.name = "features", + .type = VSH_OT_BOOL, + .help = N_("Show features that are part of the CPU model type") + }, {.name = NULL} }; @@ -6159,6 +6163,7 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) char *buffer; char *result = NULL; const char **list = NULL; + unsigned int flags = 0; int count = 0; xmlDocPtr xml = NULL; @@ -6168,6 +6173,9 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) virBuffer buf = VIR_BUFFER_INITIALIZER; size_t i; + if (vshCommandOptBool(cmd, "model_features")) + flags |= VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES; + if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) return false; @@ -6211,7 +6219,7 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) list[i] = vshStrdup(ctl, (const char *)xmlBufferContent(xml_buf)); } - result = virConnectBaselineCPU(ctl->conn, list, count, 0); + result = virConnectBaselineCPU(ctl->conn, list, count, flags); if (result) { vshPrint(ctl, "%s", result); diff --git a/tools/virsh.pod b/tools/virsh.pod index 3ff6da1..0ae5178 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -485,13 +485,16 @@ cell and the total free memory on the machine. Finally, with a numeric argument or with --cellno plus a cell number it will display the free memory for the specified cell only. -=item B<cpu-baseline> I<FILE> +=item B<cpu-baseline> I<FILE> [I<--features>] Compute baseline CPU which will be supported by all host CPUs given in <file>. The list of host CPUs is built by extracting all <cpu> elements from the <file>. Thus, the <file> can contain either a set of <cpu> elements separated by new lines or even a set of complete <capabilities> elements printed by -B<capabilities> command. +B<capabilities> command. If I<--features> is specified then the +resulting XML description will explicitly include all features that make +up the CPU, without this option features that are part of the CPU model +will not be listed in the XML description. =item B<cpu-compare> I<FILE> -- 1.7.10.4 -- Don Dugger "Censeo Toto nos in Kansa esse decisse." - D. Gale n0ano@n0ano.com Ph: 303/443-3786
Has anybody had a chance to review this patch? On Fri, Aug 02, 2013 at 01:08:19PM -0600, n0ano wrote:
Currently the virConnectBaselineCPU API does not expose the CPU features that are part of the CPU's model. This patch adds a new flag, VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, that causes the API to explictly list all features that are part of that model.
Signed-off-by: Don Dugger <donald.d.dugger@intel.com>
--- [V2 - per review comments change the flag name to be more descriptive and change errors from warnings to propogated errors.]
[V3 - Incorporate review suggestions to better handle flags and errors. Also add documentation about the API change.]
include/libvirt/libvirt.h.in | 9 ++++++ src/cpu/cpu.c | 12 ++++--- src/cpu/cpu.h | 12 ++++--- src/cpu/cpu_arm.c | 6 +++- src/cpu/cpu_generic.c | 5 ++- src/cpu/cpu_powerpc.c | 10 ++++-- src/cpu/cpu_s390.c | 6 +++- src/cpu/cpu_x86.c | 45 ++++++++++++++++++++++++--- src/libvirt.c | 7 ++++- src/qemu/qemu_driver.c | 4 +-- tests/cputest.c | 30 +++++++++--------- tests/cputestdata/x86-baseline-3-result.xml | 35 +++++++++++++++++++++ tests/cputestdata/x86-baseline-3.xml | 7 +++++ tools/virsh-domain.c | 10 +++++- tools/virsh.pod | 7 +++-- 15 files changed, 166 insertions(+), 39 deletions(-) create mode 100644 tests/cputestdata/x86-baseline-3-result.xml create mode 100644 tests/cputestdata/x86-baseline-3.xml
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 7bd3559..b843355 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4008,6 +4008,15 @@ int virConnectCompareCPU(virConnectPtr conn,
/** + * virConnectBaselineCPUFlags + * + * Flags when getting XML description of a computed CPU + */ +typedef enum { + VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES = (1 << 0), /* show all features */ +} virConnectBaselineCPUFlags; + +/** * virConnectBaselineCPU: * * @conn: virConnect connection diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 4124354..023ce26 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -167,7 +167,7 @@ cpuDecode(virCPUDefPtr cpu, return -1; }
- return driver->decode(cpu, data, models, nmodels, preferred); + return driver->decode(cpu, data, models, nmodels, preferred, 0); }
@@ -276,7 +276,8 @@ char * cpuBaselineXML(const char **xmlCPUs, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { xmlDocPtr doc = NULL; xmlXPathContextPtr ctxt = NULL; @@ -323,7 +324,7 @@ cpuBaselineXML(const char **xmlCPUs, doc = NULL; }
- if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels))) + if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels, flags))) goto error;
cpustr = virCPUDefFormat(cpu, 0); @@ -350,7 +351,8 @@ virCPUDefPtr cpuBaseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { struct cpuArchDriver *driver; size_t i; @@ -392,7 +394,7 @@ cpuBaseline(virCPUDefPtr *cpus, return NULL; }
- return driver->baseline(cpus, ncpus, models, nmodels); + return driver->baseline(cpus, ncpus, models, nmodels, flags); }
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 4003435..7f1d4bd 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -53,7 +53,8 @@ typedef int const virCPUDataPtr data, const char **models, unsigned int nmodels, - const char *preferred); + const char *preferred, + unsigned int flags);
typedef int (*cpuArchEncode) (virArch arch, @@ -81,7 +82,8 @@ typedef virCPUDefPtr (*cpuArchBaseline) (virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels); + unsigned int nmodels, + unsigned int flags);
typedef int (*cpuArchUpdate) (virCPUDefPtr guest, @@ -149,13 +151,15 @@ extern char * cpuBaselineXML(const char **xmlCPUs, unsigned int ncpus, const char **models, - unsigned int nmodels); + unsigned int nmodels, + unsigned int flags);
extern virCPUDefPtr cpuBaseline (virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels); + unsigned int nmodels, + unsigned int flags);
extern int cpuUpdate (virCPUDefPtr guest, diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index 25e25ba..d1b2a99 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -44,8 +44,12 @@ ArmDecode(virCPUDefPtr cpu ATTRIBUTE_UNUSED, const virCPUDataPtr data ATTRIBUTE_UNUSED, const char **models ATTRIBUTE_UNUSED, unsigned int nmodels ATTRIBUTE_UNUSED, - const char *preferred ATTRIBUTE_UNUSED) + const char *preferred ATTRIBUTE_UNUSED, + unsigned int flags) { + + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); + return 0; }
diff --git a/src/cpu/cpu_generic.c b/src/cpu/cpu_generic.c index 2fe792f..1264da4 100644 --- a/src/cpu/cpu_generic.c +++ b/src/cpu/cpu_generic.c @@ -113,7 +113,8 @@ static virCPUDefPtr genericBaseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { virCPUDefPtr cpu = NULL; virCPUFeatureDefPtr features = NULL; @@ -121,6 +122,8 @@ genericBaseline(virCPUDefPtr *cpus, unsigned int count; size_t i, j;
+ virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); + if (!cpuModelIsAllowed(cpus[0]->model, models, nmodels)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("CPU model %s is not supported by hypervisor"), diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c index 55a4153..647a8a1 100644 --- a/src/cpu/cpu_powerpc.c +++ b/src/cpu/cpu_powerpc.c @@ -304,12 +304,15 @@ ppcDecode(virCPUDefPtr cpu, const virCPUDataPtr data, const char **models, unsigned int nmodels, - const char *preferred ATTRIBUTE_UNUSED) + const char *preferred ATTRIBUTE_UNUSED, + unsigned int flags) { int ret = -1; struct ppc_map *map; const struct ppc_model *model;
+ virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); + if (data == NULL || (map = ppcLoadMap()) == NULL) return -1;
@@ -377,7 +380,8 @@ static virCPUDefPtr ppcBaseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { struct ppc_map *map = NULL; const struct ppc_model *model; @@ -385,6 +389,8 @@ ppcBaseline(virCPUDefPtr *cpus, virCPUDefPtr cpu = NULL; size_t i;
+ virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); + if (!(map = ppcLoadMap())) goto error;
diff --git a/src/cpu/cpu_s390.c b/src/cpu/cpu_s390.c index cbfae42..d997e06 100644 --- a/src/cpu/cpu_s390.c +++ b/src/cpu/cpu_s390.c @@ -48,8 +48,12 @@ s390Decode(virCPUDefPtr cpu ATTRIBUTE_UNUSED, const virCPUDataPtr data ATTRIBUTE_UNUSED, const char **models ATTRIBUTE_UNUSED, unsigned int nmodels ATTRIBUTE_UNUSED, - const char *preferred ATTRIBUTE_UNUSED) + const char *preferred ATTRIBUTE_UNUSED, + unsigned int flags) { + + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); + return 0; }
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index a388f0f..f16a3cb 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1319,13 +1319,41 @@ x86GuestData(virCPUDefPtr host, return x86Compute(host, guest, data, message); }
+static int +x86AddFeatures(virCPUDefPtr cpu, + struct x86_map *map) +{ + const struct x86_model *candidate; + const struct x86_feature *feature = map->features; + + candidate = map->models; + while (candidate != NULL) { + if (STREQ(cpu->model, candidate->name)) + break; + candidate = candidate->next; + } + if (!candidate) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s not a known CPU model"), cpu->model); + return -1; + } + while (feature != NULL) { + if (x86DataIsSubset(candidate->data, feature->data) && + (virCPUDefAddFeature(cpu, feature->name, VIR_CPU_FEATURE_REQUIRE) < 0)) + return -1; + feature = feature->next; + } + return 0; +} +
static int x86Decode(virCPUDefPtr cpu, const struct cpuX86Data *data, const char **models, unsigned int nmodels, - const char *preferred) + const char *preferred, + unsigned int flags) { int ret = -1; struct x86_map *map; @@ -1334,6 +1362,8 @@ x86Decode(virCPUDefPtr cpu, virCPUDefPtr cpuModel = NULL; size_t i;
+ virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); + if (data == NULL || (map = x86LoadMap()) == NULL) return -1;
@@ -1406,6 +1436,9 @@ x86Decode(virCPUDefPtr cpu, goto out; }
+ if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES && + (x86AddFeatures(cpuModel, map) < 0)) + goto out; cpu->model = cpuModel->model; cpu->vendor = cpuModel->vendor; cpu->nfeatures = cpuModel->nfeatures; @@ -1426,9 +1459,10 @@ x86DecodeCPUData(virCPUDefPtr cpu, const virCPUDataPtr data, const char **models, unsigned int nmodels, - const char *preferred) + const char *preferred, + unsigned int flags) { - return x86Decode(cpu, data->data.x86, models, nmodels, preferred); + return x86Decode(cpu, data->data.x86, models, nmodels, preferred, flags); }
@@ -1674,7 +1708,8 @@ static virCPUDefPtr x86Baseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { struct x86_map *map = NULL; struct x86_model *base_model = NULL; @@ -1755,7 +1790,7 @@ x86Baseline(virCPUDefPtr *cpus, if (vendor && x86DataAddCpuid(base_model->data, &vendor->cpuid) < 0) goto error;
- if (x86Decode(cpu, base_model->data, models, nmodels, NULL) < 0) + if (x86Decode(cpu, base_model->data, models, nmodels, NULL, flags) < 0) goto error;
if (!outputVendor) diff --git a/src/libvirt.c b/src/libvirt.c index 8157488..6ce6c32 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18522,11 +18522,16 @@ error: * @conn: virConnect connection * @xmlCPUs: array of XML descriptions of host CPUs * @ncpus: number of CPUs in xmlCPUs - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virConnectBaselineCPUFlags * * Computes the most feature-rich CPU which is compatible with all given * host CPUs. * + * If @flags includes VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES then libvirt + * will explicitly list all CPU features that are part of the host CPU, + * without this flag features that are part of the CPU model will not be + * listed. + * * Returns XML description of the computed CPU or NULL on error. */ char * diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2daafa8..2ad236e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11056,12 +11056,12 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, { char *cpu = NULL;
- virCheckFlags(0, NULL); + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL);
if (virConnectBaselineCPUEnsureACL(conn) < 0) goto cleanup;
- cpu = cpuBaselineXML(xmlCPUs, ncpus, NULL, 0); + cpu = cpuBaselineXML(xmlCPUs, ncpus, NULL, 0, flags);
cleanup: return cpu; diff --git a/tests/cputest.c b/tests/cputest.c index 2e5f0cd..959cb9f 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -75,6 +75,7 @@ struct data { const char *modelsName; unsigned int nmodels; const char *preferred; + unsigned int flags; int result; };
@@ -330,7 +331,7 @@ cpuTestBaseline(const void *arg) if (!(cpus = cpuTestLoadMultiXML(data->arch, data->name, &ncpus))) goto cleanup;
- baseline = cpuBaseline(cpus, ncpus, NULL, 0); + baseline = cpuBaseline(cpus, ncpus, NULL, 0, data->flags); if (data->result < 0) { virResetLastError(); if (!baseline) @@ -510,12 +511,12 @@ mymain(void) }
#define DO_TEST(arch, api, name, host, cpu, \ - models, nmodels, preferred, result) \ + models, nmodels, preferred, flags, result) \ do { \ static struct data data = { \ arch, api, host, cpu, models, \ models == NULL ? NULL : #models, \ - nmodels, preferred, result \ + nmodels, preferred, flags, result \ }; \ if (cpuTestRun(name, &data) < 0) \ ret = -1; \ @@ -524,31 +525,31 @@ mymain(void) #define DO_TEST_COMPARE(arch, host, cpu, result) \ DO_TEST(arch, API_COMPARE, \ host "/" cpu " (" #result ")", \ - host, cpu, NULL, 0, NULL, result) + host, cpu, NULL, 0, NULL, 0, result)
#define DO_TEST_UPDATE(arch, host, cpu, result) \ do { \ DO_TEST(arch, API_UPDATE, \ cpu " on " host, \ - host, cpu, NULL, 0, NULL, 0); \ + host, cpu, NULL, 0, NULL, 0, 0); \ DO_TEST_COMPARE(arch, host, host "+" cpu, result); \ } while (0)
-#define DO_TEST_BASELINE(arch, name, result) \ +#define DO_TEST_BASELINE(arch, name, flags, result) \ DO_TEST(arch, API_BASELINE, name, NULL, "baseline-" name, \ - NULL, 0, NULL, result) + NULL, 0, NULL, flags, result)
#define DO_TEST_HASFEATURE(arch, host, feature, result) \ DO_TEST(arch, API_HAS_FEATURE, \ host "/" feature " (" #result ")", \ - host, feature, NULL, 0, NULL, result) + host, feature, NULL, 0, NULL, 0, result)
#define DO_TEST_GUESTDATA(arch, host, cpu, models, preferred, result) \ DO_TEST(arch, API_GUEST_DATA, \ host "/" cpu " (" #models ", pref=" #preferred ")", \ host, cpu, models, \ models == NULL ? 0 : sizeof(models) / sizeof(char *), \ - preferred, result) + preferred, 0, result)
/* host to host comparison */ DO_TEST_COMPARE("x86", "host", "host", VIR_CPU_COMPARE_IDENTICAL); @@ -593,11 +594,12 @@ mymain(void) DO_TEST_UPDATE("x86", "host", "host-passthrough", VIR_CPU_COMPARE_IDENTICAL);
/* computing baseline CPUs */ - DO_TEST_BASELINE("x86", "incompatible-vendors", -1); - DO_TEST_BASELINE("x86", "no-vendor", 0); - DO_TEST_BASELINE("x86", "some-vendors", 0); - DO_TEST_BASELINE("x86", "1", 0); - DO_TEST_BASELINE("x86", "2", 0); + DO_TEST_BASELINE("x86", "incompatible-vendors", 0, -1); + DO_TEST_BASELINE("x86", "no-vendor", 0, 0); + DO_TEST_BASELINE("x86", "some-vendors", 0, 0); + DO_TEST_BASELINE("x86", "1", 0, 0); + DO_TEST_BASELINE("x86", "2", 0, 0); + DO_TEST_BASELINE("x86", "3", VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, 0);
/* CPU features */ DO_TEST_HASFEATURE("x86", "host", "vmx", YES); diff --git a/tests/cputestdata/x86-baseline-3-result.xml b/tests/cputestdata/x86-baseline-3-result.xml new file mode 100644 index 0000000..d196112 --- /dev/null +++ b/tests/cputestdata/x86-baseline-3-result.xml @@ -0,0 +1,35 @@ +<cpu mode='custom' match='exact'> + <model fallback='allow'>Westmere</model> + <feature policy='require' name='lahf_lm'/> + <feature policy='require' name='lm'/> + <feature policy='require' name='nx'/> + <feature policy='require' name='syscall'/> + <feature policy='require' name='aes'/> + <feature policy='require' name='popcnt'/> + <feature policy='require' name='sse4.2'/> + <feature policy='require' name='sse4.1'/> + <feature policy='require' name='cx16'/> + <feature policy='require' name='ssse3'/> + <feature policy='require' name='pni'/> + <feature policy='require' name='sse2'/> + <feature policy='require' name='sse'/> + <feature policy='require' name='fxsr'/> + <feature policy='require' name='mmx'/> + <feature policy='require' name='clflush'/> + <feature policy='require' name='pse36'/> + <feature policy='require' name='pat'/> + <feature policy='require' name='cmov'/> + <feature policy='require' name='mca'/> + <feature policy='require' name='pge'/> + <feature policy='require' name='mtrr'/> + <feature policy='require' name='sep'/> + <feature policy='require' name='apic'/> + <feature policy='require' name='cx8'/> + <feature policy='require' name='mce'/> + <feature policy='require' name='pae'/> + <feature policy='require' name='msr'/> + <feature policy='require' name='tsc'/> + <feature policy='require' name='pse'/> + <feature policy='require' name='de'/> + <feature policy='require' name='fpu'/> +</cpu> diff --git a/tests/cputestdata/x86-baseline-3.xml b/tests/cputestdata/x86-baseline-3.xml new file mode 100644 index 0000000..7654a1d --- /dev/null +++ b/tests/cputestdata/x86-baseline-3.xml @@ -0,0 +1,7 @@ +<cpuTest> +<cpu> + <arch>x86_64</arch> + <model>Westmere</model> + <topology sockets='1' cores='2' threads='1'/> +</cpu> +</cpuTest> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 8cafce4..a1d5ab4 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6148,6 +6148,10 @@ static const vshCmdOptDef opts_cpu_baseline[] = { .flags = VSH_OFLAG_REQ, .help = N_("file containing XML CPU descriptions") }, + {.name = "features", + .type = VSH_OT_BOOL, + .help = N_("Show features that are part of the CPU model type") + }, {.name = NULL} };
@@ -6159,6 +6163,7 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) char *buffer; char *result = NULL; const char **list = NULL; + unsigned int flags = 0; int count = 0;
xmlDocPtr xml = NULL; @@ -6168,6 +6173,9 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) virBuffer buf = VIR_BUFFER_INITIALIZER; size_t i;
+ if (vshCommandOptBool(cmd, "model_features")) + flags |= VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES; + if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) return false;
@@ -6211,7 +6219,7 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) list[i] = vshStrdup(ctl, (const char *)xmlBufferContent(xml_buf)); }
- result = virConnectBaselineCPU(ctl->conn, list, count, 0); + result = virConnectBaselineCPU(ctl->conn, list, count, flags);
if (result) { vshPrint(ctl, "%s", result); diff --git a/tools/virsh.pod b/tools/virsh.pod index 3ff6da1..0ae5178 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -485,13 +485,16 @@ cell and the total free memory on the machine. Finally, with a numeric argument or with --cellno plus a cell number it will display the free memory for the specified cell only.
-=item B<cpu-baseline> I<FILE> +=item B<cpu-baseline> I<FILE> [I<--features>]
Compute baseline CPU which will be supported by all host CPUs given in <file>. The list of host CPUs is built by extracting all <cpu> elements from the <file>. Thus, the <file> can contain either a set of <cpu> elements separated by new lines or even a set of complete <capabilities> elements printed by -B<capabilities> command. +B<capabilities> command. If I<--features> is specified then the +resulting XML description will explicitly include all features that make +up the CPU, without this option features that are part of the CPU model +will not be listed in the XML description.
=item B<cpu-compare> I<FILE>
-- 1.7.10.4
-- Don Dugger "Censeo Toto nos in Kansa esse decisse." - D. Gale n0ano@n0ano.com Ph: 303/443-3786
-- Don Dugger "Censeo Toto nos in Kansa esse decisse." - D. Gale n0ano@n0ano.com Ph: 303/443-3786
Still hoping that someone can reveiw this patch. On Fri, Aug 02, 2013 at 01:08:19PM -0600, n0ano wrote:
Currently the virConnectBaselineCPU API does not expose the CPU features that are part of the CPU's model. This patch adds a new flag, VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, that causes the API to explictly list all features that are part of that model.
Signed-off-by: Don Dugger <donald.d.dugger@intel.com>
--- [V2 - per review comments change the flag name to be more descriptive and change errors from warnings to propogated errors.]
[V3 - Incorporate review suggestions to better handle flags and errors. Also add documentation about the API change.]
include/libvirt/libvirt.h.in | 9 ++++++ src/cpu/cpu.c | 12 ++++--- src/cpu/cpu.h | 12 ++++--- src/cpu/cpu_arm.c | 6 +++- src/cpu/cpu_generic.c | 5 ++- src/cpu/cpu_powerpc.c | 10 ++++-- src/cpu/cpu_s390.c | 6 +++- src/cpu/cpu_x86.c | 45 ++++++++++++++++++++++++--- src/libvirt.c | 7 ++++- src/qemu/qemu_driver.c | 4 +-- tests/cputest.c | 30 +++++++++--------- tests/cputestdata/x86-baseline-3-result.xml | 35 +++++++++++++++++++++ tests/cputestdata/x86-baseline-3.xml | 7 +++++ tools/virsh-domain.c | 10 +++++- tools/virsh.pod | 7 +++-- 15 files changed, 166 insertions(+), 39 deletions(-) create mode 100644 tests/cputestdata/x86-baseline-3-result.xml create mode 100644 tests/cputestdata/x86-baseline-3.xml
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 7bd3559..b843355 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4008,6 +4008,15 @@ int virConnectCompareCPU(virConnectPtr conn,
/** + * virConnectBaselineCPUFlags + * + * Flags when getting XML description of a computed CPU + */ +typedef enum { + VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES = (1 << 0), /* show all features */ +} virConnectBaselineCPUFlags; + +/** * virConnectBaselineCPU: * * @conn: virConnect connection diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 4124354..023ce26 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -167,7 +167,7 @@ cpuDecode(virCPUDefPtr cpu, return -1; }
- return driver->decode(cpu, data, models, nmodels, preferred); + return driver->decode(cpu, data, models, nmodels, preferred, 0); }
@@ -276,7 +276,8 @@ char * cpuBaselineXML(const char **xmlCPUs, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { xmlDocPtr doc = NULL; xmlXPathContextPtr ctxt = NULL; @@ -323,7 +324,7 @@ cpuBaselineXML(const char **xmlCPUs, doc = NULL; }
- if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels))) + if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels, flags))) goto error;
cpustr = virCPUDefFormat(cpu, 0); @@ -350,7 +351,8 @@ virCPUDefPtr cpuBaseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { struct cpuArchDriver *driver; size_t i; @@ -392,7 +394,7 @@ cpuBaseline(virCPUDefPtr *cpus, return NULL; }
- return driver->baseline(cpus, ncpus, models, nmodels); + return driver->baseline(cpus, ncpus, models, nmodels, flags); }
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 4003435..7f1d4bd 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -53,7 +53,8 @@ typedef int const virCPUDataPtr data, const char **models, unsigned int nmodels, - const char *preferred); + const char *preferred, + unsigned int flags);
typedef int (*cpuArchEncode) (virArch arch, @@ -81,7 +82,8 @@ typedef virCPUDefPtr (*cpuArchBaseline) (virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels); + unsigned int nmodels, + unsigned int flags);
typedef int (*cpuArchUpdate) (virCPUDefPtr guest, @@ -149,13 +151,15 @@ extern char * cpuBaselineXML(const char **xmlCPUs, unsigned int ncpus, const char **models, - unsigned int nmodels); + unsigned int nmodels, + unsigned int flags);
extern virCPUDefPtr cpuBaseline (virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels); + unsigned int nmodels, + unsigned int flags);
extern int cpuUpdate (virCPUDefPtr guest, diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index 25e25ba..d1b2a99 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -44,8 +44,12 @@ ArmDecode(virCPUDefPtr cpu ATTRIBUTE_UNUSED, const virCPUDataPtr data ATTRIBUTE_UNUSED, const char **models ATTRIBUTE_UNUSED, unsigned int nmodels ATTRIBUTE_UNUSED, - const char *preferred ATTRIBUTE_UNUSED) + const char *preferred ATTRIBUTE_UNUSED, + unsigned int flags) { + + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); + return 0; }
diff --git a/src/cpu/cpu_generic.c b/src/cpu/cpu_generic.c index 2fe792f..1264da4 100644 --- a/src/cpu/cpu_generic.c +++ b/src/cpu/cpu_generic.c @@ -113,7 +113,8 @@ static virCPUDefPtr genericBaseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { virCPUDefPtr cpu = NULL; virCPUFeatureDefPtr features = NULL; @@ -121,6 +122,8 @@ genericBaseline(virCPUDefPtr *cpus, unsigned int count; size_t i, j;
+ virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); + if (!cpuModelIsAllowed(cpus[0]->model, models, nmodels)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("CPU model %s is not supported by hypervisor"), diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c index 55a4153..647a8a1 100644 --- a/src/cpu/cpu_powerpc.c +++ b/src/cpu/cpu_powerpc.c @@ -304,12 +304,15 @@ ppcDecode(virCPUDefPtr cpu, const virCPUDataPtr data, const char **models, unsigned int nmodels, - const char *preferred ATTRIBUTE_UNUSED) + const char *preferred ATTRIBUTE_UNUSED, + unsigned int flags) { int ret = -1; struct ppc_map *map; const struct ppc_model *model;
+ virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); + if (data == NULL || (map = ppcLoadMap()) == NULL) return -1;
@@ -377,7 +380,8 @@ static virCPUDefPtr ppcBaseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { struct ppc_map *map = NULL; const struct ppc_model *model; @@ -385,6 +389,8 @@ ppcBaseline(virCPUDefPtr *cpus, virCPUDefPtr cpu = NULL; size_t i;
+ virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); + if (!(map = ppcLoadMap())) goto error;
diff --git a/src/cpu/cpu_s390.c b/src/cpu/cpu_s390.c index cbfae42..d997e06 100644 --- a/src/cpu/cpu_s390.c +++ b/src/cpu/cpu_s390.c @@ -48,8 +48,12 @@ s390Decode(virCPUDefPtr cpu ATTRIBUTE_UNUSED, const virCPUDataPtr data ATTRIBUTE_UNUSED, const char **models ATTRIBUTE_UNUSED, unsigned int nmodels ATTRIBUTE_UNUSED, - const char *preferred ATTRIBUTE_UNUSED) + const char *preferred ATTRIBUTE_UNUSED, + unsigned int flags) { + + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); + return 0; }
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index a388f0f..f16a3cb 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1319,13 +1319,41 @@ x86GuestData(virCPUDefPtr host, return x86Compute(host, guest, data, message); }
+static int +x86AddFeatures(virCPUDefPtr cpu, + struct x86_map *map) +{ + const struct x86_model *candidate; + const struct x86_feature *feature = map->features; + + candidate = map->models; + while (candidate != NULL) { + if (STREQ(cpu->model, candidate->name)) + break; + candidate = candidate->next; + } + if (!candidate) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s not a known CPU model"), cpu->model); + return -1; + } + while (feature != NULL) { + if (x86DataIsSubset(candidate->data, feature->data) && + (virCPUDefAddFeature(cpu, feature->name, VIR_CPU_FEATURE_REQUIRE) < 0)) + return -1; + feature = feature->next; + } + return 0; +} +
static int x86Decode(virCPUDefPtr cpu, const struct cpuX86Data *data, const char **models, unsigned int nmodels, - const char *preferred) + const char *preferred, + unsigned int flags) { int ret = -1; struct x86_map *map; @@ -1334,6 +1362,8 @@ x86Decode(virCPUDefPtr cpu, virCPUDefPtr cpuModel = NULL; size_t i;
+ virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); + if (data == NULL || (map = x86LoadMap()) == NULL) return -1;
@@ -1406,6 +1436,9 @@ x86Decode(virCPUDefPtr cpu, goto out; }
+ if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES && + (x86AddFeatures(cpuModel, map) < 0)) + goto out; cpu->model = cpuModel->model; cpu->vendor = cpuModel->vendor; cpu->nfeatures = cpuModel->nfeatures; @@ -1426,9 +1459,10 @@ x86DecodeCPUData(virCPUDefPtr cpu, const virCPUDataPtr data, const char **models, unsigned int nmodels, - const char *preferred) + const char *preferred, + unsigned int flags) { - return x86Decode(cpu, data->data.x86, models, nmodels, preferred); + return x86Decode(cpu, data->data.x86, models, nmodels, preferred, flags); }
@@ -1674,7 +1708,8 @@ static virCPUDefPtr x86Baseline(virCPUDefPtr *cpus, unsigned int ncpus, const char **models, - unsigned int nmodels) + unsigned int nmodels, + unsigned int flags) { struct x86_map *map = NULL; struct x86_model *base_model = NULL; @@ -1755,7 +1790,7 @@ x86Baseline(virCPUDefPtr *cpus, if (vendor && x86DataAddCpuid(base_model->data, &vendor->cpuid) < 0) goto error;
- if (x86Decode(cpu, base_model->data, models, nmodels, NULL) < 0) + if (x86Decode(cpu, base_model->data, models, nmodels, NULL, flags) < 0) goto error;
if (!outputVendor) diff --git a/src/libvirt.c b/src/libvirt.c index 8157488..6ce6c32 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -18522,11 +18522,16 @@ error: * @conn: virConnect connection * @xmlCPUs: array of XML descriptions of host CPUs * @ncpus: number of CPUs in xmlCPUs - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virConnectBaselineCPUFlags * * Computes the most feature-rich CPU which is compatible with all given * host CPUs. * + * If @flags includes VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES then libvirt + * will explicitly list all CPU features that are part of the host CPU, + * without this flag features that are part of the CPU model will not be + * listed. + * * Returns XML description of the computed CPU or NULL on error. */ char * diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2daafa8..2ad236e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11056,12 +11056,12 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, { char *cpu = NULL;
- virCheckFlags(0, NULL); + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL);
if (virConnectBaselineCPUEnsureACL(conn) < 0) goto cleanup;
- cpu = cpuBaselineXML(xmlCPUs, ncpus, NULL, 0); + cpu = cpuBaselineXML(xmlCPUs, ncpus, NULL, 0, flags);
cleanup: return cpu; diff --git a/tests/cputest.c b/tests/cputest.c index 2e5f0cd..959cb9f 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -75,6 +75,7 @@ struct data { const char *modelsName; unsigned int nmodels; const char *preferred; + unsigned int flags; int result; };
@@ -330,7 +331,7 @@ cpuTestBaseline(const void *arg) if (!(cpus = cpuTestLoadMultiXML(data->arch, data->name, &ncpus))) goto cleanup;
- baseline = cpuBaseline(cpus, ncpus, NULL, 0); + baseline = cpuBaseline(cpus, ncpus, NULL, 0, data->flags); if (data->result < 0) { virResetLastError(); if (!baseline) @@ -510,12 +511,12 @@ mymain(void) }
#define DO_TEST(arch, api, name, host, cpu, \ - models, nmodels, preferred, result) \ + models, nmodels, preferred, flags, result) \ do { \ static struct data data = { \ arch, api, host, cpu, models, \ models == NULL ? NULL : #models, \ - nmodels, preferred, result \ + nmodels, preferred, flags, result \ }; \ if (cpuTestRun(name, &data) < 0) \ ret = -1; \ @@ -524,31 +525,31 @@ mymain(void) #define DO_TEST_COMPARE(arch, host, cpu, result) \ DO_TEST(arch, API_COMPARE, \ host "/" cpu " (" #result ")", \ - host, cpu, NULL, 0, NULL, result) + host, cpu, NULL, 0, NULL, 0, result)
#define DO_TEST_UPDATE(arch, host, cpu, result) \ do { \ DO_TEST(arch, API_UPDATE, \ cpu " on " host, \ - host, cpu, NULL, 0, NULL, 0); \ + host, cpu, NULL, 0, NULL, 0, 0); \ DO_TEST_COMPARE(arch, host, host "+" cpu, result); \ } while (0)
-#define DO_TEST_BASELINE(arch, name, result) \ +#define DO_TEST_BASELINE(arch, name, flags, result) \ DO_TEST(arch, API_BASELINE, name, NULL, "baseline-" name, \ - NULL, 0, NULL, result) + NULL, 0, NULL, flags, result)
#define DO_TEST_HASFEATURE(arch, host, feature, result) \ DO_TEST(arch, API_HAS_FEATURE, \ host "/" feature " (" #result ")", \ - host, feature, NULL, 0, NULL, result) + host, feature, NULL, 0, NULL, 0, result)
#define DO_TEST_GUESTDATA(arch, host, cpu, models, preferred, result) \ DO_TEST(arch, API_GUEST_DATA, \ host "/" cpu " (" #models ", pref=" #preferred ")", \ host, cpu, models, \ models == NULL ? 0 : sizeof(models) / sizeof(char *), \ - preferred, result) + preferred, 0, result)
/* host to host comparison */ DO_TEST_COMPARE("x86", "host", "host", VIR_CPU_COMPARE_IDENTICAL); @@ -593,11 +594,12 @@ mymain(void) DO_TEST_UPDATE("x86", "host", "host-passthrough", VIR_CPU_COMPARE_IDENTICAL);
/* computing baseline CPUs */ - DO_TEST_BASELINE("x86", "incompatible-vendors", -1); - DO_TEST_BASELINE("x86", "no-vendor", 0); - DO_TEST_BASELINE("x86", "some-vendors", 0); - DO_TEST_BASELINE("x86", "1", 0); - DO_TEST_BASELINE("x86", "2", 0); + DO_TEST_BASELINE("x86", "incompatible-vendors", 0, -1); + DO_TEST_BASELINE("x86", "no-vendor", 0, 0); + DO_TEST_BASELINE("x86", "some-vendors", 0, 0); + DO_TEST_BASELINE("x86", "1", 0, 0); + DO_TEST_BASELINE("x86", "2", 0, 0); + DO_TEST_BASELINE("x86", "3", VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, 0);
/* CPU features */ DO_TEST_HASFEATURE("x86", "host", "vmx", YES); diff --git a/tests/cputestdata/x86-baseline-3-result.xml b/tests/cputestdata/x86-baseline-3-result.xml new file mode 100644 index 0000000..d196112 --- /dev/null +++ b/tests/cputestdata/x86-baseline-3-result.xml @@ -0,0 +1,35 @@ +<cpu mode='custom' match='exact'> + <model fallback='allow'>Westmere</model> + <feature policy='require' name='lahf_lm'/> + <feature policy='require' name='lm'/> + <feature policy='require' name='nx'/> + <feature policy='require' name='syscall'/> + <feature policy='require' name='aes'/> + <feature policy='require' name='popcnt'/> + <feature policy='require' name='sse4.2'/> + <feature policy='require' name='sse4.1'/> + <feature policy='require' name='cx16'/> + <feature policy='require' name='ssse3'/> + <feature policy='require' name='pni'/> + <feature policy='require' name='sse2'/> + <feature policy='require' name='sse'/> + <feature policy='require' name='fxsr'/> + <feature policy='require' name='mmx'/> + <feature policy='require' name='clflush'/> + <feature policy='require' name='pse36'/> + <feature policy='require' name='pat'/> + <feature policy='require' name='cmov'/> + <feature policy='require' name='mca'/> + <feature policy='require' name='pge'/> + <feature policy='require' name='mtrr'/> + <feature policy='require' name='sep'/> + <feature policy='require' name='apic'/> + <feature policy='require' name='cx8'/> + <feature policy='require' name='mce'/> + <feature policy='require' name='pae'/> + <feature policy='require' name='msr'/> + <feature policy='require' name='tsc'/> + <feature policy='require' name='pse'/> + <feature policy='require' name='de'/> + <feature policy='require' name='fpu'/> +</cpu> diff --git a/tests/cputestdata/x86-baseline-3.xml b/tests/cputestdata/x86-baseline-3.xml new file mode 100644 index 0000000..7654a1d --- /dev/null +++ b/tests/cputestdata/x86-baseline-3.xml @@ -0,0 +1,7 @@ +<cpuTest> +<cpu> + <arch>x86_64</arch> + <model>Westmere</model> + <topology sockets='1' cores='2' threads='1'/> +</cpu> +</cpuTest> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 8cafce4..a1d5ab4 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6148,6 +6148,10 @@ static const vshCmdOptDef opts_cpu_baseline[] = { .flags = VSH_OFLAG_REQ, .help = N_("file containing XML CPU descriptions") }, + {.name = "features", + .type = VSH_OT_BOOL, + .help = N_("Show features that are part of the CPU model type") + }, {.name = NULL} };
@@ -6159,6 +6163,7 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) char *buffer; char *result = NULL; const char **list = NULL; + unsigned int flags = 0; int count = 0;
xmlDocPtr xml = NULL; @@ -6168,6 +6173,9 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) virBuffer buf = VIR_BUFFER_INITIALIZER; size_t i;
+ if (vshCommandOptBool(cmd, "model_features")) + flags |= VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES; + if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) return false;
@@ -6211,7 +6219,7 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) list[i] = vshStrdup(ctl, (const char *)xmlBufferContent(xml_buf)); }
- result = virConnectBaselineCPU(ctl->conn, list, count, 0); + result = virConnectBaselineCPU(ctl->conn, list, count, flags);
if (result) { vshPrint(ctl, "%s", result); diff --git a/tools/virsh.pod b/tools/virsh.pod index 3ff6da1..0ae5178 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -485,13 +485,16 @@ cell and the total free memory on the machine. Finally, with a numeric argument or with --cellno plus a cell number it will display the free memory for the specified cell only.
-=item B<cpu-baseline> I<FILE> +=item B<cpu-baseline> I<FILE> [I<--features>]
Compute baseline CPU which will be supported by all host CPUs given in <file>. The list of host CPUs is built by extracting all <cpu> elements from the <file>. Thus, the <file> can contain either a set of <cpu> elements separated by new lines or even a set of complete <capabilities> elements printed by -B<capabilities> command. +B<capabilities> command. If I<--features> is specified then the +resulting XML description will explicitly include all features that make +up the CPU, without this option features that are part of the CPU model +will not be listed in the XML description.
=item B<cpu-compare> I<FILE>
-- 1.7.10.4
-- Don Dugger "Censeo Toto nos in Kansa esse decisse." - D. Gale n0ano@n0ano.com Ph: 303/443-3786
-- Don Dugger "Censeo Toto nos in Kansa esse decisse." - D. Gale n0ano@n0ano.com Ph: 303/443-3786
On 08/16/2013 08:39 AM, Don Dugger wrote:
Still hoping that someone can reveiw this patch.
I'm reviewing it today.
Currently the virConnectBaselineCPU API does not expose the CPU features that are part of the CPU's model. This patch adds a new flag, VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, that causes the API to explictly list all features that are part of that model.
Signed-off-by: Don Dugger <donald.d.dugger@intel.com>
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/02/2013 01:08 PM, Don Dugger wrote:
Currently the virConnectBaselineCPU API does not expose the CPU features that are part of the CPU's model. This patch adds a new flag, VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, that causes the API to explictly
s/explictly/explicitly
list all features that are part of that model.
Signed-off-by: Don Dugger <donald.d.dugger@intel.com>
--- [V2 - per review comments change the flag name to be more descriptive and change errors from warnings to propogated errors.]
[V3 - Incorporate review suggestions to better handle flags and errors. Also add documentation about the API change.]
+++ b/src/cpu/cpu_x86.c @@ -1319,13 +1319,41 @@ x86GuestData(virCPUDefPtr host, return x86Compute(host, guest, data, message); }
+static int +x86AddFeatures(virCPUDefPtr cpu, + struct x86_map *map) +{ + const struct x86_model *candidate; + const struct x86_feature *feature = map->features; + + candidate = map->models; + while (candidate != NULL) { + if (STREQ(cpu->model, candidate->name)) + break; + candidate = candidate->next; + } + if (!candidate) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s not a known CPU model"), cpu->model); + return -1; + } + while (feature != NULL) { + if (x86DataIsSubset(candidate->data, feature->data) && + (virCPUDefAddFeature(cpu, feature->name, VIR_CPU_FEATURE_REQUIRE) < 0))
The outer () aren't needed on this line.
+ return -1;
Indentation is off.
@@ -1406,6 +1436,9 @@ x86Decode(virCPUDefPtr cpu, goto out; }
+ if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES && + (x86AddFeatures(cpuModel, map) < 0))
Extra ()
+ goto out;
Indentation is off.
+++ b/tools/virsh-domain.c @@ -6148,6 +6148,10 @@ static const vshCmdOptDef opts_cpu_baseline[] = { .flags = VSH_OFLAG_REQ, .help = N_("file containing XML CPU descriptions") }, + {.name = "features",
@@ -6168,6 +6173,9 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) virBuffer buf = VIR_BUFFER_INITIALIZER; size_t i;
+ if (vshCommandOptBool(cmd, "model_features")) + flags |= VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES;
Mismatch in names. How did you even test this? My testing: I used just-compiled virsh with existing libvirtd, and was surprised to get no failure and no difference in output; after tweaking the virsh bug, I finally got expected failure: $ tools/virsh cpu-baseline ../foo.xml --features error: unsupported flags (0x1) in function qemuConnectBaselineCPU then after restarting to just-built libvirtd, I checked that adding --features changed the output. (hmm, maybe virsh can add some sanity checking code to make sure we aren't querying for a mismatched name; but that would be a followup patch...) Those are all minor, so I squashed this in and pushed. Thanks again for being patient, and for the pings along the way. diff --git i/src/cpu/cpu_x86.c w/src/cpu/cpu_x86.c index f16a3cb..41ce21f 100644 --- i/src/cpu/cpu_x86.c +++ w/src/cpu/cpu_x86.c @@ -1339,8 +1339,9 @@ x86AddFeatures(virCPUDefPtr cpu, } while (feature != NULL) { if (x86DataIsSubset(candidate->data, feature->data) && - (virCPUDefAddFeature(cpu, feature->name, VIR_CPU_FEATURE_REQUIRE) < 0)) - return -1; + virCPUDefAddFeature(cpu, feature->name, + VIR_CPU_FEATURE_REQUIRE) < 0) + return -1; feature = feature->next; } return 0; @@ -1437,8 +1438,8 @@ x86Decode(virCPUDefPtr cpu, } if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES && - (x86AddFeatures(cpuModel, map) < 0)) - goto out; + x86AddFeatures(cpuModel, map) < 0) + goto out; cpu->model = cpuModel->model; cpu->vendor = cpuModel->vendor; cpu->nfeatures = cpuModel->nfeatures; diff --git i/tools/virsh-domain.c w/tools/virsh-domain.c index 7d31622..13e3045 100644 --- i/tools/virsh-domain.c +++ w/tools/virsh-domain.c @@ -6182,7 +6182,7 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) virBuffer buf = VIR_BUFFER_INITIALIZER; size_t i; - if (vshCommandOptBool(cmd, "model_features")) + if (vshCommandOptBool(cmd, "features")) flags |= VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES; if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Eric- Tnx much. I apologize for the `virsh' nit, I was doing all my testing against the API directly and got slightly out of sync on the `virsh' command. Again, tnx for cleaning up and pushing this, I appreciate it. On Fri, Aug 16, 2013 at 03:31:14PM -0600, Eric Blake wrote: ...
Those are all minor, so I squashed this in and pushed. Thanks again for being patient, and for the pings along the way.
-- Don Dugger "Censeo Toto nos in Kansa esse decisse." - D. Gale n0ano@n0ano.com Ph: 303/443-3786
On Fri, Aug 16, 2013 at 03:31:14PM -0600, Eric Blake wrote: ...
(hmm, maybe virsh can add some sanity checking code to make sure we aren't querying for a mismatched name; but that would be a followup patch...)
If you can explain this a little I can try and address it. Since I exposed the issue it's only fair that I fix it. -- Don Dugger "Censeo Toto nos in Kansa esse decisse." - D. Gale n0ano@n0ano.com Ph: 303/443-3786
On 08/16/2013 03:51 PM, Don Dugger wrote:
On Fri, Aug 16, 2013 at 03:31:14PM -0600, Eric Blake wrote: ...
(hmm, maybe virsh can add some sanity checking code to make sure we aren't querying for a mismatched name; but that would be a followup patch...)
If you can explain this a little I can try and address it. Since I exposed the issue it's only fair that I fix it.
You exposed a latent problem, but didn't cause the problem yourself. That's more likely to blame somewhat to me. As penance, I already wrote a proposed patch: https://www.redhat.com/archives/libvir-list/2013-August/msg00773.html -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange -
Don Dugger -
Eric Blake