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 <vsement...@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

Reply via email to