From 30377cd627a919e51cc4bb60a8a57e94e73f016c Mon Sep 17 00:00:00 2001

Message-Id: <30377cd627a919e51cc4bb60a8a57e94e73f016c@dist-git>

From: Peter Krempa <pkrempa@redhat.com>

Date: Tue, 4 Feb 2020 15:08:12 +0100

Subject: [PATCH] qemu: checkpoint: Introduce support for deleting checkpoints

 accross snapshots

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

Allow deleting of checkpoints when snapshots were created along. The

code tracks and modifies the checkpoint list so that backups can still

be taken with such a backing chain. This unfortunately requires to

rename few bitmaps (by copying and deleting them) in some cases.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>

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

(cherry picked from commit 30bc426071c0f5957af01181c063cd68ade97899)

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

Message-Id: <e9f416811658b2ba03968ae5fca783b7fb7649f7.1580824112.git.pkrempa@redhat.com>

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

---

 src/qemu/qemu_checkpoint.c | 112 ++++++++++++++++++++++++++++---------

 src/qemu/qemu_checkpoint.h |   5 +-

 tests/qemublocktest.c      |  34 +++++++----

 3 files changed, 111 insertions(+), 40 deletions(-)

diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c

index e75cdd0458..55061bbf76 100644

--- a/src/qemu/qemu_checkpoint.c

+++ b/src/qemu/qemu_checkpoint.c

@@ -24,6 +24,7 @@

 #include "qemu_capabilities.h"

 #include "qemu_monitor.h"

 #include "qemu_domain.h"

+#include "qemu_block.h"

 #include "virerror.h"

 #include "virlog.h"

@@ -150,39 +151,92 @@ qemuCheckpointFindActiveDiskInParent(virDomainObjPtr vm,

 int

 qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,

+                                 virHashTablePtr blockNamedNodeData,

                                  const char *delbitmap,

                                  const char *parentbitmap,

-                                 bool chkcurrent,

-                                 virJSONValuePtr actions)

+                                 virJSONValuePtr actions,

+                                 const char *diskdst)

 {

-    if (parentbitmap) {

-        g_autoptr(virJSONValue) arr = NULL;

+    virStorageSourcePtr n = src;

-        if (!(arr = virJSONValueNewArray()))

-            return -1;

+    /* find the backing chain entry with bitmap named '@delbitmap' */

+    while (n) {

+        qemuBlockNamedNodeDataBitmapPtr tmp;

-        if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr,

-                                                             src->nodeformat,

-                                                             delbitmap) < 0)

-            return -1;

+        if ((tmp = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,

+                                                         n, delbitmap))) {

+            break;

+        }

-        if (chkcurrent) {

-            if (qemuMonitorTransactionBitmapEnable(actions,

-                                                   src->nodeformat,

-                                                   parentbitmap) < 0)

+        n = n->backingStore;

+    }

+

+    if (!n) {

+        virReportError(VIR_ERR_INTERNAL_ERROR,

+                       _("bitmap '%s' not found in backing chain of '%s'"),

+                       delbitmap, diskdst);

+        return -1;

+    }

+

+    while (n) {

+        qemuBlockNamedNodeDataBitmapPtr srcbitmap;

+

+        if (!(srcbitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,

+                                                                n, delbitmap)))

+            break;

+

+        /* For the actual checkpoint deletion we will merge any bitmap into the

+         * bitmap of the parent checkpoint (@parentbitmap) or for any image

+         * where the parent checkpoint bitmap is not present we must rename

+         * the bitmap of the deleted checkpoint into the bitmap of the parent

+         * checkpoint as qemu can't currently take the allocation map and turn

+         * it into a bitmap and thus we wouldn't be able to do a backup. */

+        if (parentbitmap) {

+            qemuBlockNamedNodeDataBitmapPtr dstbitmap;

+            g_autoptr(virJSONValue) arr = NULL;

+

+            dstbitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,

+                                                              n, parentbitmap);

+

+            if (dstbitmap) {

+                if (srcbitmap->recording && !dstbitmap->recording) {

+                    if (qemuMonitorTransactionBitmapEnable(actions,

+                                                           n->nodeformat,

+                                                           dstbitmap->name) < 0)

+                        return -1;

+                }

+

+            } else {

+                if (qemuMonitorTransactionBitmapAdd(actions,

+                                                    n->nodeformat,

+                                                    parentbitmap,

+                                                    true,

+                                                    !srcbitmap->recording,

+                                                    srcbitmap->granularity) < 0)

+                    return -1;

+            }

+

+            if (!(arr = virJSONValueNewArray()))

+                return -1;

+

+            if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(arr,

+                                                                 n->nodeformat,

+                                                                 srcbitmap->name) < 0)

+                return -1;

+

+            if (qemuMonitorTransactionBitmapMerge(actions,

+                                                  n->nodeformat,

+                                                  parentbitmap, &arr) < 0)

                 return -1;

         }

-        if (qemuMonitorTransactionBitmapMerge(actions,

-                                              src->nodeformat,

-                                              parentbitmap, &arr) < 0)

+        if (qemuMonitorTransactionBitmapRemove(actions,

+                                               n->nodeformat,

+                                               srcbitmap->name) < 0)

             return -1;

-    }

-    if (qemuMonitorTransactionBitmapRemove(actions,

-                                           src->nodeformat,

-                                           delbitmap) < 0)

-        return -1;

+        n = n->backingStore;

+    }

     return 0;

 }

@@ -191,11 +245,11 @@ qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,

 static int

 qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,

                              virDomainCheckpointDefPtr chkdef,

-                             bool chkcurrent,

                              virDomainMomentObjPtr parent)

 {

     qemuDomainObjPrivatePtr priv = vm->privateData;

     virQEMUDriverPtr driver = priv->driver;

+    g_autoptr(virHashTable) blockNamedNodeData = NULL;

     int rc;

     g_autoptr(virJSONValue) actions = NULL;

     size_t i;

@@ -203,6 +257,11 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,

     if (!(actions = virJSONValueNewArray()))

         return -1;

+    qemuDomainObjEnterMonitor(driver, vm);

+    blockNamedNodeData = qemuMonitorBlockGetNamedNodeData(priv->mon);

+    if (qemuDomainObjExitMonitor(priv->driver, vm) < 0 || !blockNamedNodeData)

+        return -1;

+

     for (i = 0; i < chkdef->ndisks; i++) {

         virDomainCheckpointDiskDef *chkdisk = &chkdef->disks[i];

         virDomainDiskDefPtr domdisk = virDomainDiskByTarget(vm->def, chkdisk->name);

@@ -223,8 +282,9 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,

                                                                   chkdisk->name)))

             parentbitmap = parentchkdisk->name;

-        if (qemuCheckpointDiscardDiskBitmaps(domdisk->src, chkdisk->bitmap,

-                                             parentbitmap, chkcurrent, actions) < 0)

+        if (qemuCheckpointDiscardDiskBitmaps(domdisk->src, blockNamedNodeData,

+                                             chkdisk->bitmap, parentbitmap,

+                                             actions, domdisk->dst) < 0)

             return -1;

     }

@@ -262,7 +322,7 @@ qemuCheckpointDiscard(virQEMUDriverPtr driver,

         virDomainCheckpointDefPtr chkdef = virDomainCheckpointObjGetDef(chk);

         parent = virDomainCheckpointFindByName(vm->checkpoints,

                                                chk->def->parent_name);

-        if (qemuCheckpointDiscardBitmaps(vm, chkdef, chkcurrent, parent) < 0)

+        if (qemuCheckpointDiscardBitmaps(vm, chkdef, parent) < 0)

             return -1;

     }

diff --git a/src/qemu/qemu_checkpoint.h b/src/qemu/qemu_checkpoint.h

index 85fd453d50..976b1eed0f 100644

--- a/src/qemu/qemu_checkpoint.h

+++ b/src/qemu/qemu_checkpoint.h

@@ -74,7 +74,8 @@ qemuCheckpointRollbackMetadata(virDomainObjPtr vm,

 int

 qemuCheckpointDiscardDiskBitmaps(virStorageSourcePtr src,

+                                 virHashTablePtr blockNamedNodeData,

                                  const char *delbitmap,

                                  const char *parentbitmap,

-                                 bool chkcurrent,

-                                 virJSONValuePtr actions);

+                                 virJSONValuePtr actions,

+                                 const char *diskdst);

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c

index edaf82053d..e56f813424 100644

--- a/tests/qemublocktest.c

+++ b/tests/qemublocktest.c

@@ -704,6 +704,7 @@ struct testQemuCheckpointDeleteMergeData {

     virStorageSourcePtr chain;

     const char *deletebitmap;

     const char *parentbitmap;

+    const char *nodedatafile;

 };

@@ -714,22 +715,30 @@ testQemuCheckpointDeleteMerge(const void *opaque)

     g_autofree char *actual = NULL;

     g_autofree char *expectpath = NULL;

     g_autoptr(virJSONValue) actions = NULL;

-    bool currentcheckpoint;

+    g_autoptr(virJSONValue) nodedatajson = NULL;

+    g_autoptr(virHashTable) nodedata = NULL;

     expectpath = g_strdup_printf("%s/%s%s-out.json", abs_srcdir,

                                  checkpointDeletePrefix, data->name);

+    if (!(nodedatajson = virTestLoadFileJSON(bitmapDetectPrefix, data->nodedatafile,

+                                             ".json", NULL)))

+        return -1;

+

+    if (!(nodedata = qemuMonitorJSONBlockGetNamedNodeDataJSON(nodedatajson))) {

+        VIR_TEST_VERBOSE("failed to load nodedata JSON\n");

+        return -1;

+    }

+

     if (!(actions = virJSONValueNewArray()))

         return -1;

-    /* hack to get the 'current' state until the function stops accepting it */

-    currentcheckpoint = STREQ("current", data->deletebitmap);

-

     if (qemuCheckpointDiscardDiskBitmaps(data->chain,

+                                         nodedata,

                                          data->deletebitmap,

                                          data->parentbitmap,

-                                         currentcheckpoint,

-                                         actions) < 0) {

+                                         actions,

+                                         "testdisk") < 0) {

         VIR_TEST_VERBOSE("failed to generate checkpoint delete transaction\n");

         return -1;

     }

@@ -988,22 +997,23 @@ mymain(void)

     TEST_BACKUP_BITMAP_CALCULATE("snapshot-intermediate", bitmapSourceChain, "d", "snapshots");

     TEST_BACKUP_BITMAP_CALCULATE("snapshot-deep", bitmapSourceChain, "a", "snapshots");

-#define TEST_CHECKPOINT_DELETE_MERGE(testname, delbmp, parbmp) \

+#define TEST_CHECKPOINT_DELETE_MERGE(testname, delbmp, parbmp, named) \

     do { \

         checkpointdeletedata.name = testname; \

         checkpointdeletedata.chain = bitmapSourceChain; \

         checkpointdeletedata.deletebitmap = delbmp; \

         checkpointdeletedata.parentbitmap = parbmp; \

+        checkpointdeletedata.nodedatafile = named; \

         if (virTestRun("checkpoint delete " testname, \

                        testQemuCheckpointDeleteMerge, &checkpointdeletedata) < 0) \

         ret = -1; \

     } while (0)

-    TEST_CHECKPOINT_DELETE_MERGE("basic-noparent", "a", NULL);

-    TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate1", "b", "a");

-    TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate2", "c", "b");

-    TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate3", "d", "c");

-    TEST_CHECKPOINT_DELETE_MERGE("basic-current", "current", "d");

+    TEST_CHECKPOINT_DELETE_MERGE("basic-noparent", "a", NULL, "basic");

+    TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate1", "b", "a", "basic");

+    TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate2", "c", "b", "basic");

+    TEST_CHECKPOINT_DELETE_MERGE("basic-intermediate3", "d", "c", "basic");

+    TEST_CHECKPOINT_DELETE_MERGE("basic-current", "current", "d", "basic");

  cleanup:

     virHashFree(diskxmljsondata.schema);

-- 

2.25.0