Looks good. Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> On 2/6/20 11:24 AM, Jiri Denemark wrote:

The usability of a specific CPU mode may depend on machine type, let's prepare for this by passing it to virQEMUCapsIsCPUModeSupported.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 12 ++++++++---- src/qemu/qemu_capabilities.h | 3 ++- src/qemu/qemu_domain.c | 3 ++- src/qemu/qemu_process.c | 2 +- 4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index dfe7d48550..162e49e2d4 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2279,7 +2279,8 @@ bool virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps, virArch hostarch, virDomainVirtType type, - virCPUMode mode) + virCPUMode mode, + const char *machineType G_GNUC_UNUSED) { qemuMonitorCPUDefsPtr cpus;

@@ -5644,18 +5645,21 @@ virQEMUCapsFillDomainCPUCaps(virQEMUCapsPtr qemuCaps, virDomainCapsPtr domCaps) { if (virQEMUCapsIsCPUModeSupported(qemuCaps, hostarch, domCaps->virttype, - VIR_CPU_MODE_HOST_PASSTHROUGH)) + VIR_CPU_MODE_HOST_PASSTHROUGH, + domCaps->machine)) domCaps->cpu.hostPassthrough = true;

if (virQEMUCapsIsCPUModeSupported(qemuCaps, hostarch, domCaps->virttype, - VIR_CPU_MODE_HOST_MODEL)) { + VIR_CPU_MODE_HOST_MODEL, + domCaps->machine)) { virCPUDefPtr cpu = virQEMUCapsGetHostModel(qemuCaps, domCaps->virttype, VIR_QEMU_CAPS_HOST_CPU_REPORTED); domCaps->cpu.hostModel = virCPUDefCopy(cpu); }

if (virQEMUCapsIsCPUModeSupported(qemuCaps, hostarch, domCaps->virttype, - VIR_CPU_MODE_CUSTOM)) { + VIR_CPU_MODE_CUSTOM, + domCaps->machine)) { const char *blacklist[] = { "host", NULL }; VIR_AUTOSTRINGLIST models = NULL;

diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index b97c11ee1d..2473e64654 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -629,7 +629,8 @@ bool virQEMUCapsIsVirtTypeSupported(virQEMUCapsPtr qemuCaps, bool virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps, virArch hostarch, virDomainVirtType type, - virCPUMode mode); + virCPUMode mode, + const char *machineType); const char *virQEMUCapsGetCanonicalMachine(virQEMUCapsPtr qemuCaps, virDomainVirtType virtType, const char *name); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 851f750bd7..1b4825a539 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4600,7 +4600,8 @@ qemuDomainDefSetDefaultCPU(virDomainDefPtr def, if (STREQ(model, "host")) { if (ARCH_IS_S390(def->os.arch) && virQEMUCapsIsCPUModeSupported(qemuCaps, hostarch, def->virtType, - VIR_CPU_MODE_HOST_MODEL)) { + VIR_CPU_MODE_HOST_MODEL, + def->os.machine)) { def->cpu->mode = VIR_CPU_MODE_HOST_MODEL; } else { def->cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ddcc763cfd..e3df75d281 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6022,7 +6022,7 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def, }

if (!virQEMUCapsIsCPUModeSupported(qemuCaps, hostarch, def->virtType, - def->cpu->mode)) { + def->cpu->mode, def->os.machine)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("CPU mode '%s' for %s %s domain on %s host is not " "supported by hypervisor"),

-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

Would it make sense to test for cpu mode host-model to remain with machine='s390-ccw-virtio-2.8' as well? Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> On 2/6/20 11:24 AM, Jiri Denemark wrote:

Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- ...t-cpu-kvm-ccw-virtio-2.7.s390x-latest.args | 34 +++++++++++++++++++ .../s390-default-cpu-kvm-ccw-virtio-2.7.xml | 16 +++++++++ ...t-cpu-tcg-ccw-virtio-2.7.s390x-latest.args | 32 +++++++++++++++++ .../s390-default-cpu-tcg-ccw-virtio-2.7.xml | 16 +++++++++ tests/qemuxml2argvtest.c | 2 ++ ...lt-cpu-kvm-ccw-virtio-2.7.s390x-latest.xml | 24 +++++++++++++ ...lt-cpu-tcg-ccw-virtio-2.7.s390x-latest.xml | 26 ++++++++++++++ tests/qemuxml2xmltest.c | 2 ++ 8 files changed, 152 insertions(+) create mode 100644 tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-latest.args create mode 100644 tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-2.7.xml create mode 100644 tests/qemuxml2argvdata/s390-default-cpu-tcg-ccw-virtio-2.7.s390x-latest.args create mode 100644 tests/qemuxml2argvdata/s390-default-cpu-tcg-ccw-virtio-2.7.xml create mode 100644 tests/qemuxml2xmloutdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-latest.xml create mode 100644 tests/qemuxml2xmloutdata/s390-default-cpu-tcg-ccw-virtio-2.7.s390x-latest.xml

diff --git a/tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-latest.args b/tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-latest.args new file mode 100644 index 0000000000..8c25a01e74 --- /dev/null +++ b/tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-latest.args @@ -0,0 +1,34 @@ +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 \ +-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 \ +-uuid 9aa4b45c-b9dd-45ef-91fe-862b27b4231f \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,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 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-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..a3c1804f57 --- /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-2.7'>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/qemuxml2argvdata/s390-default-cpu-tcg-ccw-virtio-2.7.s390x-latest.args b/tests/qemuxml2argvdata/s390-default-cpu-tcg-ccw-virtio-2.7.s390x-latest.args new file mode 100644 index 0000000000..d30c30f648 --- /dev/null +++ b/tests/qemuxml2argvdata/s390-default-cpu-tcg-ccw-virtio-2.7.s390x-latest.args @@ -0,0 +1,32 @@ +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=tcg,usb=off,dump-guest-core=off \ +-cpu qemu \ +-m 256 \ +-overcommit mem-lock=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,fd=1729,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 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/s390-default-cpu-tcg-ccw-virtio-2.7.xml b/tests/qemuxml2argvdata/s390-default-cpu-tcg-ccw-virtio-2.7.xml new file mode 100644 index 0000000000..3451e9d81f --- /dev/null +++ b/tests/qemuxml2argvdata/s390-default-cpu-tcg-ccw-virtio-2.7.xml @@ -0,0 +1,16 @@ +<domain type='qemu'> + <name>test</name> + <uuid>9aa4b45c-b9dd-45ef-91fe-862b27b4231f</uuid> + <memory>262144</memory> + <currentMemory>262144</currentMemory> + <os> + <type arch='s390x' machine='s390-ccw-virtio-2.7'>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 5691317bad..028364a06c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -3093,6 +3093,8 @@ 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_LATEST("s390-default-cpu-kvm-ccw-virtio-2.7", "s390x"); + DO_TEST_CAPS_ARCH_LATEST("s390-default-cpu-tcg-ccw-virtio-2.7", "s390x"); 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-latest.xml b/tests/qemuxml2xmloutdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-latest.xml new file mode 100644 index 0000000000..56fd22b6e5 --- /dev/null +++ b/tests/qemuxml2xmloutdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-latest.xml @@ -0,0 +1,24 @@ +<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> + <cpu mode='host-model' check='partial'/> + <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/qemuxml2xmloutdata/s390-default-cpu-tcg-ccw-virtio-2.7.s390x-latest.xml b/tests/qemuxml2xmloutdata/s390-default-cpu-tcg-ccw-virtio-2.7.s390x-latest.xml new file mode 100644 index 0000000000..5048b09e15 --- /dev/null +++ b/tests/qemuxml2xmloutdata/s390-default-cpu-tcg-ccw-virtio-2.7.s390x-latest.xml @@ -0,0 +1,26 @@ +<domain type='qemu'> + <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> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu</model> + </cpu> + <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 fa238ec339..ecd12c3d30 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1463,6 +1463,8 @@ 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_LATEST("s390-default-cpu-kvm-ccw-virtio-2.7", "s390x"); + DO_TEST_CAPS_ARCH_LATEST("s390-default-cpu-tcg-ccw-virtio-2.7", "s390x"); 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");

-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

It does what it is supposed to do even so version checking is not really nice but not spending the effort to design a new qom interface is OK. @Christian Ehrhardt: I guess you need to add the ubuntu effected machine type(s) to s390HostPassthroughOnlyMachines as well as add an adjusted test like in patch 2 of this series for ubuntu. Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> On 2/6/20 11:24 AM, Jiri Denemark wrote:

Starting a KVM domain on s390 with old machine type (such as s390-ccw-virtio-2.5) and without any guest CPU model configured fails with

CPU models are not available: KVM doesn't support CPU models

QEMU error. This is cause by libvirt using host-model CPU as the default CPU based on QEMU reporting "host" CPU model as being the default one (see commit v5.9.0-402-g24d8202294: qemu: Use host-model CPU on s390 by default). However, even though both QEMU and KVM support CPU models on s390 and QEMU can give us the host-model CPU, we can't use it with old machine types which only support -cpu host.

https://bugzilla.redhat.com/show_bug.cgi?id=1795651

Reported-by: Christian Ehrhardt <paelzer@gmail.com> Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 19 ++++++++++++++++++- ...t-cpu-kvm-ccw-virtio-2.7.s390x-latest.args | 4 +--- ...lt-cpu-kvm-ccw-virtio-2.7.s390x-latest.xml | 2 +- 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 162e49e2d4..dd2311cfa9 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2274,16 +2274,33 @@ virQEMUCapsIsVirtTypeSupported(virQEMUCapsPtr qemuCaps, return false; }

+const char *s390HostPassthroughOnlyMachines[] = { + "s390-ccw-virtio-2.4", + "s390-ccw-virtio-2.5", + "s390-ccw-virtio-2.6", + "s390-ccw-virtio-2.7", + NULL +};

bool virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps, virArch hostarch, virDomainVirtType type, virCPUMode mode, - const char *machineType G_GNUC_UNUSED) + const char *machineType) { qemuMonitorCPUDefsPtr cpus;

+ /* CPU models (except for "host") are not supported by QEMU for on s390 + * KVM domains with old machine types regardless on QEMU version. */ + if (ARCH_IS_S390(qemuCaps->arch) && + type == VIR_DOMAIN_VIRT_KVM && + mode != VIR_CPU_MODE_HOST_PASSTHROUGH && + machineType && + g_strv_contains(s390HostPassthroughOnlyMachines, machineType)) { + return false; + } + switch (mode) { case VIR_CPU_MODE_HOST_PASSTHROUGH: return type == VIR_DOMAIN_VIRT_KVM && diff --git a/tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-latest.args b/tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-latest.args index 8c25a01e74..0c2567df6c 100644 --- a/tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-latest.args +++ b/tests/qemuxml2argvdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-latest.args @@ -13,9 +13,7 @@ QEMU_AUDIO_DRV=none \ -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 \ --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 \ +-cpu host \ -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-2.7.s390x-latest.xml b/tests/qemuxml2xmloutdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-latest.xml index 56fd22b6e5..8799584c11 100644 --- a/tests/qemuxml2xmloutdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-latest.xml +++ b/tests/qemuxml2xmloutdata/s390-default-cpu-kvm-ccw-virtio-2.7.s390x-latest.xml @@ -8,7 +8,7 @@ <type arch='s390x' machine='s390-ccw-virtio-2.7'>hvm</type> <boot dev='hd'/> </os> - <cpu mode='host-model' check='partial'/> + <cpu mode='host-passthrough' check='none'/> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot>

-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294