Re: [Qemu-devel] [PATCH 22/25] qemu-sockets: add socket_listen, socket_connect, socket_parse

[Paolo Bonzini]

> > These are QAPI-friendly versions of the qemu-sockets functions.
> > They support IP sockets, Unix sockets, and named file descriptors, using
> > a QAPI union to dispatch to the correct function.
> >
> > Reviewed-by: Luiz Capitulino <address@hidden>
> > Signed-off-by: Paolo Bonzini <address@hidden>
> > ---
> >  Makefile       |  2 +-
> >  qemu-sockets.c | 99
> >  ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  qemu-tool.c    |  6 ++++
> >  qemu_socket.h  |  5 +++
> >  4 file modificati, 111 inserzioni(+). 1 rimozione(-)
> >
> > diff --git a/Makefile b/Makefile
> > index a9c22bf..b8b1f3c 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -159,7 +159,7 @@ qemu-img.o: qemu-img-cmds.h
> >  
> >  tools-obj-y = $(oslib-obj-y) $(trace-obj-y) qemu-tool.o
> >  qemu-timer.o \
> >     qemu-timer-common.o main-loop.o notify.o \
> > -   iohandler.o cutils.o iov.o async.o
> > +   iohandler.o cutils.o iov.o async.o error.o
> >  tools-obj-$(CONFIG_POSIX) += compatfd.o
> >  
> >  qemu-img$(EXESUF): qemu-img.o $(tools-obj-y) $(block-obj-y)
> >  $(qapi-obj-y) \
> > diff --git a/qemu-sockets.c b/qemu-sockets.c
> > index 5946962..e8d0a3c 100644
> > --- a/qemu-sockets.c
> > +++ b/qemu-sockets.c
> > @@ -22,6 +22,7 @@
> >  #include <errno.h>
> >  #include <unistd.h>
> >  
> > +#include "monitor.h"
> >  #include "qemu_socket.h"
> >  #include "qemu-common.h" /* for qemu_isdigit */
> >  #include "main-loop.h"
> > @@ -845,6 +846,104 @@ int unix_nonblocking_connect(const char
> > *path,
> >      return sock;
> >  }
> >  
> > +SocketAddress *socket_parse(const char *str, Error **errp)
> > +{
> > +    SocketAddress *addr = NULL;
> > +
> > +    addr = g_new(SocketAddress, 1);
> > +    if (strstart(str, "unix:", NULL)) {
> > +        if (str[5] == '\0') {
> > +            error_setg(errp, "invalid Unix socket address\n");
> > +            goto fail;
> > +        } else {
> > +            addr->kind = SOCKET_ADDRESS_KIND_UNIX;
> > +            addr->q_unix = g_new(UnixSocketAddress, 1);
> > +            addr->q_unix->path = g_strdup(str + 5);
> > +        }
> > +    } else if (strstart(str, "fd:", NULL)) {
> > +        if (str[3] == '\0') {
> > +            error_setg(errp, "invalid file descriptor address\n");
> > +            goto fail;
> > +        } else {
> > +            addr->kind = SOCKET_ADDRESS_KIND_FD;
> > +            addr->fd = g_new(String, 1);
> > +            addr->fd->str = g_strdup(str + 3);
> > +        }
> > +    } else {
> > +        addr->kind = SOCKET_ADDRESS_KIND_INET;
> > +        addr->inet = g_new(IPSocketAddress, 1);
> > +        addr->inet = inet_parse(str, errp);
> > +        if (addr->inet == NULL) {
> > +            goto fail;
> > +        }
> > +    }
> > +    return addr;
> > +
> > +fail:
> > +    qapi_free_SocketAddress(addr);
> > +    return NULL;
> > +}
> 
> Yet another ad hoc parser.  Have you considered sticking to QemuOpts
> syntax?

This is legacy/HMP syntax support.  You may wonder why introduce
_more_ legacy support, and the plan there is basically to replace
all the legacy-syntax _connect and _listen calls with socket_parse + 
socket_connect/socket_listen, so that they automatically gain
Unix and FD support.  This should also allow unification
of migration-tcp.c and migration-unix.c (not sure about migration-fd.c,
it's possible that incoming migration screws up the plan there).

> Hmm, I guess it's for use with the existing ad hoc parser
> inet_parse().
> Correct?

Roughly, see above.

Paolo