Re: [PATCH 15/18] qapi/migration: Remove MigrationParameter

[Peter Xu <peterx@redhat.com>]

On Wed, Sep 02, 2026 at 07:15:43PM -0300, Fabiano Rosas wrote:
> This enum is convenient in two ways (just how enums work):
> 
> 1- It provides the number of migration parameters as its __MAX member.
> 
> 2- It allows iterating over an integer range and get a migration
>    parameter name string corresponding to that position in the enum.
> 
> The migration code doesn't have the need for (2) anymore.
> 
> Balancing the benefit of (1) versus the disadvantage of requiring
> migration.json to be updated in two different places whenever a
> parameter is added, experience shows that the latter churn is enough
> to decide to remove the enum.

I may have a slightly different feeling, but that takes a few things into
account, (1) we don't have issue duplicating docs for the two parameter
names anymore, (2) the dup is only about adding the same string once more
in qapi/, (3) we now have this migrate_mark_all_params_present() function
that must set all has_* fields, which the __MAX did help to guard a bit..

I think it's also fine to remove it completely, we just need to be more
careful instead on migrate_mark_all_params_present() later.  Which one is
easier to be forgotten?  I don't know..

Acked-by: Peter Xu <peterx@redhat.com>

> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  migration/options.c |  6 +-----
>  qapi/migration.json | 36 ------------------------------------
>  2 files changed, 1 insertion(+), 41 deletions(-)
> 
> diff --git a/migration/options.c b/migration/options.c
> index 5d17acdd881..f988b181f0e 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -1127,7 +1127,6 @@ static MigrationParameters *migrate_params_from_dict(QDict *d, Error **errp)
>   */
>  static void migrate_mark_all_params_present(MigrationParameters *p)
>  {
> -    int len, n_str_args = 3; /* tls-creds, tls-hostname, tls-authz */
>      bool *has_fields[] = {
>          &p->has_throttle_trigger_threshold, &p->has_cpu_throttle_initial,
>          &p->has_cpu_throttle_increment, &p->has_cpu_throttle_tailslow,
> @@ -1144,10 +1143,7 @@ static void migrate_mark_all_params_present(MigrationParameters *p)
>          &p->has_x_rdma_chunk_size, &p->has_cpr_exec_command,
>      };
>  
> -    len = ARRAY_SIZE(has_fields);
> -    assert(len + n_str_args == MIGRATION_PARAMETER__MAX);
> -
> -    for (int i = 0; i < len; i++) {
> +    for (int i = 0; i < ARRAY_SIZE(has_fields); i++) {
>          *has_fields[i] = true;
>      }
>  }
> diff --git a/qapi/migration.json b/qapi/migration.json
> index b1eaf7b0545..78c6e933cf1 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -796,42 +796,6 @@
>        'bitmaps': [ 'BitmapMigrationBitmapAlias' ]
>    } }
>  
> -##
> -# @MigrationParameter:
> -#
> -# Migration parameters enumeration.  The enumeration values mirror the
> -# members of @MigrationParameters.
> -#
> -# Features:
> -#
> -# @unstable: Members @x-checkpoint-delay, @x-rdma-chunk-size, and
> -#     @x-vcpu-dirty-limit-period are experimental.
> -#
> -# Since: 2.4
> -##
> -{ 'enum': 'MigrationParameter',
> -  'data': ['announce-initial', 'announce-max',
> -           'announce-rounds', 'announce-step',
> -           'throttle-trigger-threshold',
> -           'cpu-throttle-initial', 'cpu-throttle-increment',
> -           'cpu-throttle-tailslow',
> -           'tls-creds', 'tls-hostname', 'tls-authz', 'max-bandwidth',
> -           'avail-switchover-bandwidth', 'downtime-limit',
> -           { 'name': 'x-checkpoint-delay', 'features': [ 'unstable' ] },
> -           'multifd-channels',
> -           'xbzrle-cache-size', 'max-postcopy-bandwidth',
> -           'max-cpu-throttle', 'multifd-compression',
> -           'multifd-zlib-level', 'multifd-zstd-level',
> -           'multifd-qatzip-level',
> -           'block-bitmap-mapping',
> -           { 'name': 'x-vcpu-dirty-limit-period', 'features': ['unstable'] },
> -           'vcpu-dirty-limit',
> -           'mode',
> -           'zero-page-detection',
> -           'direct-io',
> -           { 'name': 'x-rdma-chunk-size', 'features': [ 'unstable' ] },
> -           'cpr-exec-command'] }
> -
>  ##
>  # @migrate-set-parameters:
>  #
> -- 
> 2.53.0
> 

-- 
Peter Xu