[Qemu-devel] [PATCH] sockets: remove obsolete code that updated listen address

Daniel P. Berrange posted 1 patch 6 years, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20171212111219.32601-1-berrange@redhat.com
Test checkpatch passed
Test docker passed
Test ppc passed
Test s390x passed
include/qemu/sockets.h |  2 +-
qga/channel-posix.c    |  2 +-
util/qemu-sockets.c    | 31 +++++--------------------------
3 files changed, 7 insertions(+), 28 deletions(-)
[Qemu-devel] [PATCH] sockets: remove obsolete code that updated listen address
Posted by Daniel P. Berrange 6 years, 4 months ago
When listening on unix/tcp sockets there was optional code that would update
the original SocketAddress struct with the info about the actual address that
was listened on. Since the conversion of everything to QIOChannelSocket, no
remaining caller made use of this feature. It has been replaced with the ability
to query the listen address after the fact using the function
qio_channel_socket_get_local_address. This is a better model when the input
address can result in listening on multiple distinct sockets.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 include/qemu/sockets.h |  2 +-
 qga/channel-posix.c    |  2 +-
 util/qemu-sockets.c    | 31 +++++--------------------------
 3 files changed, 7 insertions(+), 28 deletions(-)

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 4f7311b52a..8889bcb1ec 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -35,7 +35,7 @@ int inet_connect_saddr(InetSocketAddress *saddr, Error **errp);
 
 NetworkAddressFamily inet_netfamily(int family);
 
-int unix_listen(const char *path, char *ostr, int olen, Error **errp);
+int unix_listen(const char *path, Error **errp);
 int unix_connect(const char *path, Error **errp);
 
 SocketAddress *socket_parse(const char *str, Error **errp);
diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index 3f34465159..b812bf4d51 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -190,7 +190,7 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
         if (fd < 0) {
             Error *local_err = NULL;
 
-            fd = unix_listen(path, NULL, strlen(path), &local_err);
+            fd = unix_listen(path, &local_err);
             if (local_err != NULL) {
                 g_critical("%s", error_get_pretty(local_err));
                 error_free(local_err);
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index a1cf47e625..703ce8ad8e 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -199,7 +199,6 @@ static int try_bind(int socket, InetSocketAddress *saddr, struct addrinfo *e)
 
 static int inet_listen_saddr(InetSocketAddress *saddr,
                              int port_offset,
-                             bool update_addr,
                              Error **errp)
 {
     struct addrinfo ai,*res,*e;
@@ -327,15 +326,6 @@ listen_failed:
     return -1;
 
 listen_ok:
-    if (update_addr) {
-        g_free(saddr->host);
-        saddr->host = g_strdup(uaddr);
-        g_free(saddr->port);
-        saddr->port = g_strdup_printf("%d",
-                                      inet_getport(e) - port_offset);
-        saddr->has_ipv6 = saddr->ipv6 = e->ai_family == PF_INET6;
-        saddr->has_ipv4 = saddr->ipv4 = e->ai_family != PF_INET6;
-    }
     freeaddrinfo(res);
     return slisten;
 }
@@ -791,7 +781,6 @@ static int vsock_parse(VsockSocketAddress *addr, const char *str,
 #ifndef _WIN32
 
 static int unix_listen_saddr(UnixSocketAddress *saddr,
-                             bool update_addr,
                              Error **errp)
 {
     struct sockaddr_un un;
@@ -856,12 +845,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
         goto err;
     }
 
-    if (update_addr && pathbuf) {
-        g_free(saddr->path);
-        saddr->path = pathbuf;
-    } else {
-        g_free(pathbuf);
-    }
+    g_free(pathbuf);
     return sock;
 
 err:
@@ -921,7 +905,6 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
 #else
 
 static int unix_listen_saddr(UnixSocketAddress *saddr,
-                             bool update_addr,
                              Error **errp)
 {
     error_setg(errp, "unix sockets are not available on windows");
@@ -938,7 +921,7 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
 #endif
 
 /* compatibility wrapper */
-int unix_listen(const char *str, char *ostr, int olen, Error **errp)
+int unix_listen(const char *str, Error **errp)
 {
     char *path, *optstr;
     int sock, len;
@@ -958,11 +941,7 @@ int unix_listen(const char *str, char *ostr, int olen, Error **errp)
         saddr->path = g_strdup(str);
     }
 
-    sock = unix_listen_saddr(saddr, true, errp);
-
-    if (sock != -1 && ostr) {
-        snprintf(ostr, olen, "%s%s", saddr->path, optstr ? optstr : "");
-    }
+    sock = unix_listen_saddr(saddr, errp);
 
     qapi_free_UnixSocketAddress(saddr);
     return sock;
@@ -1053,11 +1032,11 @@ int socket_listen(SocketAddress *addr, Error **errp)
 
     switch (addr->type) {
     case SOCKET_ADDRESS_TYPE_INET:
-        fd = inet_listen_saddr(&addr->u.inet, 0, false, errp);
+        fd = inet_listen_saddr(&addr->u.inet, 0, errp);
         break;
 
     case SOCKET_ADDRESS_TYPE_UNIX:
-        fd = unix_listen_saddr(&addr->u.q_unix, false, errp);
+        fd = unix_listen_saddr(&addr->u.q_unix, errp);
         break;
 
     case SOCKET_ADDRESS_TYPE_FD:
-- 
2.14.3


Re: [Qemu-devel] [PATCH] sockets: remove obsolete code that updated listen address
Posted by Peter Xu 6 years, 4 months ago
On Tue, Dec 12, 2017 at 11:12:19AM +0000, Daniel P. Berrange wrote:
> When listening on unix/tcp sockets there was optional code that would update
> the original SocketAddress struct with the info about the actual address that
> was listened on. Since the conversion of everything to QIOChannelSocket, no
> remaining caller made use of this feature. It has been replaced with the ability
> to query the listen address after the fact using the function
> qio_channel_socket_get_local_address. This is a better model when the input
> address can result in listening on multiple distinct sockets.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

Re: [Qemu-devel] [PATCH] sockets: remove obsolete code that updated listen address
Posted by Paolo Bonzini 6 years, 4 months ago
On 12/12/2017 12:12, Daniel P. Berrange wrote:
> When listening on unix/tcp sockets there was optional code that would update
> the original SocketAddress struct with the info about the actual address that
> was listened on. Since the conversion of everything to QIOChannelSocket, no
> remaining caller made use of this feature. It has been replaced with the ability
> to query the listen address after the fact using the function
> qio_channel_socket_get_local_address. This is a better model when the input
> address can result in listening on multiple distinct sockets.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  include/qemu/sockets.h |  2 +-
>  qga/channel-posix.c    |  2 +-
>  util/qemu-sockets.c    | 31 +++++--------------------------
>  3 files changed, 7 insertions(+), 28 deletions(-)
> 
> diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
> index 4f7311b52a..8889bcb1ec 100644
> --- a/include/qemu/sockets.h
> +++ b/include/qemu/sockets.h
> @@ -35,7 +35,7 @@ int inet_connect_saddr(InetSocketAddress *saddr, Error **errp);
>  
>  NetworkAddressFamily inet_netfamily(int family);
>  
> -int unix_listen(const char *path, char *ostr, int olen, Error **errp);
> +int unix_listen(const char *path, Error **errp);
>  int unix_connect(const char *path, Error **errp);
>  
>  SocketAddress *socket_parse(const char *str, Error **errp);
> diff --git a/qga/channel-posix.c b/qga/channel-posix.c
> index 3f34465159..b812bf4d51 100644
> --- a/qga/channel-posix.c
> +++ b/qga/channel-posix.c
> @@ -190,7 +190,7 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
>          if (fd < 0) {
>              Error *local_err = NULL;
>  
> -            fd = unix_listen(path, NULL, strlen(path), &local_err);
> +            fd = unix_listen(path, &local_err);
>              if (local_err != NULL) {
>                  g_critical("%s", error_get_pretty(local_err));
>                  error_free(local_err);
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index a1cf47e625..703ce8ad8e 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -199,7 +199,6 @@ static int try_bind(int socket, InetSocketAddress *saddr, struct addrinfo *e)
>  
>  static int inet_listen_saddr(InetSocketAddress *saddr,
>                               int port_offset,
> -                             bool update_addr,
>                               Error **errp)
>  {
>      struct addrinfo ai,*res,*e;
> @@ -327,15 +326,6 @@ listen_failed:
>      return -1;
>  
>  listen_ok:
> -    if (update_addr) {
> -        g_free(saddr->host);
> -        saddr->host = g_strdup(uaddr);
> -        g_free(saddr->port);
> -        saddr->port = g_strdup_printf("%d",
> -                                      inet_getport(e) - port_offset);
> -        saddr->has_ipv6 = saddr->ipv6 = e->ai_family == PF_INET6;
> -        saddr->has_ipv4 = saddr->ipv4 = e->ai_family != PF_INET6;
> -    }
>      freeaddrinfo(res);
>      return slisten;
>  }
> @@ -791,7 +781,6 @@ static int vsock_parse(VsockSocketAddress *addr, const char *str,
>  #ifndef _WIN32
>  
>  static int unix_listen_saddr(UnixSocketAddress *saddr,
> -                             bool update_addr,
>                               Error **errp)
>  {
>      struct sockaddr_un un;
> @@ -856,12 +845,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
>          goto err;
>      }
>  
> -    if (update_addr && pathbuf) {
> -        g_free(saddr->path);
> -        saddr->path = pathbuf;
> -    } else {
> -        g_free(pathbuf);
> -    }
> +    g_free(pathbuf);
>      return sock;
>  
>  err:
> @@ -921,7 +905,6 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
>  #else
>  
>  static int unix_listen_saddr(UnixSocketAddress *saddr,
> -                             bool update_addr,
>                               Error **errp)
>  {
>      error_setg(errp, "unix sockets are not available on windows");
> @@ -938,7 +921,7 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
>  #endif
>  
>  /* compatibility wrapper */
> -int unix_listen(const char *str, char *ostr, int olen, Error **errp)
> +int unix_listen(const char *str, Error **errp)
>  {
>      char *path, *optstr;
>      int sock, len;
> @@ -958,11 +941,7 @@ int unix_listen(const char *str, char *ostr, int olen, Error **errp)
>          saddr->path = g_strdup(str);
>      }
>  
> -    sock = unix_listen_saddr(saddr, true, errp);
> -
> -    if (sock != -1 && ostr) {
> -        snprintf(ostr, olen, "%s%s", saddr->path, optstr ? optstr : "");
> -    }
> +    sock = unix_listen_saddr(saddr, errp);
>  
>      qapi_free_UnixSocketAddress(saddr);
>      return sock;
> @@ -1053,11 +1032,11 @@ int socket_listen(SocketAddress *addr, Error **errp)
>  
>      switch (addr->type) {
>      case SOCKET_ADDRESS_TYPE_INET:
> -        fd = inet_listen_saddr(&addr->u.inet, 0, false, errp);
> +        fd = inet_listen_saddr(&addr->u.inet, 0, errp);
>          break;
>  
>      case SOCKET_ADDRESS_TYPE_UNIX:
> -        fd = unix_listen_saddr(&addr->u.q_unix, false, errp);
> +        fd = unix_listen_saddr(&addr->u.q_unix, errp);
>          break;
>  
>      case SOCKET_ADDRESS_TYPE_FD:
> 

Queued, thanks.

Paolo