Peter Xu <[email protected]> writes:
> QEMU doesn't yet have a good way to specify migration parameters so that
> they can be available even during early stage of QEMU boots. It is because
> the migration object (who owns the migration parameters) will only be
> created after PHASE_LATE_BACKENDS_CREATED. It means anything before it
> reading migration parameters is illegal.
>
> However, QEMU does have special use cases for such, namely only-migratable
> flag, and cpr-transfer. Recently, we have one more possible user to read a
> to-be-introduced new migration parameters during backend initialization
> phase. We can introduce yet another global variable (or per-device
> parameter) to bypass this limitation, but we can also seek for a generic
> solution that we can setup migration parameters very early, even during
> backend initializations.
>
> See this discussion for more details on the context of the problem:
>
> https://lore.kernel.org/r/[email protected]
>
> This patch wants to take the latter approach.
>
> As a start, introduce a new way to specify migration parameters in QEMU
> boot commandline, as proposed in the above discussion:
>
> -incoming config:key1=value1,key2=value2,...
>
> When specified, QEMU will parse a string formatted MigrationParameters and
> keep it. When migration object is created, it will apply all the settings
> as initial value.
>
> Since the application of boot parameters will be after object_new()
> completes, it means it happens after all machine compat properties or
> -global settings (which should be done during instance_post_init()).
>
> So far, it's still only a way to specify parameters. All parameters are
> not visible before migration object created, like before. Any parameter
> that needs to be visible during boot will need to opt-in this feature.
> Follow up patches will switch the current users to use this model.
>
> Signed-off-by: Peter Xu <[email protected]>
> ---
> include/migration/misc.h | 5 ++++
> migration/migration.c | 54 ++++++++++++++++++++++++++++++++++++++++
> system/vl.c | 7 ++++++
> qemu-options.hx | 18 ++++++++++++--
> 4 files changed, 82 insertions(+), 2 deletions(-)
>
> diff --git a/include/migration/misc.h b/include/migration/misc.h
> index 3159a5e53c..aff79b1380 100644
> --- a/include/migration/misc.h
> +++ b/include/migration/misc.h
> @@ -133,6 +133,11 @@ bool migrate_is_uri(const char *uri);
> /* Parse @uri and return @channel, returning true on success */
> bool migrate_uri_parse(const char *uri, MigrationChannel **channel,
> Error **errp);
> +/*
> + * Parse @config_str in form of "config:key1=value1,..." to initialize
> + * migration parameters.
> + */
> +bool migration_parameters_boot_parse(const char *config_str, Error **errp);
>
'boot' is not the right word. Maybe 'early'?
> /* migration/multifd-device-state.c */
> typedef struct SaveCompletePrecopyThreadData {
> diff --git a/migration/migration.c b/migration/migration.c
> index 074d3f2c69..d918be7a44 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -41,6 +41,7 @@
> #include "qapi/qapi-commands-migration.h"
> #include "qapi/qapi-events-migration.h"
> #include "qapi/qmp/qerror.h"
> +#include "qapi/qobject-input-visitor.h"
> #include "qobject/qnull.h"
> #include "qemu/rcu.h"
> #include "postcopy-ram.h"
> @@ -94,6 +95,9 @@ enum mig_rp_message_type {
> static MigrationState *current_migration;
> static MigrationIncomingState *current_incoming;
>
> +/* Only used during boot, destroyed after migration object initialized */
> +static MigrationParameters *mig_boot_params;
> +
> static GSList *migration_blockers[MIG_MODE__MAX];
>
> static bool migration_object_check(MigrationState *ms, Error **errp);
> @@ -102,6 +106,45 @@ static bool
> stop_return_path_thread_on_source(MigrationState *s);
> static void migration_release_dst_files(MigrationState *ms);
> static void migration_completion_end(MigrationState *s);
>
> +bool migration_parameters_boot_parse(const char *config_str, Error **errp)
> +{
> + Visitor *v;
> +
> + if (mig_boot_params) {
> + error_setg(errp, "Only one -incoming config:* is allowed.");
> + return false;
> + }
> +
> + v = qobject_input_visitor_new_str(config_str, NULL, errp);
> + if (!v) {
> + goto fail;
> + }
> +
> + if (!visit_type_MigrationParameters(v, NULL, &mig_boot_params, errp)) {
> + goto fail;
> + }
> +
> + visit_free(v);
> + return true;
> +
> +fail:
> + visit_free(v);
> + return false;
> +}
> +
> +static void migration_parameters_boot_apply(void)
> +{
> + if (mig_boot_params) {
> + /*
> + * This can fail, because qobject visitor doesn't do sanity check
> + * on values while parsing. It's not too late; we're still in boot
> + * phase.
> + */
> + qmp_migrate_set_parameters(mig_boot_params, &error_abort);
> + g_clear_pointer(&mig_boot_params, qapi_free_MigrationParameters);
> + }
> +}
> +
> static void migration_downtime_start(MigrationState *s)
> {
> trace_vmstate_downtime_checkpoint("src-downtime-start");
> @@ -322,6 +365,17 @@ void migration_object_init(void)
>
> current_incoming->exit_on_error = INMIGRATE_DEFAULT_EXIT_ON_ERROR;
>
> + /*
> + * Apply boot migration parameters in case the user specified some via
> + * command line "-incoming config:*". NOTE: this will overwrite machine
> + * type compat properties and -global settings!
> + */
Hmm, but then it's exactly the same as -global migration.* ? Why do we
need to extend -incoming?
Also, I don't see how qmp_migrate_set_parameters can overwrite compat
properties. They're not part of MigrationParameters.
> + migration_parameters_boot_apply();
> +
> + /*
> + * The boot parameters should have been verified already, but leave it
> + * after applying boot parameters to do one check for everything.
> + */
Will it get expensive? We do that for every QEMU start.
> migration_object_check(current_migration, &error_fatal);
>
> ram_mig_init();
> diff --git a/system/vl.c b/system/vl.c
> index da36b2c6e1..49f5fa0c7b 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -1838,6 +1838,13 @@ static void incoming_option_parse(const char *str)
>
> if (!strcmp(str, "defer")) {
> channel = NULL;
> + } else if (!strncmp(str, "config:", 7)) {
> + /*
> + * This is not a channel setup, but configuration to incoming
> + * migration parameters to make them available during early boot.
> + */
> + migration_parameters_boot_parse(str + 7, &error_fatal);
> + return;
> } else if (migrate_is_uri(str)) {
> migrate_uri_parse(str, &channel, &error_fatal);
> } else {
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 96ae41f787..1fc92a409a 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -5366,11 +5366,15 @@ DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \
> "-incoming <channel>\n" \
> " accept incoming migration on the migration channel\n" \
> "-incoming defer\n" \
> - " wait for the URI to be specified via
> migrate_incoming\n",
> + " wait for the URI to be specified via migrate_incoming\n"
> + "-incoming config:key1=value1,key2=value2,...\n" \
> + " specify migration parameters valid even during boot\n",
> QEMU_ARCH_ALL)
> SRST
> +
> The -incoming option specifies the migration channel for an incoming
> -migration. It may be used multiple times to specify multiple
> +migration, or can also be used to setup migration parameters for the
> +incoming migration. It may be used multiple times to specify multiple
> migration channel types. The channel type is specified in <channel>,
> or is 'main' for all other forms of -incoming. If multiple -incoming
> options are specified for a channel type, the last one takes precedence.
> @@ -5411,6 +5415,16 @@ options are specified for a channel type, the last one
> takes precedence.
> Wait for the URI to be specified via migrate\_incoming. The monitor
> can be used to change settings (such as migration parameters) prior
> to issuing the migrate\_incoming to allow the migration to begin.
> +
> +``-incoming config:key1=value1[,key2=value2,...]``
> +
> + Specify migration parameters in QEMU commandlines, so that these
command line
> + parameters will be available even during very early boot of QEMU.
> + They will be applied properly after QEMU boots and when the migration
The "applied properly" part is implementation detail.
> + core is initialized. From that POV, it has similar effect as setting
> + these parameters using QMP command ``migrate-set-parameters`` or HMP
> + command ``migrate_set_parameter``.
> +
> ERST
>
> DEF("only-migratable", 0, QEMU_OPTION_only_migratable, \
