qemu: backup: Rewrite backup bitmap handling to the new bitmap semantics

Reuse qemuBlockGetBitmapMergeActions which allows removal of the ad-hoc
implementation of bitmap merging for backup. The new approach is simpler
and also more robust in case some of the bitmaps break as they remove
the dependency on the whole chain of bitmaps working.

The new approach also allows backups if a snapshot is created outside of
libvirt.

Additionally the code is greatly simplified.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>

diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 06490401c421e77ea9ee9590813f70d244165489..db8b2d8ff9c5fdd2601109156b6ab20163e9845d 100644 (file)
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -173,199 +173,58 @@ qemuBackupDiskDataCleanup(virDomainObjPtr vm,
 }
 
 
-/**
- * qemuBackupBeginCollectIncrementalCheckpoints:
- * @vm: domain object
- * @incrFrom: name of checkpoint representing starting point of incremental backup
- *
- * Returns a NULL terminated list of pointers to checkpoint definitions in
- * chronological order starting from the 'current' checkpoint until reaching
- * @incrFrom.
- */
-static virDomainMomentDefPtr *
-qemuBackupBeginCollectIncrementalCheckpoints(virDomainObjPtr vm,
-                                             const char *incrFrom)
-{
-    virDomainMomentObjPtr n = virDomainCheckpointGetCurrent(vm->checkpoints);
-    g_autofree virDomainMomentDefPtr *incr = NULL;
-    size_t nincr = 0;
-
-    while (n) {
-        virDomainMomentDefPtr def = n->def;
-
-        if (VIR_APPEND_ELEMENT_COPY(incr, nincr, def) < 0)
-            return NULL;
-
-        if (STREQ(def->name, incrFrom)) {
-            def = NULL;
-            if (VIR_APPEND_ELEMENT_COPY(incr, nincr, def) < 0)
-                return NULL;
-
-            return g_steal_pointer(&incr);
-        }
-
-        if (!n->def->parent_name)
-            break;
-
-        n = virDomainCheckpointFindByName(vm->checkpoints, n->def->parent_name);
-    }
-
-    virReportError(VIR_ERR_OPERATION_INVALID,
-                   _("could not locate checkpoint '%s' for incremental backup"),
-                   incrFrom);
-    return NULL;
-}
-
-
-static int
-qemuBackupGetBitmapMergeRange(virStorageSourcePtr from,
-                              const char *bitmapname,
-                              virJSONValuePtr *actions,
-                              virStorageSourcePtr *to,
-                              const char *diskdst,
-                              virHashTablePtr blockNamedNodeData)
+int
+qemuBackupDiskPrepareOneBitmapsChain(virStorageSourcePtr backingChain,
+                                     virStorageSourcePtr targetsrc,
+                                     const char *targetbitmap,
+                                     const char *incremental,
+                                     virJSONValuePtr actions,
+                                     virHashTablePtr blockNamedNodeData)
 {
-    g_autoptr(virJSONValue) act = virJSONValueNewArray();
-    virStorageSourcePtr tmpsrc = NULL;
-    virStorageSourcePtr n;
-    bool foundbitmap = false;
+    g_autoptr(virJSONValue) tmpactions = NULL;
 
-    for (n = from; virStorageSourceIsBacking(n); n = n->backingStore) {
-        qemuBlockNamedNodeDataBitmapPtr bitmap = NULL;
-
-        if (!(bitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
-                                                             n,
-                                                             bitmapname)))
-            break;
-
-        foundbitmap = true;
-
-        if (bitmap->inconsistent) {
-            virReportError(VIR_ERR_INVALID_ARG,
-                           _("bitmap '%s' for image '%s%u' is inconsistent"),
-                           bitmap->name, diskdst, n->id);
-            return -1;
-        }
-
-        if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(act,
-                                                             n->nodeformat,
-                                                             bitmapname) < 0)
-            return -1;
-
-        tmpsrc = n;
-    }
-
-    if (!foundbitmap) {
-        virReportError(VIR_ERR_INVALID_ARG,
-                       _("failed to find bitmap '%s' in image '%s%u'"),
-                       bitmapname, diskdst, from->id);
+    if (qemuBlockGetBitmapMergeActions(backingChain, NULL, targetsrc,
+                                       incremental, targetbitmap, NULL,
+                                       &tmpactions,
+                                       blockNamedNodeData) < 0)
         return -1;
-    }
 
-    *actions = g_steal_pointer(&act);
-    *to = tmpsrc;
+    if (tmpactions &&
+        virJSONValueArrayConcat(actions, tmpactions) < 0)
+        return -1;
 
     return 0;
 }
 
 
-virJSONValuePtr
-qemuBackupDiskPrepareOneBitmapsChain(virDomainMomentDefPtr *incremental,
-                                     virStorageSourcePtr backingChain,
-                                     virHashTablePtr blockNamedNodeData,
-                                     const char *diskdst)
-{
-    g_autoptr(virJSONValue) ret = NULL;
-    size_t incridx = 0;
-    virStorageSourcePtr n = backingChain;
-
-    ret = virJSONValueNewArray();
-
-    for (incridx = 0; incremental[incridx]; incridx++) {
-        g_autoptr(virJSONValue) tmp = virJSONValueNewArray();
-        virStorageSourcePtr tmpsrc = NULL;
-        virDomainCheckpointDefPtr chkdef = (virDomainCheckpointDefPtr) incremental[incridx];
-        bool checkpoint_has_disk = false;
-        size_t i;
-
-        for (i = 0; i < chkdef->ndisks; i++) {
-            if (STRNEQ_NULLABLE(diskdst, chkdef->disks[i].name))
-                continue;
-
-            if (chkdef->disks[i].type == VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
-                checkpoint_has_disk = true;
-
-            break;
-        }
-
-        if (!checkpoint_has_disk) {
-            if (!incremental[incridx + 1]) {
-                virReportError(VIR_ERR_INVALID_ARG,
-                               _("disk '%s' not found in checkpoint '%s'"),
-                               diskdst, incremental[incridx]->name);
-                return NULL;
-            }
-
-            continue;
-        }
-
-        if (qemuBackupGetBitmapMergeRange(n, incremental[incridx]->name,
-                                          &tmp, &tmpsrc, diskdst,
-                                          blockNamedNodeData) < 0)
-            return NULL;
-
-        if (virJSONValueArrayConcat(ret, tmp) < 0)
-            return NULL;
-
-        n = tmpsrc;
-    }
-
-    return g_steal_pointer(&ret);
-}
-
-
 static int
 qemuBackupDiskPrepareOneBitmaps(struct qemuBackupDiskData *dd,
                                 virJSONValuePtr actions,
-                                virDomainMomentDefPtr *incremental,
                                 virHashTablePtr blockNamedNodeData)
 {
-    g_autoptr(virJSONValue) mergebitmaps = NULL;
-    g_autoptr(virJSONValue) mergebitmapsstore = NULL;
-
-    if (!(mergebitmaps = qemuBackupDiskPrepareOneBitmapsChain(incremental,
-                                                              dd->domdisk->src,
-                                                              blockNamedNodeData,
-                                                              dd->domdisk->dst)))
-        return -1;
-
-    if (!(mergebitmapsstore = virJSONValueCopy(mergebitmaps)))
-        return -1;
-
-    if (qemuMonitorTransactionBitmapAdd(actions,
-                                        dd->domdisk->src->nodeformat,
-                                        dd->incrementalBitmap,
-                                        false,
-                                        true, 0) < 0)
-        return -1;
-
-    if (qemuMonitorTransactionBitmapMerge(actions,
-                                          dd->domdisk->src->nodeformat,
-                                          dd->incrementalBitmap,
-                                          &mergebitmaps) < 0)
+    if (!qemuBlockBitmapChainIsValid(dd->domdisk->src,
+                                     dd->backupdisk->incremental,
+                                     blockNamedNodeData)) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("missing or broken bitmap '%s' for disk '%s'"),
+                       dd->backupdisk->incremental, dd->domdisk->dst);
         return -1;
+    }
 
-    if (qemuMonitorTransactionBitmapAdd(actions,
-                                        dd->store->nodeformat,
-                                        dd->incrementalBitmap,
-                                        false,
-                                        true, 0) < 0)
+    if (qemuBackupDiskPrepareOneBitmapsChain(dd->domdisk->src,
+                                             dd->domdisk->src,
+                                             dd->incrementalBitmap,
+                                             dd->backupdisk->incremental,
+                                             actions,
+                                             blockNamedNodeData) < 0)
         return -1;
 
-    if (qemuMonitorTransactionBitmapMerge(actions,
-                                          dd->store->nodeformat,
-                                          dd->incrementalBitmap,
-                                          &mergebitmapsstore) < 0)
+    if (qemuBackupDiskPrepareOneBitmapsChain(dd->domdisk->src,
+                                             dd->store,
+                                             dd->incrementalBitmap,
+                                             dd->backupdisk->incremental,
+                                             actions,
+                                             blockNamedNodeData) < 0)
         return -1;
 
     return 0;
@@ -382,7 +241,6 @@ qemuBackupDiskPrepareDataOne(virDomainObjPtr vm,
                              virQEMUDriverConfigPtr cfg)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
-    g_autofree virDomainMomentDefPtr *incremental = NULL;
 
     /* set data structure */
     dd->backupdisk = backupdisk;
@@ -421,16 +279,12 @@ qemuBackupDiskPrepareDataOne(virDomainObjPtr vm,
         return -1;
 
     if (dd->backupdisk->incremental) {
-        if (!(incremental = qemuBackupBeginCollectIncrementalCheckpoints(vm, dd->backupdisk->incremental)))
-            return -1;
-
         if (dd->backupdisk->exportbitmap)
             dd->incrementalBitmap = g_strdup(dd->backupdisk->exportbitmap);
         else
             dd->incrementalBitmap = g_strdup_printf("backup-%s", dd->domdisk->dst);
 
-        if (qemuBackupDiskPrepareOneBitmaps(dd, actions, incremental,
-                                            blockNamedNodeData) < 0)
+        if (qemuBackupDiskPrepareOneBitmaps(dd, actions, blockNamedNodeData) < 0)
             return -1;
     }
 
@@ -884,8 +738,8 @@ qemuBackupBegin(virDomainObjPtr vm,
     if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, QEMU_ASYNC_JOB_BACKUP)))
         goto endjob;
 
-    if ((ndd = qemuBackupDiskPrepareData(vm, def, blockNamedNodeData,
-                                         actions, cfg, &dd)) <= 0) {
+    if ((ndd = qemuBackupDiskPrepareData(vm, def, blockNamedNodeData, actions,
+                                         cfg, &dd)) <= 0) {
         if (ndd == 0) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("no disks selected for backup"));

diff --git a/src/qemu/qemu_backup.h b/src/qemu/qemu_backup.h
index b19c3bf1c9f6a8865a7f3d0f72a389103830b5dc..075fde709bbc514c3aa50acff14ede653839c848 100644 (file)
--- a/src/qemu/qemu_backup.h
+++ b/src/qemu/qemu_backup.h
@@ -53,8 +53,10 @@ qemuBackupGetJobInfoStats(virQEMUDriverPtr driver,
                           qemuDomainJobInfoPtr jobInfo);
 
 /* exported for testing */
-virJSONValuePtr
-qemuBackupDiskPrepareOneBitmapsChain(virDomainMomentDefPtr *incremental,
-                                     virStorageSourcePtr backingChain,
-                                     virHashTablePtr blockNamedNodeData,
-                                     const char *diskdst);
+int
+qemuBackupDiskPrepareOneBitmapsChain(virStorageSourcePtr backingChain,
+                                     virStorageSourcePtr targetsrc,
+                                     const char *targetbitmap,
+                                     const char *incremental,
+                                     virJSONValuePtr actions,
+                                     virHashTablePtr blockNamedNodeData);

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 9e142a88516e8f46aec7cf40929da2be08319d9c..30662f6f79bce33e4d0268352fd073e2b6950d58 100644 (file)
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -716,65 +716,6 @@ testQemuBitmapGetFakeChainEntry(virStorageSourcePtr src,
 }
 
 
-typedef virDomainMomentDefPtr testMomentList;
-
-static void
-testMomentListFree(testMomentList *list)
-{
-    testMomentList *tmp = list;
-
-    if (!list)
-        return;
-
-    while (*tmp) {
-        virObjectUnref(*tmp);
-        tmp++;
-    }
-
-    g_free(list);
-}
-
-G_DEFINE_AUTOPTR_CLEANUP_FUNC(testMomentList, testMomentListFree);
-
-static virDomainMomentDefPtr
-testQemuBackupGetIncrementalMoment(const char *name)
-{
-    virDomainCheckpointDefPtr checkpoint = NULL;
-
-    if (!(checkpoint = virDomainCheckpointDefNew()))
-        abort();
-
-    checkpoint->disks = g_new0(virDomainCheckpointDiskDef, 1);
-    checkpoint->ndisks = 1;
-
-    checkpoint->disks[0].name = g_strdup("testdisk");
-    checkpoint->disks[0].type = VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP;
-
-    checkpoint->parent.name = g_strdup(name);
-
-    return (virDomainMomentDefPtr) checkpoint;
-}
-
-
-static virDomainMomentDefPtr *
-testQemuBackupGetIncremental(const char *incFrom)
-{
-    const char *checkpoints[] = {"current", "d", "c", "b", "a"};
-    virDomainMomentDefPtr *incr;
-    size_t i;
-
-    incr = g_new0(virDomainMomentDefPtr, G_N_ELEMENTS(checkpoints) + 1);
-
-    for (i = 0; i < G_N_ELEMENTS(checkpoints); i++) {
-        incr[i] = testQemuBackupGetIncrementalMoment(checkpoints[i]);
-
-        if (STREQ(incFrom, checkpoints[i]))
-            break;
-    }
-
-    return incr;
-}
-
 static const char *backupDataPrefix = "qemublocktestdata/backupmerge/";
 
 struct testQemuBackupIncrementalBitmapCalculateData {
@@ -791,10 +732,10 @@ testQemuBackupIncrementalBitmapCalculate(const void *opaque)
     const struct testQemuBackupIncrementalBitmapCalculateData *data = opaque;
     g_autoptr(virJSONValue) nodedatajson = NULL;
     g_autoptr(virHashTable) nodedata = NULL;
-    g_autoptr(virJSONValue) mergebitmaps = NULL;
-    g_autofree char *actual = NULL;
+    g_autoptr(virJSONValue) actions = virJSONValueNewArray();
     g_autofree char *expectpath = NULL;
-    g_autoptr(testMomentList) incremental = NULL;
+    g_autoptr(virStorageSource) target = NULL;
+    g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
 
     expectpath = g_strdup_printf("%s/%s%s-out.json", abs_srcdir,
                                  backupDataPrefix, data->name);
@@ -808,19 +749,24 @@ testQemuBackupIncrementalBitmapCalculate(const void *opaque)
         return -1;
     }
 
-    incremental = testQemuBackupGetIncremental(data->incremental);
+    if (!(target = virStorageSourceNew()))
+        return -1;
 
-    if ((mergebitmaps = qemuBackupDiskPrepareOneBitmapsChain(incremental,
-                                                             data->chain,
-                                                             nodedata,
-                                                             "testdisk"))) {
-        if (!(actual = virJSONValueToString(mergebitmaps, true)))
+    target->nodeformat = g_strdup_printf("target_node");
+
+    if (qemuBackupDiskPrepareOneBitmapsChain(data->chain,
+                                             target,
+                                             "target-bitmap-name",
+                                             data->incremental,
+                                             actions,
+                                             nodedata) >= 0) {
+        if (virJSONValueToBuffer(actions, &buf, true) < 0)
             return -1;
     } else {
-        actual = g_strdup("NULL\n");
+        virBufferAddLit(&buf, "NULL\n");
     }
 
-    return virTestCompareToFile(actual, expectpath);
+    return virTestCompareToFile(virBufferCurrentContent(&buf), expectpath);
 }
 
 

diff --git a/tests/qemublocktestdata/backupmerge/empty-out.json b/tests/qemublocktestdata/backupmerge/empty-out.json
index 7951defec192aa41c72f62ac9c9f4b001cdaaba8..41b42e677b97324ff33450d619b6812449bc3abe 100644 (file)
--- a/tests/qemublocktestdata/backupmerge/empty-out.json
+++ b/tests/qemublocktestdata/backupmerge/empty-out.json
@@ -1 +1,3 @@
-NULL
+[
+
+]