From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id B497D62C66 for ; Tue, 14 Jul 2020 10:45:23 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B394C24386 for ; Tue, 14 Jul 2020 10:45:23 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 90EF52437E for ; Tue, 14 Jul 2020 10:45:22 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 5D65D42BD3 for ; Tue, 14 Jul 2020 10:45:22 +0200 (CEST) From: Stefan Reiter To: pbs-devel@lists.proxmox.com Date: Tue, 14 Jul 2020 10:45:16 +0200 Message-Id: <20200714084516.6321-1-s.reiter@proxmox.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pbs-devel] [PATCH qemu] Fix dirty-bitmap PBS backup with multiple drives X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Jul 2020 08:45:23 -0000 "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 --- 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 +Signed-off-by: Stefan Reiter --- - 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 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 Signed-off-by: Thomas Lamprecht --- - 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