[PATCH 0/2] Make preallocate_co() resize the image to the correct size

[PATCH 0/2] Make preallocate_co() resize the image to the correct size

[Alberto Garcia]

preallocate_co() does not resize the image correctly if the original
size was not cluster-aligned.

This series should be applied on top of Max's block branch (I tested
it with commit 8e66c829eda997dad661d49d73668b1fd3e6043d).

   https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Alberto Garcia (2):
  qcow2: Make preallocate_co() resize the image to the correct size
  qcow2: Convert qcow2_alloc_cluster_offset() into
    qcow2_alloc_host_offset()

 block/qcow2.h              |  6 +++---
 block/qcow2-cluster.c      | 14 +++++++++----
 block/qcow2.c              | 35 +++++++++++++--------------------
 tests/qemu-iotests/125     | 40 +++++++++++++++++++++-----------------
 tests/qemu-iotests/125.out | 28 ++++++++++++++++++++++++--
 5 files changed, 74 insertions(+), 49 deletions(-)

-- 
2.20.1

[PATCH 1/2] qcow2: Make preallocate_co() resize the image to the correct size

[Alberto Garcia]

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>
---
 block/qcow2.c              |  1 +
 tests/qemu-iotests/125     | 40 +++++++++++++++++++++-----------------
 tests/qemu-iotests/125.out | 28 ++++++++++++++++++++++++--
 3 files changed, 49 insertions(+), 20 deletions(-)

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..1f35598b2b 100755
--- a/tests/qemu-iotests/125
+++ b/tests/qemu-iotests/125
@@ -168,24 +168,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.
 
-- 
2.20.1

[PATCH 2/2] qcow2: Convert qcow2_alloc_cluster_offset() into qcow2_alloc_host_offset()

[Alberto Garcia]

qcow2_alloc_cluster_offset() takes an (unaligned) guest offset and
returns the (aligned) offset of the corresponding cluster in the qcow2
image.

In practice none of the callers need to know where the cluster starts
so this patch makes the function calculate and return the final host
offset directly. The function is also renamed accordingly.

See 388e581615 for a similar change to qcow2_get_cluster_offset().

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 block/qcow2.h         |  6 +++---
 block/qcow2-cluster.c | 14 ++++++++++----
 block/qcow2.c         | 36 +++++++++++++-----------------------
 3 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index b73a4cf1f8..b71e444fca 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -901,9 +901,9 @@ int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t sector_num,
 int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
                           unsigned int *bytes, uint64_t *host_offset,
                           QCow2SubclusterType *subcluster_type);
-int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
-                               unsigned int *bytes, uint64_t *host_offset,
-                               QCowL2Meta **m);
+int qcow2_alloc_host_offset(BlockDriverState *bs, uint64_t offset,
+                            unsigned int *bytes, uint64_t *host_offset,
+                            QCowL2Meta **m);
 int qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
                                           uint64_t offset,
                                           int compressed_size,
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 1a67b2d928..9acc6ce4ae 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1719,6 +1719,10 @@ out:
  * clusters (or subclusters) if necessary. The result can span a
  * combination of allocated and previously unallocated clusters.
  *
+ * Note that offset may not be cluster aligned. In this case, the returned
+ * *host_offset points to exact byte referenced by offset and therefore
+ * isn't cluster aligned as well.
+ *
  * On return, @host_offset is set to the beginning of the requested
  * area. This area is guaranteed to be contiguous on the qcow2 file
  * but it can be smaller than initially requested. In this case @bytes
@@ -1736,9 +1740,9 @@ out:
  *
  * Return 0 on success and -errno in error cases
  */
-int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
-                               unsigned int *bytes, uint64_t *host_offset,
-                               QCowL2Meta **m)
+int qcow2_alloc_host_offset(BlockDriverState *bs, uint64_t offset,
+                            unsigned int *bytes, uint64_t *host_offset,
+                            QCowL2Meta **m)
 {
     BDRVQcow2State *s = bs->opaque;
     uint64_t start, remaining;
@@ -1759,7 +1763,7 @@ again:
     while (true) {
 
         if (*host_offset == INV_OFFSET && cluster_offset != INV_OFFSET) {
-            *host_offset = start_of_cluster(s, cluster_offset);
+            *host_offset = cluster_offset;
         }
 
         assert(remaining >= cur_bytes);
@@ -1842,6 +1846,8 @@ again:
     *bytes -= remaining;
     assert(*bytes > 0);
     assert(*host_offset != INV_OFFSET);
+    assert(offset_into_cluster(s, *host_offset) ==
+           offset_into_cluster(s, offset));
 
     return 0;
 }
diff --git a/block/qcow2.c b/block/qcow2.c
index 1cb5daf39a..b05512718c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2559,7 +2559,7 @@ static coroutine_fn int qcow2_co_pwritev_part(
     int offset_in_cluster;
     int ret;
     unsigned int cur_bytes; /* number of sectors in current iteration */
-    uint64_t cluster_offset;
+    uint64_t host_offset;
     QCowL2Meta *l2meta = NULL;
     AioTaskPool *aio = NULL;
 
@@ -2580,16 +2580,13 @@ static coroutine_fn int qcow2_co_pwritev_part(
 
         qemu_co_mutex_lock(&s->lock);
 
-        ret = qcow2_alloc_cluster_offset(bs, offset, &cur_bytes,
-                                         &cluster_offset, &l2meta);
+        ret = qcow2_alloc_host_offset(bs, offset, &cur_bytes,
+                                      &host_offset, &l2meta);
         if (ret < 0) {
             goto out_locked;
         }
 
-        assert(offset_into_cluster(s, cluster_offset) == 0);
-
-        ret = qcow2_pre_write_overlap_check(bs, 0,
-                                            cluster_offset + offset_in_cluster,
+        ret = qcow2_pre_write_overlap_check(bs, 0, host_offset,
                                             cur_bytes, true);
         if (ret < 0) {
             goto out_locked;
@@ -2601,7 +2598,7 @@ static coroutine_fn int qcow2_co_pwritev_part(
             aio = aio_task_pool_new(QCOW2_MAX_WORKERS);
         }
         ret = qcow2_add_task(bs, aio, qcow2_co_pwritev_task_entry, 0,
-                             cluster_offset + offset_in_cluster, offset,
+                             host_offset, offset,
                              cur_bytes, qiov, qiov_offset, l2meta);
         l2meta = NULL; /* l2meta is consumed by qcow2_co_pwritev_task() */
         if (ret < 0) {
@@ -3129,13 +3126,12 @@ static int coroutine_fn preallocate_co(BlockDriverState *bs, uint64_t offset,
 
     while (bytes) {
         cur_bytes = MIN(bytes, QEMU_ALIGN_DOWN(INT_MAX, s->cluster_size));
-        ret = qcow2_alloc_cluster_offset(bs, offset, &cur_bytes,
-                                         &host_offset, &meta);
+        ret = qcow2_alloc_host_offset(bs, offset, &cur_bytes,
+                                      &host_offset, &meta);
         if (ret < 0) {
             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;
@@ -4043,10 +4039,9 @@ qcow2_co_copy_range_to(BlockDriverState *bs,
                        BdrvRequestFlags write_flags)
 {
     BDRVQcow2State *s = bs->opaque;
-    int offset_in_cluster;
     int ret;
     unsigned int cur_bytes; /* number of sectors in current iteration */
-    uint64_t cluster_offset;
+    uint64_t host_offset;
     QCowL2Meta *l2meta = NULL;
 
     assert(!bs->encrypted);
@@ -4057,31 +4052,26 @@ qcow2_co_copy_range_to(BlockDriverState *bs,
 
         l2meta = NULL;
 
-        offset_in_cluster = offset_into_cluster(s, dst_offset);
         cur_bytes = MIN(bytes, INT_MAX);
 
         /* TODO:
          * If src->bs == dst->bs, we could simply copy by incrementing
          * the refcnt, without copying user data.
          * Or if src->bs == dst->bs->backing->bs, we could copy by discarding. */
-        ret = qcow2_alloc_cluster_offset(bs, dst_offset, &cur_bytes,
-                                         &cluster_offset, &l2meta);
+        ret = qcow2_alloc_host_offset(bs, dst_offset, &cur_bytes,
+                                      &host_offset, &l2meta);
         if (ret < 0) {
             goto fail;
         }
 
-        assert(offset_into_cluster(s, cluster_offset) == 0);
-
-        ret = qcow2_pre_write_overlap_check(bs, 0,
-                cluster_offset + offset_in_cluster, cur_bytes, true);
+        ret = qcow2_pre_write_overlap_check(bs, 0, host_offset, cur_bytes,
+                                            true);
         if (ret < 0) {
             goto fail;
         }
 
         qemu_co_mutex_unlock(&s->lock);
-        ret = bdrv_co_copy_range_to(src, src_offset,
-                                    s->data_file,
-                                    cluster_offset + offset_in_cluster,
+        ret = bdrv_co_copy_range_to(src, src_offset, s->data_file, host_offset,
                                     cur_bytes, read_flags, write_flags);
         qemu_co_mutex_lock(&s->lock);
         if (ret < 0) {
-- 
2.20.1

Re: [PATCH 2/2] qcow2: Convert qcow2_alloc_cluster_offset() into qcow2_alloc_host_offset()

[Max Reitz]

[-- Attachment #1.1: Type: text/plain, Size: 1080 bytes --]

On 11.09.20 16:09, Alberto Garcia wrote:
> qcow2_alloc_cluster_offset() takes an (unaligned) guest offset and
> returns the (aligned) offset of the corresponding cluster in the qcow2
> image.
> 
> In practice none of the callers need to know where the cluster starts
> so this patch makes the function calculate and return the final host
> offset directly. The function is also renamed accordingly.
> 
> See 388e581615 for a similar change to qcow2_get_cluster_offset().
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  block/qcow2.h         |  6 +++---
>  block/qcow2-cluster.c | 14 ++++++++++----
>  block/qcow2.c         | 36 +++++++++++++-----------------------
>  3 files changed, 26 insertions(+), 30 deletions(-)

First of all:

Reviewed-by: Max Reitz <mreitz@redhat.com>

However, I wonder what you think about “cluster_offset” in
qcow2_alloc_host_offset.  It isn’t a cluster offset anymore.  Can/should
we rename it?

(Perhaps call the parameter host_offset_ptr, and then rename the local
cluster_offset to host_offset?)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/2] qcow2: Make preallocate_co() resize the image to the correct size

[Max Reitz]

[-- Attachment #1.1: Type: text/plain, Size: 935 bytes --]

On 11.09.20 16:09, Alberto Garcia wrote:
> 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>
> ---
>  block/qcow2.c              |  1 +
>  tests/qemu-iotests/125     | 40 +++++++++++++++++++++-----------------
>  tests/qemu-iotests/125.out | 28 ++++++++++++++++++++++++--
>  3 files changed, 49 insertions(+), 20 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] qcow2: Convert qcow2_alloc_cluster_offset() into qcow2_alloc_host_offset()

[Alberto Garcia]

On Mon 14 Sep 2020 02:14:36 PM CEST, Max Reitz wrote:

> However, I wonder what you think about “cluster_offset” in
> qcow2_alloc_host_offset.  It isn’t a cluster offset anymore.
> Can/should we rename it?

That variable was not a cluster offset before this patch either (at
least not during the first iteration of the loop).

The difference is that *host_offset is always the offset of the
beginning of the requested region, and cluster_offset increases with
every iteration of the loop. Maybe current_offset / current_host_offset?
I don't know, but I'm fine with changing it if you have a good name.

Berto

Re: [PATCH 2/2] qcow2: Convert qcow2_alloc_cluster_offset() into qcow2_alloc_host_offset()

[Max Reitz]

[-- Attachment #1.1: Type: text/plain, Size: 916 bytes --]

On 14.09.20 18:42, Alberto Garcia wrote:
> On Mon 14 Sep 2020 02:14:36 PM CEST, Max Reitz wrote:
> 
>> However, I wonder what you think about “cluster_offset” in
>> qcow2_alloc_host_offset.  It isn’t a cluster offset anymore.
>> Can/should we rename it?
> 
> That variable was not a cluster offset before this patch either (at
> least not during the first iteration of the loop).

Hm, yes.  Doesn’t make it less wrong, but you’re right, there is no
argument why this should be addressed in this patch (or series).

> The difference is that *host_offset is always the offset of the
> beginning of the requested region, and cluster_offset increases with
> every iteration of the loop. Maybe current_offset / current_host_offset?
> I don't know, but I'm fine with changing it if you have a good name.

current_host_offset sounds nice.  But let’s just leave it how it is for now.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/2] Make preallocate_co() resize the image to the correct size

[Max Reitz]

[-- Attachment #1.1: Type: text/plain, Size: 945 bytes --]

On 11.09.20 16:09, Alberto Garcia wrote:
> preallocate_co() does not resize the image correctly if the original
> size was not cluster-aligned.
> 
> This series should be applied on top of Max's block branch (I tested
> it with commit 8e66c829eda997dad661d49d73668b1fd3e6043d).
> 
>    https://git.xanclic.moe/XanClic/qemu/commits/branch/block
> 
> Alberto Garcia (2):
>   qcow2: Make preallocate_co() resize the image to the correct size
>   qcow2: Convert qcow2_alloc_cluster_offset() into
>     qcow2_alloc_host_offset()
> 
>  block/qcow2.h              |  6 +++---
>  block/qcow2-cluster.c      | 14 +++++++++----
>  block/qcow2.c              | 35 +++++++++++++--------------------
>  tests/qemu-iotests/125     | 40 +++++++++++++++++++++-----------------
>  tests/qemu-iotests/125.out | 28 ++++++++++++++++++++++++--
>  5 files changed, 74 insertions(+), 49 deletions(-)

Thanks, applied to my block branch.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/2] qcow2: Make preallocate_co() resize the image to the correct size

[Max Reitz]

[-- Attachment #1.1: Type: text/plain, Size: 1571 bytes --]

On 11.09.20 16:09, Alberto Garcia wrote:
> 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>
> ---
>  block/qcow2.c              |  1 +
>  tests/qemu-iotests/125     | 40 +++++++++++++++++++++-----------------
>  tests/qemu-iotests/125.out | 28 ++++++++++++++++++++++++--
>  3 files changed, 49 insertions(+), 20 deletions(-)

The test additions make this test fail with compat=0.10.  Are you OK
with disabling compat=0.10 by squashing this in?

diff --git a/tests/qemu-iotests/125 b/tests/qemu-iotests/125
index 1f35598b2b..894d53f2bd 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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/2] qcow2: Make preallocate_co() resize the image to the correct size

[Alberto Garcia]

On Tue 15 Sep 2020 11:29:22 AM CEST, Max Reitz wrote:
> On 11.09.20 16:09, Alberto Garcia wrote:
>> 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>
>> ---
>>  block/qcow2.c              |  1 +
>>  tests/qemu-iotests/125     | 40 +++++++++++++++++++++-----------------
>>  tests/qemu-iotests/125.out | 28 ++++++++++++++++++++++++--
>>  3 files changed, 49 insertions(+), 20 deletions(-)
>
> The test additions make this test fail with compat=0.10.  Are you OK
> with disabling compat=0.10 by squashing this in?

Yes, thanks

Berto