[libvirt] [PATCH v3 13/13] Enable PCI Multifunction hotplug/unplug

Shivaprasad G Bhat shivaprasadbhat at gmail.com
Mon May 23 21:05:28 UTC 2016


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 at 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;
 }




More information about the libvir-list mailing list