13.06.2018 05:06, John Snow wrote:
Instead of always setting IN_USE, do this conditionally based on
whether or not the bitmap is read-only. Eliminate the two-pass
processing and move the second loop to the failure case.
This will allow us to show the flags exactly as they appear
on-disk for bitmaps in `qemu-img info`.
Signed-off-by: John Snow <js...@redhat.com>
---
block/qcow2-bitmap.c | 51 +++++++++++++++++++++------------------------------
1 file changed, 21 insertions(+), 30 deletions(-)
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 0670e5eb41..85c1b5afe3 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -919,13 +919,6 @@ fail:
return ret;
}
-/* for g_slist_foreach for GSList of BdrvDirtyBitmap* elements */
-static void release_dirty_bitmap_helper(gpointer bitmap,
- gpointer bs)
-{
- bdrv_release_dirty_bitmap(bs, bitmap);
-}
-
/* for g_slist_foreach for GSList of BdrvDirtyBitmap* elements */
static void set_readonly_helper(gpointer bitmap, gpointer value)
{
@@ -944,8 +937,7 @@ bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error
**errp)
BDRVQcow2State *s = bs->opaque;
Qcow2BitmapList *bm_list;
Qcow2Bitmap *bm;
- GSList *created_dirty_bitmaps = NULL;
- bool header_updated = false;
+ bool needs_update = false;
if (s->nb_bitmaps == 0) {
/* No bitmaps - nothing to do */
@@ -968,35 +960,34 @@ bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error
**errp)
bdrv_disable_dirty_bitmap(bitmap);
}
bdrv_dirty_bitmap_set_persistance(bitmap, true);
- bm->flags |= BME_FLAG_IN_USE;
- created_dirty_bitmaps =
- g_slist_append(created_dirty_bitmaps, bitmap);
- }
- }
-
- if (created_dirty_bitmaps != NULL) {
- if (can_write(bs)) {
- /* in_use flags must be updated */
- int ret = update_ext_header_and_dir_in_place(bs, bm_list);
- if (ret < 0) {
- error_setg_errno(errp, -ret, "Can't update bitmap directory");
- goto fail;
+ if (can_write(bs)) {
+ bm->flags |= BME_FLAG_IN_USE;
+ needs_update = true;
+ } else {
+ bdrv_dirty_bitmap_set_readonly(bitmap, true);
}
+ bm->dirty_bitmap = bitmap; [1]
- header_updated = true;
- } else {
- g_slist_foreach(created_dirty_bitmaps, set_readonly_helper,
- (gpointer)true);
}
}
- g_slist_free(created_dirty_bitmaps);
+ /* in_use flags must be updated */
+ if (needs_update) {
+ int ret = update_ext_header_and_dir_in_place(bs, bm_list);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Can't update bitmap directory");
+ goto fail;
+ }
+ }
+
bitmap_list_free(bm_list);
- return header_updated;
+ return needs_update;
fail:
- g_slist_foreach(created_dirty_bitmaps, release_dirty_bitmap_helper, bs);
- g_slist_free(created_dirty_bitmaps);
+ QSIMPLEQ_FOREACH(bm, bm_list, entry) {
+ if (bm->dirty_bitmap) {
hm stop, at this point, you didn't yet set it. may be in later patches.
I remember that you did it in previous version of patch series, so [1].
+ bdrv_release_dirty_bitmap(bs, bm->dirty_bitmap);
+ }
+ }
bitmap_list_free(bm_list);
return false;
with [1]:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
--
Best regards,
Vladimir
