Disk hotplug has slightly different semantics from media changing. Move the media change code out and add proper initialization of the new source object and proper cleanups if something fails. Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx> --- src/qemu/qemu_driver.c | 15 +------- src/qemu/qemu_hotplug.c | 77 ++++++++++++++++++++++++++--------------- 2 files changed, 51 insertions(+), 41 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 91ac3640d3..a52e2495d5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7851,12 +7851,6 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm, virDomainDeviceDef oldDev = { .type = dev->type }; int ret = -1; - if (virDomainDiskTranslateSourcePool(disk) < 0) - goto cleanup; - - if (qemuDomainDetermineDiskChain(driver, vm, disk, true) < 0) - goto cleanup; - if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def, disk->bus, disk->dst))) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -7885,16 +7879,9 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm, goto cleanup; } - /* Add the new disk src into shared disk hash table */ - if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0) - goto cleanup; - if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk, - dev->data.disk->src, force) < 0) { - ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk, - vm->def->name)); + dev->data.disk->src, force) < 0) goto cleanup; - } dev->data.disk->src = NULL; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ed7076ea01..62470b1a2f 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -733,11 +733,23 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv = vm->privateData; virStorageSourcePtr oldsrc = disk->src; + bool sharedAdded = false; int ret = -1; int rc; disk->src = newsrc; + if (virDomainDiskTranslateSourcePool(disk) < 0) + goto cleanup; + + if (qemuAddSharedDisk(driver, disk, vm->def->name) < 0) + goto cleanup; + + sharedAdded = true; + + if (qemuDomainDetermineDiskChain(driver, vm, disk, true) < 0) + goto cleanup; + if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0) goto cleanup; @@ -754,10 +766,8 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, virDomainAuditDisk(vm, oldsrc, newsrc, "update", rc >= 0); - if (rc < 0) { - ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, newsrc, true)); + if (rc < 0) goto cleanup; - } /* remove the old source from shared device list */ disk->src = oldsrc; @@ -769,11 +779,21 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, oldsrc = NULL; disk->src = newsrc; - ignore_value(qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE)); - ret = 0; cleanup: + /* undo changes to the new disk */ + if (ret < 0) { + if (sharedAdded) + ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name)); + + ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, newsrc, true)); + } + + /* remove PR manager object if unneeded */ + ignore_value(qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE)); + + /* revert old image do the disk definition */ if (oldsrc) disk->src = oldsrc; @@ -1072,9 +1092,15 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriverPtr driver, { size_t i; virDomainDiskDefPtr disk = dev->data.disk; - virDomainDiskDefPtr orig_disk = NULL; int ret = -1; + if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM || + disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("cdrom/floppy device hotplug isn't supported")); + return -1; + } + if (virDomainDiskTranslateSourcePool(disk) < 0) goto cleanup; @@ -1088,27 +1114,6 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriverPtr driver, goto cleanup; switch ((virDomainDiskDevice) disk->device) { - case VIR_DOMAIN_DISK_DEVICE_CDROM: - case VIR_DOMAIN_DISK_DEVICE_FLOPPY: - if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def, - disk->bus, disk->dst))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("No device with bus '%s' and target '%s'. " - "cdrom and floppy device hotplug isn't supported " - "by libvirt"), - virDomainDiskBusTypeToString(disk->bus), - disk->dst); - goto cleanup; - } - - if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk, - disk->src, false) < 0) - goto cleanup; - - disk->src = NULL; - ret = 0; - break; - case VIR_DOMAIN_DISK_DEVICE_DISK: case VIR_DOMAIN_DISK_DEVICE_LUN: for (i = 0; i < vm->def->ndisks; i++) { @@ -1150,6 +1155,8 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriverPtr driver, } break; + case VIR_DOMAIN_DISK_DEVICE_CDROM: + case VIR_DOMAIN_DISK_DEVICE_FLOPPY: case VIR_DOMAIN_DISK_DEVICE_LAST: break; } @@ -1176,6 +1183,22 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev) { + virDomainDiskDefPtr disk = dev->data.disk; + virDomainDiskDefPtr orig_disk = NULL; + + /* this API overloads media change semantics on disk hotplug + * for devices supporting media changes */ + if ((disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM || + disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) && + (orig_disk = virDomainDiskFindByBusAndDst(vm->def, disk->bus, disk->dst))) { + if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk, + disk->src, false) < 0) + return -1; + + disk->src = NULL; + return 0; + } + return qemuDomainAttachDeviceDiskLiveInternal(driver, vm, dev); } -- 2.17.1 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list