This patch introduces two helpers that will be used in the next patches. One is virpci.c:virPCIDeviceIsMultifunction(), and the other is virhostdev.c:virHostdevIsPCIMultifunctionDevice(). Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/libvirt_private.syms | 2 ++ src/util/virhostdev.c | 25 +++++++++++++++++++++++++ src/util/virhostdev.h | 3 +++ src/util/virpci.c | 17 +++++++++++++++++ src/util/virpci.h | 2 ++ 5 files changed, 49 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ebf830791e..962963a3ff 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2162,6 +2162,7 @@ virHostCPUStatsAssign; # util/virhostdev.h virHostdevFindUSBDevice; virHostdevIsMdevDevice; +virHostdevIsPCIMultifunctionDevice; virHostdevIsSCSIDevice; virHostdevIsVFIODevice; virHostdevManagerGetDefault; @@ -2768,6 +2769,7 @@ virPCIDeviceGetUnbindFromStub; virPCIDeviceGetUsedBy; virPCIDeviceHasPCIExpressLink; virPCIDeviceIsAssignable; +virPCIDeviceIsMultifunction; virPCIDeviceIsPCIExpress; virPCIDeviceListAdd; virPCIDeviceListAddCopy; diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index f8f7989206..f0526d97d0 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -2564,3 +2564,28 @@ virHostdevUpdateActiveNVMeDevices(virHostdevManagerPtr hostdev_mgr, } goto cleanup; } + + +bool +virHostdevIsPCIMultifunctionDevice(virDomainHostdevDefPtr hostdev) +{ + g_autoptr(virPCIDevice) pciDev = NULL; + virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; + + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || + hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) + return false; + + /* Libvirt should be able to perform all the operations in + * virPCIDeviceNew() even if it's running unprivileged, so if this + * fails, the device apparently doesn't currently exist on the host. + * Since we can't speculate, assume this device is not multifunction. + */ + pciDev = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); + + if (!pciDev) + return false; + + return virPCIDeviceIsMultifunction(pciDev); +} diff --git a/src/util/virhostdev.h b/src/util/virhostdev.h index ae84ed3d20..80aea577ed 100644 --- a/src/util/virhostdev.h +++ b/src/util/virhostdev.h @@ -198,6 +198,9 @@ bool virHostdevIsMdevDevice(const virDomainHostdevDef *hostdev) ATTRIBUTE_NONNULL(1); bool +virHostdevIsPCIMultifunctionDevice(virDomainHostdevDefPtr hostdev) + ATTRIBUTE_NONNULL(1); +bool virHostdevIsVFIODevice(const virDomainHostdevDef *hostdev) ATTRIBUTE_NONNULL(1); diff --git a/src/util/virpci.c b/src/util/virpci.c index 0b1222373e..f564d7b9fd 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -2842,6 +2842,23 @@ int virPCIGetHeaderType(virPCIDevicePtr dev, int *hdrType) } +bool +virPCIDeviceIsMultifunction(virPCIDevicePtr dev) +{ + int fd; + uint8_t type; + + if ((fd = virPCIDeviceConfigOpen(dev)) < 0) + return -1; + + type = virPCIDeviceRead8(dev, fd, PCI_HEADER_TYPE); + + virPCIDeviceConfigClose(dev, fd); + + return type & PCI_HEADER_TYPE_MULTI; +} + + void virPCIEDeviceInfoFree(virPCIEDeviceInfoPtr dev) { diff --git a/src/util/virpci.h b/src/util/virpci.h index f6796fc422..9d8dcfec05 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -271,6 +271,8 @@ int virPCIDeviceGetLinkCapSta(virPCIDevicePtr dev, int virPCIGetHeaderType(virPCIDevicePtr dev, int *hdrType); +bool virPCIDeviceIsMultifunction(virPCIDevicePtr dev); + void virPCIEDeviceInfoFree(virPCIEDeviceInfoPtr dev); ssize_t virPCIGetMdevTypes(const char *sysfspath, -- 2.24.1

From: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> For existing domains using the primary function alone of a multifunction card, the card is still treated as a multifunction card. This is done to prevent hotplug of other functions when the primary function is already hotplugged. If the secondary functions are part of the xml without the primary function being part of the xml, this has never been supported. So, Libvirt doesn't consider this either as a multifunction card. Since we're now checking PCI headers via virHostdevIsPCIMultifunctionDevice(), changes in virpcitestdata files were required to allow the test suit to recognize the 0005:90:01.N test device as multifunction. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_domain.h | 11 ++ src/qemu/qemu_domain_address.c | 148 +++++++++++++++++- tests/qemuhotplugtest.c | 1 + .../hostdev-pci-address-unassigned.args | 9 +- .../hostdev-pci-multifunction.args | 18 ++- .../hostdev-pci-multifunction.xml | 8 +- .../qemuxml2argvdata/pseries-hostdevs-1.args | 5 +- .../qemuxml2argvdata/pseries-hostdevs-3.args | 5 +- tests/qemuxml2argvtest.c | 6 +- .../hostdev-pci-address-unassigned.xml | 8 +- .../hostdev-pci-multifunction.xml | 24 +-- .../qemuxml2xmloutdata/pseries-hostdevs-1.xml | 4 +- .../qemuxml2xmloutdata/pseries-hostdevs-3.xml | 4 +- tests/virpcitestdata/0005-90-01.1.config | Bin 256 -> 256 bytes tests/virpcitestdata/0005-90-01.2.config | Bin 256 -> 256 bytes tests/virpcitestdata/0005-90-01.3.config | Bin 0 -> 256 bytes 16 files changed, 204 insertions(+), 47 deletions(-) create mode 100644 tests/virpcitestdata/0005-90-01.3.config diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index c581b3a162..c54af16fa5 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -612,6 +612,17 @@ struct _qemuDomainSaveCookie { G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuDomainSaveCookie, virObjectUnref); +typedef struct _qemuDomainPCIHostdevdata qemuDomainPCIHostdevdata; +typedef qemuDomainPCIHostdevdata *qemuDomainPCIHostdevDataPtr; +struct _qemuDomainPCIHostdevdata { + const virDomainDef *def; + virDomainPCIAddressSetPtr addrs; + virDomainHostdevDefPtr device; +}; + +typedef int (*virDomainPCIHostdevCallback)(qemuDomainPCIHostdevDataPtr data, + virDomainHostdevDefPtr hostdev); + typedef struct _qemuDomainXmlNsDef qemuDomainXmlNsDef; typedef qemuDomainXmlNsDef *qemuDomainXmlNsDefPtr; struct _qemuDomainXmlNsDef { diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index bff81082ef..3fed22d720 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1409,11 +1409,134 @@ qemuDomainSetupIsolationGroups(virDomainDefPtr def) } +#define PCI_MAX_BRIDGE_NUMBER 0xff +#define PCI_MAX_DEVICES 32 + + +/** + * qemuDomainPCIHostDevicesIter: + * @data - The data->device is the one which is called-back with for + * each hostdev + * cb() - callback to be called for each hostdev + * Return : + * If the callback for any of the hostdev fails, the Iter returns + * with the return value for that callback. + * Zero on success. + */ +static +int qemuDomainPCIHostDevicesIter(qemuDomainPCIHostdevDataPtr data, + virDomainPCIHostdevCallback cb) +{ + size_t i; + int ret = -1; + + /* Iterate through the PCI Hostdevices, the Mdev source is of type + * UUID, so skip that. */ + for (i = 0; i < data->def->nhostdevs; i++) { + virDomainHostdevSubsysPtr subsys = &data->def->hostdevs[i]->source.subsys; + virDomainHostdevDefPtr hostdev = data->def->hostdevs[i]; + if (data->device == hostdev) + continue; + if (data->def->hostdevs[i]->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + continue; + if (subsys->type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && + subsys->type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST && + (subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV && + subsys->u.mdev.model == VIR_MDEV_MODEL_TYPE_VFIO_PCI)) { + continue; + } + + if ((ret = cb(data, hostdev)) != 0) + return ret; + } + + return 0; +} + + + +static int +qemuDomainFindNextAggregationSlotIdxIter(virDomainDefPtr def G_GNUC_UNUSED, + virDomainDeviceDefPtr dev G_GNUC_UNUSED, + virDomainDeviceInfoPtr info, + void *opaque) +{ + int *aggregationSlotIdx = opaque; + + if (info && info->aggregateSlotIdx == *aggregationSlotIdx) + return -1; + + return 0; +} + + +static unsigned int +qemuDomainFindNextSlotAggregationIdx(virDomainDefPtr def) +{ + int aggregateSlotIdx = 2; + + while (aggregateSlotIdx < PCI_MAX_BRIDGE_NUMBER * PCI_MAX_DEVICES && + virDomainDeviceInfoIterate(def, + qemuDomainFindNextAggregationSlotIdxIter, + &aggregateSlotIdx) < 0) { + aggregateSlotIdx++; + } + + return aggregateSlotIdx; +} + + +static int +qemuDomainDefHostdevGetSlotAggregateIdx(qemuDomainPCIHostdevDataPtr data, + virDomainHostdevDefPtr hostdev) +{ + if (data->device && + virHostdevPCIDevicesBelongToSameSlot(data->device, hostdev)) { + if (hostdev->info->aggregateSlotIdx > 0) + return hostdev->info->aggregateSlotIdx; + } + + return 0; +} + +/** + * qemuDomainDefDeviceFindSlotAggregateIdx: + * @def : domain def + * @dev : Find the slot aggregate for the device if other + * functions are already part of the def and have + * a slot aggreate idx assigned. + * Return: + * -1: if not assigned. + * 0: If the device is not a hostdev or not a + * multifunction device. + * >0: If assinged a value; + **/ +int +qemuDomainDefDeviceFindSlotAggregateIdx(virDomainDefPtr def, + virDomainDeviceDefPtr dev) +{ + int aggregateSlotIdx = 0; + virDomainHostdevDefPtr hostdev = dev->data.hostdev; + qemuDomainPCIHostdevdata temp = {def, NULL, hostdev}; + + /* Only PCI host devices are subject to isolation */ + if (!virHostdevIsPCIMultifunctionDevice(hostdev)) + return 0; + + aggregateSlotIdx = qemuDomainPCIHostDevicesIter(&temp, qemuDomainDefHostdevGetSlotAggregateIdx); + if (aggregateSlotIdx > 0) + return aggregateSlotIdx; + + return -1; +} + + void -qemuDomainSetDeviceSlotAggregateIdx(virDomainDefPtr def G_GNUC_UNUSED, +qemuDomainSetDeviceSlotAggregateIdx(virDomainDefPtr def, virDomainDeviceDefPtr dev) { virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev); + int aggregateSlotIdx = 0; if (!info) return; @@ -1426,6 +1549,12 @@ qemuDomainSetDeviceSlotAggregateIdx(virDomainDefPtr def G_GNUC_UNUSED, cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT) { info->aggregateSlotIdx = 1; } + } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { + aggregateSlotIdx = qemuDomainDefDeviceFindSlotAggregateIdx(def, dev); + if (aggregateSlotIdx > 0) + info->aggregateSlotIdx = aggregateSlotIdx; + else if (aggregateSlotIdx < 0) + info->aggregateSlotIdx = qemuDomainFindNextSlotAggregationIdx(def); } return; @@ -2364,10 +2493,12 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, /* Host PCI devices */ for (i = 0; i < def->nhostdevs; i++) { - virDomainHostdevSubsysPtr subsys = &def->hostdevs[i]->source.subsys; - if (!virDeviceInfoPCIAddressIsWanted(def->hostdevs[i]->info)) + int function = 0; + virDomainHostdevDefPtr hostdev = def->hostdevs[i]; + virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; + if (!virDeviceInfoPCIAddressIsWanted(hostdev->info)) continue; - if (def->hostdevs[i]->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) continue; if (subsys->type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && subsys->type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST && @@ -2381,9 +2512,14 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED) continue; - if (qemuDomainPCIAddressReserveNextAddr(addrs, - def->hostdevs[i]->info) < 0) + if (hostdev->info->aggregateSlotIdx > 1) + function = hostdev->source.subsys.u.pci.addr.function; + + if (virDomainPCIAddressReserveNextAddr(addrs, hostdev->info, + hostdev->info->pciConnectFlags, + function) < 0) { return -1; + } } /* memballoon. the qemu driver only accepts virtio memballoon devices */ diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 6a3e61c54b..48284fdb3e 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -87,6 +87,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_VNC); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_SPICE); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_SPICE_FILE_XFER_DISABLE); + virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY); if (qemuTestCapsCacheInsert(driver.qemuCapsCache, priv->qemuCaps) < 0) return -1; diff --git a/tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args b/tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args index 42fae17444..8e031c0f1f 100644 --- a/tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args +++ b/tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args @@ -25,7 +25,8 @@ server,nowait \ -no-shutdown \ -no-acpi \ -usb \ --device vfio-pci,host=0005:90:01.0,id=hostdev0,bus=pci.0,addr=0x3 \ --device vfio-pci,host=0005:90:01.2,id=hostdev2,bus=pci.0,addr=0x4 \ --device vfio-pci,host=0005:90:01.3,id=hostdev3,bus=pci.0,addr=0x5 \ --device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 +-device vfio-pci,host=0005:90:01.0,id=hostdev0,bus=pci.0,multifunction=on,\ +addr=0x3 \ +-device vfio-pci,host=0005:90:01.2,id=hostdev2,bus=pci.0,addr=0x3.0x2 \ +-device vfio-pci,host=0005:90:01.3,id=hostdev3,bus=pci.0,addr=0x3.0x3 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/hostdev-pci-multifunction.args b/tests/qemuxml2argvdata/hostdev-pci-multifunction.args index d8690c010b..3bf3629d48 100644 --- a/tests/qemuxml2argvdata/hostdev-pci-multifunction.args +++ b/tests/qemuxml2argvdata/hostdev-pci-multifunction.args @@ -25,11 +25,13 @@ server,nowait \ -no-shutdown \ -no-acpi \ -usb \ --device vfio-pci,host=0005:90:01.0,id=hostdev0,bus=pci.0,addr=0x3 \ --device vfio-pci,host=0001:01:00.1,id=hostdev1,bus=pci.0,addr=0x4 \ --device vfio-pci,host=0001:01:00.0,id=hostdev2,bus=pci.0,addr=0x5 \ --device vfio-pci,host=0005:90:01.2,id=hostdev3,bus=pci.0,addr=0x6 \ --device vfio-pci,host=0005:90:01.3,id=hostdev4,bus=pci.0,addr=0x7 \ --device vfio-pci,host=0000:06:12.1,id=hostdev5,bus=pci.0,addr=0x8 \ --device vfio-pci,host=0000:06:12.2,id=hostdev6,bus=pci.0,addr=0x9 \ --device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0xa +-device vfio-pci,host=0005:90:01.0,id=hostdev0,bus=pci.0,multifunction=on,\ +addr=0x3 \ +-device vfio-pci,host=0005:90:01.2,id=hostdev1,bus=pci.0,addr=0x3.0x2 \ +-device vfio-pci,host=0005:90:01.3,id=hostdev2,bus=pci.0,addr=0x3.0x3 \ +-device vfio-pci,host=0001:01:00.1,id=hostdev3,bus=pci.0,addr=0x4.0x1 \ +-device vfio-pci,host=0001:01:00.0,id=hostdev4,bus=pci.0,multifunction=on,\ +addr=0x4 \ +-device vfio-pci,host=0000:06:12.1,id=hostdev5,bus=pci.0,addr=0x5 \ +-device vfio-pci,host=0000:06:12.2,id=hostdev6,bus=pci.0,addr=0x6 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x7 diff --git a/tests/qemuxml2argvdata/hostdev-pci-multifunction.xml b/tests/qemuxml2argvdata/hostdev-pci-multifunction.xml index 06c889c64d..a0af6c5a90 100644 --- a/tests/qemuxml2argvdata/hostdev-pci-multifunction.xml +++ b/tests/qemuxml2argvdata/hostdev-pci-multifunction.xml @@ -22,25 +22,25 @@ <hostdev mode='subsystem' type='pci' managed='yes'> <driver name='vfio'/> <source> - <address domain='0x0001' bus='0x01' slot='0x00' function='0x1'/> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x2'/> </source> </hostdev> <hostdev mode='subsystem' type='pci' managed='yes'> <driver name='vfio'/> <source> - <address domain='0x0001' bus='0x01' slot='0x00' function='0x0'/> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x3'/> </source> </hostdev> <hostdev mode='subsystem' type='pci' managed='yes'> <driver name='vfio'/> <source> - <address domain='0x0005' bus='0x90' slot='0x01' function='0x2'/> + <address domain='0x0001' bus='0x01' slot='0x00' function='0x1'/> </source> </hostdev> <hostdev mode='subsystem' type='pci' managed='yes'> <driver name='vfio'/> <source> - <address domain='0x0005' bus='0x90' slot='0x01' function='0x3'/> + <address domain='0x0001' bus='0x01' slot='0x00' function='0x0'/> </source> </hostdev> <hostdev mode='subsystem' type='pci' managed='yes'> diff --git a/tests/qemuxml2argvdata/pseries-hostdevs-1.args b/tests/qemuxml2argvdata/pseries-hostdevs-1.args index 51ec025dce..d745f5bae6 100644 --- a/tests/qemuxml2argvdata/pseries-hostdevs-1.args +++ b/tests/qemuxml2argvdata/pseries-hostdevs-1.args @@ -26,5 +26,6 @@ server,nowait \ -device spapr-pci-host-bridge,index=1,id=pci.1 \ -device spapr-pci-host-bridge,index=2,id=pci.2 \ -device vfio-pci,host=0005:90:01.0,id=hostdev0,bus=pci.1.0,addr=0x1 \ --device vfio-pci,host=0001:01:00.0,id=hostdev1,bus=pci.2.0,addr=0x1 \ --device vfio-pci,host=0001:01:00.1,id=hostdev2,bus=pci.2.0,addr=0x2 +-device vfio-pci,host=0001:01:00.0,id=hostdev1,bus=pci.2.0,multifunction=on,\ +addr=0x1 \ +-device vfio-pci,host=0001:01:00.1,id=hostdev2,bus=pci.2.0,addr=0x1.0x1 diff --git a/tests/qemuxml2argvdata/pseries-hostdevs-3.args b/tests/qemuxml2argvdata/pseries-hostdevs-3.args index 5820140065..d29b01f4d8 100644 --- a/tests/qemuxml2argvdata/pseries-hostdevs-3.args +++ b/tests/qemuxml2argvdata/pseries-hostdevs-3.args @@ -25,5 +25,6 @@ server,nowait \ -no-shutdown \ -device spapr-pci-host-bridge,index=1,id=pci.1 \ -device spapr-pci-host-bridge,index=2,id=pci.2 \ --device vfio-pci,host=0001:01:00.0,id=hostdev0,bus=pci.2.0,addr=0x1 \ --device vfio-pci,host=0001:01:00.1,id=hostdev1,bus=pci.2.0,addr=0x2 +-device vfio-pci,host=0001:01:00.0,id=hostdev0,bus=pci.2.0,multifunction=on,\ +addr=0x1 \ +-device vfio-pci,host=0001:01:00.1,id=hostdev1,bus=pci.2.0,addr=0x1.0x1 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a36183bf34..dad03f1dec 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1334,7 +1334,8 @@ mymain(void) DO_TEST("hostdev-pci-multifunction", QEMU_CAPS_KVM, - QEMU_CAPS_DEVICE_VFIO_PCI); + QEMU_CAPS_DEVICE_VFIO_PCI, + QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY); DO_TEST("hostdev-pci-address-unassigned", QEMU_CAPS_KVM, @@ -1927,14 +1928,17 @@ mymain(void) QEMU_CAPS_VIRTIO_SCSI); DO_TEST("pseries-hostdevs-1", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + X_QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_VFIO_PCI); DO_TEST("pseries-hostdevs-2", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + X_QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_VFIO_PCI); DO_TEST("pseries-hostdevs-3", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + X_QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_VFIO_PCI); diff --git a/tests/qemuxml2xmloutdata/hostdev-pci-address-unassigned.xml b/tests/qemuxml2xmloutdata/hostdev-pci-address-unassigned.xml index 2341e8432b..d6c26c3252 100644 --- a/tests/qemuxml2xmloutdata/hostdev-pci-address-unassigned.xml +++ b/tests/qemuxml2xmloutdata/hostdev-pci-address-unassigned.xml @@ -28,7 +28,7 @@ <source> <address domain='0x0005' bus='0x90' slot='0x01' function='0x0'/> </source> - <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0' multifunction='on'/> </hostdev> <hostdev mode='subsystem' type='pci' managed='yes'> <driver name='vfio'/> @@ -42,17 +42,17 @@ <source> <address domain='0x0005' bus='0x90' slot='0x01' function='0x2'/> </source> - <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x2'/> </hostdev> <hostdev mode='subsystem' type='pci' managed='yes'> <driver name='vfio'/> <source> <address domain='0x0005' bus='0x90' slot='0x01' function='0x3'/> </source> - <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x3'/> </hostdev> <memballoon model='virtio'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </memballoon> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/hostdev-pci-multifunction.xml b/tests/qemuxml2xmloutdata/hostdev-pci-multifunction.xml index 52ed86e305..c40b2130df 100644 --- a/tests/qemuxml2xmloutdata/hostdev-pci-multifunction.xml +++ b/tests/qemuxml2xmloutdata/hostdev-pci-multifunction.xml @@ -28,52 +28,52 @@ <source> <address domain='0x0005' bus='0x90' slot='0x01' function='0x0'/> </source> - <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0' multifunction='on'/> </hostdev> <hostdev mode='subsystem' type='pci' managed='yes'> <driver name='vfio'/> <source> - <address domain='0x0001' bus='0x01' slot='0x00' function='0x1'/> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x2'/> </source> - <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x2'/> </hostdev> <hostdev mode='subsystem' type='pci' managed='yes'> <driver name='vfio'/> <source> - <address domain='0x0001' bus='0x01' slot='0x00' function='0x0'/> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x3'/> </source> - <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x3'/> </hostdev> <hostdev mode='subsystem' type='pci' managed='yes'> <driver name='vfio'/> <source> - <address domain='0x0005' bus='0x90' slot='0x01' function='0x2'/> + <address domain='0x0001' bus='0x01' slot='0x00' function='0x1'/> </source> - <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x1'/> </hostdev> <hostdev mode='subsystem' type='pci' managed='yes'> <driver name='vfio'/> <source> - <address domain='0x0005' bus='0x90' slot='0x01' function='0x3'/> + <address domain='0x0001' bus='0x01' slot='0x00' function='0x0'/> </source> - <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0' multifunction='on'/> </hostdev> <hostdev mode='subsystem' type='pci' managed='yes'> <driver name='vfio'/> <source> <address domain='0x0000' bus='0x06' slot='0x12' function='0x1'/> </source> - <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </hostdev> <hostdev mode='subsystem' type='pci' managed='yes'> <driver name='vfio'/> <source> <address domain='0x0000' bus='0x06' slot='0x12' function='0x2'/> </source> - <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </hostdev> <memballoon model='virtio'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </memballoon> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/pseries-hostdevs-1.xml b/tests/qemuxml2xmloutdata/pseries-hostdevs-1.xml index e77a060a38..c09588de9d 100644 --- a/tests/qemuxml2xmloutdata/pseries-hostdevs-1.xml +++ b/tests/qemuxml2xmloutdata/pseries-hostdevs-1.xml @@ -40,14 +40,14 @@ <source> <address domain='0x0001' bus='0x01' slot='0x00' function='0x0'/> </source> - <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0' multifunction='on'/> </hostdev> <hostdev mode='subsystem' type='pci' managed='yes'> <driver name='vfio'/> <source> <address domain='0x0001' bus='0x01' slot='0x00' function='0x1'/> </source> - <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x1'/> </hostdev> <memballoon model='none'/> <panic model='pseries'/> diff --git a/tests/qemuxml2xmloutdata/pseries-hostdevs-3.xml b/tests/qemuxml2xmloutdata/pseries-hostdevs-3.xml index f91959b805..f01adf6d25 100644 --- a/tests/qemuxml2xmloutdata/pseries-hostdevs-3.xml +++ b/tests/qemuxml2xmloutdata/pseries-hostdevs-3.xml @@ -32,14 +32,14 @@ <source> <address domain='0x0001' bus='0x01' slot='0x00' function='0x0'/> </source> - <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0' multifunction='on'/> </hostdev> <hostdev mode='subsystem' type='pci' managed='yes'> <driver name='vfio'/> <source> <address domain='0x0001' bus='0x01' slot='0x00' function='0x1'/> </source> - <address type='pci' domain='0x0000' bus='0x02' slot='0x02' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x1'/> </hostdev> <memballoon model='none'/> <panic model='pseries'/> diff --git a/tests/virpcitestdata/0005-90-01.1.config b/tests/virpcitestdata/0005-90-01.1.config index beee76534041a7020c08ae9ac03d9a349c6ea12e..a60599bd342d3ebcdc7b8367ca36ad337f602fde 100644 GIT binary patch delta 44 ycmZo*YG4vE7BFRCV-R3+7GUOK;Amg~f~JXq5)*X<85t+qEn;Cc-jFjfPzC^<7YL>R delta 39 ucmZo*YG4vE7BFRCV-R3+7GUOK;9y{25MXGU7$`AON05<eqTQm22?_viD+cla diff --git a/tests/virpcitestdata/0005-90-01.2.config b/tests/virpcitestdata/0005-90-01.2.config index cfd72e4d7165bff2751ecbdc570ce7f1e0646226..a60599bd342d3ebcdc7b8367ca36ad337f602fde 100644 GIT binary patch literal 256 zcmXpOFlAt45MXi^VCG@qXkY+>CJ=!Q7z5RUfCHEW5{!&mj0{Y5Fz#TaS&cX3;ByxM DCDjDB literal 256 zcmXpOc)-BMAi%_;z|6zo!oa|wz|aIFu>xbDS`csmlR$!5K#7rosSd`)Mk^@TV-u#E T7_0Gy9FS#<5E~CbC<F-rZiWW} diff --git a/tests/virpcitestdata/0005-90-01.3.config b/tests/virpcitestdata/0005-90-01.3.config new file mode 100644 index 0000000000000000000000000000000000000000..a60599bd342d3ebcdc7b8367ca36ad337f602fde GIT binary patch literal 256 zcmXpOFlAt45MXi^VCG@qXkY+>CJ=!Q7z5RUfCHEW5{!&mj0{Y5Fz#TaS&cX3;ByxM DCDjDB literal 0 HcmV?d00001 -- 2.24.1

From: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> In some cases it may be better to have a bitmap representing the state of individual functions rather than iterating the definition. The new helper creates a bitmap representing the state from the domain definition. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 27 +++++++++++++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 31 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9b60db7ecd..5b918cc737 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16947,6 +16947,33 @@ virDomainHostdevFind(virDomainDefPtr def, return *found ? i : -1; } +#define PCI_MAX_SLOT_FUNCTIONS 8 +/** + * virDomainDefHostdevGetPCIOnlineFunctionMap: + * @def: domain definition + * @aggrSlotIdx: slot aggregation index + * Returns a bitmap representing state of individual functions of a slot. + */ +virBitmapPtr +virDomainDefHostdevGetPCIOnlineFunctionMap(virDomainDefPtr def, + int aggrSlotIdx) +{ + size_t i; + virBitmapPtr ret = NULL; + + if (!(ret = virBitmapNew(PCI_MAX_SLOT_FUNCTIONS))) + return NULL; + + for (i = 0; i < def->nhostdevs; i++) { + size_t function = def->hostdevs[i]->source.subsys.u.pci.addr.function; + + if (def->hostdevs[i]->info->aggregateSlotIdx == aggrSlotIdx) + ignore_value(virBitmapSetBit(ret, function)); + } + + return ret; +} + static bool virDomainDiskControllerMatch(int controller_type, int disk_bus) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e144f3aad3..77a01fbec0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3212,6 +3212,9 @@ virDomainHostdevDefPtr virDomainHostdevRemove(virDomainDefPtr def, size_t i); int virDomainHostdevFind(virDomainDefPtr def, virDomainHostdevDefPtr match, virDomainHostdevDefPtr *found); +virBitmapPtr virDomainDefHostdevGetPCIOnlineFunctionMap(virDomainDefPtr def, + int aggrSlotIdx); + virDomainGraphicsListenDefPtr virDomainGraphicsGetListen(virDomainGraphicsDefPtr def, size_t i); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 702cd958b1..5b247ded4b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -321,6 +321,7 @@ virDomainDefHasOldStyleUEFI; virDomainDefHasUSB; virDomainDefHasVcpusOffline; virDomainDefHasVFIOHostdev; +virDomainDefHostdevGetPCIOnlineFunctionMap; virDomainDefLifecycleActionAllowed; virDomainDefMaybeAddController; virDomainDefMaybeAddInput; -- 2.24.1

From: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> The hostdevices are the only devices which have dependencies outside of themselves such that, other functions of the PCI card should also have been detached from host driver before attempting the hotplug. This patch moves the detach to the beginning of the hotplug so that the following patch can detach all functions first before attempting to hotplug any. We don't need to move the detach for net devices using SRIOV as all SRIOV devices are single function devices and can be independently detached as usual. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_hotplug.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 8e5625fb8d..688171c7b2 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1205,6 +1205,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, g_autofree char *netdev_name = NULL; g_autoptr(virConnect) conn = NULL; virErrorPtr save_err = NULL; + virDomainHostdevDefPtr hostdev = NULL; /* preallocate new slot for device */ if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets + 1) < 0) @@ -1235,9 +1236,18 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, * as a hostdev (the hostdev code will reach over into the * netdev-specific code as appropriate), then also added to * the nets list (see cleanup:) if successful. + * + * qemuDomainAttachHostDevice uses a connection to resolve + * a SCSI hostdev secret, which is not this case, so pass NULL. */ - ret = qemuDomainAttachHostDevice(driver, vm, - virDomainNetGetActualHostdev(net)); + hostdev = virDomainNetGetActualHostdev(net); + if (qemuDomainAttachPCIHostDevicePrepare(driver, vm->def, + hostdev, priv->qemuCaps) < 0) + goto cleanup; + + if ((ret = qemuDomainAttachHostDevice(driver, vm, hostdev)) < 0) + qemuHostdevReAttachPCIDevices(driver, vm->def->name, &hostdev, 1); + goto cleanup; } @@ -1593,10 +1603,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0) return -1; - if (qemuDomainAttachPCIHostDevicePrepare(driver, vm->def, - hostdev, priv->qemuCaps) < 0) - return -1; - /* this could have been changed by qemuHostdevPreparePCIDevices */ backend = hostdev->source.subsys.u.pci.backend; @@ -1674,8 +1680,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, if (releaseaddr) qemuDomainReleaseDeviceAddress(vm, info); - qemuHostdevReAttachPCIDevices(driver, vm->def->name, &hostdev, 1); - return -1; } @@ -2905,6 +2909,8 @@ qemuDomainAttachHostDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainHostdevDefPtr hostdev) { + qemuDomainObjPrivatePtr priv = vm->privateData; + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("hotplug is not supported for hostdev mode '%s'"), @@ -2914,9 +2920,15 @@ qemuDomainAttachHostDevice(virQEMUDriverPtr driver, switch (hostdev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: - if (qemuDomainAttachHostPCIDevice(driver, vm, - hostdev) < 0) + if (qemuDomainAttachPCIHostDevicePrepare(driver, vm->def, + hostdev, priv->qemuCaps) < 0) return -1; + + if (qemuDomainAttachHostPCIDevice(driver, vm, hostdev) < 0) { + qemuHostdevReAttachPCIDevices(driver, vm->def->name, &hostdev, 1); + return -1; + } + break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: -- 2.24.1

From: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> virDomainDeviceDefParseXMLMany will be used to parse '<devices>' XML elements that will be used to supply multiple hostdevs for hotplug/unplug. The idea is to parse each dev individually, by calling the same code that already handles it today. The result is returned in a new virDomainDeviceDefListPtr pointer type, which is an array of device definitions. This idea of calling existing functions to handles a single device definition in a loop is the central idea of how PCI multifunction hotplug and hotunplug is implemented in the next patches. Roughly speaking, we'll use virDomainDeviceDefListPtr lists to call (almost) the same hotplug/unplug mechanics we have today for each hostdev in the list. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 171 +++++++++++++++++++++++++++++++++++---- src/conf/domain_conf.h | 32 ++++++++ src/libvirt_private.syms | 5 ++ 3 files changed, 194 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5b918cc737..887f7a2633 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1287,6 +1287,90 @@ virDomainXMLOptionDispose(void *obj) (xmlopt->config.privFree)(xmlopt->config.priv); } +/* virDomainDeviceDefListAddCopy - add a *copy* of the device to this list */ +static int +virDomainDeviceDefListAddCopy(virDomainDeviceDefListPtr list, + virDomainDeviceDefPtr dev, + virDomainDeviceDefListDataPtr data, + void *parseOpaque) +{ + virDomainDeviceDefPtr copy = virDomainDeviceDefCopy(dev, + data->def, + data->xmlopt, + parseOpaque); + + if (!copy) + return -1; + + if (VIR_APPEND_ELEMENT(list->devs, list->count, copy) < 0) { + virDomainDeviceDefFree(copy); + return -1; + } + + return 0; +} + +void virDomainDeviceDefListFree(virDomainDeviceDefListPtr list) +{ + size_t i; + + if (!list) + return; + for (i = 0; i < list->count; i++) + virDomainDeviceDefFree(list->devs[i]); + VIR_FREE(list->devs); +} + +void virDomainDeviceDefListFreeShallow(virDomainDeviceDefListPtr list) +{ + size_t i; + + if (!list) + return; + for (i = 0; i < list->count; i++) + VIR_FREE(list->devs[i]); +} + + +/* virDomainDeviceDefListIter - Iterate through the list with the callback*/ +int +virDomainDeviceDefListIterate(virDomainDeviceDefListPtr list, + virDomainDeviceDefListIterCallback cb, + void *data) +{ + size_t i; + + for (i = 0; i < list->count; i++) + if (cb(list->devs[i], data)) + return -1; + + return 0; +} + +virDomainDeviceDefListPtr +virDomainDeviceDefListCopy(virDomainDeviceDefListPtr list, + virDomainDeviceDefListDataPtr data, + void *parseOpaque) +{ + size_t i; + virDomainDeviceDefListPtr devlist = NULL; + + if (list && (VIR_ALLOC(devlist) < 0)) + goto cleanup; + + for (i = 0; i < list->count; i++) { + if (virDomainDeviceDefListAddCopy(devlist, list->devs[i], + data, parseOpaque) < 0) + goto cleanup; + } + + return devlist; + cleanup: + virDomainDeviceDefListFree(devlist); + return NULL; +} + + /** * virDomainKeyWrapCipherDefParseXML: * @@ -16506,23 +16590,16 @@ virDomainVsockDefParseXML(virDomainXMLOptionPtr xmlopt, return g_steal_pointer(&vsock); } -virDomainDeviceDefPtr -virDomainDeviceDefParse(const char *xmlStr, - const virDomainDef *def, - virDomainXMLOptionPtr xmlopt, - void *parseOpaque, - unsigned int flags) +static virDomainDeviceDefPtr +virDomainDeviceDefParseXML(xmlNodePtr node, + const virDomainDef *def, + virDomainXMLOptionPtr xmlopt, + xmlXPathContextPtr ctxt, + void *parseOpaque, + unsigned int flags) { - g_autoptr(xmlDoc) xml = NULL; - xmlNodePtr node; - g_autoptr(xmlXPathContext) ctxt = NULL; g_autofree virDomainDeviceDefPtr dev = NULL; - if (!(xml = virXMLParseStringCtxt(xmlStr, _("(device_definition)"), &ctxt))) - return NULL; - - node = ctxt->node; - if (VIR_ALLOC(dev) < 0) return NULL; @@ -16678,6 +16755,32 @@ virDomainDeviceDefParse(const char *xmlStr, } +virDomainDeviceDefPtr +virDomainDeviceDefParse(const char *xmlStr, + const virDomainDef *def, + virDomainXMLOptionPtr xmlopt, + void *parseOpaque, + unsigned int flags) +{ + xmlDocPtr xml; + xmlNodePtr node; + xmlXPathContextPtr ctxt = NULL; + virDomainDeviceDefPtr dev = NULL; + + if (!(xml = virXMLParseStringCtxt(xmlStr, _("(device_definition)"), &ctxt))) + return NULL; + + node = ctxt->node; + + dev = virDomainDeviceDefParseXML(node, def, xmlopt, ctxt, parseOpaque, flags); + + xmlFreeDoc(xml); + xmlXPathFreeContext(ctxt); + + return dev; + } + + virDomainDiskDefPtr virDomainDiskDefParse(const char *xmlStr, virDomainXMLOptionPtr xmlopt, @@ -31617,6 +31720,46 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) return 0; } +virDomainDeviceDefListPtr +virDomainDeviceDefParseXMLMany(const char *xml, + const virDomainDef *def, + virDomainXMLOptionPtr xmlopt, + void *parseOpaque, + unsigned int flags) +{ + xmlXPathContextPtr ctxt = NULL; + xmlDocPtr xmlPtr; + xmlNodePtr node, root; + virDomainDeviceDefPtr dev = NULL; + virDomainDeviceDefListPtr devlist; + + if (!(xmlPtr = virXMLParseStringCtxt(xml, _("(device_definition)"), &ctxt))) + return NULL; + + if (VIR_ALLOC(devlist) < 0) + goto exit; + + root = xmlDocGetRootElement(xmlPtr); + node = root->children; + while (node) { + if (node->type == XML_ELEMENT_NODE) { + dev = virDomainDeviceDefParseXML(node, def, xmlopt, ctxt, + parseOpaque, flags); + if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) { + virDomainDeviceDefFree(dev); + virDomainDeviceDefListFree(devlist); + goto exit; + } + dev = NULL; + } + node = node->next; + } + + exit: + xmlFreeDoc(xmlPtr); + xmlXPathFreeContext(ctxt); + return devlist; +} /** * virDomainDiskGetDetectZeroesMode: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 77a01fbec0..26d9ca6ba9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2648,6 +2648,33 @@ typedef enum { typedef int (*virDomainDefPostParseBasicCallback)(virDomainDefPtr def, void *opaque); +typedef struct _virDomainDeviceDefListData virDomainDeviceDefListData; +typedef virDomainDeviceDefListData *virDomainDeviceDefListDataPtr; +struct _virDomainDeviceDefListData { + const virDomainDef *def; + virDomainXMLOptionPtr xmlopt; +}; + +struct virDomainDeviceDefList { + virDomainDeviceDefPtr *devs; + size_t count; +}; +typedef struct virDomainDeviceDefList *virDomainDeviceDefListPtr; + +typedef int (*virDomainDeviceDefListIterCallback)(virDomainDeviceDefPtr dev, + void *opaque); +int virDomainDeviceDefListIterate(virDomainDeviceDefListPtr devlist, + virDomainDeviceDefListIterCallback cb, + void *data); +virDomainDeviceDefListPtr +virDomainDeviceDefListCopy(virDomainDeviceDefListPtr list, + virDomainDeviceDefListDataPtr data, + void *parseOpaque); + +void virDomainDeviceDefListFree(virDomainDeviceDefListPtr list); +void virDomainDeviceDefListFreeShallow(virDomainDeviceDefListPtr list); + + /* Called once after everything else has been parsed, for adjusting * overall domain defaults. * @parseOpaque is opaque data passed by virDomainDefParse* caller, @@ -3053,6 +3080,11 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(const char *xmlStr, virDomainXMLOptionPtr xmlopt, void *parseOpaque, unsigned int flags); +virDomainDeviceDefListPtr virDomainDeviceDefParseXMLMany(const char *xmlStr, + const virDomainDef *def, + virDomainXMLOptionPtr xmlopt, + void *parseOpaque, + unsigned int flags); virDomainDiskDefPtr virDomainDiskDefParse(const char *xmlStr, virDomainXMLOptionPtr xmlopt, unsigned int flags); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5b247ded4b..eda759c219 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -341,7 +341,12 @@ virDomainDeleteConfig; virDomainDeviceAliasIsUserAlias; virDomainDeviceDefCopy; virDomainDeviceDefFree; +virDomainDeviceDefListCopy; +virDomainDeviceDefListFree; +virDomainDeviceDefListFreeShallow; +virDomainDeviceDefListIterate; virDomainDeviceDefParse; +virDomainDeviceDefParseXMLMany; virDomainDeviceFindSCSIController; virDomainDeviceGetInfo; virDomainDeviceInfoIterate; -- 2.24.1

From: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> This helps calling the routines with a list of devices. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 45 +++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8bb845298b..2e91a2b694 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8109,11 +8109,8 @@ qemuCheckDiskConfigAgainstDomain(const virDomainDef *def, static int -qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev, - virQEMUCapsPtr qemuCaps, - unsigned int parse_flags, - virDomainXMLOptionPtr xmlopt) +qemuDomainAttachDeviceConfigInternal(virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev) { virDomainDiskDefPtr disk; virDomainNetDefPtr net; @@ -8310,19 +8307,35 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, return -1; } - if (virDomainDefPostParse(vmdef, parse_flags, xmlopt, qemuCaps) < 0) - return -1; - return 0; } static int -qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, +qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev, virQEMUCapsPtr qemuCaps, unsigned int parse_flags, virDomainXMLOptionPtr xmlopt) +{ + if (virDomainDefCompatibleDevice(vmdef, dev, NULL, + VIR_DOMAIN_DEVICE_ACTION_ATTACH, + false) < 0) + return -1; + + if (qemuDomainAttachDeviceConfigInternal(vmdef, dev)) + return -1; + + if (virDomainDefPostParse(vmdef, parse_flags, xmlopt, qemuCaps) < 0) + return -1; + + return 0; +} + + +static int +qemuDomainDetachDeviceConfigInternal(virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev) { virDomainDiskDefPtr disk, det_disk; virDomainNetDefPtr net; @@ -8512,6 +8525,20 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, return -1; } + return 0; +} + + +static int +qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev, + virQEMUCapsPtr qemuCaps, + unsigned int parse_flags, + virDomainXMLOptionPtr xmlopt) +{ + if (qemuDomainDetachDeviceConfigInternal(vmdef, dev)) + return -1; + if (virDomainDefPostParse(vmdef, parse_flags, xmlopt, qemuCaps) < 0) return -1; -- 2.24.1

From: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> With multifunction devices, multiple delete requests are sent to qemu and all the requests should be queued up. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_domain.h | 3 ++- src/qemu/qemu_hotplug.c | 41 ++++++++++++++++++++++++++--------------- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 8d13cc0e56..07d828d71a 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -232,7 +232,8 @@ typedef enum { typedef struct _qemuDomainUnpluggingDevice qemuDomainUnpluggingDevice; typedef qemuDomainUnpluggingDevice *qemuDomainUnpluggingDevicePtr; struct _qemuDomainUnpluggingDevice { - const char *alias; + const char **aliases; + size_t naliases; qemuDomainUnpluggingDeviceStatus status; bool eventSeen; /* True if DEVICE_DELETED event arrived. */ }; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 18daa7aeac..2269fd5c10 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -5074,13 +5074,19 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver, static void qemuDomainMarkDeviceAliasForRemoval(virDomainObjPtr vm, - const char *alias) + const char *alias, + bool fresh) { qemuDomainObjPrivatePtr priv = vm->privateData; - memset(&priv->unplug, 0, sizeof(priv->unplug)); + if (fresh) + memset(&priv->unplug, 0, sizeof(priv->unplug)); + + if (VIR_REALLOC_N(priv->unplug.aliases, priv->unplug.naliases + 1) < 0) + return; + + priv->unplug.aliases[priv->unplug.naliases++] = alias; - priv->unplug.alias = alias; } @@ -5089,7 +5095,7 @@ qemuDomainMarkDeviceForRemoval(virDomainObjPtr vm, virDomainDeviceInfoPtr info) { - qemuDomainMarkDeviceAliasForRemoval(vm, info->alias); + qemuDomainMarkDeviceAliasForRemoval(vm, info->alias, true); } @@ -5097,7 +5103,9 @@ static void qemuDomainResetDeviceRemoval(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; - priv->unplug.alias = NULL; + priv->unplug.aliases = NULL; + VIR_FREE(priv->unplug.aliases); + priv->unplug.naliases = 0; priv->unplug.eventSeen = false; } @@ -5132,13 +5140,14 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm) return 1; until += qemuDomainGetUnplugTimeout(vm); - while (priv->unplug.alias) { + /* All devices should get released around same time*/ + while (priv->unplug.naliases) { if ((rc = virDomainObjWaitUntil(vm, until)) == 1) return 0; if (rc < 0) { VIR_WARN("Failed to wait on unplug condition for domain '%s' " - "device '%s'", vm->def->name, priv->unplug.alias); + "device '%s'", vm->def->name, priv->unplug.aliases[0]); return 1; } } @@ -5164,14 +5173,16 @@ qemuDomainSignalDeviceRemoval(virDomainObjPtr vm, qemuDomainUnpluggingDeviceStatus status) { qemuDomainObjPrivatePtr priv = vm->privateData; + size_t i; - if (STREQ_NULLABLE(priv->unplug.alias, devAlias)) { - VIR_DEBUG("Removal of device '%s' continues in waiting thread", devAlias); - qemuDomainResetDeviceRemoval(vm); - priv->unplug.status = status; - priv->unplug.eventSeen = true; - virDomainObjBroadcast(vm); - return true; + for (i = 0; i < priv->unplug.naliases; i++) { + if (STREQ_NULLABLE(priv->unplug.aliases[i], devAlias)) { + VIR_DEBUG("Removal of device '%s' continues in waiting thread", devAlias); + VIR_DELETE_ELEMENT(priv->unplug.aliases, i, priv->unplug.naliases); + priv->unplug.status = status; + virDomainObjBroadcast(vm); + return true; + } } return false; } @@ -5984,7 +5995,7 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, return -1; } - qemuDomainMarkDeviceAliasForRemoval(vm, vcpupriv->alias); + qemuDomainMarkDeviceAliasForRemoval(vm, vcpupriv->alias, true); if (qemuDomainDeleteDevice(vm, vcpupriv->alias) < 0) { if (virDomainObjIsActive(vm)) -- 2.24.1

From: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> The support to allow multifunction device hotplug consists of using virDomainDeviceDefListPtr lists instead of single virDomainDeviceDefPtr device definitions in both qemuDomainAttachDeviceConfig() and qemuDomainAttachDeviceLive(). In AttachDeviceConfig() the same existing single device code is ran for all devs in the list. AttachDeviceLive() will verify if the list has size = 1 (meaning it is a single device attach) and redirect the code to the regular single dev attach procedure. If list size > 1, we'll execute a new specialized function called qemuDomainAttachMultifunctionDevice(), which is prepared to handle the nuances of this type of hotplug. The changes in both AttachDeviceLive() and AttachDeviceConfig() implied in changes to be made in qemuDomainAttachDeviceLiveAndConfig(), which is now handling a device list instead of a single device. All other added functions, most notably qemuDomainPCIMultifunctionHostdevEnsurePCIAddresses(), are used inside qemuDomainAttachMultifunctionDevice(). No changes in regular device hotplug mechanics were made. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_domain_address.c | 76 +++++++++++ src/qemu/qemu_domain_address.h | 5 + src/qemu/qemu_driver.c | 119 +++++++++++------- src/qemu/qemu_hotplug.c | 86 +++++++++++++ src/qemu/qemu_hotplug.h | 4 + tests/qemuhotplugtest.c | 40 ++++-- ...emuhotplug-multifunction-hostdev-pci-2.xml | 14 +++ .../qemuhotplug-multifunction-hostdev-pci.xml | 26 ++++ ...ug-base-live+multifunction-hostdev-pci.xml | 82 ++++++++++++ ...-base-live+multifunction-hostdev-pci-2.xml | 59 +++++++++ ...es-base-live+multifunction-hostdev-pci.xml | 69 ++++++++++ 11 files changed, 528 insertions(+), 52 deletions(-) create mode 100644 tests/qemuhotplugtestdevices/qemuhotplug-multifunction-hostdev-pci-2.xml create mode 100644 tests/qemuhotplugtestdevices/qemuhotplug-multifunction-hostdev-pci.xml create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-base-live+multifunction-hostdev-pci.xml create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-pseries-base-live+multifunction-hostdev-pci-2.xml create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-pseries-base-live+multifunction-hostdev-pci.xml diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index d9e36867bd..395f054b8d 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -23,6 +23,7 @@ #include "qemu_domain_address.h" #include "qemu_domain.h" +#include "domain_conf.h" #include "viralloc.h" #include "virhostdev.h" #include "virerror.h" @@ -3477,6 +3478,81 @@ qemuDomainEnsurePCIAddress(virDomainObjPtr obj, info->pciConnectFlags); } + +int +qemuDomainPCIMultifunctionHostdevEnsurePCIAddresses(virDomainObjPtr vm, + virDomainDeviceDefListPtr devlist, + virQEMUDriverPtr driver) +{ + int ret = -1, aggrslotidx = 0; + virBitmapPtr slotmap = NULL; + size_t i; + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainPCIMultifunctionAddressInfoPtr devinfos = NULL; + + if (devlist->count > VIR_PCI_MAX_FUNCTIONS) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("More devices per slot found")); + return -1; + } + + for (i = 0; i < devlist->count; i++) + qemuDomainFillDevicePCIConnectFlags(vm->def, devlist->devs[i], + priv->qemuCaps, driver); + + if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + devlist->count) < 0) + return -1; + + /* Temporarily add the devices to the domain def to get the + * next aggregateIdx */ + for (i = 0; i < devlist->count; i++) + vm->def->hostdevs[vm->def->nhostdevs++] = devlist->devs[i]->data.hostdev; + + for (i = 0; i < devlist->count; i++) { + virDomainHostdevDefPtr hostdev = devlist->devs[i]->data.hostdev; + + if (qemuDomainIsPSeries(vm->def)) { + /* Isolation groups are only relevant for pSeries guests */ + if (qemuDomainFillDeviceIsolationGroup(vm->def, devlist->devs[i]) < 0) + return -1; + } + + qemuDomainSetDeviceSlotAggregateIdx(vm->def, devlist->devs[i]); + aggrslotidx = aggrslotidx ? aggrslotidx : hostdev->info->aggregateSlotIdx; + + if (aggrslotidx != hostdev->info->aggregateSlotIdx) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Devices belong to different PCI slots")); + return -1; + } + } + + for (i = 0; i < devlist->count; i++) + vm->def->hostdevs[--(vm->def->nhostdevs)] = NULL; + + slotmap = virDomainDefHostdevGetPCIOnlineFunctionMap(vm->def, aggrslotidx); + if (!virBitmapIsAllClear(slotmap)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Device already assigned to guest")); + return -1; + } + + if (VIR_ALLOC(devinfos) < 0) + return -1; + + for (i = 0; i < devlist->count; i++) { + virDomainHostdevDefPtr hostdev = devlist->devs[i]->data.hostdev; + virPCIDeviceAddress addr = hostdev->source.subsys.u.pci.addr; + + devinfos->infos[addr.function] = hostdev->info; + } + + ret = virDomainPCIAddressEnsureMultifunctionAddress(priv->pciaddrs, devinfos); + VIR_FREE(devinfos); + return ret; +} + + void qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, virDomainDeviceInfoPtr info) diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h index a17245dbde..035b40a5a7 100644 --- a/src/qemu/qemu_domain_address.h +++ b/src/qemu/qemu_domain_address.h @@ -49,6 +49,11 @@ int qemuDomainEnsurePCIAddress(virDomainObjPtr obj, virQEMUDriverPtr driver) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +int +qemuDomainPCIMultifunctionHostdevEnsurePCIAddresses(virDomainObjPtr obj, + virDomainDeviceDefListPtr devlist, + virQEMUDriverPtr driver); + int qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def, virDomainDeviceDefPtr dev) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fb53dc79df..95ad4c5e69 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7969,17 +7969,28 @@ qemuDomainAttachDeviceLiveInternal(virDomainObjPtr vm, static int qemuDomainAttachDeviceLive(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr devlist, virQEMUDriverPtr driver) { int ret = -1; + size_t i; - if (virDomainDefCompatibleDevice(vm->def, dev, NULL, - VIR_DOMAIN_DEVICE_ACTION_ATTACH, - false) < 0) - return -1; + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vm->def, devlist->devs[i], NULL, + VIR_DOMAIN_DEVICE_ACTION_ATTACH, + false) < 0) + return ret; + + if (devlist->count > 1) { + ret = qemuDomainAttachMultifunctionDevice(vm, devlist, driver); + if (ret == 0) { + for (i = 0; i < devlist->count; i++) + devlist->devs[i]->data.hostdev = NULL; + } + } else if (devlist->count == 1) { + ret = qemuDomainAttachDeviceLiveInternal(vm, devlist->devs[0], driver); + } - ret = qemuDomainAttachDeviceLiveInternal(vm, dev, driver); if (ret == 0) ret = qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE); @@ -8328,18 +8339,24 @@ qemuDomainAttachDeviceConfigInternal(virDomainDefPtr vmdef, static int qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr devlist, virQEMUCapsPtr qemuCaps, unsigned int parse_flags, virDomainXMLOptionPtr xmlopt) { - if (virDomainDefCompatibleDevice(vmdef, dev, NULL, - VIR_DOMAIN_DEVICE_ACTION_ATTACH, - false) < 0) - return -1; + size_t i; - if (qemuDomainAttachDeviceConfigInternal(vmdef, dev)) - return -1; + for (i = 0; i < devlist->count; i++) { + if (virDomainDefCompatibleDevice(vmdef, devlist->devs[i], NULL, + VIR_DOMAIN_DEVICE_ACTION_ATTACH, + false) < 0) + return -1; + } + + for (i = 0; i < devlist->count; i++) { + if (qemuDomainAttachDeviceConfigInternal(vmdef, devlist->devs[i])) + return -1; + } if (virDomainDefPostParse(vmdef, parse_flags, xmlopt, qemuCaps) < 0) return -1; @@ -8705,9 +8722,14 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm, qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDefPtr vmdef = NULL; g_autoptr(virQEMUDriverConfig) cfg = NULL; - virDomainDeviceDefPtr devConf = NULL; - virDomainDeviceDef devConfSave = { 0 }; + virDomainDeviceDefPtr devConfSave = NULL; virDomainDeviceDefPtr devLive = NULL; + virDomainDeviceDefListPtr devListConf = NULL; + virDomainDeviceDefListPtr devListConfSave = NULL; + virDomainDeviceDefListPtr devListLive = NULL; + virDomainDeviceDefListData data = {.def = vm->def, + .xmlopt = driver->xmlopt}; + size_t i; int ret = -1; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_ABI_UPDATE; @@ -8726,52 +8748,58 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm, if (!vmdef) goto cleanup; - if (!(devConf = virDomainDeviceDefParse(xml, vmdef, - driver->xmlopt, priv->qemuCaps, - parse_flags))) + devListConf = qemuDomainDeviceParseXMLMany(xml, &data, priv->qemuCaps, + parse_flags); + if (!devListConf) goto cleanup; /* - * devConf will be NULLed out by + * devListConf will be NULLed out by * qemuDomainAttachDeviceConfig(), so save it for later use by * qemuDomainAttachDeviceLiveAndConfigHomogenize() */ - devConfSave = *devConf; - - if (virDomainDeviceValidateAliasForHotplug(vm, devConf, - VIR_DOMAIN_AFFECT_CONFIG) < 0) - goto cleanup; + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + devListConfSave = virDomainDeviceDefListCopy(devListConf, + &data, + priv->qemuCaps); + if (!devListConfSave) + goto cleanup; + } - if (virDomainDefCompatibleDevice(vmdef, devConf, NULL, - VIR_DOMAIN_DEVICE_ACTION_ATTACH, - false) < 0) - goto cleanup; + for (i = 0; i < devListConf->count; i++) { + if (virDomainDeviceValidateAliasForHotplug(vm, + devListConf->devs[i], + flags) < 0) + goto cleanup; + } - if (qemuDomainAttachDeviceConfig(vmdef, devConf, priv->qemuCaps, + if (qemuDomainAttachDeviceConfig(vmdef, devListConf, priv->qemuCaps, parse_flags, driver->xmlopt) < 0) goto cleanup; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!(devLive = virDomainDeviceDefParse(xml, vm->def, - driver->xmlopt, priv->qemuCaps, - parse_flags))) + devListLive = qemuDomainDeviceParseXMLMany(xml, &data, priv->qemuCaps, + parse_flags); + if (!devListLive) goto cleanup; - if (flags & VIR_DOMAIN_AFFECT_CONFIG) - qemuDomainAttachDeviceLiveAndConfigHomogenize(&devConfSave, devLive); + for (i = 0; i < devListLive->count; i++) { + devLive = devListLive->devs[i]; - if (virDomainDeviceValidateAliasForHotplug(vm, devLive, - VIR_DOMAIN_AFFECT_LIVE) < 0) - goto cleanup; + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + devConfSave = devListConfSave->devs[i]; + qemuDomainAttachDeviceLiveAndConfigHomogenize(devConfSave, + devLive); + } - if (virDomainDefCompatibleDevice(vm->def, devLive, NULL, - VIR_DOMAIN_DEVICE_ACTION_ATTACH, - true) < 0) - goto cleanup; + if (virDomainDeviceValidateAliasForHotplug(vm, devLive, + VIR_DOMAIN_AFFECT_LIVE) < 0) + goto cleanup; + } - if (qemuDomainAttachDeviceLive(vm, devLive, driver) < 0) + if (qemuDomainAttachDeviceLive(vm, devListLive, driver) < 0) goto cleanup; /* * update domain status forcibly because the domain status may be @@ -8794,8 +8822,11 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm, ret = 0; cleanup: virDomainDefFree(vmdef); - virDomainDeviceDefFree(devConf); - virDomainDeviceDefFree(devLive); + devConfSave = NULL; + devLive = NULL; + virDomainDeviceDefListFree(devListConf); + virDomainDeviceDefListFree(devListConfSave); + virDomainDeviceDefListFree(devListLive); return ret; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 2269fd5c10..495a5c9bdd 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1668,6 +1668,92 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, } +static int +qemuiHostdevPCIMultifunctionDevicesListSort(const void *p1, + const void *p2) +{ + virDomainDeviceDefPtr a = *(virDomainDeviceDefPtr *) p1; + virDomainDeviceDefPtr b = *(virDomainDeviceDefPtr *) p2; + virPCIDeviceAddressPtr addr1 = &a->data.hostdev->source.subsys.u.pci.addr; + virPCIDeviceAddressPtr addr2 = &b->data.hostdev->source.subsys.u.pci.addr; + + return addr1->function - addr2->function; +} + + +int +qemuDomainAttachMultifunctionDevice(virDomainObjPtr vm, + virDomainDeviceDefListPtr devlist, + virQEMUDriverPtr driver) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + size_t i, d, h = devlist->count; + int ret = -1; + char *alias; + virObjectEventPtr event; + virDomainHostdevDefPtr hostdev; + + qsort(devlist->devs, devlist->count, sizeof(*devlist->devs), + qemuiHostdevPCIMultifunctionDevicesListSort); + + if (qemuDomainPCIMultifunctionHostdevEnsurePCIAddresses(vm, devlist, + driver) < 0) + return -1; + + for (d = 0; d < devlist->count; d++) { + hostdev = devlist->devs[d]->data.hostdev; + + if (qemuDomainAttachPCIHostDevicePrepare(driver, vm->def, hostdev, + priv->qemuCaps) < 0) + goto cleanup; + } + + /* Hotplug all functions, and Primary at last */ + for (h = devlist->count; h > 0; h--) { + /* The functions need not be contiguous, as a card may be sold with + * minimal functionality and then install the additional functions on + * purchase into any of the daughter-card connectors. + */ + hostdev = devlist->devs[h-1]->data.hostdev; + + ret = qemuDomainAttachHostPCIDevice(driver, vm, hostdev); + if (ret) + goto release; + + alias = hostdev->info->alias; + hostdev = NULL; + + event = virDomainEventDeviceAddedNewFromObj(vm, alias); + virObjectEventStateQueue(driver->domainEventState, event); + } + + release: + /* Release addresses for the device which are not hotplugged. + */ + for (i = 0; i < h; i++) + qemuDomainReleaseDeviceAddress(vm, devlist->devs[i]->data.hostdev->info); + + cleanup: + /* If none are actually hotplugged and just detached from the + * host driver reattach the devices to host driver. + * + * If one of the hotplug failed, those which are already hotplugged cannot + * be unplugged as they are released by qemu only on guest reboot even + * if we issue device_del on them. + * So, dont attempt to reattach any of them. + * NB: Let them be in the guest as they are not used anyway without + * function-zero? + */ + if (d > 0 && h == devlist->count) { + for (i = 0; i < d; i++) + qemuHostdevReAttachPCIDevices(driver, vm->def->name, + &devlist->devs[i]->data.hostdev, 1); + } + + return ret; +} + + void qemuDomainDelTLSObjects(virQEMUDriverPtr driver, virDomainObjPtr vm, diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 2874db43ee..9fd262a81b 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -126,6 +126,10 @@ int qemuDomainAttachPCIHostDevicePrepare(virQEMUDriverPtr driver, virDomainDefPtr def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps); +int +qemuDomainAttachMultifunctionDevice(virDomainObjPtr vm, + virDomainDeviceDefListPtr devlist, + virQEMUDriverPtr driver); int qemuDomainChrInsert(virDomainDefPtr vmdef, diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 48284fdb3e..9c84b639a6 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -88,6 +88,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_SPICE); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_SPICE_FILE_XFER_DISABLE); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY); + virQEMUCapsSet(priv->qemuCaps, X_QEMU_CAPS_PCI_MULTIFUNCTION); if (qemuTestCapsCacheInsert(driver.qemuCapsCache, priv->qemuCaps) < 0) return -1; @@ -116,9 +117,14 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, static int testQemuHotplugAttach(virDomainObjPtr vm, - virDomainDeviceDefPtr dev) + virDomainDeviceDefListPtr devlist) { int ret = -1; + virDomainDeviceDefPtr dev; + + if (devlist->count > 1) + return qemuDomainAttachMultifunctionDevice(vm, devlist, &driver); + dev = devlist->devs[0]; switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: @@ -243,7 +249,9 @@ testQemuHotplug(const void *data) bool keep = test->keep; unsigned int device_parse_flags = 0; virDomainObjPtr vm = NULL; - virDomainDeviceDefPtr dev = NULL; + virDomainDeviceDefPtr dev = NULL; /* temporary */ + virDomainDeviceDefListPtr devlist = NULL; + virDomainDeviceDefListData listdata; virCapsPtr caps = NULL; qemuMonitorTestPtr test_mon = NULL; qemuDomainObjPrivatePtr priv = NULL; @@ -281,11 +289,15 @@ testQemuHotplug(const void *data) if (test->action == ATTACH) device_parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - if (!(dev = virDomainDeviceDefParse(device_xml, vm->def, - driver.xmlopt, NULL, - device_parse_flags))) + listdata.def = vm->def; + listdata.xmlopt = driver.xmlopt; + devlist = qemuDomainDeviceParseXMLMany(device_xml, &listdata, NULL, + device_parse_flags); + if (!devlist) goto cleanup; + dev = devlist->devs[0]; /* temporary */ + /* Now is the best time to feed the spoofed monitor with predefined * replies. */ if (!(test_mon = qemuMonitorTestNew(driver.xmlopt, vm, &driver, @@ -314,11 +326,11 @@ testQemuHotplug(const void *data) switch (test->action) { case ATTACH: - ret = testQemuHotplugAttach(vm, dev); + ret = testQemuHotplugAttach(vm, devlist); if (ret == 0) { /* vm->def stolen dev->data.* so we just need to free the dev * envelope */ - VIR_FREE(dev); + virDomainDeviceDefListFreeShallow(devlist); } if (ret == 0 || fail) ret = testQemuHotplugCheckResult(vm, result_xml, @@ -352,7 +364,7 @@ testQemuHotplug(const void *data) virObjectUnref(vm); test->vm = NULL; } - virDomainDeviceDefFree(dev); + virDomainDeviceDefListFree(devlist); virObjectUnref(caps); qemuMonitorTestFree(test_mon); return ((ret < 0 && fail) || (!ret && !fail)) ? 0 : -1; @@ -817,6 +829,18 @@ mymain(void) "device_add", QMP_OK); DO_TEST_DETACH("pseries-base-live", "hostdev-pci", false, false, "device_del", QMP_DEVICE_DELETED("hostdev0") QMP_OK); + DO_TEST_ATTACH("base-live", "multifunction-hostdev-pci", false, false, + "device_add", QMP_OK, + "device_add", QMP_OK, + "device_add", QMP_OK, + "device_add", QMP_OK); + + qemuTestSetHostArch(&driver, VIR_ARCH_PPC64); + DO_TEST_ATTACH("pseries-base-live", "multifunction-hostdev-pci-2", false, false, + "device_add", QMP_OK, + "device_add", QMP_OK, + "device_add", QMP_OK); + qemuTestSetHostArch(&driver, VIR_ARCH_X86_64); DO_TEST_ATTACH("base-live", "watchdog", false, true, "watchdog-set-action", QMP_OK, diff --git a/tests/qemuhotplugtestdevices/qemuhotplug-multifunction-hostdev-pci-2.xml b/tests/qemuhotplugtestdevices/qemuhotplug-multifunction-hostdev-pci-2.xml new file mode 100644 index 0000000000..02e2236e5d --- /dev/null +++ b/tests/qemuhotplugtestdevices/qemuhotplug-multifunction-hostdev-pci-2.xml @@ -0,0 +1,14 @@ +<devices> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0001' bus='0x01' slot='0x00' function='0x1'/> + </source> + </hostdev> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0001' bus='0x01' slot='0x00' function='0x0'/> + </source> + </hostdev> +</devices> diff --git a/tests/qemuhotplugtestdevices/qemuhotplug-multifunction-hostdev-pci.xml b/tests/qemuhotplugtestdevices/qemuhotplug-multifunction-hostdev-pci.xml new file mode 100644 index 0000000000..bef4be219f --- /dev/null +++ b/tests/qemuhotplugtestdevices/qemuhotplug-multifunction-hostdev-pci.xml @@ -0,0 +1,26 @@ +<devices> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x1'/> + </source> + </hostdev> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x2'/> + </source> + </hostdev> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x3'/> + </source> + </hostdev> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x0'/> + </source> + </hostdev> +</devices> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-live+multifunction-hostdev-pci.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+multifunction-hostdev-pci.xml new file mode 100644 index 0000000000..9dd04d3350 --- /dev/null +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+multifunction-hostdev-pci.xml @@ -0,0 +1,82 @@ +<domain type='kvm' id='7'> + <name>hotplug</name> + <uuid>d091ea82-29e6-2e34-3005-f02617b36e87</uuid> + <memory unit='KiB'>4194304</memory> + <currentMemory unit='KiB'>4194304</currentMemory> + <vcpu placement='static'>4</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0'> + <alias name='usb'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <alias name='ide'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='scsi' index='0' model='virtio-scsi'> + <alias name='scsi0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci'/> + </controller> + <controller type='virtio-serial' index='0'> + <alias name='virtio-serial0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </controller> + <input type='mouse' bus='ps2'> + <alias name='input0'/> + </input> + <input type='keyboard' bus='ps2'> + <alias name='input1'/> + </input> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x3'/> + </source> + <alias name='hostdev0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x3'/> + </hostdev> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x2'/> + </source> + <alias name='hostdev1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x2'/> + </hostdev> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x1'/> + </source> + <alias name='hostdev2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x1'/> + </hostdev> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x0'/> + </source> + <alias name='hostdev3'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0' multifunction='on'/> + </hostdev> + <memballoon model='none'/> + </devices> + <seclabel type='none' model='none'/> +</domain> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-pseries-base-live+multifunction-hostdev-pci-2.xml b/tests/qemuhotplugtestdomains/qemuhotplug-pseries-base-live+multifunction-hostdev-pci-2.xml new file mode 100644 index 0000000000..f99c1b3741 --- /dev/null +++ b/tests/qemuhotplugtestdomains/qemuhotplug-pseries-base-live+multifunction-hostdev-pci-2.xml @@ -0,0 +1,59 @@ +<domain type='kvm' id='7'> + <name>hotplug</name> + <uuid>d091ea82-29e6-2e34-3005-f02617b36e87</uuid> + <memory unit='KiB'>4194304</memory> + <currentMemory unit='KiB'>4194304</currentMemory> + <vcpu placement='static'>4</vcpu> + <os> + <type arch='ppc64' machine='pseries'>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-ppc64</emulator> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + <alias name='pci.0'/> + </controller> + <controller type='pci' index='1' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='1'/> + <alias name='pci.1'/> + </controller> + <controller type='usb' index='0'> + <alias name='usb'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <input type='keyboard' bus='usb'> + <alias name='input0'/> + <address type='usb' bus='0' port='1'/> + </input> + <input type='mouse' bus='usb'> + <alias name='input1'/> + <address type='usb' bus='0' port='2'/> + </input> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0001' bus='0x01' slot='0x00' function='0x1'/> + </source> + <alias name='hostdev0'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x01' function='0x1'/> + </hostdev> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0001' bus='0x01' slot='0x00' function='0x0'/> + </source> + <alias name='hostdev1'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x01' function='0x0' multifunction='on'/> + </hostdev> + <memballoon model='none'/> + <panic model='pseries'/> + </devices> + <seclabel type='none' model='none'/> +</domain> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-pseries-base-live+multifunction-hostdev-pci.xml b/tests/qemuhotplugtestdomains/qemuhotplug-pseries-base-live+multifunction-hostdev-pci.xml new file mode 100644 index 0000000000..9338d42e2e --- /dev/null +++ b/tests/qemuhotplugtestdomains/qemuhotplug-pseries-base-live+multifunction-hostdev-pci.xml @@ -0,0 +1,69 @@ +<domain type='kvm' id='7'> + <name>hotplug</name> + <uuid>d091ea82-29e6-2e34-3005-f02617b36e87</uuid> + <memory unit='KiB'>4194304</memory> + <currentMemory unit='KiB'>4194304</currentMemory> + <vcpu placement='static'>4</vcpu> + <os> + <type arch='ppc64' machine='pseries'>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-ppc64</emulator> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + <alias name='pci.0'/> + </controller> + <controller type='pci' index='1' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='1'/> + <alias name='pci.1'/> + </controller> + <controller type='usb' index='0'> + <alias name='usb'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <input type='keyboard' bus='usb'> + <alias name='input0'/> + <address type='usb' bus='0' port='1'/> + </input> + <input type='mouse' bus='usb'> + <alias name='input1'/> + <address type='usb' bus='0' port='2'/> + </input> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x2'/> + </source> + <alias name='hostdev0'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x01' function='0x2'/> + </hostdev> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x1'/> + </source> + <alias name='hostdev1'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x01' function='0x1'/> + </hostdev> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x0'/> + </source> + <alias name='hostdev2'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x01' function='0x0' multifunction='on'/> + </hostdev> + <memballoon model='none'> + <alias name='balloon0'/> + </memballoon> + <panic model='pseries'/> + </devices> + <seclabel type='none' model='none'/> +</domain> -- 2.24.1

From: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> The same design ideas used in multifunction hotplug were used here as well. qemuDomainDetachDeviceConfig() was changed to accept a list of devices instead of a single device definition. qemuDomainDetachDeviceLiveAndConfig() was changed to handle device lists as well. For VIR_DOMAIN_AFFECT_LIVE, check if we're handling a single device unplug (devlist size = 1) and forward it to the regular qemuDomainDetachDeviceLive() function. Otherwise, a new qemuDomainDetachMultifunctionDevice() was added to handle the hot-unplug of multifunction devices. DetachMultifunctionDevice() is not considering the new multifunction unplug mechanics for the Pseries guest, present in QEMU 4.2 (the newest release ATM), to not break compatibility with older QEMU versions. This will be done properly in a later patch. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 65 ++++++++++++++------- src/qemu/qemu_hotplug.c | 121 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.h | 5 ++ tests/qemuhotplugtest.c | 30 +++++++--- 4 files changed, 193 insertions(+), 28 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5089050328..9863a7576b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8563,17 +8563,27 @@ qemuDomainDetachDeviceConfigInternal(virDomainDefPtr vmdef, static int qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr devlist, virQEMUCapsPtr qemuCaps, unsigned int parse_flags, virDomainXMLOptionPtr xmlopt) { - if (qemuDomainDetachDeviceConfigInternal(vmdef, dev)) - return -1; + size_t i; + + for (i = 0; i < devlist->count; i++) { + if (qemuDomainDetachDeviceConfigInternal(vmdef, devlist->devs[i])) + return -1; + } if (virDomainDefPostParse(vmdef, parse_flags, xmlopt, qemuCaps) < 0) return -1; + /* Don't allow removing the primary function alone for a + * multifunction device (devlist->count greater than 1) + * leading to guest start failure later. */ + if (devlist->count > 1 && qemuDomainDefValidatePCIHostdevs(vmdef) < 0) + return -1; + return 0; } @@ -9006,7 +9016,9 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; g_autoptr(virQEMUDriverConfig) cfg = NULL; - virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; + virDomainDeviceDefListPtr devlist = NULL, devcopylist = NULL; + virDomainDeviceDefListData data = {.def = vm->def, + .xmlopt = driver->xmlopt}; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE; virDomainDefPtr vmdef = NULL; int ret = -1; @@ -9020,11 +9032,11 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver, !(flags & VIR_DOMAIN_AFFECT_LIVE)) parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE; - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, - driver->xmlopt, priv->qemuCaps, - parse_flags); - if (dev == NULL) + devlist = qemuDomainDeviceParseXMLMany(xml, &data, + priv->qemuCaps, parse_flags); + if (!devlist) goto cleanup; + devcopylist = devlist; if (flags & VIR_DOMAIN_AFFECT_CONFIG && flags & VIR_DOMAIN_AFFECT_LIVE) { @@ -9032,9 +9044,9 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver, * create a deep copy of device as adding * to CONFIG takes one instance. */ - dev_copy = virDomainDeviceDefCopy(dev, vm->def, - driver->xmlopt, priv->qemuCaps); - if (!dev_copy) + devcopylist = virDomainDeviceDefListCopy(devlist, &data, + priv->qemuCaps); + if (!devcopylist) goto cleanup; } @@ -9044,7 +9056,7 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver, if (!vmdef) goto cleanup; - if (qemuDomainDetachDeviceConfig(vmdef, dev, priv->qemuCaps, + if (qemuDomainDetachDeviceConfig(vmdef, devlist, priv->qemuCaps, parse_flags, driver->xmlopt) < 0) goto cleanup; @@ -9053,7 +9065,13 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver, if (flags & VIR_DOMAIN_AFFECT_LIVE) { int rc; - if ((rc = qemuDomainDetachDeviceLive(vm, dev_copy, driver, false)) < 0) + if (devlist->count > 1) { + if ((rc = qemuDomainDetachMultifunctionDevice(vm, devlist, driver)) < 0) + goto cleanup; + } else if ((rc = qemuDomainDetachDeviceLive(vm, + devcopylist->devs[0], + driver, + false)) < 0) goto cleanup; if (rc == 0 && qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) @@ -9080,9 +9098,9 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver, ret = 0; cleanup: - if (dev != dev_copy) - virDomainDeviceDefFree(dev_copy); - virDomainDeviceDefFree(dev); + if (devlist != devcopylist) + virDomainDeviceDefListFree(devcopylist); + virDomainDeviceDefListFree(devlist); virDomainDefFree(vmdef); return ret; } @@ -9100,6 +9118,7 @@ qemuDomainDetachDeviceAliasLiveAndConfig(virQEMUDriverPtr driver, virDomainDefPtr persistentDef = NULL; virDomainDefPtr vmdef = NULL; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE; + virDomainDeviceDefListPtr devlist = NULL; int ret = -1; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -9115,16 +9134,22 @@ qemuDomainDetachDeviceAliasLiveAndConfig(virQEMUDriverPtr driver, goto cleanup; if (persistentDef) { - virDomainDeviceDef dev; + virDomainDeviceDefPtr dev; if (!(vmdef = virDomainObjCopyPersistentDef(vm, driver->xmlopt, priv->qemuCaps))) goto cleanup; - if (virDomainDefFindDevice(vmdef, alias, &dev, true) < 0) + if (virDomainDefFindDevice(vmdef, alias, dev, true) < 0) + goto cleanup; + + if (VIR_ALLOC(devlist) < 0) goto cleanup; - if (qemuDomainDetachDeviceConfig(vmdef, &dev, priv->qemuCaps, + if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) + goto cleanup; + + if (qemuDomainDetachDeviceConfig(vmdef, devlist, priv->qemuCaps, parse_flags, driver->xmlopt) < 0) goto cleanup; } @@ -9153,6 +9178,8 @@ qemuDomainDetachDeviceAliasLiveAndConfig(virQEMUDriverPtr driver, ret = 0; cleanup: virDomainDefFree(vmdef); + virDomainDeviceDefListFree(devlist); + return ret; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 495a5c9bdd..deeb94fe79 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -5995,6 +5995,127 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, } +int +qemuDomainDetachMultifunctionDevice(virDomainObjPtr vm, + virDomainDeviceDefListPtr devlist, + virQEMUDriverPtr driver) +{ + size_t i; + int ret = -1; + virBitmapPtr tbdmap = NULL, onlinemap = NULL; + int *functions = NULL; + virDomainHostdevDefPtr hostdev, detach = NULL; + virDomainHostdevSubsysPtr subsys = NULL; + int slotaggridx = 0; + virDomainHostdevSubsysPCIPtr pcisrc = NULL; + qemuDomainObjPrivatePtr priv = vm->privateData; + + qsort(devlist->devs, devlist->count, sizeof(*devlist->devs), + qemuiHostdevPCIMultifunctionDevicesListSort); + +#define FOR_EACH_DEV_IN_DEVLIST() \ + for (i = 0; i < devlist->count; i++) { \ + hostdev = devlist->devs[i]->data.hostdev; \ + subsys = &hostdev->source.subsys; \ + pcisrc = &subsys->u.pci; \ + virDomainHostdevFind(vm->def, hostdev, &detach); + + FOR_EACH_DEV_IN_DEVLIST() + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("hot unplug is not supported for hostdev mode '%s'"), + virDomainHostdevModeTypeToString(hostdev->mode)); + goto cleanup; + } + } + + FOR_EACH_DEV_IN_DEVLIST() + if (!detach) { + virReportError(VIR_ERR_DEVICE_MISSING, + _("host pci device %.4x:%.2x:%.2x.%.1x not found"), + pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); + goto cleanup; + } + } + + /* Check if the devices belong to same guest slot.*/ + FOR_EACH_DEV_IN_DEVLIST() + /* Pick one aggregateSlotIdx and compare against rest of them */ + slotaggridx = slotaggridx ? slotaggridx : detach->info->aggregateSlotIdx; + if (slotaggridx != detach->info->aggregateSlotIdx) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("host pci device %.4x:%.2x:%.2x.%.1x does " + "not belong to same slot"), + pcisrc->addr.domain, + pcisrc->addr.bus, + pcisrc->addr.slot, + pcisrc->addr.function); + goto cleanup; + } + } + + /* Check if the whole slot is being removed or not */ + onlinemap = virDomainDefHostdevGetPCIOnlineFunctionMap(vm->def, slotaggridx); + FOR_EACH_DEV_IN_DEVLIST() + ignore_value(virBitmapClearBit(onlinemap, pcisrc->addr.function)); + } + + if (!virBitmapIsAllClear(onlinemap)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("hot unplug of partial PCI slot not allowed")); + goto cleanup; + } + + /* Mark all aliases for removal */ + memset(&priv->unplug, 0, sizeof(priv->unplug)); + FOR_EACH_DEV_IN_DEVLIST() + if (qemuAssignDeviceHostdevAlias(vm->def, &detach->info->alias, -1) < 0) + goto reset; + + qemuDomainMarkDeviceAliasForRemoval(vm, detach->info->alias, false); + } + + qemuDomainObjEnterMonitor(driver, vm); + + /* must plug non-zero first, zero at last */ + for (i = devlist->count; i > 0; i--) { + hostdev = devlist->devs[i -1]->data.hostdev; + subsys = &hostdev->source.subsys; + pcisrc = &subsys->u.pci; + virDomainHostdevFind(vm->def, hostdev, &detach); + + if (qemuMonitorDelDevice(priv->mon, detach->info->alias) < 0) { + ignore_value(qemuDomainObjExitMonitor(driver, vm)); + if (virDomainObjIsActive(vm)) + virDomainAuditHostdev(vm, detach, "detach", false); + goto reset; + } + if (ARCH_IS_X86(vm->def->os.arch)) + break; /* deleting any one is enough! */ + } + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; + + if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) { + FOR_EACH_DEV_IN_DEVLIST() + ret = qemuDomainRemoveHostDevice(driver, vm, detach); + } + } + reset: + qemuDomainResetDeviceRemoval(vm); + cleanup: + VIR_FREE(functions); + virBitmapFree(onlinemap); + virBitmapFree(tbdmap); + +#undef FOR_EACH_DEV_IN_DEVLIST + + return ret; +} + + static int qemuDomainRemoveVcpu(virQEMUDriverPtr driver, virDomainObjPtr vm, diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 9fd262a81b..072984ddf8 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -131,6 +131,11 @@ qemuDomainAttachMultifunctionDevice(virDomainObjPtr vm, virDomainDeviceDefListPtr devlist, virQEMUDriverPtr driver); +int +qemuDomainDetachMultifunctionDevice(virDomainObjPtr vm, + virDomainDeviceDefListPtr devlist, + virQEMUDriverPtr driver); + int qemuDomainChrInsert(virDomainDefPtr vmdef, virDomainChrDefPtr chr); diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 9c84b639a6..ffd91b5250 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -156,10 +156,15 @@ testQemuHotplugAttach(virDomainObjPtr vm, static int testQemuHotplugDetach(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr devlist, bool async) { int ret = -1; + virDomainDeviceDefPtr dev; + + if (devlist->count > 1) + return qemuDomainDetachMultifunctionDevice(vm, devlist, &driver); + dev = devlist->devs[0]; switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: @@ -249,7 +254,6 @@ testQemuHotplug(const void *data) bool keep = test->keep; unsigned int device_parse_flags = 0; virDomainObjPtr vm = NULL; - virDomainDeviceDefPtr dev = NULL; /* temporary */ virDomainDeviceDefListPtr devlist = NULL; virDomainDeviceDefListData listdata; virCapsPtr caps = NULL; @@ -296,8 +300,6 @@ testQemuHotplug(const void *data) if (!devlist) goto cleanup; - dev = devlist->devs[0]; /* temporary */ - /* Now is the best time to feed the spoofed monitor with predefined * replies. */ if (!(test_mon = qemuMonitorTestNew(driver.xmlopt, vm, &driver, @@ -338,14 +340,15 @@ testQemuHotplug(const void *data) break; case DETACH: - ret = testQemuHotplugDetach(vm, dev, false); + ret = testQemuHotplugDetach(vm, devlist, false); + if (ret == 0 || fail) ret = testQemuHotplugCheckResult(vm, domain_xml, domain_filename, fail); break; case UPDATE: - ret = testQemuHotplugUpdate(vm, dev); + ret = testQemuHotplugUpdate(vm, devlist->devs[0]); } cleanup: @@ -829,17 +832,26 @@ mymain(void) "device_add", QMP_OK); DO_TEST_DETACH("pseries-base-live", "hostdev-pci", false, false, "device_del", QMP_DEVICE_DELETED("hostdev0") QMP_OK); - DO_TEST_ATTACH("base-live", "multifunction-hostdev-pci", false, false, + DO_TEST_ATTACH("base-live", "multifunction-hostdev-pci", false, true, "device_add", QMP_OK, "device_add", QMP_OK, "device_add", QMP_OK, "device_add", QMP_OK); + DO_TEST_DETACH("base-live", "multifunction-hostdev-pci", false, false, + "device_del", QMP_DEVICE_DELETED("hostdev3") + QMP_DEVICE_DELETED("hostdev2") + QMP_DEVICE_DELETED("hostdev1") + QMP_DEVICE_DELETED("hostdev0") QMP_OK); qemuTestSetHostArch(&driver, VIR_ARCH_PPC64); - DO_TEST_ATTACH("pseries-base-live", "multifunction-hostdev-pci-2", false, false, - "device_add", QMP_OK, + DO_TEST_ATTACH("pseries-base-live", "multifunction-hostdev-pci-2", false, true, "device_add", QMP_OK, "device_add", QMP_OK); + DO_TEST_DETACH("pseries-base-live", "multifunction-hostdev-pci-2", false, false, + "device_del", QMP_OK, + "device_del", QMP_DEVICE_DELETED("hostdev1") + QMP_DEVICE_DELETED("hostdev0") QMP_OK); + qemuTestSetHostArch(&driver, VIR_ARCH_X86_64); DO_TEST_ATTACH("base-live", "watchdog", false, true, -- 2.24.1

The new address type 'unassigned' allows for a hostdev to be managed by Libvirt, but not be assigned for guest usage. This can allow us to execute PCI multifunction hotplug, with managed mode, leaving non-zero functions unassigned. For hotplug and unplug , we'll skip unassigned devices when setting or evaluating aggregateSlotIdx and when we're about to execute device_add/device_del in QEMU. For hotplug we'll also skip adress assignment. For unplug we won't mark the device for removal since we won't be executing device_del on it. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_domain_address.c | 6 ++ src/qemu/qemu_hotplug.c | 16 ++++ tests/qemuhotplugtest.c | 9 ++ ...plug-multifunction-hostdev-pci-partial.xml | 27 ++++++ ...live+multifunction-hostdev-pci-partial.xml | 82 +++++++++++++++++++ 5 files changed, 140 insertions(+) create mode 100644 tests/qemuhotplugtestdevices/qemuhotplug-multifunction-hostdev-pci-partial.xml create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-base-live+multifunction-hostdev-pci-partial.xml diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 395f054b8d..c8fe6bbd64 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -3511,6 +3511,9 @@ qemuDomainPCIMultifunctionHostdevEnsurePCIAddresses(virDomainObjPtr vm, for (i = 0; i < devlist->count; i++) { virDomainHostdevDefPtr hostdev = devlist->devs[i]->data.hostdev; + if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED) + continue; + if (qemuDomainIsPSeries(vm->def)) { /* Isolation groups are only relevant for pSeries guests */ if (qemuDomainFillDeviceIsolationGroup(vm->def, devlist->devs[i]) < 0) @@ -3544,6 +3547,9 @@ qemuDomainPCIMultifunctionHostdevEnsurePCIAddresses(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev = devlist->devs[i]->data.hostdev; virPCIDeviceAddress addr = hostdev->source.subsys.u.pci.addr; + if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED) + continue; + devinfos->infos[addr.function] = hostdev->info; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index fe823bb910..00bd5499fe 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1629,6 +1629,13 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, goto error; } + /* For an unassigned hostdev, add it to the domain definition + * and return without hotplugging it to QEMU. */ + if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED) { + vm->def->hostdevs[vm->def->nhostdevs++] = hostdev; + return 0; + } + if (!(devstr = qemuBuildPCIHostdevDevStr(vm->def, hostdev, 0, priv->qemuCaps))) goto error; @@ -6058,6 +6065,9 @@ qemuDomainDetachMultifunctionDevice(virDomainObjPtr vm, /* Check if the devices belong to same guest slot.*/ FOR_EACH_DEV_IN_DEVLIST() + if (detach->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED) + continue; + /* Pick one aggregateSlotIdx and compare against rest of them */ slotaggridx = slotaggridx ? slotaggridx : detach->info->aggregateSlotIdx; if (slotaggridx != detach->info->aggregateSlotIdx) { @@ -6090,6 +6100,9 @@ qemuDomainDetachMultifunctionDevice(virDomainObjPtr vm, if (qemuAssignDeviceHostdevAlias(vm->def, &detach->info->alias, -1) < 0) goto reset; + if (detach->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED) + continue; + qemuDomainMarkDeviceAliasForRemoval(vm, detach->info->alias, false); } @@ -6102,6 +6115,9 @@ qemuDomainDetachMultifunctionDevice(virDomainObjPtr vm, pcisrc = &subsys->u.pci; virDomainHostdevFind(vm->def, hostdev, &detach); + if (detach->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED) + continue; + if (qemuMonitorDelDevice(priv->mon, detach->info->alias) < 0) { ignore_value(qemuDomainObjExitMonitor(driver, vm)); if (virDomainObjIsActive(vm)) diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index ffd91b5250..50e4bd535b 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -843,6 +843,15 @@ mymain(void) QMP_DEVICE_DELETED("hostdev1") QMP_DEVICE_DELETED("hostdev0") QMP_OK); + DO_TEST_ATTACH("base-live", "multifunction-hostdev-pci-partial", false, true, + "device_add", QMP_OK, + "device_add", QMP_OK, + "device_add", QMP_OK); + DO_TEST_DETACH("base-live", "multifunction-hostdev-pci-partial", false, false, + "device_del", QMP_DEVICE_DELETED("hostdev3") + QMP_DEVICE_DELETED("hostdev2") + QMP_DEVICE_DELETED("hostdev0") QMP_OK); + qemuTestSetHostArch(&driver, VIR_ARCH_PPC64); DO_TEST_ATTACH("pseries-base-live", "multifunction-hostdev-pci-2", false, true, "device_add", QMP_OK, diff --git a/tests/qemuhotplugtestdevices/qemuhotplug-multifunction-hostdev-pci-partial.xml b/tests/qemuhotplugtestdevices/qemuhotplug-multifunction-hostdev-pci-partial.xml new file mode 100644 index 0000000000..5e01564d2b --- /dev/null +++ b/tests/qemuhotplugtestdevices/qemuhotplug-multifunction-hostdev-pci-partial.xml @@ -0,0 +1,27 @@ +<devices> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x1'/> + </source> + </hostdev> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x2'/> + </source> + <address type='unassigned'/> + </hostdev> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x3'/> + </source> + </hostdev> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x0'/> + </source> + </hostdev> +</devices> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-live+multifunction-hostdev-pci-partial.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+multifunction-hostdev-pci-partial.xml new file mode 100644 index 0000000000..1808a0071e --- /dev/null +++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+multifunction-hostdev-pci-partial.xml @@ -0,0 +1,82 @@ +<domain type='kvm' id='7'> + <name>hotplug</name> + <uuid>d091ea82-29e6-2e34-3005-f02617b36e87</uuid> + <memory unit='KiB'>4194304</memory> + <currentMemory unit='KiB'>4194304</currentMemory> + <vcpu placement='static'>4</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0'> + <alias name='usb'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <alias name='ide'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='scsi' index='0' model='virtio-scsi'> + <alias name='scsi0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci'/> + </controller> + <controller type='virtio-serial' index='0'> + <alias name='virtio-serial0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </controller> + <input type='mouse' bus='ps2'> + <alias name='input0'/> + </input> + <input type='keyboard' bus='ps2'> + <alias name='input1'/> + </input> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x3'/> + </source> + <alias name='hostdev0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x3'/> + </hostdev> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x2'/> + </source> + <alias name='hostdev1'/> + <address type='unassigned'/> + </hostdev> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x1'/> + </source> + <alias name='hostdev2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x1'/> + </hostdev> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x90' slot='0x01' function='0x0'/> + </source> + <alias name='hostdev3'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0' multifunction='on'/> + </hostdev> + <memballoon model='none'/> + </devices> + <seclabel type='none' model='none'/> +</domain> -- 2.24.1