9:44 a.m.
On s390 machines host-passthrough and host-model CPUs result in the same
guest ABI (with QEMU new enough to be able to tell us what "host" CPU is
expanded to, which was implemented around 2.9.0). So instead of using
host-passthrough CPU when there's no CPU specified in a domain XML we
can safely use host-model and benefit from CPU compatibility checks
during migration, snapshot restore and similar operations.
This series applies on top of "qemu: Store default CPU in domain XML"
which is already acked, but it's waiting for a QEMU patch to be applied
to 4.2.0.
You can fetch both series at once using
git fetch https://gitlab.com/jirkade/libvirt cpu-default-type
Jiri Denemark (4):
cpu_conf: Fix default value for CPU match attribute
cpu_conf: Don't format empty model for host-model CPUs
cpu_s390: Don't check match attribute for host-model CPUs
qemu: Use host-model CPU on s390 by default
src/conf/cpu_conf.c | 25 ++++----------
src/conf/cpu_conf.h | 2 +-
src/cpu/cpu_s390.c | 18 +++++-----
src/qemu/qemu_domain.c | 33 ++++++++++++-------
.../ppc64-host+guest-compat-none.xml | 4 +--
...t-cpu-kvm-ccw-virtio-4.2.s390x-latest.args | 4 ++-
.../cpu-check-default-partial.xml | 4 +--
.../cpu-host-model-features.xml | 1 -
...lt-cpu-kvm-ccw-virtio-4.2.s390x-latest.xml | 2 +-
9 files changed, 45 insertions(+), 48 deletions(-)
--
2.24.0
9:44 a.m.
New subject: [libvirt] [PATCH 1/4] cpu_conf: Fix default value for CPU match attribute
Commit v0.8.4-66-g95ff6b18ec (9 years ago) changed the default value for
the cpu/@match attribute to 'exact' in a rather complicated way. It did
so only if <model> subelement was present and set -1 otherwise (which is
not expected to ever happen). Thus the following two equivalent XML
elements:
<cpu mode='host-model'/>
and
<cpu mode='host-model'>
<model/>
</cpu>
would be parsed differently. The former would end up with match == -1
while the latter would have match == 1 ('exact'). This is not a big deal
since the match attribute is ignored for host-model CPUs, but we can
simplify the code and make it a little bit saner anyway.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Well, thanks to a bug (fixed in another patch in this series) the
s390 CPU driver actually looks at the match attribute even for
host-model CPUs.
src/conf/cpu_conf.c | 9 ++-------
src/conf/cpu_conf.h | 2 +-
2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 2a1bd7c9b2..3641b5ef4c 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -46,8 +46,8 @@ VIR_ENUM_IMPL(virCPUMode,
VIR_ENUM_IMPL(virCPUMatch,
VIR_CPU_MATCH_LAST,
- "minimum",
"exact",
+ "minimum",
"strict",
);
@@ -388,12 +388,7 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
char *match = virXMLPropString(ctxt->node, "match");
char *check;
- if (!match) {
- if (virXPathBoolean("boolean(./model)", ctxt))
- def->match = VIR_CPU_MATCH_EXACT;
- else
- def->match = -1;
- } else {
+ if (match) {
def->match = virCPUMatchTypeFromString(match);
VIR_FREE(match);
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index e41d47d1ae..96fda3e6b3 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -52,8 +52,8 @@ typedef enum {
VIR_ENUM_DECL(virCPUMode);
typedef enum {
- VIR_CPU_MATCH_MINIMUM,
VIR_CPU_MATCH_EXACT,
+ VIR_CPU_MATCH_MINIMUM,
VIR_CPU_MATCH_STRICT,
VIR_CPU_MATCH_LAST
--
2.24.0
9:44 a.m.
New subject: [libvirt] [PATCH 2/4] cpu_conf: Don't format empty model for host-model CPUs
Most likely for historical reasons our CPU def formatting code is
happily adding useless <model fallback='allow'/> for host-model CPUs. We
can just drop it.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/conf/cpu_conf.c | 16 +++++-----------
.../cputestdata/ppc64-host+guest-compat-none.xml | 4 +---
.../cpu-check-default-partial.xml | 4 +---
.../cpu-host-model-features.xml | 1 -
4 files changed, 7 insertions(+), 18 deletions(-)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 3641b5ef4c..4542bcb7bd 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -751,16 +751,12 @@ virCPUDefFormatBuf(virBufferPtr buf,
{
size_t i;
bool formatModel;
- bool formatFallback;
if (!def)
return 0;
formatModel = (def->mode == VIR_CPU_MODE_CUSTOM ||
def->mode == VIR_CPU_MODE_HOST_MODEL);
- formatFallback = (def->type == VIR_CPU_TYPE_GUEST &&
- (def->mode == VIR_CPU_MODE_HOST_MODEL ||
- (def->mode == VIR_CPU_MODE_CUSTOM && def->model)));
if (!def->model && def->mode == VIR_CPU_MODE_CUSTOM &&
def->nfeatures) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -768,9 +764,10 @@ virCPUDefFormatBuf(virBufferPtr buf,
return -1;
}
- if ((formatModel && def->model) || formatFallback) {
+ if (formatModel && def->model) {
virBufferAddLit(buf, "<model");
- if (formatFallback) {
+
+ if (def->type == VIR_CPU_TYPE_GUEST) {
const char *fallback;
fallback = virCPUFallbackTypeToString(def->fallback);
@@ -784,11 +781,8 @@ virCPUDefFormatBuf(virBufferPtr buf,
if (def->vendor_id)
virBufferEscapeString(buf, " vendor_id='%s'",
def->vendor_id);
}
- if (formatModel && def->model) {
- virBufferEscapeString(buf, ">%s</model>\n",
def->model);
- } else {
- virBufferAddLit(buf, "/>\n");
- }
+
+ virBufferEscapeString(buf, ">%s</model>\n", def->model);
}
if (formatModel && def->vendor)
diff --git a/tests/cputestdata/ppc64-host+guest-compat-none.xml
b/tests/cputestdata/ppc64-host+guest-compat-none.xml
index 188ebebb72..fd50c03a79 100644
--- a/tests/cputestdata/ppc64-host+guest-compat-none.xml
+++ b/tests/cputestdata/ppc64-host+guest-compat-none.xml
@@ -1,3 +1 @@
-<cpu mode='host-model'>
- <model fallback='allow'/>
-</cpu>
+<cpu mode='host-model'/>
diff --git a/tests/qemuxml2xmloutdata/cpu-check-default-partial.xml
b/tests/qemuxml2xmloutdata/cpu-check-default-partial.xml
index 4e5fa44832..b64a1f0ef7 100644
--- a/tests/qemuxml2xmloutdata/cpu-check-default-partial.xml
+++ b/tests/qemuxml2xmloutdata/cpu-check-default-partial.xml
@@ -8,9 +8,7 @@
<type arch='x86_64' machine='pc'>hvm</type>
<boot dev='network'/>
</os>
- <cpu mode='host-model' check='partial'>
- <model fallback='allow'/>
- </cpu>
+ <cpu mode='host-model' check='partial'/>
<clock offset='utc'/>
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
diff --git a/tests/qemuxml2xmloutdata/cpu-host-model-features.xml
b/tests/qemuxml2xmloutdata/cpu-host-model-features.xml
index a5de9ea38d..6480bd5494 100644
--- a/tests/qemuxml2xmloutdata/cpu-host-model-features.xml
+++ b/tests/qemuxml2xmloutdata/cpu-host-model-features.xml
@@ -14,7 +14,6 @@
<boot dev='hd'/>
</os>
<cpu mode='host-model' check='partial'>
- <model fallback='allow'/>
<feature policy='require' name='abm'/>
<feature policy='force' name='ds'/>
<feature policy='disable' name='invtsc'/>
--
2.24.0
9:44 a.m.
New subject: [libvirt] [PATCH 3/4] cpu_s390: Don't check match attribute for host-model CPUs
The match attribute is only relevant for custom mode CPUs. Reporting
failure when match == 'minimum' regardless on CPU mode can cause
unexpected failures. We should only report the error for custom CPUs. In
fact, calling virCPUs390Update on a custom mode CPU should always report
an error as optional features are not supported on s390 either.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/cpu/cpu_s390.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/cpu/cpu_s390.c b/src/cpu/cpu_s390.c
index a4a381f4b8..dd030c5a11 100644
--- a/src/cpu/cpu_s390.c
+++ b/src/cpu/cpu_s390.c
@@ -49,15 +49,15 @@ virCPUs390Update(virCPUDefPtr guest,
int ret = -1;
size_t i;
- if (guest->match == VIR_CPU_MATCH_MINIMUM) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("match mode %s not supported"),
- virCPUMatchTypeToString(guest->match));
- goto cleanup;
- }
-
- if (guest->mode != VIR_CPU_MODE_HOST_MODEL) {
- ret = 0;
+ if (guest->mode == VIR_CPU_MODE_CUSTOM) {
+ if (guest->match == VIR_CPU_MATCH_MINIMUM) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("match mode %s not supported"),
+ virCPUMatchTypeToString(guest->match));
+ } else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("optional CPU features are not supported"));
+ }
goto cleanup;
}
--
2.24.0
9:44 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: Use host-model CPU on s390 by default
On s390 machines host-passthrough and host-model CPUs result in the same
guest ABI (with QEMU new enough to be able to tell us what "host" CPU is
expanded to, which was implemented around 2.9.0). So instead of using
host-passthrough CPU when there's no CPU specified in a domain XML we
can safely use host-model and benefit from CPU compatibility checks
during migration, snapshot restore and similar operations.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_domain.c | 33 ++++++++++++-------
...t-cpu-kvm-ccw-virtio-4.2.s390x-latest.args | 4 ++-
...lt-cpu-kvm-ccw-virtio-4.2.s390x-latest.xml | 2 +-
3 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8bd05f4058..e5269e6300 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4440,6 +4440,7 @@ qemuDomainDefVcpusPostParse(virDomainDefPtr def)
static int
qemuDomainDefSetDefaultCPU(virDomainDefPtr def,
+ virCapsPtr caps,
virQEMUCapsPtr qemuCaps)
{
const char *model;
@@ -4470,26 +4471,36 @@ qemuDomainDefSetDefaultCPU(virDomainDefPtr def,
return -1;
}
- VIR_DEBUG("Setting default CPU model for domain '%s' to %s",
- def->name, model);
-
if (!def->cpu)
def->cpu = g_new0(virCPUDef, 1);
- /* We need to turn off all CPU checks when the domain is started because
- * the default CPU (e.g., qemu64) may not be runnable on any host. QEMU
- * will just disable the unavailable features and we will update the CPU
- * definition accordingly and set check to FULL when starting the domain. */
def->cpu->type = VIR_CPU_TYPE_GUEST;
- def->cpu->check = VIR_CPU_CHECK_NONE;
if (STREQ(model, "host")) {
- def->cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
+ if (ARCH_IS_S390(def->os.arch) &&
+ virQEMUCapsIsCPUModeSupported(qemuCaps, caps, def->virtType,
+ VIR_CPU_MODE_HOST_MODEL)) {
+ def->cpu->mode = VIR_CPU_MODE_HOST_MODEL;
+ } else {
+ def->cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
+ }
+
+ VIR_DEBUG("Setting default CPU mode for domain '%s' to %s",
+ def->name, virCPUModeTypeToString(def->cpu->mode));
} else {
+ /* We need to turn off all CPU checks when the domain is started
+ * because the default CPU (e.g., qemu64) may not be runnable on any
+ * host. QEMU will just disable the unavailable features and we will
+ * update the CPU definition accordingly and set check to FULL when
+ * starting the domain. */
+ def->cpu->check = VIR_CPU_CHECK_NONE;
def->cpu->mode = VIR_CPU_MODE_CUSTOM;
def->cpu->match = VIR_CPU_MATCH_EXACT;
def->cpu->fallback = VIR_CPU_FALLBACK_FORBID;
def->cpu->model = g_strdup(model);
+
+ VIR_DEBUG("Setting default CPU model for domain '%s' to %s",
+ def->name, model);
}
return 0;
@@ -4674,7 +4685,7 @@ qemuDomainDefPostParseBasic(virDomainDefPtr def,
static int
qemuDomainDefPostParse(virDomainDefPtr def,
- virCapsPtr caps G_GNUC_UNUSED,
+ virCapsPtr caps,
unsigned int parseFlags,
void *opaque,
void *parseOpaque)
@@ -4706,7 +4717,7 @@ qemuDomainDefPostParse(virDomainDefPtr def,
if (qemuCanonicalizeMachine(def, qemuCaps) < 0)
return -1;
- if (qemuDomainDefSetDefaultCPU(def, qemuCaps) < 0)
+ if (qemuDomainDefSetDefaultCPU(def, caps, qemuCaps) < 0)
return -1;
qemuDomainDefEnableDefaultFeatures(def, qemuCaps);
diff --git a/tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-4.2.s390x-latest.args
b/tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-4.2.s390x-latest.args
index 4b7345630b..0386019418 100644
--- a/tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-4.2.s390x-latest.args
+++ b/tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-4.2.s390x-latest.args
@@ -13,7 +13,9 @@ QEMU_AUDIO_DRV=none \
-object secret,id=masterKey0,format=raw,\
file=/tmp/lib/domain--1-test/master-key.aes \
-machine s390-ccw-virtio-4.2,accel=kvm,usb=off,dump-guest-core=off \
--cpu host \
+-cpu z13.2-base,aen=on,aefsi=on,msa5=on,msa4=on,msa3=on,msa2=on,msa1=on,\
+sthyi=on,edat=on,ri=on,edat2=on,vx=on,ipter=on,ap=on,esop=on,apft=on,apqci=on,\
+cte=on,bpb=on,ppa15=on,zpci=on,sea_esop2=on,te=on,cmm=on \
-m 256 \
-overcommit mem-lock=off \
-smp 1,sockets=1,cores=1,threads=1 \
diff --git a/tests/qemuxml2xmloutdata/s390-default-cpu-kvm-ccw-virtio-4.2.s390x-latest.xml
b/tests/qemuxml2xmloutdata/s390-default-cpu-kvm-ccw-virtio-4.2.s390x-latest.xml
index eec5051934..1cc2edd893 100644
--- a/tests/qemuxml2xmloutdata/s390-default-cpu-kvm-ccw-virtio-4.2.s390x-latest.xml
+++ b/tests/qemuxml2xmloutdata/s390-default-cpu-kvm-ccw-virtio-4.2.s390x-latest.xml
@@ -8,7 +8,7 @@
<type arch='s390x'
machine='s390-ccw-virtio-4.2'>hvm</type>
<boot dev='hd'/>
</os>
- <cpu mode='host-passthrough' check='none'/>
+ <cpu mode='host-model' check='partial'/>
<clock offset='utc'/>
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
--
2.24.0
8:12 a.m.
Just a heads up.
After installing libvirt rpms of this branch all my existing kvm s390
domains ended up with
<cpu mode='custom' match='exact' check='none'>
<model fallback='forbid'>qemu</model>
</cpu>
Newly defined domains without specified cpu do so as well.
On 11/14/19 4:44 PM, Jiri Denemark wrote:
On s390 machines host-passthrough and host-model CPUs result in the
same
guest ABI (with QEMU new enough to be able to tell us what "host" CPU is
expanded to, which was implemented around 2.9.0). So instead of using
host-passthrough CPU when there's no CPU specified in a domain XML we
can safely use host-model and benefit from CPU compatibility checks
during migration, snapshot restore and similar operations.
This series applies on top of "qemu: Store default CPU in domain XML"
which is already acked, but it's waiting for a QEMU patch to be applied
to 4.2.0.
You can fetch both series at once using
git fetch https://gitlab.com/jirkade/libvirt cpu-default-type
Jiri Denemark (4):
cpu_conf: Fix default value for CPU match attribute
cpu_conf: Don't format empty model for host-model CPUs
cpu_s390: Don't check match attribute for host-model CPUs
qemu: Use host-model CPU on s390 by default
src/conf/cpu_conf.c | 25 ++++----------
src/conf/cpu_conf.h | 2 +-
src/cpu/cpu_s390.c | 18 +++++-----
src/qemu/qemu_domain.c | 33 ++++++++++++-------
.../ppc64-host+guest-compat-none.xml | 4 +--
...t-cpu-kvm-ccw-virtio-4.2.s390x-latest.args | 4 ++-
.../cpu-check-default-partial.xml | 4 +--
.../cpu-host-model-features.xml | 1 -
...lt-cpu-kvm-ccw-virtio-4.2.s390x-latest.xml | 2 +-
9 files changed, 45 insertions(+), 48 deletions(-)
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
8:47 a.m.
On Fri, Nov 15, 2019 at 15:12:18 +0100, Boris Fiuczynski wrote:
Just a heads up.
After installing libvirt rpms of this branch all my existing kvm s390
domains ended up with
<cpu mode='custom' match='exact' check='none'>
<model fallback='forbid'>qemu</model>
</cpu>
Newly defined domains without specified cpu do so as well.
Unless the domains are all TCG, it seems your QEMU is too old. You need
a fairly recent one which contains commit v4.1.0-1683-gde60a92ea7
(s390x/kvm: Set default cpu model for all machine classes)
I the domains all use KVM and you have new enough QEMU, there might be a
bug somewhere. Which should not happen :=)
Jirka
8:55 a.m.
On 15.11.19 15:47, Jiri Denemark wrote:
On Fri, Nov 15, 2019 at 15:12:18 +0100, Boris Fiuczynski wrote:
> Just a heads up.
> After installing libvirt rpms of this branch all my existing kvm s390
> domains ended up with
>
> <cpu mode='custom' match='exact' check='none'>
> <model fallback='forbid'>qemu</model>
> </cpu>
>
> Newly defined domains without specified cpu do so as well.
Unless the domains are all TCG, it seems your QEMU is too old. You need
a fairly recent one which contains commit v4.1.0-1683-gde60a92ea7
(s390x/kvm: Set default cpu model for all machine classes)
I the domains all use KVM and you have new enough QEMU, there might be a
bug somewhere. Which should not happen :=)
So shouldnt libvirt fence this rework (add default model) to qemu 4.2 and newer?
9:14 a.m.
On Fri, Nov 15, 2019 at 15:55:04 +0100, Christian Borntraeger wrote:
On 15.11.19 15:47, Jiri Denemark wrote:
> On Fri, Nov 15, 2019 at 15:12:18 +0100, Boris Fiuczynski wrote:
>> Just a heads up.
>> After installing libvirt rpms of this branch all my existing kvm s390
>> domains ended up with
>>
>> <cpu mode='custom' match='exact' check='none'>
>> <model fallback='forbid'>qemu</model>
>> </cpu>
>>
>> Newly defined domains without specified cpu do so as well.
>
> Unless the domains are all TCG, it seems your QEMU is too old. You need
> a fairly recent one which contains commit v4.1.0-1683-gde60a92ea7
> (s390x/kvm: Set default cpu model for all machine classes)
>
> I the domains all use KVM and you have new enough QEMU, there might be a
> bug somewhere. Which should not happen :=)
So shouldnt libvirt fence this rework (add default model) to qemu 4.2 and newer?
Libvirt does all this only if query-machines returns default-cpu-type,
which is introduced in 4.2. But since it was introduced earlier, anyone
using qemu from git between the two commits will see this behavior.
Somewhat similar thing will happen on ppc64, but even with the current
master. Everything should be OK once QEMU 4.2.0 final release is used,
though (since it will contain all required patches).
Jirka
11:34 a.m.
On 11/15/19 4:14 PM, Jiri Denemark wrote:
On Fri, Nov 15, 2019 at 15:55:04 +0100, Christian Borntraeger wrote:
>
>
> On 15.11.19 15:47, Jiri Denemark wrote:
>> On Fri, Nov 15, 2019 at 15:12:18 +0100, Boris Fiuczynski wrote:
>>> Just a heads up.
>>> After installing libvirt rpms of this branch all my existing kvm s390
>>> domains ended up with
>>>
>>> <cpu mode='custom' match='exact'
check='none'>
>>> <model fallback='forbid'>qemu</model>
>>> </cpu>
>>>
>>> Newly defined domains without specified cpu do so as well.
>>
>> Unless the domains are all TCG, it seems your QEMU is too old. You need
>> a fairly recent one which contains commit v4.1.0-1683-gde60a92ea7
>> (s390x/kvm: Set default cpu model for all machine classes)
>>
>> I the domains all use KVM and you have new enough QEMU, there might be a
>> bug somewhere. Which should not happen :=)
>
> So shouldnt libvirt fence this rework (add default model) to qemu 4.2 and newer?
Libvirt does all this only if query-machines returns default-cpu-type,
which is introduced in 4.2. But since it was introduced earlier, anyone
using qemu from git between the two commits will see this behavior.
Somewhat similar thing will happen on ppc64, but even with the current
master. Everything should be OK once QEMU 4.2.0 final release is used,
though (since it will contain all required patches).
Jirka
I tested with a newer version of qemu and it worked as you outlined.
After that I also tested with qemu v4.1.0. I was a bit surprised at
first that a default host-model cpu was generated since I though it
would only be done when the qemu has the commit your specified above.
After reading your patch 4 the generation is tied to the cpu-model
support in qemu. Since this became available on s390 with qemu v2.8.0 I
created an additional test patch just to ensure that we do not lose
backwards compatibility.
I also attached the patch if below does not work out.
=======
From: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
Date: Mon, 18 Nov 2019 17:41:02 +0100
Subject: [PATCH] qemuxml2*test: Add test cases for not setting default CPU
models on s390
Adding tests for QEMU v2.7.0 which was the last version without CPU model
support on s390 to ensure the default CPU model is not automatically added.
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
---
...lt-cpu-kvm-ccw-virtio-2.7.s390x-2.7.0.args | 29 +++++++++++++++++++
.../s390-default-cpu-kvm-ccw-virtio-2.7.xml | 16 ++++++++++
tests/qemuxml2argvtest.c | 1 +
...ult-cpu-kvm-ccw-virtio-2.7.s390x-2.7.0.xml | 23 +++++++++++++++
tests/qemuxml2xmltest.c | 1 +
5 files changed, 70 insertions(+)
create mode 100644
tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-2.7.0.args
create mode 100644 tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-2.7.xml
create mode 100644
tests/qemuxml2xmloutdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-2.7.0.xml
diff --git a/tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-2.7.0.args
b/tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-2.7.0.args
new file mode 100644
index 0000000000..431de90bcc
--- /dev/null
+++ b/tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-2.7.0.args
@@ -0,0 +1,29 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-test \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-test/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-test/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-test/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-s390x \
+-name guest=test,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-test/master-key.aes \
+-machine s390-ccw-virtio-2.7,accel=kvm,usb=off,dump-guest-core=off \
+-m 256 \
+-realtime mlock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid 9aa4b45c-b9dd-45ef-91fe-862b27b4231f \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-test/monitor.sock,server,nowait
\
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot strict=on \
+-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0000 \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-2.7.xml
b/tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-2.7.xml
new file mode 100644
index 0000000000..5051c861ee
--- /dev/null
+++ b/tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-2.7.xml
@@ -0,0 +1,16 @@
+<domain type='kvm'>
+ <name>test</name>
+ <uuid>9aa4b45c-b9dd-45ef-91fe-862b27b4231f</uuid>
+ <memory>262144</memory>
+ <currentMemory>262144</currentMemory>
+ <os>
+ <type arch='s390x' machine='s390-ccw-virtio'>hvm</type>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-s390x</emulator>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index e3e2bc5e63..753c588f95 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -3031,6 +3031,7 @@ mymain(void)
DO_TEST_CAPS_ARCH_LATEST("ppc64-default-cpu-tcg-pseries-3.1",
"ppc64");
DO_TEST_CAPS_ARCH_LATEST("ppc64-default-cpu-kvm-pseries-4.2",
"ppc64");
DO_TEST_CAPS_ARCH_LATEST("ppc64-default-cpu-tcg-pseries-4.2",
"ppc64");
+ DO_TEST_CAPS_ARCH_VER("s390-default-cpu-kvm-ccw-virtio-2.7",
"s390x", "2.7.0");
DO_TEST_CAPS_ARCH_LATEST("s390-default-cpu-kvm-ccw-virtio-4.2",
"s390x");
DO_TEST_CAPS_ARCH_LATEST("s390-default-cpu-tcg-ccw-virtio-4.2",
"s390x");
DO_TEST_CAPS_ARCH_LATEST("x86_64-default-cpu-kvm-pc-4.2",
"x86_64");
diff --git a/tests/qemuxml2xmloutdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-2.7.0.xml
b/tests/qemuxml2xmloutdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-2.7.0.xml
new file mode 100644
index 0000000000..9ae5356bf3
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-2.7.0.xml
@@ -0,0 +1,23 @@
+<domain type='kvm'>
+ <name>test</name>
+ <uuid>9aa4b45c-b9dd-45ef-91fe-862b27b4231f</uuid>
+ <memory unit='KiB'>262144</memory>
+ <currentMemory unit='KiB'>262144</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='s390x'
machine='s390-ccw-virtio-2.7'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-s390x</emulator>
+ <controller type='pci' index='0' model='pci-root'/>
+ <memballoon model='virtio'>
+ <address type='ccw' cssid='0xfe' ssid='0x0'
devno='0x0000'/>
+ </memballoon>
+ <panic model='s390'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 64a6971740..9bf1969047 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1349,6 +1349,7 @@ mymain(void)
DO_TEST_CAPS_ARCH_LATEST("ppc64-default-cpu-tcg-pseries-3.1",
"ppc64");
DO_TEST_CAPS_ARCH_LATEST("ppc64-default-cpu-kvm-pseries-4.2",
"ppc64");
DO_TEST_CAPS_ARCH_LATEST("ppc64-default-cpu-tcg-pseries-4.2",
"ppc64");
+ DO_TEST_CAPS_ARCH_VER("s390-default-cpu-kvm-ccw-virtio-2.7",
"s390x", "2.7.0");
DO_TEST_CAPS_ARCH_LATEST("s390-default-cpu-kvm-ccw-virtio-4.2",
"s390x");
DO_TEST_CAPS_ARCH_LATEST("s390-default-cpu-tcg-ccw-virtio-4.2",
"s390x");
DO_TEST_CAPS_ARCH_LATEST("x86_64-default-cpu-kvm-pc-4.2",
"x86_64");
--
2.21.0
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
8:02 a.m.
On Mon, Nov 18, 2019 at 18:34:36 +0100, Boris Fiuczynski wrote:
On 11/15/19 4:14 PM, Jiri Denemark wrote:
> On Fri, Nov 15, 2019 at 15:55:04 +0100, Christian Borntraeger wrote:
>>
>>
>> On 15.11.19 15:47, Jiri Denemark wrote:
>>> On Fri, Nov 15, 2019 at 15:12:18 +0100, Boris Fiuczynski wrote:
>>>> Just a heads up.
>>>> After installing libvirt rpms of this branch all my existing kvm s390
>>>> domains ended up with
>>>>
>>>> <cpu mode='custom' match='exact'
check='none'>
>>>> <model fallback='forbid'>qemu</model>
>>>> </cpu>
>>>>
>>>> Newly defined domains without specified cpu do so as well.
>>>
>>> Unless the domains are all TCG, it seems your QEMU is too old. You need
>>> a fairly recent one which contains commit v4.1.0-1683-gde60a92ea7
>>> (s390x/kvm: Set default cpu model for all machine classes)
>>>
>>> I the domains all use KVM and you have new enough QEMU, there might be a
>>> bug somewhere. Which should not happen :=)
>>
>> So shouldnt libvirt fence this rework (add default model) to qemu 4.2 and
newer?
>
> Libvirt does all this only if query-machines returns default-cpu-type,
> which is introduced in 4.2. But since it was introduced earlier, anyone
> using qemu from git between the two commits will see this behavior.
> Somewhat similar thing will happen on ppc64, but even with the current
> master. Everything should be OK once QEMU 4.2.0 final release is used,
> though (since it will contain all required patches).
>
> Jirka
>
I tested with a newer version of qemu and it worked as you outlined.
After that I also tested with qemu v4.1.0. I was a bit surprised at
first that a default host-model cpu was generated since I though it
would only be done when the qemu has the commit your specified above.
After reading your patch 4 the generation is tied to the cpu-model
support in qemu. Since this became available on s390 with qemu v2.8.0 I
created an additional test patch just to ensure that we do not lose
backwards compatibility.
Oh, are you saying QEMU on s390 returns default-cpu-type in
query-machines reply even with v4.1.0 and older? I don't see it in our
test replies from anything older then v4.2.0.
Anyway, libvirt should not set the default CPU with QEMU 4.1.0 and if it
does, I have a bug in my patches :-)
qemuDomainDefSetDefaultCPU should not be called at all on QEMU 4.1.0 or
older. The check for cpu-model support in this function is an additional
check and this support should not be sufficient for the default CPU to
be filled in.
Jirka
4:06 a.m.
On 11/19/19 3:02 PM, Jiri Denemark wrote:
On Mon, Nov 18, 2019 at 18:34:36 +0100, Boris Fiuczynski wrote:
>
>
> On 11/15/19 4:14 PM, Jiri Denemark wrote:
>> On Fri, Nov 15, 2019 at 15:55:04 +0100, Christian Borntraeger wrote:
>>>
>>>
>>> On 15.11.19 15:47, Jiri Denemark wrote:
>>>> On Fri, Nov 15, 2019 at 15:12:18 +0100, Boris Fiuczynski wrote:
>>>>> Just a heads up.
>>>>> After installing libvirt rpms of this branch all my existing kvm
s390
>>>>> domains ended up with
>>>>>
>>>>> <cpu mode='custom' match='exact'
check='none'>
>>>>> <model fallback='forbid'>qemu</model>
>>>>> </cpu>
>>>>>
>>>>> Newly defined domains without specified cpu do so as well.
>>>>
>>>> Unless the domains are all TCG, it seems your QEMU is too old. You need
>>>> a fairly recent one which contains commit v4.1.0-1683-gde60a92ea7
>>>> (s390x/kvm: Set default cpu model for all machine classes)
>>>>
>>>> I the domains all use KVM and you have new enough QEMU, there might be a
>>>> bug somewhere. Which should not happen :=)
>>>
>>> So shouldnt libvirt fence this rework (add default model) to qemu 4.2 and
newer?
>>
>> Libvirt does all this only if query-machines returns default-cpu-type,
>> which is introduced in 4.2. But since it was introduced earlier, anyone
>> using qemu from git between the two commits will see this behavior.
>> Somewhat similar thing will happen on ppc64, but even with the current
>> master. Everything should be OK once QEMU 4.2.0 final release is used,
>> though (since it will contain all required patches).
>>
>> Jirka
>>
>
> I tested with a newer version of qemu and it worked as you outlined.
> After that I also tested with qemu v4.1.0. I was a bit surprised at
> first that a default host-model cpu was generated since I though it
> would only be done when the qemu has the commit your specified above.
> After reading your patch 4 the generation is tied to the cpu-model
> support in qemu. Since this became available on s390 with qemu v2.8.0 I
> created an additional test patch just to ensure that we do not lose
> backwards compatibility.
Oh, are you saying QEMU on s390 returns default-cpu-type in
query-machines reply even with v4.1.0 and older? I don't see it in our
test replies from anything older then v4.2.0.
Anyway, libvirt should not set the default CPU with QEMU 4.1.0 and if it
does, I have a bug in my patches :-)
No you don't have a bug in your patches. I
had a bug in my tests. :-(
It works as you outlined. The cpu-model is set if QEMU 4.2.0 is used but
not for 4.1.0 and older.
qemuDomainDefSetDefaultCPU should not be called at all on QEMU 4.1.0 or
older. The check for cpu-model support in this function is an additional
check and this support should not be sufficient for the default CPU to
be filled in.
Jirka
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
5:22 a.m.
On Thu, Nov 14, 2019 at 04:44:54PM +0100, Jiri Denemark wrote:
On s390 machines host-passthrough and host-model CPUs result in the
same
guest ABI (with QEMU new enough to be able to tell us what "host" CPU is
expanded to, which was implemented around 2.9.0). So instead of using
host-passthrough CPU when there's no CPU specified in a domain XML we
can safely use host-model and benefit from CPU compatibility checks
during migration, snapshot restore and similar operations.
This series applies on top of "qemu: Store default CPU in domain XML"
which is already acked, but it's waiting for a QEMU patch to be applied
to 4.2.0.
You can fetch both series at once using
git fetch https://gitlab.com/jirkade/libvirt cpu-default-type
Jiri Denemark (4):
cpu_conf: Fix default value for CPU match attribute
cpu_conf: Don't format empty model for host-model CPUs
cpu_s390: Don't check match attribute for host-model CPUs
qemu: Use host-model CPU on s390 by default
src/conf/cpu_conf.c | 25 ++++----------
src/conf/cpu_conf.h | 2 +-
src/cpu/cpu_s390.c | 18 +++++-----
src/qemu/qemu_domain.c | 33 ++++++++++++-------
.../ppc64-host+guest-compat-none.xml | 4 +--
...t-cpu-kvm-ccw-virtio-4.2.s390x-latest.args | 4 ++-
.../cpu-check-default-partial.xml | 4 +--
.../cpu-host-model-features.xml | 1 -
...lt-cpu-kvm-ccw-virtio-4.2.s390x-latest.xml | 2 +-
9 files changed, 45 insertions(+), 48 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
4:55 p.m.
On 11/14/19 10:44 AM, Jiri Denemark wrote:
On s390 machines host-passthrough and host-model CPUs result in the
same
guest ABI (with QEMU new enough to be able to tell us what "host" CPU is
expanded to, which was implemented around 2.9.0). So instead of using
host-passthrough CPU when there's no CPU specified in a domain XML we
can safely use host-model and benefit from CPU compatibility checks
during migration, snapshot restore and similar operations.
This series applies on top of "qemu: Store default CPU in domain XML"
which is already acked, but it's waiting for a QEMU patch to be applied
to 4.2.0.
You can fetch both series at once using
git fetch https://gitlab.com/jirkade/libvirt cpu-default-type
Jiri Denemark (4):
cpu_conf: Fix default value for CPU match attribute
cpu_conf: Don't format empty model for host-model CPUs
cpu_s390: Don't check match attribute for host-model CPUs
qemu: Use host-model CPU on s390 by default
src/conf/cpu_conf.c | 25 ++++----------
src/conf/cpu_conf.h | 2 +-
src/cpu/cpu_s390.c | 18 +++++-----
src/qemu/qemu_domain.c | 33 ++++++++++++-------
.../ppc64-host+guest-compat-none.xml | 4 +--
...t-cpu-kvm-ccw-virtio-4.2.s390x-latest.args | 4 ++-
.../cpu-check-default-partial.xml | 4 +--
.../cpu-host-model-features.xml | 1 -
...lt-cpu-kvm-ccw-virtio-4.2.s390x-latest.xml | 2 +-
9 files changed, 45 insertions(+), 48 deletions(-)
Guest defined as expected. Once I start a guest, a dumpxml showed
the model name and associated features as expected. Was able to
successfully migrate the guest as well.
Much preferred over the old default host-passthrough way.
Built libvirt using the repo and branch you provided
Built QEMU using master 4.1.92 (v4.2.0-rc2-27-g65e05c82bd)
- guest used machine s390-ccw-virtio-4.2
Migrated between a weird mixed environment I'm chaotically working with :)
Perhaps the testing was a bit over-extensive, but it's good to make sure.
Tested-by: Collin Walling <walling(a)linux.ibm.com>
--
Respectfully,
- Collin Walling
1824
days inactive
1835
days old
13 comments
5 participants
participants (5)
-
Boris Fiuczynski -
Christian Borntraeger -
Collin Walling -
Jiri Denemark -
Ján Tomko