From: Stefan Reiter <s.reiter@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH qemu] Fix dirty-bitmap PBS backup with multiple drives
Date: Tue, 14 Jul 2020 10:45:16 +0200 [thread overview]
Message-ID: <20200714084516.6321-1-s.reiter@proxmox.com> (raw)
"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>
---
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@proxmox.com>
+Signed-off-by: Stefan Reiter <s.reiter@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@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@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@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
next reply other threads:[~2020-07-14 8:45 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-14 8:45 Stefan Reiter [this message]
2020-07-14 9:04 ` [pbs-devel] applied: " Thomas Lamprecht
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200714084516.6321-1-s.reiter@proxmox.com \
--to=s.reiter@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.