On Fri, 9 Jan 2026 at 18:16, Fabiano Rosas <[email protected]> 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.
>
> Reviewed-by: Peter Xu <[email protected]>
> Signed-off-by: Fabiano Rosas <[email protected]>
> ---
>  migration/channel.c   | 82 +++++++++++++++++++++++++++++++++++--
>  migration/channel.h   |  9 ++--
>  migration/migration.c | 95 ++-----------------------------------------
>  3 files changed, 86 insertions(+), 100 deletions(-)
>
> diff --git a/migration/channel.c b/migration/channel.c
> index 8b71b3f430..cee78532ea 100644
> --- a/migration/channel.c
> +++ b/migration/channel.c
> @@ -284,10 +284,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 = cpr_channelp ? false : true;
> @@ -325,3 +325,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 5110fb45a4..a7d0d29058 100644
> --- a/migration/channel.h
> +++ b/migration/channel.h
> @@ -43,8 +43,9 @@ void migration_connect_outgoing(MigrationState *s, 
> MigrationAddress *addr,
>                                  Error **errp);
>  void migration_connect_incoming(MigrationAddress *addr, Error **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 98c1f38e8e..52c1bb5da2 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;
> @@ -2113,27 +2040,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;
> --

* Looks okay.
Reviewed-by: Prasad Pandit <[email protected]>

Thank you.
---
  - Prasad


Reply via email to