From f88676c149d3c7cde2710c137aa5ef31e7f92eb4 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Fri, 27 Sep 2019 15:23:50 +0300 Subject: [PATCH] block/qcow2-bitmap: get rid of bdrv_has_changed_persistent_bitmaps Firstly, no reason to optimize failure path. Then, function name is ambiguous: it checks for readonly and similar things, but someone may think that it will ignore normal bitmaps which was just unchanged, and this is in bad relation with the fact that we should drop IN_USE flag for unchanged bitmaps in the image. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: John Snow Message-id: 20190927122355.7344-5-vsementsov@virtuozzo.com Signed-off-by: John Snow --- block/dirty-bitmap.c | 12 ------------ block/qcow2-bitmap.c | 23 +++++++++++++---------- include/block/dirty-bitmap.h | 1 - 3 files changed, 13 insertions(+), 23 deletions(-) diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index 6065db8094..4bbb251b2c 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -839,18 +839,6 @@ bool bdrv_dirty_bitmap_inconsistent(const BdrvDirtyBitmap *bitmap) return bitmap->inconsistent; } -bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs) -{ - BdrvDirtyBitmap *bm; - QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) { - if (bm->persistent && !bm->readonly && !bm->skip_store) { - return true; - } - } - - return false; -} - BdrvDirtyBitmap *bdrv_dirty_bitmap_first(BlockDriverState *bs) { return QLIST_FIRST(&bs->dirty_bitmaps); diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index 99812b418b..6dfc083548 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -1464,16 +1464,7 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp) Qcow2Bitmap *bm; QSIMPLEQ_HEAD(, Qcow2BitmapTable) drop_tables; Qcow2BitmapTable *tb, *tb_next; - - if (!bdrv_has_changed_persistent_bitmaps(bs)) { - /* nothing to do */ - return; - } - - if (!can_write(bs)) { - error_setg(errp, "No write access"); - return; - } + bool need_write = false; QSIMPLEQ_INIT(&drop_tables); @@ -1499,6 +1490,8 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp) continue; } + need_write = true; + if (check_constraints_on_bitmap(bs, name, granularity, errp) < 0) { error_prepend(errp, "Bitmap '%s' doesn't satisfy the constraints: ", name); @@ -1537,6 +1530,15 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp) bm->dirty_bitmap = bitmap; } + if (!need_write) { + goto success; + } + + if (!can_write(bs)) { + error_setg(errp, "No write access"); + goto fail; + } + /* allocate clusters and store bitmaps */ QSIMPLEQ_FOREACH(bm, bm_list, entry) { if (bm->dirty_bitmap == NULL) { @@ -1578,6 +1580,7 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp) bdrv_release_dirty_bitmap(bm->dirty_bitmap); } +success: bitmap_list_free(bm_list); return; diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h index 257f0f6704..958e7474fb 100644 --- a/include/block/dirty-bitmap.h +++ b/include/block/dirty-bitmap.h @@ -95,7 +95,6 @@ bool bdrv_has_readonly_bitmaps(BlockDriverState *bs); bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap); bool bdrv_dirty_bitmap_get_persistence(BdrvDirtyBitmap *bitmap); bool bdrv_dirty_bitmap_inconsistent(const BdrvDirtyBitmap *bitmap); -bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs); BdrvDirtyBitmap *bdrv_dirty_bitmap_first(BlockDriverState *bs); BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BdrvDirtyBitmap *bitmap); -- 2.11.4.GIT