On 10/13/2016 11:44 AM, Andrea Bolognani wrote:

The code is entirely correct, but it still managed to trip me up when I first ran into it because I did not realize right away that VIR_PCI_CONNECT_TYPES_ENDPOINT was not a single flag, but rather a mask including both VIR_PCI_CONNECT_TYPE_PCI_DEVICE and VIR_PCI_CONNECT_TYPE_PCIE_DEVICE.

In order to save the next distracted traveler in PCI Address Land some time, document this fact with a comment. Add a test case for the behavior as well.

Yes. Thank you. ACK. etc.

--- src/conf/domain_addr.c | 4 +- .../qemuxml2argv-q35-pci-force-address.args | 25 +++++++++++ .../qemuxml2argv-q35-pci-force-address.xml | 32 +++++++++++++++ tests/qemuxml2argvtest.c | 7 ++++ .../qemuxml2xmlout-q35-pci-force-address.xml | 48 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 7 ++++ 6 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-pci-force-address.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-pci-force-address.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pci-force-address.xml

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 0406b50..bbeb611 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -100,7 +100,9 @@ virDomainPCIAddressFlagsCompatible(virPCIDeviceAddressPtr addr, if (fromConfig) { /* If the requested connection was manually specified in * config, allow a PCI device to connect to a PCIe slot, or - * vice versa. + * vice versa. In order to do so, we add *both* the PCI_DEVICE + * and the PCIE_DEVICE flags to the bus if it already has either + * of them, using the ENDPOINT mask. */ if (busFlags & VIR_PCI_CONNECT_TYPES_ENDPOINT) busFlags |= VIR_PCI_CONNECT_TYPES_ENDPOINT; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-pci-force-address.args b/tests/qemuxml2argvdata/qemuxml2argv-q35-pci-force-address.args new file mode 100644 index 0000000..9ca342b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-pci-force-address.args @@ -0,0 +1,25 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/libexec/qemu-kvm \ +-name q35-test \ +-S \ +-M q35 \ +-m 2048 \ +-smp 2,sockets=2,cores=1,threads=1 \ +-uuid 11dbdcdd-4c3b-482b-8903-9bdb8c0a2774 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-q35-test/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \ +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \ +-device ioh3420,port=0x10,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \ +-device e1000e,vlan=0,id=net0,mac=52:54:00:aa:bf:ef,bus=pci.2,addr=0x1 \ +-net user,vlan=0,name=hostnet0 \ +-device intel-hda,id=sound0,bus=pci.3,addr=0x0 \ +-device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-pci-force-address.xml b/tests/qemuxml2argvdata/qemuxml2argv-q35-pci-force-address.xml new file mode 100644 index 0000000..34ec7c7 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-pci-force-address.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>q35-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='q35'>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/libexec/qemu-kvm</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'/> + <controller type='pci' index='2' model='pci-bridge'/> + <controller type='pci' index='3' model='pcie-root-port'/> + <sound model='ich6'> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/> + </sound> + <interface type='user'> + <model type='e1000e'/> + <mac address='52:54:00:aa:bf:ef'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/> + </interface> + <controller type='usb' model='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b996182..fc18317 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1749,6 +1749,13 @@ mymain(void) QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_NEC_USB_XHCI); + /* Make sure the user can always override libvirt's default device + * placement policy by providing an explicit PCI address */ + DO_TEST("q35-pci-force-address", + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_HDA_DUPLEX);

DO_TEST_PARSE_ERROR("q35-wrong-root", QEMU_CAPS_DEVICE_PCI_BRIDGE, diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pci-force-address.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pci-force-address.xml new file mode 100644 index 0000000..c0332d4 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-pci-force-address.xml @@ -0,0 +1,48 @@ +<domain type='qemu'> + <name>q35-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='q35'>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/libexec/qemu-kvm</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='dmi-to-pci-bridge'> + <model name='i82801b11-bridge'/> + <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='2'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='3' port='0x10'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' 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> + <interface type='user'> + <mac address='52:54:00:aa:bf:ef'/> + <model type='e1000e'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/> + </interface> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <sound model='ich6'> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/> + </sound> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 62bff8c..95c0bf2 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -727,6 +727,13 @@ mymain(void) QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM, QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_NEC_USB_XHCI); + /* Make sure the user can always override libvirt's default device + * placement policy by providing an explicit PCI address */ + DO_TEST("q35-pci-force-address", + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_HDA_DUPLEX);

DO_TEST("hostdev-scsi-lsi", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_LSI,