[libvirt] [PATCH 0/2] replacement for Patches 5/8 & 6/8 of "aggregate multiple pcie-root-ports onto a single slot"
2:23 a.m.
During review of the aforementioned series, Andrea expressed
misgivings about patch 6, which automatically set the multifunction=on
option transiently at the time the qemu commandline was generated. Dan
agreed. These two patches replace patches 5 and 6 of that series with
two patches that will set multi='on' in the domain's config rather
than just temporarily in the commandline. This way it will remain set
even if all the devices on the non-0 function are later removed
(unless it's explicitly changed by the user/management app, that is).
The rest of the series is ACKed and ready to push, which I'd like to
do before the freeze on Wednesday; these two patche are all that's
holding it up.
Laine Stump (2):
conf: new function virDomainPCIAddressSetAllMulti()
qemu: use virDomainPCIAddressSetAllMulti() to set multi when needed
src/conf/domain_addr.c | 75 +++++++++++++
src/conf/domain_addr.h | 4 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_domain_address.c | 5 +
.../qemuxml2argv-q35-multifunction.args | 42 ++++++++
.../qemuxml2argv-q35-multifunction.xml | 51 +++++++++
tests/qemuxml2argvtest.c | 23 ++++
.../qemuxml2xmlout-q35-multifunction.xml | 120 +++++++++++++++++++++
tests/qemuxml2xmltest.c | 23 ++++
9 files changed, 344 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-multifunction.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-multifunction.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-multifunction.xml
--
2.7.4
2:23 a.m.
New subject: [libvirt] [PATCH 1/2] conf: new function virDomainPCIAddressSetAllMulti()
This utility function looks through a virPCIAddressSet for any slot of
any bus that has multiple functions in use, and sets the "multi" flag
in the virDomainDeviceInfo for the device that is assigned to function
0 of that slot (as long as it hasn't already been set explicitly by
someone who presumably has better information than we do).
It isn't yet called from anywhere, so will have no functional effect.
---
src/conf/domain_addr.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_addr.h | 4 +++
src/libvirt_private.syms | 1 +
3 files changed, 80 insertions(+)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 4d14ac4..0364e95 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -867,6 +867,81 @@ virDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs,
}
+static int
+virDomainPCIAddressSetMultiIter(virDomainDefPtr def ATTRIBUTE_UNUSED,
+ virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED,
+ virDomainDeviceInfoPtr info,
+ void *data)
+{
+ virPCIDeviceAddressPtr testAddr = data;
+ virPCIDeviceAddressPtr thisAddr;
+
+ if (!info || info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)
+ return 0;
+
+ thisAddr = &info->addr.pci;
+
+ if (thisAddr->domain == testAddr->domain &&
+ thisAddr->bus == testAddr->bus &&
+ thisAddr->slot == testAddr->slot &&
+ thisAddr->function == 0) {
+
+ /* only set to ON if it wasn't previously set
+ * (assuming that the user must have better information
+ * than us if they explicitly set it OFF)
+ */
+ if (thisAddr->multi == VIR_TRISTATE_SWITCH_ABSENT)
+ thisAddr->multi = VIR_TRISTATE_SWITCH_ON;
+
+ return -1; /* finish early, *NOT* an error */
+ }
+
+ return 0;
+}
+
+
+/**
+ * virDomainPCIAddressSetAllMulti():
+ *
+ * @def: the domain definition whose devices may need adjusting
+ * @addrs: address set keeping track of all addresses in use.
+ *
+ * Look for any PCI slots that have multiple functions assigned, and
+ * set multi to YES in the address for the device at function 0
+ * (unless it has been explicitly set to NO).
+ *
+ * No return code, since there is no possibility of failure.
+ */
+void
+virDomainPCIAddressSetAllMulti(virDomainDefPtr def,
+ virDomainPCIAddressSetPtr addrs)
+{
+ /* Scan through all the slots in @addrs looking for any that have
+ * more than just function 0 marked as in use, then use an
+ * iterator to find the DeviceInfo that uses function 0 on that
+ * slot and mark it as multi = YES
+ */
+ size_t busIdx, slotIdx;
+
+ for (busIdx = 0; busIdx < addrs->nbuses; busIdx++) {
+ virDomainPCIAddressBusPtr bus = &addrs->buses[busIdx];
+
+ for (slotIdx = bus->minSlot; slotIdx <= bus->maxSlot; slotIdx++) {
+ if (bus->slot[slotIdx].functions > 1) {
+ virPCIDeviceAddress addr = { .domain = 0,
+ .bus = busIdx,
+ .slot = slotIdx,
+ .function = 0 };
+
+ ignore_value(virDomainDeviceInfoIterate(def,
+ virDomainPCIAddressSetMultiIter,
+ &addr));
+ }
+ }
+ }
+}
+
+
static char*
virDomainCCWAddressAsString(virDomainDeviceCCWAddressPtr addr)
{
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 5f0924e..2263114 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -173,6 +173,10 @@ int virDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr
addrs,
virDomainPCIConnectFlags flags)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+void virDomainPCIAddressSetAllMulti(virDomainDefPtr def,
+ virDomainPCIAddressSetPtr addrs)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
struct _virDomainCCWAddressSet {
virHashTablePtr defined;
virDomainDeviceCCWAddress next;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a928e90..a0db583 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -104,6 +104,7 @@ virDomainPCIAddressReserveAddr;
virDomainPCIAddressReserveNextAddr;
virDomainPCIAddressReserveNextSlot;
virDomainPCIAddressReserveSlot;
+virDomainPCIAddressSetAllMulti;
virDomainPCIAddressSetAlloc;
virDomainPCIAddressSetFree;
virDomainPCIAddressSetGrow;
--
2.7.4
4:42 a.m.
New subject: [libvirt] [PATCH 1/2] conf: new function virDomainPCIAddressSetAllMulti()
On Tue, 2017-01-10 at 03:23 -0500, Laine Stump wrote:
This utility function looks through a virPCIAddressSet for any slot
of
any bus that has multiple functions in use, and sets the "multi" flag
in the virDomainDeviceInfo for the device that is assigned to function
0 of that slot (as long as it hasn't already been set explicitly by
someone who presumably has better information than we do).
It isn't yet called from anywhere, so will have no functional effect.
[...]
+static int
+virDomainPCIAddressSetMultiIter(virDomainDefPtr def ATTRIBUTE_UNUSED,
+ virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED,
+ virDomainDeviceInfoPtr info,
+ void *data)
virDomainPCIAddressSetAllMultiIter()?
Or not :)
[...]
+/**
+ * virDomainPCIAddressSetAllMulti():
+ *
+ * @def: the domain definition whose devices may need adjusting
+ * @addrs: address set keeping track of all addresses in use.
+ *
+ * Look for any PCI slots that have multiple functions assigned, and
+ * set multi to YES in the address for the device at function 0
s/YES/ON/
+ * (unless it has been explicitly set to NO).
s/NO/OFF/
+ *
+ * No return code, since there is no possibility of failure.
+ */
+void
+virDomainPCIAddressSetAllMulti(virDomainDefPtr def,
+ virDomainPCIAddressSetPtr addrs)
I also just realized that virDomainPCIAddressSetAllMulti()
might be mistaken for vir::DomainPCIAddressSet::AllMulti()
instead of vir::Domain::PCIAddressSetAllMulti().
The joys of pretending to use an object-oriented language
when actually all you have is plain old C :)
+{
+ /* Scan through all the slots in @addrs looking for any that have
+ * more than just function 0 marked as in use, then use an
+ * iterator to find the DeviceInfo that uses function 0 on that
+ * slot and mark it as multi = YES
s/YES/ON/
+ */
+ size_t busIdx, slotIdx;
+
+ for (busIdx = 0; busIdx < addrs->nbuses; busIdx++) {
+ virDomainPCIAddressBusPtr bus = &addrs->buses[busIdx];
+
+ for (slotIdx = bus->minSlot; slotIdx <= bus->maxSlot; slotIdx++) {
+ if (bus->slot[slotIdx].functions > 1) {
+ virPCIDeviceAddress addr = { .domain = 0,
This doesn't look right. What happens if the multifunction
device is at eg. 0001:00:04.0?
...
Oh, never mind, apparently virDomainPCIAddressSet has no
concept of PCI domains anyways... So that's an existing
limitation - one that I assume we're going to have to
address, especially now that ppc64 guests are going to
want to use *lots* of separate PCI domains.
On the other hand, at the point where you're going to call
this function haven't PCI addresses already been assigned
and stored in @def? Why not iterate over it twice to make
the comparison future-proof? The computational cost would
be the same AFAICT.
--
Andrea Bolognani / Red Hat / Virtualization
10:55 a.m.
New subject: [libvirt] [PATCH v2 1/2] conf: new function virDomainPCIAddressSetAllMulti()
This utility function iterates through all devices looking for any
with a PCI address that has function != 0 (which implies that multiple
functions are in use on that slot), then uses an inner iterator to
find the device that's on function 0 of that same slot and sets the
"multi" in its virDomainDeviceInfo (as long as it hasn't already been
set explicitly by someone who presumably has better information than
we do).
It isn't yet called from anywhere, so will have no functional effect.
---
Change from V1: implement as two nested deviceinfo iterators as
suggested by Andrea.
src/conf/domain_addr.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_addr.h | 3 ++
src/libvirt_private.syms | 1 +
3 files changed, 86 insertions(+)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 4d14ac4..453f287 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -867,6 +867,88 @@ virDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs,
}
+static int
+virDomainPCIAddressSetMultiIter(virDomainDefPtr def ATTRIBUTE_UNUSED,
+ virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED,
+ virDomainDeviceInfoPtr info,
+ void *data)
+{
+ virPCIDeviceAddressPtr testAddr = data;
+ virPCIDeviceAddressPtr thisAddr;
+
+ if (!info || info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)
+ return 0;
+
+ thisAddr = &info->addr.pci;
+
+ if (thisAddr->domain == testAddr->domain &&
+ thisAddr->bus == testAddr->bus &&
+ thisAddr->slot == testAddr->slot &&
+ thisAddr->function == 0) {
+
+ /* only set to ON if it wasn't previously set
+ * (assuming that the user must have better information
+ * than us if they explicitly set it OFF)
+ */
+ if (thisAddr->multi == VIR_TRISTATE_SWITCH_ABSENT)
+ thisAddr->multi = VIR_TRISTATE_SWITCH_ON;
+
+ return -1; /* finish early, *NOT* an error */
+ }
+
+ return 0;
+}
+
+
+static int
+virDomainPCIAddressSetAllMultiIter(virDomainDefPtr def,
+ virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED,
+ virDomainDeviceInfoPtr info,
+ void *data ATTRIBUTE_UNUSED)
+{
+ virPCIDeviceAddressPtr testAddr;
+
+ if (!info || info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)
+ return 0;
+
+ testAddr = &info->addr.pci;
+ if (testAddr->function != 0) {
+ ignore_value(virDomainDeviceInfoIterate(def,
+ virDomainPCIAddressSetMultiIter,
+ testAddr));
+ }
+
+ return 0;
+}
+
+
+/**
+ * virDomainPCIAddressSetAllMulti():
+ *
+ * @def: the domain definition whose devices may need adjusting
+ * @addrs: address set keeping track of all addresses in use.
+ *
+ * Look for any PCI slots that have multiple functions assigned, and
+ * set multi to ON in the address for the device at function 0
+ * (unless it has been explicitly set to OFF).
+ *
+ * No return code, since there is no possibility of failure.
+ */
+void
+virDomainPCIAddressSetAllMulti(virDomainDefPtr def)
+{
+ /* Use nested iterators over all the devices - the outer iterator
+ * scans through all the devices looking for those whose address
+ * has a non-0 function; when one is found, the inner iterator looks
+ * for the device that uses function 0 on the same slot and marks
+ * it as multi = ON
+ */
+ ignore_value(virDomainDeviceInfoIterate(def,
+ virDomainPCIAddressSetAllMultiIter,
+ NULL));
+}
+
+
static char*
virDomainCCWAddressAsString(virDomainDeviceCCWAddressPtr addr)
{
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 5f0924e..c728c00 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -173,6 +173,9 @@ int virDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr
addrs,
virDomainPCIConnectFlags flags)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+void virDomainPCIAddressSetAllMulti(virDomainDefPtr def)
+ ATTRIBUTE_NONNULL(1);
+
struct _virDomainCCWAddressSet {
virHashTablePtr defined;
virDomainDeviceCCWAddress next;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 82adf7e..bdc0272 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -104,6 +104,7 @@ virDomainPCIAddressReserveAddr;
virDomainPCIAddressReserveNextAddr;
virDomainPCIAddressReserveNextSlot;
virDomainPCIAddressReserveSlot;
+virDomainPCIAddressSetAllMulti;
virDomainPCIAddressSetAlloc;
virDomainPCIAddressSetFree;
virDomainPCIAddressSetGrow;
--
2.7.4
11:47 a.m.
New subject: [libvirt] [PATCH v2 1/2] conf: new function virDomainPCIAddressSetAllMulti()
On Tue, 2017-01-10 at 11:55 -0500, Laine Stump wrote:
This utility function iterates through all devices looking for any
with a PCI address that has function != 0 (which implies that multiple
functions are in use on that slot), then uses an inner iterator to
find the device that's on function 0 of that same slot and sets the
"multi" in its virDomainDeviceInfo (as long as it hasn't already been
set explicitly by someone who presumably has better information than
we do).
It isn't yet called from anywhere, so will have no functional effect.
[...]
+static int
+virDomainPCIAddressSetMultiIter(virDomainDefPtr def ATTRIBUTE_UNUSED,
+ virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED,
+ virDomainDeviceInfoPtr info,
+ void *data)
virDomainPCIAddressSetAllMultiIterInner()?
Or maybe virDomainPCIAddressSetAllMultiInnerIter()?
Just kidding ;)
[...]
+static int
+virDomainPCIAddressSetAllMultiIter(virDomainDefPtr def,
+ virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED,
+ virDomainDeviceInfoPtr info,
+ void *data ATTRIBUTE_UNUSED)
+{
+ virPCIDeviceAddressPtr testAddr;
+
+ if (!info || info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)
+ return 0;
+
+ testAddr = &info->addr.pci;
Empty line here.
+ if (testAddr->function != 0) {
+ ignore_value(virDomainDeviceInfoIterate(def,
+ virDomainPCIAddressSetMultiIter,
+ testAddr));
+ }
+
+ return 0;
+}
ACK
--
Andrea Bolognani / Red Hat / Virtualization
2:23 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: use virDomainPCIAddressSetAllMulti() to set multi when needed
If there are multiple devices assigned to the different functions of a
single PCI slot, they will not work properly if the device at function
0 doesn't have its "multi" attribute turned on, so it makes sense for
libvirt to turn it on during PCI address assignment. Just in case a
future change in the configuration eliminates the devices on all non-0
functions, and in case some guest OS would be confused by having the
multi flag of function 0 modified to "no" at the next boot of the
guest (it may seem like an inconsequential guest ABI change, but it
*is* a guest ABI change).
We are set multi during PCI address assignment (which happens in the
domain post-parse callback) rather than at qemu runtime so that the
setting will be stored in config and persist even in the case that the
devices in the non-0 functions of the slot are later removed from the
config - this is done to prevent a "silent" guest ABI change (although
it can only happen after shutting down and restarting the guest, and
although it may seem like an inconsequential guest ABI change, it *is*
a guest ABI change to turn off the multi bit).
---
src/qemu/qemu_domain_address.c | 5 +
.../qemuxml2argv-q35-multifunction.args | 42 ++++++++
.../qemuxml2argv-q35-multifunction.xml | 51 +++++++++
tests/qemuxml2argvtest.c | 23 ++++
.../qemuxml2xmlout-q35-multifunction.xml | 120 +++++++++++++++++++++
tests/qemuxml2xmltest.c | 23 ++++
6 files changed, 264 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-multifunction.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-multifunction.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-multifunction.xml
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 4a2a489..d350921 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -2088,6 +2088,11 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
if (qemuDomainAssignDevicePCISlots(def, qemuCaps, addrs) < 0)
goto cleanup;
+ /* set multi attribute for devices at function 0 of
+ * any slot that has multiple functions in use
+ */
+ virDomainPCIAddressSetAllMulti(def, addrs);
+
for (i = 0; i < def->ncontrollers; i++) {
virDomainControllerDefPtr cont = def->controllers[i];
int idx = cont->idx;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-multifunction.args
b/tests/qemuxml2argvdata/qemuxml2argv-q35-multifunction.args
new file mode 100644
index 0000000..6c4e9b2
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-multifunction.args
@@ -0,0 +1,42 @@
+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 ioh3420,port=0x10,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,\
+addr=0x2 \
+-device ioh3420,port=0x11,chassis=2,id=pci.2,bus=pcie.0,addr=0x2.0x1 \
+-device ioh3420,port=0x12,chassis=3,id=pci.3,bus=pcie.0,addr=0x2.0x2 \
+-device ioh3420,port=0x18,chassis=4,id=pci.4,bus=pcie.0,multifunction=on,\
+addr=0x3 \
+-device ioh3420,port=0x19,chassis=5,id=pci.5,bus=pcie.0,multifunction=on,\
+addr=0x3.0x1 \
+-device ioh3420,port=0x20,chassis=6,id=pci.6,bus=pcie.0,multifunction=off,\
+addr=0x4 \
+-device ioh3420,port=0x21,chassis=7,id=pci.7,bus=pcie.0,addr=0x4.0x1 \
+-device ioh3420,port=0x8,chassis=8,id=pci.8,bus=pcie.0,addr=0x1 \
+-device ioh3420,port=0x28,chassis=9,id=pci.9,bus=pcie.0,addr=0x5 \
+-device ioh3420,port=0x30,chassis=10,id=pci.10,bus=pcie.0,addr=0x6 \
+-device ioh3420,port=0x38,chassis=11,id=pci.11,bus=pcie.0,addr=0x7 \
+-device ioh3420,port=0x40,chassis=12,id=pci.12,bus=pcie.0,addr=0x8 \
+-device ioh3420,port=0x48,chassis=13,id=pci.13,bus=pcie.0,addr=0x9 \
+-device ioh3420,port=0x50,chassis=14,id=pci.14,bus=pcie.0,addr=0xa \
+-device ioh3420,port=0x58,chassis=15,id=pci.15,bus=pcie.0,addr=0xb \
+-device ioh3420,port=0x60,chassis=16,id=pci.16,bus=pcie.0,addr=0xc \
+-device ioh3420,port=0x68,chassis=17,id=pci.17,bus=pcie.0,addr=0xd \
+-device ioh3420,port=0x70,chassis=18,id=pci.18,bus=pcie.0,addr=0xe \
+-device nec-usb-xhci,id=usb,bus=pci.1,addr=0x0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.2,addr=0x0
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-multifunction.xml
b/tests/qemuxml2argvdata/qemuxml2argv-q35-multifunction.xml
new file mode 100644
index 0000000..c1edca1
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-multifunction.xml
@@ -0,0 +1,51 @@
+<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' model='pcie-root'/>
+ <controller type='pci' model='pcie-root-port'>
+ <address type='pci' slot='2' function='0'/>
+ </controller>
+ <controller type='pci' model='pcie-root-port'>
+ <address type='pci' slot='2' function='1'/>
+ </controller>
+ <controller type='pci' model='pcie-root-port'>
+ <address type='pci' slot='2' function='2'/>
+ </controller>
+ <controller type='pci' model='pcie-root-port'>
+ <address type='pci' slot='3' function='0'/>
+ </controller>
+ <controller type='pci' model='pcie-root-port'>
+ <address type='pci' slot='3' function='1'
multifunction='on'/>
+ </controller>
+ <controller type='pci' model='pcie-root-port'>
+ <address type='pci' slot='4' function='0'
multifunction='off'/>
+ </controller>
+ <controller type='pci' model='pcie-root-port'>
+ <address type='pci' slot='4' function='1'/>
+ </controller>
+ <controller type='pci' model='pcie-root-port'/>
+ <controller type='pci' model='pcie-root-port'/>
+ <controller type='pci' model='pcie-root-port'/>
+ <controller type='pci' model='pcie-root-port'/>
+ <controller type='pci' model='pcie-root-port'/>
+ <controller type='pci' model='pcie-root-port'/>
+ <controller type='pci' model='pcie-root-port'/>
+ <controller type='pci' model='pcie-root-port'/>
+ <controller type='pci' model='pcie-root-port'/>
+ <controller type='pci' model='pcie-root-port'/>
+ <controller type='pci' model='pcie-root-port'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index ab3ad08..307caa8 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1924,6 +1924,29 @@ mymain(void)
QEMU_CAPS_ICH9_USB_EHCI1,
QEMU_CAPS_NEC_USB_XHCI,
QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
+ DO_TEST("q35-multifunction",
+ QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY,
+ QEMU_CAPS_DEVICE_VIRTIO_RNG,
+ QEMU_CAPS_OBJECT_RNG_RANDOM,
+ QEMU_CAPS_NETDEV,
+ QEMU_CAPS_DEVICE_VIRTIO_NET,
+ QEMU_CAPS_DEVICE_VIRTIO_GPU,
+ QEMU_CAPS_VIRTIO_GPU_VIRGL,
+ QEMU_CAPS_VIRTIO_KEYBOARD,
+ QEMU_CAPS_VIRTIO_MOUSE,
+ QEMU_CAPS_VIRTIO_TABLET,
+ QEMU_CAPS_VIRTIO_INPUT_HOST,
+ QEMU_CAPS_VIRTIO_SCSI,
+ QEMU_CAPS_FSDEV,
+ QEMU_CAPS_FSDEV_WRITEOUT,
+ QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_IOH3420,
+ QEMU_CAPS_ICH9_AHCI,
+ QEMU_CAPS_PCI_MULTIFUNCTION,
+ QEMU_CAPS_ICH9_USB_EHCI1,
+ QEMU_CAPS_NEC_USB_XHCI,
+ QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
DO_TEST("q35-virt-manager-basic",
QEMU_CAPS_KVM,
QEMU_CAPS_RTC,
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-multifunction.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-multifunction.xml
new file mode 100644
index 0000000..7da78a5
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35-multifunction.xml
@@ -0,0 +1,120 @@
+<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='pcie-root-port'>
+ <model name='ioh3420'/>
+ <target chassis='1' port='0x10'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x02' function='0x0' multifunction='on'/>
+ </controller>
+ <controller type='pci' index='2'
model='pcie-root-port'>
+ <model name='ioh3420'/>
+ <target chassis='2' port='0x11'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x02' function='0x1'/>
+ </controller>
+ <controller type='pci' index='3'
model='pcie-root-port'>
+ <model name='ioh3420'/>
+ <target chassis='3' port='0x12'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x02' function='0x2'/>
+ </controller>
+ <controller type='pci' index='4'
model='pcie-root-port'>
+ <model name='ioh3420'/>
+ <target chassis='4' port='0x18'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0' multifunction='on'/>
+ </controller>
+ <controller type='pci' index='5'
model='pcie-root-port'>
+ <model name='ioh3420'/>
+ <target chassis='5' port='0x19'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x1' multifunction='on'/>
+ </controller>
+ <controller type='pci' index='6'
model='pcie-root-port'>
+ <model name='ioh3420'/>
+ <target chassis='6' port='0x20'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x04' function='0x0' multifunction='off'/>
+ </controller>
+ <controller type='pci' index='7'
model='pcie-root-port'>
+ <model name='ioh3420'/>
+ <target chassis='7' port='0x21'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x04' function='0x1'/>
+ </controller>
+ <controller type='pci' index='8'
model='pcie-root-port'>
+ <model name='ioh3420'/>
+ <target chassis='8' port='0x8'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x0'/>
+ </controller>
+ <controller type='pci' index='9'
model='pcie-root-port'>
+ <model name='ioh3420'/>
+ <target chassis='9' port='0x28'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x05' function='0x0'/>
+ </controller>
+ <controller type='pci' index='10'
model='pcie-root-port'>
+ <model name='ioh3420'/>
+ <target chassis='10' port='0x30'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x06' function='0x0'/>
+ </controller>
+ <controller type='pci' index='11'
model='pcie-root-port'>
+ <model name='ioh3420'/>
+ <target chassis='11' port='0x38'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x07' function='0x0'/>
+ </controller>
+ <controller type='pci' index='12'
model='pcie-root-port'>
+ <model name='ioh3420'/>
+ <target chassis='12' port='0x40'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x08' function='0x0'/>
+ </controller>
+ <controller type='pci' index='13'
model='pcie-root-port'>
+ <model name='ioh3420'/>
+ <target chassis='13' port='0x48'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x09' function='0x0'/>
+ </controller>
+ <controller type='pci' index='14'
model='pcie-root-port'>
+ <model name='ioh3420'/>
+ <target chassis='14' port='0x50'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x0a' function='0x0'/>
+ </controller>
+ <controller type='pci' index='15'
model='pcie-root-port'>
+ <model name='ioh3420'/>
+ <target chassis='15' port='0x58'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x0b' function='0x0'/>
+ </controller>
+ <controller type='pci' index='16'
model='pcie-root-port'>
+ <model name='ioh3420'/>
+ <target chassis='16' port='0x60'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x0c' function='0x0'/>
+ </controller>
+ <controller type='pci' index='17'
model='pcie-root-port'>
+ <model name='ioh3420'/>
+ <target chassis='17' port='0x68'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x0d' function='0x0'/>
+ </controller>
+ <controller type='pci' index='18'
model='pcie-root-port'>
+ <model name='ioh3420'/>
+ <target chassis='18' port='0x70'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x0e' function='0x0'/>
+ </controller>
+ <controller type='usb' index='0' model='nec-xhci'>
+ <address type='pci' domain='0x0000' bus='0x01'
slot='0x00' function='0x0'/>
+ </controller>
+ <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='virtio'>
+ <address type='pci' domain='0x0000' bus='0x02'
slot='0x00' function='0x0'/>
+ </memballoon>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index ddd17cb..569a375 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -795,6 +795,29 @@ mymain(void)
QEMU_CAPS_ICH9_USB_EHCI1,
QEMU_CAPS_NEC_USB_XHCI,
QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
+ DO_TEST("q35-multifunction",
+ QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY,
+ QEMU_CAPS_DEVICE_VIRTIO_RNG,
+ QEMU_CAPS_OBJECT_RNG_RANDOM,
+ QEMU_CAPS_NETDEV,
+ QEMU_CAPS_DEVICE_VIRTIO_NET,
+ QEMU_CAPS_DEVICE_VIRTIO_GPU,
+ QEMU_CAPS_VIRTIO_GPU_VIRGL,
+ QEMU_CAPS_VIRTIO_KEYBOARD,
+ QEMU_CAPS_VIRTIO_MOUSE,
+ QEMU_CAPS_VIRTIO_TABLET,
+ QEMU_CAPS_VIRTIO_INPUT_HOST,
+ QEMU_CAPS_VIRTIO_SCSI,
+ QEMU_CAPS_FSDEV,
+ QEMU_CAPS_FSDEV_WRITEOUT,
+ QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_IOH3420,
+ QEMU_CAPS_ICH9_AHCI,
+ QEMU_CAPS_PCI_MULTIFUNCTION,
+ QEMU_CAPS_ICH9_USB_EHCI1,
+ QEMU_CAPS_NEC_USB_XHCI,
+ QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
DO_TEST("q35-virt-manager-basic",
QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY,
QEMU_CAPS_DEVICE_VIRTIO_RNG,
--
2.7.4
11:59 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: use virDomainPCIAddressSetAllMulti() to set multi when needed
On Tue, 2017-01-10 at 03:23 -0500, Laine Stump wrote:
If there are multiple devices assigned to the different functions of
a
single PCI slot, they will not work properly if the device at function
0 doesn't have its "multi" attribute turned on, so it makes sense for
libvirt to turn it on during PCI address assignment. Just in case a
future change in the configuration eliminates the devices on all non-0
functions, and in case some guest OS would be confused by having the
multi flag of function 0 modified to "no" at the next boot of the
guest (it may seem like an inconsequential guest ABI change, but it
*is* a guest ABI change).
We are set multi during PCI address assignment (which happens in the
domain post-parse callback) rather than at qemu runtime so that the
setting will be stored in config and persist even in the case that the
devices in the non-0 functions of the slot are later removed from the
config - this is done to prevent a "silent" guest ABI change (although
it can only happen after shutting down and restarting the guest, and
although it may seem like an inconsequential guest ABI change, it *is*
a guest ABI change to turn off the multi bit).
The commit messages seems to contain editing artifacts that
I trust you'll get rid of before pushing.
[...]
+ /* set multi attribute for devices at function 0 of
+ * any slot that has multiple functions in use
+ */
+ virDomainPCIAddressSetAllMulti(def, addrs);
This will of course have to be changed now that
virDomainPCIAddressSetAllMulti() takes a single argument.
ACK
--
Andrea Bolognani / Red Hat / Virtualization
2903
days inactive
2903
days old
6 comments
2 participants
participants (2)
-
Andrea Bolognani -
Laine Stump