The migrate_uri_parse function is responsible for converting the URI
string into a MigrationChannel for consumption by the rest of the
code. Move it to channel.c and add a wrapper that calls both URI and
channels parsing.
While here, add some words about the memory management of the
MigrationChannel object, which is slightly different from
migrate_uri_parse() and migrate_channels_parse(). The former takes a
string and has to allocate a MigrationChannel, the latter takes a QAPI
object that is managed by the QAPI code.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/channel.c | 97 +++++++++++++++++++++++++++++++++++++++++--
migration/channel.h | 9 ++--
migration/migration.c | 95 ++----------------------------------------
3 files changed, 101 insertions(+), 100 deletions(-)
diff --git a/migration/channel.c b/migration/channel.c
index 8b43c3d983..d30e29c9b3 100644
--- a/migration/channel.c
+++ b/migration/channel.c
@@ -40,6 +40,12 @@ bool migration_connect(MigrationAddress *addr, bool out, Error **errp)
SocketAddress *saddr;
ERRP_GUARD();
+ /*
+ * This is reached from a QMP command, the transport code below
+ * must copy the relevant parts of 'addr' before this function
+ * returns because the QAPI code will free it.
+ */
+
switch (addr->transport) {
case MIGRATION_ADDRESS_TYPE_SOCKET:
saddr = &addr->u.socket;
@@ -318,10 +324,10 @@ int migration_channel_read_peek(QIOChannel *ioc,
return 0;
}
-bool migrate_channels_parse(MigrationChannelList *channels,
- MigrationChannel **main_channelp,
- MigrationChannel **cpr_channelp,
- Error **errp)
+static bool migrate_channels_parse(MigrationChannelList *channels,
+ MigrationChannel **main_channelp,
+ MigrationChannel **cpr_channelp,
+ Error **errp)
{
MigrationChannel *channelv[MIGRATION_CHANNEL_TYPE__MAX] = { NULL };
bool single_channel;
@@ -353,6 +359,15 @@ bool migrate_channels_parse(MigrationChannelList *channels,
}
}
+ /*
+ * These don't technically need to be cloned because they come
+ * from a QAPI object ('channels'). The top-level caller
+ * (qmp_migrate) needs to copy the necessary information before
+ * returning from the QMP command. Cloning here is just to keep
+ * the interface consistent with migrate_uri_parse() that _does
+ * not_ take a QAPI object and instead allocates and transfers
+ * ownership of a MigrationChannel to qmp_migrate.
+ */
if (cpr_channelp) {
*cpr_channelp = QAPI_CLONE(MigrationChannel,
channelv[MIGRATION_CHANNEL_TYPE_CPR]);
@@ -368,3 +383,77 @@ bool migrate_channels_parse(MigrationChannelList *channels,
return true;
}
+
+bool migrate_uri_parse(const char *uri, MigrationChannel **channel,
+ Error **errp)
+{
+ g_autoptr(MigrationChannel) val = g_new0(MigrationChannel, 1);
+ g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);
+ InetSocketAddress *isock = &addr->u.rdma;
+ strList **tail = &addr->u.exec.args;
+
+ if (strstart(uri, "exec:", NULL)) {
+ addr->transport = MIGRATION_ADDRESS_TYPE_EXEC;
+#ifdef WIN32
+ QAPI_LIST_APPEND(tail, g_strdup(exec_get_cmd_path()));
+ QAPI_LIST_APPEND(tail, g_strdup("/c"));
+#else
+ QAPI_LIST_APPEND(tail, g_strdup("/bin/sh"));
+ QAPI_LIST_APPEND(tail, g_strdup("-c"));
+#endif
+ QAPI_LIST_APPEND(tail, g_strdup(uri + strlen("exec:")));
+ } else if (strstart(uri, "rdma:", NULL)) {
+ if (inet_parse(isock, uri + strlen("rdma:"), errp)) {
+ qapi_free_InetSocketAddress(isock);
+ return false;
+ }
+ addr->transport = MIGRATION_ADDRESS_TYPE_RDMA;
+ } else if (strstart(uri, "tcp:", NULL) ||
+ strstart(uri, "unix:", NULL) ||
+ strstart(uri, "vsock:", NULL) ||
+ strstart(uri, "fd:", NULL)) {
+ addr->transport = MIGRATION_ADDRESS_TYPE_SOCKET;
+ SocketAddress *saddr = socket_parse(uri, errp);
+ if (!saddr) {
+ return false;
+ }
+ addr->u.socket.type = saddr->type;
+ addr->u.socket.u = saddr->u;
+ /* Don't free the objects inside; their ownership moved to "addr" */
+ g_free(saddr);
+ } else if (strstart(uri, "file:", NULL)) {
+ addr->transport = MIGRATION_ADDRESS_TYPE_FILE;
+ addr->u.file.filename = g_strdup(uri + strlen("file:"));
+ if (file_parse_offset(addr->u.file.filename, &addr->u.file.offset,
+ errp)) {
+ return false;
+ }
+ } else {
+ error_setg(errp, "unknown migration protocol: %s", uri);
+ return false;
+ }
+
+ val->channel_type = MIGRATION_CHANNEL_TYPE_MAIN;
+ val->addr = g_steal_pointer(&addr);
+ *channel = g_steal_pointer(&val);
+ return true;
+}
+
+bool migration_channel_parse_input(const char *uri,
+ MigrationChannelList *channels,
+ MigrationChannel **main_channelp,
+ MigrationChannel **cpr_channelp,
+ Error **errp)
+{
+ if (!uri == !channels) {
+ error_setg(errp, "need either 'uri' or 'channels' argument");
+ return false;
+ }
+
+ if (channels) {
+ return migrate_channels_parse(channels, main_channelp, cpr_channelp,
+ errp);
+ } else {
+ return migrate_uri_parse(uri, main_channelp, errp);
+ }
+}
diff --git a/migration/channel.h b/migration/channel.h
index b3276550b7..3724b0493a 100644
--- a/migration/channel.h
+++ b/migration/channel.h
@@ -52,8 +52,9 @@ static inline bool migration_connect_incoming(MigrationAddress *addr,
return migration_connect(addr, false, errp);
}
-bool migrate_channels_parse(MigrationChannelList *channels,
- MigrationChannel **main_channelp,
- MigrationChannel **cpr_channelp,
- Error **errp);
+bool migration_channel_parse_input(const char *uri,
+ MigrationChannelList *channels,
+ MigrationChannel **main_channelp,
+ MigrationChannel **cpr_channelp,
+ Error **errp);
#endif
diff --git a/migration/migration.c b/migration/migration.c
index 6064f1e5ea..15d8459a81 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -15,7 +15,6 @@
#include "qemu/osdep.h"
#include "qemu/ctype.h"
-#include "qemu/cutils.h"
#include "qemu/error-report.h"
#include "qemu/main-loop.h"
#include "migration/blocker.h"
@@ -659,61 +658,6 @@ bool migrate_is_uri(const char *uri)
return *uri == ':';
}
-bool migrate_uri_parse(const char *uri, MigrationChannel **channel,
- Error **errp)
-{
- g_autoptr(MigrationChannel) val = g_new0(MigrationChannel, 1);
- g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);
- InetSocketAddress *isock = &addr->u.rdma;
- strList **tail = &addr->u.exec.args;
-
- if (strstart(uri, "exec:", NULL)) {
- addr->transport = MIGRATION_ADDRESS_TYPE_EXEC;
-#ifdef WIN32
- QAPI_LIST_APPEND(tail, g_strdup(exec_get_cmd_path()));
- QAPI_LIST_APPEND(tail, g_strdup("/c"));
-#else
- QAPI_LIST_APPEND(tail, g_strdup("/bin/sh"));
- QAPI_LIST_APPEND(tail, g_strdup("-c"));
-#endif
- QAPI_LIST_APPEND(tail, g_strdup(uri + strlen("exec:")));
- } else if (strstart(uri, "rdma:", NULL)) {
- if (inet_parse(isock, uri + strlen("rdma:"), errp)) {
- qapi_free_InetSocketAddress(isock);
- return false;
- }
- addr->transport = MIGRATION_ADDRESS_TYPE_RDMA;
- } else if (strstart(uri, "tcp:", NULL) ||
- strstart(uri, "unix:", NULL) ||
- strstart(uri, "vsock:", NULL) ||
- strstart(uri, "fd:", NULL)) {
- addr->transport = MIGRATION_ADDRESS_TYPE_SOCKET;
- SocketAddress *saddr = socket_parse(uri, errp);
- if (!saddr) {
- return false;
- }
- addr->u.socket.type = saddr->type;
- addr->u.socket.u = saddr->u;
- /* Don't free the objects inside; their ownership moved to "addr" */
- g_free(saddr);
- } else if (strstart(uri, "file:", NULL)) {
- addr->transport = MIGRATION_ADDRESS_TYPE_FILE;
- addr->u.file.filename = g_strdup(uri + strlen("file:"));
- if (file_parse_offset(addr->u.file.filename, &addr->u.file.offset,
- errp)) {
- return false;
- }
- } else {
- error_setg(errp, "unknown migration protocol: %s", uri);
- return false;
- }
-
- val->channel_type = MIGRATION_CHANNEL_TYPE_MAIN;
- val->addr = g_steal_pointer(&addr);
- *channel = g_steal_pointer(&val);
- return true;
-}
-
static bool
migration_incoming_state_setup(MigrationIncomingState *mis, Error **errp)
{
@@ -744,27 +688,10 @@ static void qemu_setup_incoming_migration(const char *uri, bool has_channels,
g_autoptr(MigrationChannel) main_ch = NULL;
MigrationIncomingState *mis = migration_incoming_get_current();
- /*
- * Having preliminary checks for uri and channel
- */
- if (!uri == !channels) {
- error_setg(errp, "need either 'uri' or 'channels' argument");
+ if (!migration_channel_parse_input(uri, channels, &main_ch, NULL, errp)) {
return;
}
- if (channels) {
- if (!migrate_channels_parse(channels, &main_ch, NULL, errp)) {
- return;
- }
- }
-
- if (uri) {
- /* caller uses the old URI syntax */
- if (!migrate_uri_parse(uri, &main_ch, errp)) {
- return;
- }
- }
-
/* transport mechanism not suitable for migration? */
if (!migration_transport_compatible(main_ch->addr, errp)) {
return;
@@ -2124,27 +2051,11 @@ void qmp_migrate(const char *uri, bool has_channels,
g_autoptr(MigrationChannel) main_ch = NULL;
g_autoptr(MigrationChannel) cpr_ch = NULL;
- /*
- * Having preliminary checks for uri and channel
- */
- if (!uri == !channels) {
- error_setg(errp, "need either 'uri' or 'channels' argument");
+ if (!migration_channel_parse_input(uri, channels, &main_ch, &cpr_ch,
+ errp)) {
return;
}
- if (channels) {
- if (!migrate_channels_parse(channels, &main_ch, &cpr_ch, errp)) {
- return;
- }
- }
-
- if (uri) {
- /* caller uses the old URI syntax */
- if (!migrate_uri_parse(uri, &main_ch, errp)) {
- return;
- }
- }
-
/* transport mechanism not suitable for migration? */
if (!migration_transport_compatible(main_ch->addr, errp)) {
return;
--
2.51.0On Fri, Dec 26, 2025 at 06:19:26PM -0300, Fabiano Rosas wrote:
> The migrate_uri_parse function is responsible for converting the URI
> string into a MigrationChannel for consumption by the rest of the
> code. Move it to channel.c and add a wrapper that calls both URI and
> channels parsing.
>
> While here, add some words about the memory management of the
> MigrationChannel object, which is slightly different from
> migrate_uri_parse() and migrate_channels_parse(). The former takes a
> string and has to allocate a MigrationChannel, the latter takes a QAPI
> object that is managed by the QAPI code.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
There're some comments added into migration_connect(), that I still don't
think I fully agree upon.. but the rest looks all good to me.
That's probably to be discussed separately anyway, I think you can take
mine here:
Reviewed-by: Peter Xu <peterx@redhat.com>
> ---
> migration/channel.c | 97 +++++++++++++++++++++++++++++++++++++++++--
> migration/channel.h | 9 ++--
> migration/migration.c | 95 ++----------------------------------------
> 3 files changed, 101 insertions(+), 100 deletions(-)
>
> diff --git a/migration/channel.c b/migration/channel.c
> index 8b43c3d983..d30e29c9b3 100644
> --- a/migration/channel.c
> +++ b/migration/channel.c
> @@ -40,6 +40,12 @@ bool migration_connect(MigrationAddress *addr, bool out, Error **errp)
> SocketAddress *saddr;
> ERRP_GUARD();
>
> + /*
> + * This is reached from a QMP command, the transport code below
> + * must copy the relevant parts of 'addr' before this function
> + * returns because the QAPI code will free it.
> + */
> +
> switch (addr->transport) {
> case MIGRATION_ADDRESS_TYPE_SOCKET:
> saddr = &addr->u.socket;
> @@ -318,10 +324,10 @@ int migration_channel_read_peek(QIOChannel *ioc,
> return 0;
> }
>
> -bool migrate_channels_parse(MigrationChannelList *channels,
> - MigrationChannel **main_channelp,
> - MigrationChannel **cpr_channelp,
> - Error **errp)
> +static bool migrate_channels_parse(MigrationChannelList *channels,
> + MigrationChannel **main_channelp,
> + MigrationChannel **cpr_channelp,
> + Error **errp)
> {
> MigrationChannel *channelv[MIGRATION_CHANNEL_TYPE__MAX] = { NULL };
> bool single_channel;
> @@ -353,6 +359,15 @@ bool migrate_channels_parse(MigrationChannelList *channels,
> }
> }
>
> + /*
> + * These don't technically need to be cloned because they come
> + * from a QAPI object ('channels'). The top-level caller
> + * (qmp_migrate) needs to copy the necessary information before
> + * returning from the QMP command. Cloning here is just to keep
> + * the interface consistent with migrate_uri_parse() that _does
> + * not_ take a QAPI object and instead allocates and transfers
> + * ownership of a MigrationChannel to qmp_migrate.
> + */
> if (cpr_channelp) {
> *cpr_channelp = QAPI_CLONE(MigrationChannel,
> channelv[MIGRATION_CHANNEL_TYPE_CPR]);
> @@ -368,3 +383,77 @@ bool migrate_channels_parse(MigrationChannelList *channels,
>
> return true;
> }
> +
> +bool migrate_uri_parse(const char *uri, MigrationChannel **channel,
> + Error **errp)
> +{
> + g_autoptr(MigrationChannel) val = g_new0(MigrationChannel, 1);
> + g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);
> + InetSocketAddress *isock = &addr->u.rdma;
> + strList **tail = &addr->u.exec.args;
> +
> + if (strstart(uri, "exec:", NULL)) {
> + addr->transport = MIGRATION_ADDRESS_TYPE_EXEC;
> +#ifdef WIN32
> + QAPI_LIST_APPEND(tail, g_strdup(exec_get_cmd_path()));
> + QAPI_LIST_APPEND(tail, g_strdup("/c"));
> +#else
> + QAPI_LIST_APPEND(tail, g_strdup("/bin/sh"));
> + QAPI_LIST_APPEND(tail, g_strdup("-c"));
> +#endif
> + QAPI_LIST_APPEND(tail, g_strdup(uri + strlen("exec:")));
> + } else if (strstart(uri, "rdma:", NULL)) {
> + if (inet_parse(isock, uri + strlen("rdma:"), errp)) {
> + qapi_free_InetSocketAddress(isock);
> + return false;
> + }
> + addr->transport = MIGRATION_ADDRESS_TYPE_RDMA;
> + } else if (strstart(uri, "tcp:", NULL) ||
> + strstart(uri, "unix:", NULL) ||
> + strstart(uri, "vsock:", NULL) ||
> + strstart(uri, "fd:", NULL)) {
> + addr->transport = MIGRATION_ADDRESS_TYPE_SOCKET;
> + SocketAddress *saddr = socket_parse(uri, errp);
> + if (!saddr) {
> + return false;
> + }
> + addr->u.socket.type = saddr->type;
> + addr->u.socket.u = saddr->u;
> + /* Don't free the objects inside; their ownership moved to "addr" */
> + g_free(saddr);
> + } else if (strstart(uri, "file:", NULL)) {
> + addr->transport = MIGRATION_ADDRESS_TYPE_FILE;
> + addr->u.file.filename = g_strdup(uri + strlen("file:"));
> + if (file_parse_offset(addr->u.file.filename, &addr->u.file.offset,
> + errp)) {
> + return false;
> + }
> + } else {
> + error_setg(errp, "unknown migration protocol: %s", uri);
> + return false;
> + }
> +
> + val->channel_type = MIGRATION_CHANNEL_TYPE_MAIN;
> + val->addr = g_steal_pointer(&addr);
> + *channel = g_steal_pointer(&val);
> + return true;
> +}
> +
> +bool migration_channel_parse_input(const char *uri,
> + MigrationChannelList *channels,
> + MigrationChannel **main_channelp,
> + MigrationChannel **cpr_channelp,
> + Error **errp)
> +{
> + if (!uri == !channels) {
> + error_setg(errp, "need either 'uri' or 'channels' argument");
> + return false;
> + }
> +
> + if (channels) {
> + return migrate_channels_parse(channels, main_channelp, cpr_channelp,
> + errp);
> + } else {
> + return migrate_uri_parse(uri, main_channelp, errp);
> + }
> +}
> diff --git a/migration/channel.h b/migration/channel.h
> index b3276550b7..3724b0493a 100644
> --- a/migration/channel.h
> +++ b/migration/channel.h
> @@ -52,8 +52,9 @@ static inline bool migration_connect_incoming(MigrationAddress *addr,
> return migration_connect(addr, false, errp);
> }
>
> -bool migrate_channels_parse(MigrationChannelList *channels,
> - MigrationChannel **main_channelp,
> - MigrationChannel **cpr_channelp,
> - Error **errp);
> +bool migration_channel_parse_input(const char *uri,
> + MigrationChannelList *channels,
> + MigrationChannel **main_channelp,
> + MigrationChannel **cpr_channelp,
> + Error **errp);
> #endif
> diff --git a/migration/migration.c b/migration/migration.c
> index 6064f1e5ea..15d8459a81 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -15,7 +15,6 @@
>
> #include "qemu/osdep.h"
> #include "qemu/ctype.h"
> -#include "qemu/cutils.h"
> #include "qemu/error-report.h"
> #include "qemu/main-loop.h"
> #include "migration/blocker.h"
> @@ -659,61 +658,6 @@ bool migrate_is_uri(const char *uri)
> return *uri == ':';
> }
>
> -bool migrate_uri_parse(const char *uri, MigrationChannel **channel,
> - Error **errp)
> -{
> - g_autoptr(MigrationChannel) val = g_new0(MigrationChannel, 1);
> - g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);
> - InetSocketAddress *isock = &addr->u.rdma;
> - strList **tail = &addr->u.exec.args;
> -
> - if (strstart(uri, "exec:", NULL)) {
> - addr->transport = MIGRATION_ADDRESS_TYPE_EXEC;
> -#ifdef WIN32
> - QAPI_LIST_APPEND(tail, g_strdup(exec_get_cmd_path()));
> - QAPI_LIST_APPEND(tail, g_strdup("/c"));
> -#else
> - QAPI_LIST_APPEND(tail, g_strdup("/bin/sh"));
> - QAPI_LIST_APPEND(tail, g_strdup("-c"));
> -#endif
> - QAPI_LIST_APPEND(tail, g_strdup(uri + strlen("exec:")));
> - } else if (strstart(uri, "rdma:", NULL)) {
> - if (inet_parse(isock, uri + strlen("rdma:"), errp)) {
> - qapi_free_InetSocketAddress(isock);
> - return false;
> - }
> - addr->transport = MIGRATION_ADDRESS_TYPE_RDMA;
> - } else if (strstart(uri, "tcp:", NULL) ||
> - strstart(uri, "unix:", NULL) ||
> - strstart(uri, "vsock:", NULL) ||
> - strstart(uri, "fd:", NULL)) {
> - addr->transport = MIGRATION_ADDRESS_TYPE_SOCKET;
> - SocketAddress *saddr = socket_parse(uri, errp);
> - if (!saddr) {
> - return false;
> - }
> - addr->u.socket.type = saddr->type;
> - addr->u.socket.u = saddr->u;
> - /* Don't free the objects inside; their ownership moved to "addr" */
> - g_free(saddr);
> - } else if (strstart(uri, "file:", NULL)) {
> - addr->transport = MIGRATION_ADDRESS_TYPE_FILE;
> - addr->u.file.filename = g_strdup(uri + strlen("file:"));
> - if (file_parse_offset(addr->u.file.filename, &addr->u.file.offset,
> - errp)) {
> - return false;
> - }
> - } else {
> - error_setg(errp, "unknown migration protocol: %s", uri);
> - return false;
> - }
> -
> - val->channel_type = MIGRATION_CHANNEL_TYPE_MAIN;
> - val->addr = g_steal_pointer(&addr);
> - *channel = g_steal_pointer(&val);
> - return true;
> -}
> -
> static bool
> migration_incoming_state_setup(MigrationIncomingState *mis, Error **errp)
> {
> @@ -744,27 +688,10 @@ static void qemu_setup_incoming_migration(const char *uri, bool has_channels,
> g_autoptr(MigrationChannel) main_ch = NULL;
> MigrationIncomingState *mis = migration_incoming_get_current();
>
> - /*
> - * Having preliminary checks for uri and channel
> - */
> - if (!uri == !channels) {
> - error_setg(errp, "need either 'uri' or 'channels' argument");
> + if (!migration_channel_parse_input(uri, channels, &main_ch, NULL, errp)) {
> return;
> }
>
> - if (channels) {
> - if (!migrate_channels_parse(channels, &main_ch, NULL, errp)) {
> - return;
> - }
> - }
> -
> - if (uri) {
> - /* caller uses the old URI syntax */
> - if (!migrate_uri_parse(uri, &main_ch, errp)) {
> - return;
> - }
> - }
> -
> /* transport mechanism not suitable for migration? */
> if (!migration_transport_compatible(main_ch->addr, errp)) {
> return;
> @@ -2124,27 +2051,11 @@ void qmp_migrate(const char *uri, bool has_channels,
> g_autoptr(MigrationChannel) main_ch = NULL;
> g_autoptr(MigrationChannel) cpr_ch = NULL;
>
> - /*
> - * Having preliminary checks for uri and channel
> - */
> - if (!uri == !channels) {
> - error_setg(errp, "need either 'uri' or 'channels' argument");
> + if (!migration_channel_parse_input(uri, channels, &main_ch, &cpr_ch,
> + errp)) {
> return;
> }
>
> - if (channels) {
> - if (!migrate_channels_parse(channels, &main_ch, &cpr_ch, errp)) {
> - return;
> - }
> - }
> -
> - if (uri) {
> - /* caller uses the old URI syntax */
> - if (!migrate_uri_parse(uri, &main_ch, errp)) {
> - return;
> - }
> - }
> -
> /* transport mechanism not suitable for migration? */
> if (!migration_transport_compatible(main_ch->addr, errp)) {
> return;
> --
> 2.51.0
>
--
Peter Xu
Peter Xu <peterx@redhat.com> writes:
> On Fri, Dec 26, 2025 at 06:19:26PM -0300, Fabiano Rosas wrote:
>> The migrate_uri_parse function is responsible for converting the URI
>> string into a MigrationChannel for consumption by the rest of the
>> code. Move it to channel.c and add a wrapper that calls both URI and
>> channels parsing.
>>
>> While here, add some words about the memory management of the
>> MigrationChannel object, which is slightly different from
>> migrate_uri_parse() and migrate_channels_parse(). The former takes a
>> string and has to allocate a MigrationChannel, the latter takes a QAPI
>> object that is managed by the QAPI code.
>>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>
> There're some comments added into migration_connect(), that I still don't
> think I fully agree upon.. but the rest looks all good to me.
>
You're talking about the comments not being at the right place? I can
duplicate them in migration_connect_outgoing|incoming.
> That's probably to be discussed separately anyway, I think you can take
> mine here:
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
>
>> ---
>> migration/channel.c | 97 +++++++++++++++++++++++++++++++++++++++++--
>> migration/channel.h | 9 ++--
>> migration/migration.c | 95 ++----------------------------------------
>> 3 files changed, 101 insertions(+), 100 deletions(-)
>>
>> diff --git a/migration/channel.c b/migration/channel.c
>> index 8b43c3d983..d30e29c9b3 100644
>> --- a/migration/channel.c
>> +++ b/migration/channel.c
>> @@ -40,6 +40,12 @@ bool migration_connect(MigrationAddress *addr, bool out, Error **errp)
>> SocketAddress *saddr;
>> ERRP_GUARD();
>>
>> + /*
>> + * This is reached from a QMP command, the transport code below
>> + * must copy the relevant parts of 'addr' before this function
>> + * returns because the QAPI code will free it.
>> + */
>> +
>> switch (addr->transport) {
>> case MIGRATION_ADDRESS_TYPE_SOCKET:
>> saddr = &addr->u.socket;
>> @@ -318,10 +324,10 @@ int migration_channel_read_peek(QIOChannel *ioc,
>> return 0;
>> }
>>
>> -bool migrate_channels_parse(MigrationChannelList *channels,
>> - MigrationChannel **main_channelp,
>> - MigrationChannel **cpr_channelp,
>> - Error **errp)
>> +static bool migrate_channels_parse(MigrationChannelList *channels,
>> + MigrationChannel **main_channelp,
>> + MigrationChannel **cpr_channelp,
>> + Error **errp)
>> {
>> MigrationChannel *channelv[MIGRATION_CHANNEL_TYPE__MAX] = { NULL };
>> bool single_channel;
>> @@ -353,6 +359,15 @@ bool migrate_channels_parse(MigrationChannelList *channels,
>> }
>> }
>>
>> + /*
>> + * These don't technically need to be cloned because they come
>> + * from a QAPI object ('channels'). The top-level caller
>> + * (qmp_migrate) needs to copy the necessary information before
>> + * returning from the QMP command. Cloning here is just to keep
>> + * the interface consistent with migrate_uri_parse() that _does
>> + * not_ take a QAPI object and instead allocates and transfers
>> + * ownership of a MigrationChannel to qmp_migrate.
>> + */
>> if (cpr_channelp) {
>> *cpr_channelp = QAPI_CLONE(MigrationChannel,
>> channelv[MIGRATION_CHANNEL_TYPE_CPR]);
>> @@ -368,3 +383,77 @@ bool migrate_channels_parse(MigrationChannelList *channels,
>>
>> return true;
>> }
>> +
>> +bool migrate_uri_parse(const char *uri, MigrationChannel **channel,
>> + Error **errp)
>> +{
>> + g_autoptr(MigrationChannel) val = g_new0(MigrationChannel, 1);
>> + g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);
>> + InetSocketAddress *isock = &addr->u.rdma;
>> + strList **tail = &addr->u.exec.args;
>> +
>> + if (strstart(uri, "exec:", NULL)) {
>> + addr->transport = MIGRATION_ADDRESS_TYPE_EXEC;
>> +#ifdef WIN32
>> + QAPI_LIST_APPEND(tail, g_strdup(exec_get_cmd_path()));
>> + QAPI_LIST_APPEND(tail, g_strdup("/c"));
>> +#else
>> + QAPI_LIST_APPEND(tail, g_strdup("/bin/sh"));
>> + QAPI_LIST_APPEND(tail, g_strdup("-c"));
>> +#endif
>> + QAPI_LIST_APPEND(tail, g_strdup(uri + strlen("exec:")));
>> + } else if (strstart(uri, "rdma:", NULL)) {
>> + if (inet_parse(isock, uri + strlen("rdma:"), errp)) {
>> + qapi_free_InetSocketAddress(isock);
>> + return false;
>> + }
>> + addr->transport = MIGRATION_ADDRESS_TYPE_RDMA;
>> + } else if (strstart(uri, "tcp:", NULL) ||
>> + strstart(uri, "unix:", NULL) ||
>> + strstart(uri, "vsock:", NULL) ||
>> + strstart(uri, "fd:", NULL)) {
>> + addr->transport = MIGRATION_ADDRESS_TYPE_SOCKET;
>> + SocketAddress *saddr = socket_parse(uri, errp);
>> + if (!saddr) {
>> + return false;
>> + }
>> + addr->u.socket.type = saddr->type;
>> + addr->u.socket.u = saddr->u;
>> + /* Don't free the objects inside; their ownership moved to "addr" */
>> + g_free(saddr);
>> + } else if (strstart(uri, "file:", NULL)) {
>> + addr->transport = MIGRATION_ADDRESS_TYPE_FILE;
>> + addr->u.file.filename = g_strdup(uri + strlen("file:"));
>> + if (file_parse_offset(addr->u.file.filename, &addr->u.file.offset,
>> + errp)) {
>> + return false;
>> + }
>> + } else {
>> + error_setg(errp, "unknown migration protocol: %s", uri);
>> + return false;
>> + }
>> +
>> + val->channel_type = MIGRATION_CHANNEL_TYPE_MAIN;
>> + val->addr = g_steal_pointer(&addr);
>> + *channel = g_steal_pointer(&val);
>> + return true;
>> +}
>> +
>> +bool migration_channel_parse_input(const char *uri,
>> + MigrationChannelList *channels,
>> + MigrationChannel **main_channelp,
>> + MigrationChannel **cpr_channelp,
>> + Error **errp)
>> +{
>> + if (!uri == !channels) {
>> + error_setg(errp, "need either 'uri' or 'channels' argument");
>> + return false;
>> + }
>> +
>> + if (channels) {
>> + return migrate_channels_parse(channels, main_channelp, cpr_channelp,
>> + errp);
>> + } else {
>> + return migrate_uri_parse(uri, main_channelp, errp);
>> + }
>> +}
>> diff --git a/migration/channel.h b/migration/channel.h
>> index b3276550b7..3724b0493a 100644
>> --- a/migration/channel.h
>> +++ b/migration/channel.h
>> @@ -52,8 +52,9 @@ static inline bool migration_connect_incoming(MigrationAddress *addr,
>> return migration_connect(addr, false, errp);
>> }
>>
>> -bool migrate_channels_parse(MigrationChannelList *channels,
>> - MigrationChannel **main_channelp,
>> - MigrationChannel **cpr_channelp,
>> - Error **errp);
>> +bool migration_channel_parse_input(const char *uri,
>> + MigrationChannelList *channels,
>> + MigrationChannel **main_channelp,
>> + MigrationChannel **cpr_channelp,
>> + Error **errp);
>> #endif
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 6064f1e5ea..15d8459a81 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -15,7 +15,6 @@
>>
>> #include "qemu/osdep.h"
>> #include "qemu/ctype.h"
>> -#include "qemu/cutils.h"
>> #include "qemu/error-report.h"
>> #include "qemu/main-loop.h"
>> #include "migration/blocker.h"
>> @@ -659,61 +658,6 @@ bool migrate_is_uri(const char *uri)
>> return *uri == ':';
>> }
>>
>> -bool migrate_uri_parse(const char *uri, MigrationChannel **channel,
>> - Error **errp)
>> -{
>> - g_autoptr(MigrationChannel) val = g_new0(MigrationChannel, 1);
>> - g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);
>> - InetSocketAddress *isock = &addr->u.rdma;
>> - strList **tail = &addr->u.exec.args;
>> -
>> - if (strstart(uri, "exec:", NULL)) {
>> - addr->transport = MIGRATION_ADDRESS_TYPE_EXEC;
>> -#ifdef WIN32
>> - QAPI_LIST_APPEND(tail, g_strdup(exec_get_cmd_path()));
>> - QAPI_LIST_APPEND(tail, g_strdup("/c"));
>> -#else
>> - QAPI_LIST_APPEND(tail, g_strdup("/bin/sh"));
>> - QAPI_LIST_APPEND(tail, g_strdup("-c"));
>> -#endif
>> - QAPI_LIST_APPEND(tail, g_strdup(uri + strlen("exec:")));
>> - } else if (strstart(uri, "rdma:", NULL)) {
>> - if (inet_parse(isock, uri + strlen("rdma:"), errp)) {
>> - qapi_free_InetSocketAddress(isock);
>> - return false;
>> - }
>> - addr->transport = MIGRATION_ADDRESS_TYPE_RDMA;
>> - } else if (strstart(uri, "tcp:", NULL) ||
>> - strstart(uri, "unix:", NULL) ||
>> - strstart(uri, "vsock:", NULL) ||
>> - strstart(uri, "fd:", NULL)) {
>> - addr->transport = MIGRATION_ADDRESS_TYPE_SOCKET;
>> - SocketAddress *saddr = socket_parse(uri, errp);
>> - if (!saddr) {
>> - return false;
>> - }
>> - addr->u.socket.type = saddr->type;
>> - addr->u.socket.u = saddr->u;
>> - /* Don't free the objects inside; their ownership moved to "addr" */
>> - g_free(saddr);
>> - } else if (strstart(uri, "file:", NULL)) {
>> - addr->transport = MIGRATION_ADDRESS_TYPE_FILE;
>> - addr->u.file.filename = g_strdup(uri + strlen("file:"));
>> - if (file_parse_offset(addr->u.file.filename, &addr->u.file.offset,
>> - errp)) {
>> - return false;
>> - }
>> - } else {
>> - error_setg(errp, "unknown migration protocol: %s", uri);
>> - return false;
>> - }
>> -
>> - val->channel_type = MIGRATION_CHANNEL_TYPE_MAIN;
>> - val->addr = g_steal_pointer(&addr);
>> - *channel = g_steal_pointer(&val);
>> - return true;
>> -}
>> -
>> static bool
>> migration_incoming_state_setup(MigrationIncomingState *mis, Error **errp)
>> {
>> @@ -744,27 +688,10 @@ static void qemu_setup_incoming_migration(const char *uri, bool has_channels,
>> g_autoptr(MigrationChannel) main_ch = NULL;
>> MigrationIncomingState *mis = migration_incoming_get_current();
>>
>> - /*
>> - * Having preliminary checks for uri and channel
>> - */
>> - if (!uri == !channels) {
>> - error_setg(errp, "need either 'uri' or 'channels' argument");
>> + if (!migration_channel_parse_input(uri, channels, &main_ch, NULL, errp)) {
>> return;
>> }
>>
>> - if (channels) {
>> - if (!migrate_channels_parse(channels, &main_ch, NULL, errp)) {
>> - return;
>> - }
>> - }
>> -
>> - if (uri) {
>> - /* caller uses the old URI syntax */
>> - if (!migrate_uri_parse(uri, &main_ch, errp)) {
>> - return;
>> - }
>> - }
>> -
>> /* transport mechanism not suitable for migration? */
>> if (!migration_transport_compatible(main_ch->addr, errp)) {
>> return;
>> @@ -2124,27 +2051,11 @@ void qmp_migrate(const char *uri, bool has_channels,
>> g_autoptr(MigrationChannel) main_ch = NULL;
>> g_autoptr(MigrationChannel) cpr_ch = NULL;
>>
>> - /*
>> - * Having preliminary checks for uri and channel
>> - */
>> - if (!uri == !channels) {
>> - error_setg(errp, "need either 'uri' or 'channels' argument");
>> + if (!migration_channel_parse_input(uri, channels, &main_ch, &cpr_ch,
>> + errp)) {
>> return;
>> }
>>
>> - if (channels) {
>> - if (!migrate_channels_parse(channels, &main_ch, &cpr_ch, errp)) {
>> - return;
>> - }
>> - }
>> -
>> - if (uri) {
>> - /* caller uses the old URI syntax */
>> - if (!migrate_uri_parse(uri, &main_ch, errp)) {
>> - return;
>> - }
>> - }
>> -
>> /* transport mechanism not suitable for migration? */
>> if (!migration_transport_compatible(main_ch->addr, errp)) {
>> return;
>> --
>> 2.51.0
>>
On Mon, Dec 29, 2025 at 06:22:04PM -0300, Fabiano Rosas wrote: > You're talking about the comments not being at the right place? I can > duplicate them in migration_connect_outgoing|incoming. Yep, dup it is fine, or just drop them? Normally we should assume all parameters to be freed anytime by default for a function. AFAIU that's the common case. OTOH, IMHO we may better need comments where the function can transit ownership of the memory of the parameters... while this is not the case. -- Peter Xu
© 2016 - 2026 Red Hat, Inc.