Re: [PATCH] migration: Use QAPI_CLONE() instead of duplicating it inline

[Fabiano Rosas <farosas@suse.de>]

Bin Guo <guobin@linux.alibaba.com> writes:

> It's better to use QAPI_CLONE() in qmp_query_migrate_parameters so that
> the code is cleaner.
>

Thanks, but this is a can of worms that needs to be opened very
slowly. Could you help test/review my series instead?

https://lore.kernel.org/r/20250630195913.28033-1-farosas@suse.de

> No functional changes intended.
>
> Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
> ---
>  migration/options.c | 70 ++-------------------------------------------
>  1 file changed, 3 insertions(+), 67 deletions(-)
>
> diff --git a/migration/options.c b/migration/options.c
> index 4e923a2e07..347d762b03 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -892,81 +892,16 @@ AnnounceParameters *migrate_announce_params(void)
>  
>  MigrationParameters *qmp_query_migrate_parameters(Error **errp)
>  {
> -    MigrationParameters *params;
>      MigrationState *s = migrate_get_current();
>  
> -    /* TODO use QAPI_CLONE() instead of duplicating it inline */
> -    params = g_malloc0(sizeof(*params));
> -    params->has_throttle_trigger_threshold = true;
> -    params->throttle_trigger_threshold = s->parameters.throttle_trigger_threshold;
> -    params->has_cpu_throttle_initial = true;
> -    params->cpu_throttle_initial = s->parameters.cpu_throttle_initial;
> -    params->has_cpu_throttle_increment = true;
> -    params->cpu_throttle_increment = s->parameters.cpu_throttle_increment;
> -    params->has_cpu_throttle_tailslow = true;
> -    params->cpu_throttle_tailslow = s->parameters.cpu_throttle_tailslow;
> -    params->tls_creds = g_strdup(s->parameters.tls_creds);
> -    params->tls_hostname = g_strdup(s->parameters.tls_hostname);
> -    params->tls_authz = g_strdup(s->parameters.tls_authz ?
> -                                 s->parameters.tls_authz : "");
> -    params->has_max_bandwidth = true;
> -    params->max_bandwidth = s->parameters.max_bandwidth;
> -    params->has_avail_switchover_bandwidth = true;
> -    params->avail_switchover_bandwidth = s->parameters.avail_switchover_bandwidth;
> -    params->has_downtime_limit = true;
> -    params->downtime_limit = s->parameters.downtime_limit;
> -    params->has_x_checkpoint_delay = true;
> -    params->x_checkpoint_delay = s->parameters.x_checkpoint_delay;
> -    params->has_multifd_channels = true;
> -    params->multifd_channels = s->parameters.multifd_channels;
> -    params->has_multifd_compression = true;
> -    params->multifd_compression = s->parameters.multifd_compression;
> -    params->has_multifd_zlib_level = true;
> -    params->multifd_zlib_level = s->parameters.multifd_zlib_level;
> -    params->has_multifd_qatzip_level = true;
> -    params->multifd_qatzip_level = s->parameters.multifd_qatzip_level;
> -    params->has_multifd_zstd_level = true;
> -    params->multifd_zstd_level = s->parameters.multifd_zstd_level;
> -    params->has_xbzrle_cache_size = true;
> -    params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
> -    params->has_max_postcopy_bandwidth = true;
> -    params->max_postcopy_bandwidth = s->parameters.max_postcopy_bandwidth;
> -    params->has_max_cpu_throttle = true;
> -    params->max_cpu_throttle = s->parameters.max_cpu_throttle;
> -    params->has_announce_initial = true;
> -    params->announce_initial = s->parameters.announce_initial;
> -    params->has_announce_max = true;
> -    params->announce_max = s->parameters.announce_max;
> -    params->has_announce_rounds = true;
> -    params->announce_rounds = s->parameters.announce_rounds;
> -    params->has_announce_step = true;
> -    params->announce_step = s->parameters.announce_step;
> -
> -    if (s->parameters.has_block_bitmap_mapping) {
> -        params->has_block_bitmap_mapping = true;
> -        params->block_bitmap_mapping =
> -            QAPI_CLONE(BitmapMigrationNodeAliasList,
> -                       s->parameters.block_bitmap_mapping);
> -    }
> -
> -    params->has_x_vcpu_dirty_limit_period = true;
> -    params->x_vcpu_dirty_limit_period = s->parameters.x_vcpu_dirty_limit_period;
> -    params->has_vcpu_dirty_limit = true;
> -    params->vcpu_dirty_limit = s->parameters.vcpu_dirty_limit;
> -    params->has_mode = true;
> -    params->mode = s->parameters.mode;
> -    params->has_zero_page_detection = true;
> -    params->zero_page_detection = s->parameters.zero_page_detection;
> -    params->has_direct_io = true;
> -    params->direct_io = s->parameters.direct_io;
> -
> -    return params;
> +    return QAPI_CLONE(MigrationParameters, &s->parameters);
>  }
>  
>  void migrate_params_init(MigrationParameters *params)
>  {
>      params->tls_hostname = g_strdup("");
>      params->tls_creds = g_strdup("");
> +    params->tls_authz = g_strdup("");
>  
>      /* Set has_* up only for parameter checks */
>      params->has_throttle_trigger_threshold = true;
> @@ -974,6 +909,7 @@ void migrate_params_init(MigrationParameters *params)
>      params->has_cpu_throttle_increment = true;
>      params->has_cpu_throttle_tailslow = true;
>      params->has_max_bandwidth = true;
> +    params->has_avail_switchover_bandwidth = true;
>      params->has_downtime_limit = true;
>      params->has_x_checkpoint_delay = true;
>      params->has_multifd_channels = true;