Re: [Qemu-devel] [RFC v2 32/33] migration: allow migrate_incoming for paused VM

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Peter Xu (peterx@redhat.com) 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 <peterx@redhat.com>
> ---
>  migration/exec.c      |  2 +-
>  migration/fd.c        |  2 +-
>  migration/migration.c | 39 +++++++++++++++++++++++++++++----------
>  migration/socket.c    |  2 +-
>  4 files changed, 32 insertions(+), 13 deletions(-)
> 
> diff --git a/migration/exec.c b/migration/exec.c
> index ef1fb4c..26fc37d 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 FALSE; /* unregister */
> +    return TRUE; /* keep it registered */
>  }
>  
>  /*
> diff --git a/migration/fd.c b/migration/fd.c
> index e9a548c..7d0aefa 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 FALSE; /* unregister */
> +    return TRUE; /* keep it registered */
>  }
>  
>  /*
> diff --git a/migration/migration.c b/migration/migration.c
> index daf356b..5812478 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -175,6 +175,17 @@ void migration_incoming_state_destroy(void)
>      qemu_event_destroy(&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()) {
> @@ -432,10 +443,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);
>  }
>  
>  /*
> @@ -1291,14 +1301,25 @@ void migrate_del_blocker(Error *reason)
>  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 (!deferred_incoming &&
> +        mis->state != MIGRATION_STATUS_POSTCOPY_PAUSED) {
> +        error_setg(errp, "For use with '-incoming defer'"
> +                   " or PAUSED postcopy migration only.");
>          return;
>      }
> -    if (!once) {
> -        error_setg(errp, "The incoming migration has already been started");

What guards against someone doing a migrate_incoming after the succesful
completion of an incoming migration?
Also with RDMA the following won't happen so I'm not quite sure what
state we're in.

When we get to non-blocking commands it's also a bit interesting - we
could be getting an accept on the main thread at just the same time
this is going down the OOB side.

Dave

> +
> +    /*
> +     * Destroy existing listening task if exist. Logically this should
> +     * not really happen at all (for either deferred migration or
> +     * postcopy migration, we should both detached the listening
> +     * task). So raise an error but still we safely detach it.
> +     */
> +    if (migrate_incoming_detach_listen(mis)) {
> +        error_report("%s: detected existing listen channel, "
> +                     "while it should not exist", __func__);
> +        /* Continue */
>      }
>  
>      qemu_start_incoming_migration(uri, &local_err);
> @@ -1307,8 +1328,6 @@ void qmp_migrate_incoming(const char *uri, Error **errp)
>          error_propagate(errp, local_err);
>          return;
>      }
> -
> -    once = false;
>  }
>  
>  bool migration_is_blocked(Error **errp)
> diff --git a/migration/socket.c b/migration/socket.c
> index 6ee51ef..e3e453f 100644
> --- a/migration/socket.c
> +++ b/migration/socket.c
> @@ -154,7 +154,7 @@ static gboolean socket_accept_incoming_migration(QIOChannel *ioc,
>  out:
>      /* Close listening socket as its no longer needed */
>      qio_channel_close(ioc, NULL);
> -    return FALSE; /* unregister */
> +    return TRUE; /* keep it registered */
>  }
>  
>  
> -- 
> 2.7.4
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK