Markus Armbruster <arm...@redhat.com> writes:
> Fabiano Rosas <faro...@suse.de> writes:
>
>> Add the QMP commands query-migrate-config and migrate-set-config to
>> read and write the migration configuration options.
>
> These supersede query-migrate-capabilities, query-migrate-parameters,
> migrate-set-capabilities, and migrate-set-parameters, right?
>
They could. We haven't made a decision at this moment.
>> Signed-off-by: Fabiano Rosas <faro...@suse.de>
>> ---
>> migration/options.c | 24 ++++++++++++++++++++++++
>> migration/options.h | 2 +-
>> qapi/migration.json | 42 ++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 67 insertions(+), 1 deletion(-)
>>
>> diff --git a/migration/options.c b/migration/options.c
>> index 4e3792dec3..c85ee2e506 100644
>> --- a/migration/options.c
>> +++ b/migration/options.c
>> @@ -1441,3 +1441,27 @@ void qmp_migrate_set_parameters(MigrateSetParameters
>> *params, Error **errp)
>> migrate_config_apply(&tmp);
>> migrate_post_update_config(&tmp, errp);
>> }
>> +
>> +void qmp_migrate_set_config(MigrationConfig *config, Error **errp)
>> +{
>> + if (!migrate_config_check(config, errp)) {
>> + /* Invalid parameter */
>> + return;
>> + }
>> +
>> + migrate_config_apply(config);
>> + migrate_post_update_config(config, errp);
>> +}
>> +
>> +MigrationConfig *qmp_query_migrate_config(Error **errp)
>> +{
>> + MigrationState *s = migrate_get_current();
>> + MigrationConfig *config = g_new0(MigrationConfig, 1);
>> +
>> + QAPI_CLONE_MEMBERS(MigrationConfig, config, &s->config);
>> +
>> + /* set the has_* fields for every option */
>> + migrate_config_init(config);
>> +
>> + return config;
>> +}
>> diff --git a/migration/options.h b/migration/options.h
>> index 61ee854bb0..0e36dafe80 100644
>> --- a/migration/options.h
>> +++ b/migration/options.h
>> @@ -72,7 +72,7 @@ uint64_t migrate_xbzrle_cache_size(void);
>> ZeroPageDetection migrate_zero_page_detection(void);
>>
>> bool migrate_config_check(MigrationConfig *params, Error **errp);
>> -void migrate_config_init(MigrationConfig *params);
>> +void migrate_config_init(MigrationConfig *config);
>
> Have you considered renaming the declaration's parameter when you change
> its type in PATCH 08, or when you rename the definition's parameter in
> PATCH 11?
>
Yes, that looks like a mistake during rebase.
>> bool migrate_config_get_cap_compat(MigrationConfig *config, int i);
>> bool migrate_caps_check(MigrationConfig *new, Error **errp);
>> #endif
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 5e39f21adc..bb2487dbc6 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -2552,3 +2552,45 @@
>> 'data': { '*tls-creds': 'str',
>> '*tls-hostname': 'str',
>> '*tls-authz': 'str' } }
>> +
>> +##
>> +# @query-migrate-config:
>> +#
>> +# Returns information about the current migration configuration
>> +# options
>> +#
>> +# Returns: @MigrationConfig
>> +#
>> +# Since: 10.1
>> +#
>> +# .. qmp-example::
>> +#
>> +# -> { "execute": "query-migrate-config" }
>> +# <- { "return": {
>> +# "multifd-channels": 2,
>> +# "cpu-throttle-increment": 10,
>> +# "cpu-throttle-initial": 20,
>> +# "max-bandwidth": 33554432,
>> +# "downtime-limit": 300
>> +# }
>> +# }
>> +##
>> +{ 'command': 'query-migrate-config',
>> + 'returns': 'MigrationConfig' }
>> +
>> +##
>> +# @migrate-set-config:
>> +#
>> +# Set various migration configuration options.
>> +#
>> +# Since: 10.1
>> +#
>> +# .. qmp-example::
>> +#
>> +# -> { "execute": "migrate-set-config" ,
>> +# "arguments": { "max-bandwidth": 33554432,
>> +# "downtime-limit": 300 } }
>> +# <- { "return": {} }
>> +##
>> +{ 'command': 'migrate-set-config', 'boxed': true,
>> + 'data': 'MigrationConfig' }
>
> This patch exposes MigrationConfig externally. We should double-check
> its documentation to make sure it's what we want there. Known issue:
> how to reset @tls-creds & friends. Touched in my review of PATCH 07.
I'll take a closer look at tls options after Daniel suggested changing
MigrationParameters' TLS options type from str to StrOrNull. That allows
us to drop MigrateSetParameters entirely.
