22.12.2017 16:39, Kevin Wolf wrote:
Am 12.12.2017 um 17:04 hat Vladimir Sementsov-Ogievskiy geschrieben:
Consider migration with shared storage. Persistent bitmaps are stored
on bdrv_inactivate. Then, on destination
process_incoming_migration_bh() calls bdrv_invalidate_cache_all() which
leads to qcow2_load_autoloading_dirty_bitmaps() which fails if bitmaps
are already loaded on destination start. In this case we should call
qcow2_reopen_bitmaps_rw instead.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
Reviewed-by: John Snow <js...@redhat.com>
qcow2_invalidate_cache() calls qcow2_close() first, so why are there
still any bitmaps loaded? Isn't this a bug? Do we leak bitmaps when a
qcow2 image is closed?
Kevin
Interesting point.
Now persistent dirty bitmaps are released at the end of
bdrv_inactivate_recurse,
which is not called here. It was a separate patch
commit 615b5dcf2decbc5f0abb512d13d7e5db2385fa23
Author: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
Date: Wed Jun 28 15:05:30 2017 +0300
block: release persistent bitmaps on inactivate
May be it is more correct to release them immediately after storing, in
qcow2_inactivete. But it will not fix the issue, because qcow2_close will
call qcow2_inactivate only if (!(s->flags & BDRV_O_INACTIVE)), but it is
not the case.
or we can do like this, it fixes the new test:
static void qcow2_close(BlockDriverState *bs)
{
BDRVQcow2State *s = bs->opaque;
qemu_vfree(s->l1_table);
/* else pre-write overlap checks in cache_destroy may crash */
s->l1_table = NULL;
if (!(s->flags & BDRV_O_INACTIVE)) {
qcow2_inactivate(bs);
}
+ bdrv_release_persistent_dirty_bitmaps(bs);
What do you think?
--
Best regards,
Vladimir
