[pve-devel] [PATCH qemu v5 04/32] PVE backup: implement bitmap support for external backup access

[Fiona Ebner <f.ebner@proxmox.com>]

There can be one dirty bitmap for each backup target ID (which are
tracked in the backup_access_bitmaps hash table). The QMP user can
specify the ID of the bitmap it likes to use. This ID is then compared
to the current one for the given target. If they match, the bitmap is
re-used (should it still exist on the drive, otherwise re-created). If
there is a mismatch, the old bitmap is removed and a new one is
created.

The return value of the QMP command includes information about what
bitmap action was taken. Similar to what the query-backup QMP command
returns for regular backup. It also includes the bitmap name and
associated block node, so the management layer can then set up an NBD
export with the bitmap.

While the backup access is active, a background bitmap is also
required. This is necessary to implement bitmap handling according to
the original reference [0]. In particular:

- in the error case, new writes since the backup access was set up are
  in the background bitmap. Because of failure, the previously tracked
  writes from the backup access bitmap are still required too. Thus,
  the bitmap is merged with the background bitmap to get all new
  writes since the last backup.

- in the success case, continue tracking for the next incremental
  backup in the backup access bitmap. New writes since the backup
  access was set up are in the background bitmap. Because the backup
  was successfully, clear the backup access bitmap and merge back the
  background bitmap to get only the new writes.

Since QEMU cannot know if the backup was successful or not (except if
failure already happens during the setup QMP command), the management
layer needs to tell it via the teardown QMP command.

The bitmap action is also recorded in the device info now.

[0]: https://lore.kernel.org/qemu-devel/b68833dd-8864-4d72-7c61-c134a9835036@ya.ru/

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes in v5:
* If bitmap with the expected, previously used name is missing, do not
  return PBS_BITMAP_ACTION_INVALID, but simply PBS_BITMAP_ACTION_NEW.
  This more closely aligns the handling with what is done for PBS. A
  new status should be used for this, see the next patch.

 pve-backup.c         | 175 ++++++++++++++++++++++++++++++++++++++++++-
 pve-backup.h         |   2 +-
 qapi/block-core.json |  22 +++++-
 system/runstate.c    |   2 +-
 4 files changed, 193 insertions(+), 8 deletions(-)

diff --git a/pve-backup.c b/pve-backup.c
index 8f0f9921ab..bcbb9eeb5b 100644
--- a/pve-backup.c
+++ b/pve-backup.c
@@ -15,6 +15,7 @@
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qerror.h"
 #include "qemu/cutils.h"
+#include "qemu/error-report.h"
 
 #if defined(CONFIG_MALLOC_TRIM)
 #include <malloc.h>
@@ -41,6 +42,7 @@
  */
 
 const char *PBS_BITMAP_NAME = "pbs-incremental-dirty-bitmap";
+const char *BACKGROUND_BITMAP_NAME = "backup-access-background-bitmap";
 
 static struct PVEBackupState {
     struct {
@@ -72,6 +74,7 @@ static struct PVEBackupState {
     CoMutex backup_mutex;
     CoMutex dump_callback_mutex;
     char *target_id;
+    GHashTable *backup_access_bitmaps; // key=target_id, value=bitmap_name
 } backup_state;
 
 static void pvebackup_init(void)
@@ -99,6 +102,8 @@ typedef struct PVEBackupDevInfo {
     char* device_name;
     int completed_ret; // INT_MAX if not completed
     BdrvDirtyBitmap *bitmap;
+    BdrvDirtyBitmap *background_bitmap; // used for external backup access
+    PBSBitmapAction bitmap_action;
     BlockDriverState *target;
     BlockJob *job;
 } PVEBackupDevInfo;
@@ -362,6 +367,67 @@ static void coroutine_fn pvebackup_co_complete_stream(void *opaque)
     qemu_co_mutex_unlock(&backup_state.backup_mutex);
 }
 
+/*
+ * New writes since the backup access was set up are in the background bitmap. Because of failure,
+ * the previously tracked writes in di->bitmap are still required too. Thus, merge with the
+ * background bitmap to get all new writes since the last backup.
+ */
+static void handle_backup_access_bitmaps_in_error_case(PVEBackupDevInfo *di)
+{
+    Error *local_err = NULL;
+
+    if (di->bs && di->background_bitmap) {
+        bdrv_drained_begin(di->bs);
+        if (di->bitmap) {
+            bdrv_enable_dirty_bitmap(di->bitmap);
+            if (!bdrv_merge_dirty_bitmap(di->bitmap, di->background_bitmap, NULL, &local_err)) {
+                warn_report("backup access: %s - could not merge bitmaps in error path - %s",
+                            di->device_name,
+                            local_err ? error_get_pretty(local_err) : "unknown error");
+                /*
+                 * Could not merge, drop original bitmap too.
+                 */
+                bdrv_release_dirty_bitmap(di->bitmap);
+            }
+        } else {
+            warn_report("backup access: %s - expected bitmap not present", di->device_name);
+        }
+        bdrv_release_dirty_bitmap(di->background_bitmap);
+        bdrv_drained_end(di->bs);
+    }
+}
+
+/*
+ * Continue tracking for next incremental backup in di->bitmap. New writes since the backup access
+ * was set up are in the background bitmap. Because the backup was successful, clear di->bitmap and
+ * merge back the background bitmap to get only the new writes.
+ */
+static void handle_backup_access_bitmaps_after_success(PVEBackupDevInfo *di)
+{
+    Error *local_err = NULL;
+
+    if (di->bs && di->background_bitmap) {
+        bdrv_drained_begin(di->bs);
+        if (di->bitmap) {
+            bdrv_enable_dirty_bitmap(di->bitmap);
+            bdrv_clear_dirty_bitmap(di->bitmap, NULL);
+            if (!bdrv_merge_dirty_bitmap(di->bitmap, di->background_bitmap, NULL, &local_err)) {
+                warn_report("backup access: %s - could not merge bitmaps after backup - %s",
+                            di->device_name,
+                            local_err ? error_get_pretty(local_err) : "unknown error");
+                /*
+                 * Could not merge, drop original bitmap too.
+                 */
+                bdrv_release_dirty_bitmap(di->bitmap);
+            }
+        } else {
+            warn_report("backup access: %s - expected bitmap not present", di->device_name);
+        }
+        bdrv_release_dirty_bitmap(di->background_bitmap);
+        bdrv_drained_end(di->bs);
+    }
+}
+
 static void cleanup_snapshot_access(PVEBackupDevInfo *di)
 {
     if (di->fleecing.snapshot_access) {
@@ -602,6 +668,21 @@ static void setup_all_snapshot_access_bh(void *opaque)
 
         bdrv_drained_begin(di->bs);
 
+        if (di->bitmap) {
+            BdrvDirtyBitmap *background_bitmap =
+                bdrv_create_dirty_bitmap(di->bs, PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE,
+                                         BACKGROUND_BITMAP_NAME, &local_err);
+            if (!background_bitmap) {
+                error_setg(errp, "%s - creating background bitmap for backup access failed: %s",
+                           di->device_name,
+                           local_err ? error_get_pretty(local_err) : "unknown error");
+                bdrv_drained_end(di->bs);
+                break;
+            }
+            di->background_bitmap = background_bitmap;
+            bdrv_disable_dirty_bitmap(di->bitmap);
+        }
+
         if (setup_snapshot_access(di, &local_err) < 0) {
             cleanup_snapshot_access(di);
             bdrv_drained_end(di->bs);
@@ -850,6 +931,7 @@ err:
 BackupAccessInfoList *coroutine_fn qmp_backup_access_setup(
     const char *target_id,
     const char *devlist,
+    const char *bitmap_name,
     Error **errp)
 {
     assert(qemu_in_coroutine());
@@ -909,6 +991,77 @@ BackupAccessInfoList *coroutine_fn qmp_backup_access_setup(
         backup_state.stat.bitmap_list = NULL;
     }
 
+    if (!backup_state.backup_access_bitmaps) {
+        backup_state.backup_access_bitmaps =
+            g_hash_table_new_full(g_str_hash, g_str_equal, free, free);
+    }
+
+    /* create bitmaps if requested */
+    l = di_list;
+    while (l) {
+        PVEBackupDevInfo *di = (PVEBackupDevInfo *)l->data;
+        l = g_list_next(l);
+
+        di->block_size = PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE;
+
+        PBSBitmapAction action = PBS_BITMAP_ACTION_NOT_USED;
+        size_t dirty = di->size;
+
+        const char *old_bitmap_name =
+            (const char*)g_hash_table_lookup(backup_state.backup_access_bitmaps, target_id);
+
+        bool same_bitmap_name =
+            old_bitmap_name && bitmap_name && strcmp(bitmap_name, old_bitmap_name) == 0;
+
+        if (old_bitmap_name && !same_bitmap_name) {
+            BdrvDirtyBitmap *old_bitmap = bdrv_find_dirty_bitmap(di->bs, old_bitmap_name);
+            if (!old_bitmap) {
+                warn_report("setup backup access: expected old bitmap '%s' not found for drive "
+                            "'%s'", old_bitmap_name, di->device_name);
+            } else {
+                g_hash_table_remove(backup_state.backup_access_bitmaps, target_id);
+                bdrv_release_dirty_bitmap(old_bitmap);
+                action = PBS_BITMAP_ACTION_NOT_USED_REMOVED;
+            }
+        }
+
+        BdrvDirtyBitmap *bitmap = NULL;
+        if (bitmap_name) {
+            bitmap = bdrv_find_dirty_bitmap(di->bs, bitmap_name);
+            if (!bitmap) {
+                bitmap = bdrv_create_dirty_bitmap(di->bs, PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE,
+                                                  bitmap_name, errp);
+                if (!bitmap) {
+                    qemu_mutex_unlock(&backup_state.stat.lock);
+                    goto err;
+                }
+                bdrv_set_dirty_bitmap(bitmap, 0, di->size);
+                action = PBS_BITMAP_ACTION_NEW;
+            } else {
+                /* track clean chunks as reused */
+                dirty = MIN(bdrv_get_dirty_count(bitmap), di->size);
+                backup_state.stat.reused += di->size - dirty;
+                action = PBS_BITMAP_ACTION_USED;
+            }
+
+            if (!same_bitmap_name) {
+                g_hash_table_insert(backup_state.backup_access_bitmaps,
+                                    strdup(target_id), strdup(bitmap_name));
+            }
+
+        }
+
+        PBSBitmapInfo *info = g_malloc(sizeof(*info));
+        info->drive = g_strdup(di->device_name);
+        info->action = action;
+        info->size = di->size;
+        info->dirty = dirty;
+        backup_state.stat.bitmap_list = g_list_append(backup_state.stat.bitmap_list, info);
+
+        di->bitmap = bitmap;
+        di->bitmap_action = action;
+    }
+
     /* initialize global backup_state now */
 
     if (backup_state.stat.error) {
@@ -978,6 +1131,12 @@ BackupAccessInfoList *coroutine_fn qmp_backup_access_setup(
         info->value->node_name = g_strdup(bdrv_get_node_name(di->fleecing.snapshot_access));
         info->value->device = g_strdup(di->device_name);
         info->value->size = di->size;
+        if (bitmap_name) {
+            info->value->bitmap_node_name = g_strdup(bdrv_get_node_name(di->bs));
+            info->value->bitmap_name = g_strdup(bitmap_name);
+            info->value->bitmap_action = di->bitmap_action;
+            info->value->has_bitmap_action = true;
+        }
 
         *p_bai_next = info;
         p_bai_next = &info->next;
@@ -992,6 +1151,8 @@ err:
         PVEBackupDevInfo *di = (PVEBackupDevInfo *)l->data;
         l = g_list_next(l);
 
+        handle_backup_access_bitmaps_in_error_case(di);
+
         g_free(di->device_name);
         di->device_name = NULL;
 
@@ -1007,7 +1168,7 @@ err:
 /*
  * Caller needs to hold the backup mutex or the BQL.
  */
-void backup_access_teardown(void)
+void backup_access_teardown(bool success)
 {
     GList *l = backup_state.di_list;
 
@@ -1028,6 +1189,12 @@ void backup_access_teardown(void)
             di->fleecing.cbw = NULL;
         }
 
+        if (success) {
+            handle_backup_access_bitmaps_after_success(di);
+        } else {
+            handle_backup_access_bitmaps_in_error_case(di);
+        }
+
         g_free(di->device_name);
         di->device_name = NULL;
 
@@ -1048,13 +1215,13 @@ static void backup_access_teardown_bh(void *opaque)
 {
     CoCtxData *data = (CoCtxData*)opaque;
 
-    backup_access_teardown();
+    backup_access_teardown(*((bool*)data->data));
 
     /* return */
     aio_co_enter(data->ctx, data->co);
 }
 
-void coroutine_fn qmp_backup_access_teardown(const char *target_id, Error **errp)
+void coroutine_fn qmp_backup_access_teardown(const char *target_id, bool success, Error **errp)
 {
     assert(qemu_in_coroutine());
 
@@ -1084,6 +1251,7 @@ void coroutine_fn qmp_backup_access_teardown(const char *target_id, Error **errp
     CoCtxData waker = {
         .co = qemu_coroutine_self(),
         .ctx = qemu_get_current_aio_context(),
+        .data = &success,
     };
     aio_bh_schedule_oneshot(waker.ctx, backup_access_teardown_bh, &waker);
     qemu_coroutine_yield();
@@ -1293,6 +1461,7 @@ UuidInfo coroutine_fn *qmp_backup(
             }
 
             di->dev_id = dev_id;
+            di->bitmap_action = action;
 
             PBSBitmapInfo *info = g_malloc(sizeof(*info));
             info->drive = g_strdup(di->device_name);
diff --git a/pve-backup.h b/pve-backup.h
index 4033bc848f..9ebeef7c8f 100644
--- a/pve-backup.h
+++ b/pve-backup.h
@@ -11,6 +11,6 @@
 #ifndef PVE_BACKUP_H
 #define PVE_BACKUP_H
 
-void backup_access_teardown(void);
+void backup_access_teardown(bool success);
 
 #endif /* PVE_BACKUP_H */
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 3f092221ce..63a5be0f3f 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1114,9 +1114,17 @@
 #
 # @size: the size of the block device in bytes.
 #
+# @bitmap-node-name: the block node name the dirty bitmap is associated to.
+#
+# @bitmap-name: the name of the dirty bitmap associated to the backup access.
+#
+# @bitmap-action: the action taken on the dirty bitmap.
+#
 ##
 { 'struct': 'BackupAccessInfo',
-  'data': { 'node-name': 'str', 'device': 'str', 'size': 'size' } }
+  'data': { 'node-name': 'str', 'device': 'str', 'size': 'size',
+            '*bitmap-node-name': 'str', '*bitmap-name': 'str',
+            '*bitmap-action': 'PBSBitmapAction' } }
 
 ##
 # @backup-access-setup:
@@ -1129,11 +1137,16 @@
 # @devlist: list of block device names (separated by ',', ';' or ':'). By
 #     default the backup includes all writable block devices.
 #
+# @bitmap-name: use/create a bitmap with this name. Re-using the same name
+#     allows for making incremental backups. Check the @bitmap-action in the
+#     result to see if you can actually re-use the bitmap or if it had to be
+#     newly created.
+#
 # Returns: a list of @BackupAccessInfo, one for each device.
 #
 ##
 { 'command': 'backup-access-setup',
-  'data': { 'target-id': 'str', '*devlist': 'str' },
+  'data': { 'target-id': 'str', '*devlist': 'str', '*bitmap-name': 'str' },
   'returns': [ 'BackupAccessInfo' ], 'coroutine': true }
 
 ##
@@ -1143,8 +1156,11 @@
 #
 # @target-id: the ID of the backup target.
 #
+# @success: whether the backup done by the external provider was successful.
+#
 ##
-{ 'command': 'backup-access-teardown', 'data': { 'target-id': 'str' },
+{ 'command': 'backup-access-teardown',
+  'data': { 'target-id': 'str', 'success': 'bool' },
   'coroutine': true }
 
 ##
diff --git a/system/runstate.c b/system/runstate.c
index 6f93d7c2fb..ef3277930f 100644
--- a/system/runstate.c
+++ b/system/runstate.c
@@ -925,7 +925,7 @@ void qemu_cleanup(int status)
      * The backup access is set up by a QMP command, but is neither owned by a monitor nor
      * associated to a BlockBackend. Need to tear it down manually here.
      */
-    backup_access_teardown();
+    backup_access_teardown(false);
     job_cancel_sync_all();
     bdrv_close_all();
 
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel