| Message ID | 19afb93b267b9698edf5e0253458c5db69ef91a2.1591801197.git.berto@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add subcluster allocation to qcow2 | expand |
On 6/10/20 10:03 AM, 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. Well, it _is_ possible, but it would involve rewriting the entire L1/L2 tables (including all internal snapshots), as well as causing I/O to COW every cluster where not all subclusters are allocated; and doing that conversion while remaining crash-consistent requires some thought and a temporary extra load on disk space (we can't discard the old table until the new one is completely written). It would be more accurate to merely state that we are not prepared to implement it at this time. > > Signed-off-by: Alberto Garcia <berto@igalia.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > --- > block/qcow2-cluster.c | 8 +++++++- > tests/qemu-iotests/061 | 6 ++++++ > tests/qemu-iotests/061.out | 5 +++++ > 3 files changed, 18 insertions(+), 1 deletion(-) > Whether or not we update the commit message, R-b stands for the code.
On Wed 10 Jun 2020 09:43:53 PM CEST, Eric Blake wrote: > On 6/10/20 10:03 AM, 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. > > Well, it _is_ possible, but it would involve rewriting the entire > L1/L2 tables (including all internal snapshots) Right :-) Let's try this way: This function is only used by qcow2_expand_zero_clusters() to downgrade a qcow2 image to a previous version. This would require transforming all extended L2 entries into normal L2 entries but this is not a simple task and there are no plans to implement this at the moment. Berto
On 6/11/20 8:24 AM, Alberto Garcia wrote: > On Wed 10 Jun 2020 09:43:53 PM CEST, Eric Blake wrote: >> On 6/10/20 10:03 AM, 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. >> >> Well, it _is_ possible, but it would involve rewriting the entire >> L1/L2 tables (including all internal snapshots) > > Right :-) Let's try this way: > > This function is only used by qcow2_expand_zero_clusters() to > downgrade a qcow2 image to a previous version. This would require > transforming all extended L2 entries into normal L2 entries but > this is not a simple task and there are no plans to implement this > at the moment. Works for me.
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index c8217081f2..e8bb1f32f3 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -2157,6 +2157,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; @@ -2225,7 +2228,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; @@ -2296,6 +2300,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 10eb243164..23add2dfe3 100755 --- a/tests/qemu-iotests/061 +++ b/tests/qemu-iotests/061 @@ -303,6 +303,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 39812d8cf8..c1acdbd751 100644 --- a/tests/qemu-iotests/061.out +++ b/tests/qemu-iotests/061.out @@ -528,6 +528,11 @@ Format specific information: extended l2: 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