6:19 a.m.
Luke Yue (5):
test_driver: Implement virDomainDetachDeviceFlags
test_driver: Implement virDomainDetachDeviceAlias
test_driver: Implement virDomainDetachDevice
examples: xml: test: add xml for testing devices related APIs
tests: Test detach-device and detach-device-alias for test driver
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/testdevnetif.xml | 6 +
examples/xml/test/testdevrng.xml | 4 +
examples/xml/test/testdevshmem.xml | 4 +
examples/xml/test/testdevsound.xml | 3 +
examples/xml/test/testdevvsock.xml | 3 +
examples/xml/test/testdevwatchdog.xml | 1 +
examples/xml/test/testdomfc5.xml | 46 ++++
examples/xml/test/testnodeinline.xml | 46 ++++
src/test/test_driver.c | 290 ++++++++++++++++++++++++
tests/virshtest.c | 24 ++
18 files changed, 462 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/testdevnetif.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/testdevvsock.xml
create mode 100644 examples/xml/test/testdevwatchdog.xml
--
2.32.0
6:19 a.m.
New subject: [PATCH 1/5] test_driver: Implement virDomainDetachDeviceFlags
Introduce testDomainChgDevice for further development (just like what we
did for IOThread). And introduce testDomainDetachDeviceLiveAndConfig for
detaching devices.
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
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;
+
+ 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:
+ case VIR_DOMAIN_DEVICE_LAST:
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+ _("persistent detach of device '%s' is not
supported"),
+ 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;
+
+ case VIR_DOMAIN_DEVICE_ACTION_DETACH:
+ if (testDomainDetachDeviceLiveAndConfig(def, dev) < 0)
+ goto cleanup;
+ break;
+
+ case VIR_DOMAIN_DEVICE_ACTION_UPDATE:
+ break;
+ }
+
+ 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
7:13 a.m.
New subject: [PATCH 1/5] test_driver: Implement virDomainDetachDeviceFlags
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(a)gmail.com>
---
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
7:50 a.m.
New subject: [PATCH 1/5] test_driver: Implement virDomainDetachDeviceFlags
On Tue, 2021-08-17 at 14:13 +0200, Martin Kletzander wrote:
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(a)gmail.com>
> ---
> 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.
Yes, I was thinking if I can extract them for these drivers, but I
noticed that there may be some differences, like we have to check ACL
for some devices when using other drivers, and at least for QEMU
driver, there is no
virDomainInputDefFree(vmdef->inputs[idx]);
line in `case VIR_DOMAIN_DEVICE_INPUT:` in QEMU driver, I'm not pretty
sure whether this will lead to memory leak for QEMU or not, or it's by
design, but at least it do leak memory when using test driver. So we
may need to handle them for different drivers' case, and I am not
pretty sure whether I should extract or not, so I decide to send the
patches first. I was wondering if we can add a flag to indicate which
driver is using the function, will give it a try.
> + 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.
OK, I will try to add more of them.
> + 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
I thought there is no function would pass
VIR_DOMAIN_DEVICE_ACTION_ATTACH or UPDATE to this function at the
moment, so I just leave it there.
> + }
> +
> + 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
>
9:13 a.m.
New subject: [PATCH 1/5] test_driver: Implement virDomainDetachDeviceFlags
On Tue, Aug 17, 2021 at 08:50:59PM +0800, Luke Yue wrote:
On Tue, 2021-08-17 at 14:13 +0200, Martin Kletzander wrote:
> 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(a)gmail.com>
> > ---
> > src/test/test_driver.c | 270
> > +++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 270 insertions(+)
> >
[...]
> > +
> > + 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.
>
Yes, I was thinking if I can extract them for these drivers, but I
noticed that there may be some differences, like we have to check ACL
for some devices when using other drivers, and at least for QEMU
driver, there is no
virDomainInputDefFree(vmdef->inputs[idx]);
line in `case VIR_DOMAIN_DEVICE_INPUT:` in QEMU driver, I'm not pretty
sure whether this will lead to memory leak for QEMU or not, or it's by
design, but at least it do leak memory when using test driver. So we
may need to handle them for different drivers' case, and I am not
pretty sure whether I should extract or not, so I decide to send the
patches first. I was wondering if we can add a flag to indicate which
driver is using the function, will give it a try.
This whole thing does not have to be extracted as one function. Or it
can and there can be either flags or some callbacks, for example passed
inside xmlopt. Whatever makes it more readable, nicer etc. There will
inevitably be numerous ways to approach this and the real work is
figuring out which is the cleaner one.
> > + 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.
>
OK, I will try to add more of them.
Of course first figuring out how to approach the above may make it way
easier to then add more of these.
[...]
> > + 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
>
I thought there is no function would pass
VIR_DOMAIN_DEVICE_ACTION_ATTACH or UPDATE to this function at the
moment, so I just leave it there.
Well yes, but then it can make it hard to find an error if someone wants
to add support for attaching and they use the function, based on the
name it sounds alright and then there is no error, it just does not
work. It's not that it is bad now, but for the purpose of
future-proofing it is beneficial to handle those cases clearly in some
way.
6:19 a.m.
New subject: [PATCH 2/5] test_driver: Implement virDomainDetachDeviceAlias
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/test/test_driver.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 2ebdbaa604..e2463876fa 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -9721,6 +9721,16 @@ testDomainDetachDeviceFlags(virDomainPtr dom,
return testDomainChgDevice(dom, VIR_DOMAIN_DEVICE_ACTION_DETACH,
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
*/
@@ -9810,6 +9820,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainFSFreeze = testDomainFSFreeze, /* 5.7.0 */
.domainFSThaw = testDomainFSThaw, /* 5.7.0 */
.domainFSTrim = testDomainFSTrim, /* 5.7.0 */
+ .domainDetachDeviceAlias = testDomainDetachDeviceAlias, /* 7.7.0 */
.domainDetachDeviceFlags = testDomainDetachDeviceFlags, /* 7.7.0 */
.domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */
.domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */
--
2.32.0
6:19 a.m.
New subject: [PATCH 3/5] test_driver: Implement virDomainDetachDevice
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
src/test/test_driver.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index e2463876fa..21e813db8f 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -9731,6 +9731,14 @@ testDomainDetachDeviceAlias(virDomainPtr dom,
NULL, alias, flags);
}
+static int
+testDomainDetachDevice(virDomainPtr dom,
+ const char *xml)
+{
+ return testDomainDetachDeviceFlags(dom, xml,
+ VIR_DOMAIN_AFFECT_LIVE);
+}
+
/*
* Test driver
*/
@@ -9820,6 +9828,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainFSFreeze = testDomainFSFreeze, /* 5.7.0 */
.domainFSThaw = testDomainFSThaw, /* 5.7.0 */
.domainFSTrim = testDomainFSTrim, /* 5.7.0 */
+ .domainDetachDevice = testDomainDetachDevice, /* 7.7.0 */
.domainDetachDeviceAlias = testDomainDetachDeviceAlias, /* 7.7.0 */
.domainDetachDeviceFlags = testDomainDetachDeviceFlags, /* 7.7.0 */
.domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */
--
2.32.0
6:19 a.m.
New subject: [PATCH 4/5] examples: xml: test: add xml for testing devices related APIs
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
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/testdevnetif.xml | 6 ++++
examples/xml/test/testdevrng.xml | 4 +++
examples/xml/test/testdevshmem.xml | 4 +++
examples/xml/test/testdevsound.xml | 3 ++
examples/xml/test/testdevvsock.xml | 3 ++
examples/xml/test/testdevwatchdog.xml | 1 +
examples/xml/test/testdomfc5.xml | 46 +++++++++++++++++++++++++
examples/xml/test/testnodeinline.xml | 46 +++++++++++++++++++++++++
16 files changed, 148 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/testdevnetif.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/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/testdevnetif.xml b/examples/xml/test/testdevnetif.xml
new file mode 100644
index 0000000000..89e01a611f
--- /dev/null
+++ b/examples/xml/test/testdevnetif.xml
@@ -0,0 +1,6 @@
+<interface type='network'>
+ <source network='testbrigde' />
+ <mac address='00:16:3e:5d:c7:26' />
+ <model type='virtio' />
+ <alias name='ua-testNIC' />
+</interface>
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/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..a529505543 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,44 @@
<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>
</devices>
</domain>
diff --git a/examples/xml/test/testnodeinline.xml b/examples/xml/test/testnodeinline.xml
index 60970145a0..c7b939cdff 100644
--- a/examples/xml/test/testnodeinline.xml
+++ b/examples/xml/test/testnodeinline.xml
@@ -106,6 +106,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'/>
@@ -113,6 +119,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'>
@@ -124,6 +131,45 @@
<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>
</devices>
</domain>
<network>
--
2.32.0
7:18 a.m.
New subject: [PATCH 4/5] examples: xml: test: add xml for testing devices related APIs
On Mon, Aug 16, 2021 at 07:19:48PM +0800, Luke Yue wrote:
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
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 ++++
This one and ...
examples/xml/test/testdevinput.xml | 1 +
examples/xml/test/testdevlease.xml | 5 +++
examples/xml/test/testdevmem.xml | 6 ++++
examples/xml/test/testdevnetif.xml | 6 ++++
... this one only differ in MAC addresses, but the names suggest
something else. Since this commit does not use them anywhere it is
difficult to see if they are both useful or not.
Consider adding these in a commit where they are being used.
7:53 a.m.
New subject: [PATCH 4/5] examples: xml: test: add xml for testing devices related APIs
On Tue, 2021-08-17 at 14:18 +0200, Martin Kletzander wrote:
On Mon, Aug 16, 2021 at 07:19:48PM +0800, Luke Yue wrote:
> Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
> ---
> 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 ++++
This one and ...
> examples/xml/test/testdevinput.xml | 1 +
> examples/xml/test/testdevlease.xml | 5 +++
> examples/xml/test/testdevmem.xml | 6 ++++
> examples/xml/test/testdevnetif.xml | 6 ++++
... this one only differ in MAC addresses, but the names suggest
something else. Since this commit does not use them anywhere it is
difficult to see if they are both useful or not.
Consider adding these in a commit where they are being used.
Sorry for that, one of them is for attach APIs, and was added by
accident, will delete it in v2.
6:19 a.m.
New subject: [PATCH 5/5] tests: Test detach-device and detach-device-alias for test driver
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
tests/virshtest.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/tests/virshtest.c b/tests/virshtest.c
index 53db2aa19a..273596d636 100644
--- a/tests/virshtest.c
+++ b/tests/virshtest.c
@@ -291,6 +291,22 @@ static int testCompareDomControlInfoByName(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",
+ "../examples/xml/test/testdevif.xml", NULL };
+ const char *exp = "Device detached successfully\n\n";
+ return testCompareOutputLit(exp, NULL, argv);
+}
+
+static int testCompareDetachDeviceAlias(const void *data G_GNUC_UNUSED)
+{
+ const char *const argv[] = { VIRSH_CUSTOM, "detach-device-alias",
"fc5",
+ "ua-testCD", NULL };
+ const char *exp = "Device detach request sent successfully\n\n";
+ return testCompareOutputLit(exp, NULL, argv);
+}
+
struct testInfo {
const char *const *argv;
const char *result;
@@ -383,6 +399,14 @@ mymain(void)
testCompareDomControlInfoByName, NULL) != 0)
ret = -1;
+ if (virTestRun("virsh detach-device",
+ testCompareDetachDevice, 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.32.0
7:22 a.m.
New subject: [PATCH 5/5] tests: Test detach-device and detach-device-alias for test driver
On Mon, Aug 16, 2021 at 07:19:49PM +0800, Luke Yue wrote:
Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
---
tests/virshtest.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/tests/virshtest.c b/tests/virshtest.c
index 53db2aa19a..273596d636 100644
--- a/tests/virshtest.c
+++ b/tests/virshtest.c
@@ -291,6 +291,22 @@ static int testCompareDomControlInfoByName(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",
+ "../examples/xml/test/testdevif.xml", NULL };
+ const char *exp = "Device detached successfully\n\n";
+ return testCompareOutputLit(exp, NULL, argv);
+}
+
+static int testCompareDetachDeviceAlias(const void *data G_GNUC_UNUSED)
+{
+ const char *const argv[] = { VIRSH_CUSTOM, "detach-device-alias",
"fc5",
+ "ua-testCD", NULL };
+ const char *exp = "Device detach request sent successfully\n\n";
+ return testCompareOutputLit(exp, NULL, argv);
+}
+
Not many XMLs from the previous patch are used here. There could also
be some negative tests (failing on a device that does not exist for
example).
But if you de-duplicate the code then you can also go the other route
and test the generic functions without running 'virsh -c test:///...'
for every piece of functionality.
struct testInfo {
const char *const *argv;
const char *result;
@@ -383,6 +399,14 @@ mymain(void)
testCompareDomControlInfoByName, NULL) != 0)
ret = -1;
+ if (virTestRun("virsh detach-device",
+ testCompareDetachDevice, 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.32.0
8:01 a.m.
New subject: [PATCH 5/5] tests: Test detach-device and detach-device-alias for test driver
On Tue, 2021-08-17 at 14:22 +0200, Martin Kletzander wrote:
On Mon, Aug 16, 2021 at 07:19:49PM +0800, Luke Yue wrote:
> Signed-off-by: Luke Yue <lukedyue(a)gmail.com>
> ---
> tests/virshtest.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/tests/virshtest.c b/tests/virshtest.c
> index 53db2aa19a..273596d636 100644
> --- a/tests/virshtest.c
> +++ b/tests/virshtest.c
> @@ -291,6 +291,22 @@ static int
> testCompareDomControlInfoByName(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",
> +
> "../examples/xml/test/testdevif.xml", NULL };
> + const char *exp = "Device detached successfully\n\n";
> + return testCompareOutputLit(exp, NULL, argv);
> +}
> +
> +static int testCompareDetachDeviceAlias(const void *data
> G_GNUC_UNUSED)
> +{
> + const char *const argv[] = { VIRSH_CUSTOM, "detach-device-
> alias", "fc5",
> + "ua-testCD", NULL };
> + const char *exp = "Device detach request sent
> successfully\n\n";
> + return testCompareOutputLit(exp, NULL, argv);
> +}
> +
Not many XMLs from the previous patch are used here. There could
also
be some negative tests (failing on a device that does not exist for
example).
But if you de-duplicate the code then you can also go the other route
and test the generic functions without running 'virsh -c test:///...'
for every piece of functionality.
No problem, will reimplement and and more tests in v2.
> struct testInfo {
> const char *const *argv;
> const char *result;
> @@ -383,6 +399,14 @@ mymain(void)
> testCompareDomControlInfoByName, NULL) != 0)
> ret = -1;
>
> + if (virTestRun("virsh detach-device",
> + testCompareDetachDevice, 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.32.0
>
1193
days inactive
1194
days old
12 comments
2 participants
participants (2)
-
Luke Yue -
Martin Kletzander