Re: [PATCH v3 12/13] migration: Create migrate_block_bitmap_mapping() function

[Vladimir Sementsov-Ogievskiy]

On 24.04.23 21:32, Juan Quintela wrote:
> Notice that we changed the test of ->has_block_bitmap_mapping
> for the test that block_bitmap_mapping is not NULL.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>

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

> ---
>   migration/block-dirty-bitmap.c | 14 ++++++++------
>   migration/options.c            |  7 +++++++
>   migration/options.h            |  1 +
>   3 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
> index a6ffae0002..62b2352bbb 100644
> --- a/migration/block-dirty-bitmap.c
> +++ b/migration/block-dirty-bitmap.c
> @@ -605,11 +605,12 @@ static int init_dirty_bitmap_migration(DBMSaveState *s)
>       SaveBitmapState *dbms;
>       GHashTable *handled_by_blk = g_hash_table_new(NULL, NULL);
>       BlockBackend *blk;
> -    const MigrationParameters *mig_params = &migrate_get_current()->parameters;
>       GHashTable *alias_map = NULL;
> +    BitmapMigrationNodeAliasList *block_bitmap_mapping =
> +        migrate_block_bitmap_mapping();
>   
> -    if (mig_params->has_block_bitmap_mapping) {
> -        alias_map = construct_alias_map(mig_params->block_bitmap_mapping, true,
> +    if (block_bitmap_mapping) {
> +        alias_map = construct_alias_map(block_bitmap_mapping, true,
>                                           &error_abort);
>       }
>   
> @@ -1158,7 +1159,8 @@ 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 MigrationParameters *mig_params = &migrate_get_current()->parameters;
> +    BitmapMigrationNodeAliasList *block_bitmap_mapping =
> +        migrate_block_bitmap_mapping();
>       DBMLoadState *s = &((DBMState *)opaque)->load;
>       int ret = 0;
>   
> @@ -1170,8 +1172,8 @@ static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
>           return -EINVAL;
>       }
>   
> -    if (mig_params->has_block_bitmap_mapping) {
> -        alias_map = construct_alias_map(mig_params->block_bitmap_mapping,
> +    if (block_bitmap_mapping) {
> +        alias_map = construct_alias_map(block_bitmap_mapping,
>                                           false, &error_abort);
>       }
>   
> diff --git a/migration/options.c b/migration/options.c
> index 9fbba84b9a..ec234bf3ff 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -452,6 +452,13 @@ void 
> qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
>   
>   /* parameters */
>   
> +BitmapMigrationNodeAliasList *migrate_block_bitmap_mapping(void)

as well, this could return constant pointer. Even construct_alias_map() is 
already prepared for this.

> +{
> +    MigrationState *s = migrate_get_current();
> +
> +    return s->parameters.block_bitmap_mapping;
> +}
> +
>   bool migrate_block_incremental(void)
>   {
>       MigrationState *s = migrate_get_current();
> diff --git a/migration/options.h b/migration/options.h
> index 9123fdb5f4..43e8e9cd8f 100644
> --- a/migration/options.h
> +++ b/migration/options.h
> @@ -62,6 +62,7 @@ bool migrate_cap_set(int cap, bool value, Error **errp);
>   
>   /* parameters */
>   
> +BitmapMigrationNodeAliasList *migrate_block_bitmap_mapping(void);
>   bool migrate_block_incremental(void);
>   uint32_t migrate_checkpoint_delay(void);
>   int migrate_compress_level(void);

--
Best regards,
Vladimir