This patch introduces optional device pvpanic-pci, validates it's address and generates command line. Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com&gt; --- src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 1 + src/qemu/qemu_command.c | 19 +++++++++++++++++++ src/qemu/qemu_validate.c | 16 ++++++++++++++++ 5 files changed, 38 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ff1c78ecd1..c03ca36ae0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -859,6 +859,7 @@ VIR_ENUM_IMPL(virDomainPanicModel, "pseries", "hyperv", "s390", + "pvpanic", ); VIR_ENUM_IMPL(virDomainVideoBackend, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e684edcf95..d2cbc200e7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2677,6 +2677,7 @@ typedef enum { VIR_DOMAIN_PANIC_MODEL_PSERIES, VIR_DOMAIN_PANIC_MODEL_HYPERV, VIR_DOMAIN_PANIC_MODEL_S390, + VIR_DOMAIN_PANIC_MODEL_PVPANIC, VIR_DOMAIN_PANIC_MODEL_LAST } virDomainPanicModel; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index a57dd212ab..ab4886b783 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -8289,6 +8289,7 @@ <value>pseries</value> <value>hyperv</value> <value>s390</value> + <value>pvpanic</value> </choice> </attribute> </optional> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 90dc6b5434..64fee74671 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9557,6 +9557,25 @@ qemuBuildPanicCommandLine(virCommand *cmd, break; } + case VIR_DOMAIN_PANIC_MODEL_PVPANIC: { + g_autoptr(virJSONValue) props = NULL; + + if (virJSONValueObjectAdd(&props, + "s:driver", "pvpanic-pci", + NULL) < 0) + return -1; + + if (def->panics[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + if (qemuBuildDeviceAddressProps(props, def, &def->panics[i]->info) < 0) + return -1; + } + + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) + return -1; + + break; + } + case VIR_DOMAIN_PANIC_MODEL_S390: case VIR_DOMAIN_PANIC_MODEL_HYPERV: case VIR_DOMAIN_PANIC_MODEL_PSERIES: diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index c8c289ebb4..87adaaebdc 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1146,6 +1146,22 @@ qemuValidateDomainDefPanic(const virDomainDef *def, } break; + case VIR_DOMAIN_PANIC_MODEL_PVPANIC: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC_PCI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the QEMU binary does not support the " + "PCI pvpanic device")); + return -1; + } + + if (def->panics[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("pvpanic is supported only " + "with PCI address type")); + return -1; + } + break; + /* default model value was changed before in post parse */ case VIR_DOMAIN_PANIC_MODEL_DEFAULT: case VIR_DOMAIN_PANIC_MODEL_LAST: -- 2.39.1

On Wed, Feb 08, 2023 at 01:05:59PM +0100, Peter Krempa wrote: I imagine it was lifted from the ISA variant, handled just above, where it's necessary because not specifying an address is somehow considered a valid configuration. I agree with you that it's not needed for pvpanic-pci. -- Andrea Bolognani / Red Hat / Virtualization

Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com&gt; --- .../pvpanic-pci-aarch64.aarch64-latest.args | 43 ++++++++++++ .../qemuxml2argvdata/pvpanic-pci-aarch64.xml | 20 ++++++ ...invalid-address-aarch64.aarch64-latest.err | 1 + .../pvpanic-pci-invalid-address-aarch64.xml | 20 ++++++ .../pvpanic-pci-x86_64.x86_64-latest.args | 43 ++++++++++++ tests/qemuxml2argvdata/pvpanic-pci-x86_64.xml | 31 +++++++++ tests/qemuxml2argvtest.c | 4 ++ .../pvpanic-pci-aarch64.aarch64-latest.xml | 63 +++++++++++++++++ .../pvpanic-pci-x86_64.x86_64-latest.xml | 69 +++++++++++++++++++ tests/qemuxml2xmltest.c | 3 + 10 files changed, 297 insertions(+) create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-aarch64.aarch64-latest.args create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-aarch64.xml create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-invalid-address-aarch64.aarch64-latest.err create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-invalid-address-aarch64.xml create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-x86_64.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/pvpanic-pci-x86_64.xml create mode 100644 tests/qemuxml2xmloutdata/pvpanic-pci-aarch64.aarch64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/pvpanic-pci-x86_64.x86_64-latest.xml diff --git a/tests/qemuxml2argvdata/pvpanic-pci-aarch64.aarch64-latest.args b/tests/qemuxml2argvdata/pvpanic-pci-aarch64.aarch64-latest.args new file mode 100644 index 0000000000..3199cd0dbc --- /dev/null +++ b/tests/qemuxml2argvdata/pvpanic-pci-aarch64.aarch64-latest.args @@ -0,0 +1,43 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-guest \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-guest/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-guest/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-guest/.config \ +/usr/bin/qemu-system-aarch64 \ +-name guest=guest,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-guest/master-key.aes"}' \ +-blockdev '{"driver":"file","filename":"/usr/share/AAVMF/AAVMF_CODE.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}' \ +-blockdev '{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/guest_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}' \ +-machine virt-6.0,usb=off,gic-version=2,dump-guest-core=off,memory-backend=mach-virt.ram,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format \ +-accel tcg \ +-cpu cortex-a15 \ +-m 1024 \ +-object '{"qom-type":"memory-backend-ram","id":"mach-virt.ram","size":1073741824}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-device '{"driver":"pcie-root-port","port":8,"chassis":1,"id":"pci.1","bus":"pcie.0","multifunction":true,"addr":"0x1"}' \ +-device '{"driver":"pcie-pci-bridge","id":"pci.2","bus":"pci.1","addr":"0x0"}' \ +-device '{"driver":"pci-bridge","chassis_nr":3,"id":"pci.3","bus":"pci.2","addr":"0x1"}' \ +-device '{"driver":"pci-bridge","chassis_nr":4,"id":"pci.4","bus":"pci.2","addr":"0x2"}' \ +-device '{"driver":"pcie-root-port","port":9,"chassis":5,"id":"pci.5","bus":"pcie.0","addr":"0x1.0x1"}' \ +-device '{"driver":"pcie-root-port","port":10,"chassis":6,"id":"pci.6","bus":"pcie.0","addr":"0x1.0x2"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.5","addr":"0x0"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-device '{"driver":"pvpanic-pci","bus":"pci.4","addr":"0x1"}' \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/pvpanic-pci-aarch64.xml b/tests/qemuxml2argvdata/pvpanic-pci-aarch64.xml new file mode 100644 index 0000000000..7e2954e9cf --- /dev/null +++ b/tests/qemuxml2argvdata/pvpanic-pci-aarch64.xml @@ -0,0 +1,20 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os firmware='efi'> + <type arch='aarch64' machine='virt-6.0'>hvm</type> + </os> + <features> + <acpi/> + </features> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' model='none'/> + <memballoon model='virtio'/> + <panic model='pvpanic'> + <address type='pci' domain='0x0000' bus='0x04' slot='0x01' function='0x0'/> + </panic> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/pvpanic-pci-invalid-address-aarch64.aarch64-latest.err b/tests/qemuxml2argvdata/pvpanic-pci-invalid-address-aarch64.aarch64-latest.err new file mode 100644 index 0000000000..4e74ba5cc9 --- /dev/null +++ b/tests/qemuxml2argvdata/pvpanic-pci-invalid-address-aarch64.aarch64-latest.err @@ -0,0 +1 @@ +unsupported configuration: pvpanic is supported only with PCI address type diff --git a/tests/qemuxml2argvdata/pvpanic-pci-invalid-address-aarch64.xml b/tests/qemuxml2argvdata/pvpanic-pci-invalid-address-aarch64.xml new file mode 100644 index 0000000000..baececb2db --- /dev/null +++ b/tests/qemuxml2argvdata/pvpanic-pci-invalid-address-aarch64.xml @@ -0,0 +1,20 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os firmware='efi'> + <type arch='aarch64' machine='virt-6.0'>hvm</type> + </os> + <features> + <acpi/> + </features> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' model='none'/> + <memballoon model='virtio'/> + <panic model='pvpanic'> + <address type='isa' iobase='0x505'/> + </panic> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/pvpanic-pci-x86_64.x86_64-latest.args b/tests/qemuxml2argvdata/pvpanic-pci-x86_64.x86_64-latest.args new file mode 100644 index 0000000000..b0e173113d --- /dev/null +++ b/tests/qemuxml2argvdata/pvpanic-pci-x86_64.x86_64-latest.args @@ -0,0 +1,43 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram \ +-accel tcg \ +-cpu qemu64 \ +-m 214 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device '{"driver":"pci-bridge","chassis_nr":1,"id":"pci.1","bus":"pci.0","addr":"0x2"}' \ +-device '{"driver":"pci-bridge","chassis_nr":2,"id":"pci.2","bus":"pci.0","addr":"0x3"}' \ +-device '{"driver":"pci-bridge","chassis_nr":3,"id":"pci.3","bus":"pci.0","addr":"0x4"}' \ +-device '{"driver":"pci-bridge","chassis_nr":4,"id":"pci.4","bus":"pci.0","addr":"0x5"}' \ +-device '{"driver":"pci-bridge","chassis_nr":5,"id":"pci.5","bus":"pci.0","addr":"0x6"}' \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \ +-device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-format","id":"ide0-0-0","bootindex":1}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x7"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-device '{"driver":"pvpanic-pci","bus":"pci.5","addr":"0x1"}' \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/pvpanic-pci-x86_64.xml b/tests/qemuxml2argvdata/pvpanic-pci-x86_64.xml new file mode 100644 index 0000000000..4d8a56cb41 --- /dev/null +++ b/tests/qemuxml2argvdata/pvpanic-pci-x86_64.xml @@ -0,0 +1,31 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>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-x86_64</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='fdc' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + <panic model='pvpanic'> + <address type='pci' domain='0x0000' bus='0x05' slot='0x01' function='0x0'/> + </panic> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ae0afb6f1c..4e3815559c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2615,6 +2615,10 @@ mymain(void) DO_TEST_CAPS_LATEST("panic-double"); DO_TEST_CAPS_LATEST("panic-no-address"); + DO_TEST_CAPS_LATEST("pvpanic-pci-x86_64"); + DO_TEST_CAPS_ARCH_LATEST("pvpanic-pci-aarch64", "aarch64"); + DO_TEST_CAPS_ARCH_LATEST_PARSE_ERROR("pvpanic-pci-invalid-address-aarch64", "aarch64"); + DO_TEST_CAPS_ARCH_VER_FULL("fips-enabled", "x86_64", "5.1.0", ARG_FLAGS, FLAG_FIPS_HOST); DO_TEST_CAPS_ARCH_LATEST_FULL("fips-enabled", "x86_64", ARG_FLAGS, FLAG_FIPS_HOST); diff --git a/tests/qemuxml2xmloutdata/pvpanic-pci-aarch64.aarch64-latest.xml b/tests/qemuxml2xmloutdata/pvpanic-pci-aarch64.aarch64-latest.xml new file mode 100644 index 0000000000..bb47c4c2a6 --- /dev/null +++ b/tests/qemuxml2xmloutdata/pvpanic-pci-aarch64.aarch64-latest.xml @@ -0,0 +1,63 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os firmware='efi'> + <type arch='aarch64' machine='virt-6.0'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <gic version='2'/> + </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>cortex-a15</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-aarch64</emulator> + <controller type='usb' index='0' model='none'/> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='1' port='0x8'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0' multifunction='on'/> + </controller> + <controller type='pci' index='2' model='pcie-to-pci-bridge'> + <model name='pcie-pci-bridge'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='3'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='4' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='4'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x0'/> + </controller> + <controller type='pci' index='5' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='5' port='0x9'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='6' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='6' port='0xa'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/> + </memballoon> + <panic model='pvpanic'> + <address type='pci' domain='0x0000' bus='0x04' slot='0x01' function='0x0'/> + </panic> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/pvpanic-pci-x86_64.x86_64-latest.xml b/tests/qemuxml2xmloutdata/pvpanic-pci-x86_64.x86_64-latest.xml new file mode 100644 index 0000000000..81a984e793 --- /dev/null +++ b/tests/qemuxml2xmloutdata/pvpanic-pci-x86_64.x86_64-latest.xml @@ -0,0 +1,69 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</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-x86_64</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='fdc' index='0'/> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='pci' index='1' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='3'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </controller> + <controller type='pci' index='4' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='4'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </controller> + <controller type='pci' index='5' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='5'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </memballoon> + <panic model='pvpanic'> + <address type='pci' domain='0x0000' bus='0x05' slot='0x01' function='0x0'/> + </panic> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index ae9716062d..269b2d8016 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -908,6 +908,9 @@ mymain(void) DO_TEST("panic-no-address", QEMU_CAPS_DEVICE_PANIC); DO_TEST_CAPS_ARCH_LATEST("panic-pseries", "ppc64"); + DO_TEST_CAPS_LATEST("pvpanic-pci-x86_64"); + DO_TEST_CAPS_ARCH_LATEST("pvpanic-pci-aarch64", "aarch64"); + DO_TEST_NOCAPS("disk-backing-chains-index"); DO_TEST_NOCAPS("disk-backing-chains-noindex"); -- 2.39.1

On Wed, Feb 08, 2023 at 12:49:03PM +0100, Kristina Hanicova wrote: So, this is correct, because pvpanic-pci is indeed a conventional PCI device (as opposed to a PCI Express device). However, when used with a PCIe-based machine such as aarch64/virt, these flags result in the device being plugged into a pcie-pci-bridge, which in turn is plugged into a pcie-root-port, itself plugged into pcie.0. While this seems to work fine, it's also kind of a waste of PCI controllers. For a "system" device such as pvpanic-pci, I think plugging directly into pcie.0 might be more appropriate. This is particularly true of x86/q35, where with the current implementation switching from ISA pvpanic to pvpanic-pci would result in adding two extra PCI controllers. So I think this would be a good fit for the VIR_PCI_CONNECT_INTEGRATED flag that I've added a while ago for use with virtio-iommu. While this would technically result in libvirt being more restrictive than QEMU in what PCI addresses it accepts for pvpanic-pci, I don't think this would limit users in practice, and in the default case (libvirt automatically picking the address) the resulting configuration would be preferable. We can always decide to relax this restriction later down the line, if it turns out to really be a limiting factor. Eric, what do you think about this idea? (There's one caveat: VIR_PCI_CONNECT_INTEGRATED doesn't seem to work with pciFlags at the moment O:-) It works fine with pcieFlags and virtioFlags. I'll try to figure out why that's the case.) -- Andrea Bolognani / Red Hat / Virtualization

On Thu, Feb 09, 2023 at 06:26:14PM +0100, Eric Auger wrote: Thanks for the input! Let's go with this approach then, unless someone raises concerns with it. Fixed by this patch: https://listman.redhat.com/archives/libvir-list/2023-February/237688.html -- Andrea Bolognani / Red Hat / Virtualization

On Wed, Feb 08, 2023 at 12:49:04PM +0100, Kristina Hanicova wrote: You have forgotten the corresponding qemuxml2argvtest change. The output file is there, so I know you have made the change locally ;) -- Andrea Bolognani / Red Hat / Virtualization

On Wed, Feb 08, 2023 at 12:49:05 +0100, Kristina Hanicova wrote: s/devicen/device/ Also is KVM really required? I'd expect that it will also work with TCG VMs.

On Thu, Feb 09, 2023 at 08:41:01 -0800, Andrea Bolognani wrote: Generaly in our docs "QEMU only" means that it works only with the qemu driver/hypervisor, thus I don't think the interpretation that <domain type='qemu'> or kvm will work here.