[libvirt] [PATCH 0/6] test_driver: implement device attach/detach APIs
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 | 570 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 570 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 aae9875194..c8aad6a0bb 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4013,6 +4013,295 @@ static int testDomainUndefine(virDomainPtr domain) return testDomainUndefineFlags(domain, 0); } + +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, + 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) { @@ -8659,6 +8948,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainDefineXMLFlags = testDomainDefineXMLFlags, /* 1.2.12 */ .domainUndefine = testDomainUndefine, /* 0.1.11 */ .domainUndefineFlags = testDomainUndefineFlags, /* 0.9.4 */ + .domainAttachDeviceFlags = testDomainAttachDeviceFlags, /* 5.7.0 */ .domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */ .domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */ .domainGetDiskErrors = testDomainGetDiskErrors, /* 5.4.0 */ -- 2.22.0
On 8/1/19 9:54 AM, Ilias Stamatis wrote:
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 aae9875194..c8aad6a0bb 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4013,6 +4013,295 @@ static int testDomainUndefine(virDomainPtr domain) return testDomainUndefineFlags(domain, 0); }
+ +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, + parse_flags)))
This API now has an extra parameter "parseOpaque" right after the parse_flags. Using NULL in this parameter can be a way out, assuming nothing breaks of course.
+ 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); +} +
Compiler wasn't happy with this function: test/test_driver.c:4767:1: error: 'testDomainAttachDeviceFlags' defined but not used [-Werror=unused-function] 4767 | testDomainAttachDeviceFlags(virDomainPtr dom, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors Since you're using it in patch 02 you can move it there to avoid this error. Problem is, doing that, you'll get the same error in testDomainAttachDetachUpdateDevice. If you erase this one too you'll have the warning with testDomainDeviceOperation. And then, with testDomainAttachDeviceLiveAndConfig. Basically this first patch introduces static functions that aren't being used anywhere else, thus the compiler will not play ball. A quick solution is to merge this patch with patch 02. Thanks, DHB
static int testDomainGetAutostart(virDomainPtr domain, int *autostart) { @@ -8659,6 +8948,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainDefineXMLFlags = testDomainDefineXMLFlags, /* 1.2.12 */ .domainUndefine = testDomainUndefine, /* 0.1.11 */ .domainUndefineFlags = testDomainUndefineFlags, /* 0.9.4 */ + .domainAttachDeviceFlags = testDomainAttachDeviceFlags, /* 5.7.0 */ .domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */ .domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */ .domainGetDiskErrors = testDomainGetDiskErrors, /* 5.4.0 */ -- 2.22.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Sep 17, 2019 at 5:51 PM Daniel Henrique Barboza <danielhb413@gmail.com> wrote:
On 8/1/19 9:54 AM, Ilias Stamatis wrote:
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 aae9875194..c8aad6a0bb 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4013,6 +4013,295 @@ static int testDomainUndefine(virDomainPtr domain) return testDomainUndefineFlags(domain, 0); }
+ +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, + parse_flags)))
This API now has an extra parameter "parseOpaque" right after the parse_flags.
Using NULL in this parameter can be a way out, assuming nothing breaks of course.
Hello, There's a v2 of this series that fixes that IIRC: https://www.redhat.com/archives/libvir-list/2019-August/msg00535.html
+ 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); +} +
Compiler wasn't happy with this function:
test/test_driver.c:4767:1: error: 'testDomainAttachDeviceFlags' defined but not used [-Werror=unused-function] 4767 | testDomainAttachDeviceFlags(virDomainPtr dom, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors
Since you're using it in patch 02 you can move it there to avoid this error.
That's weird since it's used below as far as I can see.
Problem is, doing that, you'll get the same error in testDomainAttachDetachUpdateDevice. If you erase this one too you'll have the warning with testDomainDeviceOperation. And then, with testDomainAttachDeviceLiveAndConfig.
Basically this first patch introduces static functions that aren't being used anywhere else, thus the compiler will not play ball. A quick solution is to merge this patch with patch 02.
Thanks,
DHB
static int testDomainGetAutostart(virDomainPtr domain, int *autostart) { @@ -8659,6 +8948,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainDefineXMLFlags = testDomainDefineXMLFlags, /* 1.2.12 */ .domainUndefine = testDomainUndefine, /* 0.1.11 */ .domainUndefineFlags = testDomainUndefineFlags, /* 0.9.4 */ + .domainAttachDeviceFlags = testDomainAttachDeviceFlags, /* 5.7.0 */
It is used here. Thanks, Ilias
.domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */ .domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */ .domainGetDiskErrors = testDomainGetDiskErrors, /* 5.4.0 */ -- 2.22.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 9/17/19 12:26 PM, Ilias Stamatis wrote:
On Tue, Sep 17, 2019 at 5:51 PM Daniel Henrique Barboza <danielhb413@gmail.com> wrote:
On 8/1/19 9:54 AM, Ilias Stamatis wrote:
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 aae9875194..c8aad6a0bb 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4013,6 +4013,295 @@ static int testDomainUndefine(virDomainPtr domain) return testDomainUndefineFlags(domain, 0); }
+ +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, + parse_flags)))
This API now has an extra parameter "parseOpaque" right after the parse_flags.
Using NULL in this parameter can be a way out, assuming nothing breaks of course.
Hello,
There's a v2 of this series that fixes that IIRC: https://www.redhat.com/archives/libvir-list/2019-August/msg00535.html
Didn't notice it. Thanks for pointing it out. I'll have a look. DHB
+ 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); +} + Compiler wasn't happy with this function:
test/test_driver.c:4767:1: error: 'testDomainAttachDeviceFlags' defined but not used [-Werror=unused-function] 4767 | testDomainAttachDeviceFlags(virDomainPtr dom, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors
Since you're using it in patch 02 you can move it there to avoid this error. That's weird since it's used below as far as I can see.
Problem is, doing that, you'll get the same error in testDomainAttachDetachUpdateDevice. If you erase this one too you'll have the warning with testDomainDeviceOperation. And then, with testDomainAttachDeviceLiveAndConfig.
Basically this first patch introduces static functions that aren't being used anywhere else, thus the compiler will not play ball. A quick solution is to merge this patch with patch 02.
Thanks,
DHB
static int testDomainGetAutostart(virDomainPtr domain, int *autostart) { @@ -8659,6 +8948,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainDefineXMLFlags = testDomainDefineXMLFlags, /* 1.2.12 */ .domainUndefine = testDomainUndefine, /* 0.1.11 */ .domainUndefineFlags = testDomainUndefineFlags, /* 0.9.4 */ + .domainAttachDeviceFlags = testDomainAttachDeviceFlags, /* 5.7.0 */ It is used here.
Thanks, Ilias
.domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */ .domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */ .domainGetDiskErrors = testDomainGetDiskErrors, /* 5.4.0 */ -- 2.22.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com> --- 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 c8aad6a0bb..784622985c 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4302,6 +4302,15 @@ testDomainAttachDeviceFlags(virDomainPtr dom, xml, NULL, flags); } + +static int +testDomainAttachDevice(virDomainPtr dom, + const char *xml) +{ + return testDomainAttachDeviceFlags(dom, xml, VIR_DOMAIN_AFFECT_LIVE); +} + + static int testDomainGetAutostart(virDomainPtr domain, int *autostart) { @@ -8948,6 +8957,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainDefineXMLFlags = testDomainDefineXMLFlags, /* 1.2.12 */ .domainUndefine = testDomainUndefine, /* 0.1.11 */ .domainUndefineFlags = testDomainUndefineFlags, /* 0.9.4 */ + .domainAttachDevice = testDomainAttachDevice, /* 5.7.0 */ .domainAttachDeviceFlags = testDomainAttachDeviceFlags, /* 5.7.0 */ .domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */ .domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */ -- 2.22.0
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 784622985c..1d371a5832 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4202,6 +4202,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, @@ -4239,6 +4404,8 @@ testDomainDeviceOperation(testDriverPtr driver, goto cleanup; break; case TEST_DEVICE_DETACH: + if (testDomainDetachDeviceLiveAndConfig(def, dev) < 0) + goto cleanup; break; case TEST_DEVICE_UPDATE: break; @@ -4311,6 +4478,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) { @@ -8959,6 +9136,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainUndefineFlags = testDomainUndefineFlags, /* 0.9.4 */ .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 1d371a5832..de449d89fb 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4487,6 +4487,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) @@ -9136,6 +9143,7 @@ static virHypervisorDriver testHypervisorDriver = { .domainUndefineFlags = testDomainUndefineFlags, /* 0.9.4 */ .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
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 de449d89fb..cffd60c75c 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4487,6 +4487,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) @@ -9145,6 +9156,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 cffd60c75c..228e24b0ae 100755 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4367,6 +4367,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, @@ -4408,6 +4467,8 @@ testDomainDeviceOperation(testDriverPtr driver, goto cleanup; break; case TEST_DEVICE_UPDATE: + if (testDomainUpdateDeviceLiveAndConfig(def, dev) < 0) + goto cleanup; break; } @@ -4506,6 +4567,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) { @@ -9157,6 +9228,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
Hi, I didn't review the whole series because I believe you need to look at merging patches 1 and 2 and seeing how to deal with the new API of virDomainDeviceDefParse. Since this will change the series a bit I'll wait for the newer version. Also, I think your file has different permissions than the one in git. Commit 49520e9e7d fixed the permissions back, but then these patches gives a warning when applied: warning: src/test/test_driver.c has type 100644, expected 100755 I recommend changing the permissions of your local file to match the one from the repo before re-sending this series. Thanks, DHB On 8/1/19 9:54 AM, Ilias Stamatis wrote:
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 | 570 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 570 insertions(+)
-- 2.22.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
Daniel Henrique Barboza -
Ilias Stamatis