[libvirt] [PATCH 0/4] Fix virConnectBaselineCPU with expand-features flag

Jiri Denemark (4): tests: Better support for VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES cpu: Fix VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES cpu: Try to use source CPU model in virConnectBaselineCPU tests: Add more tests for virConnectBaselineCPU src/cpu/cpu_x86.c | 101 +++++++++++++++++--------- tests/cputest.c | 27 ++++++- tests/cputestdata/x86-baseline-3-expanded.xml | 35 +++++++++ tests/cputestdata/x86-baseline-3-result.xml | 34 +-------- tests/cputestdata/x86-baseline-4-expanded.xml | 46 ++++++++++++ tests/cputestdata/x86-baseline-4-result.xml | 14 ++++ tests/cputestdata/x86-baseline-4.xml | 18 +++++ tests/cputestdata/x86-baseline-5-expanded.xml | 47 ++++++++++++ tests/cputestdata/x86-baseline-5-result.xml | 10 +++ tests/cputestdata/x86-baseline-5.xml | 35 +++++++++ 10 files changed, 296 insertions(+), 71 deletions(-) create mode 100644 tests/cputestdata/x86-baseline-3-expanded.xml create mode 100644 tests/cputestdata/x86-baseline-4-expanded.xml create mode 100644 tests/cputestdata/x86-baseline-4-result.xml create mode 100644 tests/cputestdata/x86-baseline-4.xml create mode 100644 tests/cputestdata/x86-baseline-5-expanded.xml create mode 100644 tests/cputestdata/x86-baseline-5-result.xml create mode 100644 tests/cputestdata/x86-baseline-5.xml -- 1.8.5.3

https://bugzilla.redhat.com/show_bug.cgi?id=1049391 virConnectBaselineCPU test results are now stored in different files depending on VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tests/cputest.c | 22 +++++++++++++++++++--- ...ne-3-result.xml => x86-baseline-3-expanded.xml} | 0 2 files changed, 19 insertions(+), 3 deletions(-) rename tests/cputestdata/{x86-baseline-3-result.xml => x86-baseline-3-expanded.xml} (100%) diff --git a/tests/cputest.c b/tests/cputest.c index 20bc544..247fd10 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -326,6 +326,7 @@ cpuTestBaseline(const void *arg) virCPUDefPtr baseline = NULL; unsigned int ncpus = 0; char *result = NULL; + const char *suffix; size_t i; if (!(cpus = cpuTestLoadMultiXML(data->arch, data->name, &ncpus))) @@ -345,7 +346,11 @@ cpuTestBaseline(const void *arg) if (!baseline) goto cleanup; - if (virAsprintf(&result, "%s-result", data->name) < 0) + if (data->flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) + suffix = "expanded"; + else + suffix = "result"; + if (virAsprintf(&result, "%s-%s", data->name, suffix) < 0) goto cleanup; if (cpuTestCompareXML(data->arch, baseline, result, 0) < 0) @@ -537,8 +542,19 @@ mymain(void) } while (0) #define DO_TEST_BASELINE(arch, name, flags, result) \ - DO_TEST(arch, API_BASELINE, name, NULL, "baseline-" name, \ - NULL, 0, NULL, flags, result) + do { \ + const char *suffix = ""; \ + char *label; \ + if ((flags) & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) \ + suffix = " (expanded)"; \ + if (virAsprintf(&label, "%s%s", name, suffix) < 0) { \ + ret = -1; \ + } else { \ + DO_TEST(arch, API_BASELINE, label, NULL, "baseline-" name, \ + NULL, 0, NULL, flags, result); \ + } \ + VIR_FREE(label); \ + } while (0) #define DO_TEST_HASFEATURE(arch, host, feature, result) \ DO_TEST(arch, API_HAS_FEATURE, \ diff --git a/tests/cputestdata/x86-baseline-3-result.xml b/tests/cputestdata/x86-baseline-3-expanded.xml similarity index 100% rename from tests/cputestdata/x86-baseline-3-result.xml rename to tests/cputestdata/x86-baseline-3-expanded.xml -- 1.8.5.3

On 01/28/14 00:23, Jiri Denemark wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1049391
virConnectBaselineCPU test results are now stored in different files depending on VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tests/cputest.c | 22 +++++++++++++++++++--- ...ne-3-result.xml => x86-baseline-3-expanded.xml} | 0 2 files changed, 19 insertions(+), 3 deletions(-) rename tests/cputestdata/{x86-baseline-3-result.xml => x86-baseline-3-expanded.xml} (100%)
diff --git a/tests/cputest.c b/tests/cputest.c index 20bc544..247fd10 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -326,6 +326,7 @@ cpuTestBaseline(const void *arg) virCPUDefPtr baseline = NULL; unsigned int ncpus = 0; char *result = NULL; + const char *suffix; size_t i;
if (!(cpus = cpuTestLoadMultiXML(data->arch, data->name, &ncpus))) @@ -345,7 +346,11 @@ cpuTestBaseline(const void *arg) if (!baseline) goto cleanup;
- if (virAsprintf(&result, "%s-result", data->name) < 0) + if (data->flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) + suffix = "expanded"; + else + suffix = "result"; + if (virAsprintf(&result, "%s-%s", data->name, suffix) < 0) goto cleanup;
if (cpuTestCompareXML(data->arch, baseline, result, 0) < 0) @@ -537,8 +542,19 @@ mymain(void) } while (0)
#define DO_TEST_BASELINE(arch, name, flags, result) \ - DO_TEST(arch, API_BASELINE, name, NULL, "baseline-" name, \ - NULL, 0, NULL, flags, result) + do { \ + const char *suffix = ""; \ + char *label; \ + if ((flags) & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) \ + suffix = " (expanded)"; \ + if (virAsprintf(&label, "%s%s", name, suffix) < 0) { \ + ret = -1; \
This will make the one single test that fails allocation here fail silently - without even printing the test header - but I don't think this is worth working around in the test suite. Mainly because OOM is really unlikely.
+ } else { \ + DO_TEST(arch, API_BASELINE, label, NULL, "baseline-" name, \ + NULL, 0, NULL, flags, result); \ + } \ + VIR_FREE(label); \ + } while (0)
#define DO_TEST_HASFEATURE(arch, host, feature, result) \ DO_TEST(arch, API_HAS_FEATURE, \ diff --git a/tests/cputestdata/x86-baseline-3-result.xml b/tests/cputestdata/x86-baseline-3-expanded.xml similarity index 100% rename from tests/cputestdata/x86-baseline-3-result.xml rename to tests/cputestdata/x86-baseline-3-expanded.xml
ACK.

https://bugzilla.redhat.com/show_bug.cgi?id=1049391 VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES flag for virConnectBaselineCPU did not work if the resulting guest CPU would disable some features present in its base model. This patch makes sure we won't try to add such features twice. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 84 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 50 insertions(+), 34 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index ce75562..5d101a4 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -730,6 +730,36 @@ ignore: } +static virCPUx86Data * +x86DataFromCPUFeatures(virCPUDefPtr cpu, + const struct x86_map *map) +{ + virCPUx86Data *data; + size_t i; + + if (VIR_ALLOC(data) < 0) + return NULL; + + for (i = 0; i < cpu->nfeatures; i++) { + const struct x86_feature *feature; + if (!(feature = x86FeatureFind(map, cpu->features[i].name))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown CPU feature %s"), cpu->features[i].name); + goto error; + } + + if (x86DataAdd(data, feature->data) < 0) + goto error; + } + + return data; + +error: + virCPUx86DataFree(data); + return NULL; +} + + static struct x86_model * x86ModelNew(void) { @@ -1448,35 +1478,6 @@ x86GuestData(virCPUDefPtr host, static int -x86AddFeatures(virCPUDefPtr cpu, - const 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 virCPUx86Data *data, const char **models, @@ -1489,6 +1490,9 @@ x86Decode(virCPUDefPtr cpu, const struct x86_model *candidate; virCPUDefPtr cpuCandidate; virCPUDefPtr cpuModel = NULL; + virCPUx86Data *copy = NULL; + virCPUx86Data *features = NULL; + const virCPUx86Data *cpuData = NULL; size_t i; virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); @@ -1545,6 +1549,7 @@ x86Decode(virCPUDefPtr cpu, if (preferred && STREQ(cpuCandidate->model, preferred)) { virCPUDefFree(cpuModel); cpuModel = cpuCandidate; + cpuData = candidate->data; break; } @@ -1552,8 +1557,10 @@ x86Decode(virCPUDefPtr cpu, || cpuModel->nfeatures > cpuCandidate->nfeatures) { virCPUDefFree(cpuModel); cpuModel = cpuCandidate; - } else + cpuData = candidate->data; + } else { virCPUDefFree(cpuCandidate); + } next: candidate = candidate->next; @@ -1565,9 +1572,17 @@ x86Decode(virCPUDefPtr cpu, goto out; } - if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES && - x86AddFeatures(cpuModel, map) < 0) - goto out; + if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) { + if (!(copy = x86DataCopy(cpuData)) || + !(features = x86DataFromCPUFeatures(cpuModel, map))) + goto out; + + x86DataSubtract(copy, features); + if (x86DataToCPUFeatures(cpuModel, VIR_CPU_FEATURE_REQUIRE, + copy, map) < 0) + goto out; + } + cpu->model = cpuModel->model; cpu->vendor = cpuModel->vendor; cpu->nfeatures = cpuModel->nfeatures; @@ -1578,7 +1593,8 @@ x86Decode(virCPUDefPtr cpu, out: virCPUDefFree(cpuModel); - + virCPUx86DataFree(copy); + virCPUx86DataFree(features); return ret; } -- 1.8.5.3

On 01/28/14 00:23, Jiri Denemark wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1049391
VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES flag for virConnectBaselineCPU did not work if the resulting guest CPU would disable some features present in its base model. This patch makes sure we won't try to add such features twice.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 84 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 50 insertions(+), 34 deletions(-)
ACK.

https://bugzilla.redhat.com/show_bug.cgi?id=1049391 When all source CPU XMLs contain just a single CPU model (with a possibly varying set of additional feature elements), virConnectBaselineCPU will try to use this CPU model in the computed guest CPU. Thus, when used on just a single CPU (useful with VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES), the result will not use a different CPU model. If the computed CPU uses the source model, set fallback mode to 'forbid' to make sure the guest CPU will always be as close as possible to the source CPUs. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 17 ++++++++++++++++- tests/cputestdata/x86-baseline-3-expanded.xml | 2 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 5d101a4..56080ef 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1851,6 +1851,8 @@ x86Baseline(virCPUDefPtr *cpus, const struct x86_vendor *vendor = NULL; struct x86_model *model = NULL; bool outputVendor = true; + const char *modelName; + bool matchingNames = true; if (!(map = virCPUx86GetMap())) goto error; @@ -1873,9 +1875,19 @@ x86Baseline(virCPUDefPtr *cpus, goto error; } + modelName = cpus[0]->model; for (i = 1; i < ncpus; i++) { const char *vn = NULL; + if (matchingNames && cpus[i]->model) { + if (!modelName) { + modelName = cpus[i]->model; + } else if (STRNEQ(modelName, cpus[i]->model)) { + modelName = NULL; + matchingNames = false; + } + } + if (!(model = x86ModelFromCPU(cpus[i], map, VIR_CPU_FEATURE_REQUIRE))) goto error; @@ -1923,9 +1935,12 @@ x86Baseline(virCPUDefPtr *cpus, if (vendor && virCPUx86DataAddCPUID(base_model->data, &vendor->cpuid) < 0) goto error; - if (x86Decode(cpu, base_model->data, models, nmodels, NULL, flags) < 0) + if (x86Decode(cpu, base_model->data, models, nmodels, modelName, flags) < 0) goto error; + if (STREQ_NULLABLE(cpu->model, modelName)) + cpu->fallback = VIR_CPU_FALLBACK_FORBID; + if (!outputVendor) VIR_FREE(cpu->vendor); diff --git a/tests/cputestdata/x86-baseline-3-expanded.xml b/tests/cputestdata/x86-baseline-3-expanded.xml index d196112..a7e57be 100644 --- a/tests/cputestdata/x86-baseline-3-expanded.xml +++ b/tests/cputestdata/x86-baseline-3-expanded.xml @@ -1,5 +1,5 @@ <cpu mode='custom' match='exact'> - <model fallback='allow'>Westmere</model> + <model fallback='forbid'>Westmere</model> <feature policy='require' name='lahf_lm'/> <feature policy='require' name='lm'/> <feature policy='require' name='nx'/> -- 1.8.5.3

On 01/28/14 00:23, Jiri Denemark wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1049391
When all source CPU XMLs contain just a single CPU model (with a possibly varying set of additional feature elements), virConnectBaselineCPU will try to use this CPU model in the computed guest CPU. Thus, when used on just a single CPU (useful with VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES), the result will not use a different CPU model.
If the computed CPU uses the source model, set fallback mode to 'forbid' to make sure the guest CPU will always be as close as possible to the source CPUs.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 17 ++++++++++++++++- tests/cputestdata/x86-baseline-3-expanded.xml | 2 +- 2 files changed, 17 insertions(+), 2 deletions(-)
ACK.

https://bugzilla.redhat.com/show_bug.cgi?id=1049391 The new tests would fail in various ways without the two previous commits. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tests/cputest.c | 5 +++ tests/cputestdata/x86-baseline-3-result.xml | 3 ++ tests/cputestdata/x86-baseline-4-expanded.xml | 46 ++++++++++++++++++++++++++ tests/cputestdata/x86-baseline-4-result.xml | 14 ++++++++ tests/cputestdata/x86-baseline-4.xml | 18 ++++++++++ tests/cputestdata/x86-baseline-5-expanded.xml | 47 +++++++++++++++++++++++++++ tests/cputestdata/x86-baseline-5-result.xml | 10 ++++++ tests/cputestdata/x86-baseline-5.xml | 35 ++++++++++++++++++++ 8 files changed, 178 insertions(+) create mode 100644 tests/cputestdata/x86-baseline-3-result.xml create mode 100644 tests/cputestdata/x86-baseline-4-expanded.xml create mode 100644 tests/cputestdata/x86-baseline-4-result.xml create mode 100644 tests/cputestdata/x86-baseline-4.xml create mode 100644 tests/cputestdata/x86-baseline-5-expanded.xml create mode 100644 tests/cputestdata/x86-baseline-5-result.xml create mode 100644 tests/cputestdata/x86-baseline-5.xml diff --git a/tests/cputest.c b/tests/cputest.c index 247fd10..c03538a 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -619,7 +619,12 @@ mymain(void) 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", 0, 0); DO_TEST_BASELINE("x86", "3", VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, 0); + DO_TEST_BASELINE("x86", "4", 0, 0); + 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("ppc64", "incompatible-vendors", 0, -1); DO_TEST_BASELINE("ppc64", "no-vendor", 0, 0); diff --git a/tests/cputestdata/x86-baseline-3-result.xml b/tests/cputestdata/x86-baseline-3-result.xml new file mode 100644 index 0000000..7349831 --- /dev/null +++ b/tests/cputestdata/x86-baseline-3-result.xml @@ -0,0 +1,3 @@ +<cpu mode='custom' match='exact'> + <model fallback='forbid'>Westmere</model> +</cpu> diff --git a/tests/cputestdata/x86-baseline-4-expanded.xml b/tests/cputestdata/x86-baseline-4-expanded.xml new file mode 100644 index 0000000..b5671b5 --- /dev/null +++ b/tests/cputestdata/x86-baseline-4-expanded.xml @@ -0,0 +1,46 @@ +<cpu mode='custom' match='exact'> + <model fallback='forbid'>Westmere</model> + <vendor>Intel</vendor> + <feature policy='require' name='hypervisor'/> + <feature policy='require' name='avx'/> + <feature policy='require' name='osxsave'/> + <feature policy='require' name='xsave'/> + <feature policy='require' name='tsc-deadline'/> + <feature policy='require' name='x2apic'/> + <feature policy='require' name='pcid'/> + <feature policy='require' name='pclmuldq'/> + <feature policy='require' name='ss'/> + <feature policy='require' name='vme'/> + <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-4-result.xml b/tests/cputestdata/x86-baseline-4-result.xml new file mode 100644 index 0000000..44fbc38 --- /dev/null +++ b/tests/cputestdata/x86-baseline-4-result.xml @@ -0,0 +1,14 @@ +<cpu mode='custom' match='exact'> + <model fallback='forbid'>Westmere</model> + <vendor>Intel</vendor> + <feature policy='require' name='hypervisor'/> + <feature policy='require' name='avx'/> + <feature policy='require' name='osxsave'/> + <feature policy='require' name='xsave'/> + <feature policy='require' name='tsc-deadline'/> + <feature policy='require' name='x2apic'/> + <feature policy='require' name='pcid'/> + <feature policy='require' name='pclmuldq'/> + <feature policy='require' name='ss'/> + <feature policy='require' name='vme'/> +</cpu> diff --git a/tests/cputestdata/x86-baseline-4.xml b/tests/cputestdata/x86-baseline-4.xml new file mode 100644 index 0000000..7f5ae16 --- /dev/null +++ b/tests/cputestdata/x86-baseline-4.xml @@ -0,0 +1,18 @@ +<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'/> +</cpu> +</cpuTest> diff --git a/tests/cputestdata/x86-baseline-5-expanded.xml b/tests/cputestdata/x86-baseline-5-expanded.xml new file mode 100644 index 0000000..2408704 --- /dev/null +++ b/tests/cputestdata/x86-baseline-5-expanded.xml @@ -0,0 +1,47 @@ +<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'/> + <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='avx'/> + <feature policy='require' name='xsave'/> + <feature policy='require' name='aes'/> + <feature policy='require' name='tsc-deadline'/> + <feature policy='require' name='popcnt'/> + <feature policy='require' name='x2apic'/> + <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='pclmuldq'/> + <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-5-result.xml b/tests/cputestdata/x86-baseline-5-result.xml new file mode 100644 index 0000000..3c2f38c --- /dev/null +++ b/tests/cputestdata/x86-baseline-5-result.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-5.xml b/tests/cputestdata/x86-baseline-5.xml new file mode 100644 index 0000000..80cd533 --- /dev/null +++ b/tests/cputestdata/x86-baseline-5.xml @@ -0,0 +1,35 @@ +<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'/> +</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'/> +</cpu> +</cpuTest> -- 1.8.5.3

On 01/28/14 00:23, Jiri Denemark wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1049391
The new tests would fail in various ways without the two previous commits.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tests/cputest.c | 5 +++ tests/cputestdata/x86-baseline-3-result.xml | 3 ++ tests/cputestdata/x86-baseline-4-expanded.xml | 46 ++++++++++++++++++++++++++ tests/cputestdata/x86-baseline-4-result.xml | 14 ++++++++ tests/cputestdata/x86-baseline-4.xml | 18 ++++++++++ tests/cputestdata/x86-baseline-5-expanded.xml | 47 +++++++++++++++++++++++++++ tests/cputestdata/x86-baseline-5-result.xml | 10 ++++++ tests/cputestdata/x86-baseline-5.xml | 35 ++++++++++++++++++++ 8 files changed, 178 insertions(+) create mode 100644 tests/cputestdata/x86-baseline-3-result.xml create mode 100644 tests/cputestdata/x86-baseline-4-expanded.xml create mode 100644 tests/cputestdata/x86-baseline-4-result.xml create mode 100644 tests/cputestdata/x86-baseline-4.xml create mode 100644 tests/cputestdata/x86-baseline-5-expanded.xml create mode 100644 tests/cputestdata/x86-baseline-5-result.xml create mode 100644 tests/cputestdata/x86-baseline-5.xml
ACK

On Tue, Jan 28, 2014 at 15:32:37 +0100, Peter Krempa wrote:
On 01/28/14 00:23, Jiri Denemark wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1049391
The new tests would fail in various ways without the two previous commits.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tests/cputest.c | 5 +++ tests/cputestdata/x86-baseline-3-result.xml | 3 ++ tests/cputestdata/x86-baseline-4-expanded.xml | 46 ++++++++++++++++++++++++++ tests/cputestdata/x86-baseline-4-result.xml | 14 ++++++++ tests/cputestdata/x86-baseline-4.xml | 18 ++++++++++ tests/cputestdata/x86-baseline-5-expanded.xml | 47 +++++++++++++++++++++++++++ tests/cputestdata/x86-baseline-5-result.xml | 10 ++++++ tests/cputestdata/x86-baseline-5.xml | 35 ++++++++++++++++++++ 8 files changed, 178 insertions(+) create mode 100644 tests/cputestdata/x86-baseline-3-result.xml create mode 100644 tests/cputestdata/x86-baseline-4-expanded.xml create mode 100644 tests/cputestdata/x86-baseline-4-result.xml create mode 100644 tests/cputestdata/x86-baseline-4.xml create mode 100644 tests/cputestdata/x86-baseline-5-expanded.xml create mode 100644 tests/cputestdata/x86-baseline-5-result.xml create mode 100644 tests/cputestdata/x86-baseline-5.xml
ACK
Thanks for the review. I pushed the series and I'm working on a backport for 1.1.3-maint. Jirka

On 01/28/2014 04:53 AM, Jiri Denemark wrote:
Jiri Denemark (4): tests: Better support for VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES cpu: Fix VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES cpu: Try to use source CPU model in virConnectBaselineCPU tests: Add more tests for virConnectBaselineCPU
src/cpu/cpu_x86.c | 101 +++++++++++++++++--------- tests/cputest.c | 27 ++++++- tests/cputestdata/x86-baseline-3-expanded.xml | 35 +++++++++ tests/cputestdata/x86-baseline-3-result.xml | 34 +-------- tests/cputestdata/x86-baseline-4-expanded.xml | 46 ++++++++++++ tests/cputestdata/x86-baseline-4-result.xml | 14 ++++ tests/cputestdata/x86-baseline-4.xml | 18 +++++ tests/cputestdata/x86-baseline-5-expanded.xml | 47 ++++++++++++ tests/cputestdata/x86-baseline-5-result.xml | 10 +++ tests/cputestdata/x86-baseline-5.xml | 35 +++++++++ 10 files changed, 296 insertions(+), 71 deletions(-) create mode 100644 tests/cputestdata/x86-baseline-3-expanded.xml create mode 100644 tests/cputestdata/x86-baseline-4-expanded.xml create mode 100644 tests/cputestdata/x86-baseline-4-result.xml create mode 100644 tests/cputestdata/x86-baseline-4.xml create mode 100644 tests/cputestdata/x86-baseline-5-expanded.xml create mode 100644 tests/cputestdata/x86-baseline-5-result.xml create mode 100644 tests/cputestdata/x86-baseline-5.xml
Thanks Jiri, This is not a review, but a quick initial testing with these patches applied, I ran the reproducer script[1] w/ a scratch build[2]: $ python libvirt-test.py VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES = 1 Seem to work fine. Still have to test w/ OpenStack set-up. [1] https://bugzilla.redhat.com/show_bug.cgi?id=1049391#c10 [2] http://koji.fedoraproject.org/koji/taskinfo?taskID=6462665 -- /kashyap

On 01/28/2014 04:46 PM, Kashyap Chamarthy wrote:
On 01/28/2014 04:53 AM, Jiri Denemark wrote:
Jiri Denemark (4): tests: Better support for VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES cpu: Fix VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES cpu: Try to use source CPU model in virConnectBaselineCPU tests: Add more tests for virConnectBaselineCPU
src/cpu/cpu_x86.c | 101 +++++++++++++++++--------- tests/cputest.c | 27 ++++++- tests/cputestdata/x86-baseline-3-expanded.xml | 35 +++++++++ tests/cputestdata/x86-baseline-3-result.xml | 34 +-------- tests/cputestdata/x86-baseline-4-expanded.xml | 46 ++++++++++++ tests/cputestdata/x86-baseline-4-result.xml | 14 ++++ tests/cputestdata/x86-baseline-4.xml | 18 +++++ tests/cputestdata/x86-baseline-5-expanded.xml | 47 ++++++++++++ tests/cputestdata/x86-baseline-5-result.xml | 10 +++ tests/cputestdata/x86-baseline-5.xml | 35 +++++++++ 10 files changed, 296 insertions(+), 71 deletions(-) create mode 100644 tests/cputestdata/x86-baseline-3-expanded.xml create mode 100644 tests/cputestdata/x86-baseline-4-expanded.xml create mode 100644 tests/cputestdata/x86-baseline-4-result.xml create mode 100644 tests/cputestdata/x86-baseline-4.xml create mode 100644 tests/cputestdata/x86-baseline-5-expanded.xml create mode 100644 tests/cputestdata/x86-baseline-5-result.xml create mode 100644 tests/cputestdata/x86-baseline-5.xml
Thanks Jiri,
This is not a review, but a quick initial testing with these patches applied, I ran the reproducer script[1] w/ a scratch build[2]:
$ python libvirt-test.py VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES = 1
Seem to work fine. Still have to test w/ OpenStack set-up.
FWIW, also tested with OpenStack set-up, Nova Compute service comes up just fine w/ these patches. Updated the bug: https://bugzilla.redhat.com/show_bug.cgi?id=1049391#c15
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1049391#c10 [2] http://koji.fedoraproject.org/koji/taskinfo?taskID=6462665
-- /kashyap
participants (3)
-
Jiri Denemark
-
Kashyap Chamarthy
-
Peter Krempa