Peter Xu <[email protected]> writes: > 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 understand your points, I'm also unsure. I slightly prefer to have the "problem" inside migration/ than inside qapi/. Let me ponder a bit more during the respin, let's see. > 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 >>
