fix #5409: backup: fix copy-before-write timeout
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@proxmox.com> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
This commit is contained in:
		
							parent
							
								
									2cd560e0d2
								
							
						
					
					
						commit
						51232e2e40
					
				
							
								
								
									
										35
									
								
								debian/patches/extra/0005-block-copy-before-write-use-uint64_t-for-timeout-in-.patch
									
									
									
									
										vendored
									
									
										Normal file
									
								
							
							
						
						
									
										35
									
								
								debian/patches/extra/0005-block-copy-before-write-use-uint64_t-for-timeout-in-.patch
									
									
									
									
										vendored
									
									
										Normal file
									
								
							@ -0,0 +1,35 @@
 | 
				
			|||||||
 | 
					From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
 | 
				
			||||||
 | 
					From: Fiona Ebner <f.ebner@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@proxmox.com>
 | 
				
			||||||
 | 
					Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
 | 
				
			||||||
 | 
					Tested-by: Friedrich Weber <f.weber@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
 | 
				
			||||||
@ -33,7 +33,7 @@ Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
 | 
				
			|||||||
 1 file changed, 7 insertions(+), 3 deletions(-)
 | 
					 1 file changed, 7 insertions(+), 3 deletions(-)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
 | 
					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
 | 
					--- a/block/copy-before-write.c
 | 
				
			||||||
+++ b/block/copy-before-write.c
 | 
					+++ b/block/copy-before-write.c
 | 
				
			||||||
@@ -364,9 +364,13 @@ cbw_child_perm(BlockDriverState *bs, BdrvChild *c, BdrvChildRole role,
 | 
					@@ -364,9 +364,13 @@ cbw_child_perm(BlockDriverState *bs, BdrvChild *c, BdrvChildRole role,
 | 
				
			||||||
 | 
				
			|||||||
@ -15,7 +15,7 @@ Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
 | 
				
			|||||||
 1 file changed, 13 insertions(+), 3 deletions(-)
 | 
					 1 file changed, 13 insertions(+), 3 deletions(-)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
 | 
					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
 | 
					--- a/block/copy-before-write.c
 | 
				
			||||||
+++ b/block/copy-before-write.c
 | 
					+++ b/block/copy-before-write.c
 | 
				
			||||||
@@ -325,14 +325,24 @@ static int coroutine_fn GRAPH_RDLOCK
 | 
					@@ -325,14 +325,24 @@ static int coroutine_fn GRAPH_RDLOCK
 | 
				
			||||||
 | 
				
			|||||||
@ -49,7 +49,7 @@ index 9ee3dd7ef5..8fca2c3698 100644
 | 
				
			|||||||
     if (!copy_bitmap) {
 | 
					     if (!copy_bitmap) {
 | 
				
			||||||
         return NULL;
 | 
					         return NULL;
 | 
				
			||||||
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
 | 
					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
 | 
					--- a/block/copy-before-write.c
 | 
				
			||||||
+++ b/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,
 | 
					@@ -468,7 +468,7 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
 | 
				
			||||||
 | 
				
			|||||||
@ -109,13 +109,13 @@ index 8fca2c3698..7e3b378528 100644
 | 
				
			|||||||
 }
 | 
					 }
 | 
				
			||||||
 
 | 
					 
 | 
				
			||||||
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
 | 
					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
 | 
					--- a/block/copy-before-write.c
 | 
				
			||||||
+++ b/block/copy-before-write.c
 | 
					+++ b/block/copy-before-write.c
 | 
				
			||||||
@@ -44,6 +44,7 @@ typedef struct BDRVCopyBeforeWriteState {
 | 
					@@ -44,6 +44,7 @@ typedef struct BDRVCopyBeforeWriteState {
 | 
				
			||||||
     BdrvChild *target;
 | 
					     BdrvChild *target;
 | 
				
			||||||
     OnCbwError on_cbw_error;
 | 
					     OnCbwError on_cbw_error;
 | 
				
			||||||
     uint32_t cbw_timeout_ns;
 | 
					     uint64_t cbw_timeout_ns;
 | 
				
			||||||
+    bool discard_source;
 | 
					+    bool discard_source;
 | 
				
			||||||
 
 | 
					 
 | 
				
			||||||
     /*
 | 
					     /*
 | 
				
			||||||
 | 
				
			|||||||
@ -82,7 +82,7 @@ index 7e3b378528..adb1cbb440 100644
 | 
				
			|||||||
         return NULL;
 | 
					         return NULL;
 | 
				
			||||||
     }
 | 
					     }
 | 
				
			||||||
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
 | 
					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
 | 
					--- a/block/copy-before-write.c
 | 
				
			||||||
+++ b/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,
 | 
					@@ -477,7 +477,8 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
 | 
				
			||||||
 | 
				
			|||||||
@ -36,7 +36,7 @@ index 1963e47ab9..fe69723ada 100644
 | 
				
			|||||||
         goto error;
 | 
					         goto error;
 | 
				
			||||||
     }
 | 
					     }
 | 
				
			||||||
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
 | 
					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
 | 
					--- a/block/copy-before-write.c
 | 
				
			||||||
+++ b/block/copy-before-write.c
 | 
					+++ b/block/copy-before-write.c
 | 
				
			||||||
@@ -546,6 +546,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
 | 
					@@ -546,6 +546,7 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source,
 | 
				
			||||||
 | 
				
			|||||||
							
								
								
									
										1
									
								
								debian/patches/series
									
									
									
									
										vendored
									
									
								
							
							
						
						
									
										1
									
								
								debian/patches/series
									
									
									
									
										vendored
									
									
								
							@ -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/0002-scsi-megasas-Internal-cdbs-have-16-byte-length.patch
 | 
				
			||||||
extra/0003-ide-avoid-potential-deadlock-when-draining-during-tr.patch
 | 
					extra/0003-ide-avoid-potential-deadlock-when-draining-during-tr.patch
 | 
				
			||||||
extra/0004-Revert-x86-acpi-workaround-Windows-not-handling-name.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/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/0002-drive-mirror-add-support-for-conditional-and-always-.patch
 | 
				
			||||||
bitmap-mirror/0003-mirror-add-check-for-bitmap-mode-without-bitmap.patch
 | 
					bitmap-mirror/0003-mirror-add-check-for-bitmap-mode-without-bitmap.patch
 | 
				
			||||||
 | 
				
			|||||||
		Loading…
	
		Reference in New Issue
	
	Block a user