[pve-devel] [PATCH v3 qemu 1/2] fix #5409: backup: fix copy-before-write timeout
Fiona Ebner
f.ebner at proxmox.com
Mon Apr 29 17:20:21 CEST 2024
The type for the copy-before-write timeout in nanoseconds was wrong.
By being just uint32_t, a maximum of slightly over 4 seconds was
possible. Larger values would overflow and thus the 45 seconds set by
Proxmox's backup with fleecing, resulted in effectively 2 seconds
timeout for copy-before-write operations.
Reported-by: Friedrich Weber <f.weber at proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
---
Changes in v3:
* rebase on master
Upstream submission of this patch:
https://lore.kernel.org/qemu-devel/20240429141934.442154-1-f.ebner@proxmox.com/T/#u
...e-write-use-uint64_t-for-timeout-in-.patch | 35 +++++++++++++++++++
...ock-copy-before-write-fix-permission.patch | 2 +-
...e-write-support-unligned-snapshot-di.patch | 2 +-
...e-write-create-block_copy-bitmap-in-.patch | 2 +-
...-backup-add-discard-source-parameter.patch | 4 +--
...e-allow-specifying-minimum-cluster-s.patch | 2 +-
...um-cluster-size-to-performance-optio.patch | 2 +-
debian/patches/series | 1 +
8 files changed, 43 insertions(+), 7 deletions(-)
create mode 100644 debian/patches/extra/0005-block-copy-before-write-use-uint64_t-for-timeout-in-.patch
diff --git a/debian/patches/extra/0005-block-copy-before-write-use-uint64_t-for-timeout-in-.patch b/debian/patches/extra/0005-block-copy-before-write-use-uint64_t-for-timeout-in-.patch
new file mode 100644
index 0000000..a8bdd85
--- /dev/null
+++ b/debian/patches/extra/0005-block-copy-before-write-use-uint64_t-for-timeout-in-.patch
@@ -0,0 +1,35 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Fiona Ebner <f.ebner at proxmox.com>
+Date: Mon, 29 Apr 2024 15:41:11 +0200
+Subject: [PATCH] block/copy-before-write: use uint64_t for timeout in
+ nanoseconds
+
+rather than the uint32_t for which the maximum is slightly more than 4
+seconds and larger values would overflow. The QAPI interface allows
+specifying the number of seconds, so only values 0 to 4 are safe right
+now, other values lead to a much lower timeout than a user expects.
+
+The block_copy() call where this is used already takes a uint64_t for
+the timeout, so no change required there.
+
+Fixes: 6db7fd1ca9 ("block/copy-before-write: implement cbw-timeout option")
+Reported-by: Friedrich Weber <f.weber at proxmox.com>
+Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
+Tested-by: Friedrich Weber <f.weber at proxmox.com>
+---
+ block/copy-before-write.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/block/copy-before-write.c b/block/copy-before-write.c
+index 8aba27a71d..026fa9840f 100644
+--- a/block/copy-before-write.c
++++ b/block/copy-before-write.c
+@@ -43,7 +43,7 @@ typedef struct BDRVCopyBeforeWriteState {
+ BlockCopyState *bcs;
+ BdrvChild *target;
+ OnCbwError on_cbw_error;
+- uint32_t cbw_timeout_ns;
++ uint64_t cbw_timeout_ns;
+
+ /*
+ * @lock: protects access to @access_bitmap, @done_bitmap and
diff --git a/debian/patches/pve/0044-block-copy-before-write-fix-permission.patch b/debian/patches/pve/0044-block-copy-before-write-fix-permission.patch
index 0d02af9..6a759a4 100644
--- a/debian/patches/pve/0044-block-copy-before-write-fix-permission.patch
+++ b/debian/patches/pve/0044-block-copy-before-write-fix-permission.patch
@@ -33,7 +33,7 @@ Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
-index 8aba27a71d..3e3af30c08 100644
+index 026fa9840f..5a9456d426 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -364,9 +364,13 @@ cbw_child_perm(BlockDriverState *bs, BdrvChild *c, BdrvChildRole role,
diff --git a/debian/patches/pve/0045-block-copy-before-write-support-unligned-snapshot-di.patch b/debian/patches/pve/0045-block-copy-before-write-support-unligned-snapshot-di.patch
index c689750..f651c58 100644
--- a/debian/patches/pve/0045-block-copy-before-write-support-unligned-snapshot-di.patch
+++ b/debian/patches/pve/0045-block-copy-before-write-support-unligned-snapshot-di.patch
@@ -15,7 +15,7 @@ Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
-index 3e3af30c08..6d89af0b29 100644
+index 5a9456d426..c0e70669a2 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -325,14 +325,24 @@ static int coroutine_fn GRAPH_RDLOCK
diff --git a/debian/patches/pve/0046-block-copy-before-write-create-block_copy-bitmap-in-.patch b/debian/patches/pve/0046-block-copy-before-write-create-block_copy-bitmap-in-.patch
index 0fcda2e..7cd24d0 100644
--- a/debian/patches/pve/0046-block-copy-before-write-create-block_copy-bitmap-in-.patch
+++ b/debian/patches/pve/0046-block-copy-before-write-create-block_copy-bitmap-in-.patch
@@ -49,7 +49,7 @@ index 9ee3dd7ef5..8fca2c3698 100644
if (!copy_bitmap) {
return NULL;
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
-index 6d89af0b29..ed2c228da7 100644
+index c0e70669a2..94db31512d 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -468,7 +468,7 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
diff --git a/debian/patches/pve/0047-qapi-blockdev-backup-add-discard-source-parameter.patch b/debian/patches/pve/0047-qapi-blockdev-backup-add-discard-source-parameter.patch
index eb34ad2..ef44f42 100644
--- a/debian/patches/pve/0047-qapi-blockdev-backup-add-discard-source-parameter.patch
+++ b/debian/patches/pve/0047-qapi-blockdev-backup-add-discard-source-parameter.patch
@@ -109,13 +109,13 @@ index 8fca2c3698..7e3b378528 100644
}
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
-index ed2c228da7..cd65524e26 100644
+index 94db31512d..853e01a1eb 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -44,6 +44,7 @@ typedef struct BDRVCopyBeforeWriteState {
BdrvChild *target;
OnCbwError on_cbw_error;
- uint32_t cbw_timeout_ns;
+ uint64_t cbw_timeout_ns;
+ bool discard_source;
/*
diff --git a/debian/patches/pve/0048-copy-before-write-allow-specifying-minimum-cluster-s.patch b/debian/patches/pve/0048-copy-before-write-allow-specifying-minimum-cluster-s.patch
index 5b80440..50a8cd2 100644
--- a/debian/patches/pve/0048-copy-before-write-allow-specifying-minimum-cluster-s.patch
+++ b/debian/patches/pve/0048-copy-before-write-allow-specifying-minimum-cluster-s.patch
@@ -82,7 +82,7 @@ index 7e3b378528..adb1cbb440 100644
return NULL;
}
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
-index cd65524e26..ac05a4993f 100644
+index 853e01a1eb..47b3cdd09f 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -477,7 +477,8 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
diff --git a/debian/patches/pve/0049-backup-add-minimum-cluster-size-to-performance-optio.patch b/debian/patches/pve/0049-backup-add-minimum-cluster-size-to-performance-optio.patch
index fe37687..fe3ff95 100644
--- a/debian/patches/pve/0049-backup-add-minimum-cluster-size-to-performance-optio.patch
+++ b/debian/patches/pve/0049-backup-add-minimum-cluster-size-to-performance-optio.patch
@@ -36,7 +36,7 @@ index 1963e47ab9..fe69723ada 100644
goto error;
}
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
-index ac05a4993f..d1e87f8cf4 100644
+index 47b3cdd09f..bba58326d7 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -546,6 +546,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
diff --git a/debian/patches/series b/debian/patches/series
index e955579..47e8307 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -2,6 +2,7 @@ extra/0001-monitor-qmp-fix-race-with-clients-disconnecting-earl.patch
extra/0002-scsi-megasas-Internal-cdbs-have-16-byte-length.patch
extra/0003-ide-avoid-potential-deadlock-when-draining-during-tr.patch
extra/0004-Revert-x86-acpi-workaround-Windows-not-handling-name.patch
+extra/0005-block-copy-before-write-use-uint64_t-for-timeout-in-.patch
bitmap-mirror/0001-drive-mirror-add-support-for-sync-bitmap-mode-never.patch
bitmap-mirror/0002-drive-mirror-add-support-for-conditional-and-always-.patch
bitmap-mirror/0003-mirror-add-check-for-bitmap-mode-without-bitmap.patch
--
2.39.2
More information about the pve-devel
mailing list