[libvirt] [PATCH v2 0/3] Add panic device support for S390
S390 architecture inherently provides a crash detection capability which has not been reflected in the domain xml. This series adds an s390 model to the panic device which does not allow an address to be specified and is by default created on S390 guests unless already provided. v3: Grouped default panic device creation and required test code changes into one patch. v2: Added an s390 model instead of using default model for toleration. Boris Fiuczynski (3): qemu: add panic device support for S390 qemu: add default panic device to S390 guests tests: add tests for panic device model s390 docs/formatdomain.html.in | 7 +++++- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 21 +++++++++++++++- src/qemu/qemu_domain.c | 9 ++++++- .../qemuargv2xml-machine-aeskeywrap-off-argv.xml | 1 + .../qemuargv2xml-machine-aeskeywrap-on-argv.xml | 1 + .../qemuargv2xml-machine-deakeywrap-off-argv.xml | 1 + .../qemuargv2xml-machine-deakeywrap-on-argv.xml | 1 + .../qemuargv2xml-machine-keywrap-none-argv.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-panic-s390.xml | 22 ++++++++++++++++ .../qemuxml2argv-s390-panic-address.xml | 26 +++++++++++++++++++ .../qemuxml2argv-s390-panic-missing.args | 25 +++++++++++++++++++ .../qemuxml2argv-s390-panic-missing.xml | 23 +++++++++++++++++ .../qemuxml2argv-s390-panic-no-address.args | 25 +++++++++++++++++++ .../qemuxml2argv-s390-panic-no-address.xml | 24 ++++++++++++++++++ tests/qemuxml2argvtest.c | 11 +++++++- .../qemuxml2xmlout-iothreads-disk-virtio-ccw.xml | 1 + .../qemuxml2xmlout-panic-s390.xml | 28 +++++++++++++++++++++ .../qemuxml2xmlout-s390-defaultconsole.xml | 1 + .../qemuxml2xmlout-s390-panic-missing.xml | 29 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 4 +++ 23 files changed, 261 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-s390.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-address.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-missing.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-missing.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-no-address.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-no-address.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-s390.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-panic-missing.xml -- 2.3.0
If a panic device is being defined without a model in a domain the default value is always overwritten with model ISA. An ISA bus does not exist on S390 and therefore specifying a panic device results in an unsupported configuration. Since the S390 architecture inherently provides a crash detection capability the panic device should be defined in the domain xml. This patch adds an s390 panic device model and prevents setting a device address on it. Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com> --- docs/formatdomain.html.in | 7 ++++++- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 21 ++++++++++++++++++++- src/qemu/qemu_domain.c | 2 ++ 6 files changed, 32 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c2955eb..10c27fb 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6242,6 +6242,9 @@ qemu-kvm -net nic,model=? /dev/null For pSeries guests, this feature is always enabled since it's implemented by the guest firmware, thus libvirt automatically adds the <code>panic</code> element to the domain XML. + For S390 guests, this feature is always enabled since it's an + integral part of the S390 architecture, thus libvirt automatically + adds the <code>panic</code> element to the domain XML. </p> <p> Example: usage of panic configuration @@ -6269,6 +6272,8 @@ qemu-kvm -net nic,model=? /dev/null <li>'pseries' — default and valid only for pSeries guests.</li> <li>'hyperv' — for Hyper-V crash CPU feature. <span class="since">Since 1.3.0, QEMU and KVM only</span></li> + <li>'s390' — default for S390 guests. + <span class="since">Since 1.3.4</span></li> </ul> </dd> <dt><code>address</code></dt> @@ -6276,7 +6281,7 @@ qemu-kvm -net nic,model=? /dev/null <p> address of panic. The default ioport is 0x505. Most users don't need to specify an address, and doing so is forbidden - altogether for pseries and hyperv models. + altogether for s390, pseries and hyperv models. </p> </dd> </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index fa54526..03ca4fe 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5507,6 +5507,7 @@ <value>isa</value> <value>pseries</value> <value>hyperv</value> + <value>s390</value> </choice> </attribute> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 446fbc5..a6b2b90 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -521,7 +521,8 @@ VIR_ENUM_IMPL(virDomainPanicModel, VIR_DOMAIN_PANIC_MODEL_LAST, "default", "isa", "pseries", - "hyperv") + "hyperv", + "s390") VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST, "vga", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 443a1d7..936bea1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2072,6 +2072,7 @@ typedef enum { VIR_DOMAIN_PANIC_MODEL_ISA, VIR_DOMAIN_PANIC_MODEL_PSERIES, VIR_DOMAIN_PANIC_MODEL_HYPERV, + VIR_DOMAIN_PANIC_MODEL_S390, VIR_DOMAIN_PANIC_MODEL_LAST } virDomainPanicModel; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index da99e5c..028a469 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8991,6 +8991,25 @@ qemuBuildPanicCommandLine(virCommandPtr cmd, for (i = 0; i < def->npanics; i++) { switch ((virDomainPanicModel) def->panics[i]->model) { + case VIR_DOMAIN_PANIC_MODEL_S390: + /* For s390 guests, the hardware provides the same + * functionality as the pvpanic device. The address + * cannot be configured by the user */ + if (!ARCH_IS_S390(def->os.arch)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only S390 guests support " + "panic device of model 's390'")); + return -1; + } + if (def->panics[i]->info.type != + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("setting the panic device address is not " + "supported for model 's390'")); + return -1; + } + break; + case VIR_DOMAIN_PANIC_MODEL_HYPERV: /* Panic with model 'hyperv' is not a device, it should * be configured in cpu commandline. The address @@ -9034,7 +9053,7 @@ qemuBuildPanicCommandLine(virCommandPtr cmd, if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("the QEMU binary does not support the " - "panic device")); + "ISA panic device")); return -1; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 5d54fff..d3d7c11 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1883,6 +1883,8 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) dev->data.panic->model = VIR_DOMAIN_PANIC_MODEL_PSERIES; + else if (ARCH_IS_S390(def->os.arch)) + dev->data.panic->model = VIR_DOMAIN_PANIC_MODEL_S390; else dev->data.panic->model = VIR_DOMAIN_PANIC_MODEL_ISA; } -- 2.3.0
On Fri, 2016-04-15 at 10:20 +0200, Boris Fiuczynski wrote:
If a panic device is being defined without a model in a domain the default value is always overwritten with model ISA. An ISA bus does not exist on S390 and therefore specifying a panic device results in an unsupported configuration. Since the S390 architecture inherently provides a crash detection capability the panic device should be defined in the domain xml. This patch adds an s390 panic device model and prevents setting a device address on it. Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com> --- docs/formatdomain.html.in | 7 ++++++- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 21 ++++++++++++++++++++- src/qemu/qemu_domain.c | 2 ++ 6 files changed, 32 insertions(+), 3 deletions(-)
Sorry for taking so long to reply.
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c2955eb..10c27fb 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6242,6 +6242,9 @@ qemu-kvm -net nic,model=? /dev/null For pSeries guests, this feature is always enabled since it's implemented by the guest firmware, thus libvirt automatically adds the <code>panic</code> element to the domain XML. + For S390 guests, this feature is always enabled since it's an + integral part of the S390 architecture, thus libvirt automatically + adds the <code>panic</code> element to the domain XML.
Shouldn't that be "S/390" instead of "S390"? Just like we use "pSeries". Same for the commit message and in the remaining patches. This paragraph should also be merged with the existing one, as they're basically identical. Tweak the existing text as needed.
@@ -9034,7 +9053,7 @@ qemuBuildPanicCommandLine(virCommandPtr cmd, if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("the QEMU binary does not support the " - "panic device")); + "ISA panic device"));
This should be a separate commit. A very tiny one ;) Everything else looks good. -- Andrea Bolognani Software Engineer - Virtualization Team
On 04/27/2016 05:02 PM, Andrea Bolognani wrote:
On Fri, 2016-04-15 at 10:20 +0200, Boris Fiuczynski wrote:
If a panic device is being defined without a model in a domain the default value is always overwritten with model ISA. An ISA bus does not exist on S390 and therefore specifying a panic device results in an unsupported configuration. Since the S390 architecture inherently provides a crash detection capability the panic device should be defined in the domain xml.
This patch adds an s390 panic device model and prevents setting a device address on it.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com> --- docs/formatdomain.html.in | 7 ++++++- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 21 ++++++++++++++++++++- src/qemu/qemu_domain.c | 2 ++ 6 files changed, 32 insertions(+), 3 deletions(-)
Sorry for taking so long to reply.
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c2955eb..10c27fb 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6242,6 +6242,9 @@ qemu-kvm -net nic,model=? /dev/null For pSeries guests, this feature is always enabled since it's implemented by the guest firmware, thus libvirt automatically adds the <code>panic</code> element to the domain XML. + For S390 guests, this feature is always enabled since it's an + integral part of the S390 architecture, thus libvirt automatically + adds the <code>panic</code> element to the domain XML.
Shouldn't that be "S/390" instead of "S390"? I rather stick with the generic term S390 and fix the mismatching occurrences in the document accordingly in a "tiny" separate patch.
Just like we use "pSeries". Same for the commit message and in the remaining patches.
This paragraph should also be merged with the existing one, as they're basically identical. Tweak the existing text as needed.
OK
@@ -9034,7 +9053,7 @@ qemuBuildPanicCommandLine(virCommandPtr cmd, if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("the QEMU binary does not support the " - "panic device")); + "ISA panic device"));
This should be a separate commit. A very tiny one ;)
Sure
Everything else looks good.
-- Andrea Bolognani Software Engineer - Virtualization Team
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
This patch adds by default a panic device with model s390 to S390 guests. Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 7 ++++++- .../qemuargv2xmldata/qemuargv2xml-machine-aeskeywrap-off-argv.xml | 1 + tests/qemuargv2xmldata/qemuargv2xml-machine-aeskeywrap-on-argv.xml | 1 + .../qemuargv2xmldata/qemuargv2xml-machine-deakeywrap-off-argv.xml | 1 + tests/qemuargv2xmldata/qemuargv2xml-machine-deakeywrap-on-argv.xml | 1 + tests/qemuargv2xmldata/qemuargv2xml-machine-keywrap-none-argv.xml | 1 + .../qemuxml2xmlout-iothreads-disk-virtio-ccw.xml | 1 + tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml | 1 + 8 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d3d7c11..a83b43c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1505,9 +1505,11 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, break; case VIR_ARCH_S390: addDefaultUSB = false; + addPanicDevice = true; break; case VIR_ARCH_S390X: addDefaultUSB = false; + addPanicDevice = true; break; case VIR_ARCH_SPARC: @@ -1586,7 +1588,10 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, size_t j; for (j = 0; j < def->npanics; j++) { if (def->panics[j]->model == VIR_DOMAIN_PANIC_MODEL_DEFAULT || - def->panics[j]->model == VIR_DOMAIN_PANIC_MODEL_PSERIES) + (ARCH_IS_PPC64(def->os.arch) && + def->panics[j]->model == VIR_DOMAIN_PANIC_MODEL_PSERIES) || + (ARCH_IS_S390(def->os.arch) && + def->panics[j]->model == VIR_DOMAIN_PANIC_MODEL_S390)) break; } diff --git a/tests/qemuargv2xmldata/qemuargv2xml-machine-aeskeywrap-off-argv.xml b/tests/qemuargv2xmldata/qemuargv2xml-machine-aeskeywrap-off-argv.xml index 1658e12..7ccdc67 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-machine-aeskeywrap-off-argv.xml +++ b/tests/qemuargv2xmldata/qemuargv2xml-machine-aeskeywrap-off-argv.xml @@ -20,6 +20,7 @@ <target dev='vda' bus='virtio'/> </disk> <memballoon model='none'/> + <panic model='s390'/> </devices> <keywrap> <cipher name='aes' state='off'/> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-machine-aeskeywrap-on-argv.xml b/tests/qemuargv2xmldata/qemuargv2xml-machine-aeskeywrap-on-argv.xml index 3d676aa..a02523d 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-machine-aeskeywrap-on-argv.xml +++ b/tests/qemuargv2xmldata/qemuargv2xml-machine-aeskeywrap-on-argv.xml @@ -20,6 +20,7 @@ <target dev='vda' bus='virtio'/> </disk> <memballoon model='none'/> + <panic model='s390'/> </devices> <keywrap> <cipher name='aes' state='on'/> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-machine-deakeywrap-off-argv.xml b/tests/qemuargv2xmldata/qemuargv2xml-machine-deakeywrap-off-argv.xml index f3bc8af..7f0c871 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-machine-deakeywrap-off-argv.xml +++ b/tests/qemuargv2xmldata/qemuargv2xml-machine-deakeywrap-off-argv.xml @@ -20,6 +20,7 @@ <target dev='vda' bus='virtio'/> </disk> <memballoon model='none'/> + <panic model='s390'/> </devices> <keywrap> <cipher name='dea' state='off'/> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-machine-deakeywrap-on-argv.xml b/tests/qemuargv2xmldata/qemuargv2xml-machine-deakeywrap-on-argv.xml index 1e0660f..d4721dc 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-machine-deakeywrap-on-argv.xml +++ b/tests/qemuargv2xmldata/qemuargv2xml-machine-deakeywrap-on-argv.xml @@ -20,6 +20,7 @@ <target dev='vda' bus='virtio'/> </disk> <memballoon model='none'/> + <panic model='s390'/> </devices> <keywrap> <cipher name='dea' state='on'/> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-machine-keywrap-none-argv.xml b/tests/qemuargv2xmldata/qemuargv2xml-machine-keywrap-none-argv.xml index 6acee07..5483040 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-machine-keywrap-none-argv.xml +++ b/tests/qemuargv2xmldata/qemuargv2xml-machine-keywrap-none-argv.xml @@ -20,5 +20,6 @@ <target dev='vda' bus='virtio'/> </disk> <memballoon model='none'/> + <panic model='s390'/> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-iothreads-disk-virtio-ccw.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-iothreads-disk-virtio-ccw.xml index b6f7d30..f6d1039 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-iothreads-disk-virtio-ccw.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-iothreads-disk-virtio-ccw.xml @@ -31,5 +31,6 @@ <memballoon model='virtio'> <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x000a'/> </memballoon> + <panic model='s390'/> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml index 3f349b2..42ad4e4 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml @@ -20,5 +20,6 @@ <target type='virtio' port='0'/> </console> <memballoon model='none'/> + <panic model='s390'/> </devices> </domain> -- 2.3.0
On Fri, 2016-04-15 at 10:20 +0200, Boris Fiuczynski wrote:
This patch adds by default a panic device with model s390 to S390 guests. Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 7 ++++++- .../qemuargv2xmldata/qemuargv2xml-machine-aeskeywrap-off-argv.xml | 1 + tests/qemuargv2xmldata/qemuargv2xml-machine-aeskeywrap-on-argv.xml | 1 + .../qemuargv2xmldata/qemuargv2xml-machine-deakeywrap-off-argv.xml | 1 + tests/qemuargv2xmldata/qemuargv2xml-machine-deakeywrap-on-argv.xml | 1 + tests/qemuargv2xmldata/qemuargv2xml-machine-keywrap-none-argv.xml | 1 + .../qemuxml2xmlout-iothreads-disk-virtio-ccw.xml | 1 + tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml | 1 + 8 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d3d7c11..a83b43c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1505,9 +1505,11 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, break; case VIR_ARCH_S390: addDefaultUSB = false; + addPanicDevice = true; break; case VIR_ARCH_S390X: addDefaultUSB = false; + addPanicDevice = true; break;
Maybe merge these two cases into one, just like eg. SPARC and SPARC64 below? As a separate commit, of course.
case VIR_ARCH_SPARC: @@ -1586,7 +1588,10 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, size_t j; for (j = 0; j < def->npanics; j++) { if (def->panics[j]->model == VIR_DOMAIN_PANIC_MODEL_DEFAULT || - def->panics[j]->model == VIR_DOMAIN_PANIC_MODEL_PSERIES) + (ARCH_IS_PPC64(def->os.arch) && + def->panics[j]->model == VIR_DOMAIN_PANIC_MODEL_PSERIES) || + (ARCH_IS_S390(def->os.arch) && + def->panics[j]->model == VIR_DOMAIN_PANIC_MODEL_S390)) break; }
The checks on os.arch are kinda redundant - we reject panic models 'pseries' and 's390' if the arch is wrong - but being explicit about that can't possibly hurt :) ACK -- Andrea Bolognani Software Engineer - Virtualization Team
On 04/27/2016 05:54 PM, Andrea Bolognani wrote:
On Fri, 2016-04-15 at 10:20 +0200, Boris Fiuczynski wrote:
This patch adds by default a panic device with model s390 to S390 guests.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 7 ++++++- .../qemuargv2xmldata/qemuargv2xml-machine-aeskeywrap-off-argv.xml | 1 + tests/qemuargv2xmldata/qemuargv2xml-machine-aeskeywrap-on-argv.xml | 1 + .../qemuargv2xmldata/qemuargv2xml-machine-deakeywrap-off-argv.xml | 1 + tests/qemuargv2xmldata/qemuargv2xml-machine-deakeywrap-on-argv.xml | 1 + tests/qemuargv2xmldata/qemuargv2xml-machine-keywrap-none-argv.xml | 1 + .../qemuxml2xmlout-iothreads-disk-virtio-ccw.xml | 1 + tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml | 1 + 8 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d3d7c11..a83b43c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1505,9 +1505,11 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, break; case VIR_ARCH_S390: addDefaultUSB = false; + addPanicDevice = true; break; case VIR_ARCH_S390X: addDefaultUSB = false; + addPanicDevice = true; break;
Maybe merge these two cases into one, just like eg. SPARC and SPARC64 below? As a separate commit, of course. I will merge it.
case VIR_ARCH_SPARC: @@ -1586,7 +1588,10 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, size_t j; for (j = 0; j < def->npanics; j++) { if (def->panics[j]->model == VIR_DOMAIN_PANIC_MODEL_DEFAULT || - def->panics[j]->model == VIR_DOMAIN_PANIC_MODEL_PSERIES) + (ARCH_IS_PPC64(def->os.arch) && + def->panics[j]->model == VIR_DOMAIN_PANIC_MODEL_PSERIES) || + (ARCH_IS_S390(def->os.arch) && + def->panics[j]->model == VIR_DOMAIN_PANIC_MODEL_S390)) break; }
The checks on os.arch are kinda redundant - we reject panic models 'pseries' and 's390' if the arch is wrong - but being explicit about that can't possibly hurt :)
:)
ACK
-- Andrea Bolognani Software Engineer - Virtualization Team
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Adding tests for the panic device model s390. Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- tests/qemuxml2argvdata/qemuxml2argv-panic-s390.xml | 22 ++++++++++++++++ .../qemuxml2argv-s390-panic-address.xml | 26 +++++++++++++++++++ .../qemuxml2argv-s390-panic-missing.args | 25 +++++++++++++++++++ .../qemuxml2argv-s390-panic-missing.xml | 23 +++++++++++++++++ .../qemuxml2argv-s390-panic-no-address.args | 25 +++++++++++++++++++ .../qemuxml2argv-s390-panic-no-address.xml | 24 ++++++++++++++++++ tests/qemuxml2argvtest.c | 11 +++++++- .../qemuxml2xmlout-panic-s390.xml | 28 +++++++++++++++++++++ .../qemuxml2xmlout-s390-panic-missing.xml | 29 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 4 +++ 10 files changed, 216 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-s390.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-address.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-missing.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-missing.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-no-address.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-no-address.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-s390.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-panic-missing.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-panic-s390.xml b/tests/qemuxml2argvdata/qemuxml2argv-panic-s390.xml new file mode 100644 index 0000000..432a04c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-panic-s390.xml @@ -0,0 +1,22 @@ +<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-kvm</emulator> + <controller type='virtio-serial' index='0'> + </controller> + <console type='pty'> + <target type='virtio'/> + </console> + <panic/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-panic-address.xml b/tests/qemuxml2argvdata/qemuxml2argv-s390-panic-address.xml new file mode 100644 index 0000000..94521d0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-panic-address.xml @@ -0,0 +1,26 @@ +<domain type='qemu'> + <name>QEMUGuest1</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> + <controller type='virtio-serial' index='0'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> + </controller> + <console type='pty'> + <target type='virtio'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0001'/> + </console> + <panic model='s390'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0005'/> + </panic> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-panic-missing.args b/tests/qemuxml2argvdata/qemuxml2argv-s390-panic-missing.args new file mode 100644 index 0000000..f2eec5b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-panic-missing.args @@ -0,0 +1,25 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-s390x \ +-name QEMUGuest1 \ +-S \ +-M s390-ccw-virtio \ +-m 256 \ +-smp 1 \ +-uuid 9aa4b45c-b9dd-45ef-91fe-862b27b4231f \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-device virtio-serial-ccw,id=virtio-serial0,devno=fe.0.0000 \ +-chardev pty,id=charconsole0 \ +-device virtconsole,chardev=charconsole0,id=console0 \ +-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0002 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-panic-missing.xml b/tests/qemuxml2argvdata/qemuxml2argv-s390-panic-missing.xml new file mode 100644 index 0000000..8955433 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-panic-missing.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>QEMUGuest1</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> + <controller type='virtio-serial' index='0'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> + </controller> + <console type='pty'> + <target type='virtio'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0001'/> + </console> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-panic-no-address.args b/tests/qemuxml2argvdata/qemuxml2argv-s390-panic-no-address.args new file mode 100644 index 0000000..f2eec5b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-panic-no-address.args @@ -0,0 +1,25 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-s390x \ +-name QEMUGuest1 \ +-S \ +-M s390-ccw-virtio \ +-m 256 \ +-smp 1 \ +-uuid 9aa4b45c-b9dd-45ef-91fe-862b27b4231f \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-device virtio-serial-ccw,id=virtio-serial0,devno=fe.0.0000 \ +-chardev pty,id=charconsole0 \ +-device virtconsole,chardev=charconsole0,id=console0 \ +-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0002 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-panic-no-address.xml b/tests/qemuxml2argvdata/qemuxml2argv-s390-panic-no-address.xml new file mode 100644 index 0000000..856bcbe --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-panic-no-address.xml @@ -0,0 +1,24 @@ +<domain type='qemu'> + <name>QEMUGuest1</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> + <controller type='virtio-serial' index='0'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> + </controller> + <console type='pty'> + <target type='virtio'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0001'/> + </console> + <panic model='s390'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index be74178..5d6b69c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1462,11 +1462,20 @@ mymain(void) QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_BOOTINDEX, QEMU_CAPS_VIRTIO_S390, QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM); - DO_TEST("s390-allow-bogus-usb-controller", QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_BOOTINDEX, QEMU_CAPS_VIRTIO_S390, QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM); + DO_TEST("s390-panic-no-address", + QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); + DO_TEST_FAILURE("s390-panic-address", + QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); + DO_TEST("s390-panic-missing", + QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); + DO_TEST("ppc-dtb", QEMU_CAPS_KVM, QEMU_CAPS_DTB); DO_TEST("ppce500-serial", QEMU_CAPS_KVM, QEMU_CAPS_CHARDEV); diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-s390.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-s390.xml new file mode 100644 index 0000000..12fc687 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-s390.xml @@ -0,0 +1,28 @@ +<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'>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-kvm</emulator> + <controller type='virtio-serial' index='0'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> + </controller> + <console type='pty'> + <target type='virtio' port='0'/> + </console> + <memballoon model='virtio'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0001'/> + </memballoon> + <panic model='s390'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-panic-missing.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-panic-missing.xml new file mode 100644 index 0000000..8864157 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-panic-missing.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</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'>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='virtio-serial' index='0'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> + </controller> + <console type='pty'> + <target type='virtio' port='0'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0001'/> + </console> + <memballoon model='virtio'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0002'/> + </memballoon> + <panic model='s390'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index f4093f1..4f3faee 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -686,6 +686,10 @@ mymain(void) DO_TEST_FULL("s390-defaultconsole", WHEN_ACTIVE, QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); + DO_TEST_FULL("panic-s390", WHEN_ACTIVE, + QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); + DO_TEST_FULL("s390-panic-missing", WHEN_ACTIVE, + QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); DO_TEST("pcihole64"); DO_TEST("pcihole64-gib"); -- 2.3.0
On Fri, 2016-04-15 at 10:20 +0200, Boris Fiuczynski wrote:
Adding tests for the panic device model s390.
The commit message doesn't add any information that the summary didn't contain already. Just drop it.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- tests/qemuxml2argvdata/qemuxml2argv-panic-s390.xml | 22 ++++++++++++++++ .../qemuxml2argv-s390-panic-address.xml | 26 +++++++++++++++++++ .../qemuxml2argv-s390-panic-missing.args | 25 +++++++++++++++++++ .../qemuxml2argv-s390-panic-missing.xml | 23 +++++++++++++++++ .../qemuxml2argv-s390-panic-no-address.args | 25 +++++++++++++++++++ .../qemuxml2argv-s390-panic-no-address.xml | 24 ++++++++++++++++++ tests/qemuxml2argvtest.c | 11 +++++++- .../qemuxml2xmlout-panic-s390.xml | 28 +++++++++++++++++++++ .../qemuxml2xmlout-s390-panic-missing.xml | 29 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 4 +++ 10 files changed, 216 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-s390.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-address.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-missing.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-missing.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-no-address.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-no-address.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-s390.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-panic-missing.xml
Please use "s390-panic" instead of "panic-s390" to be consistent with both the existing test cases and the ones you're adding yourself. You could test "s390-panic-no-address" in the xml2xml case as well. For all test cases, please try to use symbolic links whenever possible, eg. you should be able to take the output XML that contains <panic model='s390'/> and use it as input file for the same xml2xml test and for at least one of the xml2argv tests.
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index f4093f1..4f3faee 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -686,6 +686,10 @@ mymain(void) DO_TEST_FULL("s390-defaultconsole", WHEN_ACTIVE, QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); + DO_TEST_FULL("panic-s390", WHEN_ACTIVE, + QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); + DO_TEST_FULL("s390-panic-missing", WHEN_ACTIVE, + QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390);
I see you need QEMU_CAPS_VIRTIO_CCW and QEMU_CAPS_VIRTIO_S390, so you can't just use DO_TEST(). Still, no reason not to use WHEN_BOTH, right? I tried to use it and the tests still passed. Other s390 test cases could probably be switched to WHEN_BOTH in a follow-up commit. -- Andrea Bolognani Software Engineer - Virtualization Team
A polite ping. On 04/15/2016 10:20 AM, Boris Fiuczynski wrote:
S390 architecture inherently provides a crash detection capability which has not been reflected in the domain xml. This series adds an s390 model to the panic device which does not allow an address to be specified and is by default created on S390 guests unless already provided.
v3: Grouped default panic device creation and required test code changes into one patch.
v2: Added an s390 model instead of using default model for toleration.
Boris Fiuczynski (3): qemu: add panic device support for S390 qemu: add default panic device to S390 guests tests: add tests for panic device model s390
docs/formatdomain.html.in | 7 +++++- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 21 +++++++++++++++- src/qemu/qemu_domain.c | 9 ++++++- .../qemuargv2xml-machine-aeskeywrap-off-argv.xml | 1 + .../qemuargv2xml-machine-aeskeywrap-on-argv.xml | 1 + .../qemuargv2xml-machine-deakeywrap-off-argv.xml | 1 + .../qemuargv2xml-machine-deakeywrap-on-argv.xml | 1 + .../qemuargv2xml-machine-keywrap-none-argv.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-panic-s390.xml | 22 ++++++++++++++++ .../qemuxml2argv-s390-panic-address.xml | 26 +++++++++++++++++++ .../qemuxml2argv-s390-panic-missing.args | 25 +++++++++++++++++++ .../qemuxml2argv-s390-panic-missing.xml | 23 +++++++++++++++++ .../qemuxml2argv-s390-panic-no-address.args | 25 +++++++++++++++++++ .../qemuxml2argv-s390-panic-no-address.xml | 24 ++++++++++++++++++ tests/qemuxml2argvtest.c | 11 +++++++- .../qemuxml2xmlout-iothreads-disk-virtio-ccw.xml | 1 + .../qemuxml2xmlout-panic-s390.xml | 28 +++++++++++++++++++++ .../qemuxml2xmlout-s390-defaultconsole.xml | 1 + .../qemuxml2xmlout-s390-panic-missing.xml | 29 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 4 +++ 23 files changed, 261 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-s390.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-address.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-missing.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-missing.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-no-address.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-panic-no-address.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-s390.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-panic-missing.xml
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (2)
-
Andrea Bolognani -
Boris Fiuczynski