On Fri, 9 Jan 2026 at 18:11, Fabiano Rosas <[email protected]> wrote:
> Simplify migration_channel_connect() and migration_connect() to not
> take an error as input. Move the error handling into the paths that
> generate the error.
>
> To achieve this, call migration_connect_error_propagate() from
> socket.c and tls.c, which are the async paths.
>
> For the sync paths, the handling is done as normal by returning all
> the way to qmp_migrate_finish(), except that now the sync paths don't
> pass the error forward into migration_connect() anymore.
>
> Reviewed-by: Peter Xu <[email protected]>
> Signed-off-by: Fabiano Rosas <[email protected]>
> ---
> migration/channel.c | 49 ++++++++++++++++++++----------------------
> migration/channel.h | 4 +---
> migration/exec.c | 2 +-
> migration/fd.c | 2 +-
> migration/file.c | 2 +-
> migration/migration.c | 11 ++--------
> migration/migration.h | 3 ++-
> migration/rdma.c | 2 +-
> migration/socket.c | 17 ++++++++-------
> migration/tls.c | 14 ++++++------
> migration/tls.h | 3 +--
> migration/trace-events | 2 +-
> 12 files changed, 50 insertions(+), 61 deletions(-)
>
> diff --git a/migration/channel.c b/migration/channel.c
> index ba14f66d85..26cb7bf059 100644
> --- a/migration/channel.c
> +++ b/migration/channel.c
> @@ -60,38 +60,35 @@ void migration_channel_process_incoming(QIOChannel *ioc)
> *
> * @s: Current migration state
> * @ioc: Channel to which we are connecting
> - * @error: Error indicating failure to connect, free'd here
> */
> -void migration_channel_connect(MigrationState *s,
> - QIOChannel *ioc,
> - Error *error)
> +void migration_channel_connect(MigrationState *s, QIOChannel *ioc)
> {
> - trace_migration_set_outgoing_channel(
> - ioc, object_get_typename(OBJECT(ioc)), error);
> + trace_migration_set_outgoing_channel(ioc,
> object_get_typename(OBJECT(ioc)));
>
> - if (!error) {
> - if (migrate_channel_requires_tls_upgrade(ioc)) {
> - migration_tls_channel_connect(s, ioc, &error);
> + if (migrate_channel_requires_tls_upgrade(ioc)) {
> + Error *local_err = NULL;
>
> - if (!error) {
> - /* tls_channel_connect will call back to this
> - * function after the TLS handshake,
> - * so we mustn't call migration_connect until then
> - */
> -
> - return;
> - }
> - } else {
> - QEMUFile *f = qemu_file_new_output(ioc);
> -
> - migration_ioc_register_yank(ioc);
> -
> - qemu_mutex_lock(&s->qemu_file_lock);
> - s->to_dst_file = f;
> - qemu_mutex_unlock(&s->qemu_file_lock);
> + migration_tls_channel_connect(s, ioc, &local_err);
> + if (local_err) {
> + migration_connect_error_propagate(s, local_err);
> }
> +
> + /*
> + * async: the above will call back to this function after
> + * the TLS handshake is successfully completed.
> + */
> + return;
> }
> - migration_connect(s, error);
> +
> + QEMUFile *f = qemu_file_new_output(ioc);
> +
> + migration_ioc_register_yank(ioc);
> +
> + qemu_mutex_lock(&s->qemu_file_lock);
> + s->to_dst_file = f;
> + qemu_mutex_unlock(&s->qemu_file_lock);
> +
> + migration_connect(s);
> }
>
>
> diff --git a/migration/channel.h b/migration/channel.h
> index 2215091323..ccfeaaef18 100644
> --- a/migration/channel.h
> +++ b/migration/channel.h
> @@ -20,9 +20,7 @@
>
> void migration_channel_process_incoming(QIOChannel *ioc);
>
> -void migration_channel_connect(MigrationState *s,
> - QIOChannel *ioc,
> - Error *error_in);
> +void migration_channel_connect(MigrationState *s, QIOChannel *ioc);
>
> int migration_channel_read_peek(QIOChannel *ioc,
> const char *buf,
> diff --git a/migration/exec.c b/migration/exec.c
> index 78fe0fff13..d83a07435a 100644
> --- a/migration/exec.c
> +++ b/migration/exec.c
> @@ -55,7 +55,7 @@ void exec_start_outgoing_migration(MigrationState *s,
> strList *command,
> }
>
> qio_channel_set_name(ioc, "migration-exec-outgoing");
> - migration_channel_connect(s, ioc, NULL);
> + migration_channel_connect(s, ioc);
> object_unref(OBJECT(ioc));
> }
>
> diff --git a/migration/fd.c b/migration/fd.c
> index c956b260a4..0144a70742 100644
> --- a/migration/fd.c
> +++ b/migration/fd.c
> @@ -70,7 +70,7 @@ void fd_start_outgoing_migration(MigrationState *s, const
> char *fdname, Error **
> }
>
> qio_channel_set_name(ioc, "migration-fd-outgoing");
> - migration_channel_connect(s, ioc, NULL);
> + migration_channel_connect(s, ioc);
> object_unref(OBJECT(ioc));
> }
>
> diff --git a/migration/file.c b/migration/file.c
> index c490f2b219..7bb9c1c79f 100644
> --- a/migration/file.c
> +++ b/migration/file.c
> @@ -122,7 +122,7 @@ void file_start_outgoing_migration(MigrationState *s,
> return;
> }
> qio_channel_set_name(ioc, "migration-file-outgoing");
> - migration_channel_connect(s, ioc, NULL);
> + migration_channel_connect(s, ioc);
> }
>
> static gboolean file_accept_incoming_migration(QIOChannel *ioc,
> diff --git a/migration/migration.c b/migration/migration.c
> index d57cc2dc3b..1ea6125454 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1572,7 +1572,7 @@ static void migrate_error_free(MigrationState *s)
> }
> }
>
> -static void migration_connect_error_propagate(MigrationState *s, Error
> *error)
> +void migration_connect_error_propagate(MigrationState *s, Error *error)
> {
> MigrationStatus current = s->state;
> MigrationStatus next = MIGRATION_STATUS_NONE;
> @@ -4028,7 +4028,7 @@ fail_setup:
> return NULL;
> }
>
> -void migration_connect(MigrationState *s, Error *error_in)
> +void migration_connect(MigrationState *s)
> {
> Error *local_err = NULL;
> uint64_t rate_limit;
> @@ -4036,13 +4036,6 @@ void migration_connect(MigrationState *s, Error
> *error_in)
> int ret;
>
> s->expected_downtime = migrate_downtime_limit();
> - if (error_in) {
> - migration_connect_error_propagate(s, error_in);
> - if (s->error) {
> - error_report_err(error_copy(s->error));
> - }
> - return;
> - }
>
> if (resume) {
> /* This is a resumed migration */
> diff --git a/migration/migration.h b/migration/migration.h
> index 8b55d4741a..d134881eaf 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -533,10 +533,11 @@ void migration_incoming_process(void);
>
> bool migration_has_all_channels(void);
>
> +void migration_connect_error_propagate(MigrationState *s, Error *error);
> void migrate_error_propagate(MigrationState *s, Error *error);
> bool migrate_has_error(MigrationState *s);
>
> -void migration_connect(MigrationState *s, Error *error_in);
> +void migration_connect(MigrationState *s);
>
> int migration_call_notifiers(MigrationEventType type, Error **errp);
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 337b415889..596a1aba0b 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3997,7 +3997,7 @@ void rdma_start_outgoing_migration(void *opaque,
>
> s->to_dst_file = rdma_new_output(rdma);
> s->rdma_migration = true;
> - migration_connect(s, NULL);
> + migration_connect(s);
> return;
> return_path_err:
> qemu_rdma_cleanup(rdma);
> diff --git a/migration/socket.c b/migration/socket.c
> index 426f363b99..298bac30cc 100644
> --- a/migration/socket.c
> +++ b/migration/socket.c
> @@ -59,24 +59,25 @@ static void socket_outgoing_migration(QIOTask *task,
> gpointer opaque)
> {
> struct SocketConnectData *data = opaque;
> - QIOChannel *sioc = QIO_CHANNEL(qio_task_get_source(task));
> + g_autoptr(QIOChannel) sioc = QIO_CHANNEL(qio_task_get_source(task));
> Error *err = NULL;
>
> if (qio_task_propagate_error(task, &err)) {
> - trace_migration_socket_outgoing_error(error_get_pretty(err));
> - goto out;
> + goto err;
> }
* Goto label and variable both are named the same: 'err'.
> - trace_migration_socket_outgoing_connected();
> -
> if (migrate_zero_copy_send() &&
> !qio_channel_has_feature(sioc, QIO_CHANNEL_FEATURE_WRITE_ZERO_COPY))
> {
> error_setg(&err, "Zero copy send feature not detected in host
> kernel");
> + goto err;
> }
>
> -out:
> - migration_channel_connect(data->s, sioc, err);
> - object_unref(OBJECT(sioc));
> + trace_migration_socket_outgoing_connected();
> + migration_channel_connect(data->s, sioc);
> + return;
> +err:
> + trace_migration_socket_outgoing_error(error_get_pretty(err));
> + migration_connect_error_propagate(data->s, err);
> }
>
> void socket_start_outgoing_migration(MigrationState *s,
> diff --git a/migration/tls.c b/migration/tls.c
> index 82f58cbc78..837b08294f 100644
> --- a/migration/tls.c
> +++ b/migration/tls.c
> @@ -104,16 +104,17 @@ static void migration_tls_outgoing_handshake(QIOTask
> *task,
> gpointer opaque)
> {
> MigrationState *s = opaque;
> - QIOChannel *ioc = QIO_CHANNEL(qio_task_get_source(task));
> + g_autoptr(QIOChannel) ioc = QIO_CHANNEL(qio_task_get_source(task));
> Error *err = NULL;
>
> if (qio_task_propagate_error(task, &err)) {
> trace_migration_tls_outgoing_handshake_error(error_get_pretty(err));
> - } else {
> - trace_migration_tls_outgoing_handshake_complete();
> + migration_connect_error_propagate(s, err);
> + return;
> }
> - migration_channel_connect(s, ioc, err);
> - object_unref(OBJECT(ioc));
> +
> + trace_migration_tls_outgoing_handshake_complete();
> + migration_channel_connect(s, ioc);
> }
>
> QIOChannelTLS *migration_tls_client_create(QIOChannel *ioc,
> @@ -129,8 +130,7 @@ QIOChannelTLS *migration_tls_client_create(QIOChannel
> *ioc,
> return qio_channel_tls_new_client(ioc, creds, migrate_tls_hostname(),
> errp);
> }
>
> -void migration_tls_channel_connect(MigrationState *s,
> - QIOChannel *ioc,
> +void migration_tls_channel_connect(MigrationState *s, QIOChannel *ioc,
> Error **errp)
> {
> QIOChannelTLS *tioc;
> diff --git a/migration/tls.h b/migration/tls.h
> index 7cd9c76013..0e86d38862 100644
> --- a/migration/tls.h
> +++ b/migration/tls.h
> @@ -29,8 +29,7 @@ void migration_tls_channel_process_incoming(QIOChannel
> *ioc, Error **errp);
> QIOChannelTLS *migration_tls_client_create(QIOChannel *ioc,
> Error **errp);
>
> -void migration_tls_channel_connect(MigrationState *s,
> - QIOChannel *ioc,
> +void migration_tls_channel_connect(MigrationState *s, QIOChannel *ioc,
> Error **errp);
> void migration_tls_channel_end(QIOChannel *ioc, Error **errp);
> /* Whether the QIO channel requires further TLS handshake? */
> diff --git a/migration/trace-events b/migration/trace-events
> index da8f909cac..cbf10d0b63 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -204,7 +204,7 @@ migration_transferred_bytes(uint64_t qemu_file, uint64_t
> multifd, uint64_t rdma)
>
> # channel.c
> migration_set_incoming_channel(void *ioc, const char *ioctype) "ioc=%p
> ioctype=%s"
> -migration_set_outgoing_channel(void *ioc, const char *ioctype, void *err)
> "ioc=%p ioctype=%s err=%p"
> +migration_set_outgoing_channel(void *ioc, const char *ioctype) "ioc=%p
> ioctype=%s"
/* /me makes a note to check about this syntax, strings after function
signature. */
> # global_state.c
> migrate_state_too_big(void) ""
> --
> 2.51.0
* Change looks okay.
Reviewed-by: Prasad Pandit <[email protected]>
Thank you.
---
- Prasad
