* [PATCH 0/2] qemu-img: fix getting stuck in infinite loop on
@ 2023-05-23 16:24 Andrey Drobyshev via
2023-05-23 16:24 ` [PATCH 1/2] qemu-img: rebase: stop when reaching EOF of old backing file Andrey Drobyshev via
2023-05-23 16:24 ` [PATCH 2/2] qemu-iotests: 024: add rebasing test case for overlay_size > backing_size Andrey Drobyshev via
0 siblings, 2 replies; 5+ messages in thread
From: Andrey Drobyshev via @ 2023-05-23 16:24 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, kwolf, shmuel.eiderman, andrey.drobyshev, den
Consider the following in-chain rebase case:
qemu-img create -f qcow2 base.qcow2 $(( 64 * 4 ))k
qemu-img create -f qcow2 -o backing_file=base.qcow2,backing_fmt=qcow2 inc1.qcow2 $(( 64 * 4 ))k
qemu-img create -f qcow2 -o backing_file=inc1.qcow2,backing_fmt=qcow2 inc2.qcow2 $(( 64 * 5 ))k
qemu-img rebase -f qcow2 -b base.qcow2 -F qcow2 inc2.qcow2
And then rebase operation gets stuck forever.
The 1st patch is a fix, the 2nd -- an additional test case to catch this
situation.
Andrey Drobyshev (2):
qemu-img: rebase: stop when reaching EOF of old backing file
qemu-iotests: 024: add rebasing test case for overlay_size >
backing_size
qemu-img.c | 7 ++++++
tests/qemu-iotests/024 | 48 ++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/024.out | 23 ++++++++++++++++++
3 files changed, 78 insertions(+)
--
2.31.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/2] qemu-img: rebase: stop when reaching EOF of old backing file 2023-05-23 16:24 [PATCH 0/2] qemu-img: fix getting stuck in infinite loop on Andrey Drobyshev via @ 2023-05-23 16:24 ` Andrey Drobyshev via 2023-05-24 8:30 ` Denis V. Lunev 2023-05-23 16:24 ` [PATCH 2/2] qemu-iotests: 024: add rebasing test case for overlay_size > backing_size Andrey Drobyshev via 1 sibling, 1 reply; 5+ messages in thread From: Andrey Drobyshev via @ 2023-05-23 16:24 UTC (permalink / raw) To: qemu-block; +Cc: qemu-devel, kwolf, shmuel.eiderman, andrey.drobyshev, den In case when we're rebasing within one backing chain, and when target image is larger than old backing file, bdrv_is_allocated_above() ends up setting *pnum = 0. As a result, target offset isn't getting incremented, and we get stuck in an infinite for loop. Let's detect this case and break the loop, as there's no more data to be read and merged from the old backing. Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com> --- qemu-img.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/qemu-img.c b/qemu-img.c index 27f48051b0..55b6ce407c 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -3813,6 +3813,13 @@ static int img_rebase(int argc, char **argv) strerror(-ret)); goto out; } + if (!n) { + /* + * We've reached EOF of the old backing, therefore there's + * no more mergeable data. + */ + break; + } if (!ret) { continue; } -- 2.31.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] qemu-img: rebase: stop when reaching EOF of old backing file 2023-05-23 16:24 ` [PATCH 1/2] qemu-img: rebase: stop when reaching EOF of old backing file Andrey Drobyshev via @ 2023-05-24 8:30 ` Denis V. Lunev 2023-05-25 10:37 ` Andrey Drobyshev 0 siblings, 1 reply; 5+ messages in thread From: Denis V. Lunev @ 2023-05-24 8:30 UTC (permalink / raw) To: Andrey Drobyshev, qemu-block; +Cc: qemu-devel, kwolf, shmuel.eiderman On 5/23/23 18:24, Andrey Drobyshev wrote: > In case when we're rebasing within one backing chain, and when target image > is larger than old backing file, bdrv_is_allocated_above() ends up setting > *pnum = 0. As a result, target offset isn't getting incremented, and we > get stuck in an infinite for loop. Let's detect this case and break the > loop, as there's no more data to be read and merged from the old backing. > > Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com> > --- > qemu-img.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/qemu-img.c b/qemu-img.c > index 27f48051b0..55b6ce407c 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -3813,6 +3813,13 @@ static int img_rebase(int argc, char **argv) > strerror(-ret)); > goto out; > } > + if (!n) { > + /* > + * We've reached EOF of the old backing, therefore there's > + * no more mergeable data. > + */ > + break; > + } > if (!ret) { > continue; > } nope. It seems that this is wrong patch. iris ~/tmp/1 $ qemu-img create -f qcow2 base.qcow2 $(( 65 * 4 ))k Formatting 'base.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=266240 lazy_refcounts=off refcount_bits=16 iris ~/tmp/1 $ qemu-img create -f qcow2 -o backing_file=base.qcow2,backing_fmt=qcow2 inc1.qcow2 $(( 64 * 4 )) Formatting 'inc1.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=256 backing_file=base.qcow2 backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16 iris ~/tmp/1 $ qemu-img create -f qcow2 -o backing_file=inc1.qcow2,backing_fmt=qcow2 inc2.qcow2 $(( 64 * 5 ))k Formatting 'inc2.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=327680 backing_file=inc1.qcow2 backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16 iris ~/tmp/1 $ qemu-io -c "write -P 0xae 256k 4k" base.qcow2 wrote 4096/4096 bytes at offset 262144 4 KiB, 1 ops; 00.01 sec (471.447 KiB/sec and 117.8617 ops/sec) iris ~/tmp/1 $ qemu-io -c "read -P 0xae 256k 4k" base.qcow2 read 4096/4096 bytes at offset 262144 4 KiB, 1 ops; 00.00 sec (56.076 MiB/sec and 14355.4407 ops/sec) iris ~/tmp/1 $ qemu-io -c "read -P 0xae 256k 4k" inc2.qcow2 Pattern verification failed at offset 262144, 4096 bytes read 4096/4096 bytes at offset 262144 4 KiB, 1 ops; 00.00 sec (827.771 MiB/sec and 211909.3028 ops/sec) iris ~/tmp/1 $ qemu-io -c "read -P 0 256k 4k" inc2.qcow2 read 4096/4096 bytes at offset 262144 4 KiB, 1 ops; 00.00 sec (838.611 MiB/sec and 214684.4139 ops/sec) iris ~/tmp/1 $ iris ~/tmp/1 $ /home/den/src/qemu/build/qemu-img rebase -f qcow2 -b base.qcow2 -F qcow2 inc2.qcow2 iris ~/tmp/1 $ qemu-io -c "read -P 0 256k 4k" inc2.qcow2 Pattern verification failed at offset 262144, 4096 bytes read 4096/4096 bytes at offset 262144 4 KiB, 1 ops; 00.00 sec (88.052 MiB/sec and 22541.3069 ops/sec) iris ~/tmp/1 $ the problem is the following: [----0xAE] <- base [----] <- inc1 [--------] <- inc2 In this case last 4k of base should be read as 0x00 (this offset is not written in inc2 and beyond end of the inc1). This means that all non-present clusters in inc2 MUST be zeroed during rebasing to base. Something like this... Den ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] qemu-img: rebase: stop when reaching EOF of old backing file 2023-05-24 8:30 ` Denis V. Lunev @ 2023-05-25 10:37 ` Andrey Drobyshev 0 siblings, 0 replies; 5+ messages in thread From: Andrey Drobyshev @ 2023-05-25 10:37 UTC (permalink / raw) To: Denis V. Lunev, qemu-block; +Cc: qemu-devel, kwolf, shmuel.eiderman On 5/24/23 11:30, Denis V. Lunev wrote: > On 5/23/23 18:24, Andrey Drobyshev wrote: >> In case when we're rebasing within one backing chain, and when target >> image >> is larger than old backing file, bdrv_is_allocated_above() ends up >> setting >> *pnum = 0. As a result, target offset isn't getting incremented, and we >> get stuck in an infinite for loop. Let's detect this case and break the >> loop, as there's no more data to be read and merged from the old backing. >> >> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com> >> --- >> qemu-img.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/qemu-img.c b/qemu-img.c >> index 27f48051b0..55b6ce407c 100644 >> --- a/qemu-img.c >> +++ b/qemu-img.c >> @@ -3813,6 +3813,13 @@ static int img_rebase(int argc, char **argv) >> strerror(-ret)); >> goto out; >> } >> + if (!n) { >> + /* >> + * We've reached EOF of the old backing, >> therefore there's >> + * no more mergeable data. >> + */ >> + break; >> + } >> if (!ret) { >> continue; >> } > nope. It seems that this is wrong patch. > > iris ~/tmp/1 $ qemu-img create -f qcow2 base.qcow2 $(( 65 * 4 ))k > Formatting 'base.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off > compression_type=zlib size=266240 lazy_refcounts=off refcount_bits=16 > iris ~/tmp/1 $ qemu-img create -f qcow2 -o > backing_file=base.qcow2,backing_fmt=qcow2 inc1.qcow2 $(( 64 * 4 )) > Formatting 'inc1.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off > compression_type=zlib size=256 backing_file=base.qcow2 backing_fmt=qcow2 > lazy_refcounts=off refcount_bits=16 > iris ~/tmp/1 $ qemu-img create -f qcow2 -o > backing_file=inc1.qcow2,backing_fmt=qcow2 inc2.qcow2 $(( 64 * 5 ))k > Formatting 'inc2.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off > compression_type=zlib size=327680 backing_file=inc1.qcow2 > backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16 > iris ~/tmp/1 $ qemu-io -c "write -P 0xae 256k 4k" base.qcow2 > wrote 4096/4096 bytes at offset 262144 > 4 KiB, 1 ops; 00.01 sec (471.447 KiB/sec and 117.8617 ops/sec) > iris ~/tmp/1 $ qemu-io -c "read -P 0xae 256k 4k" base.qcow2 > read 4096/4096 bytes at offset 262144 > 4 KiB, 1 ops; 00.00 sec (56.076 MiB/sec and 14355.4407 ops/sec) > iris ~/tmp/1 $ qemu-io -c "read -P 0xae 256k 4k" inc2.qcow2 > Pattern verification failed at offset 262144, 4096 bytes > read 4096/4096 bytes at offset 262144 > 4 KiB, 1 ops; 00.00 sec (827.771 MiB/sec and 211909.3028 ops/sec) > iris ~/tmp/1 $ qemu-io -c "read -P 0 256k 4k" inc2.qcow2 > read 4096/4096 bytes at offset 262144 > 4 KiB, 1 ops; 00.00 sec (838.611 MiB/sec and 214684.4139 ops/sec) > iris ~/tmp/1 $ > > > iris ~/tmp/1 $ /home/den/src/qemu/build/qemu-img rebase -f qcow2 -b > base.qcow2 -F qcow2 inc2.qcow2 > iris ~/tmp/1 $ qemu-io -c "read -P 0 256k 4k" inc2.qcow2 > Pattern verification failed at offset 262144, 4096 bytes > read 4096/4096 bytes at offset 262144 > 4 KiB, 1 ops; 00.00 sec (88.052 MiB/sec and 22541.3069 ops/sec) > iris ~/tmp/1 $ > > the problem is the following: > [----0xAE] <- base > [----] <- inc1 > [--------] <- inc2 > > In this case last 4k of base should be read as 0x00 (this offset is not > written in inc2 and beyond end of the inc1). > > This means that all non-present clusters in inc2 MUST be zeroed > during rebasing to base. > > Something like this... > > Den Thanks for pointing that out, it seems indeed that breaking the loop just yet isn't a good idea. Since the offsets beyond the old backing size were read as zeroes before the rebase, this should remain the case afterwards as well. Which means we should explicitly zero the overlay clusters which go beyond the old backing size. I'll alter the patches accordingly (including the test) and send v2. Andrey ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] qemu-iotests: 024: add rebasing test case for overlay_size > backing_size 2023-05-23 16:24 [PATCH 0/2] qemu-img: fix getting stuck in infinite loop on Andrey Drobyshev via 2023-05-23 16:24 ` [PATCH 1/2] qemu-img: rebase: stop when reaching EOF of old backing file Andrey Drobyshev via @ 2023-05-23 16:24 ` Andrey Drobyshev via 1 sibling, 0 replies; 5+ messages in thread From: Andrey Drobyshev via @ 2023-05-23 16:24 UTC (permalink / raw) To: qemu-block; +Cc: qemu-devel, kwolf, shmuel.eiderman, andrey.drobyshev, den Before previous commit, rebase was getting infitely stuck in case of rebasing within the same backing chain and when overlay_size > backing_size. Let's add this case to the rebasing test 024 to make sure it doesn't break again. Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com> --- tests/qemu-iotests/024 | 48 ++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/024.out | 23 ++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/tests/qemu-iotests/024 b/tests/qemu-iotests/024 index 25a564a150..5b6c2d973c 100755 --- a/tests/qemu-iotests/024 +++ b/tests/qemu-iotests/024 @@ -199,6 +199,54 @@ echo # $BASE_OLD and $BASE_NEW) $QEMU_IMG map "$OVERLAY" | _filter_qemu_img_map +# Check that rebase within the chain is working when +# overlay_size > old_backing_size +# +# base_new <-- base_old <-- overlay +# +# Backing (new): 11 11 11 11 +# Backing (old): 22 22 22 22 +# Overlay: -- -- -- -- -- +# +# As a result, overlay should contain data identical to base_old, with the +# last cluster remaining unallocated. + +echo +echo "=== Test rebase within one backing chain ===" +echo + +echo "Creating backing chain" +echo + +TEST_IMG=$BASE_NEW _make_test_img $(( CLUSTER_SIZE * 4 )) +TEST_IMG=$BASE_OLD _make_test_img -b "$BASE_NEW" -F $IMGFMT \ + $(( CLUSTER_SIZE * 4 )) +TEST_IMG=$OVERLAY _make_test_img -b "$BASE_OLD" -F $IMGFMT \ + $(( CLUSTER_SIZE * 5 )) + +echo +echo "Fill backing files with data" +echo + +$QEMU_IO "$BASE_NEW" -c "write -P 0x11 0 $(( CLUSTER_SIZE * 4 ))" \ + | _filter_qemu_io +$QEMU_IO "$BASE_OLD" -c "write -P 0x22 0 $(( CLUSTER_SIZE * 4 ))" \ + | _filter_qemu_io + +echo +echo "Rebase onto another image in the same chain" +echo + +$QEMU_IMG rebase -b "$BASE_NEW" -F $IMGFMT "$OVERLAY" + +# Verify the data is correct +$QEMU_IO "$OVERLAY" -c "read -P 0x22 0 $(( CLUSTER_SIZE * 4 ))" \ + | _filter_qemu_io + +echo + +# Verify that cluster #5 remained unallocated +$QEMU_IMG map "$OVERLAY" | _filter_qemu_img_map # success, all done echo "*** done" diff --git a/tests/qemu-iotests/024.out b/tests/qemu-iotests/024.out index 973a5a3711..fb63cac07c 100644 --- a/tests/qemu-iotests/024.out +++ b/tests/qemu-iotests/024.out @@ -171,4 +171,27 @@ read 65536/65536 bytes at offset 196608 Offset Length File 0 0x30000 TEST_DIR/subdir/t.IMGFMT 0x30000 0x10000 TEST_DIR/subdir/t.IMGFMT.base_new + +=== Test rebase within one backing chain === + +Creating backing chain + +Formatting 'TEST_DIR/subdir/t.IMGFMT.base_new', fmt=IMGFMT size=262144 +Formatting 'TEST_DIR/subdir/t.IMGFMT.base_old', fmt=IMGFMT size=262144 backing_file=TEST_DIR/subdir/t.IMGFMT.base_new backing_fmt=IMGFMT +Formatting 'TEST_DIR/subdir/t.IMGFMT', fmt=IMGFMT size=327680 backing_file=TEST_DIR/subdir/t.IMGFMT.base_old backing_fmt=IMGFMT + +Fill backing files with data + +wrote 262144/262144 bytes at offset 0 +256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 262144/262144 bytes at offset 0 +256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +Rebase onto another image in the same chain + +read 262144/262144 bytes at offset 0 +256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +Offset Length File +0 0x40000 TEST_DIR/subdir/t.IMGFMT *** done -- 2.31.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-05-25 10:37 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-05-23 16:24 [PATCH 0/2] qemu-img: fix getting stuck in infinite loop on Andrey Drobyshev via 2023-05-23 16:24 ` [PATCH 1/2] qemu-img: rebase: stop when reaching EOF of old backing file Andrey Drobyshev via 2023-05-24 8:30 ` Denis V. Lunev 2023-05-25 10:37 ` Andrey Drobyshev 2023-05-23 16:24 ` [PATCH 2/2] qemu-iotests: 024: add rebasing test case for overlay_size > backing_size Andrey Drobyshev via
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).