On Mon, Aug 16, 2021 at 07:19:45PM +0800, Luke Yue wrote:
Introduce testDomainChgDevice for further development (just like what we
did for IOThread). And introduce testDomainDetachDeviceLiveAndConfig for
detaching devices.
Signed-off-by: Luke Yue <lukedyue@xxxxxxxxx>
---
src/test/test_driver.c | 270 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 270 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 00cc13511a..2ebdbaa604 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -9452,6 +9452,275 @@ testDomainGetMessages(virDomainPtr dom,
return rv;
}
+static int
+testDomainDetachDeviceLiveAndConfig(virDomainDef *vmdef,
+ virDomainDeviceDef *dev)
+{
+ virDomainDiskDef *disk;
+ virDomainDiskDef *det_disk;
+ virDomainNetDef *net;
+ virDomainSoundDef *sound;
+ virDomainHostdevDef *hostdev;
+ virDomainHostdevDef *det_hostdev;
+ virDomainLeaseDef *lease;
+ virDomainLeaseDef *det_lease;
+ virDomainControllerDef *cont;
+ virDomainControllerDef *det_cont;
+ virDomainFSDef *fs;
+ virDomainMemoryDef *mem;
+ int idx;
+
+ switch (dev->type) {
+ case VIR_DOMAIN_DEVICE_DISK:
+ disk = dev->data.disk;
+ if (!(det_disk = virDomainDiskRemoveByName(vmdef, disk->dst))) {
+ virReportError(VIR_ERR_DEVICE_MISSING,
+ _("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_SOUND:
+ sound = dev->data.sound;
+ if ((idx = virDomainSoundDefFind(vmdef, sound)) < 0) {
+ virReportError(VIR_ERR_DEVICE_MISSING, "%s",
+ _("device not present in domain configuration"));
+ return -1;
+ }
+ virDomainSoundDefFree(virDomainSoundDefRemove(vmdef, idx));
+ break;
+
+ case VIR_DOMAIN_DEVICE_HOSTDEV: {
+ hostdev = dev->data.hostdev;
+ if ((idx = virDomainHostdevFind(vmdef, hostdev, &det_hostdev)) < 0) {
+ virReportError(VIR_ERR_DEVICE_MISSING, "%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_DEVICE_MISSING,
+ _("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_DEVICE_MISSING, "%s",
+ _("device not present in domain configuration"));
+ return -1;
+ }
+ det_cont = virDomainControllerRemove(vmdef, idx);
+ virDomainControllerDefFree(det_cont);
+
+ break;
+
+ case VIR_DOMAIN_DEVICE_FS:
+ fs = dev->data.fs;
+ idx = virDomainFSIndexByName(vmdef, fs->dst);
+ if (idx < 0) {
+ virReportError(VIR_ERR_DEVICE_MISSING, "%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_DEVICE_MISSING, "%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_DEVICE_MISSING, "%s",
+ _("matching memory device was not found"));
+ return -1;
+ }
+ mem = virDomainMemoryRemove(vmdef, idx);
+ vmdef->mem.cur_balloon -= mem->size;
+ virDomainMemoryDefFree(mem);
+ break;
+
+ case VIR_DOMAIN_DEVICE_REDIRDEV:
+ if ((idx = virDomainRedirdevDefFind(vmdef,
+ dev->data.redirdev)) < 0) {
+ virReportError(VIR_ERR_DEVICE_MISSING, "%s",
+ _("no matching redirdev was not found"));
+ return -1;
+ }
+
+ virDomainRedirdevDefFree(virDomainRedirdevDefRemove(vmdef, idx));
+ break;
+
+ case VIR_DOMAIN_DEVICE_SHMEM:
+ if ((idx = virDomainShmemDefFind(vmdef, dev->data.shmem)) < 0) {
+ virReportError(VIR_ERR_DEVICE_MISSING, "%s",
+ _("matching shmem device was not found"));
+ return -1;
+ }
+
+ virDomainShmemDefFree(virDomainShmemDefRemove(vmdef, idx));
+ break;
+
+
+ case VIR_DOMAIN_DEVICE_WATCHDOG:
+ if (!vmdef->watchdog) {
+ virReportError(VIR_ERR_DEVICE_MISSING, "%s",
+ _("domain has no watchdog"));
+ return -1;
+ }
+ virDomainWatchdogDefFree(vmdef->watchdog);
+ vmdef->watchdog = NULL;
+ break;
+
+ case VIR_DOMAIN_DEVICE_INPUT:
+ if ((idx = virDomainInputDefFind(vmdef, dev->data.input)) < 0) {
+ virReportError(VIR_ERR_DEVICE_MISSING, "%s",
+ _("matching input device not found"));
+ return -1;
+ }
+ virDomainInputDefFree(vmdef->inputs[idx]);
+ VIR_DELETE_ELEMENT(vmdef->inputs, idx, vmdef->ninputs);
+ break;
+
+ case VIR_DOMAIN_DEVICE_VSOCK:
+ if (!vmdef->vsock ||
+ !virDomainVsockDefEquals(dev->data.vsock, vmdef->vsock)) {
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("matching vsock device not found"));
+ return -1;
+ }
+ virDomainVsockDefFree(vmdef->vsock);
+ vmdef->vsock = NULL;
+ break;
+
There is lot of duplication here. These are already used in multiple places and de-duplicating would help us tremendously, even more than in the previous case with virDomainGetMessages. The lxc, libxl, qemu and possibly other drivers would benefit from many of these, some of them could then easily add more functionality and it would be even more tested once we start testing these properly. It is not going to be a trivial task to properlyd ecide where to split this and how, but I believe you can figure out a nice, clean way. But do not hesitate to let know if you need help with that.
+ case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_VIDEO: + 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_NONE: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_IOMMU: + case VIR_DOMAIN_DEVICE_AUDIO:
What's good about the test driver is that we can support attach and detach for all the devices. It would be nice to also have a way to test something that is not supported, but there is no need to do it with all of the above. It makes for example for an IOMMU as that is something I cannot imagine being hot(un)plugged. For --config anything is possible because it is just the XML that is being changed.
+ case VIR_DOMAIN_DEVICE_LAST:
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+ _("persistent detach of device '%s' is not supported"),
Not really persistent when it is called for both live and config.
+ virDomainDeviceTypeToString(dev->type));
+ return -1;
+ }
+
+ return 0;
+}
+
+static int
+testDomainChgDevice(virDomainPtr dom,
+ virDomainDeviceAction action,
+ const char *xml,
+ const char *alias,
+ unsigned int flags)
+{
+ testDriver *driver = dom->conn->privateData;
+ virDomainObj *vm = NULL;
+ virDomainDef *def;
+ virDomainDeviceDef *dev = NULL;
+ unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
+ int ret = -1;
+
+ virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+ if (!(vm = testDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
+ goto cleanup;
+
+ if (!(def = virDomainObjGetOneDef(vm, flags)))
+ goto cleanup;
+
+ if (action == VIR_DOMAIN_DEVICE_ACTION_DETACH)
+ parse_flags |= VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
+
+ if (xml) {
+ if (!(dev = virDomainDeviceDefParse(xml, def, driver->xmlopt,
+ driver->caps, parse_flags)))
+ goto cleanup;
+ } else if (alias) {
+ dev = g_new0(virDomainDeviceDef, 1);
+ if (virDomainDefFindDevice(def, alias, dev, true) < 0)
+ goto cleanup;
+ }
+
+ if (dev == NULL)
+ goto cleanup;
+
+ switch (action) {
+ case VIR_DOMAIN_DEVICE_ACTION_ATTACH:
+ break;
So Attach will always succeed, but nothing will show up in the XML.
+ + case VIR_DOMAIN_DEVICE_ACTION_DETACH: + if (testDomainDetachDeviceLiveAndConfig(def, dev) < 0) + goto cleanup; + break; + + case VIR_DOMAIN_DEVICE_ACTION_UPDATE: + break;
Same here
+ }
+
+ ret = 0;
+
+ cleanup:
+ if (xml) {
+ virDomainDeviceDefFree(dev);
+ } else {
+ g_free(dev);
+ }
+ virDomainObjEndAPI(&vm);
+ return ret;
+}
+
+static int
+testDomainDetachDeviceFlags(virDomainPtr dom,
+ const char *xml,
+ unsigned int flags)
+{
+ return testDomainChgDevice(dom, VIR_DOMAIN_DEVICE_ACTION_DETACH,
+ xml, NULL, flags);
+}
/*
* Test driver
*/
@@ -9541,6 +9810,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainFSFreeze = testDomainFSFreeze, /* 5.7.0 */
.domainFSThaw = testDomainFSThaw, /* 5.7.0 */
.domainFSTrim = testDomainFSTrim, /* 5.7.0 */
+ .domainDetachDeviceFlags = testDomainDetachDeviceFlags, /* 7.7.0 */
.domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */
.domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */
.domainGetDiskErrors = testDomainGetDiskErrors, /* 5.4.0 */
--
2.32.0
Attachment:
signature.asc
Description: PGP signature
