Fix dirty-bitmap PBS backup with multiple drives

"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@proxmox.com>
This commit is contained in:
Stefan Reiter 2020-07-14 10:45:16 +02:00 committed by Thomas Lamprecht
parent 9886892f10
commit f257cc05f4
3 changed files with 30 additions and 34 deletions

View File

@ -4,13 +4,14 @@ Date: Mon, 6 Jul 2020 20:05:16 +0200
Subject: [PATCH] PVE backup: rename incremental to use-dirty-bitmap Subject: [PATCH] PVE backup: rename incremental to use-dirty-bitmap
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
--- ---
pve-backup.c | 21 ++++++++++----------- pve-backup.c | 22 +++++++++++-----------
qapi/block-core.json | 6 +++--- 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 diff --git a/pve-backup.c b/pve-backup.c
index 7b5558e28e..246256f70f 100644 index 7b5558e28e..9e767d724c 100644
--- a/pve-backup.c --- a/pve-backup.c
+++ b/pve-backup.c +++ b/pve-backup.c
@@ -557,8 +557,8 @@ typedef struct QmpBackupTask { @@ -557,8 +557,8 @@ typedef struct QmpBackupTask {
@ -33,9 +34,11 @@ index 7b5558e28e..246256f70f 100644
char *pbs_err = NULL; char *pbs_err = NULL;
pbs = proxmox_backup_new( 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); BdrvDirtyBitmap *bitmap = bdrv_find_dirty_bitmap(di->bs, PBS_BITMAP_NAME);
+ bool expect_only_dirty = false;
- bool use_incremental = false; - bool use_incremental = false;
- if (incremental) { - if (incremental) {
@ -43,27 +46,25 @@ index 7b5558e28e..246256f70f 100644
if (bitmap == NULL) { if (bitmap == NULL) {
bitmap = bdrv_create_dirty_bitmap(di->bs, dump_cb_block_size, PBS_BITMAP_NAME, task->errp); bitmap = bdrv_create_dirty_bitmap(di->bs, dump_cb_block_size, PBS_BITMAP_NAME, task->errp);
if (!bitmap) { if (!bitmap) {
goto err; @@ -738,7 +738,7 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque)
}
/* mark entire bitmap as dirty to make full backup first */
+ use_dirty_bitmap = false;
bdrv_set_dirty_bitmap(bitmap, 0, di->size); bdrv_set_dirty_bitmap(bitmap, 0, di->size);
dirty += di->size; dirty += di->size;
} else { } else {
- use_incremental = true; - use_incremental = true;
+ expect_only_dirty = true;
dirty += bdrv_get_dirty_count(bitmap); dirty += bdrv_get_dirty_count(bitmap);
} }
di->bitmap = 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); 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_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) { if (dev_id < 0) {
goto err; 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.dirty = dirty;
backup_state.stat.transferred = 0; backup_state.stat.transferred = 0;
backup_state.stat.zero_bytes = 0; backup_state.stat.zero_bytes = 0;
@ -72,7 +73,7 @@ index 7b5558e28e..246256f70f 100644
qemu_mutex_unlock(&backup_state.stat.lock); 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_fingerprint, const char *fingerprint,
bool has_backup_id, const char *backup_id, bool has_backup_id, const char *backup_id,
bool has_backup_time, int64_t backup_time, bool has_backup_time, int64_t backup_time,
@ -81,7 +82,7 @@ index 7b5558e28e..246256f70f 100644
bool has_format, BackupFormat format, bool has_format, BackupFormat format,
bool has_config_file, const char *config_file, bool has_config_file, const char *config_file,
bool has_firewall_file, const char *firewall_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, .backup_id = backup_id,
.has_backup_time = has_backup_time, .has_backup_time = has_backup_time,
.backup_time = backup_time, .backup_time = backup_time,

View File

@ -9,10 +9,10 @@ Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
1 file changed, 6 insertions(+), 2 deletions(-) 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/pve-backup.c b/pve-backup.c diff --git a/pve-backup.c b/pve-backup.c
index 246256f70f..bda1635b82 100644 index 9e767d724c..c108f6a745 100644
--- a/pve-backup.c --- a/pve-backup.c
+++ b/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); dirty += bdrv_get_dirty_count(bitmap);
} }
di->bitmap = 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);

View File

@ -6,36 +6,31 @@ Subject: [PATCH] PVE: use proxmox_backup_check_incremental
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
--- ---
pve-backup.c | 11 ++++++++--- pve-backup.c | 12 ++++++++----
1 file changed, 8 insertions(+), 3 deletions(-) 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/pve-backup.c b/pve-backup.c diff --git a/pve-backup.c b/pve-backup.c
index bda1635b82..46191bb328 100644 index c108f6a745..aa62a1da16 100644
--- a/pve-backup.c --- a/pve-backup.c
+++ b/pve-backup.c +++ b/pve-backup.c
@@ -728,17 +728,22 @@ static void coroutine_fn pvebackup_co_prepare(void *opaque) @@ -734,12 +734,16 @@ 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);
if (!bitmap) { if (!bitmap) {
goto err; goto err;
} }
- /* mark entire bitmap as dirty to make full backup first */ - /* 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) { + if (expect_only_dirty) {
+ dirty += bdrv_get_dirty_count(bitmap); dirty += bdrv_get_dirty_count(bitmap);
+ } else { + } else {
+ /* mark entire bitmap as dirty to make full backup */ + /* mark entire bitmap as dirty to make full backup */
bdrv_set_dirty_bitmap(bitmap, 0, di->size); + bdrv_set_dirty_bitmap(bitmap, 0, di->size);
dirty += di->size; + dirty += di->size;
- } else {
- dirty += bdrv_get_dirty_count(bitmap);
} }
di->bitmap = bitmap; di->bitmap = bitmap;
} else { } else {