diff mbox series

[PULL,20/22] qcow2: Make preallocate_co() resize the image to the correct size

Message ID 20200915104627.699552-21-mreitz@redhat.com
State New
Headers show
Series Block patches | expand

Commit Message

Max Reitz Sept. 15, 2020, 10:46 a.m. UTC 
From: Alberto Garcia <berto@igalia.com>

This function preallocates metadata structures and then extends the
image to its new size, but that new size calculation is wrong because
it doesn't take into account that the host_offset variable is always
cluster-aligned.

This problem can be reproduced with preallocation=metadata when the
original size is not cluster-aligned but the new size is. In this case
the final image size will be shorter than expected.

   qemu-img create -f qcow2 img.qcow2 31k
   qemu-img resize --preallocation=metadata img.qcow2 128k

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-Id: <adeb8b059917b141d5f5b3bd2a016262d3052c79.1599833007.git.berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
[mreitz: Mark compat=0.10 unsupported for iotest 125]
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c              |  1 +
 tests/qemu-iotests/125     | 44 ++++++++++++++++++++++----------------
 tests/qemu-iotests/125.out | 28 ++++++++++++++++++++++--
 3 files changed, 53 insertions(+), 20 deletions(-)
diff mbox series

Patch

diff --git a/block/qcow2.c b/block/qcow2.c
index 77c43ce178..1cb5daf39a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3135,6 +3135,7 @@  static int coroutine_fn preallocate_co(BlockDriverState *bs, uint64_t offset,
             error_setg_errno(errp, -ret, "Allocating clusters failed");
             goto out;
         }
+        host_offset += offset_into_cluster(s, offset);
 
         for (m = meta; m != NULL; m = m->next) {
             m->prealloc = true;
diff --git a/tests/qemu-iotests/125 b/tests/qemu-iotests/125
index 7cb1c19730..5720e86dce 100755
--- a/tests/qemu-iotests/125
+++ b/tests/qemu-iotests/125
@@ -43,6 +43,10 @@  get_image_size_on_host()
 
 _supported_fmt qcow2
 _supported_proto file
+# Growing a file with a backing file (without preallocation=full or
+# =falloc) requires zeroing the newly added area, which is impossible
+# to do quickly for v2 images, and hence is unsupported.
+_unsupported_imgopts 'compat=0.10'
 
 if [ -z "$TEST_IMG_FILE" ]; then
     TEST_IMG_FILE=$TEST_IMG
@@ -168,24 +172,28 @@  done
 $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
+    for dst_size in 96k 128k; do
+        for prealloc in metadata full; do
+            echo "--- Resizing image from $orig_size to $dst_size (preallocation=$prealloc) ---"
+            _make_test_img -F raw -b "$TEST_IMG.base" -o cluster_size=64k "$orig_size"
+            $QEMU_IMG resize -f "$IMGFMT" --preallocation="$prealloc" "$TEST_IMG" "$dst_size"
+            # 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
+    done
 done
 
 # success, all done
diff --git a/tests/qemu-iotests/125.out b/tests/qemu-iotests/125.out
index 7f76f7af20..63a6e9e8a9 100644
--- a/tests/qemu-iotests/125.out
+++ b/tests/qemu-iotests/125.out
@@ -768,11 +768,35 @@  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 ---
+--- Resizing image from 31k to 96k (preallocation=metadata) ---
 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 ---
+--- Resizing image from 31k to 96k (preallocation=full) ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=31744 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=raw
+Image resized.
+
+--- Resizing image from 31k to 128k (preallocation=metadata) ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=31744 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=raw
+Image resized.
+
+--- Resizing image from 31k to 128k (preallocation=full) ---
+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 (preallocation=metadata) ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33792 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=raw
+Image resized.
+
+--- Resizing image from 33k to 96k (preallocation=full) ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33792 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=raw
+Image resized.
+
+--- Resizing image from 33k to 128k (preallocation=metadata) ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33792 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=raw
+Image resized.
+
+--- Resizing image from 33k to 128k (preallocation=full) ---
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33792 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=raw
 Image resized.