On Mon, 2016-11-07 at 14:50 -0500, Laine Stump wrote:
Before now, all the qemu hotplug functions assumed that all devices
to
be hotplugged were legacy PCI endpoint devices
(VIR_PCI_CONNECT_TYPE_PCI_DEVICE). This worked out "okay", because all
devices *are* legacy PCI endpoint devices on x86/440fx machinetypes,
and hotplug didn't work properly on machinetypes using PCIe anyway
(hotplugging onto a legacy PCI slot doesn't work, and until commit
b87703cf any attempt to manually specify a PCIe address for a
hotplugged device would be erroneously rejected).
This patch makes all qemu hotplug operations honor the pciConnectFlags
set by the single all-knowing function
qemuDomainDeviceCalculatePCIConnectFlags(). This is done in 3 steps,
but in a single commit since we would have to touch the other points
at each step anyway:
1) add a flags argument to the hypervisor-agnostic
virDomainPCIAddressEnsureAddr() (previously it hardcoded
..._PCI_DEVICE)
2) add a new qemu-specific function qemuDomainEnsurePCIAddress() which
gets the correct pciConnectFlags for the device from
qemuDomainDeviceConnectFlags(), then calls
virDomainPCIAddressEnsureAddr().
3) in qemu_hotplug.c replace all calls to
virDomainPCIAddressEnsureAddr() with calls to
qemuDomainEnsurePCIAddress()
So in effect, we're putting a "shim" on top of all calls to
virDomainPCIAddressEnsureAddr() that sets the right pciConnectFlags.
---
src/conf/domain_addr.c | 10 ++--------
src/conf/domain_addr.h | 3 ++-
src/qemu/qemu_domain_address.c | 27 +++++++++++++++++++++++++++
src/qemu/qemu_domain_address.h | 4 ++++
src/qemu/qemu_hotplug.c | 23 ++++++++++++++++-------
5 files changed, 51 insertions(+), 16 deletions(-)
[...]
+/**
+ * qemuDomainEnsurePCIAddress:
+ *
+ * if @dev should have a PCI address but doesn't, assign an address on
+ * a compatible PCI bus, and set it in @dev->...info. If there is an
+ * address already, validate that it is on a compatible bus, based on
+ * @dev->...info.pciConnectFlags.
+ *
+ * @obj: the virDomainObjPtr for the domain. This will include
+ * qemuCaps and address cache (if there is one)
+ *
+ * @dev: the device that we need to ensure has a PCI address
Please move the description of the function arguments before
the description of the function itself.
+ * returns 0 on success -1 on failure.
+ */
+int
+qemuDomainEnsurePCIAddress(virDomainObjPtr obj,
+ virDomainDeviceDefPtr dev)
+{
+ qemuDomainObjPrivatePtr priv = obj->privateData;
+ virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev);
virDomainDeviceGetInfo() can return NULL...
+ qemuDomainFillDevicePCIConnectFlags(obj->def, dev,
priv->qemuCaps);
+
+ return virDomainPCIAddressEnsureAddr(priv->pciaddrs, info,
+ info->pciConnectFlags);
... but you don't check for that before dereferencing info
here. I guess this function will never be called on a device
that doesn't have a DeviceInfo, but it's probably a good
idea to explicitly check anyways in case the callers change.
ACK once the comments above have been taken care of.
--
Andrea Bolognani / Red Hat / Virtualization