[RFC qemu 8/9] PVE backup: track per-target dirty bitmaps

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

Allow users of the QMP 'backup' call to specify a target ID, so a
dirty bitmap can be tracked per target.

The PBS state migration needs to be guarded by machine version for
migration compatibility to nodes that don't yet have the feature. For
this to work, qemu-server is responsible to pass in the target ID when
the machine version is new enough.

Make the PBS state a proper object to make use of the existing
machinery for feature guarding by machine version.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 block/monitor/block-hmp-cmds.c |  1 +
 hw/core/machine.c              |  2 ++
 migration/pbs-state.c          | 42 ++++++++++++++++++++++++++++------
 migration/pbs-state.h          | 32 ++++++++++++++++++++++++++
 proxmox-backup-client.c        | 12 +++++++++-
 proxmox-backup-client.h        |  1 +
 pve-backup.c                   | 24 ++++++++++++++-----
 qapi/block-core.json           |  7 +++++-
 8 files changed, 106 insertions(+), 15 deletions(-)
 create mode 100644 migration/pbs-state.h

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 3b9c3d223e..2d72e2dacb 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -1052,6 +1052,7 @@ void coroutine_fn hmp_backup(Monitor *mon, const QDict *qdict)
         devlist, qdict_haskey(qdict, "speed"), speed,
         false, 0, // BackupPerf max-workers
         false, false, // fleecing
+        NULL, // target ID
         &error);
 
     hmp_handle_error(mon, error);
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 0aa77a57e9..ae1eb9c900 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -41,6 +41,8 @@
 GlobalProperty hw_compat_10_2[] = {
     { "scsi-block", "migrate-pr", "off" },
     { "isa-cirrus-vga", "global-vmstate", "true" },
+    // FIXME add to correct machine version
+    { "pbs-state", "per-target-bitmaps", "false" },
 };
 const size_t hw_compat_10_2_len = G_N_ELEMENTS(hw_compat_10_2);
 
diff --git a/migration/pbs-state.c b/migration/pbs-state.c
index a97187e4d7..3dbbfc8c68 100644
--- a/migration/pbs-state.c
+++ b/migration/pbs-state.c
@@ -5,14 +5,11 @@
 #include "qemu/osdep.h"
 #include "migration/misc.h"
 #include "qemu-file.h"
+#include "migration/pbs-state.h"
 #include "migration/vmstate.h"
 #include "migration/register.h"
 #include "proxmox-backup-qemu.h"
 
-typedef struct PBSState {
-    bool active;
-} PBSState;
-
 /* state is accessed via this static variable directly, 'opaque' is NULL */
 static PBSState pbs_state;
 
@@ -36,7 +33,7 @@ static int pbs_state_load(QEMUFile *f, void *opaque, int version_id)
         return -EIO;
     }
 
-    proxmox_import_state(buf, buf_size);
+    proxmox_import_state(buf, buf_size, pbs_state.per_target_bitmaps);
 
     free(buf);
     return 0;
@@ -46,7 +43,7 @@ static int pbs_state_load(QEMUFile *f, void *opaque, int version_id)
 static int pbs_state_save_setup(QEMUFile *f, void *opaque, Error **errp)
 {
     size_t buf_size;
-    uint8_t *buf = proxmox_export_state(&buf_size);
+    uint8_t *buf = proxmox_export_state(&buf_size, pbs_state.per_target_bitmaps);
 
     /* LV encoding */
     qemu_put_be64(f, buf_size);
@@ -97,8 +94,39 @@ static SaveVMHandlers savevm_pbs_state_handlers = {
 
 void pbs_state_mig_init(void)
 {
-    pbs_state.active = true;
+    object_initialize(&pbs_state, sizeof(pbs_state), TYPE_PBS_STATE);
     register_savevm_live("pbs-state", 0, 1,
                          &savevm_pbs_state_handlers,
                          NULL);
 }
+
+static void pbs_state_class_init(ObjectClass *klass, const void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->user_creatable = false;
+    device_class_set_props(dc, pbs_state_properties);
+}
+
+static const TypeInfo pbs_state_type = {
+    .name = TYPE_PBS_STATE,
+    /*
+     * NOTE: TYPE_PBS_STATE is not really a device, as the object is
+     * not created using qdev_new(), it is not attached to the qdev
+     * device tree, and it is never realized.
+     *
+     * TODO: Make this TYPE_OBJECT once QOM provides something like
+     * TYPE_DEVICE's "-global" properties.
+     */
+    .parent = TYPE_DEVICE,
+    .class_init = pbs_state_class_init,
+    .class_size = sizeof(PBSStateClass),
+    .instance_size = sizeof(PBSState),
+};
+
+static void register_pbs_state_type(void)
+{
+    type_register_static(&pbs_state_type);
+}
+
+type_init(register_pbs_state_type);
diff --git a/migration/pbs-state.h b/migration/pbs-state.h
new file mode 100644
index 0000000000..d2a3be8b0b
--- /dev/null
+++ b/migration/pbs-state.h
@@ -0,0 +1,32 @@
+/*
+ * PBS (dirty-bitmap) state migration
+ */
+
+#ifndef QEMU_MIGRATION_PBS_STATE_H
+#define QEMU_MIGRATION_PBS_STATE_H
+
+#include "hw/core/qdev-properties.h"
+#include "hw/core/qdev-properties-system.h"
+
+#define TYPE_PBS_STATE "pbs-state"
+
+typedef struct PBSStateClass PBSStateClass;
+OBJECT_DECLARE_TYPE(PBSState, PBSStateClass, PBS_STATE)
+
+struct PBSStateClass {
+    DeviceClass parent_class;
+};
+
+typedef struct PBSState {
+    DeviceState parent_obj;
+
+    bool active;
+    bool per_target_bitmaps;
+} PBSState;
+
+const Property pbs_state_properties[] = {
+    DEFINE_PROP_BOOL("active", PBSState, active, true),
+    DEFINE_PROP_BOOL("per-target-bitmaps", PBSState, per_target_bitmaps, true),
+};
+
+#endif
diff --git a/proxmox-backup-client.c b/proxmox-backup-client.c
index 166604b3a6..e617332fb4 100644
--- a/proxmox-backup-client.c
+++ b/proxmox-backup-client.c
@@ -58,6 +58,7 @@ proxmox_backup_co_register_image(
     ProxmoxBackupHandle *pbs,
     const char *device_name,
     uint64_t size,
+    const char *target_id,
     bool incremental,
     Error **errp)
 {
@@ -68,7 +69,16 @@ proxmox_backup_co_register_image(
     int pbs_res = -1;
 
     proxmox_backup_register_image_async(
-        pbs, device_name, size, incremental, proxmox_backup_schedule_wake, &waker, &pbs_res, &pbs_err);
+        pbs,
+        device_name,
+        size,
+        target_id,
+        incremental,
+        proxmox_backup_schedule_wake,
+        &waker,
+        &pbs_res,
+        &pbs_err
+    );
     qemu_coroutine_yield();
     if (pbs_res < 0) {
         if (errp) error_setg(errp, "backup register image failed: %s", pbs_err ? pbs_err : "unknown error");
diff --git a/proxmox-backup-client.h b/proxmox-backup-client.h
index 8cbf645b2c..3ef06b1a77 100644
--- a/proxmox-backup-client.h
+++ b/proxmox-backup-client.h
@@ -32,6 +32,7 @@ proxmox_backup_co_register_image(
     ProxmoxBackupHandle *pbs,
     const char *device_name,
     uint64_t size,
+    const char *target_id,
     bool incremental,
     Error **errp);
 
diff --git a/pve-backup.c b/pve-backup.c
index 8a97f0425e..0cfd1065ec 100644
--- a/pve-backup.c
+++ b/pve-backup.c
@@ -41,7 +41,6 @@
  *
  */
 
-const char *PBS_BITMAP_NAME = "pbs-incremental-dirty-bitmap";
 const char *BACKGROUND_BITMAP_NAME = "backup-access-background-bitmap";
 
 static struct PVEBackupState {
@@ -1356,6 +1355,7 @@ UuidInfo coroutine_fn *qmp_backup(
     bool has_speed, int64_t speed,
     bool has_max_workers, int64_t max_workers,
     bool has_fleecing, bool fleecing,
+    const char *target_id,
     Error **errp)
 {
     assert(qemu_in_coroutine());
@@ -1427,6 +1427,14 @@ UuidInfo coroutine_fn *qmp_backup(
     clear_backup_state_bitmap_list();
 
     if (format == BACKUP_FORMAT_PBS) {
+        const char *bitmap_name = NULL;
+
+        if (target_id) {
+            bitmap_name = target_id;
+        } else {
+            bitmap_name = "pbs-incremental-dirty-bitmap";
+        }
+
         if (!password) {
             error_set(errp, ERROR_CLASS_GENERIC_ERROR, "missing parameter 'password'");
             goto err_mutex;
@@ -1481,19 +1489,19 @@ UuidInfo coroutine_fn *qmp_backup(
             PBSBitmapAction action = PBS_BITMAP_ACTION_NOT_USED;
             size_t dirty = di->size;
 
-            BdrvDirtyBitmap *bitmap = bdrv_find_dirty_bitmap(di->bs, PBS_BITMAP_NAME);
+            BdrvDirtyBitmap *bitmap = bdrv_find_dirty_bitmap(di->bs, bitmap_name);
             bool expect_only_dirty = false;
 
             if (has_use_dirty_bitmap && use_dirty_bitmap) {
                 if (bitmap == NULL) {
-                    bitmap = bdrv_create_dirty_bitmap(di->bs, dump_cb_block_size, PBS_BITMAP_NAME, errp);
+                    bitmap = bdrv_create_dirty_bitmap(di->bs, dump_cb_block_size, bitmap_name, errp);
                     if (!bitmap) {
                         goto err_mutex;
                     }
                     action = PBS_BITMAP_ACTION_NEW;
                 } else {
                     expect_only_dirty =
-                        proxmox_backup_check_incremental(pbs, di->device_name, di->size) != 0;
+                        proxmox_backup_check_incremental(pbs, di->device_name, di->size, target_id) != 0;
                 }
 
                 if (expect_only_dirty) {
@@ -1517,7 +1525,7 @@ UuidInfo coroutine_fn *qmp_backup(
                 }
             }
 
-            int dev_id = proxmox_backup_co_register_image(pbs, di->device_name, di->size,
+            int dev_id = proxmox_backup_co_register_image(pbs, di->device_name, di->size, target_id,
                                                           expect_only_dirty, errp);
             if (dev_id < 0) {
                 goto err_mutex;
@@ -1595,7 +1603,11 @@ UuidInfo coroutine_fn *qmp_backup(
     backup_state.vmaw = vmaw;
     backup_state.pbs = pbs;
 
-    backup_state_set_target_id("Proxmox");
+    if (target_id) {
+        backup_state_set_target_id(target_id);
+    } else {
+        backup_state_set_target_id("Proxmox");
+    }
     backup_state.is_backup_access = false;
 
     backup_state.di_list = di_list;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index ed37a4a22f..fbad78c56c 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1058,6 +1058,10 @@
 #     @devlist, a corresponing '-fleecing' device with the same size
 #     already needs to be present.
 #
+# @target-id: the unique ID of the backup target.  A dirty bitmap is
+#     used for each target.  If no target ID is specified, the default
+#     dirty bitmap is used.
+#
 # Returns: the uuid of the backup job
 #
 ##
@@ -1079,7 +1083,8 @@
                                     '*devlist': 'str',
                                     '*speed': 'int',
                                     '*max-workers': 'int',
-                                    '*fleecing': 'bool' },
+                                    '*fleecing': 'bool',
+                                    '*target-id': 'str' },
   'returns': 'UuidInfo', 'coroutine': true }
 
 ##
-- 
2.47.3