[Qemu-devel] [PATCH v4 2/3] qcow2: handle reopening bitmaps on bdrv_invalidate_cache

Vladimir Sementsov-Ogievskiy posted 3 patches 7 years, 10 months ago
[Qemu-devel] [PATCH v4 2/3] qcow2: handle reopening bitmaps on bdrv_invalidate_cache
Posted by Vladimir Sementsov-Ogievskiy 7 years, 10 months ago
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) {
-- 
2.11.1


Re: [Qemu-devel] [PATCH v4 2/3] qcow2: handle reopening bitmaps on bdrv_invalidate_cache DROP IT
Posted by Vladimir Sementsov-Ogievskiy 7 years, 10 months ago
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) {


-- 
Best regards,
Vladimir