Skip to content

Commit

Permalink
block/qcow2-bitmap: get rid of bdrv_has_changed_persistent_bitmaps
Browse files Browse the repository at this point in the history
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 <vsementsov@virtuozzo.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 20190927122355.7344-5-vsementsov@virtuozzo.com
Signed-off-by: John Snow <jsnow@redhat.com>
  • Loading branch information
Vladimir Sementsov-Ogievskiy authored and jnsnow committed Oct 17, 2019
1 parent 5752f89 commit f88676c
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 23 deletions.
12 changes: 0 additions & 12 deletions block/dirty-bitmap.c
Expand Up @@ -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);
Expand Down
23 changes: 13 additions & 10 deletions block/qcow2-bitmap.c
Expand Up @@ -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);

Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;

Expand Down
1 change: 0 additions & 1 deletion include/block/dirty-bitmap.h
Expand Up @@ -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);
Expand Down

0 comments on commit f88676c

Please sign in to comment.