6:45 p.m.
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
6:45 p.m.
New subject: [libvirt] [PATCH 1/4] cpu: Don't update host-model guest CPUs on ppc64
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
4:15 p.m.
New subject: [libvirt] [PATCH 1/4] cpu: Don't update host-model guest CPUs on ppc64
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
6:45 p.m.
New subject: [libvirt] [PATCH 2/4] cpu: Better support for ppc64 compatibility modes
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
4:27 p.m.
New subject: [libvirt] [PATCH 2/4] cpu: Better support for ppc64 compatibility modes
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
4:55 p.m.
New subject: [libvirt] [PATCH 2/4] cpu: Better support for ppc64 compatibility modes
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
4:58 p.m.
New subject: [libvirt] [PATCH 2/4] cpu: Better support for ppc64 compatibility modes
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
5:57 p.m.
New subject: [libvirt] [PATCH 2/4] cpu: Better support for ppc64 compatibility modes
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
6:45 p.m.
New subject: [libvirt] [PATCH 3/4] cpu: Move check for NULL CPU model inside the driver
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
4:31 p.m.
New subject: [libvirt] [PATCH 3/4] cpu: Move check for NULL CPU model inside the driver
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
6:45 p.m.
New subject: [libvirt] [PATCH 4/4] tests: Add some compatibility-related cases to the CPU tests
---
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
4:37 p.m.
New subject: [libvirt] [PATCH 4/4] tests: Add some compatibility-related cases to the CPU tests
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
4:59 p.m.
New subject: [libvirt] [PATCH 4/4] tests: Add some compatibility-related cases to the CPU tests
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
5:02 p.m.
New subject: [libvirt] [PATCH 4/4] tests: Add some compatibility-related cases to the CPU tests
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
3381
days inactive
3384
days old
13 comments
2 participants
participants (2)
-
Andrea Bolognani -
Jiri Denemark