10:28 a.m.
On 9/17/19 12:26 PM, Ilias Stamatis wrote:
On Tue, Sep 17, 2019 at 5:51 PM Daniel Henrique Barboza
<danielhb413(a)gmail.com> wrote:
>
>
> On 8/1/19 9:54 AM, Ilias Stamatis wrote:
>> Signed-off-by: Ilias Stamatis <stamatis.iliass(a)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(a)redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list