[libvirt] [PATCH 0/7] PCI Multifunction device hotplug support
The series implements the PCI multifunction hotplug functionality. The first two patch reverts the old commits which forbade the multifunction addresses. Patch2 is added because the two reverts necessiates additional checks added in Patch3. Actual implementation starts patch 4 onwards. The semantics is to encapsulate all the functions in <devices/> tag as was previously explained. (http://www.spinics.net/linux/fedora/libvir/msg127921.html The series aims to support virtio devices also the hostdevices in the <devices>. The auto addess assignment is implemented here works well for ppc64 and pc-i440fx machines. Couldn't get it working on q35 machines. May be follow up patches for that. Todo: 1) Rollback the Hotplug of already hoplugged functions if any of the function hotplug fails. 2) Hardening the hotplug checks to disallow multifunction cards hotplug as though they are single function cards. 3) Documentation update. 4) Patch 6 may need some work across drivers to add similar checks as added in qemu_driver.c 5) Test cases. --- Shivaprasad G Bhat (7): Revert "prevent hot unplugging multi function PCI device" Release address in function granularity than slot Validate address in virDomainPCIAddressReleaseAddr Introduce PCI Multifunction device parser Introduce virDomainPCIMultifunctionDeviceAddressAssign Allow calling virHostdevPreparePCIDevices twice for same device Enable PCI Multifunction hotplug/unplug src/conf/domain_addr.c | 244 ++++++++++++++++++++++++++++- src/conf/domain_addr.h | 4 src/conf/domain_conf.c | 227 +++++++++++++++++++++++++++ src/conf/domain_conf.h | 22 +++ src/libvirt_private.syms | 5 + src/qemu/qemu_domain_address.c | 2 src/qemu/qemu_driver.c | 331 ++++++++++++++++++++++++++++++++-------- src/qemu/qemu_hotplug.c | 74 ++------- src/util/virhostdev.c | 16 ++ 9 files changed, 781 insertions(+), 144 deletions(-) -- Signature
This has to go. The unlugging is supported. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/qemu/qemu_hotplug.c | 60 ----------------------------------------------- 1 file changed, 60 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f40b34d..5b822f9 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2770,35 +2770,6 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver, return ret; } - -static int qemuComparePCIDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, - virDomainDeviceDefPtr device ATTRIBUTE_UNUSED, - virDomainDeviceInfoPtr info1, - void *opaque) -{ - virDomainDeviceInfoPtr info2 = opaque; - - if (info1->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI || - info2->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) - return 0; - - if (info1->addr.pci.domain == info2->addr.pci.domain && - info1->addr.pci.bus == info2->addr.pci.bus && - info1->addr.pci.slot == info2->addr.pci.slot && - info1->addr.pci.function != info2->addr.pci.function) - return -1; - return 0; -} - -static bool qemuIsMultiFunctionDevice(virDomainDefPtr def, - virDomainDeviceInfoPtr dev) -{ - if (virDomainDeviceInfoIterate(def, qemuComparePCIDevice, dev) < 0) - return true; - return false; -} - - static int qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -3407,13 +3378,6 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; - if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("cannot hot unplug multifunction PCI device: %s"), - detach->dst); - goto cleanup; - } - if (qemuDomainMachineIsS390CCW(vm->def) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { if (!virDomainDeviceAddressIsValid(&detach->info, @@ -3636,14 +3600,6 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver, goto cleanup; } - if (detach->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && - qemuIsMultiFunctionDevice(vm->def, &detach->info)) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("cannot hot unplug multifunction PCI device: %s"), - dev->data.disk->dst); - goto cleanup; - } - if (qemuDomainControllerIsBusy(vm, detach)) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("device cannot be detached: device is busy")); @@ -3679,17 +3635,8 @@ qemuDomainDetachHostPCIDevice(virQEMUDriverPtr driver, virDomainHostdevDefPtr detach) { qemuDomainObjPrivatePtr priv = vm->privateData; - virDomainHostdevSubsysPCIPtr pcisrc = &detach->source.subsys.u.pci; int ret; - if (qemuIsMultiFunctionDevice(vm->def, detach->info)) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("cannot hot unplug multifunction PCI device: %.4x:%.2x:%.2x.%.1x"), - pcisrc->addr.domain, pcisrc->addr.bus, - pcisrc->addr.slot, pcisrc->addr.function); - return -1; - } - if (!virDomainDeviceAddressIsValid(detach->info, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { virReportError(VIR_ERR_OPERATION_FAILED, @@ -3921,13 +3868,6 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver, "%s", _("device cannot be detached without a PCI address")); goto cleanup; } - - if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("cannot hot unplug multifunction PCI device :%s"), - dev->data.disk->dst); - goto cleanup; - } } if (!detach->info.alias) {
The commit 6fe678c is partly reverted. The code is moved around and cant revert staright. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/conf/domain_addr.c | 22 +++++++++------------- src/libvirt_private.syms | 1 + src/qemu/qemu_domain_address.c | 2 +- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index acd8ce6..35c7cd4 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -472,21 +472,17 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, goto cleanup; if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - /* We do not support hotplug multi-function PCI device now, so we should - * reserve the whole slot. The function of the PCI device must be 0. - */ - if (dev->addr.pci.function != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Only PCI device addresses with function=0" - " are supported")); - goto cleanup; - } + if (((dev->addr.pci.function == 0) && (dev->addr.pci.multi == VIR_TRISTATE_SWITCH_ON)) || + dev->addr.pci.function != 0) { - if (!virDomainPCIAddressValidate(addrs, &dev->addr.pci, - addrStr, flags, true)) - goto cleanup; + if (!virDomainPCIAddressValidate(addrs, &dev->addr.pci, + addrStr, flags, true)) + goto cleanup; - ret = virDomainPCIAddressReserveSlot(addrs, &dev->addr.pci, flags); + ret = virDomainPCIAddressReserveAddr(addrs, &dev->addr.pci, flags, false, true); + } else { + ret = virDomainPCIAddressReserveSlot(addrs, &dev->addr.pci, flags); + } } else { ret = virDomainPCIAddressReserveNextSlot(addrs, dev, flags); } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fb24808..e4953b7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -98,6 +98,7 @@ virDomainPCIAddressBusSetModel; virDomainPCIAddressEnsureAddr; virDomainPCIAddressFlagsCompatible; virDomainPCIAddressGetNextSlot; +virDomainPCIAddressReleaseAddr; virDomainPCIAddressReleaseSlot; virDomainPCIAddressReserveAddr; virDomainPCIAddressReserveNextSlot; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 9c8c262..1e7d98c 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1682,7 +1682,7 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, NULLSTR(devstr)); else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - virDomainPCIAddressReleaseSlot(priv->pciaddrs, + virDomainPCIAddressReleaseAddr(priv->pciaddrs, &info->addr.pci) < 0) VIR_WARN("Unable to release PCI address on %s", NULLSTR(devstr));
This function was not used so far. Now, that we begin to use it, make sure to check the address before actually releasing the address. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/conf/domain_addr.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 35c7cd4..7ea9e4d 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -497,8 +497,24 @@ int virDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr) { + /* permit any kind of connection type in validation, since we + * already had it, and are giving it back. + */ + virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPES_MASK; + int ret = -1; + char *addrStr = NULL; + + if (!(addrStr = virDomainPCIAddressAsString(addr))) + goto cleanup; + + if (!virDomainPCIAddressValidate(addrs, addr, addrStr, flags, false)) + goto cleanup; + addrs->buses[addr->bus].slots[addr->slot] &= ~(1 << addr->function); - return 0; + ret = 0; + cleanup: + VIR_FREE(addrStr); + return ret; } int
This patch just introduces the parser function used by the later patches. The parser disallows hostdevices to be used with other virtio devices simultaneously. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 227 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 22 ++++ src/libvirt_private.syms | 3 + 3 files changed, 252 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ed0c471..6149d24 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -860,6 +860,36 @@ virDomainXMLOptionClassDispose(void *obj) (xmlopt->config.privFree)(xmlopt->config.priv); } +/* virDomainDeviceDefListAddCopy - add a *copy* of the device to this list */ +int +virDomainDeviceDefListAddCopy(virDomainDeviceDefListPtr list, + virDomainDeviceDefPtr dev, + const virDomainDef *def, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt) +{ + virDomainDeviceDefPtr copy = virDomainDeviceDefCopy(dev, def, caps, xmlopt); + + if (!copy) + return -1; + if (VIR_APPEND_ELEMENT(list->devs, list->count, copy) < 0) { + virDomainDeviceDefFree(copy); + return -1; + } + return 0; +} + +void virDomainDeviceDefListDispose(virDomainDeviceDefListPtr list) +{ + size_t i; + + if (!list) + return; + for (i = 0; i < list->count; i++) + virDomainDeviceDefFree(list->devs[i]); + VIR_FREE(list); +} + /** * virDomainKeyWrapCipherDefParseXML: * @@ -24365,3 +24395,200 @@ virDomainObjGetShortName(virDomainObjPtr vm) return ret; } + +static int +virDomainPCIMultifunctionDeviceDefParseXML(xmlXPathContextPtr ctxt, + const virDomainDef *def, + virDomainDeviceDefListPtr devlist, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + unsigned int flags) +{ + int ret = -1; + size_t i; + int n; + virDomainDeviceDef device; + xmlNodePtr *nodes = NULL; + char *netprefix = NULL; + int nhostdevs = 0; + + device.type = VIR_DOMAIN_DEVICE_DISK; + + if ((n = virXPathNodeSet("./disk", ctxt, &nodes)) < 0) + goto error; + + for (i = 0; i < n; i++) { + virDomainDiskDefPtr disk = virDomainDiskDefParseXML(xmlopt, + nodes[i], + ctxt, + NULL, + def->seclabels, + def->nseclabels, + flags); + if (!disk) + goto error; + + device.data.disk = disk; + if (virDomainDeviceDefListAddCopy(devlist, &device, def, caps, xmlopt) < 0) + goto error; + VIR_FREE(disk); + } + VIR_FREE(nodes); + + device.type = VIR_DOMAIN_DEVICE_CONTROLLER; + /* analysis of the controller devices */ + if ((n = virXPathNodeSet("./controller", ctxt, &nodes)) < 0) + goto error; + + for (i = 0; i < n; i++) { + virDomainControllerDefPtr controller = virDomainControllerDefParseXML(nodes[i], + ctxt, + flags); + if (!controller) + goto error; + + device.data.controller = controller; + if (virDomainDeviceDefListAddCopy(devlist, &device, def, caps, xmlopt) < 0) + goto error; + VIR_FREE(controller); + } + VIR_FREE(nodes); + + device.type = VIR_DOMAIN_DEVICE_NET; + if ((n = virXPathNodeSet("./interface", ctxt, &nodes)) < 0) + goto error; + + netprefix = caps->host.netprefix; + for (i = 0; i < n; i++) { + virDomainNetDefPtr net = virDomainNetDefParseXML(xmlopt, + nodes[i], + ctxt, + NULL, + netprefix, + flags); + if (!net) + goto error; + + device.data.net = net; + if (virDomainDeviceDefListAddCopy(devlist, &device, def, caps, xmlopt) < 0) + goto error; + VIR_FREE(net); + } + VIR_FREE(nodes); + + /* analysis of the host devices */ + device.type = VIR_DOMAIN_DEVICE_HOSTDEV; + if ((nhostdevs = virXPathNodeSet("./hostdev", ctxt, &nodes)) < 0) + goto error; + for (i = 0; i < nhostdevs; i++) { + virDomainHostdevDefPtr hostdev; + + hostdev = virDomainHostdevDefParseXML(xmlopt, nodes[i], ctxt, + NULL, flags); + if (!hostdev) + goto error; + + if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB || + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Can't add host USB device: " + "USB is disabled in this host")); + virDomainHostdevDefFree(hostdev); + goto error; + } + device.data.hostdev = hostdev; + if (virDomainDeviceDefListAddCopy(devlist, &device, def, caps, xmlopt) < 0) + goto error; + VIR_FREE(hostdev); + } + VIR_FREE(nodes); + + /* Parse the RNG devices */ + device.type = VIR_DOMAIN_DEVICE_RNG; + if ((n = virXPathNodeSet("./rng", ctxt, &nodes)) < 0) + goto error; + for (i = 0; i < n; i++) { + virDomainRNGDefPtr rng = virDomainRNGDefParseXML(nodes[i], + ctxt, + flags); + if (!rng) + goto error; + + device.data.rng = rng; + if (virDomainDeviceDefListAddCopy(devlist, &device, def, caps, xmlopt) < 0) + goto error; + VIR_FREE(rng); + } + VIR_FREE(nodes); + + device.type = VIR_DOMAIN_DEVICE_CHR; + if ((n = virXPathNodeSet("./serial", ctxt, &nodes)) < 0) + goto error; + + for (i = 0; i < n; i++) { + virDomainChrDefPtr chr = virDomainChrDefParseXML(ctxt, + nodes[i], + def->seclabels, + def->nseclabels, + flags); + if (!chr) + goto error; + + if (chr->target.port == -1) { + int maxport = -1; + size_t j; + for (j = 0; j < i; j++) { + if (def->serials[j]->target.port > maxport) + maxport = def->serials[j]->target.port; + } + chr->target.port = maxport + 1; + } + device.data.chr = chr; + if (virDomainDeviceDefListAddCopy(devlist, &device, def, caps, xmlopt) < 0) + goto error; + VIR_FREE(chr); + } + VIR_FREE(nodes); + + if (nhostdevs) + ret = nhostdevs != devlist->count ? -1 : 0; + else + ret = 0; + + error: + return ret; +} + + +int +virDomainPCIMultifunctionDeviceDefParseNode(const char *xml, + const virDomainDef *def, + virDomainDeviceDefListPtr devlist, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + unsigned int flags) +{ + xmlXPathContextPtr ctxt = NULL; + int ret = -1; + + xmlDocPtr xmlptr; + + if (!(xmlptr = virXMLParse(NULL, xml, _("(device_definition)")))) { + /* We report error anyway later */ + return -1; + } + + ctxt = xmlXPathNewContext(xmlptr); + if (ctxt == NULL) { + virReportOOMError(); + goto cleanup; + } + + ctxt->node = xmlDocGetRootElement(xmlptr); + ret = virDomainPCIMultifunctionDeviceDefParseXML(ctxt, def, devlist, + caps, xmlopt, flags); + + cleanup: + xmlXPathFreeContext(ctxt); + return ret; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b9e696d..9ddfbd1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2462,6 +2462,20 @@ typedef enum { typedef struct _virDomainXMLOption virDomainXMLOption; typedef virDomainXMLOption *virDomainXMLOptionPtr; +struct virDomainDeviceDefList { + virDomainDeviceDefPtr *devs; + size_t count; +}; +typedef struct virDomainDeviceDefList *virDomainDeviceDefListPtr; + +int +virDomainDeviceDefListAddCopy(virDomainDeviceDefListPtr list, virDomainDeviceDefPtr dev, + const virDomainDef *def, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt); +void virDomainDeviceDefListDispose(virDomainDeviceDefListPtr list); + + /* Called once after everything else has been parsed, for adjusting * overall domain defaults. */ typedef int (*virDomainDefPostParseCallback)(virDomainDefPtr def, @@ -3176,6 +3190,14 @@ int virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def, bool virDomainDefHasMemballoon(const virDomainDef *def) ATTRIBUTE_NONNULL(1); +int +virDomainPCIMultifunctionDeviceDefParseNode(const char *xml, + const virDomainDef *def, + virDomainDeviceDefListPtr devlist, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + unsigned int flags); + char *virDomainObjGetShortName(virDomainObjPtr vm); #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e4953b7..d6a60b5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -109,6 +109,7 @@ virDomainPCIAddressSetGrow; virDomainPCIAddressSlotInUse; virDomainPCIAddressValidate; virDomainPCIControllerModelToConnectType; +virDomainPCIMultifunctionDeviceDefParseNode; virDomainVirtioSerialAddrAssign; virDomainVirtioSerialAddrAutoAssign; virDomainVirtioSerialAddrIsComplete; @@ -249,6 +250,8 @@ virDomainDeviceAddressIsValid; virDomainDeviceAddressTypeToString; virDomainDeviceDefCopy; virDomainDeviceDefFree; +virDomainDeviceDefListAddCopy; +virDomainDeviceDefListDispose; virDomainDeviceDefParse; virDomainDeviceFindControllerModel; virDomainDeviceGetInfo;
This patch assigns multifunction pci addresses to devices in the devlist. The pciaddrs passed in the argument is not altered so that the actual call to reserve the address using virDomainPCIAddressEnsureAddr() passes. The function focuses on user given address validation and also the auto-assign of addresses. The address auto-assignment works well for PPC64 and x86-i440fx machines. The q35 machines needs some level of logic here to get the correct next valid slot so that the hotplug go through fine. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/conf/domain_addr.c | 204 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_addr.h | 4 + src/libvirt_private.syms | 1 3 files changed, 209 insertions(+) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 7ea9e4d..a78a8e4 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -454,6 +454,210 @@ virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, return virDomainPCIAddressReserveAddr(addrs, addr, flags, true, false); } +static int +virDomainPCIAddressGetNextFunctionAddr(uint8_t *slot, + virPCIDeviceAddressPtr addr) +{ + size_t i = 0; + + for (i = 0; i < 8; i++) { + if (!(*slot & 1 << i)) { + addr->function = i; + break; + } + } + + return 0; +} + +static int +virDomainPCIAddressAssignFunctionAddr(virDomainPCIAddressSetPtr addrs, + uint8_t *slot, + virPCIDeviceAddressPtr addr, + virDomainPCIConnectFlags flags, + bool fromConfig) +{ + int ret = -1; + char *addrStr = NULL; + virErrorNumber errType = (fromConfig + ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR); + + if (!(addrStr = virDomainPCIAddressAsString(addr))) + goto cleanup; + + /* Check that the requested bus exists, is the correct type, and we + * are asking for a valid slot and function + */ + if (!virDomainPCIAddressValidate(addrs, addr, addrStr, flags, fromConfig)) + goto cleanup; + + if (*slot & (1 << addr->function)) { + if (addr->function == 0) { + virReportError(errType, + _("Attempted double use of PCI Address %s"), + addrStr); + } else { + virReportError(errType, + _("Attempted double use of PCI Address %s " + "(may need \"multifunction='on'\" " + "for device on function 0)"), addrStr); + } + goto cleanup; + } + *slot |= (1 << addr->function); + if (addr->function == 0) + addr->multi = VIR_TRISTATE_SWITCH_ON; + VIR_DEBUG("Reserving PCI address %s", addrStr); + + ret = 0; + cleanup: + VIR_FREE(addrStr); + return ret; +} + + +static int +virDomainPCIAddressAssignSlotNextFunction(virDomainPCIAddressSetPtr addrs, + uint8_t *slot, + virDomainDeviceInfoPtr dev, + virDomainPCIConnectFlags flags) +{ + if (virDomainPCIAddressGetNextFunctionAddr(slot, &dev->addr.pci) < 0) + return -1; + + if (virDomainPCIAddressAssignFunctionAddr(addrs, slot, &dev->addr.pci, flags, false) < 0) + return -1; + + dev->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + + return 0; +} + + +static int +virDomainPCIAddressAssignSlotAddr(virDomainPCIAddressSetPtr addrs, + uint8_t *slot, + virDomainDeviceInfoPtr dev) +{ + int ret = -1; + char *addrStr = NULL; + virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE | + VIR_PCI_CONNECT_TYPE_PCI_DEVICE); + + if (!(addrStr = virDomainPCIAddressAsString(&dev->addr.pci))) + goto cleanup; + + if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + if (((dev->addr.pci.function == 0) && (dev->addr.pci.multi == VIR_TRISTATE_SWITCH_ON)) || + dev->addr.pci.function != 0) { + + if (!virDomainPCIAddressValidate(addrs, &dev->addr.pci, + addrStr, flags, true)) + goto cleanup; + + ret = virDomainPCIAddressAssignFunctionAddr(addrs, slot, &dev->addr.pci, flags, true); + } else { + virReportError(VIR_ERR_XML_ERROR, + _("Not a multifunction device address addressi %s"), addrStr); + } + } else { + ret = virDomainPCIAddressAssignSlotNextFunction(addrs, slot, dev, flags); + } + + cleanup: + VIR_FREE(addrStr); + return ret; +} + +int +virDomainPCIMultifunctionDeviceAddressAssign(virDomainPCIAddressSetPtr addrs, virDomainDeviceDefListPtr devlist) +{ + size_t i; + virPCIDeviceAddress addr; + virDomainPCIAddressBusPtr bus; + uint8_t slot; + virDomainDeviceInfoPtr info = NULL, privinfo = NULL; + virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPES_MASK; + + if (virDomainPCIAddressGetNextSlot(addrs, &addr, flags) < 0) + return -1; + + bus = &addrs->buses[addr.bus]; + slot = bus->slots[addr.slot]; + + for (i = 0; i < devlist->count; i++) { + virDomainDeviceDefPtr dev = devlist->devs[devlist->count - i - 1]; + switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + info = &dev->data.disk->info; + break; + case VIR_DOMAIN_DEVICE_NET: + info = &dev->data.net->info; + break; + case VIR_DOMAIN_DEVICE_RNG: + info = &dev->data.rng->info; + break; + case VIR_DOMAIN_DEVICE_HOSTDEV: + info = dev->data.hostdev->info; + break; + case VIR_DOMAIN_DEVICE_CONTROLLER: + info = &dev->data.controller->info; + break; + case VIR_DOMAIN_DEVICE_CHR: + info = &dev->data.chr->info; + break; + case VIR_DOMAIN_DEVICE_FS: + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_MEMORY: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_LAST: + break; + } + + if (privinfo) { + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + if (privinfo->addr.pci.bus != info->addr.pci.bus || + privinfo->addr.pci.slot != info->addr.pci.slot) + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Multifunction PCI device " + "cant be split across different guest PCI slots")); + return -1; + + } + } + + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + bus = &addrs->buses[info->addr.pci.bus]; + slot = bus->slots[info->addr.pci.slot]; + } + + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + info->addr.pci.bus = addr.bus; + info->addr.pci.slot = addr.slot; + info->addr.pci.domain = addr.domain; + } + + if (virDomainPCIAddressAssignSlotAddr(addrs, &slot, info) < 0) + return -1; + privinfo = info; + } + return 0; +} + + int virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev) diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index f3eda89..9eb6b9d 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -157,6 +157,10 @@ int virDomainPCIAddressReserveNextSlot(virDomainPCIAddressSetPtr addrs, virDomainPCIConnectFlags flags) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int +virDomainPCIMultifunctionDeviceAddressAssign(virDomainPCIAddressSetPtr addrs, + virDomainDeviceDefListPtr devlist); + struct _virDomainCCWAddressSet { virHashTablePtr defined; virDomainDeviceCCWAddress next; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d6a60b5..d72dc63 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -109,6 +109,7 @@ virDomainPCIAddressSetGrow; virDomainPCIAddressSlotInUse; virDomainPCIAddressValidate; virDomainPCIControllerModelToConnectType; +virDomainPCIMultifunctionDeviceAddressAssign; virDomainPCIMultifunctionDeviceDefParseNode; virDomainVirtioSerialAddrAssign; virDomainVirtioSerialAddrAutoAssign;
The Multifunction PCI devices cant be hotplugged when the other functions are bound to host driver. So, detach them all together once as part of preparation for Multifunction hotplug. The second attempt should just verify if the device was prepared for the same domain and if yes, return. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/qemu/qemu_hotplug.c | 14 +++++++++++++- src/util/virhostdev.c | 16 ++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5b822f9..24f1731 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1193,9 +1193,21 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, int backend; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); unsigned int flags = 0; + size_t i; + virPCIDeviceAddress hostdevaddr = hostdev->source.subsys.u.pci.addr; + + for (i = 0; i < vm->def->nhostdevs; i++) { + virPCIDeviceAddress curaddr = vm->def->hostdevs[i]->source.subsys.u.pci.addr; + if (virPCIDeviceAddressEqual(&curaddr, &hostdevaddr)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("PCI device already assigned to " + "currrent domain")); + goto cleanup; + } + } if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0) - return -1; + goto cleanup; if (!cfg->relaxedACS) flags |= VIR_HOSTDEV_STRICT_ACS_CHECK; diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 980e590..3f84f82 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -542,9 +542,25 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr, * have already been taken care of */ for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i); + virPCIDevicePtr actual; if (virPCIDeviceGetManaged(pci)) { + /* Its possible that the device attempted to be prepared from + * two paths once from the Multifunction device preparation + * and again before the hotplug. Let this function pass if the + * device is already in the active list assigned to current domain. + */ + if (mgr->activePCIHostdevs && + (actual = virPCIDeviceListFind(mgr->activePCIHostdevs, pci))) { + const char *actual_drvname = NULL; + const char *actual_domname = NULL; + virPCIDeviceGetUsedBy(actual, &actual_drvname, &actual_domname); + if (actual_domname && dom_name && + STREQ(actual_domname, dom_name)) + ret = 0; + goto cleanup; + } /* We can't look up the actual device because it has not been * created yet: virPCIDeviceDetach() will insert a copy of 'pci' * into the list of inactive devices, and that copy will be the
The curent patch just assumes the user has given the correct target PCI bus/slot/function address for the device in the xml. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 331 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 263 insertions(+), 68 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 37d970e..883994e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8191,6 +8191,117 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, return 0; } +static bool +qemuDomainPCIAddressIsSingleFunctionAddr(virDomainDeviceDefPtr dev) +{ + + virDomainDeviceInfoPtr info = NULL; + switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + info = &dev->data.disk->info; + break; + case VIR_DOMAIN_DEVICE_NET: + info = &dev->data.net->info; + break; + case VIR_DOMAIN_DEVICE_RNG: + info = &dev->data.rng->info; + break; + case VIR_DOMAIN_DEVICE_HOSTDEV: + info = dev->data.hostdev->info; + break; + case VIR_DOMAIN_DEVICE_CONTROLLER: + info = &dev->data.controller->info; + break; + case VIR_DOMAIN_DEVICE_CHR: + info = &dev->data.chr->info; + break; + case VIR_DOMAIN_DEVICE_FS: + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_MEMORY: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_LAST: + break; + } + + if (info && info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + /* We do not support hotplug multi-function PCI device now, so we should + * reserve the whole slot. The function of the PCI device must be 0. + */ + if (info->addr.pci.function != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Single function device addresses with function=0" + " expected")); + return false; + } + } + return true; +} + +static bool isMultifunctionDeviceXML(const char *xml) +{ + xmlDocPtr xmlptr; + + if (!(xmlptr = virXMLParse(NULL, xml, _("(device_definition)")))) { + /* We report error anyway later */ + return false; + } + + return STREQ((const char *)(xmlDocGetRootElement(xmlptr))->name, "devices"); +} + +/* + * This function focusses on preparations that have dependencies across + * functions. The hostdevice cant be hotplugged if any of other functions + * are bound to hostdriver. + * + * Any known such preparations can be handled here. + */ +static int +qemuDomainPreparePCIMultifunctionDevices(virQEMUDriverPtr driver, + virDomainDefPtr def, + virDomainDeviceDefListPtr devlist, + virQEMUCapsPtr qemuCaps) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + unsigned int flags = 0; + size_t i = 0, j = 0; + int ret = -1; + + if (devlist->devs[0]->type != VIR_DOMAIN_DEVICE_HOSTDEV) + return 0; + + if (!cfg->relaxedACS) + flags |= VIR_HOSTDEV_STRICT_ACS_CHECK; + + for (i = 0; i < devlist->count; i++) + if (qemuHostdevPreparePCIDevices(driver, def->name, def->uuid, + &devlist->devs[i]->data.hostdev, + 1, qemuCaps, flags) < 0) + goto revert; + + ret = 0; + exit: + return ret; + revert: + while (j < i) + qemuHostdevReAttachPCIDevices(driver, def->name, + &devlist->devs[j++]->data.hostdev, 1); + + goto exit; +} static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) @@ -8198,7 +8309,7 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; virDomainDefPtr vmdef = NULL; - virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; + virDomainDeviceDefListPtr devlist = NULL, devcopylist = NULL; int ret = -1; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_ABI_UPDATE; @@ -8207,6 +8318,11 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + virDomainDeviceDefPtr dev = NULL; + bool multifunction = false; + size_t i; + virDomainDeviceAction actionFlag = VIR_DOMAIN_DEVICE_ACTION_ATTACH; + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -8231,50 +8347,79 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) goto endjob; - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, - caps, driver->xmlopt, - parse_flags); - if (dev == NULL) + if (priv->qemuCaps) + qemuCaps = virObjectRef(priv->qemuCaps); + else if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, vm->def->emulator))) + goto endjob; + + multifunction = isMultifunctionDeviceXML(xml); + + if (VIR_ALLOC(devlist) < 0) goto endjob; + if (multifunction) { + if (virDomainPCIMultifunctionDeviceDefParseNode(xml, vm->def, devlist, + caps, driver->xmlopt, + parse_flags) < 0) + goto endjob; + + if (virDomainPCIMultifunctionDeviceAddressAssign(priv->pciaddrs, devlist) < 0) + goto endjob; + + if (qemuDomainPreparePCIMultifunctionDevices(driver, vm->def, + devlist, qemuCaps) < 0) + goto endjob; + + } else { + dev = virDomainDeviceDefParse(xml, vm->def, + caps, driver->xmlopt, + parse_flags); + if (!dev || !qemuDomainPCIAddressIsSingleFunctionAddr(dev) || + virDomainDeviceDefListAddCopy(devlist, dev, vm->def, caps, + driver->xmlopt) < 0) + goto endjob; + } + + devcopylist = devlist; + if (flags & VIR_DOMAIN_AFFECT_CONFIG && flags & VIR_DOMAIN_AFFECT_LIVE) { /* If we are affecting both CONFIG and LIVE * create a deep copy of device as adding * to CONFIG takes one instance. */ - dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps, driver->xmlopt); - if (!dev_copy) + if (VIR_ALLOC(devcopylist) < 0) goto endjob; + for (i = 0; i < devlist->count; i++) + if (virDomainDeviceDefListAddCopy(devcopylist, devlist->devs[i], + vm->def, caps, driver->xmlopt) < 0) + goto endjob; } - if (priv->qemuCaps) - qemuCaps = virObjectRef(priv->qemuCaps); - else if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, vm->def->emulator))) - goto cleanup; - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { /* Make a copy for updated domain. */ vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt); if (!vmdef) goto endjob; - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) - goto endjob; + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vmdef, devlist->devs[i], actionFlag) < 0) + goto endjob; - if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, dev, - dom->conn)) < 0) - goto endjob; + for (i = 0; i < devlist->count; i++) + if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, + devlist->devs[i], dom->conn)) < 0) + goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (virDomainDefCompatibleDevice(vm->def, dev_copy, - VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) - goto endjob; + for (i = 0; i < devcopylist->count; i++) + if (virDomainDefCompatibleDevice(vm->def, devcopylist->devs[i], actionFlag) < 0) + goto endjob; - if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, dom)) < 0) - goto endjob; + for (i = 0; i < devcopylist->count; i++) + if ((ret = qemuDomainAttachDeviceLive(vm, devcopylist->devs[i], dom)) < 0) + goto endjob; /* * update domain status forcibly because the domain status may be * changed even if we failed to attach the device. For example, @@ -8301,9 +8446,9 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, cleanup: virObjectUnref(qemuCaps); virDomainDefFree(vmdef); - if (dev != dev_copy) - virDomainDeviceDefFree(dev_copy); - virDomainDeviceDefFree(dev); + if (devlist != devcopylist) + virDomainDeviceDefListDispose(devcopylist); + virDomainDeviceDefListDispose(devlist); virDomainObjEndAPI(&vm); virObjectUnref(caps); virObjectUnref(cfg); @@ -8325,13 +8470,17 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; virDomainDefPtr vmdef = NULL; - virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; + virDomainDeviceDefPtr dev = NULL; bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0; int ret = -1; virQEMUCapsPtr qemuCaps = NULL; + virDomainDeviceDefListPtr devlist = NULL, devcopylist = NULL; qemuDomainObjPrivatePtr priv; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + bool multifunction = false; + size_t i; + virDomainDeviceAction actionFlag = VIR_DOMAIN_DEVICE_ACTION_UPDATE; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -8355,12 +8504,25 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, - caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE); - if (dev == NULL) + multifunction = isMultifunctionDeviceXML(xml); + + if (VIR_ALLOC(devlist) < 0) goto endjob; + if (multifunction) { + if (virDomainPCIMultifunctionDeviceDefParseNode(xml, vm->def, devlist, + caps, driver->xmlopt, VIR_DOMAIN_DEF_PARSE_INACTIVE) < 0) + goto endjob; + } else { + dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, + VIR_DOMAIN_DEF_PARSE_INACTIVE); + if (!dev || virDomainDeviceDefListAddCopy(devlist, dev, vm->def, caps, + driver->xmlopt) < 0) + goto endjob; + } + + devcopylist = devlist; + if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) goto endjob; @@ -8370,9 +8532,12 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, * create a deep copy of device as adding * to CONFIG takes one instance. */ - dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps, driver->xmlopt); - if (!dev_copy) + if (VIR_ALLOC(devcopylist) < 0) goto endjob; + for (i = 0; i < devlist->count; i++) + if (virDomainDeviceDefListAddCopy(devcopylist, dev, vm->def, + caps, driver->xmlopt) < 0) + goto endjob; } if (priv->qemuCaps) @@ -8385,22 +8550,26 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt); if (!vmdef) goto endjob; + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vmdef, devlist->devs[i], actionFlag) < 0) + goto endjob; - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) - goto endjob; - - if ((ret = qemuDomainUpdateDeviceConfig(qemuCaps, vmdef, dev)) < 0) - goto endjob; + for (i = 0; i < devlist->count; i++) + if ((ret = qemuDomainUpdateDeviceConfig(qemuCaps, vmdef, devlist->devs[i])) < 0) + goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (virDomainDefCompatibleDevice(vm->def, dev_copy, - VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) - goto endjob; + for (i = 0; i < devcopylist->count; i++) + if (virDomainDefCompatibleDevice(vm->def, devcopylist->devs[i], + actionFlag) < 0) + goto endjob; - if ((ret = qemuDomainUpdateDeviceLive(dom->conn, vm, dev_copy, dom, force)) < 0) - goto endjob; + for (i = 0; i < devcopylist->count; i++) + if ((ret = qemuDomainUpdateDeviceLive(dom->conn, vm, + devcopylist->devs[i], dom, + force)) < 0) + goto endjob; /* * update domain status forcibly because the domain status may be * changed even if we failed to attach the device. For example, @@ -8427,9 +8596,9 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, cleanup: virObjectUnref(qemuCaps); virDomainDefFree(vmdef); - if (dev != dev_copy) - virDomainDeviceDefFree(dev_copy); - virDomainDeviceDefFree(dev); + if (devlist != devcopylist) + virDomainDeviceDefListDispose(devcopylist); + virDomainDeviceDefListDispose(devlist); virDomainObjEndAPI(&vm); virObjectUnref(caps); virObjectUnref(cfg); @@ -8444,13 +8613,17 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; virDomainDefPtr vmdef = NULL; - virDomainDeviceDefPtr dev = NULL, dev_copy = NULL; + virDomainDeviceDefPtr dev = NULL; + virDomainDeviceDefListPtr devlist = NULL, devcopylist = NULL; int ret = -1; unsigned int parse_flags = 0; virQEMUCapsPtr qemuCaps = NULL; qemuDomainObjPrivatePtr priv; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + bool multifunction = false; + size_t i; + virDomainDeviceAction actionFlag = VIR_DOMAIN_DEVICE_ACTION_DETACH; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -8478,21 +8651,38 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, !(flags & VIR_DOMAIN_AFFECT_LIVE)) parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE; - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, - caps, driver->xmlopt, - parse_flags); - if (dev == NULL) + multifunction = isMultifunctionDeviceXML(xml); + + if (VIR_ALLOC(devlist) < 0) goto endjob; + if (multifunction) { + if (virDomainPCIMultifunctionDeviceDefParseNode(xml, vm->def, devlist, + caps, driver->xmlopt, + parse_flags) < 0) + goto endjob; + } else { + dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, parse_flags); + if (!dev || !qemuDomainPCIAddressIsSingleFunctionAddr(dev) || + virDomainDeviceDefListAddCopy(devlist, dev, vm->def, caps, + driver->xmlopt) < 0) + goto endjob; + } + + devcopylist = devlist; + if (flags & VIR_DOMAIN_AFFECT_CONFIG && flags & VIR_DOMAIN_AFFECT_LIVE) { /* If we are affecting both CONFIG and LIVE * create a deep copy of device as adding * to CONFIG takes one instance. */ - dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps, driver->xmlopt); - if (!dev_copy) + if (VIR_ALLOC(devcopylist) < 0) goto endjob; + for (i = 0; i < devcopylist->count; i++) + if (virDomainDeviceDefListAddCopy(devcopylist, dev, vm->def, caps, + driver->xmlopt) < 0) + goto endjob; } if (priv->qemuCaps) @@ -8506,21 +8696,26 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, if (!vmdef) goto endjob; - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) - goto endjob; - - if ((ret = qemuDomainDetachDeviceConfig(vmdef, dev)) < 0) - goto endjob; + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vmdef, devlist->devs[i], actionFlag) < 0) + goto endjob; + for (i = 0; i < devlist->count; i++) + if ((ret = qemuDomainDetachDeviceConfig(vmdef, devlist->devs[i])) < 0) + goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (virDomainDefCompatibleDevice(vm->def, dev_copy, - VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) - goto endjob; + for (i = 0; i < devcopylist->count; i++) + if (virDomainDefCompatibleDevice(vm->def, devcopylist->devs[i], actionFlag) < 0) + goto endjob; - if ((ret = qemuDomainDetachDeviceLive(vm, dev_copy, dom)) < 0) - goto endjob; + for (i = 0; i < devcopylist->count; i++) { + if ((ret = qemuDomainDetachDeviceLive(vm, devcopylist->devs[i], dom)) < 0) + goto endjob; + /* Detaching any one of the functions is sufficient to unplug */ + if (ARCH_IS_X86(vm->def->os.arch)) + break; + } /* * update domain status forcibly because the domain status may be * changed even if we failed to attach the device. For example, @@ -8547,9 +8742,9 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, cleanup: virObjectUnref(qemuCaps); virDomainDefFree(vmdef); - if (dev != dev_copy) - virDomainDeviceDefFree(dev_copy); - virDomainDeviceDefFree(dev); + if (devlist != devcopylist) + virDomainDeviceDefListDispose(devcopylist); + virDomainDeviceDefListDispose(devlist); virDomainObjEndAPI(&vm); virObjectUnref(caps); virObjectUnref(cfg);
participants (1)
-
Shivaprasad G Bhat