| Message ID | 20210709153951.2801666-2-eblake@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Let 'qemu-img convert --bitmaps' skip inconsistent bitmaps | expand |
09.07.2021 18:39, Eric Blake wrote: > Enhance the test to demonstrate existing less-than-stellar behavior of > qemu-img with a qcow2 image containing an inconsistent bitmap: we > don't diagnose the problem until after copying the entire image (a > potentially long time), and when we do diagnose the failure, we still > end up leaving an empty bitmap in the destination. This mess will be > cleaned up in the next patch. > > While at it, rename the test now that we support useful iotest names, > and fix a missing newline in the error message thus exposed. > > Signed-off-by: Eric Blake<eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
On 7/9/21 6:39 PM, Eric Blake wrote: > Enhance the test to demonstrate existing less-than-stellar behavior of > qemu-img with a qcow2 image containing an inconsistent bitmap: we > don't diagnose the problem until after copying the entire image (a > potentially long time), and when we do diagnose the failure, we still > end up leaving an empty bitmap in the destination. This mess will be > cleaned up in the next patch. > > While at it, rename the test now that we support useful iotest names, > and fix a missing newline in the error message thus exposed. Much nicer with a meaningful name! > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > block/dirty-bitmap.c | 2 +- > .../{291 => tests/qemu-img-bitmaps} | 19 +++++++- > .../{291.out => tests/qemu-img-bitmaps.out} | 48 ++++++++++++++++++- > 3 files changed, 66 insertions(+), 3 deletions(-) > rename tests/qemu-iotests/{291 => tests/qemu-img-bitmaps} (88%) > rename tests/qemu-iotests/{291.out => tests/qemu-img-bitmaps.out} (75%) > > diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c > index 68d295d6e3ed..0ef46163e3ea 100644 > --- a/block/dirty-bitmap.c > +++ b/block/dirty-bitmap.c > @@ -193,7 +193,7 @@ int bdrv_dirty_bitmap_check(const BdrvDirtyBitmap *bitmap, uint32_t flags, > error_setg(errp, "Bitmap '%s' is inconsistent and cannot be used", > bitmap->name); > error_append_hint(errp, "Try block-dirty-bitmap-remove to delete" > - " this bitmap from disk"); > + " this bitmap from disk\n"); > return -1; > } > > diff --git a/tests/qemu-iotests/291 b/tests/qemu-iotests/tests/qemu-img-bitmaps > similarity index 88% > rename from tests/qemu-iotests/291 > rename to tests/qemu-iotests/tests/qemu-img-bitmaps > index 20efb080a6c0..2f51651d0ce5 100755 > --- a/tests/qemu-iotests/291 > +++ b/tests/qemu-iotests/tests/qemu-img-bitmaps > @@ -3,7 +3,7 @@ > # > # Test qemu-img bitmap handling > # > -# Copyright (C) 2018-2020 Red Hat, Inc. > +# Copyright (C) 2018-2021 Red Hat, Inc. > # > # This program is free software; you can redistribute it and/or modify > # it under the terms of the GNU General Public License as published by > @@ -27,11 +27,13 @@ status=1 # failure is the default! > _cleanup() > { > _cleanup_test_img > + _rm_test_img "$TEST_IMG.copy" > nbd_server_stop > } > trap "_cleanup; exit \$status" 0 1 2 3 15 > > # get standard environment, filters and checks > +cd .. > . ./common.rc > . ./common.filter > . ./common.nbd > @@ -129,6 +131,21 @@ $QEMU_IMG map --output=json --image-opts \ > > nbd_server_stop > > +echo > +echo "=== Check handling of inconsistent bitmap ===" > +echo > + > +$QEMU_IO -c abort "$TEST_IMG" 2>/dev/null > +$QEMU_IMG bitmap --add "$TEST_IMG" b4 > +$QEMU_IMG bitmap --remove "$TEST_IMG" b1 > +_img_info --format-specific | _filter_irrelevant_img_info > +$QEMU_IMG convert --bitmaps -O qcow2 "$TEST_IMG" "$TEST_IMG.copy" && > + echo "unexpected success" > +# Bug - even though we failed at conversion, we left a file around with > +# a bitmap marked as not corrupt > +TEST_IMG=$TEST_IMG.copy _img_info --format-specific \ > + | _filter_irrelevant_img_info > + > # success, all done > echo '*** done' > rm -f $seq.full > diff --git a/tests/qemu-iotests/291.out b/tests/qemu-iotests/tests/qemu-img-bitmaps.out > similarity index 75% > rename from tests/qemu-iotests/291.out > rename to tests/qemu-iotests/tests/qemu-img-bitmaps.out > index 23411c0ff4d9..b762362075d1 100644 > --- a/tests/qemu-iotests/291.out > +++ b/tests/qemu-iotests/tests/qemu-img-bitmaps.out > @@ -1,4 +1,4 @@ > -QA output created by 291 > +QA output created by qemu-img-bitmaps > > === Initial image setup === > > @@ -115,4 +115,50 @@ Format specific information: > [{ "start": 0, "length": 2097152, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, > { "start": 2097152, "length": 1048576, "depth": 0, "zero": false, "data": false}, > { "start": 3145728, "length": 7340032, "depth": 0, "zero": false, "data": true, "offset": OFFSET}] > + > +=== Check handling of inconsistent bitmap === > + > +image: TEST_DIR/t.IMGFMT > +file format: IMGFMT > +virtual size: 10 MiB (10485760 bytes) > +cluster_size: 65536 > +backing file: TEST_DIR/t.IMGFMT.base > +backing file format: IMGFMT > +Format specific information: > + bitmaps: > + [0]: > + flags: > + [0]: in-use > + [1]: auto > + name: b2 > + granularity: 65536 > + [1]: > + flags: > + [0]: in-use > + name: b0 > + granularity: 65536 > + [2]: > + flags: > + [0]: auto > + name: b4 > + granularity: 65536 > + corrupt: false > +qemu-img: Failed to populate bitmap b0: Bitmap 'b0' is inconsistent and cannot be used > +Try block-dirty-bitmap-remove to delete this bitmap from disk In this context a more useful error message would be: Try "qemu-img bitmap --remove" ... but this is not a new issue. > +image: TEST_DIR/t.IMGFMT.copy > +file format: IMGFMT > +virtual size: 10 MiB (10485760 bytes) > +cluster_size: 65536 > +Format specific information: > + bitmaps: > + [0]: > + flags: > + name: b0 > + granularity: 65536 > + [1]: > + flags: > + [0]: auto > + name: b4 > + granularity: 65536 > + corrupt: false > *** done > Reviewed-by: Nir Soffer <nsoffer@redhat.com>
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index 68d295d6e3ed..0ef46163e3ea 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -193,7 +193,7 @@ int bdrv_dirty_bitmap_check(const BdrvDirtyBitmap *bitmap, uint32_t flags, error_setg(errp, "Bitmap '%s' is inconsistent and cannot be used", bitmap->name); error_append_hint(errp, "Try block-dirty-bitmap-remove to delete" - " this bitmap from disk"); + " this bitmap from disk\n"); return -1; } diff --git a/tests/qemu-iotests/291 b/tests/qemu-iotests/tests/qemu-img-bitmaps similarity index 88% rename from tests/qemu-iotests/291 rename to tests/qemu-iotests/tests/qemu-img-bitmaps index 20efb080a6c0..2f51651d0ce5 100755 --- a/tests/qemu-iotests/291 +++ b/tests/qemu-iotests/tests/qemu-img-bitmaps @@ -3,7 +3,7 @@ # # Test qemu-img bitmap handling # -# Copyright (C) 2018-2020 Red Hat, Inc. +# Copyright (C) 2018-2021 Red Hat, Inc. # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -27,11 +27,13 @@ status=1 # failure is the default! _cleanup() { _cleanup_test_img + _rm_test_img "$TEST_IMG.copy" nbd_server_stop } trap "_cleanup; exit \$status" 0 1 2 3 15 # get standard environment, filters and checks +cd .. . ./common.rc . ./common.filter . ./common.nbd @@ -129,6 +131,21 @@ $QEMU_IMG map --output=json --image-opts \ nbd_server_stop +echo +echo "=== Check handling of inconsistent bitmap ===" +echo + +$QEMU_IO -c abort "$TEST_IMG" 2>/dev/null +$QEMU_IMG bitmap --add "$TEST_IMG" b4 +$QEMU_IMG bitmap --remove "$TEST_IMG" b1 +_img_info --format-specific | _filter_irrelevant_img_info +$QEMU_IMG convert --bitmaps -O qcow2 "$TEST_IMG" "$TEST_IMG.copy" && + echo "unexpected success" +# Bug - even though we failed at conversion, we left a file around with +# a bitmap marked as not corrupt +TEST_IMG=$TEST_IMG.copy _img_info --format-specific \ + | _filter_irrelevant_img_info + # success, all done echo '*** done' rm -f $seq.full diff --git a/tests/qemu-iotests/291.out b/tests/qemu-iotests/tests/qemu-img-bitmaps.out similarity index 75% rename from tests/qemu-iotests/291.out rename to tests/qemu-iotests/tests/qemu-img-bitmaps.out index 23411c0ff4d9..b762362075d1 100644 --- a/tests/qemu-iotests/291.out +++ b/tests/qemu-iotests/tests/qemu-img-bitmaps.out @@ -1,4 +1,4 @@ -QA output created by 291 +QA output created by qemu-img-bitmaps === Initial image setup === @@ -115,4 +115,50 @@ Format specific information: [{ "start": 0, "length": 2097152, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, { "start": 2097152, "length": 1048576, "depth": 0, "zero": false, "data": false}, { "start": 3145728, "length": 7340032, "depth": 0, "zero": false, "data": true, "offset": OFFSET}] + +=== Check handling of inconsistent bitmap === + +image: TEST_DIR/t.IMGFMT +file format: IMGFMT +virtual size: 10 MiB (10485760 bytes) +cluster_size: 65536 +backing file: TEST_DIR/t.IMGFMT.base +backing file format: IMGFMT +Format specific information: + bitmaps: + [0]: + flags: + [0]: in-use + [1]: auto + name: b2 + granularity: 65536 + [1]: + flags: + [0]: in-use + name: b0 + granularity: 65536 + [2]: + flags: + [0]: auto + name: b4 + granularity: 65536 + corrupt: false +qemu-img: Failed to populate bitmap b0: Bitmap 'b0' is inconsistent and cannot be used +Try block-dirty-bitmap-remove to delete this bitmap from disk +image: TEST_DIR/t.IMGFMT.copy +file format: IMGFMT +virtual size: 10 MiB (10485760 bytes) +cluster_size: 65536 +Format specific information: + bitmaps: + [0]: + flags: + name: b0 + granularity: 65536 + [1]: + flags: + [0]: auto + name: b4 + granularity: 65536 + corrupt: false *** done
Enhance the test to demonstrate existing less-than-stellar behavior of qemu-img with a qcow2 image containing an inconsistent bitmap: we don't diagnose the problem until after copying the entire image (a potentially long time), and when we do diagnose the failure, we still end up leaving an empty bitmap in the destination. This mess will be cleaned up in the next patch. While at it, rename the test now that we support useful iotest names, and fix a missing newline in the error message thus exposed. Signed-off-by: Eric Blake <eblake@redhat.com> --- block/dirty-bitmap.c | 2 +- .../{291 => tests/qemu-img-bitmaps} | 19 +++++++- .../{291.out => tests/qemu-img-bitmaps.out} | 48 ++++++++++++++++++- 3 files changed, 66 insertions(+), 3 deletions(-) rename tests/qemu-iotests/{291 => tests/qemu-img-bitmaps} (88%) rename tests/qemu-iotests/{291.out => tests/qemu-img-bitmaps.out} (75%)