[PATCH 00/11] qemuhotplugtest: Various cleanups and improvements

While working on something else, I've taken a look at qemuhotplugtest and realized we can do various cleanups and improvements. The test still covers just live attach/detach/update, but after this it's prepared better to test also config attach/detach/update. Michal Prívozník (11): qemu: Replace @dom argument with @driver in qemuDomainUpdateDeviceLive() qemu: Move qemuDomainAttachDeviceLive() into qemu_hotplug.c qemu: Move qemuDomainUpdateDeviceLive() into qemu_hotplug.c qemuhotplugtest: Call qemuDomainDetachDeviceLive() directly qemuhotplugtest: Call qemuDomainAttachDeviceLive() directly qemuhotplugtest: Call qemuDomainUpdateDeviceLive() directly qemu_hotplug.h: Expose less functions qemuhotplugtest: Fix misleading comment on monitor unlock qemuhotplugtest: Don't overwrite vm->def->id in testQemuHotplugCheckResult() qemuhotplugtest: use g_autoptr(virDomainDeviceDef) qemuhotplugtest: Verify domain XML on UPDATE src/qemu/qemu_driver.c | 472 +--------------- src/qemu/qemu_hotplug.c | 525 +++++++++++++++++- src/qemu/qemu_hotplug.h | 68 +-- tests/qemuhotplugtest.c | 197 +------ ...hotplug-disk-cdrom+disk-cdrom-nochange.xml | 1 + .../qemuhotplug-disk-cdrom.xml | 30 +- ...graphics-spice+graphics-spice-nochange.xml | 1 + ...graphics-spice-listen-network-password.xml | 71 +++ ...imeout+graphics-spice-timeout-nochange.xml | 1 + ...imeout+graphics-spice-timeout-password.xml | 117 ++++ .../qemuhotplug-graphics-spice-timeout.xml | 50 +- .../qemuhotplug-graphics-spice.xml | 41 +- 12 files changed, 819 insertions(+), 755 deletions(-) create mode 120000 tests/qemuhotplugtestdomains/qemuhotplug-disk-cdrom+disk-cdrom-nochange.xml create mode 120000 tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice+graphics-spice-nochange.xml create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-listen-network+graphics-spice-listen-network-password.xml create mode 120000 tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-timeout+graphics-spice-timeout-nochange.xml create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-timeout+graphics-spice-timeout-password.xml -- 2.39.2

The qemuDomainUpdateDeviceLive() accepts virDomainPtr as one of its arguments, but use it only to get QEMU driver out of it. Well, the only caller already done that and thus can pass it instead of virDomainPtr. This also makes it look like the rest of device hot(un-)plug functions: qemuDomainAttachDeviceLive() and qemuDomainUpdateDeviceLive(). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 523a83682c..48eb759531 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7070,10 +7070,9 @@ qemuDomainChangeMemoryLive(virQEMUDriver *driver G_GNUC_UNUSED, static int qemuDomainUpdateDeviceLive(virDomainObj *vm, virDomainDeviceDef *dev, - virDomainPtr dom, + virQEMUDriver *driver, bool force) { - virQEMUDriver *driver = dom->conn->privateData; virDomainDeviceDef oldDev = { .type = dev->type }; int idx; @@ -7939,7 +7938,7 @@ qemuDomainUpdateDeviceFlags(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_LIVE) { /* virDomainDefCompatibleDevice call is delayed until we know the * device we're going to update. */ - if ((ret = qemuDomainUpdateDeviceLive(vm, dev_live, dom, force)) < 0) + if ((ret = qemuDomainUpdateDeviceLive(vm, dev_live, driver, force)) < 0) goto endjob; qemuDomainSaveStatus(vm); -- 2.39.2

On Fri, Apr 21, 2023 at 10:25 AM Michal Privoznik <mprivozn@redhat.com> wrote:
The qemuDomainUpdateDeviceLive() accepts virDomainPtr as one of its arguments, but use it only to get QEMU driver out of it. Well, the only caller already done that and thus can pass it
*does
instead of virDomainPtr.
This also makes it look like the rest of device hot(un-)plug functions: qemuDomainAttachDeviceLive() and qemuDomainUpdateDeviceLive().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
Reviewed-by: Kristina Hanicova <khanicov@redhat.com> Kristina

There is no good reason for qemuDomainAttachDeviceLive() to live in (ever growing) qemu_driver.c while we have qemu_hotplug.c which already contains the rest of hotplug code. Move the function to its new home. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 170 ---------------------------------------- src/qemu/qemu_hotplug.c | 170 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.h | 4 + 3 files changed, 174 insertions(+), 170 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 48eb759531..fe1ddb5ade 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6671,176 +6671,6 @@ qemuDomainUndefine(virDomainPtr dom) return qemuDomainUndefineFlags(dom, 0); } -static int -qemuDomainAttachDeviceLive(virDomainObj *vm, - virDomainDeviceDef *dev, - virQEMUDriver *driver) -{ - int ret = -1; - const char *alias = NULL; - g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - struct qemuDomainPrepareChardevSourceData chardevBackendData = { .cfg = cfg, - .hotplug = true }; - - if (qemuDomainDeviceBackendChardevForeachOne(dev, - qemuDomainPrepareChardevSourceOne, - &chardevBackendData) < 0) - return -1; - - switch ((virDomainDeviceType)dev->type) { - case VIR_DOMAIN_DEVICE_DISK: - qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, NULL); - ret = qemuDomainAttachDeviceDiskLive(driver, vm, dev); - if (!ret) { - alias = dev->data.disk->info.alias; - dev->data.disk = NULL; - } - break; - - case VIR_DOMAIN_DEVICE_CONTROLLER: - ret = qemuDomainAttachControllerDevice(vm, dev->data.controller); - if (!ret) { - alias = dev->data.controller->info.alias; - dev->data.controller = NULL; - } - break; - - case VIR_DOMAIN_DEVICE_LEASE: - ret = qemuDomainAttachLease(driver, vm, - dev->data.lease); - if (ret == 0) - dev->data.lease = NULL; - break; - - case VIR_DOMAIN_DEVICE_NET: - qemuDomainObjCheckNetTaint(driver, vm, dev->data.net, NULL); - ret = qemuDomainAttachNetDevice(driver, vm, dev->data.net); - if (!ret) { - alias = dev->data.net->info.alias; - dev->data.net = NULL; - } - break; - - case VIR_DOMAIN_DEVICE_HOSTDEV: - qemuDomainObjCheckHostdevTaint(driver, vm, dev->data.hostdev, NULL); - ret = qemuDomainAttachHostDevice(driver, vm, - dev->data.hostdev); - if (!ret) { - alias = dev->data.hostdev->info->alias; - dev->data.hostdev = NULL; - } - break; - - case VIR_DOMAIN_DEVICE_REDIRDEV: - ret = qemuDomainAttachRedirdevDevice(driver, vm, - dev->data.redirdev); - if (!ret) { - alias = dev->data.redirdev->info.alias; - dev->data.redirdev = NULL; - } - break; - - case VIR_DOMAIN_DEVICE_CHR: - ret = qemuDomainAttachChrDevice(driver, vm, dev); - if (!ret) { - alias = dev->data.chr->info.alias; - dev->data.chr = NULL; - } - break; - - case VIR_DOMAIN_DEVICE_RNG: - ret = qemuDomainAttachRNGDevice(driver, vm, - dev->data.rng); - if (!ret) { - alias = dev->data.rng->info.alias; - dev->data.rng = NULL; - } - break; - - case VIR_DOMAIN_DEVICE_MEMORY: - /* note that qemuDomainAttachMemory always consumes dev->data.memory - * and dispatches DeviceAdded event on success */ - ret = qemuDomainAttachMemory(driver, vm, - dev->data.memory); - dev->data.memory = NULL; - break; - - case VIR_DOMAIN_DEVICE_SHMEM: - ret = qemuDomainAttachShmemDevice(vm, dev->data.shmem); - if (!ret) { - alias = dev->data.shmem->info.alias; - dev->data.shmem = NULL; - } - break; - - case VIR_DOMAIN_DEVICE_WATCHDOG: - ret = qemuDomainAttachWatchdog(vm, dev->data.watchdog); - if (!ret) { - alias = dev->data.watchdog->info.alias; - dev->data.watchdog = NULL; - } - break; - - case VIR_DOMAIN_DEVICE_INPUT: - ret = qemuDomainAttachInputDevice(vm, dev->data.input); - if (ret == 0) { - alias = dev->data.input->info.alias; - dev->data.input = NULL; - } - break; - - case VIR_DOMAIN_DEVICE_VSOCK: - ret = qemuDomainAttachVsockDevice(vm, dev->data.vsock); - if (ret == 0) { - alias = dev->data.vsock->info.alias; - dev->data.vsock = NULL; - } - break; - - case VIR_DOMAIN_DEVICE_FS: - ret = qemuDomainAttachFSDevice(driver, vm, dev->data.fs); - if (ret == 0) { - alias = dev->data.fs->info.alias; - dev->data.fs = NULL; - } - break; - - case VIR_DOMAIN_DEVICE_NONE: - case VIR_DOMAIN_DEVICE_SOUND: - 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_TPM: - case VIR_DOMAIN_DEVICE_PANIC: - case VIR_DOMAIN_DEVICE_IOMMU: - case VIR_DOMAIN_DEVICE_AUDIO: - case VIR_DOMAIN_DEVICE_CRYPTO: - case VIR_DOMAIN_DEVICE_LAST: - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("live attach of device '%1$s' is not supported"), - virDomainDeviceTypeToString(dev->type)); - break; - } - - if (alias) { - /* queue the event before the alias has a chance to get freed - * if the domain disappears while qemuDomainUpdateDeviceList - * is in monitor */ - virObjectEvent *event; - event = virDomainEventDeviceAddedNewFromObj(vm, alias); - virObjectEventStateQueue(driver->domainEventState, event); - } - - if (ret == 0) - ret = qemuDomainUpdateDeviceList(vm, VIR_ASYNC_JOB_NONE); - - return ret; -} - - static int qemuDomainChangeDiskLive(virDomainObj *vm, virDomainDeviceDef *dev, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 53a0874556..bd126234fd 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3308,6 +3308,176 @@ qemuDomainAttachLease(virQEMUDriver *driver, } +int +qemuDomainAttachDeviceLive(virDomainObj *vm, + virDomainDeviceDef *dev, + virQEMUDriver *driver) +{ + int ret = -1; + const char *alias = NULL; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + struct qemuDomainPrepareChardevSourceData chardevBackendData = { .cfg = cfg, + .hotplug = true }; + + if (qemuDomainDeviceBackendChardevForeachOne(dev, + qemuDomainPrepareChardevSourceOne, + &chardevBackendData) < 0) + return -1; + + switch ((virDomainDeviceType)dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, NULL); + ret = qemuDomainAttachDeviceDiskLive(driver, vm, dev); + if (!ret) { + alias = dev->data.disk->info.alias; + dev->data.disk = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_CONTROLLER: + ret = qemuDomainAttachControllerDevice(vm, dev->data.controller); + if (!ret) { + alias = dev->data.controller->info.alias; + dev->data.controller = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_LEASE: + ret = qemuDomainAttachLease(driver, vm, + dev->data.lease); + if (ret == 0) + dev->data.lease = NULL; + break; + + case VIR_DOMAIN_DEVICE_NET: + qemuDomainObjCheckNetTaint(driver, vm, dev->data.net, NULL); + ret = qemuDomainAttachNetDevice(driver, vm, dev->data.net); + if (!ret) { + alias = dev->data.net->info.alias; + dev->data.net = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_HOSTDEV: + qemuDomainObjCheckHostdevTaint(driver, vm, dev->data.hostdev, NULL); + ret = qemuDomainAttachHostDevice(driver, vm, + dev->data.hostdev); + if (!ret) { + alias = dev->data.hostdev->info->alias; + dev->data.hostdev = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_REDIRDEV: + ret = qemuDomainAttachRedirdevDevice(driver, vm, + dev->data.redirdev); + if (!ret) { + alias = dev->data.redirdev->info.alias; + dev->data.redirdev = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_CHR: + ret = qemuDomainAttachChrDevice(driver, vm, dev); + if (!ret) { + alias = dev->data.chr->info.alias; + dev->data.chr = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_RNG: + ret = qemuDomainAttachRNGDevice(driver, vm, + dev->data.rng); + if (!ret) { + alias = dev->data.rng->info.alias; + dev->data.rng = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_MEMORY: + /* note that qemuDomainAttachMemory always consumes dev->data.memory + * and dispatches DeviceAdded event on success */ + ret = qemuDomainAttachMemory(driver, vm, + dev->data.memory); + dev->data.memory = NULL; + break; + + case VIR_DOMAIN_DEVICE_SHMEM: + ret = qemuDomainAttachShmemDevice(vm, dev->data.shmem); + if (!ret) { + alias = dev->data.shmem->info.alias; + dev->data.shmem = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_WATCHDOG: + ret = qemuDomainAttachWatchdog(vm, dev->data.watchdog); + if (!ret) { + alias = dev->data.watchdog->info.alias; + dev->data.watchdog = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_INPUT: + ret = qemuDomainAttachInputDevice(vm, dev->data.input); + if (ret == 0) { + alias = dev->data.input->info.alias; + dev->data.input = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_VSOCK: + ret = qemuDomainAttachVsockDevice(vm, dev->data.vsock); + if (ret == 0) { + alias = dev->data.vsock->info.alias; + dev->data.vsock = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_FS: + ret = qemuDomainAttachFSDevice(driver, vm, dev->data.fs); + if (ret == 0) { + alias = dev->data.fs->info.alias; + dev->data.fs = NULL; + } + break; + + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_SOUND: + 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_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_IOMMU: + case VIR_DOMAIN_DEVICE_AUDIO: + case VIR_DOMAIN_DEVICE_CRYPTO: + case VIR_DOMAIN_DEVICE_LAST: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("live attach of device '%1$s' is not supported"), + virDomainDeviceTypeToString(dev->type)); + break; + } + + if (alias) { + /* queue the event before the alias has a chance to get freed + * if the domain disappears while qemuDomainUpdateDeviceList + * is in monitor */ + virObjectEvent *event; + event = virDomainEventDeviceAddedNewFromObj(vm, alias); + virObjectEventStateQueue(driver->domainEventState, event); + } + + if (ret == 0) + ret = qemuDomainUpdateDeviceList(vm, VIR_ASYNC_JOB_NONE); + + return ret; +} + + static int qemuDomainChangeNetBridge(virDomainObj *vm, virDomainNetDef *olddev, diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index cec1423ee0..74fc3bbd65 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -111,6 +111,10 @@ int qemuDomainAttachRNGDevice(virQEMUDriver *driver, virDomainObj *vm, virDomainRNGDef *rng); +int qemuDomainAttachDeviceLive(virDomainObj *vm, + virDomainDeviceDef *dev, + virQEMUDriver *driver); + int qemuDomainDetachDeviceLive(virDomainObj *vm, virDomainDeviceDef *match, virQEMUDriver *driver, -- 2.39.2

On Fri, Apr 21, 2023 at 10:25 AM Michal Privoznik <mprivozn@redhat.com> wrote:
There is no good reason for qemuDomainAttachDeviceLive() to live in (ever growing) qemu_driver.c while we have qemu_hotplug.c which already contains the rest of hotplug code. Move the function to its new home.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 170 ---------------------------------------- src/qemu/qemu_hotplug.c | 170 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.h | 4 + 3 files changed, 174 insertions(+), 170 deletions(-)
Reviewed-by: Kristina Hanicova <khanicov@redhat.com> Kristina

There is no good reason for qemuDomainUpdateDeviceLive() to live in (ever growing) qemu_driver.c while we have qemu_hotplug.c which already contains the rest of hotplug code. Move the function to its new home. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 299 ---------------------------------------- src/qemu/qemu_hotplug.c | 299 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.h | 5 + 3 files changed, 304 insertions(+), 299 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fe1ddb5ade..5ee15bab7a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6671,305 +6671,6 @@ qemuDomainUndefine(virDomainPtr dom) return qemuDomainUndefineFlags(dom, 0); } -static int -qemuDomainChangeDiskLive(virDomainObj *vm, - virDomainDeviceDef *dev, - virQEMUDriver *driver, - bool force) -{ - virDomainDiskDef *disk = dev->data.disk; - virDomainDiskDef *orig_disk = NULL; - virDomainStartupPolicy origStartupPolicy; - virDomainDeviceDef oldDev = { .type = dev->type }; - - if (!(orig_disk = virDomainDiskByTarget(vm->def, disk->dst))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("disk '%1$s' not found"), disk->dst); - return -1; - } - - oldDev.data.disk = orig_disk; - origStartupPolicy = orig_disk->startupPolicy; - if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev, - VIR_DOMAIN_DEVICE_ACTION_UPDATE, - true) < 0) - return -1; - - if (!qemuDomainDiskChangeSupported(disk, orig_disk)) - return -1; - - if (!virStorageSourceIsSameLocation(disk->src, orig_disk->src)) { - /* Disk source can be changed only for removable devices */ - if (disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM && - disk->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("disk source can be changed only in removable " - "drives")); - return -1; - } - - /* update startup policy first before updating disk image */ - orig_disk->startupPolicy = dev->data.disk->startupPolicy; - - if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk, - dev->data.disk->src, force) < 0) { - /* revert startup policy before failing */ - orig_disk->startupPolicy = origStartupPolicy; - return -1; - } - - dev->data.disk->src = NULL; - } - - /* in case when we aren't updating disk source we update startup policy here */ - orig_disk->startupPolicy = dev->data.disk->startupPolicy; - orig_disk->snapshot = dev->data.disk->snapshot; - - return 0; -} - - -static bool -qemuDomainChangeMemoryLiveValidateChange(const virDomainMemoryDef *oldDef, - const virDomainMemoryDef *newDef) -{ - /* The only thing that is allowed to change is 'requestedsize' for - * virtio-mem model. Check if user isn't trying to sneak in change for - * something else. */ - - switch (oldDef->model) { - case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: - break; - - case VIR_DOMAIN_MEMORY_MODEL_NONE: - case VIR_DOMAIN_MEMORY_MODEL_DIMM: - case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: - case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: - case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: - case VIR_DOMAIN_MEMORY_MODEL_LAST: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("cannot modify memory of model '%1$s'"), - virDomainMemoryModelTypeToString(oldDef->model)); - return false; - break; - } - - if (oldDef->model != newDef->model) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("cannot modify memory model from '%1$s' to '%2$s'"), - virDomainMemoryModelTypeToString(oldDef->model), - virDomainMemoryModelTypeToString(newDef->model)); - return false; - } - - if (oldDef->access != newDef->access) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("cannot modify memory access from '%1$s' to '%2$s'"), - virDomainMemoryAccessTypeToString(oldDef->access), - virDomainMemoryAccessTypeToString(newDef->access)); - return false; - } - - if (oldDef->discard != newDef->discard) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("cannot modify memory discard from '%1$s' to '%2$s'"), - virTristateBoolTypeToString(oldDef->discard), - virTristateBoolTypeToString(newDef->discard)); - return false; - } - - if (!virBitmapEqual(oldDef->sourceNodes, - newDef->sourceNodes)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("cannot modify memory source nodes")); - return false; - } - - if (oldDef->pagesize != newDef->pagesize) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("cannot modify memory pagesize from '%1$llu' to '%2$llu'"), - oldDef->pagesize, - newDef->pagesize); - return false; - } - - if (STRNEQ_NULLABLE(oldDef->nvdimmPath, newDef->nvdimmPath)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("cannot modify memory path from '%1$s' to '%2$s'"), - NULLSTR(oldDef->nvdimmPath), - NULLSTR(newDef->nvdimmPath)); - return false; - } - - if (oldDef->alignsize != newDef->alignsize) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("cannot modify memory align size from '%1$llu' to '%2$llu'"), - oldDef->alignsize, newDef->alignsize); - return false; - } - - if (oldDef->nvdimmPmem != newDef->nvdimmPmem) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("cannot modify memory pmem from '%1$d' to '%2$d'"), - oldDef->nvdimmPmem, newDef->nvdimmPmem); - return false; - } - - if (oldDef->targetNode != newDef->targetNode) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("cannot modify memory targetNode from '%1$d' to '%2$d'"), - oldDef->targetNode, newDef->targetNode); - return false; - } - - if (oldDef->size != newDef->size) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("cannot modify memory size from '%1$llu' to '%2$llu'"), - oldDef->size, newDef->size); - return false; - } - - if (oldDef->labelsize != newDef->labelsize) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("cannot modify memory label size from '%1$llu' to '%2$llu'"), - oldDef->labelsize, newDef->labelsize); - return false; - } - if (oldDef->blocksize != newDef->blocksize) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("cannot modify memory block size from '%1$llu' to '%2$llu'"), - oldDef->blocksize, newDef->blocksize); - return false; - } - - /* requestedsize can change */ - - if (oldDef->readonly != newDef->readonly) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("cannot modify memory pmem flag")); - return false; - } - - if ((oldDef->uuid || newDef->uuid) && - !(oldDef->uuid && newDef->uuid && - memcmp(oldDef->uuid, newDef->uuid, VIR_UUID_BUFLEN) == 0)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("cannot modify memory UUID")); - return false; - } - - return true; -} - - -static int -qemuDomainChangeMemoryLive(virQEMUDriver *driver G_GNUC_UNUSED, - virDomainObj *vm, - virDomainDeviceDef *dev) -{ - virDomainDeviceDef oldDev = { .type = VIR_DOMAIN_DEVICE_MEMORY }; - virDomainMemoryDef *newDef = dev->data.memory; - virDomainMemoryDef *oldDef = NULL; - - oldDef = virDomainMemoryFindByDeviceInfo(vm->def, &dev->data.memory->info, NULL); - if (!oldDef) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("memory '%1$s' not found"), dev->data.memory->info.alias); - return -1; - } - - oldDev.data.memory = oldDef; - - if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev, - VIR_DOMAIN_DEVICE_ACTION_UPDATE, - true) < 0) - return -1; - - if (!qemuDomainChangeMemoryLiveValidateChange(oldDef, newDef)) - return -1; - - if (qemuDomainChangeMemoryRequestedSize(vm, newDef->info.alias, - newDef->requestedsize) < 0) - return -1; - - oldDef->requestedsize = newDef->requestedsize; - return 0; -} - - -static int -qemuDomainUpdateDeviceLive(virDomainObj *vm, - virDomainDeviceDef *dev, - virQEMUDriver *driver, - bool force) -{ - virDomainDeviceDef oldDev = { .type = dev->type }; - int idx; - - switch ((virDomainDeviceType)dev->type) { - case VIR_DOMAIN_DEVICE_DISK: - qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, NULL); - return qemuDomainChangeDiskLive(vm, dev, driver, force); - - case VIR_DOMAIN_DEVICE_GRAPHICS: - if ((idx = qemuDomainFindGraphicsIndex(vm->def, dev->data.graphics)) >= 0) { - oldDev.data.graphics = vm->def->graphics[idx]; - if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev, - VIR_DOMAIN_DEVICE_ACTION_UPDATE, - true) < 0) - return -1; - } - - return qemuDomainChangeGraphics(driver, vm, dev->data.graphics); - - case VIR_DOMAIN_DEVICE_NET: - if ((idx = virDomainNetFindIdx(vm->def, dev->data.net)) >= 0) { - oldDev.data.net = vm->def->nets[idx]; - if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev, - VIR_DOMAIN_DEVICE_ACTION_UPDATE, - true) < 0) - return -1; - } - - return qemuDomainChangeNet(driver, vm, dev); - - case VIR_DOMAIN_DEVICE_MEMORY: - return qemuDomainChangeMemoryLive(driver, vm, dev); - - case VIR_DOMAIN_DEVICE_FS: - case VIR_DOMAIN_DEVICE_INPUT: - case VIR_DOMAIN_DEVICE_SOUND: - case VIR_DOMAIN_DEVICE_VIDEO: - case VIR_DOMAIN_DEVICE_WATCHDOG: - case VIR_DOMAIN_DEVICE_HUB: - case VIR_DOMAIN_DEVICE_SMARTCARD: - case VIR_DOMAIN_DEVICE_MEMBALLOON: - case VIR_DOMAIN_DEVICE_NVRAM: - case VIR_DOMAIN_DEVICE_RNG: - case VIR_DOMAIN_DEVICE_SHMEM: - case VIR_DOMAIN_DEVICE_LEASE: - case VIR_DOMAIN_DEVICE_HOSTDEV: - case VIR_DOMAIN_DEVICE_CONTROLLER: - case VIR_DOMAIN_DEVICE_REDIRDEV: - case VIR_DOMAIN_DEVICE_CHR: - case VIR_DOMAIN_DEVICE_NONE: - case VIR_DOMAIN_DEVICE_TPM: - case VIR_DOMAIN_DEVICE_PANIC: - case VIR_DOMAIN_DEVICE_IOMMU: - case VIR_DOMAIN_DEVICE_VSOCK: - case VIR_DOMAIN_DEVICE_AUDIO: - case VIR_DOMAIN_DEVICE_CRYPTO: - case VIR_DOMAIN_DEVICE_LAST: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("live update of device '%1$s' is not supported"), - virDomainDeviceTypeToString(dev->type)); - return -1; - } - - return -1; -} - - static int qemuCheckDiskConfigAgainstDomain(const virDomainDef *def, const virDomainDiskDef *disk) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index bd126234fd..f72bb7722d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -6910,3 +6910,302 @@ qemuDomainChangeMemoryRequestedSize(virDomainObj *vm, return rc; } + + +static int +qemuDomainChangeDiskLive(virDomainObj *vm, + virDomainDeviceDef *dev, + virQEMUDriver *driver, + bool force) +{ + virDomainDiskDef *disk = dev->data.disk; + virDomainDiskDef *orig_disk = NULL; + virDomainStartupPolicy origStartupPolicy; + virDomainDeviceDef oldDev = { .type = dev->type }; + + if (!(orig_disk = virDomainDiskByTarget(vm->def, disk->dst))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("disk '%1$s' not found"), disk->dst); + return -1; + } + + oldDev.data.disk = orig_disk; + origStartupPolicy = orig_disk->startupPolicy; + if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev, + VIR_DOMAIN_DEVICE_ACTION_UPDATE, + true) < 0) + return -1; + + if (!qemuDomainDiskChangeSupported(disk, orig_disk)) + return -1; + + if (!virStorageSourceIsSameLocation(disk->src, orig_disk->src)) { + /* Disk source can be changed only for removable devices */ + if (disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM && + disk->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disk source can be changed only in removable " + "drives")); + return -1; + } + + /* update startup policy first before updating disk image */ + orig_disk->startupPolicy = dev->data.disk->startupPolicy; + + if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk, + dev->data.disk->src, force) < 0) { + /* revert startup policy before failing */ + orig_disk->startupPolicy = origStartupPolicy; + return -1; + } + + dev->data.disk->src = NULL; + } + + /* in case when we aren't updating disk source we update startup policy here */ + orig_disk->startupPolicy = dev->data.disk->startupPolicy; + orig_disk->snapshot = dev->data.disk->snapshot; + + return 0; +} + + +static bool +qemuDomainChangeMemoryLiveValidateChange(const virDomainMemoryDef *oldDef, + const virDomainMemoryDef *newDef) +{ + /* The only thing that is allowed to change is 'requestedsize' for + * virtio-mem model. Check if user isn't trying to sneak in change for + * something else. */ + + switch (oldDef->model) { + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM: + break; + + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM: + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM: + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory of model '%1$s'"), + virDomainMemoryModelTypeToString(oldDef->model)); + return false; + break; + } + + if (oldDef->model != newDef->model) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory model from '%1$s' to '%2$s'"), + virDomainMemoryModelTypeToString(oldDef->model), + virDomainMemoryModelTypeToString(newDef->model)); + return false; + } + + if (oldDef->access != newDef->access) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory access from '%1$s' to '%2$s'"), + virDomainMemoryAccessTypeToString(oldDef->access), + virDomainMemoryAccessTypeToString(newDef->access)); + return false; + } + + if (oldDef->discard != newDef->discard) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory discard from '%1$s' to '%2$s'"), + virTristateBoolTypeToString(oldDef->discard), + virTristateBoolTypeToString(newDef->discard)); + return false; + } + + if (!virBitmapEqual(oldDef->sourceNodes, + newDef->sourceNodes)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cannot modify memory source nodes")); + return false; + } + + if (oldDef->pagesize != newDef->pagesize) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory pagesize from '%1$llu' to '%2$llu'"), + oldDef->pagesize, + newDef->pagesize); + return false; + } + + if (STRNEQ_NULLABLE(oldDef->nvdimmPath, newDef->nvdimmPath)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory path from '%1$s' to '%2$s'"), + NULLSTR(oldDef->nvdimmPath), + NULLSTR(newDef->nvdimmPath)); + return false; + } + + if (oldDef->alignsize != newDef->alignsize) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory align size from '%1$llu' to '%2$llu'"), + oldDef->alignsize, newDef->alignsize); + return false; + } + + if (oldDef->nvdimmPmem != newDef->nvdimmPmem) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory pmem from '%1$d' to '%2$d'"), + oldDef->nvdimmPmem, newDef->nvdimmPmem); + return false; + } + + if (oldDef->targetNode != newDef->targetNode) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory targetNode from '%1$d' to '%2$d'"), + oldDef->targetNode, newDef->targetNode); + return false; + } + + if (oldDef->size != newDef->size) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory size from '%1$llu' to '%2$llu'"), + oldDef->size, newDef->size); + return false; + } + + if (oldDef->labelsize != newDef->labelsize) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory label size from '%1$llu' to '%2$llu'"), + oldDef->labelsize, newDef->labelsize); + return false; + } + if (oldDef->blocksize != newDef->blocksize) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot modify memory block size from '%1$llu' to '%2$llu'"), + oldDef->blocksize, newDef->blocksize); + return false; + } + + /* requestedsize can change */ + + if (oldDef->readonly != newDef->readonly) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cannot modify memory pmem flag")); + return false; + } + + if ((oldDef->uuid || newDef->uuid) && + !(oldDef->uuid && newDef->uuid && + memcmp(oldDef->uuid, newDef->uuid, VIR_UUID_BUFLEN) == 0)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cannot modify memory UUID")); + return false; + } + + return true; +} + + +static int +qemuDomainChangeMemoryLive(virQEMUDriver *driver G_GNUC_UNUSED, + virDomainObj *vm, + virDomainDeviceDef *dev) +{ + virDomainDeviceDef oldDev = { .type = VIR_DOMAIN_DEVICE_MEMORY }; + virDomainMemoryDef *newDef = dev->data.memory; + virDomainMemoryDef *oldDef = NULL; + + oldDef = virDomainMemoryFindByDeviceInfo(vm->def, &dev->data.memory->info, NULL); + if (!oldDef) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("memory '%1$s' not found"), dev->data.memory->info.alias); + return -1; + } + + oldDev.data.memory = oldDef; + + if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev, + VIR_DOMAIN_DEVICE_ACTION_UPDATE, + true) < 0) + return -1; + + if (!qemuDomainChangeMemoryLiveValidateChange(oldDef, newDef)) + return -1; + + if (qemuDomainChangeMemoryRequestedSize(vm, newDef->info.alias, + newDef->requestedsize) < 0) + return -1; + + oldDef->requestedsize = newDef->requestedsize; + return 0; +} + + +int +qemuDomainUpdateDeviceLive(virDomainObj *vm, + virDomainDeviceDef *dev, + virQEMUDriver *driver, + bool force) +{ + virDomainDeviceDef oldDev = { .type = dev->type }; + int idx; + + switch ((virDomainDeviceType)dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, NULL); + return qemuDomainChangeDiskLive(vm, dev, driver, force); + + case VIR_DOMAIN_DEVICE_GRAPHICS: + if ((idx = qemuDomainFindGraphicsIndex(vm->def, dev->data.graphics)) >= 0) { + oldDev.data.graphics = vm->def->graphics[idx]; + if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev, + VIR_DOMAIN_DEVICE_ACTION_UPDATE, + true) < 0) + return -1; + } + + return qemuDomainChangeGraphics(driver, vm, dev->data.graphics); + + case VIR_DOMAIN_DEVICE_NET: + if ((idx = virDomainNetFindIdx(vm->def, dev->data.net)) >= 0) { + oldDev.data.net = vm->def->nets[idx]; + if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev, + VIR_DOMAIN_DEVICE_ACTION_UPDATE, + true) < 0) + return -1; + } + + return qemuDomainChangeNet(driver, vm, dev); + + case VIR_DOMAIN_DEVICE_MEMORY: + return qemuDomainChangeMemoryLive(driver, vm, dev); + + case VIR_DOMAIN_DEVICE_FS: + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_RNG: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_HOSTDEV: + case VIR_DOMAIN_DEVICE_CONTROLLER: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_IOMMU: + case VIR_DOMAIN_DEVICE_VSOCK: + case VIR_DOMAIN_DEVICE_AUDIO: + case VIR_DOMAIN_DEVICE_CRYPTO: + case VIR_DOMAIN_DEVICE_LAST: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("live update of device '%1$s' is not supported"), + virDomainDeviceTypeToString(dev->type)); + return -1; + } + + return -1; +} diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 74fc3bbd65..d9a5ac1164 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -120,6 +120,11 @@ int qemuDomainDetachDeviceLive(virDomainObj *vm, virQEMUDriver *driver, bool async); +int qemuDomainUpdateDeviceLive(virDomainObj *vm, + virDomainDeviceDef *dev, + virQEMUDriver *driver, + bool force); + void qemuDomainRemoveVcpuAlias(virDomainObj *vm, const char *alias); -- 2.39.2

On Fri, Apr 21, 2023 at 10:25 AM Michal Privoznik <mprivozn@redhat.com> wrote:
There is no good reason for qemuDomainUpdateDeviceLive() to live in (ever growing) qemu_driver.c while we have qemu_hotplug.c which already contains the rest of hotplug code. Move the function to its new home.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 299 ---------------------------------------- src/qemu/qemu_hotplug.c | 299 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.h | 5 + 3 files changed, 304 insertions(+), 299 deletions(-)
Reviewed-by: Kristina Hanicova <khanicov@redhat.com> Kristina

The testQemuHotplugDetach() already does call qemuDomainDetachDeviceLive() but only for some device types. For the rest it reports an error (but only if running test verbosely). This makes no sense. Just call qemuDomainDetachDeviceLive() directly and drop testQemuHotplugDetach(). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuhotplugtest.c | 49 +---------------------------------------- 1 file changed, 1 insertion(+), 48 deletions(-) diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 6aaccce55b..4f9d1bcb2a 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -169,53 +169,6 @@ testQemuHotplugAttach(virDomainObj *vm, return ret; } -static int -testQemuHotplugDetach(virDomainObj *vm, - virDomainDeviceDef *dev, - bool async) -{ - int ret = -1; - - switch (dev->type) { - case VIR_DOMAIN_DEVICE_DISK: - case VIR_DOMAIN_DEVICE_CHR: - case VIR_DOMAIN_DEVICE_SHMEM: - case VIR_DOMAIN_DEVICE_WATCHDOG: - case VIR_DOMAIN_DEVICE_HOSTDEV: - case VIR_DOMAIN_DEVICE_NET: - ret = qemuDomainDetachDeviceLive(vm, dev, &driver, async); - break; - - case VIR_DOMAIN_DEVICE_LEASE: - case VIR_DOMAIN_DEVICE_FS: - case VIR_DOMAIN_DEVICE_INPUT: - case VIR_DOMAIN_DEVICE_SOUND: - case VIR_DOMAIN_DEVICE_VIDEO: - case VIR_DOMAIN_DEVICE_CONTROLLER: - case VIR_DOMAIN_DEVICE_GRAPHICS: - case VIR_DOMAIN_DEVICE_HUB: - case VIR_DOMAIN_DEVICE_REDIRDEV: - case VIR_DOMAIN_DEVICE_NONE: - case VIR_DOMAIN_DEVICE_SMARTCARD: - case VIR_DOMAIN_DEVICE_MEMBALLOON: - case VIR_DOMAIN_DEVICE_NVRAM: - case VIR_DOMAIN_DEVICE_LAST: - case VIR_DOMAIN_DEVICE_RNG: - case VIR_DOMAIN_DEVICE_TPM: - case VIR_DOMAIN_DEVICE_PANIC: - case VIR_DOMAIN_DEVICE_MEMORY: - case VIR_DOMAIN_DEVICE_IOMMU: - case VIR_DOMAIN_DEVICE_VSOCK: - case VIR_DOMAIN_DEVICE_AUDIO: - case VIR_DOMAIN_DEVICE_CRYPTO: - VIR_TEST_VERBOSE("device type '%s' cannot be detached", - virDomainDeviceTypeToString(dev->type)); - break; - } - - return ret; -} - static int testQemuHotplugUpdate(virDomainObj *vm, virDomainDeviceDef *dev) @@ -394,7 +347,7 @@ testQemuHotplug(const void *data) break; case DETACH: - ret = testQemuHotplugDetach(vm, dev, false); + ret = qemuDomainDetachDeviceLive(vm, dev, &driver, false); if (ret == 0 || fail) ret = testQemuHotplugCheckResult(vm, domain_xml, domain_filename, fail); -- 2.39.2

On Fri, Apr 21, 2023 at 10:25 AM Michal Privoznik <mprivozn@redhat.com> wrote:
The testQemuHotplugDetach() already does call qemuDomainDetachDeviceLive() but only for some device types. For the rest it reports an error (but only if running test verbosely). This makes no sense. Just call qemuDomainDetachDeviceLive() directly and drop testQemuHotplugDetach().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuhotplugtest.c | 49 +---------------------------------------- 1 file changed, 1 insertion(+), 48 deletions(-)
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Kristina

There's no reason for qemuhotplugtest to reimplement which device attach function to call (testQemuHotplugAttach()) when qemuDomainAttachDeviceLive() already does that. Thus, drop testQemuHotplugAttach() and call qemuDomainAttachDeviceLive() directly. There's one small catch though, qemuDomainAttachDeviceLive() now calls one monitor command more (to list all aliases). We don't care really, because we're not testing that. Therefore, just provide a dummy reply. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuhotplugtest.c | 68 +++++------------------------------------ 1 file changed, 7 insertions(+), 61 deletions(-) diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 4f9d1bcb2a..e3744964d5 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -109,66 +109,6 @@ qemuHotplugCreateObjects(virDomainXMLOption *xmlopt, return 0; } -static int -testQemuHotplugAttach(virDomainObj *vm, - virDomainDeviceDef *dev) -{ - int ret = -1; - - switch (dev->type) { - case VIR_DOMAIN_DEVICE_DISK: - /* conn in only used for storage pool and secrets lookup so as long - * as we don't use any of them, passing NULL should be safe - */ - ret = qemuDomainAttachDeviceDiskLive(&driver, vm, dev); - break; - case VIR_DOMAIN_DEVICE_CHR: - ret = qemuDomainAttachChrDevice(&driver, vm, dev); - break; - case VIR_DOMAIN_DEVICE_SHMEM: - ret = qemuDomainAttachShmemDevice(vm, dev->data.shmem); - break; - case VIR_DOMAIN_DEVICE_WATCHDOG: - ret = qemuDomainAttachWatchdog(vm, dev->data.watchdog); - break; - case VIR_DOMAIN_DEVICE_HOSTDEV: - ret = qemuDomainAttachHostDevice(&driver, vm, dev->data.hostdev); - break; - case VIR_DOMAIN_DEVICE_NET: - ret = qemuDomainAttachNetDevice(&driver, vm, dev->data.net); - break; - - case VIR_DOMAIN_DEVICE_LEASE: - case VIR_DOMAIN_DEVICE_FS: - case VIR_DOMAIN_DEVICE_INPUT: - case VIR_DOMAIN_DEVICE_SOUND: - case VIR_DOMAIN_DEVICE_VIDEO: - case VIR_DOMAIN_DEVICE_CONTROLLER: - case VIR_DOMAIN_DEVICE_GRAPHICS: - case VIR_DOMAIN_DEVICE_HUB: - case VIR_DOMAIN_DEVICE_REDIRDEV: - case VIR_DOMAIN_DEVICE_NONE: - case VIR_DOMAIN_DEVICE_SMARTCARD: - case VIR_DOMAIN_DEVICE_MEMBALLOON: - case VIR_DOMAIN_DEVICE_NVRAM: - case VIR_DOMAIN_DEVICE_LAST: - case VIR_DOMAIN_DEVICE_RNG: - case VIR_DOMAIN_DEVICE_TPM: - case VIR_DOMAIN_DEVICE_PANIC: - case VIR_DOMAIN_DEVICE_MEMORY: - case VIR_DOMAIN_DEVICE_IOMMU: - case VIR_DOMAIN_DEVICE_VSOCK: - case VIR_DOMAIN_DEVICE_AUDIO: - case VIR_DOMAIN_DEVICE_CRYPTO: - default: - VIR_TEST_VERBOSE("device type '%s' cannot be attached", - virDomainDeviceTypeToString(dev->type)); - break; - } - - return ret; -} - static int testQemuHotplugUpdate(virDomainObj *vm, virDomainDeviceDef *dev) @@ -325,6 +265,12 @@ testQemuHotplug(const void *data) goto cleanup; } + /* After successful attach, we list all aliases. We don't care for that in + * the test. Add a dummy reply. */ + if (test->action == ATTACH && + qemuMonitorTestAddItem(test_mon, "qom-list", "{\"return\":[]}") < 0) + goto cleanup; + priv = vm->privateData; priv->mon = qemuMonitorTestGetMonitor(test_mon); @@ -335,7 +281,7 @@ testQemuHotplug(const void *data) switch (test->action) { case ATTACH: - ret = testQemuHotplugAttach(vm, dev); + ret = qemuDomainAttachDeviceLive(vm, dev, &driver); if (ret == 0) { /* vm->def stolen dev->data.* so we just need to free the dev * envelope */ -- 2.39.2

On Fri, Apr 21, 2023 at 10:25 AM Michal Privoznik <mprivozn@redhat.com> wrote:
There's no reason for qemuhotplugtest to reimplement which device attach function to call (testQemuHotplugAttach()) when qemuDomainAttachDeviceLive() already does that. Thus, drop testQemuHotplugAttach() and call qemuDomainAttachDeviceLive() directly.
There's one small catch though, qemuDomainAttachDeviceLive() now calls one monitor command more (to list all aliases). We don't care really, because we're not testing that. Therefore, just provide a dummy reply.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuhotplugtest.c | 68 +++++------------------------------------ 1 file changed, 7 insertions(+), 61 deletions(-)
Reviewed-by: Kristina Hanicova <khanicov@redhat.com> Kristina

There's no reason for qemuhotplugtest to reimplement which device update function to call (testQemuHotplugUpdate()) when qemuDomainUpdateDeviceLive() already does that. Thus, drop testQemuHotplugUpdate() and call qemuDomainUpdateDeviceLive() directly. BTW: this also shows why reimplementing qemuDomainUpdateDeviceLive() is bad idea: The "disk-cdrom-nochange" test is succeeding only because testQemuHotplugUpdate() supports graphics and returns an (expected) error for every other devtype. NB, there's still missing check that the resulting XML is the expected one (just like we do for attach and detach), but that's pre-existing and will be fixed later. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuhotplugtest.c | 57 +++-------------------------------------- 1 file changed, 3 insertions(+), 54 deletions(-) diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index e3744964d5..b4c03d5374 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -109,57 +109,6 @@ qemuHotplugCreateObjects(virDomainXMLOption *xmlopt, return 0; } -static int -testQemuHotplugUpdate(virDomainObj *vm, - virDomainDeviceDef *dev) -{ - int ret = -1; - - /* XXX Ideally, we would call qemuDomainUpdateDeviceLive here. But that - * would require us to provide virConnectPtr and virDomainPtr (they're used - * in case of updating a disk device. So for now, we will proceed with - * breaking the function into pieces. If we ever learn how to fake those - * required object, we can replace this code then. */ - switch (dev->type) { - case VIR_DOMAIN_DEVICE_GRAPHICS: - ret = qemuDomainChangeGraphics(&driver, vm, dev->data.graphics); - break; - - case VIR_DOMAIN_DEVICE_DISK: - case VIR_DOMAIN_DEVICE_LEASE: - case VIR_DOMAIN_DEVICE_FS: - case VIR_DOMAIN_DEVICE_NET: - case VIR_DOMAIN_DEVICE_INPUT: - case VIR_DOMAIN_DEVICE_SOUND: - case VIR_DOMAIN_DEVICE_VIDEO: - case VIR_DOMAIN_DEVICE_HOSTDEV: - case VIR_DOMAIN_DEVICE_WATCHDOG: - case VIR_DOMAIN_DEVICE_CONTROLLER: - case VIR_DOMAIN_DEVICE_HUB: - case VIR_DOMAIN_DEVICE_REDIRDEV: - case VIR_DOMAIN_DEVICE_NONE: - case VIR_DOMAIN_DEVICE_SMARTCARD: - case VIR_DOMAIN_DEVICE_CHR: - case VIR_DOMAIN_DEVICE_MEMBALLOON: - case VIR_DOMAIN_DEVICE_NVRAM: - case VIR_DOMAIN_DEVICE_LAST: - case VIR_DOMAIN_DEVICE_RNG: - case VIR_DOMAIN_DEVICE_TPM: - case VIR_DOMAIN_DEVICE_PANIC: - case VIR_DOMAIN_DEVICE_SHMEM: - case VIR_DOMAIN_DEVICE_MEMORY: - case VIR_DOMAIN_DEVICE_IOMMU: - case VIR_DOMAIN_DEVICE_VSOCK: - case VIR_DOMAIN_DEVICE_AUDIO: - case VIR_DOMAIN_DEVICE_CRYPTO: - VIR_TEST_VERBOSE("device type '%s' cannot be updated", - virDomainDeviceTypeToString(dev->type)); - break; - } - - return ret; -} - static int testQemuHotplugCheckResult(virDomainObj *vm, const char *expected, @@ -300,7 +249,7 @@ testQemuHotplug(const void *data) break; case UPDATE: - ret = testQemuHotplugUpdate(vm, dev); + ret = qemuDomainUpdateDeviceLive(vm, dev, &driver, false); } virObjectLock(priv->mon); @@ -632,8 +581,8 @@ mymain(void) DO_TEST_UPDATE("x86_64", "graphics-spice-listen-network", "graphics-spice-listen-network-password", false, false, "set_password", QMP_OK, "expire_password", QMP_OK); cfg->spiceTLS = false; - /* Strange huh? Currently, only graphics can be updated :-P */ - DO_TEST_UPDATE("x86_64", "disk-cdrom", "disk-cdrom-nochange", true, false, NULL); + + DO_TEST_UPDATE("x86_64", "disk-cdrom", "disk-cdrom-nochange", false, false, NULL); DO_TEST_ATTACH("x86_64", "console-compat-2-live", "console-virtio", false, true, "chardev-add", "{\"return\": {\"pty\": \"/dev/pts/26\"}}", -- 2.39.2

On Fri, Apr 21, 2023 at 10:25 AM Michal Privoznik <mprivozn@redhat.com> wrote:
There's no reason for qemuhotplugtest to reimplement which device update function to call (testQemuHotplugUpdate()) when qemuDomainUpdateDeviceLive() already does that. Thus, drop testQemuHotplugUpdate() and call qemuDomainUpdateDeviceLive() directly.
BTW: this also shows why reimplementing qemuDomainUpdateDeviceLive() is bad idea: The "disk-cdrom-nochange" test is succeeding only because testQemuHotplugUpdate() supports graphics and returns an (expected) error for every other devtype.
NB, there's still missing check that the resulting XML is the expected one (just like we do for attach and detach), but that's pre-existing and will be fixed later.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuhotplugtest.c | 57 +++-------------------------------------- 1 file changed, 3 insertions(+), 54 deletions(-)
Reviewed-by: Kristina Hanicova <khanicov@redhat.com> Kristina

After previous cleanups a lot of functions from qemu_hotplug.c are called only within the file. Make them static and drop their declaration from the header file. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 56 +++++++++++++++++++++---------------- src/qemu/qemu_hotplug.h | 61 ----------------------------------------- 2 files changed, 32 insertions(+), 85 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f72bb7722d..13b1872ea3 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -75,6 +75,11 @@ VIR_LOG_INIT("qemu.qemu_hotplug"); static void qemuDomainResetDeviceRemoval(virDomainObj *vm); +static int +qemuDomainAttachHostDevice(virQEMUDriver *driver, + virDomainObj *vm, + virDomainHostdevDef *hostdev); + /** * qemuDomainDeleteDevice: * @vm: domain object @@ -546,7 +551,7 @@ qemuDomainChangeMediaBlockdev(virDomainObj *vm, * * Returns 0 on success, -1 on error and reports libvirt error */ -int +static int qemuDomainChangeEjectableMedia(virQEMUDriver *driver, virDomainObj *vm, virDomainDiskDef *disk, @@ -754,8 +759,9 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm, } -int qemuDomainAttachControllerDevice(virDomainObj *vm, - virDomainControllerDef *controller) +static int +qemuDomainAttachControllerDevice(virDomainObj *vm, + virDomainControllerDef *controller) { int ret = -1; const char* type = virDomainControllerTypeToString(controller->type); @@ -1033,7 +1039,7 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriver *driver, * This function handles all the necessary steps to attach a new storage source * to the VM. */ -int +static int qemuDomainAttachDeviceDiskLive(virQEMUDriver *driver, virDomainObj *vm, virDomainDeviceDef *dev) @@ -1077,7 +1083,7 @@ qemuDomainNetDeviceVportRemove(virDomainNetDef *net) } -int +static int qemuDomainAttachNetDevice(virQEMUDriver *driver, virDomainObj *vm, virDomainNetDef *net) @@ -1768,9 +1774,10 @@ qemuDomainDelChardevTLSObjects(virQEMUDriver *driver, } -int qemuDomainAttachRedirdevDevice(virQEMUDriver *driver, - virDomainObj *vm, - virDomainRedirdevDef *redirdev) +static int +qemuDomainAttachRedirdevDevice(virQEMUDriver *driver, + virDomainObj *vm, + virDomainRedirdevDef *redirdev) { int ret = -1; qemuDomainObjPrivate *priv = vm->privateData; @@ -1995,7 +2002,7 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainObj *vm, } -int +static int qemuDomainAttachChrDevice(virQEMUDriver *driver, virDomainObj *vm, virDomainDeviceDef *dev) @@ -2125,7 +2132,7 @@ qemuDomainAttachChrDevice(virQEMUDriver *driver, } -int +static int qemuDomainAttachRNGDevice(virQEMUDriver *driver, virDomainObj *vm, virDomainRNGDef *rng) @@ -2240,7 +2247,7 @@ qemuDomainAttachRNGDevice(virQEMUDriver *driver, * * Returns 0 on success -1 on error. */ -int +static int qemuDomainAttachMemory(virQEMUDriver *driver, virDomainObj *vm, virDomainMemoryDef *mem) @@ -2757,7 +2764,7 @@ qemuDomainAttachMediatedDevice(virQEMUDriver *driver, } -int +static int qemuDomainAttachHostDevice(virQEMUDriver *driver, virDomainObj *vm, virDomainHostdevDef *hostdev) @@ -2808,7 +2815,7 @@ qemuDomainAttachHostDevice(virQEMUDriver *driver, } -int +static int qemuDomainAttachShmemDevice(virDomainObj *vm, virDomainShmemDef *shmem) { @@ -2914,7 +2921,7 @@ qemuDomainAttachShmemDevice(virDomainObj *vm, } -int +static int qemuDomainAttachWatchdog(virDomainObj *vm, virDomainWatchdogDef *watchdog) { @@ -3017,7 +3024,7 @@ qemuDomainAttachWatchdog(virDomainObj *vm, } -int +static int qemuDomainAttachInputDevice(virDomainObj *vm, virDomainInputDef *input) { @@ -3118,7 +3125,7 @@ qemuDomainAttachInputDevice(virDomainObj *vm, } -int +static int qemuDomainAttachVsockDevice(virDomainObj *vm, virDomainVsockDef *vsock) { @@ -3195,7 +3202,7 @@ qemuDomainAttachVsockDevice(virDomainObj *vm, } -int +static int qemuDomainAttachFSDevice(virQEMUDriver *driver, virDomainObj *vm, virDomainFSDef *fs) @@ -3288,7 +3295,7 @@ qemuDomainAttachFSDevice(virQEMUDriver *driver, } -int +static int qemuDomainAttachLease(virQEMUDriver *driver, virDomainObj *vm, virDomainLeaseDef *lease) @@ -3596,9 +3603,10 @@ qemuDomainChangeNetFilter(virDomainObj *vm, return 0; } -int qemuDomainChangeNetLinkState(virDomainObj *vm, - virDomainNetDef *dev, - int linkstate) +static int +qemuDomainChangeNetLinkState(virDomainObj *vm, + virDomainNetDef *dev, + int linkstate) { int ret = -1; qemuDomainObjPrivate *priv = vm->privateData; @@ -3626,7 +3634,7 @@ int qemuDomainChangeNetLinkState(virDomainObj *vm, return ret; } -int +static int qemuDomainChangeNet(virQEMUDriver *driver, virDomainObj *vm, virDomainDeviceDef *dev) @@ -4232,7 +4240,7 @@ qemuDomainChangeGraphicsPasswords(virDomainObj *vm, } -int +static int qemuDomainChangeGraphics(virQEMUDriver *driver, virDomainObj *vm, virDomainGraphicsDef *dev) @@ -6896,7 +6904,7 @@ qemuDomainSetVcpuInternal(virQEMUDriver *driver, } -int +static int qemuDomainChangeMemoryRequestedSize(virDomainObj *vm, const char *alias, unsigned long long requestedsize) diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index d9a5ac1164..4fe7f4923e 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -24,12 +24,6 @@ #include "qemu_conf.h" #include "qemu_domain.h" -int qemuDomainChangeEjectableMedia(virQEMUDriver *driver, - virDomainObj *vm, - virDomainDiskDef *disk, - virStorageSource *newsrc, - bool force); - void qemuDomainDelTLSObjects(virDomainObj *vm, virDomainAsyncJob asyncJob, const char *secAlias, @@ -48,68 +42,17 @@ int qemuDomainGetTLSObjects(qemuDomainSecretInfo *secinfo, virJSONValue **tlsProps, virJSONValue **secProps); -int qemuDomainAttachControllerDevice(virDomainObj *vm, - virDomainControllerDef *controller); -int qemuDomainAttachDeviceDiskLive(virQEMUDriver *driver, - virDomainObj *vm, - virDomainDeviceDef *dev); - int qemuDomainAttachDiskGeneric(virDomainObj *vm, virDomainDiskDef *disk, virDomainAsyncJob asyncJob); -int qemuDomainAttachNetDevice(virQEMUDriver *driver, - virDomainObj *vm, - virDomainNetDef *net); -int qemuDomainAttachRedirdevDevice(virQEMUDriver *driver, - virDomainObj *vm, - virDomainRedirdevDef *hostdev); -int qemuDomainAttachHostDevice(virQEMUDriver *driver, - virDomainObj *vm, - virDomainHostdevDef *hostdev); -int qemuDomainAttachShmemDevice(virDomainObj *vm, - virDomainShmemDef *shmem); -int qemuDomainAttachWatchdog(virDomainObj *vm, - virDomainWatchdogDef *watchdog); int qemuDomainFindGraphicsIndex(virDomainDef *def, virDomainGraphicsDef *dev); -int qemuDomainAttachMemory(virQEMUDriver *driver, - virDomainObj *vm, - virDomainMemoryDef *mem); -int qemuDomainChangeGraphics(virQEMUDriver *driver, - virDomainObj *vm, - virDomainGraphicsDef *dev); int qemuDomainChangeGraphicsPasswords(virDomainObj *vm, int type, virDomainGraphicsAuthDef *auth, const char *defaultPasswd, int asyncJob); -int qemuDomainChangeNet(virQEMUDriver *driver, - virDomainObj *vm, - virDomainDeviceDef *dev); -int qemuDomainChangeNetLinkState(virDomainObj *vm, - virDomainNetDef *dev, - int linkstate); - -int qemuDomainAttachInputDevice(virDomainObj *vm, - virDomainInputDef *input); - -int qemuDomainAttachVsockDevice(virDomainObj *vm, - virDomainVsockDef *vsock); -int -qemuDomainAttachFSDevice(virQEMUDriver *driver, - virDomainObj *vm, - virDomainFSDef *fs); - -int qemuDomainAttachLease(virQEMUDriver *driver, - virDomainObj *vm, - virDomainLeaseDef *lease); -int qemuDomainAttachChrDevice(virQEMUDriver *driver, - virDomainObj *vm, - virDomainDeviceDef *dev); -int qemuDomainAttachRNGDevice(virQEMUDriver *driver, - virDomainObj *vm, - virDomainRNGDef *rng); int qemuDomainAttachDeviceLive(virDomainObj *vm, virDomainDeviceDef *dev, @@ -165,7 +108,3 @@ int qemuHotplugAttachDBusVMState(virQEMUDriver *driver, int qemuHotplugRemoveDBusVMState(virDomainObj *vm, virDomainAsyncJob asyncJob); - -int qemuDomainChangeMemoryRequestedSize(virDomainObj *vm, - const char *alias, - unsigned long long requestedsize); -- 2.39.2

On Fri, Apr 21, 2023 at 10:25 AM Michal Privoznik <mprivozn@redhat.com> wrote:
After previous cleanups a lot of functions from qemu_hotplug.c are called only within the file. Make them static and drop their declaration from the header file.
*declarations
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 56 +++++++++++++++++++++---------------- src/qemu/qemu_hotplug.h | 61 ----------------------------------------- 2 files changed, 32 insertions(+), 85 deletions(-)
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Kristina

There's a comment in testQemuHotplug() trying to explain why we need to unlock the monitor object. Well, while it might have been correct when being introduced, it's no long factually correct as just any function (attach/detach/update) might talk to the monitor and it expects the monitor to be unlocked (as it calls qemuDomainObjEnterMonitor() + qemuDomainObjExitMonitor()). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuhotplugtest.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index b4c03d5374..9a1cf8ab2f 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -223,9 +223,8 @@ testQemuHotplug(const void *data) priv = vm->privateData; priv->mon = qemuMonitorTestGetMonitor(test_mon); - /* XXX We need to unlock the monitor here, as - * qemuDomainObjEnterMonitorInternal (called from qemuDomainChangeGraphics) - * tries to lock it again */ + /* We need to unlock the monitor here, as any function below talks + * (transitively) on the monitor. */ virObjectUnlock(priv->mon); switch (test->action) { -- 2.39.2

On Fri, Apr 21, 2023 at 10:25 AM Michal Privoznik <mprivozn@redhat.com> wrote:
There's a comment in testQemuHotplug() trying to explain why we need to unlock the monitor object. Well, while it might have been correct when being introduced, it's no long factually correct as
*no longer
just any function (attach/detach/update) might talk to the monitor and it expects the monitor to be unlocked (as it calls qemuDomainObjEnterMonitor() + qemuDomainObjExitMonitor()).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuhotplugtest.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
Kristina

This is a leftover from v2.0.0-rc1~300. In v1.2.12-rc1~43 we've introduced a code that explicitly sets vm->def->id to -1 to force generation of inactive XML. But this was removed in the later commit, which forgot to remove the restoration of the original dom ID. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuhotplugtest.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 9a1cf8ab2f..d5ceb1373b 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -122,7 +122,6 @@ testQemuHotplugCheckResult(virDomainObj *vm, VIR_DOMAIN_DEF_FORMAT_SECURE); if (!actual) return -1; - vm->def->id = QEMU_HOTPLUG_TEST_DOMAIN_ID; if (STREQ(expected, actual)) { if (fail) -- 2.39.2

On Fri, Apr 21, 2023 at 10:25 AM Michal Privoznik <mprivozn@redhat.com> wrote:
This is a leftover from v2.0.0-rc1~300. In v1.2.12-rc1~43 we've introduced a code that explicitly sets vm->def->id to -1 to force generation of inactive XML. But this was removed in the later commit, which forgot to remove the restoration of the original dom ID.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuhotplugtest.c | 1 - 1 file changed, 1 deletion(-)
Reviewed-by: Kristina Hanicova <khanicov@redhat.com> Kristina

This brings us one step closer to the caller of qemuDomainAttachDeviceLive() (qemuDomainAttachDeviceLiveAndConfig()). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuhotplugtest.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index d5ceb1373b..fffb4a3410 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -154,7 +154,7 @@ testQemuHotplug(const void *data) bool keep = test->keep; unsigned int device_parse_flags = 0; virDomainObj *vm = NULL; - virDomainDeviceDef *dev = NULL; + g_autoptr(virDomainDeviceDef) dev = NULL; g_autoptr(qemuMonitorTest) test_mon = NULL; qemuDomainObjPrivate *priv = NULL; @@ -229,11 +229,6 @@ testQemuHotplug(const void *data) switch (test->action) { case ATTACH: ret = qemuDomainAttachDeviceLive(vm, dev, &driver); - if (ret == 0) { - /* vm->def stolen dev->data.* so we just need to free the dev - * envelope */ - VIR_FREE(dev); - } if (ret == 0 || fail) ret = testQemuHotplugCheckResult(vm, result_xml, result_filename, fail); @@ -262,7 +257,6 @@ testQemuHotplug(const void *data) virObjectUnref(vm); test->vm = NULL; } - virDomainDeviceDefFree(dev); return ((ret < 0 && fail) || (!ret && !fail)) ? 0 : -1; } -- 2.39.2

On Fri, Apr 21, 2023 at 10:25 AM Michal Privoznik <mprivozn@redhat.com> wrote:
This brings us one step closer to the caller of qemuDomainAttachDeviceLive() (qemuDomainAttachDeviceLiveAndConfig()).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuhotplugtest.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
Reviewed-by: Kristina Hanicova <khanicov@redhat.com> making impact, Kristina

Just like we check the resulting domain XML after ATTACH and DETACH, we should do the same after UPDATE action. This is as simple as calling testQemuHotplugCheckResult() and providing missing XMLs. For those test cases where no change is done, we can just make the expected XML a symlink to the input XML. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuhotplugtest.c | 9 +- ...hotplug-disk-cdrom+disk-cdrom-nochange.xml | 1 + .../qemuhotplug-disk-cdrom.xml | 30 ++++- ...graphics-spice+graphics-spice-nochange.xml | 1 + ...graphics-spice-listen-network-password.xml | 71 +++++++++++ ...imeout+graphics-spice-timeout-nochange.xml | 1 + ...imeout+graphics-spice-timeout-password.xml | 117 ++++++++++++++++++ .../qemuhotplug-graphics-spice-timeout.xml | 50 ++++++-- .../qemuhotplug-graphics-spice.xml | 41 ++++-- 9 files changed, 295 insertions(+), 26 deletions(-) create mode 120000 tests/qemuhotplugtestdomains/qemuhotplug-disk-cdrom+disk-cdrom-nochange.xml create mode 120000 tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice+graphics-spice-nochange.xml create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-listen-network+graphics-spice-listen-network-password.xml create mode 120000 tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-timeout+graphics-spice-timeout-nochange.xml create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-timeout+graphics-spice-timeout-password.xml diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index fffb4a3410..091fcc4d49 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -123,7 +123,7 @@ testQemuHotplugCheckResult(virDomainObj *vm, if (!actual) return -1; - if (STREQ(expected, actual)) { + if (STREQ_NULLABLE(expected, actual)) { if (fail) VIR_TEST_VERBOSE("domain XML should not match the expected result"); ret = 0; @@ -170,7 +170,9 @@ testQemuHotplug(const void *data) virTestLoadFile(device_filename, &device_xml) < 0) goto cleanup; - if (test->action == ATTACH && + if (!fail && + (test->action == ATTACH || + test->action == UPDATE) && virTestLoadFile(result_filename, &result_xml) < 0) goto cleanup; @@ -243,6 +245,9 @@ testQemuHotplug(const void *data) case UPDATE: ret = qemuDomainUpdateDeviceLive(vm, dev, &driver, false); + if (ret == 0 || fail) + ret = testQemuHotplugCheckResult(vm, result_xml, + result_filename, fail); } virObjectLock(priv->mon); diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-disk-cdrom+disk-cdrom-nochange.xml b/tests/qemuhotplugtestdomains/qemuhotplug-disk-cdrom+disk-cdrom-nochange.xml new file mode 120000 index 0000000000..e07b01b301 --- /dev/null +++ b/tests/qemuhotplugtestdomains/qemuhotplug-disk-cdrom+disk-cdrom-nochange.xml @@ -0,0 +1 @@ +qemuhotplug-disk-cdrom.xml \ No newline at end of file diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-disk-cdrom.xml b/tests/qemuhotplugtestdomains/qemuhotplug-disk-cdrom.xml index 3acf55ab17..bcf4cf9bf8 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-disk-cdrom.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-disk-cdrom.xml @@ -1,4 +1,4 @@ -<domain type='qemu'> +<domain type='qemu' id='7'> <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> <memory unit='KiB'>219100</memory> @@ -8,6 +8,9 @@ <type arch='x86_64' machine='pc'>hvm</type> <boot dev='hd'/> </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> @@ -18,6 +21,7 @@ <driver name='qemu' type='raw'/> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> + <alias name='ide0-0-0'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> @@ -25,13 +29,27 @@ <source file='/root/boot.iso'/> <target dev='hdc' bus='ide'/> <readonly/> + <alias name='ide0-1-0'/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> - <controller type='usb' index='0'/> - <controller type='ide' index='0'/> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> + <controller type='usb' index='0' model='piix3-uhci'> + <alias name='usb'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <alias name='ide'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci.0'/> + </controller> + <input type='mouse' bus='ps2'> + <alias name='input0'/> + </input> + <input type='keyboard' bus='ps2'> + <alias name='input1'/> + </input> + <audio id='1' type='none'/> <memballoon model='none'/> </devices> </domain> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice+graphics-spice-nochange.xml b/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice+graphics-spice-nochange.xml new file mode 120000 index 0000000000..4f731cb8db --- /dev/null +++ b/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice+graphics-spice-nochange.xml @@ -0,0 +1 @@ +qemuhotplug-graphics-spice.xml \ No newline at end of file diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-listen-network+graphics-spice-listen-network-password.xml b/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-listen-network+graphics-spice-listen-network-password.xml new file mode 100644 index 0000000000..9cc273435a --- /dev/null +++ b/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-listen-network+graphics-spice-listen-network-password.xml @@ -0,0 +1,71 @@ +<domain type='qemu' id='7'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <alias name='ide0-0-0'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0' model='piix3-uhci'> + <alias name='usb'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <alias name='ide'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci.0'/> + </controller> + <input type='mouse' bus='ps2'> + <alias name='input0'/> + </input> + <input type='keyboard' bus='ps2'> + <alias name='input1'/> + </input> + <graphics type='spice' port='5900' tlsPort='5901' autoport='yes' listen='10.65.210.231' keymap='en-us' passwd='password2' passwdValidTo='2013-06-20T01:34:37' connected='disconnect'> + <listen type='network' address='10.65.210.231' network='vdsm-rhevm'/> + <channel name='main' mode='secure'/> + <channel name='display' mode='secure'/> + <channel name='inputs' mode='secure'/> + <channel name='cursor' mode='secure'/> + <channel name='playback' mode='secure'/> + <channel name='record' mode='secure'/> + <channel name='smartcard' mode='secure'/> + <channel name='usbredir' mode='secure'/> + </graphics> + <audio id='1' type='spice'/> + <video> + <model type='qxl' ram='65536' vram='32768' vgamem='16384' heads='1' primary='yes'/> + <alias name='video0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <video> + <model type='qxl' ram='65536' vram='32768' vgamem='16384' heads='1'/> + <alias name='video1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </video> + <memballoon model='virtio'> + <alias name='balloon0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-timeout+graphics-spice-timeout-nochange.xml b/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-timeout+graphics-spice-timeout-nochange.xml new file mode 120000 index 0000000000..3a5bce52e4 --- /dev/null +++ b/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-timeout+graphics-spice-timeout-nochange.xml @@ -0,0 +1 @@ +qemuhotplug-graphics-spice-timeout.xml \ No newline at end of file diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-timeout+graphics-spice-timeout-password.xml b/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-timeout+graphics-spice-timeout-password.xml new file mode 100644 index 0000000000..03964ad01c --- /dev/null +++ b/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-timeout+graphics-spice-timeout-password.xml @@ -0,0 +1,117 @@ +<domain type='kvm' id='7'> + <name>f14</name> + <uuid>553effab-b5e1-2d80-dfe3-da4344826c43</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='cdrom'/> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <cpu mode='custom' match='exact' check='partial'> + <model fallback='allow'>core2duo</model> + <vendor>Intel</vendor> + <topology sockets='1' dies='1' cores='2' threads='1'/> + <feature policy='require' name='lahf_lm'/> + <feature policy='require' name='xtpr'/> + <feature policy='require' name='cx16'/> + <feature policy='require' name='tm2'/> + <feature policy='require' name='est'/> + <feature policy='require' name='vmx'/> + <feature policy='require' name='ds_cpl'/> + <feature policy='require' name='pbe'/> + <feature policy='require' name='tm'/> + <feature policy='require' name='ht'/> + <feature policy='require' name='ss'/> + <feature policy='require' name='acpi'/> + <feature policy='require' name='ds'/> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/var/lib/libvirt/images/f14.img'/> + <target dev='vda' bus='virtio'/> + <alias name='virtio-disk0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw'/> + <source file='/var/lib/libvirt/Fedora-14-x86_64-Live-KDE.iso'/> + <target dev='hdc' bus='ide'/> + <readonly/> + <alias name='ide0-1-0'/> + <address type='drive' controller='0' bus='1' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'> + <alias name='ide'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='virtio-serial' index='0'> + <alias name='virtio-serial0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </controller> + <controller type='usb' index='0' model='piix3-uhci'> + <alias name='usb'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci.0'/> + </controller> + <interface type='ethernet'> + <mac address='52:54:00:71:70:89'/> + <script path='/etc/qemu-ifup'/> + <model type='rtl8139'/> + <alias name='net0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </interface> + <serial type='pty'> + <target type='isa-serial' port='0'> + <model name='isa-serial'/> + </target> + <alias name='serial0'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + <alias name='serial0'/> + </console> + <input type='tablet' bus='usb'> + <alias name='input0'/> + <address type='usb' bus='0' port='1'/> + </input> + <input type='mouse' bus='ps2'> + <alias name='input1'/> + </input> + <input type='keyboard' bus='ps2'> + <alias name='input2'/> + </input> + <graphics type='spice' port='5900' autoport='no' passwd='secret' passwdValidTo='2013-05-31T16:11:22' connected='disconnect'> + <listen type='address'/> + </graphics> + <sound model='ac97'> + <alias name='sound0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </sound> + <audio id='1' type='spice'/> + <video> + <model type='vga' vram='16384' heads='1' primary='yes'/> + <alias name='video0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='virtio'> + <alias name='balloon0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-timeout.xml b/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-timeout.xml index 0e96033fc8..e6b0cc833a 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-timeout.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-timeout.xml @@ -1,4 +1,4 @@ -<domain type='kvm'> +<domain type='kvm' id='7'> <name>f14</name> <uuid>553effab-b5e1-2d80-dfe3-da4344826c43</uuid> <memory unit='KiB'>1048576</memory> @@ -15,10 +15,10 @@ <apic/> <pae/> </features> - <cpu match='exact'> - <model>core2duo</model> + <cpu mode='custom' match='exact' check='partial'> + <model fallback='allow'>core2duo</model> <vendor>Intel</vendor> - <topology sockets='1' cores='2' threads='1'/> + <topology sockets='1' dies='1' cores='2' threads='1'/> <feature policy='require' name='lahf_lm'/> <feature policy='require' name='xtpr'/> <feature policy='require' name='cx16'/> @@ -43,6 +43,7 @@ <driver name='qemu' type='qcow2'/> <source file='/var/lib/libvirt/images/f14.img'/> <target dev='vda' bus='virtio'/> + <alias name='virtio-disk0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </disk> <disk type='file' device='cdrom'> @@ -50,37 +51,66 @@ <source file='/var/lib/libvirt/Fedora-14-x86_64-Live-KDE.iso'/> <target dev='hdc' bus='ide'/> <readonly/> + <alias name='ide0-1-0'/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> <controller type='ide' index='0'> + <alias name='ide'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> <controller type='virtio-serial' index='0'> + <alias name='virtio-serial0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </controller> + <controller type='usb' index='0' model='piix3-uhci'> + <alias name='usb'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci.0'/> + </controller> <interface type='ethernet'> <mac address='52:54:00:71:70:89'/> <script path='/etc/qemu-ifup'/> + <model type='rtl8139'/> + <alias name='net0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </interface> <serial type='pty'> - <target port='0'/> + <target type='isa-serial' port='0'> + <model name='isa-serial'/> + </target> + <alias name='serial0'/> </serial> <console type='pty'> <target type='serial' port='0'/> + <alias name='serial0'/> </console> - <input type='tablet' bus='usb'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> - <graphics type='spice' port='5900' autoport='no' passwd='sercet' passwdValidTo='2011-05-31T16:11:22' connected='disconnect'/> + <input type='tablet' bus='usb'> + <alias name='input0'/> + <address type='usb' bus='0' port='1'/> + </input> + <input type='mouse' bus='ps2'> + <alias name='input1'/> + </input> + <input type='keyboard' bus='ps2'> + <alias name='input2'/> + </input> + <graphics type='spice' port='5900' autoport='no' passwd='sercet' passwdValidTo='2011-05-31T16:11:22' connected='disconnect'> + <listen type='address'/> + </graphics> <sound model='ac97'> + <alias name='sound0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </sound> + <audio id='1' type='spice'/> <video> - <model type='vga' vram='16384' heads='1'/> + <model type='vga' vram='16384' heads='1' primary='yes'/> + <alias name='video0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> <memballoon model='virtio'> + <alias name='balloon0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </memballoon> </devices> diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice.xml b/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice.xml index ec761d6619..c4924eae09 100644 --- a/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice.xml +++ b/tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice.xml @@ -1,4 +1,4 @@ -<domain type='qemu'> +<domain type='qemu' id='7'> <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> <memory unit='KiB'>219136</memory> @@ -8,6 +8,9 @@ <type arch='x86_64' machine='pc'>hvm</type> <boot dev='hd'/> </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> @@ -15,15 +18,29 @@ <devices> <emulator>/usr/bin/qemu-system-x86_64</emulator> <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> + <alias name='ide0-0-0'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> - <controller type='usb' index='0'/> - <controller type='ide' index='0'/> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> + <controller type='usb' index='0' model='piix3-uhci'> + <alias name='usb'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <alias name='ide'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci.0'/> + </controller> + <input type='mouse' bus='ps2'> + <alias name='input0'/> + </input> + <input type='keyboard' bus='ps2'> + <alias name='input1'/> + </input> <graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1' defaultMode='secure'> <listen type='address' address='127.0.0.1'/> <channel name='main' mode='secure'/> @@ -36,12 +53,20 @@ <clipboard copypaste='no'/> <filetransfer enable='no'/> </graphics> + <audio id='1' type='spice'/> <video> - <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1'/> + <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1' primary='yes'/> + <alias name='video0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> <video> <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1'/> + <alias name='video1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </video> - <memballoon model='virtio'/> + <memballoon model='virtio'> + <alias name='balloon0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> </devices> </domain> -- 2.39.2

Just like we check the resulting domain XML after ATTACH and DETACH, we should do the same after UPDATE action. This is as simple as calling testQemuHotplugCheckResult() and providing missing XMLs. For those test cases where no change is done, we can just make the expected XML a symlink to the input XML.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuhotplugtest.c | 9 +- ...hotplug-disk-cdrom+disk-cdrom-nochange.xml | 1 + .../qemuhotplug-disk-cdrom.xml | 30 ++++- ...graphics-spice+graphics-spice-nochange.xml | 1 + ...graphics-spice-listen-network-password.xml | 71 +++++++++++ ...imeout+graphics-spice-timeout-nochange.xml | 1 + ...imeout+graphics-spice-timeout-password.xml | 117 ++++++++++++++++++ .../qemuhotplug-graphics-spice-timeout.xml | 50 ++++++-- .../qemuhotplug-graphics-spice.xml | 41 ++++-- 9 files changed, 295 insertions(+), 26 deletions(-) create mode 120000 tests/qemuhotplugtestdomains/qemuhotplug-disk-cdrom+disk-cdrom-nochange.xml create mode 120000 tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice+graphics-spice-nochange.xml create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-listen-network+graphics-spice-listen-network-password.xml create mode 120000 tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-timeout+graphics-spice-timeout-nochange.xml create mode 100644 tests/qemuhotplugtestdomains/qemuhotplug-graphics-spice-timeout+graphics-spice-timeout-password.xml
Reviewed-by: Kristina Hanicova <khanicov@redhat.com>
On Fri, Apr 21, 2023 at 10:25 AM Michal Privoznik <mprivozn@redhat.com> wrote: thanks for your impact Michal, Kristina
participants (2)
-
Kristina Hanicova
-
Michal Privoznik