[libvirt] [PATCH 0/5] Add VIR_CONNECT_BASELINE_CPU_MIGRATABLE flag
Add a flag to virConnectBaselineCPU to filter out features that block migration. Ján Tomko (5): Also store features blocking migration as CPUx86Data in the cpu map Add VIR_CONNECT_BASELINE_CPU_MIGRATABLE flag Implement VIR_CONNECT_BASELINE_CPU_MIGRATABLE in the x86 cpu driver Trivially implement VIR_CONNECT_BASELINE_CPU_MIGRATABLE for non-x86 cpus Add --migratable support to virsh cpu-baseline include/libvirt/libvirt-host.h | 1 + src/bhyve/bhyve_driver.c | 3 +- src/cpu/cpu_aarch64.c | 3 +- src/cpu/cpu_arm.c | 3 +- src/cpu/cpu_generic.c | 3 +- src/cpu/cpu_powerpc.c | 3 +- src/cpu/cpu_x86.c | 47 ++++++++++++++++++++++++- src/libvirt-host.c | 3 ++ src/qemu/qemu_driver.c | 3 +- tests/cputest.c | 6 ++++ tests/cputestdata/x86-baseline-6-migratable.xml | 10 ++++++ tests/cputestdata/x86-baseline-6-result.xml | 11 ++++++ tests/cputestdata/x86-baseline-6.xml | 37 +++++++++++++++++++ tools/virsh-domain.c | 6 ++++ tools/virsh.pod | 5 +-- 15 files changed, 135 insertions(+), 9 deletions(-) create mode 100644 tests/cputestdata/x86-baseline-6-migratable.xml create mode 100644 tests/cputestdata/x86-baseline-6-result.xml create mode 100644 tests/cputestdata/x86-baseline-6.xml -- 2.0.5
Allowing their use with x86Data* helpers for easier filtering. --- src/cpu/cpu_x86.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 45be262..f6e8eec 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -90,6 +90,7 @@ struct x86_map { struct x86_feature *features; struct x86_model *models; struct x86_feature *migrate_blockers; + virCPUx86Data *migrate_blocker_data; }; static struct x86_map* virCPUx86Map; @@ -689,6 +690,36 @@ x86ParseCPUID(xmlXPathContextPtr ctxt, } +static virCPUx86Data * +x86DataFromCPUFeatureList(const struct x86_feature *list, + const struct x86_map *map) +{ + virCPUx86Data *data; + const struct x86_feature *feat; + + if (VIR_ALLOC(data) < 0) + return NULL; + + for (feat = list; feat; feat = feat->next) { + const struct x86_feature *feature; + if (!(feature = x86FeatureFind(map, feat->name))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown CPU feature %s"), feat->name); + goto error; + } + + if (x86DataAdd(data, feature->data) < 0) + goto error; + } + + return data; + + error: + virCPUx86DataFree(data); + return NULL; +} + + static int x86FeatureLoad(xmlXPathContextPtr ctxt, struct x86_map *map) @@ -1139,6 +1170,7 @@ x86MapFree(struct x86_map *map) x86FeatureFree(migrate_blocker); } + virCPUx86DataFree(map->migrate_blocker_data); VIR_FREE(map); } @@ -1221,6 +1253,12 @@ virCPUx86LoadMap(void) if (x86MapLoadInternalFeatures(map) < 0) goto error; + map->migrate_blocker_data = x86DataFromCPUFeatureList(map->migrate_blockers, + map); + if (!map->migrate_blocker_data) + goto error; + + return map; error: -- 2.0.5
On Thu, Feb 05, 2015 at 15:47:52 +0100, Ján Tomko wrote:
Allowing their use with x86Data* helpers for easier filtering. --- src/cpu/cpu_x86.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 45be262..f6e8eec 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -90,6 +90,7 @@ struct x86_map { struct x86_feature *features; struct x86_model *models; struct x86_feature *migrate_blockers; + virCPUx86Data *migrate_blocker_data; };
static struct x86_map* virCPUx86Map; @@ -689,6 +690,36 @@ x86ParseCPUID(xmlXPathContextPtr ctxt, }
+static virCPUx86Data * +x86DataFromCPUFeatureList(const struct x86_feature *list, + const struct x86_map *map) +{ + virCPUx86Data *data; + const struct x86_feature *feat; + + if (VIR_ALLOC(data) < 0) + return NULL; + + for (feat = list; feat; feat = feat->next) { + const struct x86_feature *feature; + if (!(feature = x86FeatureFind(map, feat->name))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown CPU feature %s"), feat->name); + goto error; + }
This is just weired. We already have migrate_blockers list of x86_feature structs so having migrate_blocker_data here as well seems pretty redundant. Not to mention that we go through the migrate_blockers list and search for each of the feature in the features list to get the data? This doesn't sound right. Jirka
This flag for virConnectBaselineCPU will allow filtering out CPU features that block migration from the result. https://bugzilla.redhat.com/show_bug.cgi?id=1171484 --- include/libvirt/libvirt-host.h | 1 + src/libvirt-host.c | 3 +++ 2 files changed, 4 insertions(+) diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index f760a55..77058af 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -862,6 +862,7 @@ int virConnectGetCPUModelNames(virConnectPtr conn, */ typedef enum { VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES = (1 << 0), /* show all features */ + VIR_CONNECT_BASELINE_CPU_MIGRATABLE = (1 << 1), /* filter out non-migratable features */ } virConnectBaselineCPUFlags; char *virConnectBaselineCPU(virConnectPtr conn, diff --git a/src/libvirt-host.c b/src/libvirt-host.c index 78ee770..b4dc13e 100644 --- a/src/libvirt-host.c +++ b/src/libvirt-host.c @@ -1052,6 +1052,9 @@ virConnectGetCPUModelNames(virConnectPtr conn, const char *arch, char ***models, * without this flag features that are part of the CPU model will not be * listed. * + * If @flags includes VIR_CONNECT_BASELINE_CPU_MIGRATABLE, the resulting + * CPU will not include features that block migration. + * * Returns XML description of the computed CPU (caller frees) or NULL on error. */ char * -- 2.0.5
On Thu, Feb 05, 2015 at 15:47:53 +0100, Ján Tomko wrote:
This flag for virConnectBaselineCPU will allow filtering out CPU features that block migration from the result.
https://bugzilla.redhat.com/show_bug.cgi?id=1171484 --- include/libvirt/libvirt-host.h | 1 + src/libvirt-host.c | 3 +++ 2 files changed, 4 insertions(+)
diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index f760a55..77058af 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -862,6 +862,7 @@ int virConnectGetCPUModelNames(virConnectPtr conn, */ typedef enum { VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES = (1 << 0), /* show all features */ + VIR_CONNECT_BASELINE_CPU_MIGRATABLE = (1 << 1), /* filter out non-migratable features */ } virConnectBaselineCPUFlags;
ACK Jirka
Subtract the migrate blocker data from the result if this flag was specified. --- src/bhyve/bhyve_driver.c | 3 +- src/cpu/cpu_x86.c | 9 +++++- src/qemu/qemu_driver.c | 3 +- tests/cputest.c | 6 ++++ tests/cputestdata/x86-baseline-6-migratable.xml | 10 +++++++ tests/cputestdata/x86-baseline-6-result.xml | 11 ++++++++ tests/cputestdata/x86-baseline-6.xml | 37 +++++++++++++++++++++++++ 7 files changed, 76 insertions(+), 3 deletions(-) create mode 100644 tests/cputestdata/x86-baseline-6-migratable.xml create mode 100644 tests/cputestdata/x86-baseline-6-result.xml create mode 100644 tests/cputestdata/x86-baseline-6.xml diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 56cc8ab..2a1cd2f 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1340,7 +1340,8 @@ bhyveConnectBaselineCPU(virConnectPtr conn, { char *cpu = NULL; - virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | + VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); if (virConnectBaselineCPUEnsureACL(conn) < 0) goto cleanup; diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index f6e8eec..a95d15a 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1953,6 +1953,9 @@ x86Baseline(virCPUDefPtr *cpus, const char *modelName; bool matchingNames = true; + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | + VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); + if (!(map = virCPUx86GetMap())) goto error; @@ -2025,6 +2028,9 @@ x86Baseline(virCPUDefPtr *cpus, model = NULL; } + if (flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE) + x86DataSubtract(base_model->data, map->migrate_blocker_data); + if (x86DataIsEmpty(base_model->data)) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("CPUs are incompatible")); @@ -2034,7 +2040,8 @@ x86Baseline(virCPUDefPtr *cpus, if (vendor && virCPUx86DataAddCPUID(base_model->data, &vendor->cpuid) < 0) goto error; - if (x86Decode(cpu, base_model->data, models, nmodels, modelName, flags) < 0) + if (x86Decode(cpu, base_model->data, models, nmodels, modelName, + flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) < 0) goto error; if (STREQ_NULLABLE(cpu->model, modelName)) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cf351e6..8eded5e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12376,7 +12376,8 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, { char *cpu = NULL; - virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | + VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); if (virConnectBaselineCPUEnsureACL(conn) < 0) goto cleanup; diff --git a/tests/cputest.c b/tests/cputest.c index e49ae24..ab3efdf 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -346,6 +346,8 @@ cpuTestBaseline(const void *arg) if (data->flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) suffix = "expanded"; + else if (data->flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE) + suffix = "migratable"; else suffix = "result"; if (virAsprintf(&result, "%s-%s", data->name, suffix) < 0) @@ -533,6 +535,8 @@ mymain(void) char *label; \ if ((flags) & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) \ suffix = " (expanded)"; \ + if ((flags) & VIR_CONNECT_BASELINE_CPU_MIGRATABLE) \ + suffix = " (migratable)"; \ if (virAsprintf(&label, "%s%s", name, suffix) < 0) { \ ret = -1; \ } else { \ @@ -612,6 +616,8 @@ mymain(void) DO_TEST_BASELINE("x86", "4", VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, 0); DO_TEST_BASELINE("x86", "5", 0, 0); DO_TEST_BASELINE("x86", "5", VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, 0); + DO_TEST_BASELINE("x86", "6", 0, 0); + DO_TEST_BASELINE("x86", "6", VIR_CONNECT_BASELINE_CPU_MIGRATABLE, 0); DO_TEST_BASELINE("ppc64", "incompatible-vendors", 0, -1); DO_TEST_BASELINE("ppc64", "no-vendor", 0, 0); diff --git a/tests/cputestdata/x86-baseline-6-migratable.xml b/tests/cputestdata/x86-baseline-6-migratable.xml new file mode 100644 index 0000000..3c2f38c --- /dev/null +++ b/tests/cputestdata/x86-baseline-6-migratable.xml @@ -0,0 +1,10 @@ +<cpu mode='custom' match='exact'> + <model fallback='allow'>SandyBridge</model> + <vendor>Intel</vendor> + <feature policy='require' name='hypervisor'/> + <feature policy='require' name='osxsave'/> + <feature policy='require' name='pcid'/> + <feature policy='require' name='ss'/> + <feature policy='require' name='vme'/> + <feature policy='disable' name='rdtscp'/> +</cpu> diff --git a/tests/cputestdata/x86-baseline-6-result.xml b/tests/cputestdata/x86-baseline-6-result.xml new file mode 100644 index 0000000..bea0beb --- /dev/null +++ b/tests/cputestdata/x86-baseline-6-result.xml @@ -0,0 +1,11 @@ +<cpu mode='custom' match='exact'> + <model fallback='allow'>SandyBridge</model> + <vendor>Intel</vendor> + <feature policy='require' name='invtsc'/> + <feature policy='require' name='hypervisor'/> + <feature policy='require' name='osxsave'/> + <feature policy='require' name='pcid'/> + <feature policy='require' name='ss'/> + <feature policy='require' name='vme'/> + <feature policy='disable' name='rdtscp'/> +</cpu> diff --git a/tests/cputestdata/x86-baseline-6.xml b/tests/cputestdata/x86-baseline-6.xml new file mode 100644 index 0000000..9845b93 --- /dev/null +++ b/tests/cputestdata/x86-baseline-6.xml @@ -0,0 +1,37 @@ +<cpuTest> +<cpu> + <arch>x86_64</arch> + <model>Westmere</model> + <vendor>Intel</vendor> + <topology sockets='4' cores='1' threads='1'/> + <feature name='hypervisor'/> + <feature name='avx'/> + <feature name='osxsave'/> + <feature name='xsave'/> + <feature name='tsc-deadline'/> + <feature name='x2apic'/> + <feature name='pcid'/> + <feature name='pclmuldq'/> + <feature name='ss'/> + <feature name='vme'/> + <feature name='invtsc'/> +</cpu> +<cpu> + <arch>x86_64</arch> + <model>Nehalem</model> + <vendor>Intel</vendor> + <topology sockets='4' cores='1' threads='1'/> + <feature name='aes'/> + <feature name='hypervisor'/> + <feature name='avx'/> + <feature name='osxsave'/> + <feature name='xsave'/> + <feature name='tsc-deadline'/> + <feature name='x2apic'/> + <feature name='pcid'/> + <feature name='pclmuldq'/> + <feature name='ss'/> + <feature name='vme'/> + <feature name='invtsc'/> +</cpu> +</cpuTest> -- 2.0.5
Filter out non-migratable features if VIR_CONNECT_BASELINE_CPU_MIGRATABLE was specified. --- v2: use the existing migrate_blocker field This removes the need for patch 1/5. src/bhyve/bhyve_driver.c | 3 +- src/cpu/cpu_x86.c | 25 +++++++++++++++-- src/qemu/qemu_driver.c | 3 +- tests/cputest.c | 6 ++++ tests/cputestdata/x86-baseline-6-migratable.xml | 10 +++++++ tests/cputestdata/x86-baseline-6-result.xml | 11 ++++++++ tests/cputestdata/x86-baseline-6.xml | 37 +++++++++++++++++++++++++ 7 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 tests/cputestdata/x86-baseline-6-migratable.xml create mode 100644 tests/cputestdata/x86-baseline-6-result.xml create mode 100644 tests/cputestdata/x86-baseline-6.xml diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index ae39917..2817f8e 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1340,7 +1340,8 @@ bhyveConnectBaselineCPU(virConnectPtr conn, { char *cpu = NULL; - virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | + VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); if (virConnectBaselineCPUEnsureACL(conn) < 0) goto cleanup; diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 45be262..2a568aa 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1556,7 +1556,8 @@ x86Decode(virCPUDefPtr cpu, const virCPUx86Data *cpuData = NULL; size_t i; - virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | + VIR_CONNECT_BASELINE_CPU_MIGRATABLE, -1); if (!data || !(map = virCPUx86GetMap())) return -1; @@ -1633,6 +1634,21 @@ x86Decode(virCPUDefPtr cpu, goto out; } + /* Remove non-migratable features by default + * Note: this only works as long as no CPU model contains non-migratable + * features directly */ + if (flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE) { + for (i = 0; i < cpuModel->nfeatures; i++) { + const struct x86_feature *feat; + for (feat = map->migrate_blockers; feat; feat = feat->next) { + if (STREQ(feat->name, cpuModel->features[i].name)) { + VIR_FREE(cpuModel->features[i].name); + VIR_DELETE_ELEMENT_INPLACE(cpuModel->features, i, cpuModel->nfeatures); + } + } + } + } + if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) { if (!(copy = x86DataCopy(cpuData)) || !(features = x86DataFromCPUFeatures(cpuModel, map))) @@ -1915,6 +1931,9 @@ x86Baseline(virCPUDefPtr *cpus, const char *modelName; bool matchingNames = true; + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | + VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); + if (!(map = virCPUx86GetMap())) goto error; @@ -1996,7 +2015,9 @@ x86Baseline(virCPUDefPtr *cpus, if (vendor && virCPUx86DataAddCPUID(base_model->data, &vendor->cpuid) < 0) goto error; - if (x86Decode(cpu, base_model->data, models, nmodels, modelName, flags) < 0) + if (x86Decode(cpu, base_model->data, models, nmodels, modelName, + flags & (VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | + VIR_CONNECT_BASELINE_CPU_MIGRATABLE)) < 0) goto error; if (STREQ_NULLABLE(cpu->model, modelName)) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8f0cf2b..8d765d9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12416,7 +12416,8 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, { char *cpu = NULL; - virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | + VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); if (virConnectBaselineCPUEnsureACL(conn) < 0) goto cleanup; diff --git a/tests/cputest.c b/tests/cputest.c index e49ae24..ab3efdf 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -346,6 +346,8 @@ cpuTestBaseline(const void *arg) if (data->flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) suffix = "expanded"; + else if (data->flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE) + suffix = "migratable"; else suffix = "result"; if (virAsprintf(&result, "%s-%s", data->name, suffix) < 0) @@ -533,6 +535,8 @@ mymain(void) char *label; \ if ((flags) & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) \ suffix = " (expanded)"; \ + if ((flags) & VIR_CONNECT_BASELINE_CPU_MIGRATABLE) \ + suffix = " (migratable)"; \ if (virAsprintf(&label, "%s%s", name, suffix) < 0) { \ ret = -1; \ } else { \ @@ -612,6 +616,8 @@ mymain(void) DO_TEST_BASELINE("x86", "4", VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, 0); DO_TEST_BASELINE("x86", "5", 0, 0); DO_TEST_BASELINE("x86", "5", VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, 0); + DO_TEST_BASELINE("x86", "6", 0, 0); + DO_TEST_BASELINE("x86", "6", VIR_CONNECT_BASELINE_CPU_MIGRATABLE, 0); DO_TEST_BASELINE("ppc64", "incompatible-vendors", 0, -1); DO_TEST_BASELINE("ppc64", "no-vendor", 0, 0); diff --git a/tests/cputestdata/x86-baseline-6-migratable.xml b/tests/cputestdata/x86-baseline-6-migratable.xml new file mode 100644 index 0000000..3c2f38c --- /dev/null +++ b/tests/cputestdata/x86-baseline-6-migratable.xml @@ -0,0 +1,10 @@ +<cpu mode='custom' match='exact'> + <model fallback='allow'>SandyBridge</model> + <vendor>Intel</vendor> + <feature policy='require' name='hypervisor'/> + <feature policy='require' name='osxsave'/> + <feature policy='require' name='pcid'/> + <feature policy='require' name='ss'/> + <feature policy='require' name='vme'/> + <feature policy='disable' name='rdtscp'/> +</cpu> diff --git a/tests/cputestdata/x86-baseline-6-result.xml b/tests/cputestdata/x86-baseline-6-result.xml new file mode 100644 index 0000000..bea0beb --- /dev/null +++ b/tests/cputestdata/x86-baseline-6-result.xml @@ -0,0 +1,11 @@ +<cpu mode='custom' match='exact'> + <model fallback='allow'>SandyBridge</model> + <vendor>Intel</vendor> + <feature policy='require' name='invtsc'/> + <feature policy='require' name='hypervisor'/> + <feature policy='require' name='osxsave'/> + <feature policy='require' name='pcid'/> + <feature policy='require' name='ss'/> + <feature policy='require' name='vme'/> + <feature policy='disable' name='rdtscp'/> +</cpu> diff --git a/tests/cputestdata/x86-baseline-6.xml b/tests/cputestdata/x86-baseline-6.xml new file mode 100644 index 0000000..9845b93 --- /dev/null +++ b/tests/cputestdata/x86-baseline-6.xml @@ -0,0 +1,37 @@ +<cpuTest> +<cpu> + <arch>x86_64</arch> + <model>Westmere</model> + <vendor>Intel</vendor> + <topology sockets='4' cores='1' threads='1'/> + <feature name='hypervisor'/> + <feature name='avx'/> + <feature name='osxsave'/> + <feature name='xsave'/> + <feature name='tsc-deadline'/> + <feature name='x2apic'/> + <feature name='pcid'/> + <feature name='pclmuldq'/> + <feature name='ss'/> + <feature name='vme'/> + <feature name='invtsc'/> +</cpu> +<cpu> + <arch>x86_64</arch> + <model>Nehalem</model> + <vendor>Intel</vendor> + <topology sockets='4' cores='1' threads='1'/> + <feature name='aes'/> + <feature name='hypervisor'/> + <feature name='avx'/> + <feature name='osxsave'/> + <feature name='xsave'/> + <feature name='tsc-deadline'/> + <feature name='x2apic'/> + <feature name='pcid'/> + <feature name='pclmuldq'/> + <feature name='ss'/> + <feature name='vme'/> + <feature name='invtsc'/> +</cpu> +</cpuTest> -- 2.0.5
Oops, I started looking at this patch the moment you sent it but got distracted and never finished the review. Sorry about this. On Thu, Feb 19, 2015 at 16:22:38 +0100, Ján Tomko wrote:
Filter out non-migratable features if VIR_CONNECT_BASELINE_CPU_MIGRATABLE was specified. --- v2: use the existing migrate_blocker field This removes the need for patch 1/5. ... diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 45be262..2a568aa 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c ... @@ -1633,6 +1634,21 @@ x86Decode(virCPUDefPtr cpu, goto out; }
+ /* Remove non-migratable features by default
Well, the features are removed on request rather than by default.
+ * Note: this only works as long as no CPU model contains non-migratable + * features directly */ + if (flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE) { + for (i = 0; i < cpuModel->nfeatures; i++) { + const struct x86_feature *feat; + for (feat = map->migrate_blockers; feat; feat = feat->next) { + if (STREQ(feat->name, cpuModel->features[i].name)) { + VIR_FREE(cpuModel->features[i].name); + VIR_DELETE_ELEMENT_INPLACE(cpuModel->features, i, cpuModel->nfeatures); + } + } + } + } + if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) { if (!(copy = x86DataCopy(cpuData)) || !(features = x86DataFromCPUFeatures(cpuModel, map))) @@ -1915,6 +1931,9 @@ x86Baseline(virCPUDefPtr *cpus, const char *modelName; bool matchingNames = true;
+ virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | + VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); + if (!(map = virCPUx86GetMap())) goto error;
@@ -1996,7 +2015,9 @@ x86Baseline(virCPUDefPtr *cpus, if (vendor && virCPUx86DataAddCPUID(base_model->data, &vendor->cpuid) < 0) goto error;
- if (x86Decode(cpu, base_model->data, models, nmodels, modelName, flags) < 0) + if (x86Decode(cpu, base_model->data, models, nmodels, modelName, + flags & (VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | + VIR_CONNECT_BASELINE_CPU_MIGRATABLE)) < 0)
No need to filter the features as x86Baseline does not support any additional feature. Looks like a leftover from v1.
goto error;
if (STREQ_NULLABLE(cpu->model, modelName))
ACK with the nits fixed. Jirka
Wire up VIR_CONNECT_BASELINE_CPU_MIGRATABLE to this command line option. --- tools/virsh-domain.c | 6 ++++++ tools/virsh.pod | 5 +++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index bab44fe..2b4f06d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6860,6 +6860,10 @@ static const vshCmdOptDef opts_cpu_baseline[] = { .type = VSH_OT_BOOL, .help = N_("Show features that are part of the CPU model type") }, + {.name = "migratable", + .type = VSH_OT_BOOL, + .help = N_("Do not include features that block migration") + }, {.name = NULL} }; @@ -6882,6 +6886,8 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "features")) flags |= VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES; + if (vshCommandOptBool(cmd, "migratable")) + flags |= VIR_CONNECT_BASELINE_CPU_MIGRATABLE; if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) return false; diff --git a/tools/virsh.pod b/tools/virsh.pod index e367e04..3881614 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -573,7 +573,7 @@ I<cellno> modifier can be used to narrow the modification down to a single host NUMA cell. On the other end of spectrum lies I<--all> which executes the modification on all NUMA cells. -=item B<cpu-baseline> I<FILE> [I<--features>] +=item B<cpu-baseline> I<FILE> [I<--features>] [I<--migratable>] 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 @@ -582,7 +582,8 @@ by new lines or even a set of complete <capabilities> elements printed by 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. +will not be listed in the XML description. If I<--migratable> is specified, +features that block migration will not be included in the resulting CPU. =item B<cpu-compare> I<FILE> [I<--error>] -- 2.0.5
On Thu, Feb 05, 2015 at 15:47:56 +0100, Ján Tomko wrote:
Wire up VIR_CONNECT_BASELINE_CPU_MIGRATABLE to this command line option. --- tools/virsh-domain.c | 6 ++++++ tools/virsh.pod | 5 +++-- 2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index bab44fe..2b4f06d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6860,6 +6860,10 @@ static const vshCmdOptDef opts_cpu_baseline[] = { .type = VSH_OT_BOOL, .help = N_("Show features that are part of the CPU model type") }, + {.name = "migratable", + .type = VSH_OT_BOOL, + .help = N_("Do not include features that block migration") + }, {.name = NULL} };
@@ -6882,6 +6886,8 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, "features")) flags |= VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES; + if (vshCommandOptBool(cmd, "migratable")) + flags |= VIR_CONNECT_BASELINE_CPU_MIGRATABLE;
if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) return false; diff --git a/tools/virsh.pod b/tools/virsh.pod index e367e04..3881614 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -573,7 +573,7 @@ I<cellno> modifier can be used to narrow the modification down to a single host NUMA cell. On the other end of spectrum lies I<--all> which executes the modification on all NUMA cells.
-=item B<cpu-baseline> I<FILE> [I<--features>] +=item B<cpu-baseline> I<FILE> [I<--features>] [I<--migratable>]
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 @@ -582,7 +582,8 @@ by new lines or even a set of complete <capabilities> elements printed by 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. +will not be listed in the XML description. If I<--migratable> is specified, +features that block migration will not be included in the resulting CPU.
=item B<cpu-compare> I<FILE> [I<--error>]
I understand two spaces after a dot which ends a sentence, but three spaces seem to be more than enough :-) ACK Jirka
Assume no features block migration. --- src/cpu/cpu_aarch64.c | 3 ++- src/cpu/cpu_arm.c | 3 ++- src/cpu/cpu_generic.c | 3 ++- src/cpu/cpu_powerpc.c | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/cpu/cpu_aarch64.c b/src/cpu/cpu_aarch64.c index 6346f9b..e6d5f53 100644 --- a/src/cpu/cpu_aarch64.c +++ b/src/cpu/cpu_aarch64.c @@ -94,7 +94,8 @@ AArch64Baseline(virCPUDefPtr *cpus, { virCPUDefPtr cpu = NULL; - virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | + VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); if (VIR_ALLOC(cpu) < 0 || VIR_STRDUP(cpu->model, cpus[0]->model) < 0) { diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index ec755bd..0403a8b 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -97,7 +97,8 @@ ArmBaseline(virCPUDefPtr *cpus, { virCPUDefPtr cpu = NULL; - virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | + VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); if (VIR_ALLOC(cpu) < 0 || VIR_STRDUP(cpu->model, cpus[0]->model) < 0) { diff --git a/src/cpu/cpu_generic.c b/src/cpu/cpu_generic.c index d6890c0..a9cde4c 100644 --- a/src/cpu/cpu_generic.c +++ b/src/cpu/cpu_generic.c @@ -126,7 +126,8 @@ genericBaseline(virCPUDefPtr *cpus, unsigned int count; size_t i, j; - virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | + VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); if (!cpuModelIsAllowed(cpus[0]->model, models, nmodels)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c index 1cd6874..c77374c 100644 --- a/src/cpu/cpu_powerpc.c +++ b/src/cpu/cpu_powerpc.c @@ -580,7 +580,8 @@ ppcBaseline(virCPUDefPtr *cpus, virCPUDefPtr cpu = NULL; size_t i; - virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | + VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); if (!(map = ppcLoadMap())) goto error; -- 2.0.5
On Fri, Feb 06, 2015 at 13:00:00 +0100, Ján Tomko wrote:
Assume no features block migration. --- src/cpu/cpu_aarch64.c | 3 ++- src/cpu/cpu_arm.c | 3 ++- src/cpu/cpu_generic.c | 3 ++- src/cpu/cpu_powerpc.c | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-)
ACK Jirka
participants (2)
-
Jiri Denemark -
Ján Tomko