[pbs-devel] [PATCH qemu] Fix dirty-bitmap PBS backup with multiple drives

Stefan Reiter s.reiter at proxmox.com
Tue Jul 14 10:45:16 CEST 2020


"PVE backup: rename incremental to use-dirty-bitmap" merged two
variables (use_dirty_bitmap and incremental) into one, but they served
two different purposes. Rename the original use_dirty_bitmap to
"expect_only_dirty" so the new one doesn't conflict, and rework "PVE:
use proxmox_backup_check_incremental" around that semantic.

In practice, this had the effect that only one disk at a time would
have a bitmap added, as after the first "use_dirty_bitmap" would be set
to one and the rest would behave as if the QMP parameter of the same
name was unset.

Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
---

I changed it in the original patches to avoid a fixup on top of fixups, hope
that's good.

 ...name-incremental-to-use-dirty-bitmap.patch | 27 ++++++++--------
 ...irty-counter-for-non-incremental-bac.patch |  6 ++--
 ...use-proxmox_backup_check_incremental.patch | 31 ++++++++-----------
 3 files changed, 30 insertions(+), 34 deletions(-)

diff --git a/debian/patches/pve/0039-PVE-backup-rename-incremental-to-use-dirty-bitmap.patch b/debian/patches/pve/0039-PVE-backup-rename-incremental-to-use-dirty-bitmap.patch
index eeab056..104699d 100644
--- a/debian/patches/pve/0039-PVE-backup-rename-incremental-to-use-dirty-bitmap.patch
+++ b/debian/patches/pve/0039-PVE-backup-rename-incremental-to-use-dirty-bitmap.patch
@@ -4,13 +4,14 @@ Date: Mon, 6 Jul 2020 20:05:16 +0200
 Subject: [PATCH] PVE backup: rename incremental to use-dirty-bitmap
 
 Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
+Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
 ---
- pve-backup.c         | 21 ++++++++++-----------
+ pve-backup.c         | 22 +++++++++++-----------
  qapi/block-core.json |  6 +++---
- 2 files changed, 13 insertions(+), 14 deletions(-)
+ 2 files changed, 14 insertions(+), 14 deletions(-)
 
 diff --git a/pve-backup.c b/pve-backup.c
-index 7b5558e28e..246256f70f 100644
+index 7b5558e28e..9e767d724c 100644
 --- a/pve-backup.c
 +++ b/pve-backup.c
 @@ -557,8 +557,8 @@ typedef struct QmpBackupTask {
@@ -33,9 +34,11 @@ index 7b5558e28e..246256f70f 100644
  
          char *pbs_err = NULL;
          pbs = proxmox_backup_new(
-@@ -727,18 +727,17 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
+@@ -726,9 +726,9 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
+             const char *devname = bdrv_get_device_name(di->bs);
  
              BdrvDirtyBitmap *bitmap = bdrv_find_dirty_bitmap(di->bs, PBS_BITMAP_NAME);
++            bool expect_only_dirty = false;
  
 -            bool use_incremental = false;
 -            if (incremental) {
@@ -43,27 +46,25 @@ index 7b5558e28e..246256f70f 100644
                  if (bitmap == NULL) {
                      bitmap = bdrv_create_dirty_bitmap(di->bs, dump_cb_block_size, PBS_BITMAP_NAME, task->errp);
                      if (!bitmap) {
-                         goto err;
-                     }
-                     /* mark entire bitmap as dirty to make full backup first */
-+                    use_dirty_bitmap = false;
+@@ -738,7 +738,7 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
                      bdrv_set_dirty_bitmap(bitmap, 0, di->size);
                      dirty += di->size;
                  } else {
 -                    use_incremental = true;
++                    expect_only_dirty = true;
                      dirty += bdrv_get_dirty_count(bitmap);
                  }
                  di->bitmap = bitmap;
-@@ -747,7 +746,7 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
+@@ -747,7 +747,7 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
                  bdrv_release_dirty_bitmap(bitmap);
              }
  
 -            int dev_id = proxmox_backup_co_register_image(pbs, devname, di->size, use_incremental, task->errp);
-+            int dev_id = proxmox_backup_co_register_image(pbs, devname, di->size, use_dirty_bitmap, task->errp);
++            int dev_id = proxmox_backup_co_register_image(pbs, devname, di->size, expect_only_dirty, task->errp);
              if (dev_id < 0) {
                  goto err;
              }
-@@ -865,7 +864,7 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
+@@ -865,7 +865,7 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
      backup_state.stat.dirty = dirty;
      backup_state.stat.transferred = 0;
      backup_state.stat.zero_bytes = 0;
@@ -72,7 +73,7 @@ index 7b5558e28e..246256f70f 100644
  
      qemu_mutex_unlock(&backup_state.stat.lock);
  
-@@ -934,7 +933,7 @@ UuidInfo *qmp_backup(
+@@ -934,7 +934,7 @@ UuidInfo *qmp_backup(
      bool has_fingerprint, const char *fingerprint,
      bool has_backup_id, const char *backup_id,
      bool has_backup_time, int64_t backup_time,
@@ -81,7 +82,7 @@ index 7b5558e28e..246256f70f 100644
      bool has_format, BackupFormat format,
      bool has_config_file, const char *config_file,
      bool has_firewall_file, const char *firewall_file,
-@@ -953,8 +952,8 @@ UuidInfo *qmp_backup(
+@@ -953,8 +953,8 @@ UuidInfo *qmp_backup(
          .backup_id = backup_id,
          .has_backup_time = has_backup_time,
          .backup_time = backup_time,
diff --git a/debian/patches/pve/0041-PVE-always-set-dirty-counter-for-non-incremental-bac.patch b/debian/patches/pve/0041-PVE-always-set-dirty-counter-for-non-incremental-bac.patch
index 5726233..9087e9e 100644
--- a/debian/patches/pve/0041-PVE-always-set-dirty-counter-for-non-incremental-bac.patch
+++ b/debian/patches/pve/0041-PVE-always-set-dirty-counter-for-non-incremental-bac.patch
@@ -9,10 +9,10 @@ Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
  1 file changed, 6 insertions(+), 2 deletions(-)
 
 diff --git a/pve-backup.c b/pve-backup.c
-index 246256f70f..bda1635b82 100644
+index 9e767d724c..c108f6a745 100644
 --- a/pve-backup.c
 +++ b/pve-backup.c
-@@ -741,9 +741,13 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
+@@ -742,9 +742,13 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
                      dirty += bdrv_get_dirty_count(bitmap);
                  }
                  di->bitmap = bitmap;
@@ -27,4 +27,4 @@ index 246256f70f..bda1635b82 100644
 +                }
              }
  
-             int dev_id = proxmox_backup_co_register_image(pbs, devname, di->size, use_dirty_bitmap, task->errp);
+             int dev_id = proxmox_backup_co_register_image(pbs, devname, di->size, expect_only_dirty, task->errp);
diff --git a/debian/patches/pve/0042-PVE-use-proxmox_backup_check_incremental.patch b/debian/patches/pve/0042-PVE-use-proxmox_backup_check_incremental.patch
index 95cd4b3..bbf32d0 100644
--- a/debian/patches/pve/0042-PVE-use-proxmox_backup_check_incremental.patch
+++ b/debian/patches/pve/0042-PVE-use-proxmox_backup_check_incremental.patch
@@ -6,36 +6,31 @@ Subject: [PATCH] PVE: use proxmox_backup_check_incremental
 Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
 Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
 ---
- pve-backup.c | 11 ++++++++---
- 1 file changed, 8 insertions(+), 3 deletions(-)
+ pve-backup.c | 12 ++++++++----
+ 1 file changed, 8 insertions(+), 4 deletions(-)
 
 diff --git a/pve-backup.c b/pve-backup.c
-index bda1635b82..46191bb328 100644
+index c108f6a745..aa62a1da16 100644
 --- a/pve-backup.c
 +++ b/pve-backup.c
-@@ -728,17 +728,22 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
-             BdrvDirtyBitmap *bitmap = bdrv_find_dirty_bitmap(di->bs, PBS_BITMAP_NAME);
- 
-             if (use_dirty_bitmap) {
-+                use_dirty_bitmap = proxmox_backup_check_incremental(pbs, devname, di->size) != 0;
-+
-                 if (bitmap == NULL) {
-                     bitmap = bdrv_create_dirty_bitmap(di->bs, dump_cb_block_size, PBS_BITMAP_NAME, task->errp);
+@@ -734,12 +734,16 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
                      if (!bitmap) {
                          goto err;
                      }
 -                    /* mark entire bitmap as dirty to make full backup first */
-                     use_dirty_bitmap = false;
+-                    bdrv_set_dirty_bitmap(bitmap, 0, di->size);
+-                    dirty += di->size;
+                 } else {
+-                    expect_only_dirty = true;
++                    expect_only_dirty = proxmox_backup_check_incremental(pbs, devname, di->size) != 0;
 +                }
 +
-+                if (use_dirty_bitmap) {
-+                    dirty += bdrv_get_dirty_count(bitmap);
++                if (expect_only_dirty) {
+                     dirty += bdrv_get_dirty_count(bitmap);
 +                } else {
 +                    /* mark entire bitmap as dirty to make full backup */
-                     bdrv_set_dirty_bitmap(bitmap, 0, di->size);
-                     dirty += di->size;
--                } else {
--                    dirty += bdrv_get_dirty_count(bitmap);
++                    bdrv_set_dirty_bitmap(bitmap, 0, di->size);
++                    dirty += di->size;
                  }
                  di->bitmap = bitmap;
              } else {
-- 
2.20.1






More information about the pbs-devel mailing list