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