Re: [PATCH v2 4/7] migration: Split migration_channel_process_incoming()

[Fabiano Rosas <farosas@suse.de>]

Peter Xu <peterx@redhat.com> writes:

> Introduce two smaller functions for it,
>
> - migration_incoming_channel_install(): peek at channels whenever possible,
>   and install the channel to one of the slots (main, multifd, preempt)
>
> - migration_incoming_error_propagate(): handling an error happened during
>   channel establish phase of an incoming migration.
>
> When at it, move the set channel tracepoint into the 1st helper, replacing
> the existing tracepoint with trace_migration_channel_process_incoming().
> So far, they should fire almost the same time if the channel peek worked.
> This fact will change in follow up refactorings.
>
> I also moved migration_ioc_register_yank() to be after identify, hopefully
> it makes more sense: we'd better be careful on registration of yank, or we
> should be prepared for explosions.  Previously it worked, my guess is when
> error happens we never unregister yank but hang dest QEMU or quit,
> depending on exit_on_error value.
>
> No real functional change intended.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/channel.c    | 57 +++++++++++++++++++++++++++++-------------
>  migration/trace-events |  1 +
>  2 files changed, 41 insertions(+), 17 deletions(-)
>
> diff --git a/migration/channel.c b/migration/channel.c
> index 266ae8f776..8407ea46cb 100644
> --- a/migration/channel.c
> +++ b/migration/channel.c
> @@ -197,6 +197,42 @@ out:
>      return channel;
>  }
>  
> +static void migration_incoming_error_propagate(MigrationIncomingState *mis,
> +                                               Error *error)
> +{
> +    error_report_err(error);
> +    migrate_set_state(&mis->state, mis->state, MIGRATION_STATUS_FAILED);
> +    if (mis->exit_on_error) {
> +        exit(EXIT_FAILURE);
> +    }
> +}
> +
> +static bool migration_incoming_channel_install(MigrationIncomingState *mis,
> +                                               QIOChannel *ioc,
> +                                               Error **errp)
> +{
> +    MigChannelType ch = migration_channel_identify(mis, ioc, errp);
> +
> +    if (!ch) {
> +        assert(*errp);
> +        return false;
> +    }
> +
> +    trace_migration_set_incoming_channel(ioc,
> +                                         object_get_typename(OBJECT(ioc)));
> +    migration_ioc_register_yank(ioc);
> +
> +    if (migration_incoming_setup(ioc, ch, errp)) {
> +        migration_start_incoming();
> +    }
> +
> +    if (*errp) {
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
>  /**
>   * @migration_channel_process_incoming - Create new incoming migration channel
>   *
> @@ -209,31 +245,18 @@ void migration_channel_process_incoming(QIOChannel *ioc)
>  {
>      MigrationIncomingState *mis = migration_incoming_get_current();
>      Error *local_err = NULL;
> -    MigChannelType ch;
>  
> -    trace_migration_set_incoming_channel(
> +    trace_migration_channel_process_incoming(
>          ioc, object_get_typename(OBJECT(ioc)));
>  
>      if (migrate_channel_requires_tls_upgrade(ioc)) {
>          migration_tls_channel_process_incoming(ioc, &local_err);
>      } else {
> -        migration_ioc_register_yank(ioc);
> -        ch = migration_channel_identify(mis, ioc, &local_err);
> -        if (!ch) {
> -            goto out;
> -        }
> -
> -        if (migration_incoming_setup(ioc, ch, &local_err)) {
> -            migration_start_incoming();
> -        }
> +        migration_incoming_channel_install(mis, ioc, &local_err);
>      }
> -out:
> +
>      if (local_err) {
> -        error_report_err(local_err);
> -        migrate_set_state(&mis->state, mis->state, MIGRATION_STATUS_FAILED);
> -        if (mis->exit_on_error) {
> -            exit(EXIT_FAILURE);
> -        }
> +        migration_incoming_error_propagate(mis, local_err);
>      }
>  }
>  
> diff --git a/migration/trace-events b/migration/trace-events
> index 68bcf12dc7..c93c9f9e75 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -207,6 +207,7 @@ migration_start_incoming(void) ""
>  migration_transferred_bytes(uint64_t qemu_file, uint64_t multifd, uint64_t rdma) "qemu_file %" PRIu64 " multifd %" PRIu64 " RDMA %" PRIu64
>  
>  # channel.c
> +migration_channel_process_incoming(void *ioc, const char *ioctype) "ioc=%p ioctype=%s"
>  migration_set_incoming_channel(void *ioc, const char *ioctype) "ioc=%p ioctype=%s"
>  migration_set_outgoing_channel(void *ioc, const char *ioctype) "ioc=%p ioctype=%s"

Reviewed-by: Fabiano Rosas <farosas@suse.de>