[PATCH for 6.2.0] qemuDomainSnapshotDiskPrepareOne: Fix logic of relative backing store update

Peter Krempa posted 1 patch 4 years ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1c247857ad77862ad4e370fbb0e7b313bc94d174.1585806242.git.pkrempa@redhat.com
src/qemu/qemu_driver.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
[PATCH for 6.2.0] qemuDomainSnapshotDiskPrepareOne: Fix logic of relative backing store update
Posted by Peter Krempa 4 years ago
Commit 2ace7a87a8aced68c250 introduced a logic bug by an improperly
modified condition where we'd skip to the else branch when reusing of
external images was requested and blockdev is available.

The original intentions were to skip the backing store update with
blockdev.

Fix it by only asserting the boolean which was used to track whether we
support update of the backing store only when blockdev is not present
along with the appropriate rename.

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

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

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 78024614cf..ff97f10f11 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15129,7 +15129,7 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver,
 {
     virDomainDiskDefPtr persistdisk;
     bool supportsCreate;
-    bool supportsBacking;
+    bool updateRelativeBacking = false;

     dd->disk = disk;

@@ -15158,19 +15158,22 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver,
     }

     supportsCreate = virStorageFileSupportsCreate(dd->src);
-    supportsBacking = virStorageFileSupportsBackingChainTraversal(dd->src);

-    if (supportsCreate || supportsBacking) {
+    /* relative backing store paths need to be updated so that relative
+     * block commit still works. With blockdev we must update it when doing
+     * commit anyways so it's skipped here */
+    if (!blockdev &&
+        virStorageFileSupportsBackingChainTraversal(dd->src))
+        updateRelativeBacking = true;
+
+    if (supportsCreate || updateRelativeBacking) {
         if (qemuDomainStorageFileInit(driver, vm, dd->src, NULL) < 0)
             return -1;

         dd->initialized = true;

-        /* relative backing store paths need to be updated so that relative
-         * block commit still works. With blockdev we must update it when doing
-         * commit anyways so it's skipped here */
-        if (reuse && !blockdev) {
-            if (supportsBacking) {
+        if (reuse) {
+            if (updateRelativeBacking) {
                 g_autofree char *backingStoreStr = NULL;

                 if (virStorageFileGetBackingStoreStr(dd->src, &backingStoreStr) < 0)
-- 
2.25.1

Re: [PATCH for 6.2.0] qemuDomainSnapshotDiskPrepareOne: Fix logic of relative backing store update
Posted by Ján Tomko 4 years ago
On a Thursday in 2020, Peter Krempa wrote:
>Commit 2ace7a87a8aced68c250 introduced a logic bug by an improperly
>modified condition where we'd skip to the else branch when reusing of
>external images was requested and blockdev is available.
>
>The original intentions were to skip the backing store update with
>blockdev.
>
>Fix it by only asserting the boolean which was used to track whether we
>support update of the backing store only when blockdev is not present
>along with the appropriate rename.
>
>https://bugzilla.redhat.com/show_bug.cgi?id=1820016
>
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/qemu/qemu_driver.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>

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

Jano