Re: [PATCH v9 08/10] migration: Implement MigrateChannelList to qmp migration flow.

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, Jul 21, 2023 at 02:49:34PM +0000, Het Gala wrote:
> Integrate MigrateChannelList with all transport backends
> (socket, exec and rdma) for both src and dest migration
> endpoints for qmp migration.
> 
> For current series, limit the size of MigrateChannelList
> to single element (single interface) as runtime check.
> 
> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
> Signed-off-by: Het Gala <het.gala@nutanix.com>
> ---
>  migration/migration.c | 77 ++++++++++++++++++++++++++++---------------
>  1 file changed, 50 insertions(+), 27 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 6e0a8beaf2..acf80b3590 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -426,9 +426,10 @@ void migrate_add_address(SocketAddress *address)
>  }
>  
>  static bool migrate_uri_parse(const char *uri,
> -                              MigrationAddress **channel,
> +                              MigrationChannel **channel,
>                                Error **errp)
>  {
> +    g_autoptr(MigrationChannel) val = g_new0(MigrationChannel, 1);
>      g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);
>      SocketAddress *saddr = &addr->u.socket;
>      InetSocketAddress *isock = &addr->u.rdma;
> @@ -465,7 +466,9 @@ static bool migrate_uri_parse(const char *uri,
>          return false;
>      }
>  
> -    *channel = addr;
> +    val->channel_type = MIGRATION_CHANNEL_TYPE_MAIN;
> +    val->addr = addr;
> +    *channel = val;
>      return true;
>  }
>  
> @@ -473,7 +476,8 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
>                                            MigrationChannelList *channels,
>                                            Error **errp)
>  {
> -    g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
> +    g_autoptr(MigrationChannel) channel = g_new0(MigrationChannel, 1);
> +    g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);
>  
>      /*
>       * Having preliminary checks for uri and channel
> @@ -483,20 +487,29 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
>                     "exclusive; exactly one of the two should be present in "
>                     "'migrate-incoming' qmp command ");
>          return;
> +    } else if (channels) {
> +        /* To verify that Migrate channel list has only item */
> +        if (channels->next) {
> +            error_setg(errp, "Channel list has more than one entries");
> +            return;
> +        }
> +        channel = channels->value;
> +    } else {
> +        /* caller uses the old URI syntax */
> +        if (uri && !migrate_uri_parse(uri, &channel, errp)) {
> +            return;
> +        }
>      }
> -
> -    if (uri && !migrate_uri_parse(uri, &channel, errp)) {
> -        return;
> -    }
> +    addr = channel->addr;
>  
>      /* transport mechanism not suitable for migration? */
> -    if (!migration_channels_and_transport_compatible(channel, errp)) {
> +    if (!migration_channels_and_transport_compatible(addr, errp)) {
>          return;
>      }
>  
>      qapi_event_send_migration(MIGRATION_STATUS_SETUP);
> -    if (channel->transport == MIGRATION_ADDRESS_TYPE_SOCKET) {
> -        SocketAddress *saddr = &channel->u.socket;
> +    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) {
> @@ -505,11 +518,11 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels,
>              fd_start_incoming_migration(saddr->u.fd.str, errp);
>          }
>  #ifdef CONFIG_RDMA
> -    } else if (channel->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
> -        rdma_start_incoming_migration(&channel->u.rdma, errp);
> -#endif
> -    } else if (channel->transport == MIGRATION_ADDRESS_TYPE_EXEC) {
> -        exec_start_incoming_migration(channel->u.exec.args, errp);
> +    } else if (addr->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
> +        rdma_start_incoming_migration(&addr->u.rdma, errp);
> + #endif
> +    } else if (addr->transport == MIGRATION_ADDRESS_TYPE_EXEC) {
> +        exec_start_incoming_migration(addr->u.exec.args, errp);
>      } else {
>          error_setg(errp, "unknown migration protocol: %s", uri);
>      }
> @@ -1709,7 +1722,8 @@ void qmp_migrate(const char *uri, bool has_channels,
>      bool resume_requested;
>      Error *local_err = NULL;
>      MigrationState *s = migrate_get_current();
> -    g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
> +    g_autoptr(MigrationChannel) channel = g_new0(MigrationChannel, 1);
> +    g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);
>  
>      /*
>       * Having preliminary checks for uri and channel
> @@ -1719,14 +1733,23 @@ void qmp_migrate(const char *uri, bool has_channels,
>                     "exclusive; exactly one of the two should be present in "
>                     "'migrate' qmp command ");
>          return;
> +    } else if (channels) {
> +        /* To verify that Migrate channel list has only item */
> +        if (channels->next) {
> +            error_setg(errp, "Channel list has more than one entries");
> +            return;
> +        }
> +        channel = channels->value;
> +    } else {

} else if (uri) {
     if (!migrate_uri_parse(....))
} else {
    ...error - neither uri or has_channels ...

}
> +        /* caller uses the old URI syntax */
> +        if (uri && !migrate_uri_parse(uri, &channel, errp)) {
> +            return;
> +        }
>      }
> -
> -    if (!migrate_uri_parse(uri, &channel, errp)) {
> -        return;
> -    }
> +    addr = channel->addr;
>  
>      /* transport mechanism not suitable for migration? */
> -    if (!migration_channels_and_transport_compatible(channel, errp)) {
> +    if (!migration_channels_and_transport_compatible(addr, errp)) {
>          return;
>      }
>  
> @@ -1743,8 +1766,8 @@ void qmp_migrate(const char *uri, bool has_channels,
>          }
>      }
>  
> -    if (channel->transport == MIGRATION_ADDRESS_TYPE_SOCKET) {
> -        SocketAddress *saddr = &channel->u.socket;
> +    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) {
> @@ -1753,11 +1776,11 @@ void qmp_migrate(const char *uri, bool has_channels,
>              fd_start_outgoing_migration(s, saddr->u.fd.str, &local_err);
>          }
>  #ifdef CONFIG_RDMA
> -    } else if (channel->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
> -        rdma_start_outgoing_migration(s, &channel->u.rdma, &local_err);
> +    } else if (addr->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
> +        rdma_start_outgoing_migration(s, &addr->u.rdma, &local_err);
>  #endif
> -    } else if (channel->transport == MIGRATION_ADDRESS_TYPE_EXEC) {
> -        exec_start_outgoing_migration(s, channel->u.exec.args, &local_err);
> +    } else if (addr->transport == MIGRATION_ADDRESS_TYPE_EXEC) {
> +        exec_start_outgoing_migration(s, addr->u.exec.args, &local_err);
>      } else {
>          if (!resume_requested) {
>              yank_unregister_instance(MIGRATION_YANK_INSTANCE);
> -- 
> 2.22.3
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|