Re: [PATCH v2] qcow2: Fix preallocation on images with unaligned sizes
On 17.06.20 16:00, Alberto Garcia wrote: > When resizing an image with qcow2_co_truncate() using the falloc or > full preallocation modes the code assumes that both the old and new > sizes are cluster-aligned. > > There are two problems with this: > > 1) The calculation of how many clusters are involved does not always > get the right result. > > Example: creating a 60KB image and resizing it (with > preallocation=full) to 80KB won't allocate the second cluster. > > 2) No copy-on-write is performed, so in the previous example if > there is a backing file then the first 60KB of the first cluster > won't be filled with data from the backing file. > > This patch fixes both issues. > > Signed-off-by: Alberto Garcia > --- > v2: iotests: don't check the image size if data_file is set [Max] > > block/qcow2.c | 17 ++--- > tests/qemu-iotests/125 | 24 > tests/qemu-iotests/125.out | 9 + > 3 files changed, 47 insertions(+), 3 deletions(-) Thanks, applied to my block branch: https://git.xanclic.moe/XanClic/qemu/commits/branch/block signature.asc Description: OpenPGP digital signature
Re: [PATCH v2] qcow2: Fix preallocation on images with unaligned sizes
ping On Wed 17 Jun 2020 04:00:36 PM CEST, Alberto Garcia wrote: > When resizing an image with qcow2_co_truncate() using the falloc or > full preallocation modes the code assumes that both the old and new > sizes are cluster-aligned. > > There are two problems with this: > > 1) The calculation of how many clusters are involved does not always > get the right result. > > Example: creating a 60KB image and resizing it (with > preallocation=full) to 80KB won't allocate the second cluster. > > 2) No copy-on-write is performed, so in the previous example if > there is a backing file then the first 60KB of the first cluster > won't be filled with data from the backing file. > > This patch fixes both issues. > > Signed-off-by: Alberto Garcia
Re: [PATCH v2] qcow2: Fix preallocation on images with unaligned sizes
On 6/17/20 9:00 AM, Alberto Garcia wrote: When resizing an image with qcow2_co_truncate() using the falloc or full preallocation modes the code assumes that both the old and new sizes are cluster-aligned. There are two problems with this: 1) The calculation of how many clusters are involved does not always get the right result. Example: creating a 60KB image and resizing it (with preallocation=full) to 80KB won't allocate the second cluster. 2) No copy-on-write is performed, so in the previous example if there is a backing file then the first 60KB of the first cluster won't be filled with data from the backing file. This patch fixes both issues. Signed-off-by: Alberto Garcia --- v2: iotests: don't check the image size if data_file is set [Max] Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v2] qcow2: Fix preallocation on images with unaligned sizes
Patchew URL: https://patchew.org/QEMU/20200617140036.20311-1-be...@igalia.com/ Hi, This series failed the asan build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1 === TEST SCRIPT END === GEN docs/interop/qemu-qmp-ref.txt GEN docs/interop/qemu-qmp-ref.7 CC qga/commands.o /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) CC qga/guest-agent-command-state.o CC qga/main.o CC qga/commands-posix.o --- GEN docs/interop/qemu-ga-ref.html GEN docs/interop/qemu-ga-ref.txt GEN docs/interop/qemu-ga-ref.7 /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) AS pc-bios/optionrom/multiboot.o AS pc-bios/optionrom/linuxboot.o CC pc-bios/optionrom/linuxboot_dma.o --- SIGNpc-bios/optionrom/kvmvapic.bin SIGNpc-bios/optionrom/multiboot.bin SIGNpc-bios/optionrom/linuxboot.bin /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINKivshmem-server /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINKqemu-nbd BUILD pc-bios/optionrom/linuxboot_dma.img BUILD pc-bios/optionrom/pvh.img --- BUILD pc-bios/optionrom/pvh.raw SIGNpc-bios/optionrom/linuxboot_dma.bin SIGNpc-bios/optionrom/pvh.bin /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINKqemu-storage-daemon /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINKqemu-img /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINKqemu-io LINKqemu-edid /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINKfsdev/virtfs-proxy-helper LINKscsi/qemu-pr-helper /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINKqemu-bridge-helper /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINKvirtiofsd /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINKvhost-user-input
[PATCH v2] qcow2: Fix preallocation on images with unaligned sizes
When resizing an image with qcow2_co_truncate() using the falloc or full preallocation modes the code assumes that both the old and new sizes are cluster-aligned. There are two problems with this: 1) The calculation of how many clusters are involved does not always get the right result. Example: creating a 60KB image and resizing it (with preallocation=full) to 80KB won't allocate the second cluster. 2) No copy-on-write is performed, so in the previous example if there is a backing file then the first 60KB of the first cluster won't be filled with data from the backing file. This patch fixes both issues. Signed-off-by: Alberto Garcia --- v2: iotests: don't check the image size if data_file is set [Max] block/qcow2.c | 17 ++--- tests/qemu-iotests/125 | 24 tests/qemu-iotests/125.out | 9 + 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 0cd2e6757e..e20590c3b7 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -4239,8 +4239,8 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset, old_file_size = ROUND_UP(old_file_size, s->cluster_size); } -nb_new_data_clusters = DIV_ROUND_UP(offset - old_length, -s->cluster_size); +nb_new_data_clusters = (ROUND_UP(offset, s->cluster_size) - +start_of_cluster(s, old_length)) >> s->cluster_bits; /* This is an overestimation; we will not actually allocate space for * these in the file but just make sure the new refcount structures are @@ -4317,10 +4317,21 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset, int64_t nb_clusters = MIN( nb_new_data_clusters, s->l2_slice_size - offset_to_l2_slice_index(s, guest_offset)); -QCowL2Meta allocation = { +unsigned cow_start_length = offset_into_cluster(s, guest_offset); +QCowL2Meta allocation; +guest_offset = start_of_cluster(s, guest_offset); +allocation = (QCowL2Meta) { .offset = guest_offset, .alloc_offset = host_offset, .nb_clusters = nb_clusters, +.cow_start= { +.offset = 0, +.nb_bytes = cow_start_length, +}, +.cow_end = { +.offset = nb_clusters << s->cluster_bits, +.nb_bytes = 0, +}, }; qemu_co_queue_init(_requests); diff --git a/tests/qemu-iotests/125 b/tests/qemu-iotests/125 index d510984045..7cb1c19730 100755 --- a/tests/qemu-iotests/125 +++ b/tests/qemu-iotests/125 @@ -164,6 +164,30 @@ for GROWTH_SIZE in 16 48 80; do done done +# Test image resizing using preallocation and unaligned offsets +$QEMU_IMG create -f raw "$TEST_IMG.base" 128k | _filter_img_create +$QEMU_IO -c 'write -q -P 1 0 128k' -f raw "$TEST_IMG.base" +for orig_size in 31k 33k; do +echo "--- Resizing image from $orig_size to 96k ---" +_make_test_img -F raw -b "$TEST_IMG.base" -o cluster_size=64k "$orig_size" +$QEMU_IMG resize -f "$IMGFMT" --preallocation=full "$TEST_IMG" 96k +# The first part of the image should contain data from the backing file +$QEMU_IO -c "read -q -P 1 0 ${orig_size}" "$TEST_IMG" +# The resized part of the image should contain zeroes +$QEMU_IO -c "read -q -P 0 ${orig_size} 63k" "$TEST_IMG" +# If the image does not have an external data file we can also verify its +# actual size. The resized image should have 7 clusters: +# header, L1 table, L2 table, refcount table, refcount block, 2 data clusters +if ! _get_data_file "$TEST_IMG" > /dev/null; then +expected_file_length=$((65536 * 7)) +file_length=$(stat -c '%s' "$TEST_IMG_FILE") +if [ "$file_length" != "$expected_file_length" ]; then +echo "ERROR: file length $file_length (expected $expected_file_length)" +fi +fi +echo +done + # success, all done echo '*** done' rm -f $seq.full diff --git a/tests/qemu-iotests/125.out b/tests/qemu-iotests/125.out index 596905f533..7f76f7af20 100644 --- a/tests/qemu-iotests/125.out +++ b/tests/qemu-iotests/125.out @@ -767,4 +767,13 @@ wrote 2048000/2048000 bytes at offset 0 wrote 81920/81920 bytes at offset 2048000 80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=raw size=131072 +--- Resizing image from 31k to 96k --- +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=31744 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=raw +Image resized. + +--- Resizing image from 33k to 96k --- +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33792 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=raw +Image resized. + *** done --