On Mon, Jan 05, 2026 at 04:06:42PM -0300, Fabiano Rosas wrote: > Make the synchronous calls evident by not hiding the call to > migration_channel_connect_outgoing() in the transport code. Have those > functions return and call the function at the upper level. > > This helps with navigation: the transport code returns the ioc, > there's no need to look into them when browsing the code. > > It also allows RDMA in the source side to use the same path as the > rest of the transports. > > While here, document the async calls which are the exception. > > Signed-off-by: Fabiano Rosas <[email protected]>
Reviewed-by: Peter Xu <[email protected]> One question inline. > --- > migration/channel.c | 28 ++++++++++++++++++++++++---- > migration/exec.c | 8 ++++---- > migration/exec.h | 5 ++++- > migration/fd.c | 13 +++++++------ > migration/fd.h | 7 +++++-- > migration/file.c | 18 ++++++++++-------- > migration/file.h | 5 +++-- > migration/rdma.c | 11 +++++------ > migration/rdma.h | 4 ++-- > 9 files changed, 64 insertions(+), 35 deletions(-) > > diff --git a/migration/channel.c b/migration/channel.c > index 6bb2077274..a8516837cf 100644 > --- a/migration/channel.c > +++ b/migration/channel.c > @@ -37,26 +37,40 @@ > void migration_connect_outgoing(MigrationState *s, MigrationAddress *addr, > Error **errp) > { > + g_autoptr(QIOChannel) ioc = NULL; > + > if (addr->transport == MIGRATION_ADDRESS_TYPE_SOCKET) { > SocketAddress *saddr = &addr->u.socket; > if (saddr->type == SOCKET_ADDRESS_TYPE_INET || > saddr->type == SOCKET_ADDRESS_TYPE_UNIX || > saddr->type == SOCKET_ADDRESS_TYPE_VSOCK) { > socket_connect_outgoing(s, saddr, errp); > + /* > + * async: after the socket is connected, calls > + * migration_channel_connect_outgoing() directly. > + */ > + return; > + > } else if (saddr->type == SOCKET_ADDRESS_TYPE_FD) { > - fd_connect_outgoing(s, saddr->u.fd.str, errp); > + ioc = fd_connect_outgoing(s, saddr->u.fd.str, errp); > } > #ifdef CONFIG_RDMA > } else if (addr->transport == MIGRATION_ADDRESS_TYPE_RDMA) { > - rdma_connect_outgoing(s, &addr->u.rdma, errp); > + ioc = rdma_connect_outgoing(s, &addr->u.rdma, errp); > #endif > } else if (addr->transport == MIGRATION_ADDRESS_TYPE_EXEC) { > - exec_connect_outgoing(s, addr->u.exec.args, errp); > + ioc = exec_connect_outgoing(s, addr->u.exec.args, errp); > } else if (addr->transport == MIGRATION_ADDRESS_TYPE_FILE) { > - file_connect_outgoing(s, &addr->u.file, errp); > + ioc = file_connect_outgoing(s, &addr->u.file, errp); > } else { > error_setg(errp, "uri is not a valid migration protocol"); > } > + > + if (ioc) { > + migration_channel_connect_outgoing(s, ioc); > + } > + > + return; > } > > void migration_connect_incoming(MigrationAddress *addr, Error **errp) > @@ -81,6 +95,12 @@ void migration_connect_incoming(MigrationAddress *addr, > Error **errp) > } else { > error_setg(errp, "unknown migration protocol"); > } > + > + /* > + * async: the above routines all wait for the incoming connection > + * and call back to migration_channel_process_incoming() to start > + * the migration. > + */ > } > > bool migration_has_main_and_multifd_channels(void) > diff --git a/migration/exec.c b/migration/exec.c > index c3085e803e..a1a7ede3b4 100644 > --- a/migration/exec.c > +++ b/migration/exec.c > @@ -40,7 +40,8 @@ const char *exec_get_cmd_path(void) > } > #endif > > -void exec_connect_outgoing(MigrationState *s, strList *command, Error **errp) > +QIOChannel *exec_connect_outgoing(MigrationState *s, strList *command, > + Error **errp) > { > QIOChannel *ioc = NULL; > g_auto(GStrv) argv = strv_from_str_list(command); > @@ -50,12 +51,11 @@ void exec_connect_outgoing(MigrationState *s, strList > *command, Error **errp) > trace_migration_exec_outgoing(new_command); > ioc = QIO_CHANNEL(qio_channel_command_new_spawn(args, O_RDWR, errp)); > if (!ioc) { > - return; > + return NULL; > } > > qio_channel_set_name(ioc, "migration-exec-outgoing"); > - migration_channel_connect_outgoing(s, ioc); > - object_unref(OBJECT(ioc)); > + return ioc; > } > > static gboolean exec_accept_incoming_migration(QIOChannel *ioc, > diff --git a/migration/exec.h b/migration/exec.h > index e7e8e475ac..3e39270dce 100644 > --- a/migration/exec.h > +++ b/migration/exec.h > @@ -20,10 +20,13 @@ > #ifndef QEMU_MIGRATION_EXEC_H > #define QEMU_MIGRATION_EXEC_H > > +#include "io/channel.h" > + > #ifdef WIN32 > const char *exec_get_cmd_path(void); > #endif > void exec_connect_incoming(strList *host_port, Error **errp); > > -void exec_connect_outgoing(MigrationState *s, strList *host_port, Error > **errp); > +QIOChannel *exec_connect_outgoing(MigrationState *s, strList *host_port, > + Error **errp); > #endif > diff --git a/migration/fd.c b/migration/fd.c > index b689426ad4..bbf380d1a0 100644 > --- a/migration/fd.c > +++ b/migration/fd.c > @@ -49,12 +49,13 @@ static bool migration_fd_valid(int fd) > return false; > } > > -void fd_connect_outgoing(MigrationState *s, const char *fdname, Error **errp) > +QIOChannel *fd_connect_outgoing(MigrationState *s, const char *fdname, > + Error **errp) > { > - QIOChannel *ioc; > + QIOChannel *ioc = NULL; > int fd = monitor_get_fd(monitor_cur(), fdname, errp); > if (fd == -1) { > - return; > + goto out; > } > > if (!migration_fd_valid(fd)) { > @@ -66,12 +67,12 @@ void fd_connect_outgoing(MigrationState *s, const char > *fdname, Error **errp) > ioc = qio_channel_new_fd(fd, errp); > if (!ioc) { > close(fd); > - return; > + goto out; > } > > qio_channel_set_name(ioc, "migration-fd-outgoing"); > - migration_channel_connect_outgoing(s, ioc); > - object_unref(OBJECT(ioc)); > +out: > + return ioc; > } > > static gboolean fd_accept_incoming_migration(QIOChannel *ioc, > diff --git a/migration/fd.h b/migration/fd.h > index 7211629270..ce0b751273 100644 > --- a/migration/fd.h > +++ b/migration/fd.h > @@ -16,8 +16,11 @@ > > #ifndef QEMU_MIGRATION_FD_H > #define QEMU_MIGRATION_FD_H > + > +#include "io/channel.h" > + > void fd_connect_incoming(const char *fdname, Error **errp); > > -void fd_connect_outgoing(MigrationState *s, const char *fdname, > - Error **errp); > +QIOChannel *fd_connect_outgoing(MigrationState *s, const char *fdname, > + Error **errp); > #endif > diff --git a/migration/file.c b/migration/file.c > index b7b0fb5194..5618aced49 100644 > --- a/migration/file.c > +++ b/migration/file.c > @@ -93,36 +93,38 @@ out: > return ret; > } > > -void file_connect_outgoing(MigrationState *s, > - FileMigrationArgs *file_args, Error **errp) > +QIOChannel *file_connect_outgoing(MigrationState *s, > + FileMigrationArgs *file_args, Error **errp) > { > - g_autoptr(QIOChannelFile) fioc = NULL; > + QIOChannelFile *fioc = NULL; > g_autofree char *filename = g_strdup(file_args->filename); > uint64_t offset = file_args->offset; > - QIOChannel *ioc; > + QIOChannel *ioc = NULL; > > trace_migration_file_outgoing(filename); > > fioc = qio_channel_file_new_path(filename, O_CREAT | O_WRONLY, 0600, > errp); > if (!fioc) { > - return; > + goto out; > } > > if (ftruncate(fioc->fd, offset)) { > error_setg_errno(errp, errno, > "failed to truncate migration file to offset %" > PRIx64, > offset); > - return; > + goto out; > } > > outgoing_args.fname = g_strdup(filename); > > ioc = QIO_CHANNEL(fioc); > if (offset && qio_channel_io_seek(ioc, offset, SEEK_SET, errp) < 0) { > - return; > + ioc = NULL; > + goto out; > } > qio_channel_set_name(ioc, "migration-file-outgoing"); > - migration_channel_connect_outgoing(s, ioc); > +out: > + return ioc; > } > > static gboolean file_accept_incoming_migration(QIOChannel *ioc, > diff --git a/migration/file.h b/migration/file.h > index 9b1e874bb7..5936c64fea 100644 > --- a/migration/file.h > +++ b/migration/file.h > @@ -9,14 +9,15 @@ > #define QEMU_MIGRATION_FILE_H > > #include "qapi/qapi-types-migration.h" > +#include "io/channel.h" > #include "io/task.h" > #include "channel.h" > #include "multifd.h" > > void file_connect_incoming(FileMigrationArgs *file_args, Error **errp); > > -void file_connect_outgoing(MigrationState *s, > - FileMigrationArgs *file_args, Error **errp); > +QIOChannel *file_connect_outgoing(MigrationState *s, > + FileMigrationArgs *file_args, Error > **errp); > int file_parse_offset(char *filespec, uint64_t *offsetp, Error **errp); > void file_cleanup_outgoing_migration(void); > bool file_send_channel_create(gpointer opaque, Error **errp); > diff --git a/migration/rdma.c b/migration/rdma.c > index 582e0651d4..66bc337b6b 100644 > --- a/migration/rdma.c > +++ b/migration/rdma.c > @@ -3934,8 +3934,8 @@ err: > g_free(rdma); > } > > -void rdma_connect_outgoing(void *opaque, > - InetSocketAddress *host_port, Error **errp) > +QIOChannel *rdma_connect_outgoing(void *opaque, > + InetSocketAddress *host_port, Error **errp) > { > MigrationState *s = opaque; > RDMAContext *rdma_return_path = NULL; > @@ -3945,7 +3945,7 @@ void rdma_connect_outgoing(void *opaque, > /* Avoid ram_block_discard_disable(), cannot change during migration. */ > if (ram_block_discard_is_required()) { > error_setg(errp, "RDMA: cannot disable RAM discard"); > - return; > + return NULL; > } > > rdma = qemu_rdma_data_init(host_port, errp); > @@ -3995,12 +3995,11 @@ void rdma_connect_outgoing(void *opaque, > trace_rdma_connect_outgoing_after_rdma_connect(); > > s->rdma_migration = true; > - migration_outgoing_setup(rdma_new_output(rdma)); > - migration_start_outgoing(s); migration_channel_connect_outgoing() does two more things: - check for migrate_channel_requires_tls_upgrade() - migration_ioc_register_yank() The latter is probably fine because rdma iochannel doesn't support .shutdown() API. The former... may not be relevant to this patch, but anyway I wonder if it'll always be better to fail a QMP migrate command when RDMA is used with TLS, in migration_capabilities_and_transport_compatible(). It can be a separate small patch rather than reposting this wholeset. > - return; > + return rdma_new_output(rdma); > return_path_err: > qemu_rdma_cleanup(rdma); > err: > g_free(rdma); > g_free(rdma_return_path); > + return NULL; > } > diff --git a/migration/rdma.h b/migration/rdma.h > index 170c25cf44..8a6515f130 100644 > --- a/migration/rdma.h > +++ b/migration/rdma.h > @@ -21,8 +21,8 @@ > > #include "system/memory.h" > > -void rdma_connect_outgoing(void *opaque, InetSocketAddress *host_port, > - Error **errp); > +QIOChannel *rdma_connect_outgoing(void *opaque, InetSocketAddress *host_port, > + Error **errp); > > void rdma_connect_incoming(InetSocketAddress *host_port, Error **errp); > > -- > 2.51.0 > -- Peter Xu
