[libvirt] [PATCH v3 10/18] qemu: snapshot: Fix image lock handling when taking a snapshot

Peter Krempa posted 18 patches 6 years, 5 months ago
[libvirt] [PATCH v3 10/18] qemu: snapshot: Fix image lock handling when taking a snapshot
Posted by Peter Krempa 6 years, 5 months ago
When we take a snapshot we must properly remove our locking
infrastructure locks. This was broken by commit 3817fa10c4ad9 which
attempted to properly track the readonly state for the image as the
locking code was executed after this change. Since we forced the image
which was locked as read-write to read-only prior to unlocking it the
write lock was not dropped.

Fix it by moving the locking code prior to modifying the readonly flag.

https://bugzilla.redhat.com/show_bug.cgi?id=1745618

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_driver.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1fa8fe8391..ccbec5408d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15432,6 +15432,13 @@ qemuDomainSnapshotUpdateDiskSources(virQEMUDriverPtr driver,
     if (qemuSecurityMoveImageMetadata(driver, vm, dd->disk->src, dd->src) < 0)
         VIR_WARN("Unable to move disk metadata on vm %s", vm->def->name);

+    /* unlock the write lock on the original image as qemu will no longer write to it */
+    virDomainLockImageDetach(driver->lockManager, vm, dd->disk->src);
+
+    /* unlock also the new image if the VM is paused to follow the locking semantics */
+    if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING)
+        virDomainLockImageDetach(driver->lockManager, vm, dd->src);
+
     /* the old disk image is now readonly */
     dd->disk->src->readonly = true;

@@ -15550,23 +15557,8 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
     ret = 0;

  cleanup:
-    if (ret < 0) {
+    if (ret < 0)
         virErrorPreserveLast(&orig_err);
-    } else {
-        /* on successful snapshot we need to remove locks from the now-old
-         * disks and if the VM is paused release locks on the images since qemu
-         * stopped using them*/
-        bool paused = virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING;
-
-        for (i = 0; i < ndiskdata; i++) {
-            if (paused)
-                virDomainLockImageDetach(driver->lockManager, vm,
-                                         diskdata[i].disk->src);
-
-            virDomainLockImageDetach(driver->lockManager, vm,
-                                     diskdata[i].disk->src->backingStore);
-        }
-    }

     if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0 ||
         (vm->newDef && virDomainSaveConfig(cfg->configDir, driver->caps,
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 10/18] qemu: snapshot: Fix image lock handling when taking a snapshot
Posted by Ján Tomko 6 years, 5 months ago
On Tue, Sep 03, 2019 at 04:08:03PM +0200, Peter Krempa wrote:
>When we take a snapshot we must properly remove our locking
>infrastructure locks. This was broken by commit 3817fa10c4ad9 which
>attempted to properly track the readonly state for the image as the
>locking code was executed after this change. Since we forced the image
>which was locked as read-write to read-only prior to unlocking it the
>write lock was not dropped.
>
>Fix it by moving the locking code prior to modifying the readonly flag.
>
>https://bugzilla.redhat.com/show_bug.cgi?id=1745618
>
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/qemu/qemu_driver.c | 24 ++++++++----------------
> 1 file changed, 8 insertions(+), 16 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list