libxl / lxc / qemu drivers share some common codes in their DomainDetachDeviceConfig functions, so extract them to domain_driver and reuse them. At the same time, this will enable test driver to test these functions with virshtest in the future. Signed-off-by: Luke Yue <lukedyue(a)gmail.com&gt; --- Not pretty sure whether this is a proper way to make these functions reusable, maybe there is a more elegant choice. --- src/hypervisor/domain_driver.c | 266 +++++++++++++++++++++++++++++++++ src/hypervisor/domain_driver.h | 41 +++++ src/libvirt_private.syms | 14 ++ src/libxl/libxl_driver.c | 41 +---- src/lxc/lxc_driver.c | 37 +---- src/qemu/qemu_driver.c | 123 +++------------ 6 files changed, 356 insertions(+), 166 deletions(-) diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index 31737b0f4a..01ecb4e30e 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -644,3 +644,269 @@ virDomainDriverGetIOThreadsConfig(virDomainDef *targetDef, return ret; } + + +int +virDomainDriverDetachDiskDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev) +{ + virDomainDiskDef *disk; + virDomainDiskDef *det_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); + + return 0; +} + +int +virDomainDriverDetachNetDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev) +{ + virDomainNetDef *net; + int idx; + + net = dev->data.net; + if ((idx = virDomainNetFindIdx(vmdef, net)) < 0) + return -1; + + /* this is guaranteed to succeed */ + virDomainNetDefFree(virDomainNetRemove(vmdef, idx)); + + return 0; +} + + +int +virDomainDriverDetachSoundDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev) +{ + virDomainSoundDef *sound; + int idx; + + 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)); + + return 0; +} + + +int +virDomainDriverDetachHostdevDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev) +{ + virDomainHostdevDef *hostdev; + virDomainHostdevDef *det_hostdev; + int idx; + + 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); + + return 0; +} + + +int +virDomainDriverDetachLeaseDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev) +{ + virDomainLeaseDef *lease; + virDomainLeaseDef *det_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); + + return 0; +} + + +int +virDomainDriverDetachControllerDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev) +{ + virDomainControllerDef *cont; + virDomainControllerDef *det_cont; + int idx; + + 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); + + return 0; +} + + +int +virDomainDriverDetachFSDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev) +{ + virDomainFSDef *fs; + int idx; + + 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); + + return 0; +} + + +int +virDomainDriverDetachRNGDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev) +{ + int idx; + + 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)); + + return 0; +} + + +int +virDomainDriverDetachMemoryDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev) +{ + virDomainMemoryDef *mem; + int idx; + + 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); + + return 0; +} + + +int +virDomainDriverDetachRedirdevDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev) +{ + int idx; + + 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)); + + return 0; +} + + +int +virDomainDriverDetachShmemDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev) +{ + int idx; + + 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)); + + return 0; +} + + +int +virDomainDriverDetachWatchdogDeviceConfig(virDomainDef *vmdef) +{ + if (!vmdef->watchdog) { + virReportError(VIR_ERR_DEVICE_MISSING, "%s", + _("domain has no watchdog")); + return -1; + } + virDomainWatchdogDefFree(vmdef->watchdog); + vmdef->watchdog = NULL; + + return 0; +} + + +int +virDomainDriverDetachInputDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev) +{ + int idx; + + if ((idx = virDomainInputDefFind(vmdef, dev->data.input)) < 0) { + virReportError(VIR_ERR_DEVICE_MISSING, "%s", + _("matching input device not found")); + return -1; + } + + virDomainInputDefFree(virDomainInputDefRemove(vmdef, idx)); + + return 0; +} + + +int +virDomainDriverDetachVsockDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev) +{ + 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; + + return 0; +} diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h index 7b0fbae2fd..e7fbd70d7b 100644 --- a/src/hypervisor/domain_driver.h +++ b/src/hypervisor/domain_driver.h @@ -70,3 +70,44 @@ int virDomainDriverDelIOThreadCheck(virDomainDef *def, int virDomainDriverGetIOThreadsConfig(virDomainDef *targetDef, virDomainIOThreadInfoPtr **info, unsigned int bitmap_size); + +int virDomainDriverDetachDiskDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev); + +int virDomainDriverDetachNetDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev); + +int virDomainDriverDetachSoundDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev); + +int virDomainDriverDetachHostdevDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev); + +int virDomainDriverDetachLeaseDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev); + +int virDomainDriverDetachControllerDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev); + +int virDomainDriverDetachFSDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev); + +int virDomainDriverDetachRNGDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev); + +int virDomainDriverDetachMemoryDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev); + +int virDomainDriverDetachRedirdevDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev); + +int virDomainDriverDetachShmemDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev); + +int virDomainDriverDetachWatchdogDeviceConfig(virDomainDef *vmdef); + +int virDomainDriverDetachInputDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev); + +int virDomainDriverDetachVsockDeviceConfig(virDomainDef *vmdef, + virDomainDeviceDef *dev); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 000c9893f0..0252f7c9d6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1551,6 +1551,20 @@ virDomainCgroupSetupMemtune; # hypervisor/domain_driver.h virDomainDriverAddIOThreadCheck; virDomainDriverDelIOThreadCheck; +virDomainDriverDetachControllerDeviceConfig; +virDomainDriverDetachDiskDeviceConfig; +virDomainDriverDetachFSDeviceConfig; +virDomainDriverDetachHostdevDeviceConfig; +virDomainDriverDetachInputDeviceConfig; +virDomainDriverDetachLeaseDeviceConfig; +virDomainDriverDetachMemoryDeviceConfig; +virDomainDriverDetachNetDeviceConfig; +virDomainDriverDetachRedirdevDeviceConfig; +virDomainDriverDetachRNGDeviceConfig; +virDomainDriverDetachShmemDeviceConfig; +virDomainDriverDetachSoundDeviceConfig; +virDomainDriverDetachVsockDeviceConfig; +virDomainDriverDetachWatchdogDeviceConfig; virDomainDriverGenerateMachineName; virDomainDriverGenerateRootHash; virDomainDriverGetIOThreadsConfig; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 7ea157f9c4..1e3d40f448 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3912,56 +3912,29 @@ libxlDomainDetachDeviceLive(libxlDriverPrivate *driver, static int libxlDomainDetachDeviceConfig(virDomainDef *vmdef, virDomainDeviceDef *dev) { - virDomainDiskDef *disk; - virDomainDiskDef *detach; - virDomainHostdevDef *hostdev; - virDomainHostdevDef *det_hostdev; - virDomainControllerDef *cont; - virDomainControllerDef *det_cont; - virDomainNetDef *net; - int idx; - switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: - disk = dev->data.disk; - if (!(detach = virDomainDiskRemoveByName(vmdef, disk->dst))) { - virReportError(VIR_ERR_INVALID_ARG, - _("no target device %s"), disk->dst); + if (virDomainDriverDetachDiskDeviceConfig(vmdef, dev) < 0) return -1; - } - virDomainDiskDefFree(detach); + break; case VIR_DOMAIN_DEVICE_CONTROLLER: - cont = dev->data.controller; - if ((idx = virDomainControllerFind(vmdef, cont->type, - cont->idx)) < 0) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("device not present in domain configuration")); + if (virDomainDriverDetachControllerDeviceConfig(vmdef, dev) < 0) return -1; - } - det_cont = virDomainControllerRemove(vmdef, idx); - virDomainControllerDefFree(det_cont); + break; case VIR_DOMAIN_DEVICE_NET: - net = dev->data.net; - if ((idx = virDomainNetFindIdx(vmdef, net)) < 0) + if (virDomainDriverDetachNetDeviceConfig(vmdef, dev) < 0) return -1; - /* this is guaranteed to succeed */ - virDomainNetDefFree(virDomainNetRemove(vmdef, idx)); break; case VIR_DOMAIN_DEVICE_HOSTDEV: { - hostdev = dev->data.hostdev; - if ((idx = virDomainHostdevFind(vmdef, hostdev, &det_hostdev)) < 0) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("device not present in domain configuration")); + if (virDomainDriverDetachHostdevDeviceConfig(vmdef, dev) < 0) return -1; - } - virDomainHostdevRemove(vmdef, idx); - virDomainHostdevDefFree(det_hostdev); + break; } diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index e2720a6f89..485683fe3a 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3129,56 +3129,33 @@ static int lxcDomainDetachDeviceConfig(virDomainDef *vmdef, virDomainDeviceDef *dev) { - int ret = -1; - virDomainDiskDef *disk; - virDomainDiskDef *det_disk; - virDomainNetDef *net; - virDomainHostdevDef *hostdev; - virDomainHostdevDef *det_hostdev; - int idx; - switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: - disk = dev->data.disk; - if (!(det_disk = virDomainDiskRemoveByName(vmdef, disk->dst))) { - virReportError(VIR_ERR_INVALID_ARG, - _("no target device %s"), disk->dst); + if (virDomainDriverDetachDiskDeviceConfig(vmdef, dev) < 0) return -1; - } - virDomainDiskDefFree(det_disk); - ret = 0; + break; case VIR_DOMAIN_DEVICE_NET: - net = dev->data.net; - if ((idx = virDomainNetFindIdx(vmdef, net)) < 0) + if (virDomainDriverDetachNetDeviceConfig(vmdef, dev) < 0) return -1; - /* this is guaranteed to succeed */ - virDomainNetDefFree(virDomainNetRemove(vmdef, idx)); - ret = 0; break; case VIR_DOMAIN_DEVICE_HOSTDEV: { - hostdev = dev->data.hostdev; - if ((idx = virDomainHostdevFind(vmdef, hostdev, &det_hostdev)) < 0) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("device not present in domain configuration")); + if (virDomainDriverDetachHostdevDeviceConfig(vmdef, dev) < 0) return -1; - } - virDomainHostdevRemove(vmdef, idx); - virDomainHostdevDefFree(det_hostdev); - ret = 0; + break; } default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("persistent detach of device is not supported")); - break; + return -1; } - return ret; + return 0; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cf9407c358..d149cd22b2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7517,84 +7517,43 @@ qemuDomainDetachDeviceConfig(virDomainDef *vmdef, unsigned int parse_flags, virDomainXMLOption *xmlopt) { - virDomainDiskDef *disk; - virDomainDiskDef *det_disk; - virDomainNetDef *net; - virDomainSoundDef *sound; - virDomainHostdevDef *hostdev; - virDomainHostdevDef *det_hostdev; - virDomainLeaseDef *lease; - virDomainLeaseDef *det_lease; - virDomainControllerDef *cont; - virDomainControllerDef *det_cont; virDomainChrDef *chr; - virDomainFSDef *fs; - virDomainMemoryDef *mem; - int idx; switch ((virDomainDeviceType)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); + if (virDomainDriverDetachDiskDeviceConfig(vmdef, dev) < 0) return -1; - } - virDomainDiskDefFree(det_disk); + break; case VIR_DOMAIN_DEVICE_NET: - net = dev->data.net; - if ((idx = virDomainNetFindIdx(vmdef, net)) < 0) + if (virDomainDriverDetachNetDeviceConfig(vmdef, dev) < 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")); + if (virDomainDriverDetachSoundDeviceConfig(vmdef, dev) < 0) 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")); + if (virDomainDriverDetachHostdevDeviceConfig(vmdef, dev) < 0) 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)); + if (virDomainDriverDetachLeaseDeviceConfig(vmdef, dev) < 0) 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")); + if (virDomainDriverDetachControllerDeviceConfig(vmdef, dev) < 0) return -1; - } - det_cont = virDomainControllerRemove(vmdef, idx); - virDomainControllerDefFree(det_cont); break; @@ -7606,91 +7565,51 @@ qemuDomainDetachDeviceConfig(virDomainDef *vmdef, 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")); + if (virDomainDriverDetachFSDeviceConfig(vmdef, dev) < 0) 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")); + if (virDomainDriverDetachRNGDeviceConfig(vmdef, dev) < 0) 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")); + if (virDomainDriverDetachMemoryDeviceConfig(vmdef, dev) < 0) 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")); + if (virDomainDriverDetachRedirdevDeviceConfig(vmdef, dev) < 0) 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")); + if (virDomainDriverDetachShmemDeviceConfig(vmdef, dev) < 0) 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")); + if (virDomainDriverDetachWatchdogDeviceConfig(vmdef) < 0) 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")); + if (virDomainDriverDetachInputDeviceConfig(vmdef, dev) < 0) return -1; - } - virDomainInputDefFree(virDomainInputDefRemove(vmdef, idx)); 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")); + if (virDomainDriverDetachVsockDeviceConfig(vmdef, dev) < 0) return -1; - } - virDomainVsockDefFree(vmdef->vsock); - vmdef->vsock = NULL; + break; case VIR_DOMAIN_DEVICE_VIDEO: -- 2.33.1

On Fri, 2021-11-26 at 16:43 +0100, Martin Kletzander wrote: Thanks for the review! No problem, I would move the code to domain_conf.c in next version. So if I understand you correctly, I will unified all these functions into one, just like the original ones for QEMU / lxc / libxl, but this one is for all, and maybe I should create a new enum for flags of detachable device that different hypervisor support, and xmlopt etc for special devices. I will do that in next version. Thanks, Luke

On Mon, 2021-11-29 at 15:33 +0100, Martin Kletzander wrote: OK, thanks, now I see.

On Wed, Nov 10, 2021 at 10:24:22PM +0800, Luke Yue wrote: My comment from the previous patch would be nicely usable here as we could easily just make use of it. Also I see no reason for splitting the next two patches from this one. [...] So here you check for both live and config, saying both of them are supported, ... But here it would fail with both since you are explicitly saying you want just one definition. And I kind of see now why you call the function "LiveAndConfig" since it can do both based on what DomainDef you call it with. This function could be very easily modified to do both live and config properly.

As we already implement testDomainChgDevice for both DetachDeviceFlags and DetachDeviceAlias, so it's simple to implement this API by changing the parameter. Signed-off-by: Luke Yue <lukedyue(a)gmail.com&gt; --- src/test/test_driver.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 6a7eb12f77..9843340fec 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -10252,6 +10252,15 @@ testDomainDetachDeviceFlags(virDomainPtr dom, xml, NULL, flags); } +static int +testDomainDetachDeviceAlias(virDomainPtr dom, + const char *alias, + unsigned int flags) +{ + return testDomainChgDevice(dom, VIR_DOMAIN_DEVICE_ACTION_DETACH, + NULL, alias, flags); +} + /* * Test driver */ @@ -10349,6 +10358,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainFSFreeze = testDomainFSFreeze, /* 5.7.0 */ .domainFSThaw = testDomainFSThaw, /* 5.7.0 */ .domainFSTrim = testDomainFSTrim, /* 5.7.0 */ + .domainDetachDeviceAlias = testDomainDetachDeviceAlias, /* 7.10.0 */ .domainDetachDeviceFlags = testDomainDetachDeviceFlags, /* 7.10.0 */ .domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */ .domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */ -- 2.33.1

Currently it will only be used in the test driver. Signed-off-by: Luke Yue <lukedyue(a)gmail.com&gt; --- src/conf/domain_conf.c | 67 ++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 6 ++++ src/libvirt_private.syms | 2 ++ 3 files changed, 75 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3193120b79..512bfab9e9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16931,6 +16931,73 @@ virDomainVsockDefEquals(const virDomainVsockDef *a, } +static bool +virDomainTPMDefEquals(const virDomainTPMDef *a, + const virDomainTPMDef *b) +{ + if (a->type != b->type) + return false; + + if (a->model != b->model) + return false; + + if (a->version != b->version) + return false; + + if (a->type == VIR_DOMAIN_TPM_TYPE_PASSTHROUGH) { + if (STRNEQ_NULLABLE(a->data.passthrough.source.data.file.path, + b->data.passthrough.source.data.file.path)) + return false; + } else { + if (a->data.emulator.hassecretuuid != b->data.emulator.hassecretuuid) + return false; + + if (a->data.emulator.hassecretuuid == true && + memcmp(a->data.emulator.secretuuid, + b->data.emulator.secretuuid, + VIR_UUID_BUFLEN)) + return false; + + if (a->data.emulator.persistent_state != + b->data.emulator.persistent_state) + return false; + } + + if (a->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + !virDomainDeviceInfoAddressIsEqual(&a->info, &b->info)) + return false; + + return true; +} + + +ssize_t +virDomainTPMDefFind(const virDomainDef *def, + const virDomainTPMDef *tpm) +{ + size_t i; + + for (i = 0; i < def->ntpms; i++) { + if (virDomainTPMDefEquals(tpm, def->tpms[i])) + return i; + } + + return -1; +} + + +virDomainTPMDef * +virDomainTPMDefRemove(virDomainDef *def, + size_t idx) +{ + virDomainTPMDef *ret = def->tpms[idx]; + + VIR_DELETE_ELEMENT(def->tpms, idx, def->ntpms); + + return ret; +} + + char * virDomainDefGetDefaultEmulator(virDomainDef *def, virCaps *caps) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a089b0b3de..715c8fbd16 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3857,6 +3857,12 @@ bool virDomainVsockDefEquals(const virDomainVsockDef *a, const virDomainVsockDef *b) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; +ssize_t virDomainTPMDefFind(const virDomainDef *def, + const virDomainTPMDef *tpm) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; +virDomainTPMDef *virDomainTPMDefRemove(virDomainDef *def, size_t idx) + ATTRIBUTE_NONNULL(1); + VIR_ENUM_DECL(virDomainTaint); VIR_ENUM_DECL(virDomainTaintMessage); VIR_ENUM_DECL(virDomainVirt); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0252f7c9d6..68cc9c51cb 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -665,7 +665,9 @@ virDomainTimerTrackTypeFromString; virDomainTimerTrackTypeToString; virDomainTPMBackendTypeFromString; virDomainTPMBackendTypeToString; +virDomainTPMDefFind; virDomainTPMDefFree; +virDomainTPMDefRemove; virDomainTPMModelTypeFromString; virDomainTPMModelTypeToString; virDomainTPMPcrBankTypeFromString; -- 2.33.1

On Fri, 2021-11-26 at 16:43 +0100, Martin Kletzander wrote: I just noticed in commit 42b0000, the `source` was changed to be a pointer, will fix in next version. Hmm, I will look into how libvirt adds these default devices and refine this function in next version or in the future. Thanks, Luke

Currently it will only be used in test driver. Signed-off-by: Luke Yue <lukedyue(a)gmail.com&gt; --- src/conf/domain_conf.c | 24 ++++++++++++++++++++++++ src/conf/domain_conf.h | 4 ++++ src/libvirt_private.syms | 1 + 3 files changed, 29 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 512bfab9e9..92a8bd63f3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16998,6 +16998,30 @@ virDomainTPMDefRemove(virDomainDef *def, } +bool +virDomainMemballoonDefEquals(const virDomainMemballoonDef *a, + const virDomainMemballoonDef *b) +{ + if (a->model != b->model) + return false; + + if (a->period != b->period) + return false; + + if (a->autodeflate != b->autodeflate) + return false; + + if (a->free_page_reporting != b->free_page_reporting) + return false; + + if (a->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + !virDomainDeviceInfoAddressIsEqual(&a->info, &b->info)) + return false; + + return true; +} + + char * virDomainDefGetDefaultEmulator(virDomainDef *def, virCaps *caps) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 715c8fbd16..f60ba37d19 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3863,6 +3863,10 @@ ssize_t virDomainTPMDefFind(const virDomainDef *def, virDomainTPMDef *virDomainTPMDefRemove(virDomainDef *def, size_t idx) ATTRIBUTE_NONNULL(1); +bool virDomainMemballoonDefEquals(const virDomainMemballoonDef *a, + const virDomainMemballoonDef *b) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; + VIR_ENUM_DECL(virDomainTaint); VIR_ENUM_DECL(virDomainTaintMessage); VIR_ENUM_DECL(virDomainVirt); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cfda58320a..b143537bb4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -506,6 +506,7 @@ virDomainLoaderTypeFromString; virDomainLoaderTypeToString; virDomainLockFailureTypeFromString; virDomainLockFailureTypeToString; +virDomainMemballoonDefEquals; virDomainMemballoonModelTypeFromString; virDomainMemballoonModelTypeToString; virDomainMemoryDefFree; -- 2.33.1

On Fri, 2021-11-26 at 16:44 +0100, Martin Kletzander wrote: Oh, sorry, I didn't realize that, I forgot that the device would just present once, so that we can just remove the device without comparison, I will remove the comparison function. Thanks, Luke

Signed-off-by: Luke Yue <lukedyue(a)gmail.com&gt; --- examples/xml/test/testdevcontroller.xml | 1 + examples/xml/test/testdevdiskcdrom.xml | 5 +++ examples/xml/test/testdevfs.xml | 6 +++ examples/xml/test/testdevhostdev.xml | 5 +++ examples/xml/test/testdevif.xml | 6 +++ examples/xml/test/testdevinput.xml | 1 + examples/xml/test/testdevlease.xml | 5 +++ examples/xml/test/testdevmem.xml | 6 +++ examples/xml/test/testdevmemballoon.xml | 3 ++ examples/xml/test/testdevrng.xml | 4 ++ examples/xml/test/testdevshmem.xml | 4 ++ examples/xml/test/testdevsound.xml | 3 ++ examples/xml/test/testdevtpm.xml | 5 +++ examples/xml/test/testdevvsock.xml | 3 ++ examples/xml/test/testdevwatchdog.xml | 1 + examples/xml/test/testdomfc5.xml | 54 +++++++++++++++++++++++++ examples/xml/test/testnodeinline.xml | 54 +++++++++++++++++++++++++ 17 files changed, 166 insertions(+) create mode 100644 examples/xml/test/testdevcontroller.xml create mode 100644 examples/xml/test/testdevdiskcdrom.xml create mode 100644 examples/xml/test/testdevfs.xml create mode 100644 examples/xml/test/testdevhostdev.xml create mode 100644 examples/xml/test/testdevif.xml create mode 100644 examples/xml/test/testdevinput.xml create mode 100644 examples/xml/test/testdevlease.xml create mode 100644 examples/xml/test/testdevmem.xml create mode 100644 examples/xml/test/testdevmemballoon.xml create mode 100644 examples/xml/test/testdevrng.xml create mode 100644 examples/xml/test/testdevshmem.xml create mode 100644 examples/xml/test/testdevsound.xml create mode 100644 examples/xml/test/testdevtpm.xml create mode 100644 examples/xml/test/testdevvsock.xml create mode 100644 examples/xml/test/testdevwatchdog.xml diff --git a/examples/xml/test/testdevcontroller.xml b/examples/xml/test/testdevcontroller.xml new file mode 100644 index 0000000000..d855bfa17f --- /dev/null +++ b/examples/xml/test/testdevcontroller.xml @@ -0,0 +1 @@ +<controller type='ide' index='0'/> diff --git a/examples/xml/test/testdevdiskcdrom.xml b/examples/xml/test/testdevdiskcdrom.xml new file mode 100644 index 0000000000..edc90556bb --- /dev/null +++ b/examples/xml/test/testdevdiskcdrom.xml @@ -0,0 +1,5 @@ +<disk type='block' device='cdrom'> + <source dev='/dev/sr0'/> + <target dev='hdb' bus='ide'/> + <readonly/> +</disk> diff --git a/examples/xml/test/testdevfs.xml b/examples/xml/test/testdevfs.xml new file mode 100644 index 0000000000..cee5ed4ed9 --- /dev/null +++ b/examples/xml/test/testdevfs.xml @@ -0,0 +1,6 @@ +<filesystem type='file' accessmode='passthrough'> + <driver type='loop' format='raw'/> + <source file='/root/test/guest.img'/> + <target dir='/root/libvirt/test'/> + <readonly/> +</filesystem> diff --git a/examples/xml/test/testdevhostdev.xml b/examples/xml/test/testdevhostdev.xml new file mode 100644 index 0000000000..e364b50f36 --- /dev/null +++ b/examples/xml/test/testdevhostdev.xml @@ -0,0 +1,5 @@ +<hostdev mode='capabilities' type='storage'> + <source> + <block>/dev/sdf1</block>s + </source> +</hostdev> diff --git a/examples/xml/test/testdevif.xml b/examples/xml/test/testdevif.xml new file mode 100644 index 0000000000..7e0be80050 --- /dev/null +++ b/examples/xml/test/testdevif.xml @@ -0,0 +1,6 @@ +<interface type='network'> + <source network='testbrigde' /> + <mac address='00:11:22:33:44:55' /> + <model type='virtio' /> + <alias name='ua-testNIC' /> +</interface> diff --git a/examples/xml/test/testdevinput.xml b/examples/xml/test/testdevinput.xml new file mode 100644 index 0000000000..d958f5b931 --- /dev/null +++ b/examples/xml/test/testdevinput.xml @@ -0,0 +1 @@ +<input type='mouse' bus='virtio'/> diff --git a/examples/xml/test/testdevlease.xml b/examples/xml/test/testdevlease.xml new file mode 100644 index 0000000000..c53c0c7e7d --- /dev/null +++ b/examples/xml/test/testdevlease.xml @@ -0,0 +1,5 @@ +<lease> + <lockspace>testarea</lockspace> + <key>testkey</key> + <target path='/root/test/lease/path' offset='1024'/> +</lease> diff --git a/examples/xml/test/testdevmem.xml b/examples/xml/test/testdevmem.xml new file mode 100644 index 0000000000..49efd4af55 --- /dev/null +++ b/examples/xml/test/testdevmem.xml @@ -0,0 +1,6 @@ +<memory model='dimm' access='private' discard='yes'> + <target> + <size unit='KiB'>524287</size> + <node>0</node> + </target> +</memory> diff --git a/examples/xml/test/testdevmemballoon.xml b/examples/xml/test/testdevmemballoon.xml new file mode 100644 index 0000000000..fde18a9db6 --- /dev/null +++ b/examples/xml/test/testdevmemballoon.xml @@ -0,0 +1,3 @@ +<memballoon model='virtio'> + <stats period='10' /> +</memballoon> diff --git a/examples/xml/test/testdevrng.xml b/examples/xml/test/testdevrng.xml new file mode 100644 index 0000000000..369f423740 --- /dev/null +++ b/examples/xml/test/testdevrng.xml @@ -0,0 +1,4 @@ +<rng model='virtio'> + <rate period="2000" bytes="1234" /> + <backend model='builtin' /> +</rng> diff --git a/examples/xml/test/testdevshmem.xml b/examples/xml/test/testdevshmem.xml new file mode 100644 index 0000000000..04bf3d9a53 --- /dev/null +++ b/examples/xml/test/testdevshmem.xml @@ -0,0 +1,4 @@ +<shmem name='my_shmem0' role='peer'> + <model type='ivshmem-plain'/> + <size unit='M'>4</size> +</shmem> diff --git a/examples/xml/test/testdevsound.xml b/examples/xml/test/testdevsound.xml new file mode 100644 index 0000000000..cf7323077f --- /dev/null +++ b/examples/xml/test/testdevsound.xml @@ -0,0 +1,3 @@ +<sound model='ich6'> + <codec type='micro'/> +</sound> diff --git a/examples/xml/test/testdevtpm.xml b/examples/xml/test/testdevtpm.xml new file mode 100644 index 0000000000..cc08b7bf6d --- /dev/null +++ b/examples/xml/test/testdevtpm.xml @@ -0,0 +1,5 @@ +<tpm model='tpm-tis'> + <backend type='emulator' version='2.0'> + <encryption secret='6dd3e4a5-1d76-44ce-961f-f119f5aad935'/> + </backend> +</tpm> diff --git a/examples/xml/test/testdevvsock.xml b/examples/xml/test/testdevvsock.xml new file mode 100644 index 0000000000..dda45780c4 --- /dev/null +++ b/examples/xml/test/testdevvsock.xml @@ -0,0 +1,3 @@ +<vsock model='virtio'> + <cid auto='no' address='3'/> +</vsock> diff --git a/examples/xml/test/testdevwatchdog.xml b/examples/xml/test/testdevwatchdog.xml new file mode 100644 index 0000000000..a02086f296 --- /dev/null +++ b/examples/xml/test/testdevwatchdog.xml @@ -0,0 +1 @@ +<watchdog model='i6300esb'/> diff --git a/examples/xml/test/testdomfc5.xml b/examples/xml/test/testdomfc5.xml index a8afc211f6..3b9edb9da9 100644 --- a/examples/xml/test/testdomfc5.xml +++ b/examples/xml/test/testdomfc5.xml @@ -29,6 +29,12 @@ <mac address='00:16:3e:5d:c7:26'/> <script path='vif-bridge'/> </interface> + <interface type='network'> + <source network='testbrigde' /> + <mac address='00:11:22:33:44:55' /> + <model type='virtio' /> + <alias name='ua-testNIC' /> + </interface> <disk type='file'> <source file='/root/fv0'/> <target dev='hda'/> @@ -36,6 +42,7 @@ <disk type='block' device='cdrom'> <source dev='/dev/sr0'/> <target dev='hdb' bus='ide'/> + <alias name='ua-testCD' /> <readonly/> </disk> <disk type='file' device='floppy'> @@ -47,5 +54,52 @@ <target dev='sda' bus='scsi'/> </disk> <graphics type='vnc' port='5904'/> + <sound model='ich6'> + <codec type='micro'/> + </sound> + <hostdev mode='capabilities' type='storage'> + <source> + <block>/dev/sdf1</block> + </source> + </hostdev> + <lease> + <lockspace>testarea</lockspace> + <key>testkey</key> + <target path='/root/test/lease/path' offset='1024'/> + </lease> + <controller type='ide' index='0'/> + <filesystem type='file' accessmode='passthrough'> + <driver type='loop' format='raw'/> + <source file='/root/test/guest.img'/> + <target dir='/root/libvirt/test'/> + <readonly/> + </filesystem> + <rng model='virtio'> + <rate period="2000" bytes="1234" /> + <backend model='builtin' /> + </rng> + <memory model='dimm' access='private' discard='yes'> + <target> + <size unit='KiB'>524287</size> + <node>0</node> + </target> + </memory> + <shmem name='my_shmem0' role='peer'> + <model type='ivshmem-plain'/> + <size unit='M'>4</size> + </shmem> + <watchdog model='i6300esb'/> + <input type='mouse' bus='virtio'/> + <vsock model='virtio'> + <cid auto='no' address='3'/> + </vsock> + <tpm model='tpm-tis'> + <backend type='emulator' version='2.0'> + <encryption secret='6dd3e4a5-1d76-44ce-961f-f119f5aad935'/> + </backend> + </tpm> + <memballoon model='virtio'> + <stats period='10' /> + </memballoon> </devices> </domain> diff --git a/examples/xml/test/testnodeinline.xml b/examples/xml/test/testnodeinline.xml index 9165d9302d..1124146f73 100644 --- a/examples/xml/test/testnodeinline.xml +++ b/examples/xml/test/testnodeinline.xml @@ -117,6 +117,12 @@ <mac address='00:16:3e:5d:c7:26'/> <script path='vif-bridge'/> </interface> + <interface type='network'> + <source network='testbrigde' /> + <mac address='00:11:22:33:44:55' /> + <model type='virtio' /> + <alias name='ua-testNIC' /> + </interface> <disk type='file'> <source file='/root/fv0'/> <target dev='hda'/> @@ -124,6 +130,7 @@ <disk type='block' device='cdrom'> <source dev='/dev/sr0'/> <target dev='hdb' bus='ide'/> + <alias name='ua-testCD' /> <readonly/> </disk> <disk type='file' device='floppy'> @@ -135,6 +142,53 @@ <target dev='sda' bus='scsi'/> </disk> <graphics type='vnc' port='5904'/> + <sound model='ich6'> + <codec type='micro'/> + </sound> + <hostdev mode='capabilities' type='storage'> + <source> + <block>/dev/sdf1</block> + </source> + </hostdev> + <lease> + <lockspace>testarea</lockspace> + <key>testkey</key> + <target path='/root/test/lease/path' offset='1024'/> + </lease> + <controller type='ide' index='0'/> + <filesystem type='file' accessmode='passthrough'> + <driver type='loop' format='raw'/> + <source file='/root/test/guest.img'/> + <target dir='/root/libvirt/test'/> + <readonly/> + </filesystem> + <rng model='virtio'> + <rate period="2000" bytes="1234" /> + <backend model='builtin' /> + </rng> + <memory model='dimm' access='private' discard='yes'> + <target> + <size unit='KiB'>524287</size> + <node>0</node> + </target> + </memory> + <shmem name='my_shmem0' role='peer'> + <model type='ivshmem-plain'/> + <size unit='M'>4</size> + </shmem> + <watchdog model='i6300esb'/> + <input type='mouse' bus='virtio'/> + <vsock model='virtio'> + <cid auto='no' address='3'/> + </vsock> + <tpm model='tpm-tis'> + <backend type='emulator' version='2.0'> + <encryption secret='6dd3e4a5-1d76-44ce-961f-f119f5aad935'/> + </backend> + </tpm> + <memballoon model='virtio'> + <stats period='10' /> + </memballoon> </devices> </domain> <network> -- 2.33.1

Signed-off-by: Luke Yue <lukedyue(a)gmail.com&gt; --- tests/virshtest.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/tests/virshtest.c b/tests/virshtest.c index af2a70f5fb..8e5b397420 100644 --- a/tests/virshtest.c +++ b/tests/virshtest.c @@ -160,6 +160,8 @@ static char *custom_uri; "--connect", \ custom_uri +# define TEST_XML_PATH abs_top_builddir "/../examples/xml/test" + static int testCompareListDefault(const void *data G_GNUC_UNUSED) { const char *const argv[] = { VIRSH_DEFAULT, "list", NULL }; @@ -437,6 +439,88 @@ static int testIOThreadPin(const void *data G_GNUC_UNUSED) return testCompareOutputLit(exp, "", NULL, argv); } +static int testCompareDetachDevice(const void *data G_GNUC_UNUSED) +{ + const char *const argv[] = { VIRSH_CUSTOM, "detach-device fc5\ + " TEST_XML_PATH "/testdevif.xml;\ + detach-device fc5\ + " TEST_XML_PATH "/testdevdiskcdrom.xml;\ + detach-device fc5\ + " TEST_XML_PATH "/testdevsound.xml;\ + detach-device fc5\ + " TEST_XML_PATH "/testdevhostdev.xml;\ + detach-device fc5\ + " TEST_XML_PATH "/testdevlease.xml;\ + detach-device fc5\ + " TEST_XML_PATH "/testdevcontroller.xml;\ + detach-device fc5\ + " TEST_XML_PATH "/testdevfs.xml;\ + detach-device fc5\ + " TEST_XML_PATH "/testdevrng.xml;\ + detach-device fc5\ + " TEST_XML_PATH "/testdevmem.xml;\ + detach-device fc5\ + " TEST_XML_PATH "/testdevshmem.xml;\ + detach-device fc5\ + " TEST_XML_PATH "/testdevwatchdog.xml;\ + detach-device fc5\ + " TEST_XML_PATH "/testdevinput.xml;\ + detach-device fc5\ + " TEST_XML_PATH "/testdevvsock.xml;\ + detach-device fc5\ + " TEST_XML_PATH "/testdevtpm.xml", + NULL }; + const char *exp = +"Device detached successfully\n\n\ +Device detached successfully\n\n\ +Device detached successfully\n\n\ +Device detached successfully\n\n\ +Device detached successfully\n\n\ +Device detached successfully\n\n\ +Device detached successfully\n\n\ +Device detached successfully\n\n\ +Device detached successfully\n\n\ +Device detached successfully\n\n\ +Device detached successfully\n\n\ +Device detached successfully\n\n\ +Device detached successfully\n\n\ +Device detached successfully\n\n"; + return testCompareOutputLit(exp, "", NULL, argv); +} + +static int testCompareDetachDeviceError(const void *data G_GNUC_UNUSED) +{ + const char *const argv[] = { VIRSH_CUSTOM, "detach-device fc5\ + " TEST_XML_PATH "/testdevtpm.xml;\ + detach-device fc5\ + " TEST_XML_PATH "/testdevtpm.xml;\ + detach-device fc5 --live\ + " TEST_XML_PATH "/testdevmemballoon.xml", + NULL }; + const char *exp = +"Device detached successfully\n\n\n\n"; + const char *error_msg = +"error: Failed to detach device from " TEST_XML_PATH "/testdevtpm.xml\n\ +error: device not found: matching tpm device not found\n\ +error: Failed to detach device from " TEST_XML_PATH "/testdevmemballoon.xml\n\ +error: Operation not supported: detach of device 'memballoon' on running domain is not supported\n"; + return testCompareOutputLit(exp, error_msg, NULL, argv); +} + +static int testCompareDetachDeviceAlias(const void *data G_GNUC_UNUSED) +{ + const char *const argv[] = { VIRSH_CUSTOM, + "detach-device-alias fc5 ua-testCD;\ + detach-device fc5\ + " TEST_XML_PATH "/testdevdiskcdrom.xml", + NULL }; + const char *exp = "Device detach request sent successfully\n\n\n"; + const char *error_msg = +"error: Failed to detach device from " TEST_XML_PATH "/testdevdiskcdrom.xml\n\ +error: device not found: no target device hdb\n"; + return testCompareOutputLit(exp, error_msg, NULL, argv); +} + struct testInfo { const char *const *argv; const char *result; @@ -553,6 +637,18 @@ mymain(void) testIOThreadPin, NULL) != 0) ret = -1; + if (virTestRun("virsh detach-device", + testCompareDetachDevice, NULL) != 0) + ret = -1; + + if (virTestRun("virsh detach-device (with failure)", + testCompareDetachDeviceError, NULL) != 0) + ret = -1; + + if (virTestRun("virsh detach-device-alias", + testCompareDetachDeviceAlias, NULL) != 0) + ret = -1; + /* It's a bit awkward listing result before argument, but that's a * limitation of C99 vararg macros. */ # define DO_TEST(i, result, ...) \ -- 2.33.1

On Fri, 2021-11-26 at 16:44 +0100, Martin Kletzander wrote: Thanks for the explanation, I will look into how qemuhotplugtest is implemented. Should I create a new file for these tests? As this file is for `virsh` test but no for test using internal APIs? Thanks, Luke

On Mon, 2021-11-29 at 15:50 +0100, Martin Kletzander wrote: Thanks, will reimplement these tests with internal APIs in next version.