Re: [Qemu-devel] [PATCH v4 28/32] migration: allow migrate_incoming for paused VM

[Dr. David Alan Gilbert]

* Peter Xu (address@hidden) wrote:
> migrate_incoming command is previously only used when we were providing
> "-incoming defer" in the command line, to defer the incoming migration
> channel creation.
> 
> However there is similar requirement when we are paused during postcopy
> migration. The old incoming channel might have been destroyed already.
> We may need another new channel for the recovery to happen.
> 
> This patch leveraged the same interface, but allows the user to specify
> incoming migration channel even for paused postcopy.
> 
> Meanwhile, now migration listening ports are always detached manually
> using the tag, rather than using return values of dispatchers.
> 
> Signed-off-by: Peter Xu <address@hidden>

I think this patch is OK now, except for that top level question I've asked
against 00 about how the new incoming ever gets to start up.

Reviewed-by: Dr. David Alan Gilbert <address@hidden>

> ---
>  migration/exec.c       |  2 +-
>  migration/fd.c         |  2 +-
>  migration/migration.c  | 58 
> +++++++++++++++++++++++++++++++++++++++++++-------
>  migration/socket.c     |  4 +---
>  migration/trace-events |  2 ++
>  5 files changed, 55 insertions(+), 13 deletions(-)
> 
> diff --git a/migration/exec.c b/migration/exec.c
> index a0796c2c70..9d20d10899 100644
> --- a/migration/exec.c
> +++ b/migration/exec.c
> @@ -49,7 +49,7 @@ static gboolean exec_accept_incoming_migration(QIOChannel 
> *ioc,
>  {
>      migration_channel_process_incoming(ioc);
>      object_unref(OBJECT(ioc));
> -    return G_SOURCE_REMOVE;
> +    return G_SOURCE_CONTINUE;
>  }
>  
>  /*
> diff --git a/migration/fd.c b/migration/fd.c
> index 7ead2f26cc..54b36888e2 100644
> --- a/migration/fd.c
> +++ b/migration/fd.c
> @@ -49,7 +49,7 @@ static gboolean fd_accept_incoming_migration(QIOChannel 
> *ioc,
>  {
>      migration_channel_process_incoming(ioc);
>      object_unref(OBJECT(ioc));
> -    return G_SOURCE_REMOVE;
> +    return G_SOURCE_CONTINUE;
>  }
>  
>  /*
> diff --git a/migration/migration.c b/migration/migration.c
> index a4cdedcde8..9b7fc56ed8 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -179,6 +179,17 @@ void migration_incoming_state_destroy(void)
>      qemu_event_reset(&mis->main_thread_load_event);
>  }
>  
> +static bool migrate_incoming_detach_listen(MigrationIncomingState *mis)
> +{
> +    if (mis->listen_task_tag) {
> +        /* Never fail */
> +        g_source_remove(mis->listen_task_tag);
> +        mis->listen_task_tag = 0;
> +        return true;
> +    }
> +    return false;
> +}
> +
>  static void migrate_generate_event(int new_state)
>  {
>      if (migrate_use_events()) {
> @@ -463,10 +474,9 @@ void migration_fd_process_incoming(QEMUFile *f)
>  
>      /*
>       * When reach here, we should not need the listening port any
> -     * more. We'll detach the listening task soon, let's reset the
> -     * listen task tag.
> +     * more.  Detach the listening port explicitly.
>       */
> -    mis->listen_task_tag = 0;
> +    migrate_incoming_detach_listen(mis);
>  }
>  
>  void migration_ioc_process_incoming(QIOChannel *ioc)
> @@ -1422,14 +1432,46 @@ void qmp_migrate_incoming(const char *uri, Error 
> **errp)
>  {
>      Error *local_err = NULL;
>      static bool once = true;
> +    MigrationIncomingState *mis = migration_incoming_get_current();
> +
>  
> -    if (!deferred_incoming) {
> -        error_setg(errp, "For use with '-incoming defer'");
> +    if (mis->state == MIGRATION_STATUS_POSTCOPY_PAUSED) {
> +        if (mis->listen_task_tag) {
> +            error_setg(errp, "We already have a listening port!");
> +            return;
> +        } else {
> +            /*
> +            * We are in postcopy-paused state, and we don't have
> +            * listening port.  It's very possible that the old
> +            * listening port is already gone, so we allow to create a
> +            * new one.
> +            *
> +            * NOTE: RDMA migration currently does not really use
> +            * listen_task_tag for now, so even if listen_task_tag is
> +            * zero, RDMA can still have its accept port listening.
> +            * However, RDMA is not supported by postcopy at all (yet), so
> +            * we are safe here.
> +            */
> +            trace_migrate_incoming_recover();
> +        }
> +    } else if (deferred_incoming) {
> +        /*
> +         * We don't need recovery, but we possibly has a deferred
> +         * incoming parameter, this allows us to manually specify
> +         * incoming port once.
> +         */
> +        if (!once) {
> +            error_setg(errp, "The incoming migration has already been 
> started");
> +            return;
> +        } else {
> +            /* PASS */
> +            trace_migrate_incoming_deferred();
> +        }
> +    } else {
> +        error_setg(errp, "Migrate-incoming is only allowed for either "
> +                   "deferred incoming, or postcopy paused stage.");
>          return;
>      }
> -    if (!once) {
> -        error_setg(errp, "The incoming migration has already been started");
> -    }
>  
>      qemu_start_incoming_migration(uri, &local_err);
>  
> diff --git a/migration/socket.c b/migration/socket.c
> index e8f3325155..54095a80a0 100644
> --- a/migration/socket.c
> +++ b/migration/socket.c
> @@ -155,10 +155,8 @@ out:
>      if (migration_has_all_channels()) {
>          /* Close listening socket as its no longer needed */
>          qio_channel_close(ioc, NULL);
> -        return G_SOURCE_REMOVE;
> -    } else {
> -        return G_SOURCE_CONTINUE;
>      }
> +    return G_SOURCE_CONTINUE;
>  }
>  
>  
> diff --git a/migration/trace-events b/migration/trace-events
> index 98c2e4de58..65b1c7e459 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -136,6 +136,8 @@ process_incoming_migration_co_end(int ret, int ps) 
> "ret=%d postcopy-state=%d"
>  process_incoming_migration_co_postcopy_end_main(void) ""
>  migration_set_incoming_channel(void *ioc, const char *ioctype) "ioc=%p 
> ioctype=%s"
>  migration_set_outgoing_channel(void *ioc, const char *ioctype, const char 
> *hostname)  "ioc=%p ioctype=%s hostname=%s"
> +migrate_incoming_deferred(void) ""
> +migrate_incoming_recover(void) ""
>  
>  # migration/rdma.c
>  qemu_rdma_accept_incoming_migration(void) ""
> -- 
> 2.13.6
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK