Re: [PATCH] qcow2-bitmaps: fix qcow2_can_store_new_dirty_bitmap

[Eric Blake <eblake@redhat.com>]

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 <vsementsov@virtuozzo.com>
>> ---
>>
>> 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