On Mon, Jun 02, 2025 at 10:38:08PM -0300, Fabiano Rosas wrote:
> Allow the migrate and migrate_incoming commands to pass the migration
> configuration options all at once, dispensing the use of
> migrate-set-parameters and migrate-set-capabilities.
>
> The motivation of this is to simplify the interface with the
> management layer and avoid the usage of several command invocations to
> configure a migration. It also avoids stale parameters from a previous
> migration to influence the current migration.
>
> The options that are changed during the migration can still be set
> with the existing commands.
>
> The order of precedence is:
>
> 'config' argument > -global cmdline > defaults (migration_properties)
Could we still keep the QMP migrate-set-parameters values?
'config' argument > QMP setups using migrate-set-parameters >
-global cmdline > defaults (migration_properties)
I asked this before, maybe I forgot the answer..
>
> I.e. the config takes precedence over all, values not present in the
> config assume the default values. The (debug) -global command line
> option allows the defaults to be overridden.
>
> Signed-off-by: Fabiano Rosas <[email protected]>
> ---
> migration/migration-hmp-cmds.c | 5 +++--
> migration/migration.c | 29 ++++++++++++++++++++++++++---
> migration/migration.h | 1 +
> migration/options.c | 30 ++++++++++++++++++++++++++++++
> migration/options.h | 3 +++
> qapi/migration.json | 25 +++++++++++++++++++++++--
> system/vl.c | 3 ++-
> 7 files changed, 88 insertions(+), 8 deletions(-)
>
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index a8c3515e9d..38b289e8d8 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -575,7 +575,7 @@ void hmp_migrate_incoming(Monitor *mon, const QDict
> *qdict)
> }
> QAPI_LIST_PREPEND(caps, g_steal_pointer(&channel));
>
> - qmp_migrate_incoming(NULL, true, caps, true, false, &err);
> + qmp_migrate_incoming(NULL, true, caps, NULL, true, false, &err);
> qapi_free_MigrationChannelList(caps);
>
> end:
> @@ -952,7 +952,8 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
> }
> QAPI_LIST_PREPEND(caps, g_steal_pointer(&channel));
>
> - qmp_migrate(NULL, true, caps, false, false, true, resume, &err);
> + qmp_migrate(NULL, true, caps, NULL, false, false, true, resume,
> + &err);
> if (hmp_handle_error(mon, err)) {
> return;
> }
> diff --git a/migration/migration.c b/migration/migration.c
> index 75c4ec9a95..7b450b8836 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -335,6 +335,7 @@ void migration_object_init(void)
> current_incoming->exit_on_error = INMIGRATE_DEFAULT_EXIT_ON_ERROR;
>
> migration_object_check(current_migration, &error_fatal);
> + migrate_params_store_defaults(current_migration);
>
> ram_mig_init();
> dirty_bitmap_mig_init();
> @@ -1916,13 +1917,24 @@ void migrate_del_blocker(Error **reasonp)
>
> void qmp_migrate_incoming(const char *uri, bool has_channels,
> MigrationChannelList *channels,
> - bool has_exit_on_error, bool exit_on_error,
> - Error **errp)
> + MigrationParameters *config, bool
> has_exit_on_error,
> + bool exit_on_error, Error **errp)
> {
> Error *local_err = NULL;
> static bool once = true;
> + MigrationState *s = migrate_get_current();
> MigrationIncomingState *mis = migration_incoming_get_current();
>
> + if (config) {
> + /*
> + * If a config was provided, all options set previously get
> + * ignored.
> + */
> + if (!migrate_params_override(s, config, errp)) {
> + return;
> + }
> + }
> +
> if (!once) {
> error_setg(errp, "The incoming migration has already been started");
> return;
> @@ -2182,7 +2194,8 @@ static gboolean qmp_migrate_finish_cb(QIOChannel
> *channel,
> }
>
> void qmp_migrate(const char *uri, bool has_channels,
> - MigrationChannelList *channels, bool has_detach, bool
> detach,
> + MigrationChannelList *channels,
> + bool has_detach, bool detach, MigrationParameters *config,
> bool has_resume, bool resume, Error **errp)
> {
> bool resume_requested;
> @@ -2193,6 +2206,16 @@ void qmp_migrate(const char *uri, bool has_channels,
> MigrationChannel *channelv[MIGRATION_CHANNEL_TYPE__MAX] = { NULL };
> MigrationChannel *cpr_channel = NULL;
>
> + if (config) {
> + /*
> + * If a config was provided, all options set previously get
> + * ignored.
> + */
> + if (!migrate_params_override(s, config, errp)) {
> + return;
> + }
> + }
> +
> /*
> * Having preliminary checks for uri and channel
> */
> diff --git a/migration/migration.h b/migration/migration.h
> index 993d51aedd..49761f4699 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -319,6 +319,7 @@ struct MigrationState {
>
> /* params from 'migrate-set-parameters' */
> MigrationParameters parameters;
> + MigrationParameters defaults;
This is also prone to be a pointer; I still think embeded qapi objects are
too error prone. Since it's new, make it a pointer from start?
>
> MigrationStatus state;
>
> diff --git a/migration/options.c b/migration/options.c
> index fa3f7035c8..dd2288187d 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -1333,6 +1333,36 @@ static void migrate_params_apply(MigrationParameters
> *params)
> params->block_bitmap_mapping);
> }
>
> +void migrate_params_store_defaults(MigrationState *s)
> +{
> + /*
> + * The defaults set for each qdev property in migration_properties
> + * will be stored as the default values for each migration
> + * parameter. For debugging, using -global can override the
> + * defaults.
> + */
> + QAPI_CLONE_MEMBERS(MigrationParameters, &s->defaults, &s->parameters);
> +}
> +
> +bool migrate_params_override(MigrationState *s, MigrationParameters *new,
> + Error **errp)
> +{
> + ERRP_GUARD();
> +
> + assert(bql_locked());
> +
> + /* reset to default parameters */
> + migrate_params_apply(&s->defaults);
> +
> + /* overwrite with the new ones */
> + qmp_migrate_set_parameters(new, errp);
> + if (*errp) {
> + return false;
> + }
> +
> + return true;
> +}
> +
> void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
> {
> MigrationParameters *tmp = g_new0(MigrationParameters, 1);
> diff --git a/migration/options.h b/migration/options.h
> index fcfd120cd7..3630c2a0dd 100644
> --- a/migration/options.h
> +++ b/migration/options.h
> @@ -83,4 +83,7 @@ void migrate_capability_set_compat(MigrationParameters
> *params, int i,
> void migrate_capabilities_set_compat(MigrationParameters *params,
> MigrationCapabilityStatusList *caps);
> bool migrate_caps_check(MigrationParameters *new, Error **errp);
> +void migrate_params_store_defaults(MigrationState *s);
> +bool migrate_params_override(MigrationState *s, MigrationParameters *new,
> + Error **errp);
> #endif
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 7282e4b9eb..64a92d8d28 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1474,9 +1474,16 @@
> #
> # @resume: resume one paused migration, default "off". (since 3.0)
> #
> +# @config: migration configuration options, previously set via
> +# @migrate-set-parameters and @migrate-set-capabilities. (since
> +# 10.1)
> +#
> # Features:
> #
> # @deprecated: Argument @detach is deprecated.
> +# @config: Indicates this command can receive the entire migration
> +# configuration via the @config field, dispensing the use of
> +# @migrate-set-parameters.
> #
> # Since: 0.14
> #
> @@ -1538,7 +1545,9 @@
> 'data': {'*uri': 'str',
> '*channels': [ 'MigrationChannel' ],
> '*detach': { 'type': 'bool', 'features': [ 'deprecated' ] },
> - '*resume': 'bool' } }
> + '*config': 'MigrationParameters',
> + '*resume': 'bool' },
> + 'features': [ 'config' ] }
>
> ##
> # @migrate-incoming:
> @@ -1557,6 +1566,16 @@
> # error details could be retrieved with query-migrate.
> # (since 9.1)
> #
> +# @config: migration configuration options, previously set via
> +# @migrate-set-parameters and @migrate-set-capabilities. (since
> +# 10.1)
> +#
> +# Features:
> +#
> +# @config: Indicates this command can receive the entire migration
> +# configuration via the @config field, dispensing the use of
> +# @migrate-set-parameters.
> +#
> # Since: 2.3
> #
> # .. admonition:: Notes
> @@ -1610,7 +1629,9 @@
> { 'command': 'migrate-incoming',
> 'data': {'*uri': 'str',
> '*channels': [ 'MigrationChannel' ],
> - '*exit-on-error': 'bool' } }
> + '*config': 'MigrationParameters',
> + '*exit-on-error': 'bool' },
> + 'features': [ 'config' ] }
>
> ##
> # @xen-save-devices-state:
> diff --git a/system/vl.c b/system/vl.c
> index 3b7057e6c6..b29fd24d08 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -2823,7 +2823,8 @@ void qmp_x_exit_preconfig(Error **errp)
> g_new0(MigrationChannelList, 1);
>
> channels->value = incoming_channels[MIGRATION_CHANNEL_TYPE_MAIN];
> - qmp_migrate_incoming(NULL, true, channels, true, true,
> &local_err);
> + qmp_migrate_incoming(NULL, true, channels, NULL, true, true,
> + &local_err);
> if (local_err) {
> error_reportf_err(local_err, "-incoming %s: ", incoming);
> exit(1);
> --
> 2.35.3
>
--
Peter Xu
