Patchwork Patch to improve handling of server sockets

login
register
mail settings
Submitter Reinhard Max
Date May 5, 2010, 5:44 p.m.
Message ID <alpine.LNX.2.00.1005051928170.3715@nitsch.suse.de>
Download mbox | patch
Permalink /patch/51750/
State New
Headers show

Comments

Reinhard Max - May 5, 2010, 5:44 p.m.
Hi,

I've attached a revised version of the patch against git as of this 
morning.

It has even more FIXMEs in it than the previous version, because of 
the changes that went into QEMU since 0.11.0 and I think I'll have to 
leave those to you guys who are more familiar with QEMUs internals 
than I am, but I hope my pactch can serve as a starting point getting 
the socket handling in QEMU more correct and flexible.

On Wed, 5 May 2010 at 09:29, Daniel P. Berrange wrote:

> The approach looks reasonable, though the patch is somewhat mangled 
> by the mix of tabs + spaces

Well, that's what XEmacs gives me by default. ;)

How about adding magic comments for vim and emacs to the source files 
that bring the editors in line with the project's coding style to make 
it easier for new contributors to follow the style?

BTW, while converting the tabs to spaces in my changes, I found that 
there are also quite a few tabs in the code already.


cu
 	Reinhard

Patch

diff --git a/qemu-char.c b/qemu-char.c

index ac65a1c..aeb5afb 100644

--- a/qemu-char.c

+++ b/qemu-char.c

@@ -1915,7 +1915,8 @@  return_err:

 /* TCP Net console */
 
 typedef struct {
-    int fd, listen_fd;

+    int fd;

+    FdList *listen_fds;

     int connected;
     int max_size;
     int do_telnetopt;
@@ -2068,6 +2069,7 @@  static void tcp_chr_read(void *opaque)

     TCPCharDriver *s = chr->opaque;
     uint8_t buf[READ_BUF_LEN];
     int len, size;
+    FdList *fdl;

 
     if (!s->connected || s->max_size <= 0)
         return;
@@ -2078,9 +2080,8 @@  static void tcp_chr_read(void *opaque)

     if (size == 0) {
         /* connection closed */
         s->connected = 0;
-        if (s->listen_fd >= 0) {

-            qemu_set_fd_handler(s->listen_fd, tcp_chr_accept, NULL, chr);

-        }

+        for (fdl = s->listen_fds; fdl != NULL; fdl = fdl->next)

+            qemu_set_fd_handler(fdl->fd, tcp_chr_accept, NULL, fdl);

         qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
         closesocket(s->fd);
         s->fd = -1;
@@ -2127,7 +2128,8 @@  static void socket_set_nodelay(int fd)

 
 static void tcp_chr_accept(void *opaque)
 {
-    CharDriverState *chr = opaque;

+    FdList *fds = opaque;

+    CharDriverState *chr = fds->opaque;

     TCPCharDriver *s = chr->opaque;
     struct sockaddr_in saddr;
 #ifndef _WIN32
@@ -2148,7 +2150,7 @@  static void tcp_chr_accept(void *opaque)

 	    len = sizeof(saddr);
 	    addr = (struct sockaddr *)&saddr;
 	}
-        fd = qemu_accept(s->listen_fd, addr, &len);

+        fd = qemu_accept(fds->fd, addr, &len);

         if (fd < 0 && errno != EINTR) {
             return;
         } else if (fd >= 0) {
@@ -2161,21 +2163,24 @@  static void tcp_chr_accept(void *opaque)

     if (s->do_nodelay)
         socket_set_nodelay(fd);
     s->fd = fd;
-    qemu_set_fd_handler(s->listen_fd, NULL, NULL, NULL);

+    for (fds = s->listen_fds; fds != NULL; fds = fds->next)

+        qemu_set_fd_handler(fds->fd, NULL, NULL, NULL);

     tcp_chr_connect(chr);
 }
 
 static void tcp_chr_close(CharDriverState *chr)
 {
     TCPCharDriver *s = chr->opaque;
+    FdList *fds;

     if (s->fd >= 0) {
         qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
         closesocket(s->fd);
     }
-    if (s->listen_fd >= 0) {

-        qemu_set_fd_handler(s->listen_fd, NULL, NULL, NULL);

-        closesocket(s->listen_fd);

+    for (fds = s->listen_fds; fds != NULL; fds = fds->next) {

+        qemu_set_fd_handler(fds->fd, NULL, NULL, NULL);

+        closesocket(fds->fd);

     }
+    fdlist_free(fds);

     qemu_free(s);
     qemu_chr_event(chr, CHR_EVENT_CLOSED);
 }
@@ -2190,6 +2195,7 @@  static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)

     int do_nodelay;
     int is_unix;
     int is_telnet;
+    FdList *socks = NULL;

 
     is_listen      = qemu_opt_get_bool(opts, "server", 0);
     is_waitconnect = qemu_opt_get_bool(opts, "wait", 1);
@@ -2205,17 +2211,20 @@  static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)

     if (is_unix) {
         if (is_listen) {
             fd = unix_listen_opts(opts);
+            if (fd <= 0)

+                socks = fdlist_new(fd);

         } else {
             fd = unix_connect_opts(opts);
         }
     } else {
         if (is_listen) {
-            fd = inet_listen_opts(opts, 0);

+            socks = inet_listen_opts(opts, 0);

         } else {
             fd = inet_connect_opts(opts);
         }
     }
-    if (fd < 0)

+

+    if (socks == NULL && fd < 0)

         goto fail;
 
     if (!is_waitconnect)
@@ -2223,7 +2232,7 @@  static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)

 
     s->connected = 0;
     s->fd = -1;
-    s->listen_fd = -1;

+    s->listen_fds = NULL;

     s->msgfd = -1;
     s->is_unix = is_unix;
     s->do_nodelay = do_nodelay && !is_unix;
@@ -2234,8 +2243,11 @@  static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)

     chr->get_msgfd = tcp_get_msgfd;
 
     if (is_listen) {
-        s->listen_fd = fd;

-        qemu_set_fd_handler(s->listen_fd, tcp_chr_accept, NULL, chr);

+        s->listen_fds = socks;

+        for (; socks != NULL; socks = socks->next) {

+            socks->opaque = chr;

+            qemu_set_fd_handler(socks->fd, tcp_chr_accept, NULL, socks);

+        }

         if (is_telnet)
             s->do_telnetopt = 1;
 
@@ -2266,7 +2278,9 @@  static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)

         printf("QEMU waiting for connection on: %s\n",
                chr->filename);
         tcp_chr_accept(chr);
-        socket_set_nonblock(s->listen_fd);

+        for (socks = s->listen_fds; socks != NULL; socks = socks->next) {

+            socket_set_nonblock(socks->fd);

+        }

     }
     return chr;
 
@@ -2275,6 +2289,7 @@  static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)

         closesocket(fd);
     qemu_free(s);
     qemu_free(chr);
+    fdlist_free(socks);

     return NULL;
 }
 
diff --git a/qemu-sockets.c b/qemu-sockets.c

index a7399aa..ecda69d 100644

--- a/qemu-sockets.c

+++ b/qemu-sockets.c

@@ -57,6 +57,25 @@  static QemuOptsList dummy_opts = {

     },
 };
 
+FdList* fdlist_new(int fd)

+{

+    FdList *new = malloc(sizeof(FdList));

+    new->fd = fd;

+    new->opaque = NULL;

+    new->next = NULL;

+    return new;

+}

+

+void fdlist_free(FdList *fds)

+{

+    FdList *tmp;

+    while (fds) {

+        tmp = fds->next;

+        free(fds);

+        fds = tmp;

+    }

+}

+

 static int inet_getport(struct addrinfo *e)
 {
     struct sockaddr_in *i4;
@@ -116,14 +135,15 @@  static void inet_print_addrinfo(const char *tag, struct addrinfo *res)

     }
 }
 
-int inet_listen_opts(QemuOpts *opts, int port_offset)

+FdList *inet_listen_opts(QemuOpts *opts, int port_offset)

 {
-    struct addrinfo ai,*res,*e;

+    struct addrinfo ai,*res,*e,*last=NULL;

     const char *addr;
     char port[33];
     char uaddr[INET6_ADDRSTRLEN+1];
     char uport[33];
     int slisten,rc,to,try_next;
+    FdList *fds = NULL, *newfd;

 
     memset(&ai,0, sizeof(ai));
     ai.ai_flags = AI_PASSIVE | AI_ADDRCONFIG;
@@ -132,7 +152,7 @@  int inet_listen_opts(QemuOpts *opts, int port_offset)

 
     if (qemu_opt_get(opts, "port") == NULL) {
         fprintf(stderr, "%s: host and/or port not specified\n", __FUNCTION__);
-        return -1;

+        return NULL;

     }
     pstrcpy(port, sizeof(port), qemu_opt_get(opts, "port"));
     addr = qemu_opt_get(opts, "host");
@@ -150,7 +170,7 @@  int inet_listen_opts(QemuOpts *opts, int port_offset)

     if (rc != 0) {
         fprintf(stderr,"getaddrinfo(%s,%s): %s\n", addr, port,
                 gai_strerror(rc));
-        return -1;

+        return NULL;

     }
     if (sockets_debug)
         inet_print_addrinfo(__FUNCTION__, res);
@@ -170,9 +190,9 @@  int inet_listen_opts(QemuOpts *opts, int port_offset)

         setsockopt(slisten,SOL_SOCKET,SO_REUSEADDR,(void*)&on,sizeof(on));
 #ifdef IPV6_V6ONLY
         if (e->ai_family == PF_INET6) {
-            /* listen on both ipv4 and ipv6 */

-            setsockopt(slisten,IPPROTO_IPV6,IPV6_V6ONLY,(void*)&off,

-                sizeof(off));

+            /* listen on IPv6 only, IPv4 is handled by a separate socket */

+            setsockopt(slisten,IPPROTO_IPV6,IPV6_V6ONLY,(void*)&on,

+                sizeof(on));

         }
 #endif
 
@@ -181,9 +201,19 @@  int inet_listen_opts(QemuOpts *opts, int port_offset)

                 if (sockets_debug)
                     fprintf(stderr,"%s: bind(%s,%s,%d): OK\n", __FUNCTION__,
                         inet_strfamily(e->ai_family), uaddr, inet_getport(e));
-                goto listen;

+                if (listen(slisten, 1) != 0) {

+                    closesocket(slisten);

+                } else {

+                    fprintf(stderr, "slisten= %d\n", slisten);

+                    newfd = fdlist_new(slisten);

+                    newfd->next = fds;

+                    fds = newfd;

+                    last = e;

+                }

+                break;

             }
             try_next = to && (inet_getport(e) <= to + port_offset);
+            /* FIXME: should make sure that all sockets get the same port */

             if (!try_next || sockets_debug)
                 fprintf(stderr,"%s: bind(%s,%s,%d): %s\n", __FUNCTION__,
                         inet_strfamily(e->ai_family), uaddr, inet_getport(e),
@@ -192,28 +222,23 @@  int inet_listen_opts(QemuOpts *opts, int port_offset)

                 inet_setport(e, inet_getport(e) + 1);
                 continue;
             }
+            closesocket(slisten);

             break;
         }
-        closesocket(slisten);

     }
+    if (fds == NULL) {

     fprintf(stderr, "%s: FAILED\n", __FUNCTION__);
-    freeaddrinfo(res);

-    return -1;

-

-listen:

-    if (listen(slisten,1) != 0) {

-        perror("listen");

-        closesocket(slisten);

-        freeaddrinfo(res);

-        return -1;

-    }

-    snprintf(uport, sizeof(uport), "%d", inet_getport(e) - port_offset);

+    } else {

+        /* FIXME: should save all addresses here, not just the last one */

+        snprintf(uport, sizeof(uport), "%d", inet_getport(last) - port_offset);

     qemu_opt_set(opts, "host", uaddr);
     qemu_opt_set(opts, "port", uport);
-    qemu_opt_set(opts, "ipv6", (e->ai_family == PF_INET6) ? "on" : "off");

-    qemu_opt_set(opts, "ipv4", (e->ai_family != PF_INET6) ? "on" : "off");

+        qemu_opt_set(opts, "ipv6", (last->ai_family == PF_INET6) ? "on" : "off");

+        qemu_opt_set(opts, "ipv4", (last->ai_family != PF_INET6) ? "on" : "off");

+    }

     freeaddrinfo(res);
-    return slisten;

+    fprintf (stderr, "fds=%p\n", fds);

+    return fds;

 }
 
 int inet_connect_opts(QemuOpts *opts)
@@ -456,17 +481,17 @@  static int inet_parse(QemuOpts *opts, const char *str)

     return 0;
 }
 
-int inet_listen(const char *str, char *ostr, int olen,

+FdList* inet_listen(const char *str, char *ostr, int olen,

                 int socktype, int port_offset)
 {
     QemuOpts *opts;
     char *optstr;
-    int sock = -1;

+    FdList *socks = NULL;

 
     opts = qemu_opts_create(&dummy_opts, NULL, 0);
     if (inet_parse(opts, str) == 0) {
-        sock = inet_listen_opts(opts, port_offset);

-        if (sock != -1 && ostr) {

+        socks = inet_listen_opts(opts, port_offset);

+        if (socks != NULL && ostr) {

             optstr = strchr(str, ',');
             if (qemu_opt_get_bool(opts, "ipv6", 0)) {
                 snprintf(ostr, olen, "[%s]:%s%s",
@@ -482,7 +507,7 @@  int inet_listen(const char *str, char *ostr, int olen,

         }
     }
     qemu_opts_del(opts);
-    return sock;

+    return socks;

 }
 
 int inet_connect(const char *str, int socktype)
diff --git a/qemu_socket.h b/qemu_socket.h

index 164ae3e..1f52663 100644

--- a/qemu_socket.h

+++ b/qemu_socket.h

@@ -29,6 +29,13 @@  int inet_aton(const char *cp, struct in_addr *ia);

 
 #endif /* !_WIN32 */
 
+struct FdList;

+typedef struct FdList {

+    int fd;

+    void *opaque;

+    struct FdList *next;

+} FdList;

+

 #include "qemu-option.h"
 
 /* misc helpers */
@@ -36,10 +43,12 @@  int qemu_socket(int domain, int type, int protocol);

 int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
 void socket_set_nonblock(int fd);
 int send_all(int fd, const void *buf, int len1);
+FdList *fdlist_new(int fd);

+void fdlist_free(FdList *fds);

 
 /* New, ipv6-ready socket helper functions, see qemu-sockets.c */
-int inet_listen_opts(QemuOpts *opts, int port_offset);

-int inet_listen(const char *str, char *ostr, int olen,

+FdList *inet_listen_opts(QemuOpts *opts, int port_offset);

+FdList *inet_listen(const char *str, char *ostr, int olen,

                 int socktype, int port_offset);
 int inet_connect_opts(QemuOpts *opts);
 int inet_connect(const char *str, int socktype);
diff --git a/vnc.c b/vnc.c

index b1a3fdb..aab8931 100644

--- a/vnc.c

+++ b/vnc.c

@@ -26,7 +26,6 @@ 

 
 #include "vnc.h"
 #include "sysemu.h"
-#include "qemu_socket.h"

 #include "qemu-timer.h"
 #include "acl.h"
 #include "qemu-objects.h"
@@ -202,7 +201,8 @@  static const char *vnc_auth_name(VncDisplay *vd) {

 
 static int vnc_server_info_put(QDict *qdict)
 {
-    if (vnc_server_addr_put(qdict, vnc_display->lsock) < 0) {

+    /* FIXME: need to consider all sockets here, not just the first one */

+    if (vnc_server_addr_put(qdict, vnc_display->lsocks->fd) < 0) {

         return -1;
     }
 
@@ -2247,14 +2247,15 @@  static void vnc_connect(VncDisplay *vd, int csock)

 
 static void vnc_listen_read(void *opaque)
 {
-    VncDisplay *vs = opaque;

+    FdList *sock = opaque;

+    VncDisplay *vs = sock->opaque;

     struct sockaddr_in addr;
     socklen_t addrlen = sizeof(addr);
 
     /* Catch-up */
     vga_hw_update();
 
-    int csock = qemu_accept(vs->lsock, (struct sockaddr *)&addr, &addrlen);

+    int csock = qemu_accept(sock->fd, (struct sockaddr *)&addr, &addrlen);

     if (csock != -1) {
         vnc_connect(vs, csock);
     }
@@ -2270,7 +2271,7 @@  void vnc_display_init(DisplayState *ds)

     dcl->idle = 1;
     vnc_display = vs;
 
-    vs->lsock = -1;

+    vs->lsocks = NULL;

 
     vs->ds = ds;
     QTAILQ_INIT(&vs->clients);
@@ -2294,6 +2295,7 @@  void vnc_display_init(DisplayState *ds)

 void vnc_display_close(DisplayState *ds)
 {
     VncDisplay *vs = ds ? (VncDisplay *)ds->opaque : vnc_display;
+    FdList *fds, *fdtmp;

 
     if (!vs)
         return;
@@ -2301,11 +2303,13 @@  void vnc_display_close(DisplayState *ds)

         qemu_free(vs->display);
         vs->display = NULL;
     }
-    if (vs->lsock != -1) {

-        qemu_set_fd_handler2(vs->lsock, NULL, NULL, NULL, NULL);

-        close(vs->lsock);

-        vs->lsock = -1;

+    for (fds = vs->lsocks; fds != NULL; fds = fdtmp) {

+        fdtmp = fds->next;

+        qemu_set_fd_handler2(fds->fd, NULL, NULL, NULL, NULL);

+        close(fds->fd);

+        free(fds);

     }
+    vs->lsocks = NULL;

     vs->auth = VNC_AUTH_INVALID;
 #ifdef CONFIG_VNC_TLS
     vs->subauth = VNC_AUTH_INVALID;
@@ -2342,7 +2346,8 @@  char *vnc_display_local_addr(DisplayState *ds)

 {
     VncDisplay *vs = ds ? (VncDisplay *)ds->opaque : vnc_display;
     
-    return vnc_socket_local_addr("%s:%s", vs->lsock);

+    /* FIXME: should return all addresses */

+    return vnc_socket_local_addr("%s:%s", vs->lsocks->fd);

 }
 
 int vnc_display_open(DisplayState *ds, const char *display)
@@ -2527,39 +2532,49 @@  int vnc_display_open(DisplayState *ds, const char *display)

     vs->lock_key_sync = lock_key_sync;
 
     if (reverse) {
+        int sock;

         /* connect to viewer */
         if (strncmp(display, "unix:", 5) == 0)
-            vs->lsock = unix_connect(display+5);

+            sock = unix_connect(display+5);

         else
-            vs->lsock = inet_connect(display, SOCK_STREAM);

-        if (-1 == vs->lsock) {

+            sock = inet_connect(display, SOCK_STREAM);

+        if (-1 == sock) {

             free(vs->display);
             vs->display = NULL;
             return -1;
         } else {
-            int csock = vs->lsock;

-            vs->lsock = -1;

-            vnc_connect(vs, csock);

+            vnc_connect(vs, sock);

         }
         return 0;
 
     } else {
         /* listen for connects */
         char *dpy;
+        int fd;

+        FdList *socks = NULL;

+

         dpy = qemu_malloc(256);
         if (strncmp(display, "unix:", 5) == 0) {
             pstrcpy(dpy, 256, "unix:");
-            vs->lsock = unix_listen(display+5, dpy+5, 256-5);

+            fd = unix_listen(display+5, dpy+5, 256-5);

+            if (fd >= 0)

+                socks = fdlist_new(fd);

         } else {
-            vs->lsock = inet_listen(display, dpy, 256, SOCK_STREAM, 5900);

+            socks = inet_listen(display, dpy, 256, SOCK_STREAM, 5900);

         }
-        if (-1 == vs->lsock) {

+        

+        if (socks == NULL) {

             free(dpy);
             return -1;
         } else {
+            vs->lsocks = socks;

             free(vs->display);
             vs->display = dpy;
         }
+        for (; socks != NULL; socks = socks->next) {

+            socks->opaque = vs;

+            qemu_set_fd_handler2(socks->fd, NULL, vnc_listen_read, NULL, socks);

+        }

+        return 0;

     }
-    return qemu_set_fd_handler2(vs->lsock, NULL, vnc_listen_read, NULL, vs);

 }
diff --git a/vnc.h b/vnc.h

index 1aa71b0..a4fa9a9 100644

--- a/vnc.h

+++ b/vnc.h

@@ -28,6 +28,7 @@ 

 #define __QEMU_VNC_H
 
 #include "qemu-common.h"
+#include "qemu_socket.h"

 #include "qemu-queue.h"
 #include "console.h"
 #include "monitor.h"
@@ -96,7 +97,7 @@  struct VncDisplay

     QTAILQ_HEAD(, VncState) clients;
     QEMUTimer *timer;
     int timer_interval;
-    int lsock;

+    FdList *lsocks;

     DisplayState *ds;
     kbd_layout_t *kbd_layout;
     int lock_key_sync;