[libvirt] [PATCH v2 0/6] test_driver: implement device attach/detach APIs

This series needed a respin since there were conflicts when trying to apply on the current master. Ilias Stamatis (6): test_driver: implement virDomainAttachDeviceFlags test_driver: implement virDomainAttachDevice test_driver: implement virDomainDetachDeviceFlags test_driver: implement virDomainDetachDevice test_driver: implement virDomainDetachDeviceAlias test_driver: implement virDomainUpdateDeviceFlags src/test/test_driver.c | 569 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 569 insertions(+) -- 2.22.0

Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com> --- src/test/test_driver.c | 290 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 290 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index c39eef2d4b..5f28e9017f 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4646,6 +4646,295 @@ testDomainFSTrim(virDomainPtr dom, } +static int +testDomainAttachDeviceLiveAndConfig(virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr disk; + virDomainNetDefPtr net; + virDomainHostdevDefPtr hostdev; + virDomainControllerDefPtr controller; + virDomainHostdevDefPtr found; + virDomainLeaseDefPtr lease; + virDomainFSDefPtr fs; + virDomainRedirdevDefPtr redirdev; + virDomainShmemDefPtr shmem; + char mac[VIR_MAC_STRING_BUFLEN]; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + if (virDomainDiskIndexByName(vmdef, disk->dst, true) >= 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("target %s already exists."), disk->dst); + return -1; + } + + if (virDomainDiskInsert(vmdef, disk) < 0) + return -1; + + dev->data.disk = NULL; + break; + + case VIR_DOMAIN_DEVICE_CONTROLLER: + controller = dev->data.controller; + if (controller->idx != -1 && + virDomainControllerFind(vmdef, controller->type, + controller->idx) >= 0) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Target already exists")); + return -1; + } + + if (virDomainControllerInsert(vmdef, controller) < 0) + return -1; + + dev->data.controller = NULL; + break; + + case VIR_DOMAIN_DEVICE_NET: + net = dev->data.net; + if (virDomainHasNet(vmdef, net)) { + virReportError(VIR_ERR_INVALID_ARG, + _("network device with mac %s already exists"), + virMacAddrFormat(&net->mac, mac)); + return -1; + } + + if (virDomainNetInsert(vmdef, net)) + return -1; + + dev->data.net = NULL; + break; + + case VIR_DOMAIN_DEVICE_HOSTDEV: + hostdev = dev->data.hostdev; + if (virDomainHostdevFind(vmdef, hostdev, &found) >= 0) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("device is already in the domain configuration")); + return -1; + } + + if (virDomainHostdevInsert(vmdef, hostdev) < 0) + return -1; + + dev->data.hostdev = NULL; + break; + + case VIR_DOMAIN_DEVICE_LEASE: + lease = dev->data.lease; + if (virDomainLeaseIndex(vmdef, lease) >= 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("Lease %s in lockspace %s already exists"), + lease->key, NULLSTR(lease->lockspace)); + return -1; + } + + if (virDomainLeaseInsert(vmdef, lease) < 0) + return -1; + + dev->data.lease = NULL; + break; + + case VIR_DOMAIN_DEVICE_FS: + fs = dev->data.fs; + if (virDomainFSIndexByName(vmdef, fs->dst) >= 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("Target already exists")); + return -1; + } + + if (virDomainFSInsert(vmdef, fs) < 0) + return -1; + + dev->data.fs = NULL; + break; + + case VIR_DOMAIN_DEVICE_RNG: + if (dev->data.rng->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + virDomainDefHasDeviceAddress(vmdef, &dev->data.rng->info)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("a device with the same address already exists ")); + return -1; + } + + if (VIR_APPEND_ELEMENT(vmdef->rngs, vmdef->nrngs, dev->data.rng) < 0) + return -1; + + dev->data.rng = NULL; + break; + + case VIR_DOMAIN_DEVICE_MEMORY: + if (vmdef->nmems == vmdef->mem.memory_slots) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("no free memory device slot available")); + return -1; + } + + vmdef->mem.cur_balloon += dev->data.memory->size; + if (virDomainMemoryInsert(vmdef, dev->data.memory) < 0) + return -1; + + dev->data.memory = NULL; + break; + + case VIR_DOMAIN_DEVICE_REDIRDEV: + redirdev = dev->data.redirdev; + + if (VIR_APPEND_ELEMENT(vmdef->redirdevs, vmdef->nredirdevs, redirdev) < 0) + return -1; + + dev->data.redirdev = NULL; + break; + + case VIR_DOMAIN_DEVICE_SHMEM: + shmem = dev->data.shmem; + if (virDomainShmemDefFind(vmdef, shmem) >= 0) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("device is already in the domain configuration")); + return -1; + } + + if (virDomainShmemDefInsert(vmdef, shmem) < 0) + return -1; + + dev->data.shmem = NULL; + break; + + case VIR_DOMAIN_DEVICE_WATCHDOG: + if (vmdef->watchdog) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain already has a watchdog")); + return -1; + } + VIR_STEAL_PTR(vmdef->watchdog, dev->data.watchdog); + break; + + case VIR_DOMAIN_DEVICE_INPUT: + if (VIR_APPEND_ELEMENT(vmdef->inputs, vmdef->ninputs, dev->data.input) < 0) + return -1; + break; + + case VIR_DOMAIN_DEVICE_VSOCK: + if (vmdef->vsock) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain already has a vsock device")); + return -1; + } + VIR_STEAL_PTR(vmdef->vsock, dev->data.vsock); + break; + + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("persistent attach of device is not supported")); + return -1; + } + + return 0; +} + + +typedef enum { + TEST_DEVICE_ATTACH = 0, + TEST_DEVICE_DETACH, + TEST_DEVICE_UPDATE, +} virTestDeviceOperation; + + +static int +testDomainDeviceOperation(testDriverPtr driver, + virTestDeviceOperation operation, + const char *xml, + const char *alias, + virDomainDefPtr def) +{ + virDomainDeviceDefPtr dev = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; + int ret = -1; + + if (operation == TEST_DEVICE_DETACH) + parse_flags |= VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE; + + if (xml) { + if (!(dev = virDomainDeviceDefParse(xml, def, + driver->caps, driver->xmlopt, + NULL, parse_flags))) + goto cleanup; + } else if (alias) { + if (VIR_ALLOC(dev) < 0 || virDomainDefFindDevice(def, alias, dev, true) < 0) + goto cleanup; + } + + switch (operation) { + case TEST_DEVICE_ATTACH: + if (testDomainAttachDeviceLiveAndConfig(def, dev) < 0) + goto cleanup; + break; + case TEST_DEVICE_DETACH: + break; + case TEST_DEVICE_UPDATE: + break; + } + + ret = 0; + cleanup: + if (xml) + virDomainDeviceDefFree(dev); + else + VIR_FREE(dev); + return ret; +} + + +static int +testDomainAttachDetachUpdateDevice(virDomainPtr dom, + virTestDeviceOperation operation, + const char *xml, + const char *alias, + unsigned int flags) +{ + testDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + virDomainDefPtr def; + virDomainDefPtr persistentDef; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + if (!(vm = testDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) + goto cleanup; + + if (persistentDef) { + if (testDomainDeviceOperation(driver, operation, xml, alias, persistentDef) < 0) + goto cleanup; + } + + if (def) { + if (testDomainDeviceOperation(driver, operation, xml, alias, def) < 0) + goto cleanup; + } + + ret = 0; + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + + +static int +testDomainAttachDeviceFlags(virDomainPtr dom, + const char *xml, + unsigned int flags) +{ + return testDomainAttachDetachUpdateDevice(dom, TEST_DEVICE_ATTACH, + xml, NULL, flags); +} + + static int testDomainGetAutostart(virDomainPtr domain, int *autostart) { @@ -9450,6 +9739,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainFSFreeze = testDomainFSFreeze, /* 5.7.0 */ .domainFSThaw = testDomainFSThaw, /* 5.7.0 */ .domainFSTrim = testDomainFSTrim, /* 5.7.0 */ + .domainAttachDeviceFlags = testDomainAttachDeviceFlags, /* 5.7.0 */ .domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */ .domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */ .domainGetDiskErrors = testDomainGetDiskErrors, /* 5.4.0 */ -- 2.22.0

...
+ + if (operation == TEST_DEVICE_DETACH) + parse_flags |= VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
^This should be added by patch 3/5
+ + if (xml) { + if (!(dev = virDomainDeviceDefParse(xml, def, + driver->caps, driver->xmlopt, + NULL, parse_flags))) + goto cleanup; + } else if (alias) { + if (VIR_ALLOC(dev) < 0 || virDomainDefFindDevice(def, alias, dev, true) < 0) + goto cleanup; + } + + switch (operation) { + case TEST_DEVICE_ATTACH: + if (testDomainAttachDeviceLiveAndConfig(def, dev) < 0) + goto cleanup; + break; + case TEST_DEVICE_DETACH: + break; + case TEST_DEVICE_UPDATE: + break; + } + + ret = 0; + cleanup: + if (xml) + virDomainDeviceDefFree(dev); + else + VIR_FREE(dev);
virDomainDeviceDefFree() can handle both cases. Reviewed-by: Erik Skultety <eskultet@redhat.com>

On Fri, Aug 16, 2019 at 5:39 PM Erik Skultety <eskultet@redhat.com> wrote:
...
+ + if (operation == TEST_DEVICE_DETACH) + parse_flags |= VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
^This should be added by patch 3/5
+ + if (xml) { + if (!(dev = virDomainDeviceDefParse(xml, def, + driver->caps, driver->xmlopt, + NULL, parse_flags))) + goto cleanup; + } else if (alias) { + if (VIR_ALLOC(dev) < 0 || virDomainDefFindDevice(def, alias, dev, true) < 0) + goto cleanup; + } + + switch (operation) { + case TEST_DEVICE_ATTACH: + if (testDomainAttachDeviceLiveAndConfig(def, dev) < 0) + goto cleanup; + break; + case TEST_DEVICE_DETACH: + break; + case TEST_DEVICE_UPDATE: + break; + } + + ret = 0; + cleanup: + if (xml) + virDomainDeviceDefFree(dev); + else + VIR_FREE(dev);
virDomainDeviceDefFree() can handle both cases.
It cannot! This got me as well and made me wonder! Try attaching a device with an alias and then try detaching it with virDomainDetachDeviceAlias and use virDomainDeviceDefFree to free the resource. The program crashes with: free(): double free detected in tcache 2 Ilias
Reviewed-by: Erik Skultety <eskultet@redhat.com>

On Fri, Aug 16, 2019 at 05:57:36PM +0300, Ilias Stamatis wrote:
On Fri, Aug 16, 2019 at 5:39 PM Erik Skultety <eskultet@redhat.com> wrote:
...
+ + if (operation == TEST_DEVICE_DETACH) + parse_flags |= VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
^This should be added by patch 3/5
+ + if (xml) { + if (!(dev = virDomainDeviceDefParse(xml, def, + driver->caps, driver->xmlopt, + NULL, parse_flags))) + goto cleanup; + } else if (alias) { + if (VIR_ALLOC(dev) < 0 || virDomainDefFindDevice(def, alias, dev, true) < 0) + goto cleanup; + } + + switch (operation) { + case TEST_DEVICE_ATTACH: + if (testDomainAttachDeviceLiveAndConfig(def, dev) < 0) + goto cleanup; + break; + case TEST_DEVICE_DETACH: + break; + case TEST_DEVICE_UPDATE: + break; + } + + ret = 0; + cleanup: + if (xml) + virDomainDeviceDefFree(dev); + else + VIR_FREE(dev);
virDomainDeviceDefFree() can handle both cases.
It cannot! This got me as well and made me wonder!
Try attaching a device with an alias and then try detaching it with virDomainDetachDeviceAlias and use virDomainDeviceDefFree to free the resource.
The program crashes with: free(): double free detected in tcache 2
Hmm, I'll have a look at why that is, probably on Sunday, I'll go ahead and perform the other changes on my branch, but I won't merge the series yet. Regards, Erik

Signed-off-by: Ilias Stamatis <stamatis.iliass@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 5f28e9017f..abf80b97cf 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4935,6 +4935,14 @@ testDomainAttachDeviceFlags(virDomainPtr dom, } +static int +testDomainAttachDevice(virDomainPtr dom, + const char *xml) +{ + return testDomainAttachDeviceFlags(dom, xml, VIR_DOMAIN_AFFECT_LIVE); +} + + static int testDomainGetAutostart(virDomainPtr domain, int *autostart) { @@ -9739,6 +9747,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainFSFreeze = testDomainFSFreeze, /* 5.7.0 */ .domainFSThaw = testDomainFSThaw, /* 5.7.0 */ .domainFSTrim = testDomainFSTrim, /* 5.7.0 */ + .domainAttachDevice = testDomainAttachDevice, /* 5.7.0 */ .domainAttachDeviceFlags = testDomainAttachDeviceFlags, /* 5.7.0 */ .domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */ .domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */ -- 2.22.0

On Wed, Aug 14, 2019 at 07:47:06PM +0300, Ilias Stamatis wrote:
Signed-off-by: Ilias Stamatis <stamatis.iliass@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 5f28e9017f..abf80b97cf 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4935,6 +4935,14 @@ testDomainAttachDeviceFlags(virDomainPtr dom, }
+static int +testDomainAttachDevice(virDomainPtr dom, + const char *xml) +{ + return testDomainAttachDeviceFlags(dom, xml, VIR_DOMAIN_AFFECT_LIVE); +} + + static int testDomainGetAutostart(virDomainPtr domain, int *autostart) { @@ -9739,6 +9747,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainFSFreeze = testDomainFSFreeze, /* 5.7.0 */ .domainFSThaw = testDomainFSThaw, /* 5.7.0 */ .domainFSTrim = testDomainFSTrim, /* 5.7.0 */ + .domainAttachDevice = testDomainAttachDevice, /* 5.7.0 */ .domainAttachDeviceFlags = testDomainAttachDeviceFlags, /* 5.7.0 */ .domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */ .domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */
Reviewed-by: Erik Skultety <eskultet@redhat.com>

Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com> --- src/test/test_driver.c | 178 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 178 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index abf80b97cf..ff9693ccb7 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4834,6 +4834,171 @@ testDomainAttachDeviceLiveAndConfig(virDomainDefPtr vmdef, } +static int +testDomainDetachDeviceLiveAndConfig(virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr disk, detach; + virDomainHostdevDefPtr hostdev, det_hostdev; + virDomainControllerDefPtr cont, det_cont; + virDomainNetDefPtr net; + virDomainLeaseDefPtr lease, det_lease; + virDomainFSDefPtr fs; + virDomainMemoryDefPtr mem; + 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); + 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")); + 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) + 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")); + 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_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; + } + 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; + + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("persistent detach of device is not supported")); + return -1; + } + + return 0; +} + + typedef enum { TEST_DEVICE_ATTACH = 0, TEST_DEVICE_DETACH, @@ -4871,6 +5036,8 @@ testDomainDeviceOperation(testDriverPtr driver, goto cleanup; break; case TEST_DEVICE_DETACH: + if (testDomainDetachDeviceLiveAndConfig(def, dev) < 0) + goto cleanup; break; case TEST_DEVICE_UPDATE: break; @@ -4943,6 +5110,16 @@ testDomainAttachDevice(virDomainPtr dom, } +static int +testDomainDetachDeviceFlags(virDomainPtr dom, + const char *xml, + unsigned int flags) +{ + return testDomainAttachDetachUpdateDevice(dom, TEST_DEVICE_DETACH, + xml, NULL, flags); +} + + static int testDomainGetAutostart(virDomainPtr domain, int *autostart) { @@ -9749,6 +9926,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainFSTrim = testDomainFSTrim, /* 5.7.0 */ .domainAttachDevice = testDomainAttachDevice, /* 5.7.0 */ .domainAttachDeviceFlags = testDomainAttachDeviceFlags, /* 5.7.0 */ + .domainDetachDeviceFlags = testDomainDetachDeviceFlags, /* 5.7.0 */ .domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */ .domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */ .domainGetDiskErrors = testDomainGetDiskErrors, /* 5.4.0 */ -- 2.22.0

Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com> --- src/test/test_driver.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index ff9693ccb7..1152ca5bed 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5119,6 +5119,13 @@ testDomainDetachDeviceFlags(virDomainPtr dom, xml, NULL, flags); } +static int +testDomainDetachDevice(virDomainPtr dom, + const char *xml) +{ + return testDomainDetachDeviceFlags(dom, xml, VIR_DOMAIN_AFFECT_LIVE); +} + static int testDomainGetAutostart(virDomainPtr domain, int *autostart) @@ -9926,6 +9933,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainFSTrim = testDomainFSTrim, /* 5.7.0 */ .domainAttachDevice = testDomainAttachDevice, /* 5.7.0 */ .domainAttachDeviceFlags = testDomainAttachDeviceFlags, /* 5.7.0 */ + .domainDetachDevice = testDomainDetachDevice, /* 5.7.0 */ .domainDetachDeviceFlags = testDomainDetachDeviceFlags, /* 5.7.0 */ .domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */ .domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */ -- 2.22.0

On Wed, Aug 14, 2019 at 07:47:08PM +0300, Ilias Stamatis wrote:
Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com> --- src/test/test_driver.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index ff9693ccb7..1152ca5bed 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5119,6 +5119,13 @@ testDomainDetachDeviceFlags(virDomainPtr dom, xml, NULL, flags); }
+static int +testDomainDetachDevice(virDomainPtr dom, + const char *xml) +{ + return testDomainDetachDeviceFlags(dom, xml, VIR_DOMAIN_AFFECT_LIVE); +} +
static int testDomainGetAutostart(virDomainPtr domain, int *autostart) @@ -9926,6 +9933,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainFSTrim = testDomainFSTrim, /* 5.7.0 */ .domainAttachDevice = testDomainAttachDevice, /* 5.7.0 */ .domainAttachDeviceFlags = testDomainAttachDeviceFlags, /* 5.7.0 */ + .domainDetachDevice = testDomainDetachDevice, /* 5.7.0 */ .domainDetachDeviceFlags = testDomainDetachDeviceFlags, /* 5.7.0 */ .domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */ .domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */
Reviewed-by: Erik Skultety <eskultet@redhat.com>

Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com> --- src/test/test_driver.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 1152ca5bed..e271bc58f8 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5119,6 +5119,17 @@ testDomainDetachDeviceFlags(virDomainPtr dom, xml, NULL, flags); } + +static int +testDomainDetachDeviceAlias(virDomainPtr dom, + const char *alias, + unsigned int flags) +{ + return testDomainAttachDetachUpdateDevice(dom, TEST_DEVICE_DETACH, + NULL, alias, flags); +} + + static int testDomainDetachDevice(virDomainPtr dom, const char *xml) @@ -9935,6 +9946,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainAttachDeviceFlags = testDomainAttachDeviceFlags, /* 5.7.0 */ .domainDetachDevice = testDomainDetachDevice, /* 5.7.0 */ .domainDetachDeviceFlags = testDomainDetachDeviceFlags, /* 5.7.0 */ + .domainDetachDeviceAlias = testDomainDetachDeviceAlias, /* 5.7.0 */ .domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */ .domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */ .domainGetDiskErrors = testDomainGetDiskErrors, /* 5.4.0 */ -- 2.22.0

Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com> --- src/test/test_driver.c | 72 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index e271bc58f8..9bf3728654 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4999,6 +4999,65 @@ testDomainDetachDeviceLiveAndConfig(virDomainDefPtr vmdef, } +static int +testDomainUpdateDeviceLiveAndConfig(virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr newDisk; + virDomainDeviceDef oldDev = { .type = dev->type }; + virDomainNetDefPtr net; + int idx; + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + newDisk = dev->data.disk; + if ((idx = virDomainDiskIndexByName(vmdef, newDisk->dst, false)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("target %s doesn't exist."), newDisk->dst); + return -1; + } + + oldDev.data.disk = vmdef->disks[idx]; + if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev, + VIR_DOMAIN_DEVICE_ACTION_UPDATE, + false) < 0) + return -1; + + virDomainDiskDefFree(vmdef->disks[idx]); + vmdef->disks[idx] = newDisk; + dev->data.disk = NULL; + break; + + case VIR_DOMAIN_DEVICE_NET: + net = dev->data.net; + if ((idx = virDomainNetFindIdx(vmdef, net)) < 0) + goto cleanup; + + oldDev.data.net = vmdef->nets[idx]; + if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev, + VIR_DOMAIN_DEVICE_ACTION_UPDATE, + false) < 0) + return -1; + + virDomainNetDefFree(vmdef->nets[idx]); + vmdef->nets[idx] = net; + dev->data.net = NULL; + ret = 0; + break; + + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("neither persistent nor live update of device is supported")); + goto cleanup; + } + + ret = 0; + cleanup: + return ret; +} + + typedef enum { TEST_DEVICE_ATTACH = 0, TEST_DEVICE_DETACH, @@ -5040,6 +5099,8 @@ testDomainDeviceOperation(testDriverPtr driver, goto cleanup; break; case TEST_DEVICE_UPDATE: + if (testDomainUpdateDeviceLiveAndConfig(def, dev) < 0) + goto cleanup; break; } @@ -5138,6 +5199,16 @@ testDomainDetachDevice(virDomainPtr dom, } +static int +testDomainUpdateDeviceFlags(virDomainPtr dom, + const char *xml, + unsigned int flags) +{ + return testDomainAttachDetachUpdateDevice(dom, TEST_DEVICE_UPDATE, + xml, NULL, flags); +} + + static int testDomainGetAutostart(virDomainPtr domain, int *autostart) { @@ -9947,6 +10018,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainDetachDevice = testDomainDetachDevice, /* 5.7.0 */ .domainDetachDeviceFlags = testDomainDetachDeviceFlags, /* 5.7.0 */ .domainDetachDeviceAlias = testDomainDetachDeviceAlias, /* 5.7.0 */ + .domainUpdateDeviceFlags = testDomainUpdateDeviceFlags, /* 5.7.0 */ .domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */ .domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */ .domainGetDiskErrors = testDomainGetDiskErrors, /* 5.4.0 */ -- 2.22.0

On Wed, Aug 14, 2019 at 07:47:10PM +0300, Ilias Stamatis wrote:
Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com> --- src/test/test_driver.c | 72 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index e271bc58f8..9bf3728654 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4999,6 +4999,65 @@ testDomainDetachDeviceLiveAndConfig(virDomainDefPtr vmdef, }
+static int +testDomainUpdateDeviceLiveAndConfig(virDomainDefPtr vmdef, + virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr newDisk; + virDomainDeviceDef oldDev = { .type = dev->type }; + virDomainNetDefPtr net; + int idx; + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + newDisk = dev->data.disk; + if ((idx = virDomainDiskIndexByName(vmdef, newDisk->dst, false)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("target %s doesn't exist."), newDisk->dst); + return -1; + } + + oldDev.data.disk = vmdef->disks[idx]; + if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev, + VIR_DOMAIN_DEVICE_ACTION_UPDATE, + false) < 0) + return -1; + + virDomainDiskDefFree(vmdef->disks[idx]); + vmdef->disks[idx] = newDisk; + dev->data.disk = NULL; + break; + + case VIR_DOMAIN_DEVICE_NET: + net = dev->data.net; + if ((idx = virDomainNetFindIdx(vmdef, net)) < 0) + goto cleanup; + + oldDev.data.net = vmdef->nets[idx]; + if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev, + VIR_DOMAIN_DEVICE_ACTION_UPDATE, + false) < 0) + return -1; + + virDomainNetDefFree(vmdef->nets[idx]); + vmdef->nets[idx] = net; + dev->data.net = NULL; + ret = 0; + break; + + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("neither persistent nor live update of device is supported")); + goto cleanup;
return -1;
+ } + + ret = 0; + cleanup:
drop the cleanup label Reviewed-by: Erik Skultety <eskultet@redhat.com>
participants (2)
-
Erik Skultety
-
Ilias Stamatis