On 9/27/18 11:09 AM, Peter Krempa wrote:
Some internal helpers use only a virDomainDiskDef to do their job. If we want to change source for the disk we need to update it to the new source in the disk definition for those to work correctly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 86afda636e..3043b3257b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -730,9 +730,12 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, bool force) { qemuDomainObjPrivatePtr priv = vm->privateData; + virStorageSourcePtr oldsrc = disk->src; int ret = -1; int rc;
+ disk->src = newsrc; +
Doesn't seem like this would be right especially considering what each of the following calls passing both @disk and @newsrc do.
if (qemuHotplugPrepareDiskAccess(driver, vm, disk, newsrc, false) < 0) goto cleanup;
Passing both @disk and @newsrc here after assigning newsrc into disk->src would seem to do nothing since it temporarily swaps them for processing, then returns with them swapped back. Shortly after this passing both @disk and @newsrc into either qemuDomainChangeMediaBlockdev or qemuDomainChangeMediaLegacy would seem to be incorrect especially since each deals with what's in the drive first which would now be incorrect, wouldn't it?
@@ -744,7 +747,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, else rc = qemuDomainChangeMediaLegacy(driver, vm, disk, newsrc, force);
- virDomainAuditDisk(vm, disk->src, newsrc, "update", rc >= 0); + virDomainAuditDisk(vm, oldsrc, newsrc, "update", rc >= 0);
if (rc < 0) { ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, newsrc, true));
This portion would seemingly be problematic too. Hard to review the rest... John
@@ -755,7 +758,8 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name)); ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, true));
- virStorageSourceFree(disk->src); + virStorageSourceFree(oldsrc); + oldsrc = NULL; VIR_STEAL_PTR(disk->src, newsrc);
ignore_value(qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE)); @@ -763,6 +767,9 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, ret = 0;
cleanup: + if (oldsrc) + disk->src = oldsrc; + return ret; }