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 <[email protected]> > > Signed-off-by: Fabiano Rosas <[email protected]> > --- > 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
