qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>
Subject: [PULL 02/31] qcow2: Fix preallocation on images with unaligned sizes
Date: Mon,  6 Jul 2020 12:04:03 +0200	[thread overview]
Message-ID: <20200706100432.2301919-3-mreitz@redhat.com> (raw)
In-Reply-To: <20200706100432.2301919-1-mreitz@redhat.com>

From: Alberto Garcia <berto@igalia.com>

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 <berto@igalia.com>
Message-Id: <20200617140036.20311-1-berto@igalia.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 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(&allocation.dependent_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
-- 
2.26.2



  parent reply	other threads:[~2020-07-06 10:06 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-06 10:04 [PULL 00/31] Block patches Max Reitz
2020-07-06 10:04 ` [PULL 01/31] block/block-copy: block_copy_dirty_clusters: fix failure check Max Reitz
2020-07-06 10:04 ` Max Reitz [this message]
2020-07-06 10:04 ` [PULL 03/31] iotests: Make _filter_img_create more active Max Reitz
2020-07-10 15:48   ` John Snow
2020-07-06 10:04 ` [PULL 04/31] iotests: filter few more luks specific create options Max Reitz
2020-07-06 10:04 ` [PULL 05/31] iotests/common.rc: Add _require_working_luks Max Reitz
2020-07-06 10:04 ` [PULL 06/31] iotests.py: Add qemu_img_pipe_and_status() Max Reitz
2020-07-06 10:04 ` [PULL 07/31] iotests.py: Add (verify|has)_working_luks() Max Reitz
2020-07-06 10:04 ` [PULL 08/31] iotests: Check whether luks works Max Reitz
2020-07-06 10:04 ` [PULL 09/31] qcrypto/core: add generic infrastructure for crypto options amendment Max Reitz
2020-07-06 10:04 ` [PULL 10/31] qcrypto/luks: implement encryption key management Max Reitz
2020-07-06 10:04 ` [PULL 11/31] block/amend: add 'force' option Max Reitz
2020-07-06 10:04 ` [PULL 12/31] block/amend: separate amend and create options for qemu-img Max Reitz
2020-07-06 10:04 ` [PULL 13/31] block/amend: refactor qcow2 amend options Max Reitz
2020-07-06 10:04 ` [PULL 14/31] block/crypto: rename two functions Max Reitz
2020-07-06 10:04 ` [PULL 15/31] block/crypto: implement the encryption key management Max Reitz
2020-07-06 10:04 ` [PULL 16/31] block/qcow2: extend qemu-img amend interface with crypto options Max Reitz
2020-07-06 10:04 ` [PULL 17/31] iotests: qemu-img tests for luks key management Max Reitz
2020-07-06 10:04 ` [PULL 18/31] block/core: add generic infrastructure for x-blockdev-amend qmp command Max Reitz
2020-07-09 15:09   ` Peter Maydell
2020-07-10  9:54     ` Max Reitz
2020-07-06 10:04 ` [PULL 19/31] block/crypto: implement blockdev-amend Max Reitz
2020-07-06 10:04 ` [PULL 20/31] block/qcow2: " Max Reitz
2020-07-06 10:04 ` [PULL 21/31] iotests: add tests for blockdev-amend Max Reitz
2020-07-06 10:04 ` [PULL 22/31] qemu-img: convert: don't use unallocated_blocks_are_zero Max Reitz
2020-07-06 10:04 ` [PULL 23/31] block: inline bdrv_unallocated_blocks_are_zero() Max Reitz
2020-07-06 10:04 ` [PULL 24/31] block/vdi: return ZERO block-status when appropriate Max Reitz
2020-07-06 10:04 ` [PULL 25/31] block/vpc: " Max Reitz
2020-07-06 10:04 ` [PULL 26/31] block/crypto: drop unallocated_blocks_are_zero Max Reitz
2020-07-06 10:04 ` [PULL 27/31] block/iscsi: " Max Reitz
2020-07-06 10:04 ` [PULL 28/31] block/file-posix: " Max Reitz
2020-07-06 10:04 ` [PULL 29/31] block/vhdx: " Max Reitz
2020-07-06 10:04 ` [PULL 30/31] block: " Max Reitz
2020-07-06 10:04 ` [PULL 31/31] qed: Simplify backing reads Max Reitz
2020-07-06 10:31 ` [PULL 00/31] Block patches no-reply
2020-07-07 20:40 ` Peter Maydell
2020-07-08 15:11   ` Maxim Levitsky
2020-07-08 15:45     ` Kevin Wolf
2020-07-08 15:50     ` Maxim Levitsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200706100432.2301919-3-mreitz@redhat.com \
    --to=mreitz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).