[PATCH] migration: Fix block_bitmap_mapping migration

Juan Quintela posted 1 patch 12 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230503181036.14890-1-quintela@redhat.com
Maintainers: Eric Blake <eblake@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>, John Snow <jsnow@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Fam Zheng <fam@euphon.net>, Juan Quintela <quintela@redhat.com>, Peter Xu <peterx@redhat.com>, Leonardo Bras <leobras@redhat.com>
migration/block-dirty-bitmap.c | 14 +++++---------
migration/options.c            |  7 +++++++
migration/options.h            |  2 ++
3 files changed, 14 insertions(+), 9 deletions(-)
[PATCH] migration: Fix block_bitmap_mapping migration
Posted by Juan Quintela 12 months ago
It is valid that params->has_block_bitmap_mapping is true and
params->block_bitmap_mapping is NULL.  So we can't use the trick of
having a single function.

Move to two functions one for each value and the tests are fixed.

Fixes: b804b35b1c8a0edfd127ac20819c234be55ac7fc
       migration: Create migrate_block_bitmap_mapping() function

Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/block-dirty-bitmap.c | 14 +++++---------
 migration/options.c            |  7 +++++++
 migration/options.h            |  2 ++
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 6624f39bc6..20f36e6bd8 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -606,11 +606,9 @@ static int init_dirty_bitmap_migration(DBMSaveState *s)
     GHashTable *handled_by_blk = g_hash_table_new(NULL, NULL);
     BlockBackend *blk;
     GHashTable *alias_map = NULL;
-    const BitmapMigrationNodeAliasList *block_bitmap_mapping =
-        migrate_block_bitmap_mapping();
 
-    if (block_bitmap_mapping) {
-        alias_map = construct_alias_map(block_bitmap_mapping, true,
+    if (migrate_has_block_bitmap_mapping()) {
+        alias_map = construct_alias_map(migrate_block_bitmap_mapping(), true,
                                         &error_abort);
     }
 
@@ -1159,8 +1157,6 @@ static int dirty_bitmap_load_header(QEMUFile *f, DBMLoadState *s,
 static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
 {
     GHashTable *alias_map = NULL;
-    const BitmapMigrationNodeAliasList *block_bitmap_mapping =
-        migrate_block_bitmap_mapping();
     DBMLoadState *s = &((DBMState *)opaque)->load;
     int ret = 0;
 
@@ -1172,9 +1168,9 @@ static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
         return -EINVAL;
     }
 
-    if (block_bitmap_mapping) {
-        alias_map = construct_alias_map(block_bitmap_mapping,
-                                        false, &error_abort);
+    if (migrate_has_block_bitmap_mapping()) {
+        alias_map = construct_alias_map(migrate_block_bitmap_mapping(), false,
+                                        &error_abort);
     }
 
     do {
diff --git a/migration/options.c b/migration/options.c
index 53b7fc5d5d..7395787960 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -626,6 +626,13 @@ const BitmapMigrationNodeAliasList *migrate_block_bitmap_mapping(void)
     return s->parameters.block_bitmap_mapping;
 }
 
+bool migrate_has_block_bitmap_mapping(void)
+{
+    MigrationState *s = migrate_get_current();
+
+    return s->parameters.has_block_bitmap_mapping;
+}
+
 bool migrate_block_incremental(void)
 {
     MigrationState *s = migrate_get_current();
diff --git a/migration/options.h b/migration/options.h
index 3c322867cd..09841d6a63 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -71,6 +71,8 @@ bool migrate_cap_set(int cap, bool value, Error **errp);
 /* parameters */
 
 const BitmapMigrationNodeAliasList *migrate_block_bitmap_mapping(void);
+bool migrate_has_block_bitmap_mapping(void);
+
 bool migrate_block_incremental(void);
 uint32_t migrate_checkpoint_delay(void);
 int migrate_compress_level(void);
-- 
2.40.0
Re: [PATCH] migration: Fix block_bitmap_mapping migration
Posted by Kevin Wolf 12 months ago
Am 03.05.2023 um 20:10 hat Juan Quintela geschrieben:
> It is valid that params->has_block_bitmap_mapping is true and
> params->block_bitmap_mapping is NULL.  So we can't use the trick of
> having a single function.
> 
> Move to two functions one for each value and the tests are fixed.
> 
> Fixes: b804b35b1c8a0edfd127ac20819c234be55ac7fc
>        migration: Create migrate_block_bitmap_mapping() function
> 
> Reported-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Thanks, applied to the block branch.

Kevin
Re: [PATCH] migration: Fix block_bitmap_mapping migration
Posted by Vladimir Sementsov-Ogievskiy 12 months ago
On 03.05.23 21:10, Juan Quintela wrote:
> It is valid that params->has_block_bitmap_mapping is true and
> params->block_bitmap_mapping is NULL.  So we can't use the trick of
> having a single function.
> 
> Move to two functions one for each value and the tests are fixed.
> 
> Fixes: b804b35b1c8a0edfd127ac20819c234be55ac7fc
>         migration: Create migrate_block_bitmap_mapping() function
> 
> Reported-by: Kevin Wolf<kwolf@redhat.com>
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

-- 
Best regards,
Vladimir