qcow2: try load bitmaps only once

Checking reopen by existence of some bitmaps is wrong, as it may be
some other bitmaps, or on the other hand, user may remove bitmaps. This
criteria is bad. To simplify things and make behavior more predictable
let's just add a flag to remember, that we've already tried to load
bitmaps on open and do not want do it again.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20180411122606.367301-2-vsementsov@virtuozzo.com
[mreitz: Changed comment wording according to Eric Blake's suggestion]
Signed-off-by: Max Reitz <mreitz@redhat.com>

diff --git a/block/qcow2.c b/block/qcow2.c
index 486f3e83b779c99099d307365aa41152ea99daa8..ef68772acad052b1321cefa98bcdb6422c1d09c2 100644 (file)
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1142,6 +1142,7 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
     uint64_t ext_end;
     uint64_t l1_vm_state_index;
     bool update_header = false;
+    bool header_updated = false;
 
     ret = bdrv_pread(bs->file, 0, &header, sizeof(header));
     if (ret < 0) {
@@ -1480,10 +1481,9 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
         s->autoclear_features &= QCOW2_AUTOCLEAR_MASK;
     }
 
-    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.
+    if (s->dirty_bitmaps_loaded) {
+        /* It's some kind of reopen. There are no known cases where we need to
+         * reload bitmaps in such a situation, so it's safer to skip them.
          *
          * Moreover, if we have some readonly bitmaps and we are reopening for
          * rw we should reopen bitmaps correspondingly.
@@ -1491,13 +1491,13 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
         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;
+    } else {
+        header_updated = qcow2_load_dirty_bitmaps(bs, &local_err);
+        s->dirty_bitmaps_loaded = true;
     }
+    update_header = update_header && !header_updated;
     if (local_err != NULL) {
         error_propagate(errp, local_err);
         ret = -EINVAL;

diff --git a/block/qcow2.h b/block/qcow2.h
index d301f77cea57d305b4a18bf861b38a7fb68c2f3e..adf5c3950fd3ee744ec2f1850d9df6bbb4e3f58e 100644 (file)
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -298,6 +298,7 @@ typedef struct BDRVQcow2State {
     uint32_t nb_bitmaps;
     uint64_t bitmap_directory_size;
     uint64_t bitmap_directory_offset;
+    bool dirty_bitmaps_loaded;
 
     int flags;
     int qcow_version;