Re: [PATCH] qcow2-bitmaps: fix qcow2_can_store_new_dirty_bitmap

[Eric Blake]

On 6/7/19 1:53 PM, Vladimir Sementsov-Ogievskiy wrote:
> 07.06.2019 21:48, Vladimir Sementsov-Ogievskiy wrote:
> > qcow2_can_store_new_dirty_bitmap works wrong, as it considers only
> > bitmaps already stored in the qcow2 image and ignores persistent
> > BdrvDirtyBitmap objects.
> >
> > So, let's instead count persistent BdrvDirtyBitmaps. We load all qcow2
> > bitmaps on open, so there should not be any bitmap in the image for
> > which we don't have BdrvDirtyBitmaps version. If it is - it's a kind of
> > corruption, and no reason to check for corruptions here (open() and
> > close() are better places for it).
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> > ---
> >
> > Hi!
> >
> > Patch is better than discussing I thing, so here is my counter-suggestion for
> > "[PATCH 0/5] block/dirty-bitmap: check number and size constraints against queued 
> > bitmaps"
> > by John.
> >
> > It's of course needs some additional refactoring, as first assert shows bad 
> > design,
> > I just wrote it in such manner to make minimum changes to fix the bug.

> > +    assert(!bdrv_find_dirty_bitmap(bs, name));
>
> exactly bad, this should be checked in qmp_block_dirty_bitmap_add(), before 
> checks around
> persistence. and aio_context_acquire may be dropped..
>
> But anyway, idea is clear I think, consider it as RFC patch

Are you planning to post a v2, as this affects at least 
https://bugzilla.redhat.com/show_bug.cgi?id=1712636

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