[libvirt] [PATCH v3 0/3] qemu: Deal with panic device on pSeries
Currently, having the <panic> element in a domain XML prevents it from starting on pSeries. The error message in this case is not very accurate, and the first commit improves it. Since the guest firmware provides the same features as the pvpanic device for pSeries guests, the element should be allowed. The second commit implements this change. It being part of the firmware, the <panic> element should actually always be present in the domain XML: the third commit makes sure this is the case. Andrea Bolognani (3): qemu: Improve error message for missing QEMU_CAPS_DEVICE_PANIC. qemu: Allow panic device for pSeries guests qemu: Automatically add <panic> element for pSeries guests. docs/formatdomain.html.in | 8 ++++- src/qemu/qemu_command.c | 34 ++++++++++++++++------ src/qemu/qemu_domain.c | 14 +++++++++ .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 1 + .../qemuxml2argv-pseries-nvram.xml | 1 + .../qemuxml2argv-pseries-panic-address.xml | 32 ++++++++++++++++++++ .../qemuxml2argv-pseries-panic-missing.args | 7 +++++ .../qemuxml2argv-pseries-panic-missing.xml | 29 ++++++++++++++++++ .../qemuxml2argv-pseries-panic-no-address.args | 7 +++++ .../qemuxml2argv-pseries-panic-no-address.xml | 30 +++++++++++++++++++ tests/qemuxml2argvtest.c | 6 ++++ .../qemuxml2xmlout-pseries-panic-missing.xml | 30 +++++++++++++++++++ tests/qemuxml2xmltest.c | 2 ++ 13 files changed, 191 insertions(+), 10 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-address.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml -- 2.1.0
--- src/qemu/qemu_command.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d8ce511..f39442b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10840,7 +10840,7 @@ qemuBuildCommandLine(virConnectPtr conn, } } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("your QEMU is too old to support pvpanic")); + _("QEMU does not support the panic device")); goto error; } } -- 2.1.0
The guest firmware provides the same functionality as the pvpanic device, which is not available in QEMU on pSeries, so the domain XML should be allowed to contain the <panic> element. On the other hand, unlike the pvpanic device, the guest firmware can't be configured, so report an error if an address has been provided in the XML. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1182388 --- docs/formatdomain.html.in | 3 +- src/qemu/qemu_command.c | 34 ++++++++++++++++------ .../qemuxml2argv-pseries-panic-address.xml | 32 ++++++++++++++++++++ .../qemuxml2argv-pseries-panic-no-address.args | 7 +++++ .../qemuxml2argv-pseries-panic-no-address.xml | 30 +++++++++++++++++++ tests/qemuxml2argvtest.c | 4 +++ tests/qemuxml2xmltest.c | 1 + 7 files changed, 101 insertions(+), 10 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-address.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6d58a40..7dd5fa9 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5960,7 +5960,8 @@ qemu-kvm -net nic,model=? /dev/null <dd> <p> address of panic. The default ioport is 0x505. Most users - don't need to specify an address. + don't need to specify an address, and doing so is forbidden + altogether for pSeries guests. </p> </dd> </dl> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f39442b..f74a392 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10824,24 +10824,40 @@ qemuBuildCommandLine(virConnectPtr conn, } if (def->panic) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) { - if (def->panic->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA) { + if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) { + /* For pSeries guests, the firmware provides the same + * functionality as the pvpanic device. The address + * cannot be configured by the user */ + if (def->panic->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("setting the panic device address is not " + "supported for pSeries guests")); + goto error; + } + } else { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("QEMU does not support the panic device")); + goto error; + } + + switch (def->panic->info.type) { + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA: virCommandAddArg(cmd, "-device"); virCommandAddArgFormat(cmd, "pvpanic,ioport=%d", def->panic->info.addr.isa.iobase); - } else if (def->panic->info.type == - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + break; + + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE: virCommandAddArgList(cmd, "-device", "pvpanic", NULL); - } else { + break; + + default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("panic is supported only " "with ISA address type")); goto error; } - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("QEMU does not support the panic device")); - goto error; } } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-address.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-address.xml new file mode 100644 index 0000000..e62ead8 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-address.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>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-ppc64</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <serial type='pty'> + <target port='0'/> + <address type='spapr-vio'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + <address type='spapr-vio'/> + </console> + <memballoon model='none'/> + <panic> + <address type='isa' iobase='0x505'/> + </panic> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.args new file mode 100644 index 0000000..30e4b43 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.args @@ -0,0 +1,7 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-ppc64 -S -M pseries -m 512 -smp 1 -nographic \ +-nodefconfig -nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -usb \ +-chardev pty,id=charserial0 \ +-device spapr-vty,chardev=charserial0,reg=0x30000000 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.xml new file mode 100644 index 0000000..9312975 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>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-ppc64</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <serial type='pty'> + <target port='0'/> + <address type='spapr-vio'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + <address type='spapr-vio'/> + </console> + <memballoon model='none'/> + <panic/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 958f786..f822670 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1365,6 +1365,10 @@ mymain(void) QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("pseries-cpu-le", QEMU_CAPS_KVM, QEMU_CAPS_CPU_HOST, QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + DO_TEST("pseries-panic-no-address", + QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + DO_TEST_FAILURE("pseries-panic-address", + QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("disk-ide-drive-split", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_IDE_CD); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 53bcc9f..c147795 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -537,6 +537,7 @@ mymain(void) DO_TEST("virtio-rng-egd"); DO_TEST("pseries-nvram"); + DO_TEST("pseries-panic-no-address"); /* These tests generate different XML */ DO_TEST_DIFFERENT("balloon-device-auto"); -- 2.1.0
The guest firmware provides the same functionality as the pvpanic device, and the relevant element should always be present in the domain XML to reflect this fact, so add it after parsing the definition if it wasn't there already. --- docs/formatdomain.html.in | 5 ++++ src/qemu/qemu_domain.c | 14 ++++++++++ .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 1 + .../qemuxml2argv-pseries-nvram.xml | 1 + .../qemuxml2argv-pseries-panic-missing.args | 7 +++++ .../qemuxml2argv-pseries-panic-missing.xml | 29 +++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ .../qemuxml2xmlout-pseries-panic-missing.xml | 30 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 9 files changed, 90 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7dd5fa9..bffa412 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5944,6 +5944,11 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">Since 1.2.1, QEMU and KVM only</span> </p> <p> + For pSeries guests, this feature is always enabled because it's + implemented by the guest firmware. libvirt will automatically add the + <code>panic</code> element to the domain XML to reflect this fact. + </p> + <p> Example: usage of panic configuration </p> <pre> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index db8554b..12a1d97 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -959,6 +959,7 @@ qemuDomainDefPostParse(virDomainDefPtr def, bool addDefaultMemballoon = true; bool addDefaultUSBKBD = false; bool addDefaultUSBMouse = false; + bool addPanicDevice = false; if (def->os.bootloader || def->os.bootloaderArgs) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -1011,6 +1012,11 @@ qemuDomainDefPostParse(virDomainDefPtr def, addPCIRoot = true; addDefaultUSBKBD = true; addDefaultUSBMouse = true; + /* For pSeries guests, the firmware provides the same + * functionality as the pvpanic device, so automatically + * add the definition if not already present */ + if (STRPREFIX(def->os.machine, "pseries")) + addPanicDevice = true; break; case VIR_ARCH_ALPHA: @@ -1093,6 +1099,14 @@ qemuDomainDefPostParse(virDomainDefPtr def, VIR_DOMAIN_INPUT_BUS_USB) < 0) return -1; + if (addPanicDevice && !def->panic) { + virDomainPanicDefPtr panic; + if (VIR_ALLOC(panic) < 0) + return -1; + + def->panic = panic; + } + return 0; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml index d9ae4af..3a96209 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml @@ -37,5 +37,6 @@ <model type='cirrus' vram='16384' heads='1'/> </video> <memballoon model='none'/> + <panic/> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml index 9703bd4..619186a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-nvram.xml @@ -20,5 +20,6 @@ <nvram> <address type='spapr-vio' reg='0x4000'/> </nvram> + <panic/> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.args new file mode 100644 index 0000000..30e4b43 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.args @@ -0,0 +1,7 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-ppc64 -S -M pseries -m 512 -smp 1 -nographic \ +-nodefconfig -nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -usb \ +-chardev pty,id=charserial0 \ +-device spapr-vty,chardev=charserial0,reg=0x30000000 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.xml new file mode 100644 index 0000000..8980847 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>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-ppc64</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <serial type='pty'> + <target port='0'/> + <address type='spapr-vio'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + <address type='spapr-vio'/> + </console> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index f822670..11e09ce 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1365,6 +1365,8 @@ mymain(void) QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("pseries-cpu-le", QEMU_CAPS_KVM, QEMU_CAPS_CPU_HOST, QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + DO_TEST("pseries-panic-missing", + QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("pseries-panic-no-address", QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST_FAILURE("pseries-panic-address", diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml new file mode 100644 index 0000000..9312975 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>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-ppc64</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <serial type='pty'> + <target port='0'/> + <address type='spapr-vio'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + <address type='spapr-vio'/> + </console> + <memballoon model='none'/> + <panic/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index c147795..4cc1b6a 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -537,6 +537,7 @@ mymain(void) DO_TEST("virtio-rng-egd"); DO_TEST("pseries-nvram"); + DO_TEST_DIFFERENT("pseries-panic-missing"); DO_TEST("pseries-panic-no-address"); /* These tests generate different XML */ -- 2.1.0
On 05/28/2015 10:39 AM, Andrea Bolognani wrote:
Currently, having the <panic> element in a domain XML prevents it from starting on pSeries. The error message in this case is not very accurate, and the first commit improves it.
Since the guest firmware provides the same features as the pvpanic device for pSeries guests, the element should be allowed. The second commit implements this change.
It being part of the firmware, the <panic> element should actually always be present in the domain XML: the third commit makes sure this is the case.
Andrea Bolognani (3): qemu: Improve error message for missing QEMU_CAPS_DEVICE_PANIC. qemu: Allow panic device for pSeries guests qemu: Automatically add <panic> element for pSeries guests.
docs/formatdomain.html.in | 8 ++++- src/qemu/qemu_command.c | 34 ++++++++++++++++------ src/qemu/qemu_domain.c | 14 +++++++++ .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 1 + .../qemuxml2argv-pseries-nvram.xml | 1 + .../qemuxml2argv-pseries-panic-address.xml | 32 ++++++++++++++++++++ .../qemuxml2argv-pseries-panic-missing.args | 7 +++++ .../qemuxml2argv-pseries-panic-missing.xml | 29 ++++++++++++++++++ .../qemuxml2argv-pseries-panic-no-address.args | 7 +++++ .../qemuxml2argv-pseries-panic-no-address.xml | 30 +++++++++++++++++++ tests/qemuxml2argvtest.c | 6 ++++ .../qemuxml2xmlout-pseries-panic-missing.xml | 30 +++++++++++++++++++ tests/qemuxml2xmltest.c | 2 ++ 13 files changed, 191 insertions(+), 10 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-address.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml
Other than minor adjustment to error message in 1/3 (use 'the QEMU binary' like other similar error messages not just 'QEMU') and a tweak to formatdomain.html.in in 3/3: - For pSeries guests, this feature is always enabled because it's - implemented by the guest firmware. libvirt will automatically add the - <code>panic</code> element to the domain XML to reflect this fact. + 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. ACK series... I'll push soon. John
participants (2)
-
Andrea Bolognani -
John Ferlan