From: Peter Krempa <pkrempa@redhat.com> Since we only do image locking on disk level 'qemuDomainStorageSourceAccessModify' is not the correct place as that is used also from various places which partially modify the image state e.g. with blockjobs. Move the locking code only to disk hot(un)plug code paths and directly apply the same fix as with <lease> where we don't acquire the lock if the VM isn't running. Locking is then handled when resuming the VM. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 12 ------------ src/qemu/qemu_hotplug.c | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3751deb85d..f6e36acb0f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6736,7 +6736,6 @@ qemuDomainStorageSourceAccessModify(virQEMUDriver *driver, bool revoke_label = false; bool revoke_namespace = false; bool revoke_nvme = false; - bool revoke_lockspace = false; bool revoke_nbdkit = false; VIR_DEBUG("src='%s' readonly=%d force_ro=%d force_rw=%d revoke=%d chain=%d", @@ -6755,17 +6754,11 @@ qemuDomainStorageSourceAccessModify(virQEMUDriver *driver, revoke_label = true; revoke_namespace = true; revoke_nvme = true; - revoke_lockspace = true; revoke_nbdkit = true; ret = 0; goto revoke; } - if (virDomainLockImageAttach(driver->lockManager, cfg->uri, vm, src) < 0) - goto revoke; - - revoke_lockspace = true; - if (!(flags & QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_MODIFY_ACCESS)) { if (qemuDomainStorageSourceAccessModifyNVMe(driver, vm, src, false) < 0) goto revoke; @@ -6826,11 +6819,6 @@ qemuDomainStorageSourceAccessModify(virQEMUDriver *driver, if (revoke_nvme) qemuDomainStorageSourceAccessModifyNVMe(driver, vm, src, true); - if (revoke_lockspace) { - if (virDomainLockImageDetach(driver->lockManager, vm, src) < 0) - VIR_WARN("Unable to release lock on %s", srcstr); - } - if (revoke_nbdkit) qemuNbdkitStopStorageSource(src, vm, chain); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 31726daf62..64226438fa 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -612,6 +612,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriver *driver, qemuDomainObjPrivate *priv = vm->privateData; virStorageSource *oldsrc = disk->src; qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + bool locked = false; int rc; if (diskPriv->blockjob && qemuBlockJobIsRunning(diskPriv->blockjob)) { @@ -634,6 +635,15 @@ qemuDomainChangeEjectableMedia(virQEMUDriver *driver, if (qemuDomainStorageSourceChainAccessAllow(driver, vm, newsrc) < 0) goto rollback; + /* If the VM is paused or in another state we don't actually want to + * lock the image yet. It will be done when the VM is resumed */ + if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { + if (virDomainLockImageAttach(driver->lockManager, cfg->uri, vm, newsrc) < 0) + goto rollback; + + locked = true; + } + if (qemuHotplugAttachManagedPR(vm, newsrc, VIR_ASYNC_JOB_NONE) < 0) goto rollback; @@ -655,6 +665,9 @@ qemuDomainChangeEjectableMedia(virQEMUDriver *driver, rollback: ignore_value(qemuDomainStorageSourceChainAccessRevoke(driver, vm, newsrc)); + if (locked) + ignore_value(virDomainLockImageDetach(driver->lockManager, vm, newsrc)); + qemuHotplugRemoveManagedPR(vm, newsrc, VIR_ASYNC_JOB_NONE); /* revert old image do the disk definition */ @@ -982,6 +995,7 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriver *driver, bool releaseUSB = false; bool releaseVirtio = false; bool releaseSeclabel = false; + bool releaseLock = false; int ret = -1; if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { @@ -1083,6 +1097,15 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriver *driver, releaseSeclabel = true; + /* If the VM is paused or in another state we don't actually want to + * lock the image yet. It will be done when the VM is resumed */ + if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { + if (virDomainLockImageAttach(driver->lockManager, cfg->uri, vm, disk->src) < 0) + goto cleanup; + + releaseLock = true; + } + if (qemuProcessPrepareHostStorageDisk(vm, disk) < 0) goto cleanup; @@ -1110,6 +1133,9 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriver *driver, if (releaseSeclabel) ignore_value(qemuDomainStorageSourceChainAccessRevoke(driver, vm, disk->src)); + if (releaseLock) + ignore_value(virDomainLockImageDetach(driver->lockManager, vm, disk->src)); + qemuHotplugRemoveManagedPR(vm, disk->src, VIR_ASYNC_JOB_NONE); } qemuDomainSecretDiskDestroy(disk); @@ -4895,9 +4921,15 @@ qemuDomainRemoveDiskDevice(virQEMUDriver *driver, qemuDomainReleaseDeviceAddress(vm, &disk->info); /* tear down disk security access */ - if (diskBackend) + if (diskBackend) { qemuDomainStorageSourceChainAccessRevoke(driver, vm, disk->src); + /* If the VM is paused or in another state the image is not locked anymore */ + if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { + ignore_value(virDomainLockImageDetach(driver->lockManager, vm, disk->src)); + } + } + qemuHotplugRemoveManagedPR(vm, disk->src, VIR_ASYNC_JOB_NONE); qemuNbdkitStopStorageSource(disk->src, vm, true); -- 2.54.0