Blob Blame History Raw
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