4:11 p.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 | 66 +++++++++++++++-------
src/qemu/qemu_hotplug.c | 116 +++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_hotplug.h | 5 ++
src/qemu/qemu_validate.c | 2 +-
src/qemu/qemu_validate.h | 1 +
tests/qemuhotplugtest.c | 27 ++++++---
6 files changed, 189 insertions(+), 28 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ce44a1455a..e7119fda8c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -50,6 +50,7 @@
#include "qemu_security.h"
#include "qemu_checkpoint.h"
#include "qemu_backup.h"
+#include "qemu_validate.h"
#include "virerror.h"
#include "virlog.h"
@@ -8569,17 +8570,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 && qemuValidateDomainDefPCIHostdevs(vmdef) <
0)
+ return -1;
+
return 0;
}
@@ -9012,7 +9023,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;
@@ -9026,11 +9039,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) {
@@ -9038,9 +9051,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;
}
@@ -9050,7 +9063,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;
@@ -9059,7 +9072,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)
@@ -9086,9 +9105,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;
}
@@ -9106,6 +9125,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 |
@@ -9121,16 +9141,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;
}
@@ -9159,6 +9185,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 a6c520ec1b..9d6671f4f6 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -6086,6 +6086,122 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
}
+int
+qemuDomainDetachMultifunctionDevice(virDomainObjPtr vm,
+ virDomainDeviceDefListPtr devlist,
+ virQEMUDriverPtr driver)
+{
+ size_t i;
+ int ret = -1;
+ g_autoptr(virBitmap) onlinemap = 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));
+ return -1;
+ }
+ }
+
+ 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);
+ return -1;
+ }
+ }
+
+ /* 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);
+ return -1;
+ }
+ }
+
+ /* 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"));
+ return -1;
+ }
+
+ /* 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);
+
+#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 b62528a6d9..6e74473936 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/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 956e3daf7d..b913ce2c5b 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -802,7 +802,7 @@ static int
qemuValidateDomainDefPCIMultifunctionHostdev(qemuDomainPCIHostdevData
}
-static int qemuValidateDomainDefPCIHostdevs(const virDomainDef *def)
+int qemuValidateDomainDefPCIHostdevs(const virDomainDef *def)
{
qemuDomainPCIHostdevdata hostdevdata = {def, NULL, NULL};
diff --git a/src/qemu/qemu_validate.h b/src/qemu/qemu_validate.h
index acf7d26ce0..c6782066eb 100644
--- a/src/qemu/qemu_validate.h
+++ b/src/qemu/qemu_validate.h
@@ -33,3 +33,4 @@ int qemuValidateDomainDeviceDefDisk(const virDomainDiskDef *disk,
int qemuValidateDomainDeviceDef(const virDomainDeviceDef *dev,
const virDomainDef *def,
void *opaque);
+int qemuValidateDomainDefPCIHostdevs(const virDomainDef *def);
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 32d186ef73..175f3ffed8 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -158,10 +158,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:
@@ -251,7 +256,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;
@@ -298,8 +302,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,
@@ -340,14 +342,14 @@ 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]);
}
virObjectLock(priv->mon);
@@ -852,16 +854,25 @@ 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,
+ 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.26.2