|
From: | Vladimir Sementsov-Ogievskiy |
Subject: | Re: [Qemu-block] [PATCH v2 2/3] qcow2: handle reopening bitmaps on bdrv_invalidate_cache |
Date: | Fri, 22 Dec 2017 17:25:11 +0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 |
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 <address@hidden> Reviewed-by: John Snow <address@hidden>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 <address@hidden> 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
[Prev in Thread] | Current Thread | [Next in Thread] |