diff mbox series

[v4,27/30] qcow2: Assert that expand_zero_clusters_in_l1() does not support subclusters

Message ID 5cc70489bfeb7d2f8f6c8a113dc530cab504db9e.1584468723.git.berto@igalia.com (mailing list archive)
State New, archived
Headers show
Series Add subcluster allocation to qcow2 | expand

Commit Message

Alberto Garcia March 17, 2020, 6:16 p.m. UTC
This function is only used by qcow2_expand_zero_clusters() to
downgrade a qcow2 image to a previous version. It is however not
possible to downgrade an image with extended L2 entries because older
versions of qcow2 do not have this feature.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 block/qcow2-cluster.c      | 8 +++++++-
 tests/qemu-iotests/061     | 6 ++++++
 tests/qemu-iotests/061.out | 5 +++++
 3 files changed, 18 insertions(+), 1 deletion(-)

Comments

Max Reitz April 9, 2020, 10:27 a.m. UTC | #1
On 17.03.20 19:16, Alberto Garcia wrote:
> This function is only used by qcow2_expand_zero_clusters() to
> downgrade a qcow2 image to a previous version. It is however not
> possible to downgrade an image with extended L2 entries because older
> versions of qcow2 do not have this feature.
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  block/qcow2-cluster.c      | 8 +++++++-
>  tests/qemu-iotests/061     | 6 ++++++
>  tests/qemu-iotests/061.out | 5 +++++
>  3 files changed, 18 insertions(+), 1 deletion(-)

[...]

> diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
> index 36b040491f..66bfd23179 100755
> --- a/tests/qemu-iotests/061
> +++ b/tests/qemu-iotests/061
> @@ -266,6 +266,12 @@ $QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
>  _img_info --format-specific
>  _check_test_img
>  
> +echo
> +echo "=== Testing version downgrade with extended L2 entries ==="
> +echo
> +_make_test_img -o "compat=1.1,extended_l2=on" 64M
> +$QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
> +
>  echo
>  echo "=== Try changing the external data file ==="
>  echo
> diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
> index 8b3091a412..5d009867a2 100644
> --- a/tests/qemu-iotests/061.out
> +++ b/tests/qemu-iotests/061.out
> @@ -498,6 +498,11 @@ Format specific information:
>      corrupt: false
>  No errors were found on the image.
>  
> +=== Testing version downgrade with extended L2 entries ===
> +
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
> +qemu-img: Cannot downgrade an image with incompatible features 0x10 set

This test fails in this commit, because extended_l2 is only available
after the next commit.  The code changes and the test itself look good
to me, though.

Max
Alberto Garcia April 10, 2020, 4:42 p.m. UTC | #2
On Thu 09 Apr 2020 12:27:36 PM CEST, Max Reitz wrote:
>> +=== Testing version downgrade with extended L2 entries ===
>> +
>> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
>> +qemu-img: Cannot downgrade an image with incompatible features 0x10 set
>
> This test fails in this commit, because extended_l2 is only available
> after the next commit.  The code changes and the test itself look good
> to me, though.

You're right, thanks! Since this one only adds an assertion I'll just
swap both commits.

Berto
diff mbox series

Patch

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 1f471db98c..125d2852f6 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -2039,6 +2039,9 @@  static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
     int ret;
     int i, j;
 
+    /* qcow2_downgrade() is not allowed in images with subclusters */
+    assert(!has_subclusters(s));
+
     slice_size2 = s->l2_slice_size * l2_entry_size(s);
     n_slices = s->cluster_size / slice_size2;
 
@@ -2107,7 +2110,8 @@  static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
                 if (cluster_type == QCOW2_CLUSTER_ZERO_PLAIN) {
                     if (!bs->backing) {
                         /* not backed; therefore we can simply deallocate the
-                         * cluster */
+                         * cluster. No need to call set_l2_bitmap(), this
+                         * function doesn't support images with subclusters. */
                         set_l2_entry(s, l2_slice, j, 0);
                         l2_dirty = true;
                         continue;
@@ -2178,6 +2182,8 @@  static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
                 } else {
                     set_l2_entry(s, l2_slice, j, offset);
                 }
+                /* No need to call set_l2_bitmap() after set_l2_entry() because
+                 * this function doesn't support images with subclusters. */
                 l2_dirty = true;
             }
 
diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
index 36b040491f..66bfd23179 100755
--- a/tests/qemu-iotests/061
+++ b/tests/qemu-iotests/061
@@ -266,6 +266,12 @@  $QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
 _img_info --format-specific
 _check_test_img
 
+echo
+echo "=== Testing version downgrade with extended L2 entries ==="
+echo
+_make_test_img -o "compat=1.1,extended_l2=on" 64M
+$QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
+
 echo
 echo "=== Try changing the external data file ==="
 echo
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index 8b3091a412..5d009867a2 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -498,6 +498,11 @@  Format specific information:
     corrupt: false
 No errors were found on the image.
 
+=== Testing version downgrade with extended L2 entries ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+qemu-img: Cannot downgrade an image with incompatible features 0x10 set
+
 === Try changing the external data file ===
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864