Re: [RFC PATCH 12/13] [PoC] migration: Add query/set commands for MigrationConfig

[Markus Armbruster <armbru@redhat.com>]

Fabiano Rosas <farosas@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?

> Signed-off-by: Fabiano Rosas <farosas@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?

>  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.