Re: [Qemu-devel] [PATCH v3 1/3] Add -incoming defer

[Markus Armbruster]

"Dr. David Alan Gilbert (git)" <address@hidden> writes:

> From: "Dr. David Alan Gilbert" <address@hidden>
>
> -incoming defer causes qemu to wait for an incoming migration
> to be specified later.  The monitor can be used to set migration
> capabilities that may affect the incoming connection process.
>
> Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> Reviewed-by: Juan Quintela <address@hidden>
> ---
>  migration/migration.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index b3adbc6..f3d49d5 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -49,6 +49,8 @@ enum {
>  static NotifierList migration_state_notifiers =
>      NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
>  
> +static bool deferred_incoming;
> +
>  /* When we add fault tolerance, we could have several
>     migrations at once.  For now we don't need to add
>     dynamic creation of migration */
> @@ -65,25 +67,40 @@ MigrationState *migrate_get_current(void)
>      return &current_migration;
>  }
>  
> +/*
> + * Called on -incoming with a defer: uri.

The colon in "defer:" is inaccurate, because...

> + * The migration can be started later after any parameters have been
> + * changed.
> + */
> +static void deferred_incoming_migration(Error **errp)
> +{
> +    if (deferred_incoming) {
> +        error_setg(errp, "Incoming migration already deferred");
> +    }
> +    deferred_incoming = true;
> +}
> +
>  void qemu_start_incoming_migration(const char *uri, Error **errp)
>  {
>      const char *p;
>  
> -    if (strstart(uri, "tcp:", &p))
> +    if (!strcmp(uri, "defer")) {

... you recognize exactly "defer" here.  <pedantic>Which makes it not an
URI</pedantic>.

> +        deferred_incoming_migration(errp);
> +    } else if (strstart(uri, "tcp:", &p)) {
>          tcp_start_incoming_migration(p, errp);
>  #ifdef CONFIG_RDMA
> -    else if (strstart(uri, "rdma:", &p))
> +    } else if (strstart(uri, "rdma:", &p)) {
>          rdma_start_incoming_migration(p, errp);
>  #endif
>  #if !defined(WIN32)
> -    else if (strstart(uri, "exec:", &p))
> +    } else if (strstart(uri, "exec:", &p)) {
>          exec_start_incoming_migration(p, errp);
> -    else if (strstart(uri, "unix:", &p))
> +    } else if (strstart(uri, "unix:", &p)) {
>          unix_start_incoming_migration(p, errp);
> -    else if (strstart(uri, "fd:", &p))
> +    } else if (strstart(uri, "fd:", &p)) {
>          fd_start_incoming_migration(p, errp);
>  #endif
> -    else {
> +    } else {
>          error_setg(errp, "unknown migration protocol: %s", uri);
>      }
>  }

How did you test the new error?

I tried, but ran into this preexisting bug:

    $ qemu-system-x86_64 -nodefaults -S -display none -incoming defer -incoming 
defer
    ERROR: invalid runstate transition: 'inmigrate' -> 'inmigrate'
    Aborted (core dumped)

In my opinion, multiple -incoming should behave like command line
options usually do: last one wins silently.