[libvirt] [PATCH 0/4] Improve handling of ppc64 compatibility modes
This series fixes an issue that prevented save / restore from working when using compatibility modes; it also introduces some new checks to make sure the requested compability configuration is actually supported and a few test cases. Cheers. Andrea Bolognani (4): cpu: Don't update host-model guest CPUs on ppc64 cpu: Better support for ppc64 compatibility modes cpu: Move check for NULL CPU model inside the driver tests: Add some compatibility-related cases to the CPU tests src/cpu/cpu.c | 12 --- src/cpu/cpu_generic.c | 6 ++ src/cpu/cpu_ppc64.c | 98 ++++++++++++++++++++-- src/cpu/cpu_x86.c | 6 ++ tests/cputest.c | 14 ++++ .../ppc64-guest-compat-incompatible.xml | 3 + tests/cputestdata/ppc64-guest-compat-invalid.xml | 3 + tests/cputestdata/ppc64-guest-compat-none.xml | 1 + tests/cputestdata/ppc64-guest-compat-valid.xml | 3 + tests/cputestdata/ppc64-guest-host-model.xml | 3 + .../ppc64-host+guest-compat-incompatible.xml | 3 + .../ppc64-host+guest-compat-invalid.xml | 3 + tests/cputestdata/ppc64-host+guest-compat-none.xml | 3 + .../cputestdata/ppc64-host+guest-compat-valid.xml | 3 + tests/cputestdata/ppc64-host+guest-host-model.xml | 3 + .../ppc64-host+guest-legacy-incompatible.xml | 3 + .../ppc64-host+guest-legacy-invalid.xml | 3 + tests/cputestdata/ppc64-host+guest-legacy.xml | 3 + tests/cputestdata/ppc64-host+guest-nofallback.xml | 3 + tests/cputestdata/ppc64-host+guest.xml | 3 + 20 files changed, 162 insertions(+), 17 deletions(-) create mode 100644 tests/cputestdata/ppc64-guest-compat-incompatible.xml create mode 100644 tests/cputestdata/ppc64-guest-compat-invalid.xml create mode 100644 tests/cputestdata/ppc64-guest-compat-none.xml create mode 100644 tests/cputestdata/ppc64-guest-compat-valid.xml create mode 100644 tests/cputestdata/ppc64-guest-host-model.xml create mode 100644 tests/cputestdata/ppc64-host+guest-compat-incompatible.xml create mode 100644 tests/cputestdata/ppc64-host+guest-compat-invalid.xml create mode 100644 tests/cputestdata/ppc64-host+guest-compat-none.xml create mode 100644 tests/cputestdata/ppc64-host+guest-compat-valid.xml create mode 100644 tests/cputestdata/ppc64-host+guest-host-model.xml create mode 100644 tests/cputestdata/ppc64-host+guest-legacy-incompatible.xml create mode 100644 tests/cputestdata/ppc64-host+guest-legacy-invalid.xml create mode 100644 tests/cputestdata/ppc64-host+guest-legacy.xml create mode 100644 tests/cputestdata/ppc64-host+guest-nofallback.xml create mode 100644 tests/cputestdata/ppc64-host+guest.xml -- 2.4.3
If a guest CPU is defined using <cpu mode='host-model'/> the <model> sub-element will contain the compatibility mode to use. That means we can't just copy the host CPU model on cpuUpdate(), otherwise we'll overwrite that information and migration of such guests will fail. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1251927 --- src/cpu/cpu_ppc64.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 85aa5bc..72b8fa0 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -667,13 +667,13 @@ ppc64DriverUpdate(virCPUDefPtr guest, const virCPUDef *host) { switch ((virCPUMode) guest->mode) { - case VIR_CPU_MODE_HOST_MODEL: case VIR_CPU_MODE_HOST_PASSTHROUGH: guest->match = VIR_CPU_MATCH_EXACT; guest->fallback = VIR_CPU_FALLBACK_FORBID; virCPUDefFreeModel(guest); return virCPUDefCopyModel(guest, host, true); + case VIR_CPU_MODE_HOST_MODEL: case VIR_CPU_MODE_CUSTOM: return 0; -- 2.4.3
On Tue, Aug 18, 2015 at 16:45:04 -0700, Andrea Bolognani wrote:
If a guest CPU is defined using
<cpu mode='host-model'/>
the <model> sub-element will contain the compatibility mode to use. That means we can't just copy the host CPU model on cpuUpdate(), otherwise we'll overwrite that information and migration of such guests will fail.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1251927
ACK Jirka
Not all combinations of host CPU models and compatibility modes are valid, so we need to make sure we don't try to do something that QEMU will reject. Moreover, we need to apply a different logic to guests using host-model and host-passthrough modes when testing them for host compatibility. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1251927 --- src/cpu/cpu_ppc64.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 90 insertions(+), 3 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 72b8fa0..1a1b15b 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -84,6 +84,57 @@ ppc64ConvertLegacyCPUDef(const virCPUDef *legacy) return cpu; } +/* Some hosts can run guests in compatibility mode, but not all + * host CPUs support this and not all combinations are valid. + * This function performs the necessary checks */ +static virCPUCompareResult +ppc64CheckCompatibilityMode(const char *host_model, + const char *compat_mode) +{ + int host; + int compat; + char *tmp; + virCPUCompareResult ret = VIR_CPU_COMPARE_IDENTICAL; + + if (!compat_mode) + goto out; + + ret = VIR_CPU_COMPARE_ERROR; + + /* Valid host CPUs: POWER6, POWER7, POWER8 */ + if (!STRPREFIX(host_model, "POWER") || + !(tmp = (char *) host_model + strlen("POWER")) || + virStrToLong_i(tmp, NULL, 10, &host) < 0 || + host < 6 || host > 8) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", + _("Host CPU does not support compatibility modes")); + goto out; + } + + /* Valid compatibility modes: power6, power7, power8 */ + if (!STRPREFIX(compat_mode, "power") || + !(tmp = (char *) compat_mode + strlen("power")) || + virStrToLong_i(tmp, NULL, 10, &compat) < 0 || + compat < 6 || compat > 8) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown compatibility mode %s"), + compat_mode); + goto out; + } + + ret = VIR_CPU_COMPARE_INCOMPATIBLE; + + /* Version check */ + if (compat > host) + goto out; + + ret = VIR_CPU_COMPARE_IDENTICAL; + + out: + return ret; +} + static void ppc64DataFree(virCPUppc64Data *data) { @@ -509,11 +560,47 @@ ppc64Compute(virCPUDefPtr host, goto cleanup; } - if (!(map = ppc64LoadMap()) || - !(host_model = ppc64ModelFromCPU(host, map)) || - !(guest_model = ppc64ModelFromCPU(cpu, map))) + if (!(map = ppc64LoadMap())) goto cleanup; + /* Host CPU information */ + if (!(host_model = ppc64ModelFromCPU(host, map))) + goto cleanup; + + if (cpu->type == VIR_CPU_TYPE_GUEST) { + /* Guest CPU information */ + virCPUCompareResult tmp; + switch (cpu->mode) { + case VIR_CPU_MODE_HOST_MODEL: + /* host-model only: + * we need to take compatibility modes into account */ + tmp = ppc64CheckCompatibilityMode(host->model, cpu->model); + if (tmp != VIR_CPU_COMPARE_IDENTICAL) { + ret = tmp; + goto cleanup; + } + /* fallthrough */ + + case VIR_CPU_MODE_HOST_PASSTHROUGH: + /* host-model and host-passthrough: + * the guest CPU is the same as the host */ + if (!(guest_model = ppc64ModelCopy(host_model))) + goto cleanup; + break; + + case VIR_CPU_MODE_CUSTOM: + /* custom: + * look up guest CPU information */ + if (!(guest_model = ppc64ModelFromCPU(cpu, map))) + goto cleanup; + break; + } + } else { + /* Other host CPU information */ + if (!(guest_model = ppc64ModelFromCPU(cpu, map))) + goto cleanup; + } + if (STRNEQ(guest_model->name, host_model->name)) { VIR_DEBUG("host CPU model does not match required CPU model %s", guest_model->name); -- 2.4.3
On Tue, Aug 18, 2015 at 16:45:05 -0700, Andrea Bolognani wrote:
Not all combinations of host CPU models and compatibility modes are valid, so we need to make sure we don't try to do something that QEMU will reject.
Moreover, we need to apply a different logic to guests using host-model and host-passthrough modes when testing them for host compatibility.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1251927 --- src/cpu/cpu_ppc64.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 90 insertions(+), 3 deletions(-)
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 72b8fa0..1a1b15b 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -84,6 +84,57 @@ ppc64ConvertLegacyCPUDef(const virCPUDef *legacy) return cpu; }
+/* Some hosts can run guests in compatibility mode, but not all + * host CPUs support this and not all combinations are valid. + * This function performs the necessary checks */ +static virCPUCompareResult +ppc64CheckCompatibilityMode(const char *host_model, + const char *compat_mode) +{ + int host; + int compat; + char *tmp; + virCPUCompareResult ret = VIR_CPU_COMPARE_IDENTICAL;
Shouldn't ret be initialized to VIR_CPU_COMPARE_ERROR so that we don't report everything is OK on errors?
+ + if (!compat_mode) + goto out; + + ret = VIR_CPU_COMPARE_ERROR; + + /* Valid host CPUs: POWER6, POWER7, POWER8 */ + if (!STRPREFIX(host_model, "POWER") || + !(tmp = (char *) host_model + strlen("POWER")) || + virStrToLong_i(tmp, NULL, 10, &host) < 0 || + host < 6 || host > 8) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", + _("Host CPU does not support compatibility modes")); + goto out; + } + + /* Valid compatibility modes: power6, power7, power8 */ + if (!STRPREFIX(compat_mode, "power") || + !(tmp = (char *) compat_mode + strlen("power")) || + virStrToLong_i(tmp, NULL, 10, &compat) < 0 || + compat < 6 || compat > 8) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown compatibility mode %s"), + compat_mode); + goto out; + } + + ret = VIR_CPU_COMPARE_INCOMPATIBLE; + + /* Version check */ + if (compat > host) + goto out; + + ret = VIR_CPU_COMPARE_IDENTICAL;
if (compat > host) ret = VIR_CPU_COMPARE_INCOMPATIBLE; else ret = VIR_CPU_COMPARE_IDENTICAL; would be a bit more obvious I think.
+ + out: + return ret; +} + static void ppc64DataFree(virCPUppc64Data *data) { @@ -509,11 +560,47 @@ ppc64Compute(virCPUDefPtr host, goto cleanup; }
- if (!(map = ppc64LoadMap()) || - !(host_model = ppc64ModelFromCPU(host, map)) || - !(guest_model = ppc64ModelFromCPU(cpu, map))) + if (!(map = ppc64LoadMap())) goto cleanup;
+ /* Host CPU information */ + if (!(host_model = ppc64ModelFromCPU(host, map))) + goto cleanup; + + if (cpu->type == VIR_CPU_TYPE_GUEST) { + /* Guest CPU information */ + virCPUCompareResult tmp; + switch (cpu->mode) { + case VIR_CPU_MODE_HOST_MODEL: + /* host-model only: + * we need to take compatibility modes into account */ + tmp = ppc64CheckCompatibilityMode(host->model, cpu->model); + if (tmp != VIR_CPU_COMPARE_IDENTICAL) { + ret = tmp; + goto cleanup; + } + /* fallthrough */ + + case VIR_CPU_MODE_HOST_PASSTHROUGH: + /* host-model and host-passthrough: + * the guest CPU is the same as the host */ + if (!(guest_model = ppc64ModelCopy(host_model))) + goto cleanup; + break; + + case VIR_CPU_MODE_CUSTOM: + /* custom: + * look up guest CPU information */ + if (!(guest_model = ppc64ModelFromCPU(cpu, map))) + goto cleanup; + break; + } + } else { + /* Other host CPU information */ + if (!(guest_model = ppc64ModelFromCPU(cpu, map))) + goto cleanup; + } + if (STRNEQ(guest_model->name, host_model->name)) { VIR_DEBUG("host CPU model does not match required CPU model %s", guest_model->name);
In the long term, I think we should store compatibility modes within cpu_map.xml, but ACK to this with the small issues addressed. Jirka
On Fri, 2015-08-21 at 14:27 -0700, Jiri Denemark wrote:
+static virCPUCompareResult +ppc64CheckCompatibilityMode(const char *host_model, + const char *compat_mode) +{ + int host; + int compat; + char *tmp; + virCPUCompareResult ret = VIR_CPU_COMPARE_IDENTICAL;
Shouldn't ret be initialized to VIR_CPU_COMPARE_ERROR so that we don't report everything is OK on errors?
Initializing it to VIR_CPU_COMPARE_IDENTICAL allows us to just jump to the exit point if a compatibility mode is not used (see check right below). That could be replaced with an explicit return if you think that would make the code easier to understand.
+ + if (!compat_mode) + goto out; [...] + ret = VIR_CPU_COMPARE_INCOMPATIBLE; + + /* Version check */ + if (compat > host) + goto out; + + ret = VIR_CPU_COMPARE_IDENTICAL;
if (compat > host) ret = VIR_CPU_COMPARE_INCOMPATIBLE; else ret = VIR_CPU_COMPARE_IDENTICAL;
would be a bit more obvious I think.
Both work for me, so why not :)
In the long term, I think we should store compatibility modes within cpu_map.xml, but ACK to this with the small issues addressed.
Sure, that will come later on. Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team
On Fri, Aug 21, 2015 at 14:55:02 -0700, Andrea Bolognani wrote:
On Fri, 2015-08-21 at 14:27 -0700, Jiri Denemark wrote:
+static virCPUCompareResult +ppc64CheckCompatibilityMode(const char *host_model, + const char *compat_mode) +{ + int host; + int compat; + char *tmp; + virCPUCompareResult ret = VIR_CPU_COMPARE_IDENTICAL;
Shouldn't ret be initialized to VIR_CPU_COMPARE_ERROR so that we don't report everything is OK on errors?
Initializing it to VIR_CPU_COMPARE_IDENTICAL allows us to just jump to the exit point if a compatibility mode is not used (see check right below).
That could be replaced with an explicit return if you think that would make the code easier to understand.
Oops, I'm blind, I completely missed the "ret = VIR_CPU_COMPARE_ERROR" after the check. But yes, I think initializing to *_ERROR and an explicit return if compatibility mode is not used would be more readable. Jirka
On Fri, 2015-08-21 at 14:58 -0700, Jiri Denemark wrote:
On Fri, Aug 21, 2015 at 14:55:02 -0700, Andrea Bolognani wrote:
On Fri, 2015-08-21 at 14:27 -0700, Jiri Denemark wrote:
+static virCPUCompareResult +ppc64CheckCompatibilityMode(const char *host_model, + const char *compat_mode) +{ + int host; + int compat; + char *tmp; + virCPUCompareResult ret = VIR_CPU_COMPARE_IDENTICAL;
Shouldn't ret be initialized to VIR_CPU_COMPARE_ERROR so that we don't report everything is OK on errors?
Initializing it to VIR_CPU_COMPARE_IDENTICAL allows us to just jump to the exit point if a compatibility mode is not used (see check right below).
That could be replaced with an explicit return if you think that would make the code easier to understand.
Oops, I'm blind, I completely missed the "ret = VIR_CPU_COMPARE_ERROR" after the check. But yes, I think initializing to *_ERROR and an explicit return if compatibility mode is not used would be more readable.
Pushed with those changes squashed in. Thanks. -- Andrea Bolognani Software Engineer - Virtualization Team
While the check is appropriate for eg. the x86 and generic drivers, there are some valid ppc64 guest configurations where the CPU model is supposed to be NULL. Moving this check from the generic code to the drivers makes it possible to accomodate both use cases. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1251927 --- src/cpu/cpu.c | 12 ------------ src/cpu/cpu_generic.c | 6 ++++++ src/cpu/cpu_ppc64.c | 3 ++- src/cpu/cpu_x86.c | 6 ++++++ 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 731df26..1952b53 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -142,12 +142,6 @@ cpuCompare(virCPUDefPtr host, VIR_DEBUG("host=%p, cpu=%p", host, cpu); - if (!cpu->model) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("no guest CPU model specified")); - return VIR_CPU_COMPARE_ERROR; - } - if ((driver = cpuGetSubDriver(host->arch)) == NULL) return VIR_CPU_COMPARE_ERROR; @@ -376,12 +370,6 @@ cpuGuestData(virCPUDefPtr host, VIR_DEBUG("host=%p, guest=%p, data=%p, msg=%p", host, guest, data, msg); - if (!guest->model) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("no guest CPU model specified")); - return VIR_CPU_COMPARE_ERROR; - } - if ((driver = cpuGetSubDriver(host->arch)) == NULL) return VIR_CPU_COMPARE_ERROR; diff --git a/src/cpu/cpu_generic.c b/src/cpu/cpu_generic.c index a9cde4c..f26a62d 100644 --- a/src/cpu/cpu_generic.c +++ b/src/cpu/cpu_generic.c @@ -65,6 +65,12 @@ genericCompare(virCPUDefPtr host, size_t i; unsigned int reqfeatures; + if (!cpu->model) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("no guest CPU model specified")); + goto cleanup; + } + if ((cpu->arch != VIR_ARCH_NONE && host->arch != cpu->arch) || STRNEQ(host->model, cpu->model)) { diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 1a1b15b..4fdefb7 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -71,7 +71,8 @@ ppc64ConvertLegacyCPUDef(const virCPUDef *legacy) if (!(cpu = virCPUDefCopy(legacy))) goto out; - if (!(STREQ(cpu->model, "POWER7_v2.1") || + if (!cpu->model || + !(STREQ(cpu->model, "POWER7_v2.1") || STREQ(cpu->model, "POWER7_v2.3") || STREQ(cpu->model, "POWER7+_v2.1") || STREQ(cpu->model, "POWER8_v1.0"))) { diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index f5f7697..90949f6 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1371,6 +1371,12 @@ x86Compute(virCPUDefPtr host, virArch arch; size_t i; + if (!cpu->model) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("no guest CPU model specified")); + return VIR_CPU_COMPARE_ERROR; + } + if (cpu->arch != VIR_ARCH_NONE) { bool found = false; -- 2.4.3
On Tue, Aug 18, 2015 at 16:45:06 -0700, Andrea Bolognani wrote:
While the check is appropriate for eg. the x86 and generic drivers, there are some valid ppc64 guest configurations where the CPU model is supposed to be NULL.
Moving this check from the generic code to the drivers makes it possible to accomodate both use cases.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1251927
ACK Jirka
--- tests/cputest.c | 14 ++++++++++++++ tests/cputestdata/ppc64-guest-compat-incompatible.xml | 3 +++ tests/cputestdata/ppc64-guest-compat-invalid.xml | 3 +++ tests/cputestdata/ppc64-guest-compat-none.xml | 1 + tests/cputestdata/ppc64-guest-compat-valid.xml | 3 +++ tests/cputestdata/ppc64-guest-host-model.xml | 3 +++ tests/cputestdata/ppc64-host+guest-compat-incompatible.xml | 3 +++ tests/cputestdata/ppc64-host+guest-compat-invalid.xml | 3 +++ tests/cputestdata/ppc64-host+guest-compat-none.xml | 3 +++ tests/cputestdata/ppc64-host+guest-compat-valid.xml | 3 +++ tests/cputestdata/ppc64-host+guest-host-model.xml | 3 +++ tests/cputestdata/ppc64-host+guest-legacy-incompatible.xml | 3 +++ tests/cputestdata/ppc64-host+guest-legacy-invalid.xml | 3 +++ tests/cputestdata/ppc64-host+guest-legacy.xml | 3 +++ tests/cputestdata/ppc64-host+guest-nofallback.xml | 3 +++ tests/cputestdata/ppc64-host+guest.xml | 3 +++ 16 files changed, 57 insertions(+) create mode 100644 tests/cputestdata/ppc64-guest-compat-incompatible.xml create mode 100644 tests/cputestdata/ppc64-guest-compat-invalid.xml create mode 100644 tests/cputestdata/ppc64-guest-compat-none.xml create mode 100644 tests/cputestdata/ppc64-guest-compat-valid.xml create mode 100644 tests/cputestdata/ppc64-guest-host-model.xml create mode 100644 tests/cputestdata/ppc64-host+guest-compat-incompatible.xml create mode 100644 tests/cputestdata/ppc64-host+guest-compat-invalid.xml create mode 100644 tests/cputestdata/ppc64-host+guest-compat-none.xml create mode 100644 tests/cputestdata/ppc64-host+guest-compat-valid.xml create mode 100644 tests/cputestdata/ppc64-host+guest-host-model.xml create mode 100644 tests/cputestdata/ppc64-host+guest-legacy-incompatible.xml create mode 100644 tests/cputestdata/ppc64-host+guest-legacy-invalid.xml create mode 100644 tests/cputestdata/ppc64-host+guest-legacy.xml create mode 100644 tests/cputestdata/ppc64-host+guest-nofallback.xml create mode 100644 tests/cputestdata/ppc64-host+guest.xml diff --git a/tests/cputest.c b/tests/cputest.c index 5992dd0..431b587 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -607,6 +607,10 @@ mymain(void) DO_TEST_COMPARE("ppc64", "host", "guest-legacy", VIR_CPU_COMPARE_IDENTICAL); DO_TEST_COMPARE("ppc64", "host", "guest-legacy-incompatible", VIR_CPU_COMPARE_INCOMPATIBLE); DO_TEST_COMPARE("ppc64", "host", "guest-legacy-invalid", VIR_CPU_COMPARE_ERROR); + DO_TEST_COMPARE("ppc64", "host", "guest-compat-none", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_COMPARE("ppc64", "host", "guest-compat-valid", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_COMPARE("ppc64", "host", "guest-compat-invalid", VIR_CPU_COMPARE_ERROR); + DO_TEST_COMPARE("ppc64", "host", "guest-compat-incompatible", VIR_CPU_COMPARE_INCOMPATIBLE); /* guest updates for migration * automatically compares host CPU with the result */ @@ -618,6 +622,16 @@ mymain(void) DO_TEST_UPDATE("x86", "host", "host-passthrough", VIR_CPU_COMPARE_IDENTICAL); DO_TEST_UPDATE("x86", "host-invtsc", "host-model", VIR_CPU_COMPARE_SUPERSET); + DO_TEST_UPDATE("ppc64", "host", "guest", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_UPDATE("ppc64", "host", "guest-nofallback", VIR_CPU_COMPARE_INCOMPATIBLE); + DO_TEST_UPDATE("ppc64", "host", "guest-legacy", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_UPDATE("ppc64", "host", "guest-legacy-incompatible", VIR_CPU_COMPARE_INCOMPATIBLE); + DO_TEST_UPDATE("ppc64", "host", "guest-legacy-invalid", VIR_CPU_COMPARE_ERROR); + DO_TEST_UPDATE("ppc64", "host", "guest-compat-none", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_UPDATE("ppc64", "host", "guest-compat-valid", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_UPDATE("ppc64", "host", "guest-compat-invalid", VIR_CPU_COMPARE_ERROR); + DO_TEST_UPDATE("ppc64", "host", "guest-compat-incompatible", VIR_CPU_COMPARE_INCOMPATIBLE); + /* computing baseline CPUs */ DO_TEST_BASELINE("x86", "incompatible-vendors", 0, -1); DO_TEST_BASELINE("x86", "no-vendor", 0, 0); diff --git a/tests/cputestdata/ppc64-guest-compat-incompatible.xml b/tests/cputestdata/ppc64-guest-compat-incompatible.xml new file mode 100644 index 0000000..3f130fa --- /dev/null +++ b/tests/cputestdata/ppc64-guest-compat-incompatible.xml @@ -0,0 +1,3 @@ +<cpu mode='host-model'> + <model>power8</model> +</cpu> diff --git a/tests/cputestdata/ppc64-guest-compat-invalid.xml b/tests/cputestdata/ppc64-guest-compat-invalid.xml new file mode 100644 index 0000000..f35cb21 --- /dev/null +++ b/tests/cputestdata/ppc64-guest-compat-invalid.xml @@ -0,0 +1,3 @@ +<cpu mode='host-model'> + <model>power7+</model> +</cpu> diff --git a/tests/cputestdata/ppc64-guest-compat-none.xml b/tests/cputestdata/ppc64-guest-compat-none.xml new file mode 100644 index 0000000..fd50c03 --- /dev/null +++ b/tests/cputestdata/ppc64-guest-compat-none.xml @@ -0,0 +1 @@ +<cpu mode='host-model'/> diff --git a/tests/cputestdata/ppc64-guest-compat-valid.xml b/tests/cputestdata/ppc64-guest-compat-valid.xml new file mode 100644 index 0000000..bf01e54 --- /dev/null +++ b/tests/cputestdata/ppc64-guest-compat-valid.xml @@ -0,0 +1,3 @@ +<cpu mode='host-model'> + <model>power6</model> +</cpu> diff --git a/tests/cputestdata/ppc64-guest-host-model.xml b/tests/cputestdata/ppc64-guest-host-model.xml new file mode 100644 index 0000000..188ebeb --- /dev/null +++ b/tests/cputestdata/ppc64-guest-host-model.xml @@ -0,0 +1,3 @@ +<cpu mode='host-model'> + <model fallback='allow'/> +</cpu> diff --git a/tests/cputestdata/ppc64-host+guest-compat-incompatible.xml b/tests/cputestdata/ppc64-host+guest-compat-incompatible.xml new file mode 100644 index 0000000..1fab751 --- /dev/null +++ b/tests/cputestdata/ppc64-host+guest-compat-incompatible.xml @@ -0,0 +1,3 @@ +<cpu mode='host-model' match='exact'> + <model fallback='allow'>power8</model> +</cpu> diff --git a/tests/cputestdata/ppc64-host+guest-compat-invalid.xml b/tests/cputestdata/ppc64-host+guest-compat-invalid.xml new file mode 100644 index 0000000..bc0d829 --- /dev/null +++ b/tests/cputestdata/ppc64-host+guest-compat-invalid.xml @@ -0,0 +1,3 @@ +<cpu mode='host-model' match='exact'> + <model fallback='allow'>power7+</model> +</cpu> diff --git a/tests/cputestdata/ppc64-host+guest-compat-none.xml b/tests/cputestdata/ppc64-host+guest-compat-none.xml new file mode 100644 index 0000000..188ebeb --- /dev/null +++ b/tests/cputestdata/ppc64-host+guest-compat-none.xml @@ -0,0 +1,3 @@ +<cpu mode='host-model'> + <model fallback='allow'/> +</cpu> diff --git a/tests/cputestdata/ppc64-host+guest-compat-valid.xml b/tests/cputestdata/ppc64-host+guest-compat-valid.xml new file mode 100644 index 0000000..da9cc91 --- /dev/null +++ b/tests/cputestdata/ppc64-host+guest-compat-valid.xml @@ -0,0 +1,3 @@ +<cpu mode='host-model' match='exact'> + <model fallback='allow'>power6</model> +</cpu> diff --git a/tests/cputestdata/ppc64-host+guest-host-model.xml b/tests/cputestdata/ppc64-host+guest-host-model.xml new file mode 100644 index 0000000..188ebeb --- /dev/null +++ b/tests/cputestdata/ppc64-host+guest-host-model.xml @@ -0,0 +1,3 @@ +<cpu mode='host-model'> + <model fallback='allow'/> +</cpu> diff --git a/tests/cputestdata/ppc64-host+guest-legacy-incompatible.xml b/tests/cputestdata/ppc64-host+guest-legacy-incompatible.xml new file mode 100644 index 0000000..f5b8384 --- /dev/null +++ b/tests/cputestdata/ppc64-host+guest-legacy-incompatible.xml @@ -0,0 +1,3 @@ +<cpu mode='custom' match='exact'> + <model fallback='allow'>POWER8_v1.0</model> +</cpu> diff --git a/tests/cputestdata/ppc64-host+guest-legacy-invalid.xml b/tests/cputestdata/ppc64-host+guest-legacy-invalid.xml new file mode 100644 index 0000000..be059c3 --- /dev/null +++ b/tests/cputestdata/ppc64-host+guest-legacy-invalid.xml @@ -0,0 +1,3 @@ +<cpu mode='custom' match='exact'> + <model fallback='allow'>POWER8E_v1.0</model> +</cpu> diff --git a/tests/cputestdata/ppc64-host+guest-legacy.xml b/tests/cputestdata/ppc64-host+guest-legacy.xml new file mode 100644 index 0000000..36bae52 --- /dev/null +++ b/tests/cputestdata/ppc64-host+guest-legacy.xml @@ -0,0 +1,3 @@ +<cpu mode='custom' match='exact'> + <model fallback='allow'>POWER7_v2.3</model> +</cpu> diff --git a/tests/cputestdata/ppc64-host+guest-nofallback.xml b/tests/cputestdata/ppc64-host+guest-nofallback.xml new file mode 100644 index 0000000..0223018 --- /dev/null +++ b/tests/cputestdata/ppc64-host+guest-nofallback.xml @@ -0,0 +1,3 @@ +<cpu mode='custom' match='exact'> + <model fallback='forbid'>POWER8</model> +</cpu> diff --git a/tests/cputestdata/ppc64-host+guest.xml b/tests/cputestdata/ppc64-host+guest.xml new file mode 100644 index 0000000..7fac4b7 --- /dev/null +++ b/tests/cputestdata/ppc64-host+guest.xml @@ -0,0 +1,3 @@ +<cpu mode='custom' match='exact'> + <model fallback='allow'>POWER7</model> +</cpu> -- 2.4.3
On Tue, Aug 18, 2015 at 16:45:07 -0700, Andrea Bolognani wrote:
--- tests/cputest.c | 14 ++++++++++++++ tests/cputestdata/ppc64-guest-compat-incompatible.xml | 3 +++ tests/cputestdata/ppc64-guest-compat-invalid.xml | 3 +++ tests/cputestdata/ppc64-guest-compat-none.xml | 1 + tests/cputestdata/ppc64-guest-compat-valid.xml | 3 +++ tests/cputestdata/ppc64-guest-host-model.xml | 3 +++ tests/cputestdata/ppc64-host+guest-compat-incompatible.xml | 3 +++ tests/cputestdata/ppc64-host+guest-compat-invalid.xml | 3 +++ tests/cputestdata/ppc64-host+guest-compat-none.xml | 3 +++ tests/cputestdata/ppc64-host+guest-compat-valid.xml | 3 +++ tests/cputestdata/ppc64-host+guest-host-model.xml | 3 +++ tests/cputestdata/ppc64-host+guest-legacy-incompatible.xml | 3 +++ tests/cputestdata/ppc64-host+guest-legacy-invalid.xml | 3 +++ tests/cputestdata/ppc64-host+guest-legacy.xml | 3 +++ tests/cputestdata/ppc64-host+guest-nofallback.xml | 3 +++ tests/cputestdata/ppc64-host+guest.xml | 3 +++ 16 files changed, 57 insertions(+) create mode 100644 tests/cputestdata/ppc64-guest-compat-incompatible.xml create mode 100644 tests/cputestdata/ppc64-guest-compat-invalid.xml create mode 100644 tests/cputestdata/ppc64-guest-compat-none.xml create mode 100644 tests/cputestdata/ppc64-guest-compat-valid.xml create mode 100644 tests/cputestdata/ppc64-guest-host-model.xml create mode 100644 tests/cputestdata/ppc64-host+guest-compat-incompatible.xml create mode 100644 tests/cputestdata/ppc64-host+guest-compat-invalid.xml create mode 100644 tests/cputestdata/ppc64-host+guest-compat-none.xml create mode 100644 tests/cputestdata/ppc64-host+guest-compat-valid.xml create mode 100644 tests/cputestdata/ppc64-host+guest-host-model.xml create mode 100644 tests/cputestdata/ppc64-host+guest-legacy-incompatible.xml create mode 100644 tests/cputestdata/ppc64-host+guest-legacy-invalid.xml create mode 100644 tests/cputestdata/ppc64-host+guest-legacy.xml create mode 100644 tests/cputestdata/ppc64-host+guest-nofallback.xml create mode 100644 tests/cputestdata/ppc64-host+guest.xml
diff --git a/tests/cputest.c b/tests/cputest.c index 5992dd0..431b587 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -607,6 +607,10 @@ mymain(void) DO_TEST_COMPARE("ppc64", "host", "guest-legacy", VIR_CPU_COMPARE_IDENTICAL); DO_TEST_COMPARE("ppc64", "host", "guest-legacy-incompatible", VIR_CPU_COMPARE_INCOMPATIBLE); DO_TEST_COMPARE("ppc64", "host", "guest-legacy-invalid", VIR_CPU_COMPARE_ERROR); + DO_TEST_COMPARE("ppc64", "host", "guest-compat-none", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_COMPARE("ppc64", "host", "guest-compat-valid", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_COMPARE("ppc64", "host", "guest-compat-invalid", VIR_CPU_COMPARE_ERROR);
I'm wondering how the above test could have passed when ret was initialized to VIR_CPU_COMPARE_IDENTICAL in patch 2 :-)
+ DO_TEST_COMPARE("ppc64", "host", "guest-compat-incompatible", VIR_CPU_COMPARE_INCOMPATIBLE);
/* guest updates for migration * automatically compares host CPU with the result */ @@ -618,6 +622,16 @@ mymain(void) DO_TEST_UPDATE("x86", "host", "host-passthrough", VIR_CPU_COMPARE_IDENTICAL); DO_TEST_UPDATE("x86", "host-invtsc", "host-model", VIR_CPU_COMPARE_SUPERSET);
+ DO_TEST_UPDATE("ppc64", "host", "guest", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_UPDATE("ppc64", "host", "guest-nofallback", VIR_CPU_COMPARE_INCOMPATIBLE); + DO_TEST_UPDATE("ppc64", "host", "guest-legacy", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_UPDATE("ppc64", "host", "guest-legacy-incompatible", VIR_CPU_COMPARE_INCOMPATIBLE); + DO_TEST_UPDATE("ppc64", "host", "guest-legacy-invalid", VIR_CPU_COMPARE_ERROR); + DO_TEST_UPDATE("ppc64", "host", "guest-compat-none", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_UPDATE("ppc64", "host", "guest-compat-valid", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_UPDATE("ppc64", "host", "guest-compat-invalid", VIR_CPU_COMPARE_ERROR); + DO_TEST_UPDATE("ppc64", "host", "guest-compat-incompatible", VIR_CPU_COMPARE_INCOMPATIBLE); + /* computing baseline CPUs */ DO_TEST_BASELINE("x86", "incompatible-vendors", 0, -1); DO_TEST_BASELINE("x86", "no-vendor", 0, 0); ... diff --git a/tests/cputestdata/ppc64-host+guest-compat-incompatible.xml b/tests/cputestdata/ppc64-host+guest-compat-incompatible.xml new file mode 100644 index 0000000..1fab751 --- /dev/null +++ b/tests/cputestdata/ppc64-host+guest-compat-incompatible.xml @@ -0,0 +1,3 @@ +<cpu mode='host-model' match='exact'> + <model fallback='allow'>power8</model> +</cpu>
The test is supposed to fail for this (and some other cases too) so why do we need to have an output XML? Jirka
On Fri, 2015-08-21 at 14:37 -0700, Jiri Denemark wrote:
--- a/tests/cputest.c +++ b/tests/cputest.c @@ -607,6 +607,10 @@ mymain(void) DO_TEST_COMPARE("ppc64", "host", "guest-legacy", VIR_CPU_COMPARE_IDENTICAL); DO_TEST_COMPARE("ppc64", "host", "guest-legacy-incompatible", VIR_CPU_COMPARE_INCOMPATIBLE); DO_TEST_COMPARE("ppc64", "host", "guest-legacy-invalid", VIR_CPU_COMPARE_ERROR); + DO_TEST_COMPARE("ppc64", "host", "guest-compat-none", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_COMPARE("ppc64", "host", "guest-compat-valid", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_COMPARE("ppc64", "host", "guest-compat-invalid", VIR_CPU_COMPARE_ERROR);
I'm wondering how the above test could have passed when ret was initialized to VIR_CPU_COMPARE_IDENTICAL in patch 2 :-)
All tests pass both on my laptop and on actual POWER hardware... Did you get a failure instead?
diff --git a/tests/cputestdata/ppc64-host+guest-compat -incompatible.xml b/tests/cputestdata/ppc64-host+guest-compat -incompatible.xml new file mode 100644 index 0000000..1fab751 --- /dev/null +++ b/tests/cputestdata/ppc64-host+guest-compat-incompatible.xml @@ -0,0 +1,3 @@ +<cpu mode='host-model' match='exact'> + <model fallback='allow'>power8</model> +</cpu>
The test is supposed to fail for this (and some other cases too) so why do we need to have an output XML?
Test cases that use VIR_TEST_UPDATE() run in two steps: 1. run cpuUpdate() and compare the returned XML to the expected XML - the file you're referring to 2. the obtained XML definition is tested for compatibility with the host CPU using cpuCompare() So the expected failures refer to the second check, and you still need to have the expected XML for the first part - if you try and remove it, the test case will fail. Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team
On Fri, Aug 21, 2015 at 14:59:19 -0700, Andrea Bolognani wrote:
On Fri, 2015-08-21 at 14:37 -0700, Jiri Denemark wrote:
--- a/tests/cputest.c +++ b/tests/cputest.c @@ -607,6 +607,10 @@ mymain(void) DO_TEST_COMPARE("ppc64", "host", "guest-legacy", VIR_CPU_COMPARE_IDENTICAL); DO_TEST_COMPARE("ppc64", "host", "guest-legacy-incompatible", VIR_CPU_COMPARE_INCOMPATIBLE); DO_TEST_COMPARE("ppc64", "host", "guest-legacy-invalid", VIR_CPU_COMPARE_ERROR); + DO_TEST_COMPARE("ppc64", "host", "guest-compat-none", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_COMPARE("ppc64", "host", "guest-compat-valid", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_COMPARE("ppc64", "host", "guest-compat-invalid", VIR_CPU_COMPARE_ERROR);
I'm wondering how the above test could have passed when ret was initialized to VIR_CPU_COMPARE_IDENTICAL in patch 2 :-)
All tests pass both on my laptop and on actual POWER hardware... Did you get a failure instead?
No, I just missed one line in the patch 2. Everything is correct as it is.
diff --git a/tests/cputestdata/ppc64-host+guest-compat -incompatible.xml b/tests/cputestdata/ppc64-host+guest-compat -incompatible.xml new file mode 100644 index 0000000..1fab751 --- /dev/null +++ b/tests/cputestdata/ppc64-host+guest-compat-incompatible.xml @@ -0,0 +1,3 @@ +<cpu mode='host-model' match='exact'> + <model fallback='allow'>power8</model> +</cpu>
The test is supposed to fail for this (and some other cases too) so why do we need to have an output XML?
Test cases that use VIR_TEST_UPDATE() run in two steps:
1. run cpuUpdate() and compare the returned XML to the expected XML - the file you're referring to
2. the obtained XML definition is tested for compatibility with the host CPU using cpuCompare()
So the expected failures refer to the second check, and you still need to have the expected XML for the first part - if you try and remove it, the test case will fail.
I see (thinking is more difficult than just asking :-P) ACK Jirka
participants (2)
-
Andrea Bolognani -
Jiri Denemark