From: Author Het Gala <het.gala@nutanix.com>

Modified 'migrate-incoming' QAPI supports MigrateChannel parameters to prevent
multi-level encodings of uri on the destination side.

socket_start_incoming_migration() has been depricated as it's only purpose was
uri parsing.
Renamed socket_outgoing_migration_internal() as socket_start_incoming_migration().
qemu_uri_parsing() is used to populate the new struct from 'uri' string
needed for migration connection. The function will no longer be used once the
'uri' parameter is depricated, as the parameters will already be mapped into
new struct.

Suggested-by: Daniel P. Berrange <berrange@redhat.com>
Suggested-by: Manish Mishra <manish.mishra@nutanix.com>
Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
---
 migration/migration.c | 48 ++++++++++++++++++++++++++++---------------
 migration/socket.c    | 16 ++-------------
 migration/socket.h    |  2 +-
 3 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 838940fd55..c70fd0ab4f 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -520,27 +520,43 @@ static void qemu_uri_parsing(const char *uri,
     }
 }
 
-static void qemu_start_incoming_migration(const char *uri, Error **errp)
+static void qemu_start_incoming_migration(const char *uri,
+                                          MigrateChannel *channel,
+                                          Error **errp)
 {
-    const char *p = NULL;
+    MigrateAddress *addrs = g_new0(MigrateAddress, 1);
+    SocketAddress *saddr = g_new0(SocketAddress, 1);
+
+    /*
+     * motive here is just to have checks and convert uri into
+     * socketaddress struct
+     */
+    if (uri && channel) {
+        error_setg(errp, "uri and channels options should be used "
+                          "mutually exclusive");
+    } else if (uri) {
+        qemu_uri_parsing(uri, &channel, errp);
+    }
 
     migrate_protocol_allow_multi_channels(false); /* reset it anyway */
     qapi_event_send_migration(MIGRATION_STATUS_SETUP);
-    if (strstart(uri, "tcp:", &p) ||
-        strstart(uri, "unix:", NULL) ||
-        strstart(uri, "vsock:", NULL)) {
-        migrate_protocol_allow_multi_channels(true);
-        socket_start_incoming_migration(p ? p : uri, errp);
+    if (addrs->transport == MIGRATE_TRANSPORT_SOCKET) {
+        if (saddr->type == SOCKET_ADDRESS_TYPE_INET ||
+            saddr->type == SOCKET_ADDRESS_TYPE_UNIX ||
+            saddr->type == SOCKET_ADDRESS_TYPE_VSOCK) {
+            migrate_protocol_allow_multi_channels(true);
+            socket_start_incoming_migration(saddr, errp);
+        } else if (saddr->type == SOCKET_ADDRESS_TYPE_FD) {
+            fd_start_incoming_migration(saddr->u.fd.str, errp);
+        }
 #ifdef CONFIG_RDMA
-    } else if (strstart(uri, "rdma:", &p)) {
-        rdma_start_incoming_migration(p, errp);
+    } else if (addrs->transport == MIGRATE_TRANSPORT_RDMA) {
+        rdma_start_incomng_migration(addrs->u.rdma.rdma_str, errp);
 #endif
-    } else if (strstart(uri, "exec:", &p)) {
-        exec_start_incoming_migration(p, errp);
-    } else if (strstart(uri, "fd:", &p)) {
-        fd_start_incoming_migration(p, errp);
+    } else if (addrs->transport == MIGRATE_TRANSPORT_EXEC) {
+        exec_start_incoming_migration(addrs->u.exec.exec_str, errp);
     } else {
-        error_setg(errp, "unknown migration protocol: %s", uri);
+        error_setg(errp, "unknown migration protocol: %i", addrs->transport);
     }
 }
 
@@ -2256,7 +2272,7 @@ void qmp_migrate_incoming(const char *uri, MigrateChannel *channel,
         return;
     }
 
-    qemu_start_incoming_migration(uri, &local_err);
+    qemu_start_incoming_migration(uri, channel, &local_err);
 
     if (local_err) {
         yank_unregister_instance(MIGRATION_YANK_INSTANCE);
@@ -2292,7 +2308,7 @@ void qmp_migrate_recover(const char *uri, Error **errp)
      * only re-setup the migration stream and poke existing migration
      * to continue using that newly established channel.
      */
-    qemu_start_incoming_migration(uri, errp);
+    qemu_start_incoming_migration(uri, NULL, errp);
 }
 
 void qmp_migrate_pause(Error **errp)
diff --git a/migration/socket.c b/migration/socket.c
index ecf98b7e6b..3558821298 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -158,9 +158,8 @@ socket_incoming_migration_end(void *opaque)
     object_unref(OBJECT(listener));
 }
 
-static void
-socket_start_incoming_migration_internal(SocketAddress *saddr,
-                                         Error **errp)
+void socket_start_incoming_migration(SocketAddress *saddr,
+                                     Error **errp)
 {
     QIONetListener *listener = qio_net_listener_new();
     MigrationIncomingState *mis = migration_incoming_get_current();
@@ -198,14 +197,3 @@ socket_start_incoming_migration_internal(SocketAddress *saddr,
         qapi_free_SocketAddress(address);
     }
 }
-
-void socket_start_incoming_migration(const char *str, Error **errp)
-{
-    Error *err = NULL;
-    SocketAddress *saddr = socket_parse(str, &err);
-    if (!err) {
-        socket_start_incoming_migration_internal(saddr, &err);
-    }
-    qapi_free_SocketAddress(saddr);
-    error_propagate(errp, err);
-}
diff --git a/migration/socket.h b/migration/socket.h
index 95c9c166ec..4769a2bdf9 100644
--- a/migration/socket.h
+++ b/migration/socket.h
@@ -25,7 +25,7 @@ void socket_send_channel_create(QIOTaskFunc f, void *data);
 QIOChannel *socket_send_channel_create_sync(Error **errp);
 int socket_send_channel_destroy(QIOChannel *send);
 
-void socket_start_incoming_migration(const char *str, Error **errp);
+void socket_start_incoming_migration(SocketAddress *saddr, Error **errp);
 
 void socket_start_outgoing_migration(MigrationState *s, SocketAddress *saddr,
                                      Error **errp);
-- 
2.22.3

Het Gala <het.gala@nutanix.com> writes:

> From: Author Het Gala <het.gala@nutanix.com>
>
> Modified 'migrate-incoming' QAPI supports MigrateChannel parameters to prevent
> multi-level encodings of uri on the destination side.
>
> socket_start_incoming_migration() has been depricated as it's only purpose was
> uri parsing.
> Renamed socket_outgoing_migration_internal() as socket_start_incoming_migration().
> qemu_uri_parsing() is used to populate the new struct from 'uri' string
> needed for migration connection. The function will no longer be used once the
> 'uri' parameter is depricated, as the parameters will already be mapped into
> new struct.
>
> Suggested-by: Daniel P. Berrange <berrange@redhat.com>
> Suggested-by: Manish Mishra <manish.mishra@nutanix.com>
> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
> Signed-off-by: Het Gala <het.gala@nutanix.com>
> ---
>  migration/migration.c | 48 ++++++++++++++++++++++++++++---------------
>  migration/socket.c    | 16 ++-------------
>  migration/socket.h    |  2 +-
>  3 files changed, 35 insertions(+), 31 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 838940fd55..c70fd0ab4f 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -520,27 +520,43 @@ static void qemu_uri_parsing(const char *uri,
>      }
>  }
>  
> -static void qemu_start_incoming_migration(const char *uri, Error **errp)
> +static void qemu_start_incoming_migration(const char *uri,
> +                                          MigrateChannel *channel,
> +                                          Error **errp)
>  {
> -    const char *p = NULL;
> +    MigrateAddress *addrs = g_new0(MigrateAddress, 1);
> +    SocketAddress *saddr = g_new0(SocketAddress, 1);
> +
> +    /*
> +     * motive here is just to have checks and convert uri into
> +     * socketaddress struct
> +     */
> +    if (uri && channel) {
> +        error_setg(errp, "uri and channels options should be used "
> +                          "mutually exclusive");
> +    } else if (uri) {
> +        qemu_uri_parsing(uri, &channel, errp);
> +    }
>  
>      migrate_protocol_allow_multi_channels(false); /* reset it anyway */
>      qapi_event_send_migration(MIGRATION_STATUS_SETUP);
> -    if (strstart(uri, "tcp:", &p) ||
> -        strstart(uri, "unix:", NULL) ||
> -        strstart(uri, "vsock:", NULL)) {
> -        migrate_protocol_allow_multi_channels(true);
> -        socket_start_incoming_migration(p ? p : uri, errp);
> +    if (addrs->transport == MIGRATE_TRANSPORT_SOCKET) {
> +        if (saddr->type == SOCKET_ADDRESS_TYPE_INET ||
> +            saddr->type == SOCKET_ADDRESS_TYPE_UNIX ||
> +            saddr->type == SOCKET_ADDRESS_TYPE_VSOCK) {
> +            migrate_protocol_allow_multi_channels(true);
> +            socket_start_incoming_migration(saddr, errp);
> +        } else if (saddr->type == SOCKET_ADDRESS_TYPE_FD) {
> +            fd_start_incoming_migration(saddr->u.fd.str, errp);
> +        }
>  #ifdef CONFIG_RDMA
> -    } else if (strstart(uri, "rdma:", &p)) {
> -        rdma_start_incoming_migration(p, errp);
> +    } else if (addrs->transport == MIGRATE_TRANSPORT_RDMA) {
> +        rdma_start_incomng_migration(addrs->u.rdma.rdma_str, errp);


Fails to compile:

    ../migration/migration.c: In function ‘qemu_start_incoming_migration’:
    ../migration/migration.c:554:9: error: implicit declaration of function ‘rdma_start_incomng_migration’; did you mean ‘rdma_start_incoming_migration’? [-Werror=implicit-function-declaration]
      554 |         rdma_start_incomng_migration(addrs->u.rdma.rdma_str, errp);
          |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
          |         rdma_start_incoming_migration
    ../migration/migration.c:554:9: error: nested extern declaration of ‘rdma_start_incomng_migration’ [-Werror=nested-externs]

Please fix that, and also test RDMA.

>  #endif
> -    } else if (strstart(uri, "exec:", &p)) {
> -        exec_start_incoming_migration(p, errp);
> -    } else if (strstart(uri, "fd:", &p)) {
> -        fd_start_incoming_migration(p, errp);
> +    } else if (addrs->transport == MIGRATE_TRANSPORT_EXEC) {
> +        exec_start_incoming_migration(addrs->u.exec.exec_str, errp);
>      } else {
> -        error_setg(errp, "unknown migration protocol: %s", uri);
> +        error_setg(errp, "unknown migration protocol: %i", addrs->transport);
>      }
>  }
>  

[...]