Re: [PATCH v5 07/12] net: stream: add unix socket

[Markus Armbruster <armbru@redhat.com>]

Laurent Vivier <lvivier@redhat.com> writes:

> On 30/06/2022 11:28, Dr. David Alan Gilbert wrote:
>> * Laurent Vivier (lvivier@redhat.com) wrote:
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>> Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
>>> ---
>>>   net/stream.c    | 106 +++++++++++++++++++++++++++++++++++++++++++++---
>>>   qemu-options.hx |   1 +
>>>   2 files changed, 102 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/net/stream.c b/net/stream.c
>>> index e8afbaca50b6..1a2bc237707e 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);
>>> +        snprintf(s->nc.info_str, sizeof(s->nc.info_str),
>>> +                 "connect from %s", saddr_un.sun_path);
>>> +        break;
>>> +    }
>>> +    default:
>>> +        g_assert_not_reached();
>>> +    }
>>>   }
>>>   
>>>   static int net_stream_server_init(NetClientState *peer,
>>> @@ -295,6 +313,42 @@ static int net_stream_server_init(NetClientState *peer,
>>>           }
>>>           break;
>>>       }
>>> +    case SOCKET_ADDRESS_TYPE_UNIX: {
>>> +        struct sockaddr_un saddr_un;
>>> +
>>> +        ret = unlink(addr->u.q_unix.path);
>>> +        if (ret < 0 && errno != ENOENT) {
>>> +            error_setg_errno(errp, errno, "failed to unlink socket %s",
>>> +                             addr->u.q_unix.path);
>>> +            return -1;
>>> +        }
>>> +
>>> +        saddr_un.sun_family = PF_UNIX;
>>> +        ret = snprintf(saddr_un.sun_path, sizeof(saddr_un.sun_path), "%s",
>>> +                       addr->u.q_unix.path);
>>> +        if (ret < 0 || ret >= sizeof(saddr_un.sun_path)) {
>>> +            error_setg(errp, "UNIX socket path '%s' is too long",
>>> +                       addr->u.q_unix.path);
>>> +            error_append_hint(errp, "Path must be less than %zu bytes\n",
>>> +                              sizeof(saddr_un.sun_path));
>> 
>> return here?

If you don't, and ...

>>> +
>>> +        fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
>>> +        if (fd < 0) {

... we then get here, we trip error_setv()'s assertion "no error has
been set".

>>> +            error_setg_errno(errp, errno, "can't create stream socket");
>>> +            return -1;
>>> +        }
>>> +        qemu_socket_set_nonblock(fd);
>>> +
>>> +        ret = bind(fd, (struct sockaddr *)&saddr_un, sizeof(saddr_un));
>>> +        if (ret < 0) {
>>> +            error_setg_errno(errp, errno, "can't create socket with path: %s",
>>> +                             saddr_un.sun_path);
>>> +            closesocket(fd);
>>> +            return -1;
>>> +        }
>>> +        break;
>>> +    }
>>>       case SOCKET_ADDRESS_TYPE_FD:
>>>           fd = monitor_fd_param(monitor_cur(), addr->u.fd.str, errp);
>>>           if (fd == -1) {
>>> @@ -380,6 +434,48 @@ static int net_stream_client_init(NetClientState *peer,
>>>                                      ntohs(saddr_in.sin_port));
>>>           break;
>>>       }
>>> +    case SOCKET_ADDRESS_TYPE_UNIX: {
>>> +        struct sockaddr_un saddr_un;
>>> +
>>> +        saddr_un.sun_family = PF_UNIX;
>>> +        ret = snprintf(saddr_un.sun_path, sizeof(saddr_un.sun_path), "%s",
>>> +                       addr->u.q_unix.path);
>>> +        if (ret < 0 || ret >= sizeof(saddr_un.sun_path)) {
>>> +            error_setg(errp, "UNIX socket path '%s' is too long",
>>> +                       addr->u.q_unix.path);
>>> +            error_append_hint(errp, "Path must be less than %zu bytes\n",
>>> +                              sizeof(saddr_un.sun_path));
>> 
>> and here

Likewise.

[...]