From b1336cc2ec87a6593e1167af6e5b2cbd3d31acac Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Tue, 20 Mar 2018 20:05:18 +0300 Subject: qcow2-bitmap: add qcow2_reopen_bitmaps_rw_hint() Add version of qcow2_reopen_bitmaps_rw, which do the same work but also return a hint about was header updated or not. This will be used in the following fix for bitmaps reloading after migration. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: John Snow Reviewed-by: Max Reitz Message-id: 20180320170521.32152-2-vsementsov@virtuozzo.com Signed-off-by: Max Reitz --- block/qcow2-bitmap.c | 15 ++++++++++++++- block/qcow2.h | 2 ++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index 3010adb909..6e93ec43e1 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -1004,7 +1004,8 @@ fail: return false; } -int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp) +int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_updated, + Error **errp) { BDRVQcow2State *s = bs->opaque; Qcow2BitmapList *bm_list; @@ -1012,6 +1013,10 @@ int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp) GSList *ro_dirty_bitmaps = NULL; int ret = 0; + if (header_updated != NULL) { + *header_updated = false; + } + if (s->nb_bitmaps == 0) { /* No bitmaps - nothing to do */ return 0; @@ -1055,6 +1060,9 @@ int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp) error_setg_errno(errp, -ret, "Can't update bitmap directory"); goto out; } + if (header_updated != NULL) { + *header_updated = true; + } g_slist_foreach(ro_dirty_bitmaps, set_readonly_helper, false); } @@ -1065,6 +1073,11 @@ out: return ret; } +int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp) +{ + return qcow2_reopen_bitmaps_rw_hint(bs, NULL, errp); +} + /* store_bitmap_data() * Store bitmap to image, filling bitmap table accordingly. */ diff --git a/block/qcow2.h b/block/qcow2.h index ccb92a9696..d301f77cea 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -671,6 +671,8 @@ int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res, void **refcount_table, int64_t *refcount_table_size); bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error **errp); +int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_updated, + Error **errp); int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp); void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp); int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp); -- cgit v1.2.3 From 2d949dfcefcf12831620549cea8f4e537f9de5f2 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Tue, 20 Mar 2018 20:05:19 +0300 Subject: qcow2: fix bitmaps loading when bitmaps already exist On reopen with existing bitmaps, instead of loading bitmaps, lets reopen them if needed. This also fixes bitmaps migration through shared storage. Consider the case. 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. Signed-off-by: Vladimir Sementsov-Ogievskiy Message-id: 20180320170521.32152-3-vsementsov@virtuozzo.com Signed-off-by: Max Reitz --- block/qcow2.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/block/qcow2.c b/block/qcow2.c index cf4f3becae..486f3e83b7 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1480,7 +1480,22 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options, s->autoclear_features &= QCOW2_AUTOCLEAR_MASK; } - if (qcow2_load_dirty_bitmaps(bs, &local_err)) { + if (bdrv_dirty_bitmap_next(bs, NULL)) { + /* It's some kind of reopen with already existing dirty bitmaps. There + * are no known cases where we need loading bitmaps in such situation, + * so it's safer don't load them. + * + * Moreover, if we have some readonly bitmaps and we are reopening for + * rw we should reopen bitmaps correspondingly. + */ + if (bdrv_has_readonly_bitmaps(bs) && + !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE)) + { + bool header_updated = false; + qcow2_reopen_bitmaps_rw_hint(bs, &header_updated, &local_err); + update_header = update_header && !header_updated; + } + } else if (qcow2_load_dirty_bitmaps(bs, &local_err)) { update_header = false; } if (local_err != NULL) { -- cgit v1.2.3 From f7640f0dbc7becbd0927dd3fcf65cdf5a55adf89 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Tue, 20 Mar 2018 20:05:21 +0300 Subject: iotests: enable shared migration cases in 169 Shared migration for dirty bitmaps is fixed by previous patches, so we can enable the test. Signed-off-by: Vladimir Sementsov-Ogievskiy Message-id: 20180320170521.32152-5-vsementsov@virtuozzo.com Signed-off-by: Max Reitz --- tests/qemu-iotests/169 | 8 +++----- tests/qemu-iotests/169.out | 4 ++-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/qemu-iotests/169 b/tests/qemu-iotests/169 index 3a8db91f6f..153b10b6e7 100755 --- a/tests/qemu-iotests/169 +++ b/tests/qemu-iotests/169 @@ -140,16 +140,14 @@ def inject_test_case(klass, name, method, *args, **kwargs): mc = operator.methodcaller(method, *args, **kwargs) setattr(klass, 'test_' + name, new.instancemethod(mc, None, klass)) -for cmb in list(itertools.product((True, False), repeat=3)): +for cmb in list(itertools.product((True, False), repeat=4)): name = ('_' if cmb[0] else '_not_') + 'persistent_' name += ('_' if cmb[1] else '_not_') + 'migbitmap_' name += '_online' if cmb[2] else '_offline' - - # TODO fix shared-storage bitmap migration and enable cases for it - args = list(cmb) + [False] + name += '_shared' if cmb[3] else '_nonshared' inject_test_case(TestDirtyBitmapMigration, name, 'do_test_migration', - *args) + *list(cmb)) if __name__ == '__main__': diff --git a/tests/qemu-iotests/169.out b/tests/qemu-iotests/169.out index 594c16f49f..b6f257674e 100644 --- a/tests/qemu-iotests/169.out +++ b/tests/qemu-iotests/169.out @@ -1,5 +1,5 @@ -........ +................ ---------------------------------------------------------------------- -Ran 8 tests +Ran 16 tests OK -- cgit v1.2.3 From a77672ea3d95094a0cb4f974de84fb7353c67cc0 Mon Sep 17 00:00:00 2001 From: yuchenlin Date: Thu, 22 Mar 2018 21:33:37 +0800 Subject: vmdk: return ERROR when cluster sector is larger than vmdk limitation VMDK has a hard limitation of extent size, which is due to the size of grain table entry is 32 bits. It means it can only point to a grain located at offset = 2^32. To avoid writing the user data beyond limitation and record a useless offset in grain table. We should return ERROR here. Signed-off-by: yuchenlin Message-id: 20180322133337.28024-1-yuchenlin@synology.com Reviewed-by: Fam Zheng Signed-off-by: Max Reitz --- block/vmdk.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/block/vmdk.c b/block/vmdk.c index f94c49a9c0..84f8bbe480 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -47,6 +47,8 @@ #define VMDK4_FLAG_MARKER (1 << 17) #define VMDK4_GD_AT_END 0xffffffffffffffffULL +#define VMDK_EXTENT_MAX_SECTORS (1ULL << 32) + #define VMDK_GTE_ZEROED 0x1 /* VMDK internal error codes */ @@ -1250,6 +1252,10 @@ static int get_cluster_offset(BlockDriverState *bs, return zeroed ? VMDK_ZEROED : VMDK_UNALLOC; } + if (extent->next_cluster_sector >= VMDK_EXTENT_MAX_SECTORS) { + return VMDK_ERROR; + } + cluster_sector = extent->next_cluster_sector; extent->next_cluster_sector += extent->cluster_sectors; -- cgit v1.2.3