diff mbox series

[v4,2/3] qcow2: handle reopening bitmaps on bdrv_invalidate_cache

Message ID 20180320170521.32152-4-vsementsov@virtuozzo.com
State New
Headers show
Series fix bitmaps migration through shared storage | expand

Commit Message

Vladimir Sementsov-Ogievskiy March 20, 2018, 5:05 p.m. UTC
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 <vsementsov@virtuozzo.com>
---
 block/qcow2.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Comments

Vladimir Sementsov-Ogievskiy March 20, 2018, 5:07 p.m. UTC | #1
Oh, sorry, just drop it.
(the only difference is commit message subject)

20.03.2018 20:05, Vladimir Sementsov-Ogievskiy wrote:
> 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 <vsementsov@virtuozzo.com>
> ---
>   block/qcow2.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 7472af6931..d2d129a2e6 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) {
diff mbox series

Patch

diff --git a/block/qcow2.c b/block/qcow2.c
index 7472af6931..d2d129a2e6 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) {