[libvirt] [PATCH v3 00/13] PCI Multifunction device hotplug support

V2 was posted here. http://www.redhat.com/archives/libvir-list/2016-May/msg01384.html Changes from V2: -Squashed the original patch 1 with the patch which actually enables multifunction hotplug as suggested. -As suggested, introduced the virDomainDeviceDefParseMany for parsing. -Retained the patch which validates the function address before releasing. I feel this should be retained as originally checks used to happen for slots. -Changed the assign address logic to simply decrementally assign function numbers to the devices in the list. The CONNECT_TYPE flags are yet to be explored. -Split the original patch 8 into multiple simpler patches for easier reading. The original motivation was to help introduce test cases for multifunction hotplug which is not possible for functions static in qemu_driver.c. So, moved the functions to qemu_domain.c and qemu_hotplug.c. Hopefully, the movements helps adding good number of test cases. V1 was posted here. http://www.redhat.com/archives/libvir-list/2016-May/msg01178.html Changes from V1 Fixed couple of issues in address validation and assignment. Added Rollback of the hotplug if anything fails in between. Removed Patch 6 completely as it exposed way too many bugs. Changed the approach a bit by introducing 2 new patches which take care of hostdevice preparation and help reverting. Todo: 1) Hardening the hotplug checks to disallow multifunction cards hotplug as though they are single function cards. 2) Documentation update. 3) Test cases. This is easier now with the helper functions moved out of qemu_driver.c 4) Should the events be delayed till all the functions are hotplugged/unplugged? Something can fail and the revert may not be possible Or the guest may refuse to free the device. If we show events for those which got free, the rest of them may never be freed and also that may not be usable by guest if the function 0 is not part of it. Need to think more on this. Any suggestions ? --- Shivaprasad G Bhat (13): Release address in function granularity than slot Validate address in virDomainPCIAddressReleaseAddr Introduce PCI Multifunction device parser Introduce virDomainPCIMultifunctionDeviceAddressAssign Separate the hostdevice preparation and checks to a new funtion Move the qemu[*]DomainDeviceConfig to qemu_domain.c Move qemuDomain[*]Live functions to qemu_hotplug.c Introduce qemuDomain*DeviceLiveInternal Introduce qemuDomain*DeviceConfigInternal Move the virDomainDefCompatibleDevice checks a level down Move the detach of PCI device to the beginnging of live hotplug Pass virDomainDeviceDefListPtr to hotplug functions Enable PCI Multifunction hotplug/unplug src/conf/domain_addr.c | 153 ++++++- src/conf/domain_addr.h | 4 src/conf/domain_conf.c | 123 +++++- src/conf/domain_conf.h | 19 + src/libvirt_private.syms | 5 src/qemu/qemu_domain.c | 460 +++++++++++++++++++++ src/qemu/qemu_domain.h | 17 + src/qemu/qemu_domain_address.c | 2 src/qemu/qemu_driver.c | 882 ++++++---------------------------------- src/qemu/qemu_hotplug.c | 625 ++++++++++++++++++++++++---- src/qemu/qemu_hotplug.h | 24 + 11 files changed, 1421 insertions(+), 893 deletions(-) -- Signature

The commit 6fe678c is partly reverted. The code is moved around and cant revert staright. Doing this at function level is necessary because some devices may not be removed by guest or may take time and new hotplug shouldn't assume the slot is free just because one of the function was freed by the guest. 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 | 123 ++++++++++++++++++++++++++++++++++++++-------- src/conf/domain_conf.h | 19 +++++++ src/libvirt_private.syms | 3 + 3 files changed, 124 insertions(+), 21 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ed0c471..873a309 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 virDomainDeviceDefListFree(virDomainDeviceDefListPtr list) +{ + size_t i; + + if (!list) + return; + for (i = 0; i < list->count; i++) + virDomainDeviceDefFree(list->devs[i]); + VIR_FREE(list); +} + /** * virDomainKeyWrapCipherDefParseXML: * @@ -12880,25 +12910,16 @@ virDomainMemoryDefParseXML(xmlNodePtr memdevNode, return NULL; } - -virDomainDeviceDefPtr -virDomainDeviceDefParse(const char *xmlStr, - const virDomainDef *def, - virCapsPtr caps, - virDomainXMLOptionPtr xmlopt, - unsigned int flags) +static +virDomainDeviceDefPtr virDomainDeviceDefParseXML(xmlNodePtr node, + const virDomainDef *def, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + xmlXPathContextPtr ctxt, + unsigned int flags) { - xmlDocPtr xml; - xmlNodePtr node; - xmlXPathContextPtr ctxt = NULL; virDomainDeviceDefPtr dev = NULL; char *netprefix; - - if (!(xml = virXMLParseStringCtxt(xmlStr, _("(device_definition)"), &ctxt))) - goto error; - - node = ctxt->node; - if (VIR_ALLOC(dev) < 0) goto error; @@ -13031,14 +13052,33 @@ virDomainDeviceDefParse(const char *xmlStr, if (virDomainDeviceDefPostParse(dev, def, caps, flags, xmlopt) < 0) goto error; - cleanup: + return dev; + error: + return NULL; +} + +virDomainDeviceDefPtr +virDomainDeviceDefParse(const char *xmlStr, + const virDomainDef *def, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + 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, caps, xmlopt, ctxt, flags); + xmlFreeDoc(xml); xmlXPathFreeContext(ctxt); return dev; - - error: - VIR_FREE(dev); - goto cleanup; } @@ -24365,3 +24405,44 @@ virDomainObjGetShortName(virDomainObjPtr vm) return ret; } + + +virDomainDeviceDefListPtr +virDomainDeviceDefParseXMLMany(const char *xml, + const virDomainDef *def, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + 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, caps, xmlopt, ctxt, 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; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b9e696d..ed188f1 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 virDomainDeviceDefListFree(virDomainDeviceDefListPtr list); + + /* Called once after everything else has been parsed, for adjusting * overall domain defaults. */ typedef int (*virDomainDefPostParseCallback)(virDomainDefPtr def, @@ -2717,6 +2731,11 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(const char *xmlStr, virCapsPtr caps, virDomainXMLOptionPtr xmlopt, unsigned int flags); +virDomainDeviceDefListPtr virDomainDeviceDefParseXMLMany(const char *xmlStr, + const virDomainDef *def, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + unsigned int flags); virStorageSourcePtr virDomainDiskDefSourceParse(const char *xmlStr, const virDomainDef *def, virDomainXMLOptionPtr xmlopt, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e4953b7..0e8a887 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -249,7 +249,10 @@ virDomainDeviceAddressIsValid; virDomainDeviceAddressTypeToString; virDomainDeviceDefCopy; virDomainDeviceDefFree; +virDomainDeviceDefListAddCopy; +virDomainDeviceDefListFree; virDomainDeviceDefParse; +virDomainDeviceDefParseXMLMany; virDomainDeviceFindControllerModel; virDomainDeviceGetInfo; virDomainDeviceInfoCopy;

Shivaprasad G Bhat <shivaprasadbhat@gmail.com> writes:
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 | 123 ++++++++++++++++++++++++++++++++++++++-------- src/conf/domain_conf.h | 19 +++++++ src/libvirt_private.syms | 3 + 3 files changed, 124 insertions(+), 21 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ed0c471..873a309 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 virDomainDeviceDefListFree(virDomainDeviceDefListPtr list) +{ + size_t i; + + if (!list) + return; + for (i = 0; i < list->count; i++) + virDomainDeviceDefFree(list->devs[i]); + VIR_FREE(list); +} + /** * virDomainKeyWrapCipherDefParseXML: * @@ -12880,25 +12910,16 @@ virDomainMemoryDefParseXML(xmlNodePtr memdevNode, return NULL; }
- -virDomainDeviceDefPtr -virDomainDeviceDefParse(const char *xmlStr, - const virDomainDef *def, - virCapsPtr caps, - virDomainXMLOptionPtr xmlopt, - unsigned int flags) +static +virDomainDeviceDefPtr virDomainDeviceDefParseXML(xmlNodePtr node, + const virDomainDef *def, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + xmlXPathContextPtr ctxt, + unsigned int flags) { - xmlDocPtr xml; - xmlNodePtr node; - xmlXPathContextPtr ctxt = NULL; virDomainDeviceDefPtr dev = NULL; char *netprefix; - - if (!(xml = virXMLParseStringCtxt(xmlStr, _("(device_definition)"), &ctxt))) - goto error; - - node = ctxt->node; - if (VIR_ALLOC(dev) < 0) goto error;
@@ -13031,14 +13052,33 @@ virDomainDeviceDefParse(const char *xmlStr, if (virDomainDeviceDefPostParse(dev, def, caps, flags, xmlopt) < 0) goto error;
- cleanup: + return dev; + error: + return NULL; +} + +virDomainDeviceDefPtr +virDomainDeviceDefParse(const char *xmlStr, + const virDomainDef *def, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + 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, caps, xmlopt, ctxt, flags); + xmlFreeDoc(xml); xmlXPathFreeContext(ctxt); return dev; - - error: - VIR_FREE(dev);
Missed this ...
- goto cleanup; }
@@ -24365,3 +24405,44 @@ virDomainObjGetShortName(virDomainObjPtr vm)
return ret; } + + +virDomainDeviceDefListPtr +virDomainDeviceDefParseXMLMany(const char *xml, + const virDomainDef *def, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + 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, caps, xmlopt, ctxt, 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; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b9e696d..ed188f1 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 virDomainDeviceDefListFree(virDomainDeviceDefListPtr list); + + /* Called once after everything else has been parsed, for adjusting * overall domain defaults. */ typedef int (*virDomainDefPostParseCallback)(virDomainDefPtr def, @@ -2717,6 +2731,11 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(const char *xmlStr, virCapsPtr caps, virDomainXMLOptionPtr xmlopt, unsigned int flags); +virDomainDeviceDefListPtr virDomainDeviceDefParseXMLMany(const char *xmlStr, + const virDomainDef *def, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt, + unsigned int flags); virStorageSourcePtr virDomainDiskDefSourceParse(const char *xmlStr, const virDomainDef *def, virDomainXMLOptionPtr xmlopt, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e4953b7..0e8a887 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -249,7 +249,10 @@ virDomainDeviceAddressIsValid; virDomainDeviceAddressTypeToString; virDomainDeviceDefCopy; virDomainDeviceDefFree; +virDomainDeviceDefListAddCopy; +virDomainDeviceDefListFree; virDomainDeviceDefParse; +virDomainDeviceDefParseXMLMany; virDomainDeviceFindControllerModel; virDomainDeviceGetInfo; virDomainDeviceInfoCopy;
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

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 | 113 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_addr.h | 4 ++ src/libvirt_private.syms | 1 3 files changed, 118 insertions(+) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 7ea9e4d..c171067 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -455,6 +455,119 @@ virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, } int +virDomainPCIMultifunctionDeviceAddressValidateAssign(virDomainPCIAddressSetPtr addrs, virDomainDeviceDefListPtr devlist) +{ + size_t i; + char *addrStr = NULL; + 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[i]; + 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: + 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: + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Only PCI devices are allowed")); + break; + } + + if (i == 0 && info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + /* User has given an address in xml use that slot + * and bus instead */ + bus = &addrs->buses[info->addr.pci.bus]; + slot = bus->slots[info->addr.pci.slot]; + if (slot & 0xFF) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Slot not free")); + goto error; + } + } + + 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_NONE) { + info->addr.pci = addr; + info->addr.pci.function = devlist->count - i - 1; + info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + if (info->addr.pci.function == 0) + info->addr.pci.multi = VIR_TRISTATE_SWITCH_ON; + } else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + if (info->addr.pci.function != devlist->count - i - 1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("PCI function numbers to be in decreasing order")); + goto error; + } + } + + if (!(addrStr = virDomainPCIAddressAsString(&info->addr.pci))) + goto error; + + if (!virDomainPCIAddressValidate(addrs, &info->addr.pci, + addrStr, flags, true)) + goto error; + + privinfo = info; + } + + return 0; + error: + return -1; +} + + +int virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev) { diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index f3eda89..1e810b0 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 +virDomainPCIMultifunctionDeviceAddressValidateAssign(virDomainPCIAddressSetPtr addrs, + virDomainDeviceDefListPtr devlist); + struct _virDomainCCWAddressSet { virHashTablePtr defined; virDomainDeviceCCWAddress next; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0e8a887..9262859 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -109,6 +109,7 @@ virDomainPCIAddressSetGrow; virDomainPCIAddressSlotInUse; virDomainPCIAddressValidate; virDomainPCIControllerModelToConnectType; +virDomainPCIMultifunctionDeviceAddressValidateAssign; virDomainVirtioSerialAddrAssign; virDomainVirtioSerialAddrAutoAssign; virDomainVirtioSerialAddrIsComplete;

No Functional change. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/qemu/qemu_hotplug.c | 90 +++++++++++++++++++++++++++++------------------ src/qemu/qemu_hotplug.h | 5 +++ 2 files changed, 61 insertions(+), 34 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f40b34d..7ae7bf3 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -705,6 +705,59 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver, goto cleanup; } +int qemuDomainAttachPCIHostDevicePrepare(virQEMUDriverPtr driver, + virDomainDefPtr def, + virDomainHostdevDefPtr hostdev, + virQEMUCapsPtr qemuCaps) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + unsigned int flags = 0; + int ret = -1; + int backend; + + if (!cfg->relaxedACS) + flags |= VIR_HOSTDEV_STRICT_ACS_CHECK; + if (qemuHostdevPreparePCIDevices(driver, def->name, def->uuid, + &hostdev, 1, qemuCaps, flags) < 0) + goto exit; + + /* this could have been changed by qemuHostdevPreparePCIDevices */ + backend = hostdev->source.subsys.u.pci.backend; + + switch ((virDomainHostdevSubsysPCIBackendType) backend) { + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("VFIO PCI device assignment is not " + "supported by this version of qemu")); + goto error; + } + break; + + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM: + break; + + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN: + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("QEMU does not support device assignment mode '%s'"), + virDomainHostdevSubsysPCIBackendTypeToString(backend)); + goto error; + break; + } + + ret = 0; + exit: + virObjectUnref(cfg); + return ret; + error: + qemuHostdevReAttachPCIDevices(driver, def->name, &hostdev, 1); + goto exit; +} + + + int qemuDomainAttachDeviceDiskLive(virConnectPtr conn, @@ -1191,44 +1244,16 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, bool teardowncgroup = false; bool teardownlabel = false; int backend; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - unsigned int flags = 0; if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0) return -1; - if (!cfg->relaxedACS) - flags |= VIR_HOSTDEV_STRICT_ACS_CHECK; - if (qemuHostdevPreparePCIDevices(driver, vm->def->name, vm->def->uuid, - &hostdev, 1, priv->qemuCaps, flags) < 0) - goto cleanup; + 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; - switch ((virDomainHostdevSubsysPCIBackendType) backend) { - case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO: - if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("VFIO PCI device assignment is not " - "supported by this version of qemu")); - goto error; - } - break; - - case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: - case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM: - break; - - case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN: - case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("QEMU does not support device assignment mode '%s'"), - virDomainHostdevSubsysPCIBackendTypeToString(backend)); - goto error; - break; - } - /* Temporarily add the hostdev to the domain definition. This is needed * because qemuDomainAdjustMaxMemLock() requires the hostdev to be already * part of the domain definition, but other functions like @@ -1291,7 +1316,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, VIR_FREE(devstr); VIR_FREE(configfd_name); VIR_FORCE_CLOSE(configfd); - virObjectUnref(cfg); return 0; @@ -1312,8 +1336,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, VIR_FREE(configfd_name); VIR_FORCE_CLOSE(configfd); - cleanup: - virObjectUnref(cfg); return -1; } diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 868b4cf..c127a6d 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -108,6 +108,11 @@ int qemuDomainDetachRNGDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainRNGDefPtr rng); +int qemuDomainAttachPCIHostDevicePrepare(virQEMUDriverPtr driver, + virDomainDefPtr def, + virDomainHostdevDefPtr dev, + virQEMUCapsPtr qemuCaps); + int qemuDomainChrInsert(virDomainDefPtr vmdef, virDomainChrDefPtr chr);

No functional change. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 401 ++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 17 ++ src/qemu/qemu_driver.c | 401 ------------------------------------------------ 3 files changed, 418 insertions(+), 401 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b0eb3b6..e1d3824 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -30,6 +30,7 @@ #include "qemu_parse_command.h" #include "qemu_capabilities.h" #include "qemu_migration.h" +#include "qemu_hotplug.h" #include "viralloc.h" #include "virlog.h" #include "virerror.h" @@ -5352,3 +5353,403 @@ qemuDomainDefValidateDiskLunSource(const virStorageSource *src) return 0; } + +int +qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, + virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev, + virConnectPtr conn) +{ + virDomainDiskDefPtr disk; + virDomainNetDefPtr net; + virDomainHostdevDefPtr hostdev; + virDomainLeaseDefPtr lease; + virDomainControllerDefPtr controller; + virDomainFSDefPtr fs; + + switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + if (virDomainDiskIndexByName(vmdef, disk->dst, true) >= 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("target %s already exists"), disk->dst); + return -1; + } + if (virStorageTranslateDiskSourcePool(conn, disk) < 0) + return -1; + if (qemuCheckDiskConfig(disk) < 0) + return -1; + if (virDomainDiskInsert(vmdef, disk)) + return -1; + /* vmdef has the pointer. Generic codes for vmdef will do all jobs */ + dev->data.disk = NULL; + if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) + if (virDomainDefAddImplicitDevices(vmdef) < 0) + return -1; + if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) + return -1; + break; + + case VIR_DOMAIN_DEVICE_NET: + net = dev->data.net; + if (virDomainNetInsert(vmdef, net)) + return -1; + dev->data.net = NULL; + if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) + return -1; + break; + + case VIR_DOMAIN_DEVICE_HOSTDEV: + hostdev = dev->data.hostdev; + if (virDomainHostdevFind(vmdef, hostdev, NULL) >= 0) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("device is already in the domain configuration")); + return -1; + } + if (virDomainHostdevInsert(vmdef, hostdev)) + return -1; + dev->data.hostdev = NULL; + if (virDomainDefAddImplicitDevices(vmdef) < 0) + return -1; + if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) + return -1; + break; + + case VIR_DOMAIN_DEVICE_LEASE: + lease = dev->data.lease; + if (virDomainLeaseIndex(vmdef, lease) >= 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("Lease %s in lockspace %s already exists"), + lease->key, NULLSTR(lease->lockspace)); + return -1; + } + if (virDomainLeaseInsert(vmdef, lease) < 0) + return -1; + + /* vmdef has the pointer. Generic codes for vmdef will do all jobs */ + dev->data.lease = NULL; + break; + + case VIR_DOMAIN_DEVICE_CONTROLLER: + controller = dev->data.controller; + if (virDomainControllerFind(vmdef, controller->type, + controller->idx) >= 0) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Target already exists")); + return -1; + } + + if (virDomainControllerInsert(vmdef, controller) < 0) + return -1; + dev->data.controller = NULL; + + if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) + return -1; + break; + + case VIR_DOMAIN_DEVICE_CHR: + if (qemuDomainChrInsert(vmdef, dev->data.chr) < 0) + return -1; + dev->data.chr = NULL; + if (virDomainDefAddImplicitDevices(vmdef) < 0) + return -1; + if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) + return -1; + break; + + case VIR_DOMAIN_DEVICE_FS: + fs = dev->data.fs; + if (virDomainFSIndexByName(vmdef, fs->dst) >= 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("Target already exists")); + return -1; + } + + if (virDomainFSInsert(vmdef, fs) < 0) + return -1; + dev->data.fs = NULL; + break; + + case VIR_DOMAIN_DEVICE_RNG: + if (dev->data.rng->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + virDomainDefHasDeviceAddress(vmdef, &dev->data.rng->info)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("a device with the same address already exists ")); + return -1; + } + + if (virDomainRNGInsert(vmdef, dev->data.rng, false) < 0) + return -1; + dev->data.rng = NULL; + + if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) + return -1; + break; + + case VIR_DOMAIN_DEVICE_MEMORY: + if (vmdef->nmems == vmdef->mem.memory_slots) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("no free memory device slot available")); + return -1; + } + + if (vmdef->mem.cur_balloon == virDomainDefGetMemoryActual(vmdef)) + vmdef->mem.cur_balloon += dev->data.memory->size; + + if (virDomainMemoryInsert(vmdef, dev->data.memory) < 0) + return -1; + dev->data.memory = NULL; + break; + + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_GRAPHICS: + 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_REDIRDEV: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_LAST: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("persistent attach of device '%s' is not supported"), + virDomainDeviceTypeToString(dev->type)); + return -1; + } + return 0; +} + +int +qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr disk, det_disk; + virDomainNetDefPtr net; + virDomainHostdevDefPtr hostdev, det_hostdev; + virDomainLeaseDefPtr lease, det_lease; + virDomainControllerDefPtr cont, det_cont; + virDomainChrDefPtr chr; + virDomainFSDefPtr fs; + int idx; + + switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + if (!(det_disk = virDomainDiskRemoveByName(vmdef, disk->dst))) { + virReportError(VIR_ERR_INVALID_ARG, + _("no target device %s"), disk->dst); + return -1; + } + virDomainDiskDefFree(det_disk); + break; + + case VIR_DOMAIN_DEVICE_NET: + net = dev->data.net; + if ((idx = virDomainNetFindIdx(vmdef, net)) < 0) + return -1; + + /* this is guaranteed to succeed */ + virDomainNetDefFree(virDomainNetRemove(vmdef, idx)); + break; + + case VIR_DOMAIN_DEVICE_HOSTDEV: { + hostdev = dev->data.hostdev; + if ((idx = virDomainHostdevFind(vmdef, hostdev, &det_hostdev)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("device not present in domain configuration")); + return -1; + } + virDomainHostdevRemove(vmdef, idx); + virDomainHostdevDefFree(det_hostdev); + break; + } + + case VIR_DOMAIN_DEVICE_LEASE: + lease = dev->data.lease; + if (!(det_lease = virDomainLeaseRemove(vmdef, lease))) { + virReportError(VIR_ERR_INVALID_ARG, + _("Lease %s in lockspace %s does not exist"), + lease->key, NULLSTR(lease->lockspace)); + return -1; + } + virDomainLeaseDefFree(det_lease); + break; + + case VIR_DOMAIN_DEVICE_CONTROLLER: + cont = dev->data.controller; + if ((idx = virDomainControllerFind(vmdef, cont->type, + cont->idx)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("device not present in domain configuration")); + return -1; + } + det_cont = virDomainControllerRemove(vmdef, idx); + virDomainControllerDefFree(det_cont); + + break; + + case VIR_DOMAIN_DEVICE_CHR: + if (!(chr = qemuDomainChrRemove(vmdef, dev->data.chr))) + return -1; + + virDomainChrDefFree(chr); + virDomainChrDefFree(dev->data.chr); + dev->data.chr = NULL; + break; + + case VIR_DOMAIN_DEVICE_FS: + fs = dev->data.fs; + idx = virDomainFSIndexByName(vmdef, fs->dst); + if (idx < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("no matching filesystem device was found")); + return -1; + } + + fs = virDomainFSRemove(vmdef, idx); + virDomainFSDefFree(fs); + break; + + case VIR_DOMAIN_DEVICE_RNG: + if ((idx = virDomainRNGFind(vmdef, dev->data.rng)) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("no matching RNG device was found")); + return -1; + } + + virDomainRNGDefFree(virDomainRNGRemove(vmdef, idx)); + break; + + case VIR_DOMAIN_DEVICE_MEMORY: + if ((idx = virDomainMemoryFindInactiveByDef(vmdef, + dev->data.memory)) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("matching memory device was not found")); + return -1; + } + + virDomainMemoryDefFree(virDomainMemoryRemove(vmdef, idx)); + break; + + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_GRAPHICS: + 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_REDIRDEV: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_LAST: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("persistent detach of device '%s' is not supported"), + virDomainDeviceTypeToString(dev->type)); + return -1; + } + return 0; +} + +int +qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, + virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr orig, disk; + virDomainGraphicsDefPtr newGraphics; + virDomainNetDefPtr net; + int pos; + + switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + if (!(orig = virDomainDiskByName(vmdef, disk->dst, false))) { + virReportError(VIR_ERR_INVALID_ARG, + _("target %s doesn't exist."), disk->dst); + return -1; + } + if (!(orig->device == VIR_DOMAIN_DISK_DEVICE_CDROM) && + !(orig->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("this disk doesn't support update")); + return -1; + } + /* + * Update 'orig' + * We allow updating src/type//driverType/cachemode/ + */ + orig->cachemode = disk->cachemode; + orig->startupPolicy = disk->startupPolicy; + + virStorageSourceFree(orig->src); + orig->src = disk->src; + disk->src = NULL; + break; + + case VIR_DOMAIN_DEVICE_GRAPHICS: + newGraphics = dev->data.graphics; + pos = qemuDomainFindGraphicsIndex(vmdef, newGraphics); + if (pos < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot find existing graphics type '%s' device to modify"), + virDomainGraphicsTypeToString(newGraphics->type)); + return -1; + } + + virDomainGraphicsDefFree(vmdef->graphics[pos]); + + vmdef->graphics[pos] = newGraphics; + dev->data.graphics = NULL; + break; + + case VIR_DOMAIN_DEVICE_NET: + net = dev->data.net; + if ((pos = virDomainNetFindIdx(vmdef, net)) < 0) + return -1; + + virDomainNetDefFree(vmdef->nets[pos]); + + vmdef->nets[pos] = net; + dev->data.net = NULL; + + if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) + return -1; + 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_RNG: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_HOSTDEV: + case VIR_DOMAIN_DEVICE_CONTROLLER: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_MEMORY: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_LAST: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("persistent update of device '%s' is not supported"), + virDomainDeviceTypeToString(dev->type)); + return -1; + } + return 0; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index dd90e67..82e3308 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -669,4 +669,21 @@ int qemuDomainSecretPrepare(virConnectPtr conn, virDomainObjPtr vm) int qemuDomainDefValidateDiskLunSource(const virStorageSource *src) ATTRIBUTE_NONNULL(1); +int +qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, + virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev, + virConnectPtr conn); + +int +qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev); + + +int +qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, + virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev); + + #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 37d970e..832aa17 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7790,407 +7790,6 @@ qemuDomainUpdateDeviceLive(virConnectPtr conn, return ret; } -static int -qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, - virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev, - virConnectPtr conn) -{ - virDomainDiskDefPtr disk; - virDomainNetDefPtr net; - virDomainHostdevDefPtr hostdev; - virDomainLeaseDefPtr lease; - virDomainControllerDefPtr controller; - virDomainFSDefPtr fs; - - switch ((virDomainDeviceType) dev->type) { - case VIR_DOMAIN_DEVICE_DISK: - disk = dev->data.disk; - if (virDomainDiskIndexByName(vmdef, disk->dst, true) >= 0) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("target %s already exists"), disk->dst); - return -1; - } - if (virStorageTranslateDiskSourcePool(conn, disk) < 0) - return -1; - if (qemuCheckDiskConfig(disk) < 0) - return -1; - if (virDomainDiskInsert(vmdef, disk)) - return -1; - /* vmdef has the pointer. Generic codes for vmdef will do all jobs */ - dev->data.disk = NULL; - if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) - if (virDomainDefAddImplicitDevices(vmdef) < 0) - return -1; - if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) - return -1; - break; - - case VIR_DOMAIN_DEVICE_NET: - net = dev->data.net; - if (virDomainNetInsert(vmdef, net)) - return -1; - dev->data.net = NULL; - if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) - return -1; - break; - - case VIR_DOMAIN_DEVICE_HOSTDEV: - hostdev = dev->data.hostdev; - if (virDomainHostdevFind(vmdef, hostdev, NULL) >= 0) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("device is already in the domain configuration")); - return -1; - } - if (virDomainHostdevInsert(vmdef, hostdev)) - return -1; - dev->data.hostdev = NULL; - if (virDomainDefAddImplicitDevices(vmdef) < 0) - return -1; - if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) - return -1; - break; - - case VIR_DOMAIN_DEVICE_LEASE: - lease = dev->data.lease; - if (virDomainLeaseIndex(vmdef, lease) >= 0) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("Lease %s in lockspace %s already exists"), - lease->key, NULLSTR(lease->lockspace)); - return -1; - } - if (virDomainLeaseInsert(vmdef, lease) < 0) - return -1; - - /* vmdef has the pointer. Generic codes for vmdef will do all jobs */ - dev->data.lease = NULL; - break; - - case VIR_DOMAIN_DEVICE_CONTROLLER: - controller = dev->data.controller; - if (virDomainControllerFind(vmdef, controller->type, - controller->idx) >= 0) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Target already exists")); - return -1; - } - - if (virDomainControllerInsert(vmdef, controller) < 0) - return -1; - dev->data.controller = NULL; - - if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) - return -1; - break; - - case VIR_DOMAIN_DEVICE_CHR: - if (qemuDomainChrInsert(vmdef, dev->data.chr) < 0) - return -1; - dev->data.chr = NULL; - if (virDomainDefAddImplicitDevices(vmdef) < 0) - return -1; - if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) - return -1; - break; - - case VIR_DOMAIN_DEVICE_FS: - fs = dev->data.fs; - if (virDomainFSIndexByName(vmdef, fs->dst) >= 0) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("Target already exists")); - return -1; - } - - if (virDomainFSInsert(vmdef, fs) < 0) - return -1; - dev->data.fs = NULL; - break; - - case VIR_DOMAIN_DEVICE_RNG: - if (dev->data.rng->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && - virDomainDefHasDeviceAddress(vmdef, &dev->data.rng->info)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("a device with the same address already exists ")); - return -1; - } - - if (virDomainRNGInsert(vmdef, dev->data.rng, false) < 0) - return -1; - dev->data.rng = NULL; - - if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) - return -1; - break; - - case VIR_DOMAIN_DEVICE_MEMORY: - if (vmdef->nmems == vmdef->mem.memory_slots) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("no free memory device slot available")); - return -1; - } - - if (vmdef->mem.cur_balloon == virDomainDefGetMemoryActual(vmdef)) - vmdef->mem.cur_balloon += dev->data.memory->size; - - if (virDomainMemoryInsert(vmdef, dev->data.memory) < 0) - return -1; - dev->data.memory = NULL; - break; - - case VIR_DOMAIN_DEVICE_INPUT: - case VIR_DOMAIN_DEVICE_SOUND: - case VIR_DOMAIN_DEVICE_VIDEO: - case VIR_DOMAIN_DEVICE_WATCHDOG: - case VIR_DOMAIN_DEVICE_GRAPHICS: - 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_REDIRDEV: - case VIR_DOMAIN_DEVICE_NONE: - case VIR_DOMAIN_DEVICE_TPM: - case VIR_DOMAIN_DEVICE_PANIC: - case VIR_DOMAIN_DEVICE_LAST: - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("persistent attach of device '%s' is not supported"), - virDomainDeviceTypeToString(dev->type)); - return -1; - } - return 0; -} - - -static int -qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev) -{ - virDomainDiskDefPtr disk, det_disk; - virDomainNetDefPtr net; - virDomainHostdevDefPtr hostdev, det_hostdev; - virDomainLeaseDefPtr lease, det_lease; - virDomainControllerDefPtr cont, det_cont; - virDomainChrDefPtr chr; - virDomainFSDefPtr fs; - int idx; - - switch ((virDomainDeviceType) dev->type) { - case VIR_DOMAIN_DEVICE_DISK: - disk = dev->data.disk; - if (!(det_disk = virDomainDiskRemoveByName(vmdef, disk->dst))) { - virReportError(VIR_ERR_INVALID_ARG, - _("no target device %s"), disk->dst); - return -1; - } - virDomainDiskDefFree(det_disk); - break; - - case VIR_DOMAIN_DEVICE_NET: - net = dev->data.net; - if ((idx = virDomainNetFindIdx(vmdef, net)) < 0) - return -1; - - /* this is guaranteed to succeed */ - virDomainNetDefFree(virDomainNetRemove(vmdef, idx)); - break; - - case VIR_DOMAIN_DEVICE_HOSTDEV: { - hostdev = dev->data.hostdev; - if ((idx = virDomainHostdevFind(vmdef, hostdev, &det_hostdev)) < 0) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("device not present in domain configuration")); - return -1; - } - virDomainHostdevRemove(vmdef, idx); - virDomainHostdevDefFree(det_hostdev); - break; - } - - case VIR_DOMAIN_DEVICE_LEASE: - lease = dev->data.lease; - if (!(det_lease = virDomainLeaseRemove(vmdef, lease))) { - virReportError(VIR_ERR_INVALID_ARG, - _("Lease %s in lockspace %s does not exist"), - lease->key, NULLSTR(lease->lockspace)); - return -1; - } - virDomainLeaseDefFree(det_lease); - break; - - case VIR_DOMAIN_DEVICE_CONTROLLER: - cont = dev->data.controller; - if ((idx = virDomainControllerFind(vmdef, cont->type, - cont->idx)) < 0) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("device not present in domain configuration")); - return -1; - } - det_cont = virDomainControllerRemove(vmdef, idx); - virDomainControllerDefFree(det_cont); - - break; - - case VIR_DOMAIN_DEVICE_CHR: - if (!(chr = qemuDomainChrRemove(vmdef, dev->data.chr))) - return -1; - - virDomainChrDefFree(chr); - virDomainChrDefFree(dev->data.chr); - dev->data.chr = NULL; - break; - - case VIR_DOMAIN_DEVICE_FS: - fs = dev->data.fs; - idx = virDomainFSIndexByName(vmdef, fs->dst); - if (idx < 0) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("no matching filesystem device was found")); - return -1; - } - - fs = virDomainFSRemove(vmdef, idx); - virDomainFSDefFree(fs); - break; - - case VIR_DOMAIN_DEVICE_RNG: - if ((idx = virDomainRNGFind(vmdef, dev->data.rng)) < 0) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("no matching RNG device was found")); - return -1; - } - - virDomainRNGDefFree(virDomainRNGRemove(vmdef, idx)); - break; - - case VIR_DOMAIN_DEVICE_MEMORY: - if ((idx = virDomainMemoryFindInactiveByDef(vmdef, - dev->data.memory)) < 0) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("matching memory device was not found")); - return -1; - } - - virDomainMemoryDefFree(virDomainMemoryRemove(vmdef, idx)); - break; - - case VIR_DOMAIN_DEVICE_INPUT: - case VIR_DOMAIN_DEVICE_SOUND: - case VIR_DOMAIN_DEVICE_VIDEO: - case VIR_DOMAIN_DEVICE_WATCHDOG: - case VIR_DOMAIN_DEVICE_GRAPHICS: - 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_REDIRDEV: - case VIR_DOMAIN_DEVICE_NONE: - case VIR_DOMAIN_DEVICE_TPM: - case VIR_DOMAIN_DEVICE_PANIC: - case VIR_DOMAIN_DEVICE_LAST: - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("persistent detach of device '%s' is not supported"), - virDomainDeviceTypeToString(dev->type)); - return -1; - } - return 0; -} - -static int -qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, - virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev) -{ - virDomainDiskDefPtr orig, disk; - virDomainGraphicsDefPtr newGraphics; - virDomainNetDefPtr net; - int pos; - - switch ((virDomainDeviceType) dev->type) { - case VIR_DOMAIN_DEVICE_DISK: - disk = dev->data.disk; - if (!(orig = virDomainDiskByName(vmdef, disk->dst, false))) { - virReportError(VIR_ERR_INVALID_ARG, - _("target %s doesn't exist."), disk->dst); - return -1; - } - if (!(orig->device == VIR_DOMAIN_DISK_DEVICE_CDROM) && - !(orig->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("this disk doesn't support update")); - return -1; - } - /* - * Update 'orig' - * We allow updating src/type//driverType/cachemode/ - */ - orig->cachemode = disk->cachemode; - orig->startupPolicy = disk->startupPolicy; - - virStorageSourceFree(orig->src); - orig->src = disk->src; - disk->src = NULL; - break; - - case VIR_DOMAIN_DEVICE_GRAPHICS: - newGraphics = dev->data.graphics; - pos = qemuDomainFindGraphicsIndex(vmdef, newGraphics); - if (pos < 0) { - virReportError(VIR_ERR_INVALID_ARG, - _("cannot find existing graphics type '%s' device to modify"), - virDomainGraphicsTypeToString(newGraphics->type)); - return -1; - } - - virDomainGraphicsDefFree(vmdef->graphics[pos]); - - vmdef->graphics[pos] = newGraphics; - dev->data.graphics = NULL; - break; - - case VIR_DOMAIN_DEVICE_NET: - net = dev->data.net; - if ((pos = virDomainNetFindIdx(vmdef, net)) < 0) - return -1; - - virDomainNetDefFree(vmdef->nets[pos]); - - vmdef->nets[pos] = net; - dev->data.net = NULL; - - if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) - return -1; - 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_RNG: - case VIR_DOMAIN_DEVICE_SHMEM: - case VIR_DOMAIN_DEVICE_LEASE: - case VIR_DOMAIN_DEVICE_HOSTDEV: - case VIR_DOMAIN_DEVICE_CONTROLLER: - case VIR_DOMAIN_DEVICE_REDIRDEV: - case VIR_DOMAIN_DEVICE_CHR: - case VIR_DOMAIN_DEVICE_MEMORY: - case VIR_DOMAIN_DEVICE_NONE: - case VIR_DOMAIN_DEVICE_TPM: - case VIR_DOMAIN_DEVICE_PANIC: - case VIR_DOMAIN_DEVICE_LAST: - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("persistent update of device '%s' is not supported"), - virDomainDeviceTypeToString(dev->type)); - return -1; - } - return 0; -} - static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags)

No functional change. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 337 ----------------------------------------------- src/qemu/qemu_hotplug.c | 337 +++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.h | 19 +++ 3 files changed, 356 insertions(+), 337 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 832aa17..98e0bfd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7453,343 +7453,6 @@ qemuDomainUndefine(virDomainPtr dom) return qemuDomainUndefineFlags(dom, 0); } -static int -qemuDomainAttachDeviceLive(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, - virDomainPtr dom) -{ - virQEMUDriverPtr driver = dom->conn->privateData; - int ret = -1; - const char *alias = NULL; - - switch ((virDomainDeviceType) dev->type) { - case VIR_DOMAIN_DEVICE_DISK: - qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, NULL); - ret = qemuDomainAttachDeviceDiskLive(dom->conn, driver, vm, dev); - if (!ret) { - alias = dev->data.disk->info.alias; - dev->data.disk = NULL; - } - break; - - case VIR_DOMAIN_DEVICE_CONTROLLER: - ret = qemuDomainAttachControllerDevice(driver, vm, dev->data.controller); - if (!ret) { - alias = dev->data.controller->info.alias; - dev->data.controller = NULL; - } - break; - - case VIR_DOMAIN_DEVICE_LEASE: - ret = qemuDomainAttachLease(driver, vm, - dev->data.lease); - if (ret == 0) - dev->data.lease = NULL; - break; - - case VIR_DOMAIN_DEVICE_NET: - qemuDomainObjCheckNetTaint(driver, vm, dev->data.net, NULL); - ret = qemuDomainAttachNetDevice(driver, vm, dev->data.net); - if (!ret) { - alias = dev->data.net->info.alias; - dev->data.net = NULL; - } - break; - - case VIR_DOMAIN_DEVICE_HOSTDEV: - qemuDomainObjCheckHostdevTaint(driver, vm, dev->data.hostdev, NULL); - ret = qemuDomainAttachHostDevice(dom->conn, driver, vm, - dev->data.hostdev); - if (!ret) { - alias = dev->data.hostdev->info->alias; - dev->data.hostdev = NULL; - } - break; - - case VIR_DOMAIN_DEVICE_REDIRDEV: - ret = qemuDomainAttachRedirdevDevice(driver, vm, - dev->data.redirdev); - if (!ret) { - alias = dev->data.redirdev->info.alias; - dev->data.redirdev = NULL; - } - break; - - case VIR_DOMAIN_DEVICE_CHR: - ret = qemuDomainAttachChrDevice(driver, vm, - dev->data.chr); - if (!ret) { - alias = dev->data.chr->info.alias; - dev->data.chr = NULL; - } - break; - - case VIR_DOMAIN_DEVICE_RNG: - ret = qemuDomainAttachRNGDevice(driver, vm, - dev->data.rng); - if (!ret) { - alias = dev->data.rng->info.alias; - dev->data.rng = NULL; - } - break; - - case VIR_DOMAIN_DEVICE_MEMORY: - /* note that qemuDomainAttachMemory always consumes dev->data.memory - * and dispatches DeviceAdded event on success */ - ret = qemuDomainAttachMemory(driver, vm, - dev->data.memory); - dev->data.memory = NULL; - break; - - case VIR_DOMAIN_DEVICE_NONE: - 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_GRAPHICS: - 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_TPM: - case VIR_DOMAIN_DEVICE_PANIC: - case VIR_DOMAIN_DEVICE_LAST: - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("live attach of device '%s' is not supported"), - virDomainDeviceTypeToString(dev->type)); - break; - } - - if (alias) { - /* queue the event before the alias has a chance to get freed - * if the domain disappears while qemuDomainUpdateDeviceList - * is in monitor */ - virObjectEventPtr event; - event = virDomainEventDeviceAddedNewFromObj(vm, alias); - qemuDomainEventQueue(driver, event); - } - - if (ret == 0) - ret = qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE); - - return ret; -} - -static int -qemuDomainDetachDeviceControllerLive(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainDeviceDefPtr dev) -{ - virDomainControllerDefPtr cont = dev->data.controller; - int ret = -1; - - switch (cont->type) { - case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: - ret = qemuDomainDetachControllerDevice(driver, vm, dev); - break; - default : - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("'%s' controller cannot be hot unplugged."), - virDomainControllerTypeToString(cont->type)); - } - return ret; -} - -static int -qemuDomainDetachDeviceLive(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, - virDomainPtr dom) -{ - virQEMUDriverPtr driver = dom->conn->privateData; - int ret = -1; - - switch ((virDomainDeviceType) dev->type) { - case VIR_DOMAIN_DEVICE_DISK: - ret = qemuDomainDetachDeviceDiskLive(driver, vm, dev); - break; - case VIR_DOMAIN_DEVICE_CONTROLLER: - ret = qemuDomainDetachDeviceControllerLive(driver, vm, dev); - break; - case VIR_DOMAIN_DEVICE_LEASE: - ret = qemuDomainDetachLease(driver, vm, dev->data.lease); - break; - case VIR_DOMAIN_DEVICE_NET: - ret = qemuDomainDetachNetDevice(driver, vm, dev); - break; - case VIR_DOMAIN_DEVICE_HOSTDEV: - ret = qemuDomainDetachHostDevice(driver, vm, dev); - break; - case VIR_DOMAIN_DEVICE_CHR: - ret = qemuDomainDetachChrDevice(driver, vm, dev->data.chr); - break; - case VIR_DOMAIN_DEVICE_RNG: - ret = qemuDomainDetachRNGDevice(driver, vm, dev->data.rng); - break; - case VIR_DOMAIN_DEVICE_MEMORY: - ret = qemuDomainDetachMemoryDevice(driver, vm, dev->data.memory); - 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_GRAPHICS: - 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_REDIRDEV: - case VIR_DOMAIN_DEVICE_NONE: - case VIR_DOMAIN_DEVICE_TPM: - case VIR_DOMAIN_DEVICE_PANIC: - case VIR_DOMAIN_DEVICE_LAST: - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("live detach of device '%s' is not supported"), - virDomainDeviceTypeToString(dev->type)); - break; - } - - if (ret == 0) - ret = qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE); - - return ret; -} - -static int -qemuDomainChangeDiskLive(virConnectPtr conn, - virDomainObjPtr vm, - virDomainDeviceDefPtr dev, - virQEMUDriverPtr driver, - bool force) -{ - virDomainDiskDefPtr disk = dev->data.disk; - virDomainDiskDefPtr orig_disk = NULL; - int startupPolicy; - int snapshot; - int ret = -1; - - if (virStorageTranslateDiskSourcePool(conn, disk) < 0) - goto cleanup; - - if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0) - goto cleanup; - - if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def, - disk->bus, disk->dst))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("No device with bus '%s' and target '%s'"), - virDomainDiskBusTypeToString(disk->bus), - disk->dst); - goto cleanup; - } - - startupPolicy = orig_disk->startupPolicy; - snapshot = orig_disk->snapshot; - - switch ((virDomainDiskDevice) disk->device) { - case VIR_DOMAIN_DISK_DEVICE_CDROM: - case VIR_DOMAIN_DISK_DEVICE_FLOPPY: - if (!qemuDomainDiskChangeSupported(disk, orig_disk)) - goto cleanup; - - orig_disk->startupPolicy = dev->data.disk->startupPolicy; - orig_disk->snapshot = dev->data.disk->snapshot; - - if (qemuDomainDiskSourceDiffers(disk, orig_disk)) { - /* Add the new disk src into shared disk hash table */ - if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0) - goto cleanup; - - if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk, - dev->data.disk->src, - force) < 0) { - ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk, - vm->def->name)); - goto rollback; - } - - dev->data.disk->src = NULL; - } - break; - - case VIR_DOMAIN_DISK_DEVICE_DISK: - case VIR_DOMAIN_DISK_DEVICE_LUN: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk bus '%s' cannot be updated."), - virDomainDiskBusTypeToString(disk->bus)); - goto cleanup; - break; - - case VIR_DOMAIN_DISK_DEVICE_LAST: - /* nada */ - break; - } - - ret = 0; - cleanup: - return ret; - - rollback: - orig_disk->snapshot = snapshot; - orig_disk->startupPolicy = startupPolicy; - goto cleanup; -} - -static int -qemuDomainUpdateDeviceLive(virConnectPtr conn, - virDomainObjPtr vm, - virDomainDeviceDefPtr dev, - virDomainPtr dom, - bool force) -{ - virQEMUDriverPtr driver = dom->conn->privateData; - int ret = -1; - - switch ((virDomainDeviceType) dev->type) { - case VIR_DOMAIN_DEVICE_DISK: - qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, NULL); - ret = qemuDomainChangeDiskLive(conn, vm, dev, driver, force); - break; - case VIR_DOMAIN_DEVICE_GRAPHICS: - ret = qemuDomainChangeGraphics(driver, vm, dev->data.graphics); - break; - case VIR_DOMAIN_DEVICE_NET: - ret = qemuDomainChangeNet(driver, vm, dev); - 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_RNG: - case VIR_DOMAIN_DEVICE_SHMEM: - case VIR_DOMAIN_DEVICE_LEASE: - case VIR_DOMAIN_DEVICE_HOSTDEV: - case VIR_DOMAIN_DEVICE_CONTROLLER: - case VIR_DOMAIN_DEVICE_REDIRDEV: - case VIR_DOMAIN_DEVICE_MEMORY: - case VIR_DOMAIN_DEVICE_CHR: - case VIR_DOMAIN_DEVICE_NONE: - case VIR_DOMAIN_DEVICE_TPM: - case VIR_DOMAIN_DEVICE_PANIC: - case VIR_DOMAIN_DEVICE_LAST: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("live update of device '%s' is not supported"), - virDomainDeviceTypeToString(dev->type)); - break; - } - - return ret; -} - static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 7ae7bf3..c191919 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4255,3 +4255,340 @@ qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver, qemuDomainResetDeviceRemoval(vm); return ret; } + +int +qemuDomainAttachDeviceLive(virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virDomainPtr dom) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + int ret = -1; + const char *alias = NULL; + + switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, NULL); + ret = qemuDomainAttachDeviceDiskLive(dom->conn, driver, vm, dev); + if (!ret) { + alias = dev->data.disk->info.alias; + dev->data.disk = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_CONTROLLER: + ret = qemuDomainAttachControllerDevice(driver, vm, dev->data.controller); + if (!ret) { + alias = dev->data.controller->info.alias; + dev->data.controller = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_LEASE: + ret = qemuDomainAttachLease(driver, vm, + dev->data.lease); + if (ret == 0) + dev->data.lease = NULL; + break; + + case VIR_DOMAIN_DEVICE_NET: + qemuDomainObjCheckNetTaint(driver, vm, dev->data.net, NULL); + ret = qemuDomainAttachNetDevice(driver, vm, dev->data.net); + if (!ret) { + alias = dev->data.net->info.alias; + dev->data.net = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_HOSTDEV: + qemuDomainObjCheckHostdevTaint(driver, vm, dev->data.hostdev, NULL); + ret = qemuDomainAttachHostDevice(dom->conn, driver, vm, + dev->data.hostdev); + if (!ret) { + alias = dev->data.hostdev->info->alias; + dev->data.hostdev = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_REDIRDEV: + ret = qemuDomainAttachRedirdevDevice(driver, vm, + dev->data.redirdev); + if (!ret) { + alias = dev->data.redirdev->info.alias; + dev->data.redirdev = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_CHR: + ret = qemuDomainAttachChrDevice(driver, vm, + dev->data.chr); + if (!ret) { + alias = dev->data.chr->info.alias; + dev->data.chr = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_RNG: + ret = qemuDomainAttachRNGDevice(driver, vm, + dev->data.rng); + if (!ret) { + alias = dev->data.rng->info.alias; + dev->data.rng = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_MEMORY: + /* note that qemuDomainAttachMemory always consumes dev->data.memory + * and dispatches DeviceAdded event on success */ + ret = qemuDomainAttachMemory(driver, vm, + dev->data.memory); + dev->data.memory = NULL; + break; + + case VIR_DOMAIN_DEVICE_NONE: + 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_GRAPHICS: + 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_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_LAST: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("live attach of device '%s' is not supported"), + virDomainDeviceTypeToString(dev->type)); + break; + } + + if (alias) { + /* queue the event before the alias has a chance to get freed + * if the domain disappears while qemuDomainUpdateDeviceList + * is in monitor */ + virObjectEventPtr event; + event = virDomainEventDeviceAddedNewFromObj(vm, alias); + qemuDomainEventQueue(driver, event); + } + + if (ret == 0) + ret = qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE); + + return ret; +} + +static int +qemuDomainDetachDeviceControllerLive(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev) +{ + virDomainControllerDefPtr cont = dev->data.controller; + int ret = -1; + + switch (cont->type) { + case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: + ret = qemuDomainDetachControllerDevice(driver, vm, dev); + break; + default : + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("'%s' controller cannot be hot unplugged."), + virDomainControllerTypeToString(cont->type)); + } + return ret; +} + +int +qemuDomainDetachDeviceLive(virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virDomainPtr dom) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + int ret = -1; + + switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + ret = qemuDomainDetachDeviceDiskLive(driver, vm, dev); + break; + case VIR_DOMAIN_DEVICE_CONTROLLER: + ret = qemuDomainDetachDeviceControllerLive(driver, vm, dev); + break; + case VIR_DOMAIN_DEVICE_LEASE: + ret = qemuDomainDetachLease(driver, vm, dev->data.lease); + break; + case VIR_DOMAIN_DEVICE_NET: + ret = qemuDomainDetachNetDevice(driver, vm, dev); + break; + case VIR_DOMAIN_DEVICE_HOSTDEV: + ret = qemuDomainDetachHostDevice(driver, vm, dev); + break; + case VIR_DOMAIN_DEVICE_CHR: + ret = qemuDomainDetachChrDevice(driver, vm, dev->data.chr); + break; + case VIR_DOMAIN_DEVICE_RNG: + ret = qemuDomainDetachRNGDevice(driver, vm, dev->data.rng); + break; + case VIR_DOMAIN_DEVICE_MEMORY: + ret = qemuDomainDetachMemoryDevice(driver, vm, dev->data.memory); + 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_GRAPHICS: + 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_REDIRDEV: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_LAST: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("live detach of device '%s' is not supported"), + virDomainDeviceTypeToString(dev->type)); + break; + } + + if (ret == 0) + ret = qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE); + + return ret; +} + +static int +qemuDomainChangeDiskLive(virConnectPtr conn, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virQEMUDriverPtr driver, + bool force) +{ + virDomainDiskDefPtr disk = dev->data.disk; + virDomainDiskDefPtr orig_disk = NULL; + int startupPolicy; + int snapshot; + int ret = -1; + + if (virStorageTranslateDiskSourcePool(conn, disk) < 0) + goto cleanup; + + if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0) + goto cleanup; + + if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def, + disk->bus, disk->dst))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("No device with bus '%s' and target '%s'"), + virDomainDiskBusTypeToString(disk->bus), + disk->dst); + goto cleanup; + } + + startupPolicy = orig_disk->startupPolicy; + snapshot = orig_disk->snapshot; + + switch ((virDomainDiskDevice) disk->device) { + case VIR_DOMAIN_DISK_DEVICE_CDROM: + case VIR_DOMAIN_DISK_DEVICE_FLOPPY: + if (!qemuDomainDiskChangeSupported(disk, orig_disk)) + goto cleanup; + + orig_disk->startupPolicy = dev->data.disk->startupPolicy; + orig_disk->snapshot = dev->data.disk->snapshot; + + if (qemuDomainDiskSourceDiffers(disk, orig_disk)) { + /* Add the new disk src into shared disk hash table */ + if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0) + goto cleanup; + + if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk, + dev->data.disk->src, + force) < 0) { + ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk, + vm->def->name)); + goto rollback; + } + + dev->data.disk->src = NULL; + } + break; + + case VIR_DOMAIN_DISK_DEVICE_DISK: + case VIR_DOMAIN_DISK_DEVICE_LUN: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk bus '%s' cannot be updated."), + virDomainDiskBusTypeToString(disk->bus)); + goto cleanup; + break; + + case VIR_DOMAIN_DISK_DEVICE_LAST: + /* nada */ + break; + } + + ret = 0; + cleanup: + return ret; + + rollback: + orig_disk->snapshot = snapshot; + orig_disk->startupPolicy = startupPolicy; + goto cleanup; +} + +int +qemuDomainUpdateDeviceLive(virConnectPtr conn, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virDomainPtr dom, + bool force) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + int ret = -1; + + switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, NULL); + ret = qemuDomainChangeDiskLive(conn, vm, dev, driver, force); + break; + case VIR_DOMAIN_DEVICE_GRAPHICS: + ret = qemuDomainChangeGraphics(driver, vm, dev->data.graphics); + break; + case VIR_DOMAIN_DEVICE_NET: + ret = qemuDomainChangeNet(driver, vm, dev); + 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_RNG: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_HOSTDEV: + case VIR_DOMAIN_DEVICE_CONTROLLER: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_MEMORY: + case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_LAST: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("live update of device '%s' is not supported"), + virDomainDeviceTypeToString(dev->type)); + break; + } + + return ret; +} diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index c127a6d..3fedd25 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -128,4 +128,23 @@ bool qemuDomainSignalDeviceRemoval(virDomainObjPtr vm, const char *devAlias, qemuDomainUnpluggingDeviceStatus status); +int +qemuDomainAttachDeviceLive(virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virDomainPtr dom); + +int +qemuDomainUpdateDeviceLive(virConnectPtr conn, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virDomainPtr dom, + bool force); + +int +qemuDomainDetachDeviceLive(virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virDomainPtr dom); + + + #endif /* __QEMU_HOTPLUG_H__ */

This helps moving some checks into the top function before actually going ahead with the operation. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/qemu/qemu_hotplug.c | 47 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index c191919..96caef3 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4256,8 +4256,9 @@ qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver, return ret; } -int -qemuDomainAttachDeviceLive(virDomainObjPtr vm, + +static int +qemuDomainAttachDeviceLiveInternal(virDomainObjPtr vm, virDomainDeviceDefPtr dev, virDomainPtr dom) { @@ -4380,6 +4381,14 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, return ret; } +int +qemuDomainAttachDeviceLive(virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virDomainPtr dom) +{ + return qemuDomainAttachDeviceLiveInternal(vm, dev, dom); +} + static int qemuDomainDetachDeviceControllerLive(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -4400,10 +4409,10 @@ qemuDomainDetachDeviceControllerLive(virQEMUDriverPtr driver, return ret; } -int -qemuDomainDetachDeviceLive(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, - virDomainPtr dom) +static int +qemuDomainDetachDeviceLiveInternal(virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virDomainPtr dom) { virQEMUDriverPtr driver = dom->conn->privateData; int ret = -1; @@ -4462,6 +4471,16 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, return ret; } + +int +qemuDomainDetachDeviceLive(virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virDomainPtr dom) +{ + return qemuDomainDetachDeviceLiveInternal(vm, dev, dom); +} + + static int qemuDomainChangeDiskLive(virConnectPtr conn, virDomainObjPtr vm, @@ -4542,8 +4561,9 @@ qemuDomainChangeDiskLive(virConnectPtr conn, goto cleanup; } -int -qemuDomainUpdateDeviceLive(virConnectPtr conn, + +static int +qemuDomainUpdateDeviceLiveInternal(virConnectPtr conn, virDomainObjPtr vm, virDomainDeviceDefPtr dev, virDomainPtr dom, @@ -4592,3 +4612,14 @@ qemuDomainUpdateDeviceLive(virConnectPtr conn, return ret; } + +int +qemuDomainUpdateDeviceLive(virConnectPtr conn, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virDomainPtr dom, + bool force) +{ + return qemuDomainUpdateDeviceLiveInternal(conn, vm, dev, + dom, force); +}

This helps moving some checks into the top function before actually going ahead with the operation. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 45 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e1d3824..bf91db3 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5354,11 +5354,11 @@ qemuDomainDefValidateDiskLunSource(const virStorageSource *src) return 0; } -int -qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, - virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev, - virConnectPtr conn) +static int +qemuDomainAttachDeviceConfigInternal(virQEMUCapsPtr qemuCaps, + virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev, + virConnectPtr conn) { virDomainDiskDefPtr disk; virDomainNetDefPtr net; @@ -5524,9 +5524,20 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, return 0; } + int -qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev) +qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, + virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev, + virConnectPtr conn) +{ + return qemuDomainAttachDeviceConfigInternal(qemuCaps, vmdef, dev, conn); +} + + +static int +qemuDomainDetachDeviceConfigInternal(virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev) { virDomainDiskDefPtr disk, det_disk; virDomainNetDefPtr net; @@ -5660,10 +5671,18 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, } int -qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, - virDomainDefPtr vmdef, +qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) { + return qemuDomainDetachDeviceConfigInternal(vmdef, dev); +} + + +static int +qemuDomainUpdateDeviceConfigInternal(virQEMUCapsPtr qemuCaps, + virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev) +{ virDomainDiskDefPtr orig, disk; virDomainGraphicsDefPtr newGraphics; virDomainNetDefPtr net; @@ -5753,3 +5772,11 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, } return 0; } + +int +qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, + virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev) +{ + return qemuDomainUpdateDeviceConfigInternal(qemuCaps, vmdef, dev); +}

The checks need to be performed per device and its better to do them a level down in stack as we prepare for multifunction hotplug. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 12 ++++++++++++ src/qemu/qemu_driver.c | 24 ------------------------ src/qemu/qemu_hotplug.c | 12 ++++++++++++ 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index bf91db3..da5f97d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5531,6 +5531,10 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, virDomainDeviceDefPtr dev, virConnectPtr conn) { + if (virDomainDefCompatibleDevice(vmdef, dev, + VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) + return -1; + return qemuDomainAttachDeviceConfigInternal(qemuCaps, vmdef, dev, conn); } @@ -5674,6 +5678,10 @@ int qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) { + if (virDomainDefCompatibleDevice(vmdef, dev, + VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) + return -1; + return qemuDomainDetachDeviceConfigInternal(vmdef, dev); } @@ -5778,5 +5786,9 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) { + if (virDomainDefCompatibleDevice(vmdef, dev, + VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) + return -1; + return qemuDomainUpdateDeviceConfigInternal(qemuCaps, vmdef, dev); } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 98e0bfd..9484576 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7521,20 +7521,12 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (!vmdef) goto endjob; - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) - goto endjob; - if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, dev, 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; - if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, dom)) < 0) goto endjob; /* @@ -7648,19 +7640,11 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, if (!vmdef) goto endjob; - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) - goto endjob; - if ((ret = qemuDomainUpdateDeviceConfig(qemuCaps, vmdef, dev)) < 0) goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (virDomainDefCompatibleDevice(vm->def, dev_copy, - VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) - goto endjob; - if ((ret = qemuDomainUpdateDeviceLive(dom->conn, vm, dev_copy, dom, force)) < 0) goto endjob; /* @@ -7768,19 +7752,11 @@ 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; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (virDomainDefCompatibleDevice(vm->def, dev_copy, - VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) - goto endjob; - if ((ret = qemuDomainDetachDeviceLive(vm, dev_copy, dom)) < 0) goto endjob; /* diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 96caef3..9a546e2 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4386,6 +4386,10 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev, virDomainPtr dom) { + if (virDomainDefCompatibleDevice(vm->def, dev, + VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) + return -1; + return qemuDomainAttachDeviceLiveInternal(vm, dev, dom); } @@ -4477,6 +4481,10 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev, virDomainPtr dom) { + if (virDomainDefCompatibleDevice(vm->def, dev, + VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) + return -1; + return qemuDomainDetachDeviceLiveInternal(vm, dev, dom); } @@ -4620,6 +4628,10 @@ qemuDomainUpdateDeviceLive(virConnectPtr conn, virDomainPtr dom, bool force) { + if (virDomainDefCompatibleDevice(vm->def, dev, + VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) + return -1; + return qemuDomainUpdateDeviceLiveInternal(conn, vm, dev, dom, force); }

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 funtions first before attempting to hotplug any. We need not 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> --- src/qemu/qemu_hotplug.c | 43 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9a546e2..6821ed5 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -899,6 +899,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, actualType = virDomainNetGetActualType(net); if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + virDomainHostdevDefPtr hostdev = virDomainNetGetActualHostdev(net); /* This is really a "smart hostdev", so it should be attached * as a hostdev (the hostdev code will reach over into the * netdev-specific code as appropriate), then also added to @@ -907,8 +908,14 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, * qemuDomainAttachHostDevice uses a connection to resolve * a SCSI hostdev secret, which is not this case, so pass NULL. */ - ret = qemuDomainAttachHostDevice(NULL, driver, vm, - virDomainNetGetActualHostdev(net)); + if (qemuDomainAttachPCIHostDevicePrepare(driver, vm->def, + hostdev, priv->qemuCaps) < 0) + goto cleanup; + + ret = qemuDomainAttachHostDevice(NULL, driver, vm, hostdev); + if (!ret) + qemuHostdevReAttachPCIDevices(driver, vm->def->name, &hostdev, 1); + goto cleanup; } @@ -1248,10 +1255,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; - backend = hostdev->source.subsys.u.pci.backend; /* Temporarily add the hostdev to the domain definition. This is needed @@ -1330,8 +1333,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, if (releaseaddr) qemuDomainReleaseDeviceAddress(vm, hostdev->info, NULL); - qemuHostdevReAttachPCIDevices(driver, vm->def->name, &hostdev, 1); - VIR_FREE(devstr); VIR_FREE(configfd_name); VIR_FORCE_CLOSE(configfd); @@ -4386,11 +4387,35 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev, virDomainPtr dom) { + virQEMUDriverPtr driver = dom->conn->privateData; + virQEMUCapsPtr qemuCaps = NULL; + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (priv->qemuCaps) + qemuCaps = virObjectRef(priv->qemuCaps); + else if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, vm->def->emulator))) + return -1; + if (virDomainDefCompatibleDevice(vm->def, dev, VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) return -1; - return qemuDomainAttachDeviceLiveInternal(vm, dev, dom); + if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && + dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && + qemuDomainAttachPCIHostDevicePrepare(driver, vm->def, dev->data.hostdev, qemuCaps) < 0) + return -1; + + if (qemuDomainAttachDeviceLiveInternal(vm, dev, dom) < 0) + goto undoprepare; + + return 0; + + undoprepare: + if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && + dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) + qemuHostdevReAttachPCIDevices(driver, vm->def->name, &dev->data.hostdev, 1); + + return -1; } static int

Shivaprasad G Bhat <shivaprasadbhat@gmail.com> writes:
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 funtions first before attempting to hotplug any.
We need not 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> --- src/qemu/qemu_hotplug.c | 43 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9a546e2..6821ed5 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -899,6 +899,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, actualType = virDomainNetGetActualType(net);
if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + virDomainHostdevDefPtr hostdev = virDomainNetGetActualHostdev(net); /* This is really a "smart hostdev", so it should be attached * as a hostdev (the hostdev code will reach over into the * netdev-specific code as appropriate), then also added to @@ -907,8 +908,14 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, * qemuDomainAttachHostDevice uses a connection to resolve * a SCSI hostdev secret, which is not this case, so pass NULL. */ - ret = qemuDomainAttachHostDevice(NULL, driver, vm, - virDomainNetGetActualHostdev(net)); + if (qemuDomainAttachPCIHostDevicePrepare(driver, vm->def, + hostdev, priv->qemuCaps) < 0) + goto cleanup; + + ret = qemuDomainAttachHostDevice(NULL, driver, vm, hostdev); + if (!ret) + qemuHostdevReAttachPCIDevices(driver, vm->def->name, &hostdev, 1); + goto cleanup; }
@@ -1248,10 +1255,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; - backend = hostdev->source.subsys.u.pci.backend;
/* Temporarily add the hostdev to the domain definition. This is needed @@ -1330,8 +1333,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, if (releaseaddr) qemuDomainReleaseDeviceAddress(vm, hostdev->info, NULL);
- qemuHostdevReAttachPCIDevices(driver, vm->def->name, &hostdev, 1); - VIR_FREE(devstr); VIR_FREE(configfd_name); VIR_FORCE_CLOSE(configfd); @@ -4386,11 +4387,35 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev, virDomainPtr dom) { + virQEMUDriverPtr driver = dom->conn->privateData; + virQEMUCapsPtr qemuCaps = NULL; + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (priv->qemuCaps) + qemuCaps = virObjectRef(priv->qemuCaps);
Unref / free the qemuCaps
+ else if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, vm->def->emulator))) + return -1; + if (virDomainDefCompatibleDevice(vm->def, dev, VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) return -1;
- return qemuDomainAttachDeviceLiveInternal(vm, dev, dom); + if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && + dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && + qemuDomainAttachPCIHostDevicePrepare(driver, vm->def, dev->data.hostdev, qemuCaps) < 0) + return -1; + + if (qemuDomainAttachDeviceLiveInternal(vm, dev, dom) < 0) + goto undoprepare; + + return 0; + + undoprepare: + if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && + dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) + qemuHostdevReAttachPCIDevices(driver, vm->def->name, &dev->data.hostdev, 1); + + return -1; }
static int
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

This actually does all the things as though there were more than one device in list. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 50 +++++++++++++++------- src/qemu/qemu_domain.h | 6 +-- src/qemu/qemu_driver.c | 82 +++++++++++++++++++++++------------- src/qemu/qemu_hotplug.c | 108 ++++++++++++++++++++++++++++++++++++----------- src/qemu/qemu_hotplug.h | 6 +-- 5 files changed, 176 insertions(+), 76 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index da5f97d..9f9ad3a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5528,14 +5528,21 @@ qemuDomainAttachDeviceConfigInternal(virQEMUCapsPtr qemuCaps, int qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr devlist, virConnectPtr conn) { - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) - return -1; + size_t i; + + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vmdef, devlist->devs[i], + VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) + return -1; - return qemuDomainAttachDeviceConfigInternal(qemuCaps, vmdef, dev, conn); + for (i = 0; i < devlist->count; i++) + if (qemuDomainAttachDeviceConfigInternal(qemuCaps, vmdef, devlist->devs[i], conn) < 0) + return -1; + + return 0; } @@ -5676,13 +5683,20 @@ qemuDomainDetachDeviceConfigInternal(virDomainDefPtr vmdef, int qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev) + virDomainDeviceDefListPtr devlist) { - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) - return -1; + size_t i; + + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vmdef, devlist->devs[i], + VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) + return -1; - return qemuDomainDetachDeviceConfigInternal(vmdef, dev); + for (i = 0; i < devlist->count; i++) + if (qemuDomainDetachDeviceConfigInternal(vmdef, devlist->devs[i]) < 0) + return -1; + + return 0; } @@ -5784,11 +5798,17 @@ qemuDomainUpdateDeviceConfigInternal(virQEMUCapsPtr qemuCaps, int qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev) + virDomainDeviceDefListPtr devlist) { - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) - return -1; + size_t i; + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vmdef, devlist->devs[i], + VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) + return -1; + + for (i = 0; i < devlist->count; i++) + if (qemuDomainUpdateDeviceConfigInternal(qemuCaps, vmdef, devlist->devs[i]) < 0) + return -1; - return qemuDomainUpdateDeviceConfigInternal(qemuCaps, vmdef, dev); + return 0; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 82e3308..acb3c4c 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -672,18 +672,18 @@ int qemuDomainDefValidateDiskLunSource(const virStorageSource *src) int qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr dev, virConnectPtr conn); int qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev); + virDomainDeviceDefListPtr dev); int qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev); + virDomainDeviceDefListPtr dev); #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9484576..e970ad6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7460,7 +7460,8 @@ 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; + virDomainDeviceDefPtr dev = NULL; + virDomainDeviceDefListPtr devlist = NULL, devcopylist = NULL; int ret = -1; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_ABI_UPDATE; @@ -7493,20 +7494,27 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) goto endjob; - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, + dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, parse_flags); - if (dev == NULL) + if (!dev || VIR_ALLOC(devlist) < 0) goto endjob; + if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 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; + if (virDomainDeviceDefListAddCopy(devcopylist, devlist->devs[0], + vm->def, caps, driver->xmlopt) < 0) goto endjob; } @@ -7521,13 +7529,13 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (!vmdef) goto endjob; - if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, dev, + if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, devlist, dom->conn)) < 0) goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, dom)) < 0) + if ((ret = qemuDomainAttachDeviceLive(vm, devcopylist, dom)) < 0) goto endjob; /* * update domain status forcibly because the domain status may be @@ -7555,9 +7563,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) + virDomainDeviceDefListFree(devcopylist); + virDomainDeviceDefListFree(devlist); virDomainObjEndAPI(&vm); virObjectUnref(caps); virObjectUnref(cfg); @@ -7579,7 +7587,8 @@ 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; + virDomainDeviceDefListPtr devlist = NULL, devcopylist = NULL; bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0; int ret = -1; virQEMUCapsPtr qemuCaps = NULL; @@ -7609,12 +7618,17 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, + dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, VIR_DOMAIN_DEF_PARSE_INACTIVE); - if (dev == NULL) + if (!dev || VIR_ALLOC(devlist) < 0) + goto endjob; + + if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) goto endjob; + devcopylist = devlist; + if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) goto endjob; @@ -7624,8 +7638,10 @@ 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; + if (virDomainDeviceDefListAddCopy(devcopylist, devlist->devs[0], + vm->def, caps, driver->xmlopt) < 0) goto endjob; } @@ -7640,12 +7656,12 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, if (!vmdef) goto endjob; - if ((ret = qemuDomainUpdateDeviceConfig(qemuCaps, vmdef, dev)) < 0) + if ((ret = qemuDomainUpdateDeviceConfig(qemuCaps, vmdef, devlist)) < 0) goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if ((ret = qemuDomainUpdateDeviceLive(dom->conn, vm, dev_copy, dom, force)) < 0) + if ((ret = qemuDomainUpdateDeviceLive(dom->conn, vm, devcopylist, dom, force)) < 0) goto endjob; /* * update domain status forcibly because the domain status may be @@ -7673,9 +7689,9 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, cleanup: virObjectUnref(qemuCaps); virDomainDefFree(vmdef); - if (dev != dev_copy) - virDomainDeviceDefFree(dev_copy); - virDomainDeviceDefFree(dev); + if (devlist != devcopylist) + virDomainDeviceDefListFree(devcopylist); + virDomainDeviceDefListFree(devlist); virDomainObjEndAPI(&vm); virObjectUnref(caps); virObjectUnref(cfg); @@ -7690,7 +7706,8 @@ 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; @@ -7724,20 +7741,27 @@ 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, + dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, parse_flags); - if (dev == NULL) + if (!dev || VIR_ALLOC(devlist) < 0) goto endjob; + if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 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; + if (virDomainDeviceDefListAddCopy(devcopylist, devlist->devs[0], + vm->def, caps, driver->xmlopt) < 0) goto endjob; } @@ -7752,12 +7776,12 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, if (!vmdef) goto endjob; - if ((ret = qemuDomainDetachDeviceConfig(vmdef, dev)) < 0) + if ((ret = qemuDomainDetachDeviceConfig(vmdef, devlist)) < 0) goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if ((ret = qemuDomainDetachDeviceLive(vm, dev_copy, dom)) < 0) + if ((ret = qemuDomainDetachDeviceLive(vm, devcopylist, dom)) < 0) goto endjob; /* * update domain status forcibly because the domain status may be @@ -7785,8 +7809,8 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, cleanup: virObjectUnref(qemuCaps); virDomainDefFree(vmdef); - if (dev != dev_copy) - virDomainDeviceDefFree(dev_copy); + if (devlist != devcopylist) + virDomainDeviceDefListFree(devcopylist); virDomainDeviceDefFree(dev); virDomainObjEndAPI(&vm); virObjectUnref(caps); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6821ed5..adee545 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4384,38 +4384,81 @@ qemuDomainAttachDeviceLiveInternal(virDomainObjPtr vm, int qemuDomainAttachDeviceLive(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr devlist, virDomainPtr dom) { + size_t i, j, d; + virDomainDeviceDefListPtr rollbacklist = NULL; virQEMUDriverPtr driver = dom->conn->privateData; + bool pciHostdevs = false; virQEMUCapsPtr qemuCaps = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; + virCapsPtr caps = NULL; + int ret = -1; + + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + return -1; if (priv->qemuCaps) qemuCaps = virObjectRef(priv->qemuCaps); else if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, vm->def->emulator))) return -1; - if (virDomainDefCompatibleDevice(vm->def, dev, - VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) - return -1; + if (devlist->devs[0]->type == VIR_DOMAIN_DEVICE_HOSTDEV && + devlist->devs[0]->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) + pciHostdevs = true; - if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && - dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && - qemuDomainAttachPCIHostDevicePrepare(driver, vm->def, dev->data.hostdev, qemuCaps) < 0) - return -1; + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vm->def, devlist->devs[i], + VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) + return -1; + + for (d = 0; d < devlist->count; d++) + if (pciHostdevs && + qemuDomainAttachPCIHostDevicePrepare(driver, vm->def, + devlist->devs[d]->data.hostdev, qemuCaps) < 0) + goto undoprepare; - if (qemuDomainAttachDeviceLiveInternal(vm, dev, dom) < 0) + if (VIR_ALLOC(rollbacklist) < 0) goto undoprepare; - return 0; + for (i = 0; i < devlist->count; i++) { + if (virDomainDeviceDefListAddCopy(rollbacklist, devlist->devs[i], + vm->def, caps, driver->xmlopt) < 0) + goto rollback_livehotplug; + if ((ret = qemuDomainAttachDeviceLiveInternal(vm, devlist->devs[i], dom)) < 0) + goto rollback_livehotplug; + } - undoprepare: - if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && - dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) - qemuHostdevReAttachPCIDevices(driver, vm->def->name, &dev->data.hostdev, 1); + ret = 0; + exit: + virDomainDeviceDefListFree(rollbacklist); + return ret; + rollback_livehotplug: + /* The last hotplug on list failed. So "count-1". + * Rollback takes care of cleaning up of any preparations done + * by the respective devices. So, not necessary to do + * any cleanup explicitly here. + */ + VIR_DELETE_ELEMENT(rollbacklist->devs, rollbacklist->count, rollbacklist->count); - return -1; + if ((qemuDomainDetachDeviceLive(vm, rollbacklist, dom)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Couldnt revert hotplug")); + ret = -1; + goto exit; + } + + undoprepare: + if (pciHostdevs) { + /* Devices from o to rollbacklist->count are reattached as part of + * device_deleted event rest of the devices should be bound back to host + * here. 'd' is the last device we detached */ + for (j = rollbacklist ? rollbacklist->count : 0; j < d; j++) + qemuHostdevReAttachPCIDevices(driver, vm->def->name, + &devlist->devs[j]->data.hostdev, 1); + } + ret = -1; + goto exit; } static int @@ -4503,14 +4546,21 @@ qemuDomainDetachDeviceLiveInternal(virDomainObjPtr vm, int qemuDomainDetachDeviceLive(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr devlist, virDomainPtr dom) { - if (virDomainDefCompatibleDevice(vm->def, dev, - VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) - return -1; + size_t i; + + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vm->def, devlist->devs[i], + VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) + return -1; + + for (i = 0; i < devlist->count; i++) + if (qemuDomainDetachDeviceLiveInternal(vm, devlist->devs[i], dom) < 0) + return -1; - return qemuDomainDetachDeviceLiveInternal(vm, dev, dom); + return 0; } @@ -4649,14 +4699,20 @@ qemuDomainUpdateDeviceLiveInternal(virConnectPtr conn, int qemuDomainUpdateDeviceLive(virConnectPtr conn, virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr devlist, virDomainPtr dom, bool force) { - if (virDomainDefCompatibleDevice(vm->def, dev, - VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) - return -1; + size_t i; + + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vm->def, devlist->devs[i], + VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) + return -1; - return qemuDomainUpdateDeviceLiveInternal(conn, vm, dev, - dom, force); + for (i = 0; i < devlist->count; i++) + if (qemuDomainUpdateDeviceLiveInternal(conn, vm, devlist->devs[i], dom, force) < 0) + return -1; + + return 0; } diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 3fedd25..47a5bb4 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -130,19 +130,19 @@ bool qemuDomainSignalDeviceRemoval(virDomainObjPtr vm, int qemuDomainAttachDeviceLive(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr devlist, virDomainPtr dom); int qemuDomainUpdateDeviceLive(virConnectPtr conn, virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr dev, virDomainPtr dom, bool force); int qemuDomainDetachDeviceLive(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr dev, virDomainPtr dom);

Shivaprasad G Bhat <shivaprasadbhat@gmail.com> writes:
This actually does all the things as though there were more than one device in list.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 50 +++++++++++++++------- src/qemu/qemu_domain.h | 6 +-- src/qemu/qemu_driver.c | 82 +++++++++++++++++++++++------------- src/qemu/qemu_hotplug.c | 108 ++++++++++++++++++++++++++++++++++++----------- src/qemu/qemu_hotplug.h | 6 +-- 5 files changed, 176 insertions(+), 76 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index da5f97d..9f9ad3a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5528,14 +5528,21 @@ qemuDomainAttachDeviceConfigInternal(virQEMUCapsPtr qemuCaps, int qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr devlist, virConnectPtr conn) { - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) - return -1; + size_t i; + + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vmdef, devlist->devs[i], + VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) + return -1;
- return qemuDomainAttachDeviceConfigInternal(qemuCaps, vmdef, dev, conn); + for (i = 0; i < devlist->count; i++) + if (qemuDomainAttachDeviceConfigInternal(qemuCaps, vmdef, devlist->devs[i], conn) < 0) + return -1; + + return 0; }
@@ -5676,13 +5683,20 @@ qemuDomainDetachDeviceConfigInternal(virDomainDefPtr vmdef,
int qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev) + virDomainDeviceDefListPtr devlist) { - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) - return -1; + size_t i; + + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vmdef, devlist->devs[i], + VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) + return -1;
- return qemuDomainDetachDeviceConfigInternal(vmdef, dev); + for (i = 0; i < devlist->count; i++) + if (qemuDomainDetachDeviceConfigInternal(vmdef, devlist->devs[i]) < 0) + return -1; + + return 0; }
@@ -5784,11 +5798,17 @@ qemuDomainUpdateDeviceConfigInternal(virQEMUCapsPtr qemuCaps, int qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev) + virDomainDeviceDefListPtr devlist) { - if (virDomainDefCompatibleDevice(vmdef, dev, - VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) - return -1; + size_t i; + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vmdef, devlist->devs[i], + VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) + return -1; + + for (i = 0; i < devlist->count; i++) + if (qemuDomainUpdateDeviceConfigInternal(qemuCaps, vmdef, devlist->devs[i]) < 0) + return -1;
- return qemuDomainUpdateDeviceConfigInternal(qemuCaps, vmdef, dev); + return 0; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 82e3308..acb3c4c 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -672,18 +672,18 @@ int qemuDomainDefValidateDiskLunSource(const virStorageSource *src) int qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr dev, virConnectPtr conn);
int qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev); + virDomainDeviceDefListPtr dev);
int qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, virDomainDefPtr vmdef, - virDomainDeviceDefPtr dev); + virDomainDeviceDefListPtr dev);
#endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9484576..e970ad6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7460,7 +7460,8 @@ 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; + virDomainDeviceDefPtr dev = NULL; + virDomainDeviceDefListPtr devlist = NULL, devcopylist = NULL; int ret = -1; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_ABI_UPDATE; @@ -7493,20 +7494,27 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) goto endjob;
- dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, + dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, parse_flags); - if (dev == NULL) + if (!dev || VIR_ALLOC(devlist) < 0) goto endjob;
+ if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 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)
devcopylist = NULL ?
+ if (VIR_ALLOC(devcopylist) < 0) + goto endjob; + if (virDomainDeviceDefListAddCopy(devcopylist, devlist->devs[0], + vm->def, caps, driver->xmlopt) < 0) goto endjob; }
@@ -7521,13 +7529,13 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (!vmdef) goto endjob;
- if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, dev, + if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, devlist, dom->conn)) < 0) goto endjob; }
if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, dom)) < 0) + if ((ret = qemuDomainAttachDeviceLive(vm, devcopylist, dom)) < 0) goto endjob; /* * update domain status forcibly because the domain status may be @@ -7555,9 +7563,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) + virDomainDeviceDefListFree(devcopylist); + virDomainDeviceDefListFree(devlist); virDomainObjEndAPI(&vm); virObjectUnref(caps); virObjectUnref(cfg); @@ -7579,7 +7587,8 @@ 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; + virDomainDeviceDefListPtr devlist = NULL, devcopylist = NULL; bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0; int ret = -1; virQEMUCapsPtr qemuCaps = NULL; @@ -7609,12 +7618,17 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup;
- dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, + dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, VIR_DOMAIN_DEF_PARSE_INACTIVE); - if (dev == NULL) + if (!dev || VIR_ALLOC(devlist) < 0) + goto endjob; + + if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) goto endjob;
+ devcopylist = devlist; + if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) goto endjob;
@@ -7624,8 +7638,10 @@ 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; + if (virDomainDeviceDefListAddCopy(devcopylist, devlist->devs[0], + vm->def, caps, driver->xmlopt) < 0) goto endjob; }
@@ -7640,12 +7656,12 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, if (!vmdef) goto endjob;
- if ((ret = qemuDomainUpdateDeviceConfig(qemuCaps, vmdef, dev)) < 0) + if ((ret = qemuDomainUpdateDeviceConfig(qemuCaps, vmdef, devlist)) < 0) goto endjob; }
if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if ((ret = qemuDomainUpdateDeviceLive(dom->conn, vm, dev_copy, dom, force)) < 0) + if ((ret = qemuDomainUpdateDeviceLive(dom->conn, vm, devcopylist, dom, force)) < 0) goto endjob; /* * update domain status forcibly because the domain status may be @@ -7673,9 +7689,9 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, cleanup: virObjectUnref(qemuCaps); virDomainDefFree(vmdef); - if (dev != dev_copy) - virDomainDeviceDefFree(dev_copy); - virDomainDeviceDefFree(dev); + if (devlist != devcopylist) + virDomainDeviceDefListFree(devcopylist); + virDomainDeviceDefListFree(devlist); virDomainObjEndAPI(&vm); virObjectUnref(caps); virObjectUnref(cfg); @@ -7690,7 +7706,8 @@ 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; @@ -7724,20 +7741,27 @@ 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, + dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, parse_flags); - if (dev == NULL) + if (!dev || VIR_ALLOC(devlist) < 0) goto endjob;
+ if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 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)
set to null
+ if (VIR_ALLOC(devcopylist) < 0) + goto endjob; + if (virDomainDeviceDefListAddCopy(devcopylist, devlist->devs[0], + vm->def, caps, driver->xmlopt) < 0) goto endjob; }
@@ -7752,12 +7776,12 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, if (!vmdef) goto endjob;
- if ((ret = qemuDomainDetachDeviceConfig(vmdef, dev)) < 0) + if ((ret = qemuDomainDetachDeviceConfig(vmdef, devlist)) < 0) goto endjob; }
if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if ((ret = qemuDomainDetachDeviceLive(vm, dev_copy, dom)) < 0) + if ((ret = qemuDomainDetachDeviceLive(vm, devcopylist, dom)) < 0) goto endjob; /* * update domain status forcibly because the domain status may be @@ -7785,8 +7809,8 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, cleanup: virObjectUnref(qemuCaps); virDomainDefFree(vmdef); - if (dev != dev_copy) - virDomainDeviceDefFree(dev_copy); + if (devlist != devcopylist) + virDomainDeviceDefListFree(devcopylist); virDomainDeviceDefFree(dev);
free devlist ??
virDomainObjEndAPI(&vm); virObjectUnref(caps); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6821ed5..adee545 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4384,38 +4384,81 @@ qemuDomainAttachDeviceLiveInternal(virDomainObjPtr vm,
int qemuDomainAttachDeviceLive(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr devlist, virDomainPtr dom) { + size_t i, j, d; + virDomainDeviceDefListPtr rollbacklist = NULL; virQEMUDriverPtr driver = dom->conn->privateData; + bool pciHostdevs = false; virQEMUCapsPtr qemuCaps = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; + virCapsPtr caps = NULL; + int ret = -1; + + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + return -1;
if (priv->qemuCaps) qemuCaps = virObjectRef(priv->qemuCaps); else if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, vm->def->emulator))) return -1;
- if (virDomainDefCompatibleDevice(vm->def, dev, - VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) - return -1; + if (devlist->devs[0]->type == VIR_DOMAIN_DEVICE_HOSTDEV && + devlist->devs[0]->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) + pciHostdevs = true;
Can go in the above patch where prepare is introduced.
- if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && - dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && - qemuDomainAttachPCIHostDevicePrepare(driver, vm->def, dev->data.hostdev, qemuCaps) < 0) - return -1; + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vm->def, devlist->devs[i], + VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) + return -1; + + for (d = 0; d < devlist->count; d++) + if (pciHostdevs && + qemuDomainAttachPCIHostDevicePrepare(driver, vm->def, + devlist->devs[d]->data.hostdev, qemuCaps) < 0) + goto undoprepare;
- if (qemuDomainAttachDeviceLiveInternal(vm, dev, dom) < 0) + if (VIR_ALLOC(rollbacklist) < 0) goto undoprepare;
- return 0; + for (i = 0; i < devlist->count; i++) { + if (virDomainDeviceDefListAddCopy(rollbacklist, devlist->devs[i], + vm->def, caps, driver->xmlopt) < 0) + goto rollback_livehotplug; + if ((ret = qemuDomainAttachDeviceLiveInternal(vm, devlist->devs[i], dom)) < 0) + goto rollback_livehotplug; + }
- undoprepare: - if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && - dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) - qemuHostdevReAttachPCIDevices(driver, vm->def->name, &dev->data.hostdev, 1); + ret = 0; + exit: + virDomainDeviceDefListFree(rollbacklist);
unref of caps?
+ return ret; + rollback_livehotplug: + /* The last hotplug on list failed. So "count-1". + * Rollback takes care of cleaning up of any preparations done + * by the respective devices. So, not necessary to do + * any cleanup explicitly here. + */ + VIR_DELETE_ELEMENT(rollbacklist->devs, rollbacklist->count, rollbacklist->count);
- return -1; + if ((qemuDomainDetachDeviceLive(vm, rollbacklist, dom)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Couldnt revert hotplug")); + ret = -1; + goto exit; + } + + undoprepare: + if (pciHostdevs) { + /* Devices from o to rollbacklist->count are reattached as part of + * device_deleted event rest of the devices should be bound back to host + * here. 'd' is the last device we detached */ + for (j = rollbacklist ? rollbacklist->count : 0; j < d; j++)
********* -1
+ qemuHostdevReAttachPCIDevices(driver, vm->def->name, + &devlist->devs[j]->data.hostdev, 1); + } + ret = -1; + goto exit;
unref of caps?
}
static int @@ -4503,14 +4546,21 @@ qemuDomainDetachDeviceLiveInternal(virDomainObjPtr vm,
int qemuDomainDetachDeviceLive(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr devlist, virDomainPtr dom) { - if (virDomainDefCompatibleDevice(vm->def, dev, - VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) - return -1; + size_t i; + + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vm->def, devlist->devs[i], + VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) + return -1; + + for (i = 0; i < devlist->count; i++) + if (qemuDomainDetachDeviceLiveInternal(vm, devlist->devs[i], dom) < 0) + return -1;
- return qemuDomainDetachDeviceLiveInternal(vm, dev, dom); + return 0; }
@@ -4649,14 +4699,20 @@ qemuDomainUpdateDeviceLiveInternal(virConnectPtr conn, int qemuDomainUpdateDeviceLive(virConnectPtr conn, virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr devlist, virDomainPtr dom, bool force) { - if (virDomainDefCompatibleDevice(vm->def, dev, - VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) - return -1; + size_t i; + + for (i = 0; i < devlist->count; i++) + if (virDomainDefCompatibleDevice(vm->def, devlist->devs[i], + VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) + return -1;
- return qemuDomainUpdateDeviceLiveInternal(conn, vm, dev, - dom, force); + for (i = 0; i < devlist->count; i++) + if (qemuDomainUpdateDeviceLiveInternal(conn, vm, devlist->devs[i], dom, force) < 0) + return -1; + + return 0; } diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 3fedd25..47a5bb4 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -130,19 +130,19 @@ bool qemuDomainSignalDeviceRemoval(virDomainObjPtr vm,
int qemuDomainAttachDeviceLive(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr devlist, virDomainPtr dom);
int qemuDomainUpdateDeviceLive(virConnectPtr conn, virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr dev, virDomainPtr dom, bool force);
int qemuDomainDetachDeviceLive(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, + virDomainDeviceDefListPtr dev, virDomainPtr dom);
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

The flow is to parse and create a list of devices and pass onto the hotplug functions. The patch also removes all checks forbidding the multifunction hotplug. Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 166 ++++++++++++++++++++++++++++++++++++++--------- src/qemu/qemu_hotplug.c | 66 +------------------ 2 files changed, 140 insertions(+), 92 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e970ad6..0519a33 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7453,6 +7453,77 @@ qemuDomainUndefine(virDomainPtr dom) return qemuDomainUndefineFlags(dom, 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"); +} + static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) @@ -7469,6 +7540,8 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, qemuDomainObjPrivatePtr priv; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + bool multifunction = false; + size_t i; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -7494,14 +7567,25 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) goto endjob; - dev = virDomainDeviceDefParse(xml, vm->def, - caps, driver->xmlopt, - parse_flags); - if (!dev || VIR_ALLOC(devlist) < 0) - goto endjob; + multifunction = isMultifunctionDeviceXML(xml); - if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) - goto endjob; + if (multifunction) { + if (!(devlist = virDomainDeviceDefParseXMLMany(xml, vm->def, + caps, driver->xmlopt, + parse_flags))) + goto endjob; + if (virDomainPCIMultifunctionDeviceAddressValidateAssign(priv->pciaddrs, devlist) < 0) + goto endjob; + } else { + dev = virDomainDeviceDefParse(xml, vm->def, + caps, driver->xmlopt, + parse_flags); + if (!dev || VIR_ALLOC(devlist) < 0) + goto endjob; + if (!qemuDomainPCIAddressIsSingleFunctionAddr(dev) || + VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) + goto endjob; + } devcopylist = devlist; @@ -7513,15 +7597,16 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, */ if (VIR_ALLOC(devcopylist) < 0) goto endjob; - if (virDomainDeviceDefListAddCopy(devcopylist, devlist->devs[0], - vm->def, caps, driver->xmlopt) < 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; + goto endjob; if (flags & VIR_DOMAIN_AFFECT_CONFIG) { /* Make a copy for updated domain. */ @@ -7529,14 +7614,14 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (!vmdef) goto endjob; - if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, devlist, - dom->conn)) < 0) + if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, devlist, dom->conn)) < 0) goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { if ((ret = qemuDomainAttachDeviceLive(vm, devcopylist, 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, @@ -7595,6 +7680,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, qemuDomainObjPrivatePtr priv; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + bool multifunction = false; + size_t i; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -7618,14 +7705,22 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - dev = virDomainDeviceDefParse(xml, vm->def, - caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE); - if (!dev || VIR_ALLOC(devlist) < 0) - goto endjob; + multifunction = isMultifunctionDeviceXML(xml); - if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) - goto endjob; + if (multifunction) { + if (!(devlist = virDomainDeviceDefParseXMLMany(xml, vm->def, + caps, driver->xmlopt, + VIR_DOMAIN_DEF_PARSE_INACTIVE))) + goto endjob; + } else { + dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, + VIR_DOMAIN_DEF_PARSE_INACTIVE); + if (!dev || VIR_ALLOC(devlist) < 0) + goto endjob; + + if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) + goto endjob; + } devcopylist = devlist; @@ -7640,9 +7735,10 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, */ if (VIR_ALLOC(devcopylist) < 0) goto endjob; - if (virDomainDeviceDefListAddCopy(devcopylist, devlist->devs[0], - vm->def, caps, driver->xmlopt) < 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) @@ -7714,6 +7810,7 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, qemuDomainObjPrivatePtr priv; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + bool multifunction = false; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -7741,14 +7838,21 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, !(flags & VIR_DOMAIN_AFFECT_LIVE)) parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE; - dev = virDomainDeviceDefParse(xml, vm->def, - caps, driver->xmlopt, - parse_flags); - if (!dev || VIR_ALLOC(devlist) < 0) - goto endjob; + multifunction = isMultifunctionDeviceXML(xml); - if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) - goto endjob; + if (multifunction) { + if (!(devlist = virDomainDeviceDefParseXMLMany(xml, vm->def, + caps, driver->xmlopt, + parse_flags))) + goto endjob; + } else { + dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, parse_flags); + if (!dev || VIR_ALLOC(devlist) < 0) + goto endjob; + if (!qemuDomainPCIAddressIsSingleFunctionAddr(dev) || + VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) + goto endjob; + } devcopylist = devlist; @@ -7811,7 +7915,7 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, virDomainDefFree(vmdef); if (devlist != devcopylist) virDomainDeviceDefListFree(devcopylist); - virDomainDeviceDefFree(dev); + virDomainDeviceDefListFree(devlist); virDomainObjEndAPI(&vm); virObjectUnref(caps); virObjectUnref(cfg); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index adee545..35c1071 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2793,35 +2793,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, @@ -3430,13 +3401,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, @@ -3659,14 +3623,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")); @@ -3702,17 +3658,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, @@ -3944,13 +3891,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) { @@ -4556,9 +4496,13 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) return -1; - for (i = 0; i < devlist->count; i++) + for (i = 0; i < devlist->count; i++) { if (qemuDomainDetachDeviceLiveInternal(vm, devlist->devs[i], dom) < 0) return -1; + /* Detaching any one of the functions is sufficient to unplug */ + if (ARCH_IS_X86(vm->def->os.arch)) + break; + } return 0; }

Shivaprasad G Bhat <shivaprasadbhat@gmail.com> writes:
The flow is to parse and create a list of devices and pass onto the hotplug functions.
The patch also removes all checks forbidding the multifunction hotplug.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 166 ++++++++++++++++++++++++++++++++++++++--------- src/qemu/qemu_hotplug.c | 66 +------------------ 2 files changed, 140 insertions(+), 92 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e970ad6..0519a33 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7453,6 +7453,77 @@ qemuDomainUndefine(virDomainPtr dom) return qemuDomainUndefineFlags(dom, 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;
Remove this, not supported for PCI
+ 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)
isPCIMultifunctionDeviceXML
+{ + xmlDocPtr xmlptr; + + if (!(xmlptr = virXMLParse(NULL, xml, _("(device_definition)")))) { + /* We report error anyway later */ + return false; + } + + return STREQ((const char *)(xmlDocGetRootElement(xmlptr))->name, "devices"); +} +
static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) @@ -7469,6 +7540,8 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, qemuDomainObjPrivatePtr priv; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + bool multifunction = false; + size_t i;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -7494,14 +7567,25 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) goto endjob;
- dev = virDomainDeviceDefParse(xml, vm->def, - caps, driver->xmlopt, - parse_flags); - if (!dev || VIR_ALLOC(devlist) < 0) - goto endjob; + multifunction = isMultifunctionDeviceXML(xml);
- if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) - goto endjob; + if (multifunction) { + if (!(devlist = virDomainDeviceDefParseXMLMany(xml, vm->def, + caps, driver->xmlopt, + parse_flags))) + goto endjob; + if (virDomainPCIMultifunctionDeviceAddressValidateAssign(priv->pciaddrs, devlist) < 0) + goto endjob; + } else { + dev = virDomainDeviceDefParse(xml, vm->def, + caps, driver->xmlopt, + parse_flags); + if (!dev || VIR_ALLOC(devlist) < 0) + goto endjob; + if (!qemuDomainPCIAddressIsSingleFunctionAddr(dev) || + VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) + goto endjob; + }
devcopylist = devlist;
@@ -7513,15 +7597,16 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, */ if (VIR_ALLOC(devcopylist) < 0) goto endjob; - if (virDomainDeviceDefListAddCopy(devcopylist, devlist->devs[0], - vm->def, caps, driver->xmlopt) < 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; + goto endjob;
if (flags & VIR_DOMAIN_AFFECT_CONFIG) { /* Make a copy for updated domain. */ @@ -7529,14 +7614,14 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, if (!vmdef) goto endjob;
- if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, devlist, - dom->conn)) < 0) + if ((ret = qemuDomainAttachDeviceConfig(qemuCaps, vmdef, devlist, dom->conn)) < 0) goto endjob; }
if (flags & VIR_DOMAIN_AFFECT_LIVE) { if ((ret = qemuDomainAttachDeviceLive(vm, devcopylist, 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, @@ -7595,6 +7680,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, qemuDomainObjPrivatePtr priv; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + bool multifunction = false; + size_t i;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -7618,14 +7705,22 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup;
- dev = virDomainDeviceDefParse(xml, vm->def, - caps, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE); - if (!dev || VIR_ALLOC(devlist) < 0) - goto endjob; + multifunction = isMultifunctionDeviceXML(xml);
- if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) - goto endjob; + if (multifunction) { + if (!(devlist = virDomainDeviceDefParseXMLMany(xml, vm->def, + caps, driver->xmlopt, + VIR_DOMAIN_DEF_PARSE_INACTIVE))) + goto endjob; + } else { + dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, + VIR_DOMAIN_DEF_PARSE_INACTIVE); + if (!dev || VIR_ALLOC(devlist) < 0) + goto endjob; + + if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) + goto endjob; + }
devcopylist = devlist;
@@ -7640,9 +7735,10 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, */ if (VIR_ALLOC(devcopylist) < 0) goto endjob; - if (virDomainDeviceDefListAddCopy(devcopylist, devlist->devs[0], - vm->def, caps, driver->xmlopt) < 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) @@ -7714,6 +7810,7 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, qemuDomainObjPrivatePtr priv; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + bool multifunction = false;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -7741,14 +7838,21 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, !(flags & VIR_DOMAIN_AFFECT_LIVE)) parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE;
- dev = virDomainDeviceDefParse(xml, vm->def, - caps, driver->xmlopt, - parse_flags); - if (!dev || VIR_ALLOC(devlist) < 0) - goto endjob; + multifunction = isMultifunctionDeviceXML(xml);
- if (VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) - goto endjob; + if (multifunction) { + if (!(devlist = virDomainDeviceDefParseXMLMany(xml, vm->def, + caps, driver->xmlopt, + parse_flags))) + goto endjob; + } else { + dev = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, parse_flags); + if (!dev || VIR_ALLOC(devlist) < 0) + goto endjob; + if (!qemuDomainPCIAddressIsSingleFunctionAddr(dev) || + VIR_APPEND_ELEMENT(devlist->devs, devlist->count, dev) < 0) + goto endjob; + }
devcopylist = devlist;
@@ -7811,7 +7915,7 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, virDomainDefFree(vmdef); if (devlist != devcopylist) virDomainDeviceDefListFree(devcopylist); - virDomainDeviceDefFree(dev); + virDomainDeviceDefListFree(devlist); virDomainObjEndAPI(&vm); virObjectUnref(caps); virObjectUnref(cfg); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index adee545..35c1071 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2793,35 +2793,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, @@ -3430,13 +3401,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, @@ -3659,14 +3623,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")); @@ -3702,17 +3658,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, @@ -3944,13 +3891,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) { @@ -4556,9 +4496,13 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) return -1;
- for (i = 0; i < devlist->count; i++) + for (i = 0; i < devlist->count; i++) { if (qemuDomainDetachDeviceLiveInternal(vm, devlist->devs[i], dom) < 0) return -1; + /* Detaching any one of the functions is sufficient to unplug */ + if (ARCH_IS_X86(vm->def->os.arch)) + break; + }
return 0; }
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
Nikunj A Dadhania
-
Shivaprasad G Bhat