Re: [PATCH 1/2] migration: unify the framework of socket-type channel

[Dr. David Alan Gilbert]

* Longpeng(Mike) (longpeng2@huawei.com) wrote:
> Currently, the only difference of tcp channel and unix channel in
> migration/socket.c is the way to build SocketAddress, but socket_parse()
> can handle these two types, so use it to instead of tcp_build_address()
> and unix_build_address().
> 
> The socket-type channel can be further unified based on the up, this
> would be helpful for us to add other socket-type channels.
> 
> Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>

Nice; this saves a chunk of code.


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  migration/migration.c | 18 ++++++-------
>  migration/socket.c    | 72 
> +++++++++++----------------------------------------
>  migration/socket.h    | 11 +++-----
>  3 files changed, 26 insertions(+), 75 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 8fe3633..3160b95 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -377,21 +377,20 @@ void migrate_add_address(SocketAddress *address)
>  
>  void qemu_start_incoming_migration(const char *uri, Error **errp)
>  {
> -    const char *p;
> +    const char *p = NULL;
>  
>      qapi_event_send_migration(MIGRATION_STATUS_SETUP);
>      if (!strcmp(uri, "defer")) {
>          deferred_incoming_migration(errp);
> -    } else if (strstart(uri, "tcp:", &p)) {
> -        tcp_start_incoming_migration(p, errp);
> +    } else if (strstart(uri, "tcp:", &p) ||
> +               strstart(uri, "unix:", NULL)) {
> +        socket_start_incoming_migration(p ? p : uri, errp);
>  #ifdef CONFIG_RDMA
>      } else if (strstart(uri, "rdma:", &p)) {
>          rdma_start_incoming_migration(p, errp);
>  #endif
>      } else if (strstart(uri, "exec:", &p)) {
>          exec_start_incoming_migration(p, errp);
> -    } else if (strstart(uri, "unix:", &p)) {
> -        unix_start_incoming_migration(p, errp);
>      } else if (strstart(uri, "fd:", &p)) {
>          fd_start_incoming_migration(p, errp);
>      } else {
> @@ -2064,7 +2063,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool 
> blk,
>  {
>      Error *local_err = NULL;
>      MigrationState *s = migrate_get_current();
> -    const char *p;
> +    const char *p = NULL;
>  
>      if (!migrate_prepare(s, has_blk && blk, has_inc && inc,
>                           has_resume && resume, errp)) {
> @@ -2072,16 +2071,15 @@ void qmp_migrate(const char *uri, bool has_blk, bool 
> blk,
>          return;
>      }
>  
> -    if (strstart(uri, "tcp:", &p)) {
> -        tcp_start_outgoing_migration(s, p, &local_err);
> +    if (strstart(uri, "tcp:", &p) ||
> +        strstart(uri, "unix:", NULL)) {
> +        socket_start_outgoing_migration(s, p ? p : uri, &local_err);
>  #ifdef CONFIG_RDMA
>      } else if (strstart(uri, "rdma:", &p)) {
>          rdma_start_outgoing_migration(s, p, &local_err);
>  #endif
>      } else if (strstart(uri, "exec:", &p)) {
>          exec_start_outgoing_migration(s, p, &local_err);
> -    } else if (strstart(uri, "unix:", &p)) {
> -        unix_start_outgoing_migration(s, p, &local_err);
>      } else if (strstart(uri, "fd:", &p)) {
>          fd_start_outgoing_migration(s, p, &local_err);
>      } else {
> diff --git a/migration/socket.c b/migration/socket.c
> index 97c9efd..6016642 100644
> --- a/migration/socket.c
> +++ b/migration/socket.c
> @@ -50,34 +50,6 @@ int socket_send_channel_destroy(QIOChannel *send)
>      return 0;
>  }
>  
> -static SocketAddress *tcp_build_address(const char *host_port, Error **errp)
> -{
> -    SocketAddress *saddr;
> -
> -    saddr = g_new0(SocketAddress, 1);
> -    saddr->type = SOCKET_ADDRESS_TYPE_INET;
> -
> -    if (inet_parse(&saddr->u.inet, host_port, errp)) {
> -        qapi_free_SocketAddress(saddr);
> -        return NULL;
> -    }
> -
> -    return saddr;
> -}
> -
> -
> -static SocketAddress *unix_build_address(const char *path)
> -{
> -    SocketAddress *saddr;
> -
> -    saddr = g_new0(SocketAddress, 1);
> -    saddr->type = SOCKET_ADDRESS_TYPE_UNIX;
> -    saddr->u.q_unix.path = g_strdup(path);
> -
> -    return saddr;
> -}
> -
> -
>  struct SocketConnectData {
>      MigrationState *s;
>      char *hostname;
> @@ -109,9 +81,10 @@ static void socket_outgoing_migration(QIOTask *task,
>      object_unref(OBJECT(sioc));
>  }
>  
> -static void socket_start_outgoing_migration(MigrationState *s,
> -                                            SocketAddress *saddr,
> -                                            Error **errp)
> +static void
> +socket_start_outgoing_migration_internal(MigrationState *s,
> +                                         SocketAddress *saddr,
> +                                         Error **errp)
>  {
>      QIOChannelSocket *sioc = qio_channel_socket_new();
>      struct SocketConnectData *data = g_new0(struct SocketConnectData, 1);
> @@ -135,27 +108,18 @@ static void 
> socket_start_outgoing_migration(MigrationState *s,
>                                       NULL);
>  }
>  
> -void tcp_start_outgoing_migration(MigrationState *s,
> -                                  const char *host_port,
> -                                  Error **errp)
> +void socket_start_outgoing_migration(MigrationState *s,
> +                                     const char *str,
> +                                     Error **errp)
>  {
>      Error *err = NULL;
> -    SocketAddress *saddr = tcp_build_address(host_port, &err);
> +    SocketAddress *saddr = socket_parse(str, &err);
>      if (!err) {
> -        socket_start_outgoing_migration(s, saddr, &err);
> +        socket_start_outgoing_migration_internal(s, saddr, &err);
>      }
>      error_propagate(errp, err);
>  }
>  
> -void unix_start_outgoing_migration(MigrationState *s,
> -                                   const char *path,
> -                                   Error **errp)
> -{
> -    SocketAddress *saddr = unix_build_address(path);
> -    socket_start_outgoing_migration(s, saddr, errp);
> -}
> -
> -
>  static void socket_accept_incoming_migration(QIONetListener *listener,
>                                               QIOChannelSocket *cioc,
>                                               gpointer opaque)
> @@ -173,8 +137,9 @@ static void 
> socket_accept_incoming_migration(QIONetListener *listener,
>  }
>  
>  
> -static void socket_start_incoming_migration(SocketAddress *saddr,
> -                                            Error **errp)
> +static void
> +socket_start_incoming_migration_internal(SocketAddress *saddr,
> +                                         Error **errp)
>  {
>      QIONetListener *listener = qio_net_listener_new();
>      size_t i;
> @@ -207,20 +172,13 @@ static void 
> socket_start_incoming_migration(SocketAddress *saddr,
>      }
>  }
>  
> -void tcp_start_incoming_migration(const char *host_port, Error **errp)
> +void socket_start_incoming_migration(const char *str, Error **errp)
>  {
>      Error *err = NULL;
> -    SocketAddress *saddr = tcp_build_address(host_port, &err);
> +    SocketAddress *saddr = socket_parse(str, &err);
>      if (!err) {
> -        socket_start_incoming_migration(saddr, &err);
> +        socket_start_incoming_migration_internal(saddr, &err);
>      }
>      qapi_free_SocketAddress(saddr);
>      error_propagate(errp, err);
>  }
> -
> -void unix_start_incoming_migration(const char *path, Error **errp)
> -{
> -    SocketAddress *saddr = unix_build_address(path);
> -    socket_start_incoming_migration(saddr, errp);
> -    qapi_free_SocketAddress(saddr);
> -}
> diff --git a/migration/socket.h b/migration/socket.h
> index 528c3b0..891dbcc 100644
> --- a/migration/socket.h
> +++ b/migration/socket.h
> @@ -23,13 +23,8 @@
>  void socket_send_channel_create(QIOTaskFunc f, void *data);
>  int socket_send_channel_destroy(QIOChannel *send);
>  
> -void tcp_start_incoming_migration(const char *host_port, Error **errp);
> +void socket_start_incoming_migration(const char *str, Error **errp);
>  
> -void tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
> -                                  Error **errp);
> -
> -void unix_start_incoming_migration(const char *path, Error **errp);
> -
> -void unix_start_outgoing_migration(MigrationState *s, const char *path,
> -                                   Error **errp);
> +void socket_start_outgoing_migration(MigrationState *s, const char *str,
> +                                     Error **errp);
>  #endif
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK