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 <farosas@suse.de>
---
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);
- 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.0On 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 <farosas@suse.de>
Reviewed-by: Peter Xu <peterx@redhat.com>
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
Peter Xu <peterx@redhat.com> writes:
> 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 <farosas@suse.de>
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
>
> 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.
Hm, let me make sure this doesn't blow up when calling yank.
>
> 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.
>
Right, we even discussed this on IRC. I don't remember now, but I hit
some issue because TLS is not really a capability. I'll take another
look. Thanks!
>> - 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
>>
© 2016 - 2026 Red Hat, Inc.