[libvirt] [PATCH 0/3] qemu: Clean up PCI controller options

See patch 3/3 for an explanation. Andrea Bolognani (3): qemu: Add missing checks for pcie-root-port options tests: Add some tests for PCI controller options qemu: Clean up PCI controller options src/qemu/qemu_command.c | 4 +- src/qemu/qemu_domain.c | 117 +++++++++++++++++++++ .../i440fx-controllers-pciopts.args | 24 +++++ .../i440fx-controllers-pciopts.xml | 36 +++++++ .../pseries-controllers-pciopts.args | 22 ++++ .../pseries-controllers-pciopts.xml | 36 +++++++ .../qemuxml2argvdata/q35-controllers-pciopts.args | 28 +++++ tests/qemuxml2argvdata/q35-controllers-pciopts.xml | 60 +++++++++++ tests/qemuxml2argvtest.c | 17 +++ .../i440fx-controllers-pciopts.xml | 42 ++++++++ tests/qemuxml2xmloutdata/pcie-expander-bus.xml | 4 +- .../pseries-controllers-pciopts.xml | 41 ++++++++ .../qemuxml2xmloutdata/q35-controllers-pciopts.xml | 63 +++++++++++ tests/qemuxml2xmltest.c | 17 +++ 14 files changed, 507 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/i440fx-controllers-pciopts.args create mode 100644 tests/qemuxml2argvdata/i440fx-controllers-pciopts.xml create mode 100644 tests/qemuxml2argvdata/pseries-controllers-pciopts.args create mode 100644 tests/qemuxml2argvdata/pseries-controllers-pciopts.xml create mode 100644 tests/qemuxml2argvdata/q35-controllers-pciopts.args create mode 100644 tests/qemuxml2argvdata/q35-controllers-pciopts.xml create mode 100644 tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml create mode 100644 tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml create mode 100644 tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml -- 2.14.3

We format the 'chassis' and 'port' properties on the QEMU command line later on, so we should make sure they've been set. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_command.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6a8da1d58..c0ea9eded 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2922,7 +2922,9 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: if (def->opts.pciopts.modelName - == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { + == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || + def->opts.pciopts.chassis == -1 || + def->opts.pciopts.port == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("autogenerated pcie-root-port options not set")); goto error; -- 2.14.3

On 12/06/2017 05:15 AM, Andrea Bolognani wrote:
We format the 'chassis' and 'port' properties on the QEMU command line later on, so we should make sure they've been set.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_command.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
I've been working on moving changes from qemu_command to qemu_domain as part of a validate the controller def sequence of calls... v3 is here: https://www.redhat.com/archives/libvir-list/2017-December/msg00209.html Do you mind if I merge that series before I post v4? John
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6a8da1d58..c0ea9eded 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2922,7 +2922,9 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: if (def->opts.pciopts.modelName - == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { + == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || + def->opts.pciopts.chassis == -1 || + def->opts.pciopts.port == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("autogenerated pcie-root-port options not set")); goto error;

On Sat, 2017-12-09 at 11:26 -0500, John Ferlan wrote:
I've been working on moving changes from qemu_command to qemu_domain as part of a validate the controller def sequence of calls... v3 is here:
https://www.redhat.com/archives/libvir-list/2017-December/msg00209.html
Do you mind if I merge that series before I post v4?
I'm... Actually not sure what you're asking here :) -- Andrea Bolognani / Red Hat / Virtualization

On 12/11/2017 05:36 AM, Andrea Bolognani wrote:
On Sat, 2017-12-09 at 11:26 -0500, John Ferlan wrote:
I've been working on moving changes from qemu_command to qemu_domain as part of a validate the controller def sequence of calls... v3 is here:
https://www.redhat.com/archives/libvir-list/2017-December/msg00209.html
Do you mind if I merge that series before I post v4?
I'm... Actually not sure what you're asking here :)
Funny made sense when my brain told my fingers what to type, but they seem to have their own problems ;-)... So let's try again... I'm working on a series that's moving a bunch of the qemu_command controller validation checks into qemu_domain. So my question is, do you mind if I merge this one patch into what I have built up for the next posting of that series. One way or another one of us was going to have to merge, I just figured I could get in front of that bus. John

On Mon, 2017-12-11 at 08:13 -0500, John Ferlan wrote:
On 12/11/2017 05:36 AM, Andrea Bolognani wrote:
On Sat, 2017-12-09 at 11:26 -0500, John Ferlan wrote:
I've been working on moving changes from qemu_command to qemu_domain as part of a validate the controller def sequence of calls... v3 is here:
https://www.redhat.com/archives/libvir-list/2017-December/msg00209.html
Do you mind if I merge that series before I post v4?
I'm... Actually not sure what you're asking here :)
Funny made sense when my brain told my fingers what to type, but they seem to have their own problems ;-)... So let's try again...
I'm working on a series that's moving a bunch of the qemu_command controller validation checks into qemu_domain. So my question is, do you mind if I merge this one patch into what I have built up for the next posting of that series. One way or another one of us was going to have to merge, I just figured I could get in front of that bus.
Sure, feel free to pick up the patch and merge it along with the rest of your work. I'll rebase the remaining patches afterwards. -- Andrea Bolognani / Red Hat / Virtualization

The input configurations set all existing options for all PCI controllers, to see what ends up sticking in the output. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- .../i440fx-controllers-pciopts.args | 24 +++++++ .../i440fx-controllers-pciopts.xml | 36 ++++++++++ .../pseries-controllers-pciopts.args | 22 +++++++ .../pseries-controllers-pciopts.xml | 35 ++++++++++ .../qemuxml2argvdata/q35-controllers-pciopts.args | 28 ++++++++ tests/qemuxml2argvdata/q35-controllers-pciopts.xml | 60 +++++++++++++++++ tests/qemuxml2argvtest.c | 17 +++++ .../i440fx-controllers-pciopts.xml | 45 +++++++++++++ .../pseries-controllers-pciopts.xml | 43 ++++++++++++ .../qemuxml2xmloutdata/q35-controllers-pciopts.xml | 76 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 17 +++++ 11 files changed, 403 insertions(+) create mode 100644 tests/qemuxml2argvdata/i440fx-controllers-pciopts.args create mode 100644 tests/qemuxml2argvdata/i440fx-controllers-pciopts.xml create mode 100644 tests/qemuxml2argvdata/pseries-controllers-pciopts.args create mode 100644 tests/qemuxml2argvdata/pseries-controllers-pciopts.xml create mode 100644 tests/qemuxml2argvdata/q35-controllers-pciopts.args create mode 100644 tests/qemuxml2argvdata/q35-controllers-pciopts.xml create mode 100644 tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml create mode 100644 tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml create mode 100644 tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml diff --git a/tests/qemuxml2argvdata/i440fx-controllers-pciopts.args b/tests/qemuxml2argvdata/i440fx-controllers-pciopts.args new file mode 100644 index 000000000..d85fae5c9 --- /dev/null +++ b/tests/qemuxml2argvdata/i440fx-controllers-pciopts.args @@ -0,0 +1,24 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name guest \ +-S \ +-M pc \ +-m 1024 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-numa node,nodeid=0,cpus=0,mem=1024 \ +-uuid 496d7ea8-9739-544b-4ebd-ef08be936e8b \ +-nographic \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-guest/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-global i440FX-pcihost.pci-hole64-size=1024K \ +-device pci-bridge,chassis_nr=2,id=pci.1,bus=pci.0,addr=0x3 \ +-device pxb,bus_nr=3,id=pci.2,numa_node=0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/i440fx-controllers-pciopts.xml b/tests/qemuxml2argvdata/i440fx-controllers-pciopts.xml new file mode 100644 index 000000000..861a66589 --- /dev/null +++ b/tests/qemuxml2argvdata/i440fx-controllers-pciopts.xml @@ -0,0 +1,36 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid> + <memory unit='KiB'>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + </os> + <cpu> + <numa> + <cell id='0' cpus='0' memory='1048576' unit='KiB'/> + </numa> + </cpu> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='pci' index='0' model='pci-root'> + <!-- Only pcihole64 should be preserved --> + <target busNr='1' chassis='1' chassisNr='1' index='0' port='1'/> + <pcihole64 unit='KiB'>1024</pcihole64> + </controller> + <controller type='pci' index='1' model='pci-bridge'> + <!-- Only chassisNr should be preserved --> + <target busNr='2' chassis='2' chassisNr='2' index='2' port='2'> + <node>0</node> + </target> + </controller> + <controller type='pci' index='2' model='pci-expander-bus'> + <!-- Only busNr and numaNode should be preserved --> + <target busNr='3' chassis='3' chassisNr='3' index='3' port='3'> + <node>0</node> + </target> + </controller> + <controller type='usb' index='0' model='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/pseries-controllers-pciopts.args b/tests/qemuxml2argvdata/pseries-controllers-pciopts.args new file mode 100644 index 000000000..5f1edfc83 --- /dev/null +++ b/tests/qemuxml2argvdata/pseries-controllers-pciopts.args @@ -0,0 +1,22 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-ppc64 \ +-name guest \ +-S \ +-M pseries \ +-m 1024 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-numa node,nodeid=0,cpus=0,mem=1024 \ +-uuid 496d7ea8-9739-544b-4ebd-ef08be936e8b \ +-nographic \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-guest/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-boot c \ +-device spapr-pci-host-bridge,index=1,id=pci.1,numa_node=0 \ +-device pci-bridge,chassis_nr=3,id=pci.2,bus=pci.0,addr=0x1 diff --git a/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml b/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml new file mode 100644 index 000000000..43353cba6 --- /dev/null +++ b/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid> + <memory unit='KiB'>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <cpu> + <numa> + <cell id='0' cpus='0' memory='1048576' unit='KiB'/> + </numa> + </cpu> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <controller type='pci' index='0' model='pci-root'> + <!-- Only targetIndex should be preserved --> + <target busNr='1' chassis='1' chassisNr='1' index='0' port='1'/> + </controller> + <controller type='pci' index='1' model='pci-root'> + <!-- Only numaNode and targetIndex should be preserved --> + <target busNr='2' chassis='2' chassisNr='2' index='1' port='2'> + <node>0</node> + </target> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <!-- Only chassisNr should be preserved --> + <target busNr='3' chassis='3' chassisNr='3' index='3' port='3'> + <node>0</node> + </target> + </controller> + <controller type='usb' index='0' model='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/q35-controllers-pciopts.args b/tests/qemuxml2argvdata/q35-controllers-pciopts.args new file mode 100644 index 000000000..44259f502 --- /dev/null +++ b/tests/qemuxml2argvdata/q35-controllers-pciopts.args @@ -0,0 +1,28 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name guest \ +-S \ +-M q35 \ +-m 1024 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-numa node,nodeid=0,cpus=0,mem=1024 \ +-uuid 496d7ea8-9739-544b-4ebd-ef08be936e8b \ +-nographic \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-guest/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-global q35-pcihost.pci-hole64-size=1024K \ +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ +-device pci-bridge,chassis_nr=3,id=pci.2,bus=pci.1,addr=0x0 \ +-device pxb-pcie,bus_nr=4,id=pci.3,numa_node=0,bus=pcie.0,addr=0x2 \ +-device pcie-root-port,port=0x5,chassis=5,id=pci.4,bus=pcie.0,addr=0x3 \ +-device x3130-upstream,id=pci.5,bus=pci.4,addr=0x0 \ +-device xio3130-downstream,port=0x7,chassis=7,id=pci.6,bus=pci.5,addr=0x0 diff --git a/tests/qemuxml2argvdata/q35-controllers-pciopts.xml b/tests/qemuxml2argvdata/q35-controllers-pciopts.xml new file mode 100644 index 000000000..40db83615 --- /dev/null +++ b/tests/qemuxml2argvdata/q35-controllers-pciopts.xml @@ -0,0 +1,60 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid> + <memory unit='KiB'>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + </os> + <cpu> + <numa> + <cell id='0' cpus='0' memory='1048576' unit='KiB'/> + </numa> + </cpu> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='pci' index='0' model='pcie-root'> + <!-- Only pcihole64 should be preserved --> + <target busNr='1' chassis='1' chassisNr='1' index='0' port='1'/> + <pcihole64 unit='KiB'>1024</pcihole64> + </controller> + <controller type='pci' index='1' model='dmi-to-pci-bridge'> + <!-- No option should be preserved --> + <target busNr='2' chassis='2' chassisNr='2' index='2' port='2'> + <node>0</node> + </target> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <!-- Only chassisNr should be preserved --> + <target busNr='3' chassis='3' chassisNr='3' index='3' port='3'> + <node>0</node> + </target> + </controller> + <controller type='pci' index='3' model='pcie-expander-bus'> + <!-- Only busNr and numaNode should be preserved --> + <target busNr='4' chassis='4' chassisNr='4' index='4' port='4'> + <node>0</node> + </target> + </controller> + <controller type='pci' index='4' model='pcie-root-port'> + <!-- Only chassis and port should be preserved --> + <target busNr='5' chassis='5' chassisNr='5' index='5' port='5'> + <node>0</node> + </target> + </controller> + <controller type='pci' index='5' model='pcie-switch-upstream-port'> + <!-- No option should be preserved --> + <target busNr='6' chassis='6' chassisNr='6' index='6' port='6'> + <node>0</node> + </target> + </controller> + <controller type='pci' index='6' model='pcie-switch-downstream-port'> + <!-- Only chassis and port should be preserved --> + <target busNr='7' chassis='7' chassisNr='7' index='7' port='7'> + <node>0</node> + </target> + </controller> + <controller type='usb' index='0' model='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ca24e0bbb..a7b205446 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2428,6 +2428,23 @@ mymain(void) QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_DEVICE_PXB_PCIE); + DO_TEST("i440fx-controllers-pciopts", + QEMU_CAPS_I440FX_PCI_HOLE64_SIZE, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PXB); + DO_TEST("q35-controllers-pciopts", + QEMU_CAPS_Q35_PCI_HOLE64_SIZE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PXB_PCIE, + QEMU_CAPS_DEVICE_PCIE_ROOT_PORT, + QEMU_CAPS_DEVICE_X3130_UPSTREAM, + QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM); + DO_TEST("pseries-controllers-pciopts", + QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE, + QEMU_CAPS_DEVICE_PCI_BRIDGE); + DO_TEST("hostdev-scsi-lsi", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_LSI, QEMU_CAPS_DEVICE_SCSI_GENERIC); diff --git a/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml b/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml new file mode 100644 index 000000000..d171d1370 --- /dev/null +++ b/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml @@ -0,0 +1,45 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu> + <numa> + <cell id='0' cpus='0' memory='1048576' unit='KiB'/> + </numa> + </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> + <controller type='pci' index='0' model='pci-root'> + <target chassisNr='1' chassis='1' port='0x1' busNr='1' index='0'/> + <pcihole64 unit='KiB'>1024</pcihole64> + </controller> + <controller type='pci' index='1' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='2' chassis='2' port='0x2' busNr='2' index='2'> + <node>0</node> + </target> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pci-expander-bus'> + <model name='pxb'/> + <target chassisNr='3' chassis='3' port='0x3' busNr='3' index='3'> + <node>0</node> + </target> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </controller> + <controller type='usb' index='0' model='none'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml b/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml new file mode 100644 index 000000000..bbe360e25 --- /dev/null +++ b/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml @@ -0,0 +1,43 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + <boot dev='hd'/> + </os> + <cpu> + <numa> + <cell id='0' cpus='0' memory='1048576' unit='KiB'/> + </numa> + </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-ppc64</emulator> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target chassisNr='1' chassis='1' port='0x1' busNr='1' index='0'/> + </controller> + <controller type='pci' index='1' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target chassisNr='2' chassis='2' port='0x2' busNr='2' index='1'> + <node>0</node> + </target> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='3' chassis='3' port='0x3' busNr='3' index='3'> + <node>0</node> + </target> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='usb' index='0' model='none'/> + <memballoon model='none'/> + <panic model='pseries'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml b/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml new file mode 100644 index 000000000..5ef7aa564 --- /dev/null +++ b/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml @@ -0,0 +1,76 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <cpu> + <numa> + <cell id='0' cpus='0' memory='1048576' unit='KiB'/> + </numa> + </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> + <controller type='pci' index='0' model='pcie-root'> + <target chassisNr='1' chassis='1' port='0x1' busNr='1' index='0'/> + <pcihole64 unit='KiB'>1024</pcihole64> + </controller> + <controller type='pci' index='1' model='dmi-to-pci-bridge'> + <model name='i82801b11-bridge'/> + <target chassisNr='2' chassis='2' port='0x2' busNr='2' index='2'> + <node>0</node> + </target> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pci-bridge'> + <model name='pci-bridge'/> + <target chassisNr='3' chassis='3' port='0x3' busNr='3' index='3'> + <node>0</node> + </target> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-expander-bus'> + <model name='pxb-pcie'/> + <target chassisNr='4' chassis='4' port='0x4' busNr='4' index='4'> + <node>0</node> + </target> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <controller type='pci' index='4' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassisNr='5' chassis='5' port='0x5' busNr='5' index='5'> + <node>0</node> + </target> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </controller> + <controller type='pci' index='5' model='pcie-switch-upstream-port'> + <model name='x3130-upstream'/> + <target chassisNr='6' chassis='6' port='0x6' busNr='6' index='6'> + <node>0</node> + </target> + <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='6' model='pcie-switch-downstream-port'> + <model name='xio3130-downstream'/> + <target chassisNr='7' chassis='7' port='0x7' busNr='7' index='7'> + <node>0</node> + </target> + <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/> + </controller> + <controller type='usb' index='0' model='none'/> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 2be8eb2c1..97f8f4d18 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1103,6 +1103,23 @@ mymain(void) QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_HDA_DUPLEX); + DO_TEST("i440fx-controllers-pciopts", + QEMU_CAPS_I440FX_PCI_HOLE64_SIZE, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PXB); + DO_TEST("q35-controllers-pciopts", + QEMU_CAPS_Q35_PCI_HOLE64_SIZE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_PXB_PCIE, + QEMU_CAPS_DEVICE_PCIE_ROOT_PORT, + QEMU_CAPS_DEVICE_X3130_UPSTREAM, + QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM); + DO_TEST("pseries-controllers-pciopts", + QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE, + QEMU_CAPS_DEVICE_PCI_BRIDGE); + DO_TEST("hostdev-scsi-vhost-scsi-ccw", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_VHOST_SCSI, QEMU_CAPS_DEVICE_SCSI_GENERIC, QEMU_CAPS_VIRTIO_CCW); -- 2.14.3

Most of the options are only applicable to one or two controller types, so they should be filtered out everywhere else. This will reduce user confusion and, in at least one corner case, prevent guests from disappearing on daemon restart. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1483816 Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_domain.c | 117 +++++++++++++++++++++ .../pseries-controllers-pciopts.xml | 1 + .../i440fx-controllers-pciopts.xml | 7 +- tests/qemuxml2xmloutdata/pcie-expander-bus.xml | 4 +- .../pseries-controllers-pciopts.xml | 8 +- .../qemuxml2xmloutdata/q35-controllers-pciopts.xml | 21 +--- 6 files changed, 128 insertions(+), 30 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e8e03134f..4c6fab131 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4110,6 +4110,121 @@ qemuDomainShmemDefPostParse(virDomainShmemDefPtr shm) #define QEMU_USB_XHCI_MAXPORTS 15 +/** + * qemuDomainPCIControllerCleanupOpts: + * @cont: controller + * + * Clean up PCI options for @cont so that only options applicable to + * the controller model will actually be set. + */ +static void +qemuDomainPCIControllerCleanupOpts(const virDomainDef *def, + virDomainControllerDefPtr cont) +{ + if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI) + return; + + /* pcihole64 and targetIndex */ + switch ((virDomainControllerModelPCI) cont->model) { + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + /* These controllers support all options; however, + * targetIndex is only supported for pSeries guests and + * pcihole64 is only supported on x86 */ + if (!qemuDomainIsPSeries(def)) + cont->opts.pciopts.targetIndex = -1; + if (!ARCH_IS_X86(def->os.arch)) { + cont->opts.pciopts.pcihole64 = false; + cont->opts.pciopts.pcihole64size = 0; + } + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: + case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: + /* These controllers don't support any option */ + cont->opts.pciopts.pcihole64 = false; + cont->opts.pciopts.pcihole64size = 0; + ATTRIBUTE_FALLTHROUGH; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: + /* These controllers support all options except targetIndex */ + cont->opts.pciopts.targetIndex = -1; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: + break; + } + + /* busNr and numaNode */ + switch ((virDomainControllerModelPCI) cont->model) { + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: + /* These controllers support all options */ + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: + case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: + /* These controllers don't support any option */ + cont->opts.pciopts.numaNode = -1; + ATTRIBUTE_FALLTHROUGH; + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + /* These controllers support all options except busNr; however, + * numaNode is only supported for pSeries guests */ + cont->opts.pciopts.busNr = -1; + if (!qemuDomainIsPSeries(def)) + cont->opts.pciopts.numaNode = -1; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: + break; + } + + /* chassis and port */ + switch ((virDomainControllerModelPCI) cont->model) { + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: + /* These controllers support all options */ + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: + case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: + /* These controllers don't support any option */ + cont->opts.pciopts.chassis = -1; + cont->opts.pciopts.port = -1; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: + break; + } + + /* chassisNr */ + switch ((virDomainControllerModelPCI) cont->model) { + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: + /* These controllers support all options */ + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: + case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: + /* These controllers don't support any option */ + cont->opts.pciopts.chassisNr = -1; + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: + break; + } +} + + static int qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont, const virDomainDef *def, @@ -4194,6 +4309,8 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont, case VIR_DOMAIN_CONTROLLER_TYPE_PCI: + qemuDomainPCIControllerCleanupOpts(def, cont); + /* pSeries guests can have multiple pci-root controllers, * but other machine types only support a single one */ if (!qemuDomainIsPSeries(def) && diff --git a/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml b/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml index 43353cba6..a3237ff2c 100644 --- a/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml +++ b/tests/qemuxml2argvdata/pseries-controllers-pciopts.xml @@ -16,6 +16,7 @@ <controller type='pci' index='0' model='pci-root'> <!-- Only targetIndex should be preserved --> <target busNr='1' chassis='1' chassisNr='1' index='0' port='1'/> + <pcihole64 unit='KiB'>1024</pcihole64> </controller> <controller type='pci' index='1' model='pci-root'> <!-- Only numaNode and targetIndex should be preserved --> diff --git a/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml b/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml index d171d1370..d1868fdc2 100644 --- a/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml +++ b/tests/qemuxml2xmloutdata/i440fx-controllers-pciopts.xml @@ -20,19 +20,16 @@ <devices> <emulator>/usr/bin/qemu-system-x86_64</emulator> <controller type='pci' index='0' model='pci-root'> - <target chassisNr='1' chassis='1' port='0x1' busNr='1' index='0'/> <pcihole64 unit='KiB'>1024</pcihole64> </controller> <controller type='pci' index='1' model='pci-bridge'> <model name='pci-bridge'/> - <target chassisNr='2' chassis='2' port='0x2' busNr='2' index='2'> - <node>0</node> - </target> + <target chassisNr='2'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </controller> <controller type='pci' index='2' model='pci-expander-bus'> <model name='pxb'/> - <target chassisNr='3' chassis='3' port='0x3' busNr='3' index='3'> + <target busNr='3'> <node>0</node> </target> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> diff --git a/tests/qemuxml2xmloutdata/pcie-expander-bus.xml b/tests/qemuxml2xmloutdata/pcie-expander-bus.xml index aaac423ca..b6498fd2e 100644 --- a/tests/qemuxml2xmloutdata/pcie-expander-bus.xml +++ b/tests/qemuxml2xmloutdata/pcie-expander-bus.xml @@ -36,9 +36,7 @@ </controller> <controller type='pci' index='2' model='pcie-root-port'> <model name='ioh3420'/> - <target chassis='2' port='0x0' busNr='220'> - <node>1</node> - </target> + <target chassis='2' port='0x0'/> <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> </controller> <controller type='pci' index='3' model='pcie-switch-upstream-port'> diff --git a/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml b/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml index bbe360e25..b5d646765 100644 --- a/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml +++ b/tests/qemuxml2xmloutdata/pseries-controllers-pciopts.xml @@ -21,19 +21,17 @@ <emulator>/usr/bin/qemu-system-ppc64</emulator> <controller type='pci' index='0' model='pci-root'> <model name='spapr-pci-host-bridge'/> - <target chassisNr='1' chassis='1' port='0x1' busNr='1' index='0'/> + <target index='0'/> </controller> <controller type='pci' index='1' model='pci-root'> <model name='spapr-pci-host-bridge'/> - <target chassisNr='2' chassis='2' port='0x2' busNr='2' index='1'> + <target index='1'> <node>0</node> </target> </controller> <controller type='pci' index='2' model='pci-bridge'> <model name='pci-bridge'/> - <target chassisNr='3' chassis='3' port='0x3' busNr='3' index='3'> - <node>0</node> - </target> + <target chassisNr='3'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </controller> <controller type='usb' index='0' model='none'/> diff --git a/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml b/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml index 5ef7aa564..d8a014853 100644 --- a/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml +++ b/tests/qemuxml2xmloutdata/q35-controllers-pciopts.xml @@ -20,49 +20,36 @@ <devices> <emulator>/usr/bin/qemu-system-x86_64</emulator> <controller type='pci' index='0' model='pcie-root'> - <target chassisNr='1' chassis='1' port='0x1' busNr='1' index='0'/> <pcihole64 unit='KiB'>1024</pcihole64> </controller> <controller type='pci' index='1' model='dmi-to-pci-bridge'> <model name='i82801b11-bridge'/> - <target chassisNr='2' chassis='2' port='0x2' busNr='2' index='2'> - <node>0</node> - </target> <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/> </controller> <controller type='pci' index='2' model='pci-bridge'> <model name='pci-bridge'/> - <target chassisNr='3' chassis='3' port='0x3' busNr='3' index='3'> - <node>0</node> - </target> + <target chassisNr='3'/> <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> </controller> <controller type='pci' index='3' model='pcie-expander-bus'> <model name='pxb-pcie'/> - <target chassisNr='4' chassis='4' port='0x4' busNr='4' index='4'> + <target busNr='4'> <node>0</node> </target> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </controller> <controller type='pci' index='4' model='pcie-root-port'> <model name='pcie-root-port'/> - <target chassisNr='5' chassis='5' port='0x5' busNr='5' index='5'> - <node>0</node> - </target> + <target chassis='5' port='0x5'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </controller> <controller type='pci' index='5' model='pcie-switch-upstream-port'> <model name='x3130-upstream'/> - <target chassisNr='6' chassis='6' port='0x6' busNr='6' index='6'> - <node>0</node> - </target> <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/> </controller> <controller type='pci' index='6' model='pcie-switch-downstream-port'> <model name='xio3130-downstream'/> - <target chassisNr='7' chassis='7' port='0x7' busNr='7' index='7'> - <node>0</node> - </target> + <target chassis='7' port='0x7'/> <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/> </controller> <controller type='usb' index='0' model='none'/> -- 2.14.3
participants (2)
-
Andrea Bolognani
-
John Ferlan