[libvirt] [RFC PATCH 28/28] qemu: hotplug: Implement multifunction device unplug

Shivaprasad G Bhat posted 28 patches 6 years, 8 months ago
[libvirt] [RFC PATCH 28/28] qemu: hotplug: Implement multifunction device unplug
Posted by Shivaprasad G Bhat 6 years, 8 months ago
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
---
 src/qemu/qemu_driver.c  |   48 +++++++++++++-------
 src/qemu/qemu_hotplug.c |  111 +++++++++++++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_hotplug.h |    5 ++
 tests/qemuhotplugtest.c |   26 ++++++++---
 4 files changed, 165 insertions(+), 25 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 94f76979e5..0640395b00 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7858,12 +7858,18 @@ qemuDomainDetachDeviceLiveInternal(virDomainObjPtr vm,
 
 static int
 qemuDomainDetachDeviceLive(virDomainObjPtr vm,
-                           virDomainDeviceDefPtr dev,
+                           virDomainDeviceDefListPtr devlist,
                            virQEMUDriverPtr driver)
 {
     int ret = -1;
+    virDomainDeviceDefPtr dev = devlist->devs[0];
+
+    if (devlist->count > 1) {
+        ret = qemuDomainDetachMultifunctionDevice(vm, devlist, driver);
+    } else {
+        ret = qemuDomainDetachDeviceLiveInternal(vm, dev, driver);
+    }
 
-    ret = qemuDomainDetachDeviceLiveInternal(vm, dev, driver);
     if (ret == 0)
         ret = qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE);
 
@@ -8392,17 +8398,24 @@ qemuDomainDetachDeviceConfigInternal(virDomainDefPtr vmdef,
 
 static int
 qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
-                             virDomainDeviceDefPtr dev,
+                             virDomainDeviceDefListPtr devlist,
                              virCapsPtr caps,
                              unsigned int parse_flags,
                              virDomainXMLOptionPtr xmlopt)
 {
-    if (qemuDomainDetachDeviceConfigInternal(vmdef, dev))
-        return -1;
+    size_t i;
+    for (i = 0; i < devlist->count; i++)
+        if (qemuDomainDetachDeviceConfigInternal(vmdef, devlist->devs[i]))
+            return -1;
 
     if (virDomainDefPostParse(vmdef, caps, parse_flags, xmlopt, NULL) < 0)
         return -1;
 
+    /* Dont allow removing the primary function alone for a multifunction
+     * device leading to guest start failure later. */
+    if (devlist->count > 1 && qemuDomainDefValidatePCIHostdevs(vmdef) < 0)
+        return -1;
+
     return 0;
 }
 
@@ -8765,8 +8778,9 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver,
 {
     virCapsPtr caps = NULL;
     virQEMUDriverConfigPtr cfg = NULL;
-    virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
     unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
+    virDomainDeviceDefListPtr devlist = NULL, devcopylist = NULL;
+    virDomainDeviceDefListData data = {.def = vm->def, .xmlopt = driver->xmlopt, .caps = NULL};
     virDomainDefPtr vmdef = NULL;
     int ret = -1;
 
@@ -8775,6 +8789,7 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver,
 
     if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
         goto cleanup;
+    data.caps = caps;
 
     cfg = virQEMUDriverGetConfig(driver);
 
@@ -8782,11 +8797,10 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver,
         !(flags & VIR_DOMAIN_AFFECT_LIVE))
         parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE;
 
-    dev = dev_copy = virDomainDeviceDefParse(xml, vm->def,
-                                             caps, driver->xmlopt,
-                                             parse_flags);
-    if (dev == NULL)
+    devlist = qemuDomainDeviceParseXMLMany(xml, &data, parse_flags);
+    if (!devlist)
         goto cleanup;
+    devcopylist = devlist;
 
     if (flags & VIR_DOMAIN_AFFECT_CONFIG &&
         flags & VIR_DOMAIN_AFFECT_LIVE) {
@@ -8794,8 +8808,8 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver,
          * 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 =  virDomainDeviceDefListCopy(devlist, &data);
+        if (!devcopylist)
             goto cleanup;
     }
 
@@ -8805,14 +8819,14 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver,
         if (!vmdef)
             goto cleanup;
 
-        if ((ret = qemuDomainDetachDeviceConfig(vmdef, dev, caps,
+        if ((ret = qemuDomainDetachDeviceConfig(vmdef, devlist, caps,
                                                 parse_flags,
                                                 driver->xmlopt)) < 0)
             goto cleanup;
     }
 
     if (flags & VIR_DOMAIN_AFFECT_LIVE) {
-        if ((ret = qemuDomainDetachDeviceLive(vm, dev_copy, driver)) < 0)
+        if ((ret = qemuDomainDetachDeviceLive(vm, devcopylist, driver)) < 0)
             goto cleanup;
         /*
          * update domain status forcibly because the domain status may be
@@ -8837,9 +8851,9 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver,
  cleanup:
     virObjectUnref(caps);
     virObjectUnref(cfg);
-    if (dev != dev_copy)
-        virDomainDeviceDefFree(dev_copy);
-    virDomainDeviceDefFree(dev);
+    if (devlist != devcopylist)
+        virDomainDeviceDefListFree(devcopylist);
+    virDomainDeviceDefListFree(devlist);
     virDomainDefFree(vmdef);
     return ret;
 }
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 5f6302eaf9..87acd5cf69 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5210,6 +5210,117 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver,
 
 
 int
+qemuDomainDetachMultifunctionDevice(virDomainObjPtr vm,
+                                    virDomainDeviceDefListPtr devlist,
+                                    virQEMUDriverPtr driver)
+{
+    size_t i;
+    int ret = -1;
+    virBitmapPtr tbdmap = NULL, onlinemap = NULL;
+    int *functions = NULL;
+    virDomainHostdevDefPtr hostdev, detach = NULL;
+    virDomainHostdevSubsysPtr subsys = NULL;
+    int slotaggridx = 0;
+    virDomainHostdevSubsysPCIPtr pcisrc = NULL;
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+
+    if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT))
+        return -1;
+
+#define FOR_EACH_DEV_IN_DEVLIST() \
+    for (i = 0; i < devlist->count; i++) { \
+        hostdev = devlist->devs[i]->data.hostdev; \
+        subsys = &hostdev->source.subsys; \
+        pcisrc = &subsys->u.pci; \
+        virDomainHostdevFind(vm->def, hostdev, &detach);
+
+    FOR_EACH_DEV_IN_DEVLIST()
+        if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("hot unplug is not supported for hostdev mode '%s'"),
+                           virDomainHostdevModeTypeToString(hostdev->mode));
+            goto cleanup;
+        }
+    }
+
+    FOR_EACH_DEV_IN_DEVLIST()
+        if (!detach) {
+            virReportError(VIR_ERR_DEVICE_MISSING,
+                           _("host pci device %.4x:%.2x:%.2x.%.1x not found"),
+                           pcisrc->addr.domain, pcisrc->addr.bus,
+                           pcisrc->addr.slot, pcisrc->addr.function);
+            goto cleanup;
+        }
+    }
+
+    /* Check if the devices belong to same guest slot.*/
+    FOR_EACH_DEV_IN_DEVLIST()
+        /* Pick one aggregateSlotIdx and compare against rest of them */
+        slotaggridx = slotaggridx ? slotaggridx : detach->info->aggregateSlotIdx;
+        if (slotaggridx != detach->info->aggregateSlotIdx) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("host pci device %.4x:%.2x:%.2x.%.1x doesnt not belong to"
+                             " same slot"), pcisrc->addr.domain, pcisrc->addr.bus,
+                           pcisrc->addr.slot, pcisrc->addr.function);
+            goto cleanup;
+        }
+    }
+
+    /* Check if the whole slot is being removed or not */
+    onlinemap = virDomainDefHostdevGetPCIOnlineFunctionMap(vm->def, slotaggridx);
+    FOR_EACH_DEV_IN_DEVLIST()
+        ignore_value(virBitmapClearBit(onlinemap, pcisrc->addr.function));
+    }
+
+    if (!virBitmapIsAllClear(onlinemap)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("hot unplug of partial PCI slot not allowed"));
+        goto cleanup;
+    }
+
+    /* Mark all aliases for removal */
+    memset(&priv->unplug, 0, sizeof(priv->unplug));
+    FOR_EACH_DEV_IN_DEVLIST()
+        if (qemuAssignDeviceHostdevAlias(vm->def, &detach->info->alias, -1) < 0)
+            goto reset;
+
+        qemuDomainMarkDeviceAliasForRemoval(vm, detach->info->alias, false);
+    }
+
+    qemuDomainObjEnterMonitor(driver, vm);
+    FOR_EACH_DEV_IN_DEVLIST()
+        if (qemuMonitorDelDevice(priv->mon, detach->info->alias) < 0) {
+            ignore_value(qemuDomainObjExitMonitor(driver, vm));
+            if (virDomainObjIsActive(vm))
+                virDomainAuditHostdev(vm, detach, "detach", false);
+            goto reset;
+        }
+        if (ARCH_IS_X86(vm->def->os.arch))
+            break; /* deleting any one is enough! */
+    }
+
+    if (qemuDomainObjExitMonitor(driver, vm) < 0)
+        ret = -1;
+
+    if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) {
+        FOR_EACH_DEV_IN_DEVLIST()
+             ret = qemuDomainRemoveHostDevice(driver, vm, detach);
+        }
+    }
+ reset:
+    qemuDomainResetDeviceRemoval(vm);
+ cleanup:
+    VIR_FREE(functions);
+    virBitmapFree(onlinemap);
+    virBitmapFree(tbdmap);
+
+#undef FOR_EACH_DEV_IN_DEVLIST
+
+    return ret;
+}
+
+
+int
 qemuDomainDetachShmemDevice(virQEMUDriverPtr driver,
                             virDomainObjPtr vm,
                             virDomainShmemDefPtr dev)
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 7a6e2dfb60..bc850ecc44 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -164,6 +164,11 @@ qemuDomainAttachMultifunctionDevice(virDomainObjPtr vm,
                                     virQEMUDriverPtr driver);
 
 int
+qemuDomainDetachMultifunctionDevice(virDomainObjPtr vm,
+                                    virDomainDeviceDefListPtr devlist,
+                                    virQEMUDriverPtr driver);
+
+int
 qemuDomainChrInsert(virDomainDefPtr vmdef,
                     virDomainChrDefPtr chr);
 virDomainChrDefPtr
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 39f122e083..61557ce562 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -156,9 +156,14 @@ testQemuHotplugAttach(virDomainObjPtr vm,
 
 static int
 testQemuHotplugDetach(virDomainObjPtr vm,
-                      virDomainDeviceDefPtr dev)
+                      virDomainDeviceDefListPtr devlist)
 {
     int ret = -1;
+    virDomainDeviceDefPtr dev;
+
+    if (devlist->count > 1)
+        return qemuDomainDetachMultifunctionDevice(vm, devlist, &driver);
+    dev = devlist->devs[0];
 
     switch (dev->type) {
     case VIR_DOMAIN_DEVICE_DISK:
@@ -256,7 +261,6 @@ testQemuHotplug(const void *data)
     bool keep = test->keep;
     unsigned int device_parse_flags = 0;
     virDomainObjPtr vm = NULL;
-    virDomainDeviceDefPtr dev = NULL; /*temperory */
     virDomainDeviceDefListPtr devlist = NULL;
     virDomainDeviceDefListData listdata;
     virCapsPtr caps = NULL;
@@ -301,7 +305,6 @@ testQemuHotplug(const void *data)
     devlist = qemuDomainDeviceParseXMLMany(device_xml, &listdata, device_parse_flags);
     if (!devlist)
         goto cleanup;
-    dev = devlist->devs[0]; /* temporary */
 
     /* Now is the best time to feed the spoofed monitor with predefined
      * replies. */
@@ -343,14 +346,14 @@ testQemuHotplug(const void *data)
         break;
 
     case DETACH:
-        ret = testQemuHotplugDetach(vm, dev);
+        ret = testQemuHotplugDetach(vm, devlist);
         if (ret == 0 || fail)
             ret = testQemuHotplugCheckResult(vm, domain_xml,
                                              domain_filename, fail);
         break;
 
     case UPDATE:
-        ret = testQemuHotplugUpdate(vm, dev);
+        ret = testQemuHotplugUpdate(vm, devlist->devs[0]);
     }
 
  cleanup:
@@ -868,16 +871,23 @@ mymain(void)
                    "device_add", QMP_OK);
     DO_TEST_DETACH("pseries-base-live", "hostdev-pci", false, false,
                    "device_del", QMP_DEVICE_DELETED("hostdev0") QMP_OK);
-    DO_TEST_ATTACH("base-live", "multifunction-hostdev-pci", false, false,
+    DO_TEST_ATTACH_EVENT("base-live", "multifunction-hostdev-pci", false, true,
                    "device_add", QMP_OK,
                    "device_add", QMP_OK,
                    "device_add", QMP_OK);
+    DO_TEST_DETACH("base-live", "multifunction-hostdev-pci", false, false,
+                   "device_del", QMP_DEVICE_DELETED("hostdev2")
+                                 QMP_DEVICE_DELETED("hostdev1")
+                                 QMP_DEVICE_DELETED("hostdev0") QMP_OK);
 
     qemuTestSetHostArch(driver.caps, VIR_ARCH_PPC64);
-    DO_TEST_ATTACH("pseries-base-live", "multifunction-hostdev-pci-2", false, false,
-                   "device_add", QMP_OK,
+    DO_TEST_ATTACH_EVENT("pseries-base-live", "multifunction-hostdev-pci-2", false, true,
                    "device_add", QMP_OK,
                    "device_add", QMP_OK);
+    DO_TEST_DETACH("pseries-base-live", "multifunction-hostdev-pci-2", false, false,
+                   "device_del", QMP_OK,
+                   "device_del", QMP_DEVICE_DELETED("hostdev1")
+                                 QMP_DEVICE_DELETED("hostdev0") QMP_OK);
     qemuTestSetHostArch(driver.caps, VIR_ARCH_X86_64);
 
     DO_TEST_ATTACH("base-live", "watchdog", false, true,

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list