Firstly, our coding guidelines suggest using 'cleanup' label instead of 'end'. Then, @ret should be set to value representing success as the last statement before the 'cleanup' label. And while I am at this function, lets enumerate all the possible enum items (virDomainDiskDevice) and avoid using 'default' in switch(). Pooh. Also, nothing bad happens if we look up the disk to change in the domain upfront. In fact, it's going to be helpful later when we want to keep some old values for performing a rollback. Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> --- src/qemu/qemu_driver.c | 48 +++++++++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8946b20..6a8e863 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7925,48 +7925,54 @@ qemuDomainChangeDiskLive(virConnectPtr conn, int ret = -1; if (virStorageTranslateDiskSourcePool(conn, disk) < 0) - goto end; + goto cleanup; if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0) - goto end; + goto cleanup; - switch (disk->device) { + if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def, + disk->bus, disk->dst))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("No device with bus '%s' and target '%s'"), + virDomainDiskBusTypeToString(disk->bus), + disk->dst); + 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'"), - virDomainDiskBusTypeToString(disk->bus), - disk->dst); - goto end; - } - if (!virDomainDiskDiffersSourceOnly(disk, orig_disk)) - goto end; + goto cleanup; /* Add the new disk src into shared disk hash table */ if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0) - goto end; + goto cleanup; if (qemuDomainChangeEjectableMedia(driver, conn, vm, - orig_disk, dev->data.disk->src, force) < 0) { - ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk, vm->def->name)); - goto end; + orig_disk, disk->src, force) < 0) { + ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name)); + goto cleanup; } - dev->data.disk->src = NULL; - ret = 0; + disk->src = NULL; break; - default: + case VIR_DOMAIN_DISK_DEVICE_DISK: + case VIR_DOMAIN_DISK_DEVICE_LUN: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk bus '%s' cannot be updated."), virDomainDiskBusTypeToString(disk->bus)); + goto cleanup; + break; + + case VIR_DOMAIN_DISK_DEVICE_LAST: + /* nada */ break; } - end: + ret = 0; + cleanup: return ret; } -- 2.4.6 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list