Re: [PATCH v8 07/14] net: stream: add unix socket

[Laurent Vivier]

On 9/14/22 07:16, David Gibson wrote:
On Tue, Sep 13, 2022 at 08:39:53AM +0200, Laurent Vivier wrote:
Signed-off-by: Laurent Vivier <lviv...@redhat.com>
Reviewed-by: Stefano Brivio <sbri...@redhat.com>
---
  net/stream.c    | 108 +++++++++++++++++++++++++++++++++++++++++++++---
  qapi/net.json   |   2 +-
  qemu-options.hx |   1 +
  3 files changed, 105 insertions(+), 6 deletions(-)
diff --git a/net/stream.c b/net/stream.c
index e8afbaca50b6..0f91ff20df61 100644
--- a/net/stream.c
+++ b/net/stream.c
@@ -235,7 +235,7 @@ static NetStreamState 
*net_stream_fd_init_stream(NetClientState *peer,
  static void net_stream_accept(void *opaque)
  {
      NetStreamState *s = opaque;
-    struct sockaddr_in saddr;
+    struct sockaddr_storage saddr;
      socklen_t len;
      int fd;
  
@@ -253,9 +253,27 @@ static void net_stream_accept(void *opaque)
      s->fd = fd;
      s->nc.link_down = false;
      net_stream_connect(s);
-    snprintf(s->nc.info_str, sizeof(s->nc.info_str),
-             "connection from %s:%d",
-             inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
+    switch (saddr.ss_family) {
+    case AF_INET: {
+        struct sockaddr_in *saddr_in = (struct sockaddr_in *)&saddr;
+
+        snprintf(s->nc.info_str, sizeof(s->nc.info_str),
+                 "connection from %s:%d",
+                 inet_ntoa(saddr_in->sin_addr), ntohs(saddr_in->sin_port));
+        break;
+    }
+    case AF_UNIX: {
+        struct sockaddr_un saddr_un;
+
+        len = sizeof(saddr_un);
+        getsockname(s->listen_fd, (struct sockaddr *)&saddr_un, &len);

It seems like there's an asymmetry here in that we've gotten the
connection address outside the switch for inet sockets, but we need an
additional getsockname() for unix sockets.  Am I missing something?


This is because unix socket are not symmetric.

connect() returns the peer socket address.

With inet, server is waiting on, for instance, 127.0.0.1:1234 and peer address will appear 
to be something like 127.0.0.1:33946. So info_str will be set with "127.0.0.1:33946".

In case of unix, server is waiting on, for instance, /tmp/qemu0 and there is no peer 
address as the client connect to this same file. So the sun_path is empty when it returns 
from accept(). So to set the path of the socket (sun_path) in info_str I use the 
getsockname() to retrieve the server sun_path.

Thanks,
Laurent