Re: [PATCH 1/2] iotests: Improve and rename test 291 to qemu-img-bitmap

[Eric Blake]

On Fri, Jul 09, 2021 at 04:49:01PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 09.07.2021 16:16, Eric Blake wrote:
> > On Fri, Jul 09, 2021 at 09:33:50AM +0300, Vladimir Sementsov-Ogievskiy 
> > wrote:
> > > > +++ b/tests/qemu-iotests/tests/qemu-img-bitmaps
> > 
> > > > +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"
> > > 
> > > Worth then removing remaining inconsistent bitmaps and try again?
> > > 
> > > I think you should now remove $TEST_IMG.copy in _cleanup
> > 
> > $TEST_IMG.copy isn't created on failure (or if it is, that in itself
> > is a problem we should be avoiding),
> 
> Seems that's the case:

Oh. Thanks for checking.  Yeah, re-reading the code, we don't attempt
to copy the bitmaps until after the rest of the convert operation is
done.  Which means a fix to pre-validate bitmaps before starting the
convert requires more code, but may be the right thing (failing at the
very end of a long convert attempt, but leaving the file around, is
not ideal compared to failing fast).

Looks like my v2 will be a bit more beefy.
> ./build/qemu-img convert --bitmaps -O qcow2 x y
> qemu-img: Failed to populate bitmap b1: Bitmap 'b1' is inconsistent and 
> cannot be used
> Try block-dirty-bitmap-remove to delete this bitmap from disk[root@kvm 
> master]#
> # ls y
> y
> ./build/qemu-img info y
> image: y
> file format: qcow2
> virtual size: 1 MiB (1048576 bytes)
> disk size: 204 KiB
> cluster_size: 65536
> Format specific information:
>     compat: 1.1
>     compression type: zlib
>     lazy refcounts: false
>     bitmaps:
>         [0]:
>             flags:
>             name: b1
>             granularity: 65536
>     refcount bits: 16
>     corrupt: false
>     extended l2: false
> 
> 
> WOW! It even contains the bitmap not marked in-use. That's a real bug.

So we left things dangling with a bitmap that has completely different
contents than in the original file.  Yeah, that looks like an issue
worth improving.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org