[Qemu-devel] [PATCH v2 3/3] Fix address handling in inet_nonblocking_connect

[Orit Wasserman <owasserm@redhat.com>]

getaddrinfo can give us a list of addresses, but we only try to
connect to the first one. If that fails we never proceed to
the next one.  This is common on desktop setups that often have ipv6
configured but not actually working.

To fix this make inet_connect_nonblocking retry connection with a different
address.
callers on inet_nonblocking_connect register a callback function that will
be called when connect opertion completes, in case of failure the fd will have
a negative value

Signed-off-by: Orit Wasserman <owasserm@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 migration-tcp.c |   29 +++-------
 qemu-sockets.c  |  169 +++++++++++++++++++++++++++++++++++++++++--------------
 qemu_socket.h   |    9 ++-
 3 files changed, 142 insertions(+), 65 deletions(-)

diff --git a/migration-tcp.c b/migration-tcp.c
index 7f6ad98..cadea36 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -53,29 +53,18 @@ static int tcp_close(MigrationState *s)
     return r;
 }
 
-static void tcp_wait_for_connect(void *opaque)
+static void tcp_wait_for_connect(int fd, void *opaque)
 {
     MigrationState *s = opaque;
-    int val, ret;
-    socklen_t valsize = sizeof(val);
 
-    DPRINTF("connect completed\n");
-    do {
-        ret = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val, &valsize);
-    } while (ret == -1 && (socket_error()) == EINTR);
-
-    if (ret < 0) {
+    if (fd < 0) {
+        DPRINTF("migrate connect error\n");
+        s->fd = -1;
         migrate_fd_error(s);
-        return;
-    }
-
-    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
-
-    if (val == 0)
+    } else {
+        DPRINTF("migrate connect success\n");
+        s->fd = fd;
         migrate_fd_connect(s);
-    else {
-        DPRINTF("error connecting %d\n", val);
-        migrate_fd_error(s);
     }
 }
 
@@ -88,7 +77,8 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
     s->write = socket_write;
     s->close = tcp_close;
 
-    s->fd = inet_nonblocking_connect(host_port, &in_progress, errp);
+    s->fd = inet_nonblocking_connect(host_port, tcp_wait_for_connect, s,
+                                     &in_progress, errp);
     if (error_is_set(errp)) {
         migrate_fd_error(s);
         return -1;
@@ -96,7 +86,6 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
 
     if (in_progress) {
         DPRINTF("connect in progress\n");
-        qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
     } else {
         migrate_fd_connect(s);
     }
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 4f5eca8..f5d64c8 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -24,6 +24,7 @@
 
 #include "qemu_socket.h"
 #include "qemu-common.h" /* for qemu_isdigit */
+#include "main-loop.h"
 
 #ifndef AI_ADDRCONFIG
 # define AI_ADDRCONFIG 0
@@ -209,41 +210,27 @@ listen:
     return slisten;
 }
 
-static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp)
-{
-    struct addrinfo ai, *res;
-    int rc;
-    const char *addr;
-    const char *port;
-
-    memset(&ai,0, sizeof(ai));
-    ai.ai_flags = AI_CANONNAME | AI_ADDRCONFIG;
-    ai.ai_family = PF_UNSPEC;
-    ai.ai_socktype = SOCK_STREAM;
-
-    addr = qemu_opt_get(opts, "host");
-    port = qemu_opt_get(opts, "port");
-    if (addr == NULL || port == NULL) {
-        fprintf(stderr,
-                "inet_parse_connect_opts: host and/or port not specified\n");
-        error_set(errp, QERR_SOCKET_CREATE_FAILED);
-        return NULL;
-    }
-
-    if (qemu_opt_get_bool(opts, "ipv4", 0))
-        ai.ai_family = PF_INET;
-    if (qemu_opt_get_bool(opts, "ipv6", 0))
-        ai.ai_family = PF_INET6;
+#ifdef _WIN32
+#define QEMU_SOCKET_RC_INPROGRESS(rc) \
+    ((rc) == -EINPROGRESS || rc == -EWOULDBLOCK || rc == -WSAEALREADY)
+#else
+#define QEMU_SOCKET_RC_INPROGRESS(rc) \
+    ((rc) == -EINPROGRESS)
+#endif
 
-    /* lookup */
-    if (0 != (rc = getaddrinfo(addr, port, &ai, &res))) {
-        fprintf(stderr,"getaddrinfo(%s,%s): %s\n", addr, port,
-                gai_strerror(rc));
-        error_set(errp, QERR_SOCKET_CREATE_FAILED);
-        return NULL;
-    }
-    return res;
-}
+/* Struct to store connect state for non blocking connect */
+typedef struct ConnectState {
+    int fd;
+    struct addrinfo *addr_list;
+    struct addrinfo *current_addr;
+    ConnectHandler *callback;
+    void *opaque;
+    Error *errp;
+} ConnectState;
+
+static ConnectState connect_state = {
+    .fd = -1,
+};
 
 #ifdef _WIN32
 #define QEMU_SOCKET_RC_INPROGRESS(rc) \
@@ -254,12 +241,17 @@ static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp)
 #endif
 
 static int inet_connect_addr(struct addrinfo *addr, bool block,
-                             bool *in_progress, Error **errp)
+                             IOHandler *handler, bool *in_progress,
+                             Error **errp)
 {
     char uaddr[INET6_ADDRSTRLEN + 1];
     char uport[33];
     int sock, rc;
 
+    if (in_progress) {
+        *in_progress = false;
+    }
+
     if (getnameinfo((struct sockaddr *)addr->ai_addr, addr->ai_addrlen,
                     uaddr, INET6_ADDRSTRLEN, uport, 32,
                     NI_NUMERICHOST | NI_NUMERICSERV)) {
@@ -285,6 +277,8 @@ static int inet_connect_addr(struct addrinfo *addr, bool block,
     } while (rc == -EINTR);
 
     if (!block && QEMU_SOCKET_RC_INPROGRESS(rc)) {
+        connect_state.fd = sock;
+        qemu_set_fd_handler2(sock, NULL, NULL, handler, &connect_state);
         if (in_progress) {
             *in_progress = true;
         }
@@ -295,6 +289,94 @@ static int inet_connect_addr(struct addrinfo *addr, bool block,
     return sock;
 }
 
+static void wait_for_connect(void *opaque)
+{
+    ConnectState *s = opaque;
+    int val = 0, rc = 0;
+    socklen_t valsize = sizeof(val);
+    bool in_progress = false;
+
+    do {
+        rc = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val, &valsize);
+    } while (rc == -1 && (socket_error()) == EINTR);
+
+    /* connect succeded */
+    if (!rc && !val) {
+        qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+        freeaddrinfo(s->addr_list);
+        if (s->callback) {
+            s->callback(s->fd, s->opaque);
+        }
+        return;
+    }
+
+    if (!rc && val) {
+        rc = -val;
+    }
+
+    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+    closesocket(s->fd);
+    if (s->current_addr != NULL && s->current_addr->ai_next != NULL) {
+        s->current_addr = s->current_addr->ai_next;
+        s->fd = inet_connect_addr(s->current_addr, false, wait_for_connect,
+                                  &in_progress, &s->errp);
+        if (in_progress) {
+            return;
+        }
+    }
+
+    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+
+    closesocket(s->fd);
+    s->fd = rc;
+    freeaddrinfo(s->addr_list);
+    /* connect failed */
+    if (s->callback) {
+        s->callback(s->fd, s->opaque);
+    }
+    return;
+}
+
+static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp)
+{
+    struct addrinfo ai, *res;
+    int rc;
+    const char *addr;
+    const char *port;
+
+    memset(&ai, 0, sizeof(ai));
+    ai.ai_flags = AI_CANONNAME | AI_ADDRCONFIG;
+    ai.ai_family = PF_UNSPEC;
+    ai.ai_socktype = SOCK_STREAM;
+
+    addr = qemu_opt_get(opts, "host");
+    port = qemu_opt_get(opts, "port");
+    if (addr == NULL || port == NULL) {
+        fprintf(stderr,
+                "inet_parse_connect_opts: host and/or port not specified\n");
+        error_set(errp, QERR_SOCKET_CREATE_FAILED);
+        return NULL;
+    }
+
+    if (qemu_opt_get_bool(opts, "ipv4", 0)) {
+        ai.ai_family = PF_INET;
+    }
+    if (qemu_opt_get_bool(opts, "ipv6", 0)) {
+        ai.ai_family = PF_INET6;
+    }
+
+    /* lookup */
+    rc = getaddrinfo(addr, port, &ai, &res);
+    if (rc != 0) {
+        fprintf(stderr, "getaddrinfo(%s,%s): %s\n", addr, port,
+                gai_strerror(rc));
+        error_set(errp, QERR_SOCKET_CREATE_FAILED);
+        return NULL;
+    }
+    return res;
+}
+
+
 int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp)
 {
     struct addrinfo *res, *e;
@@ -306,12 +388,12 @@ int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp)
         return -1;
     }
 
-    if (in_progress) {
-        *in_progress = false;
-    }
-
     for (e = res; e != NULL; e = e->ai_next) {
-        sock = inet_connect_addr(e, block, in_progress, errp);
+        if (!block) {
+            connect_state.addr_list = res;
+            connect_state.current_addr = e;
+        }
+        sock = inet_connect_addr(e, block, wait_for_connect, in_progress, errp);
         if (in_progress && *in_progress) {
             return sock;
         } else if (sock >= 0) {
@@ -534,9 +616,8 @@ int inet_connect(const char *str, Error **errp)
     return sock;
 }
 
-
-int inet_nonblocking_connect(const char *str, bool *in_progress,
-                             Error **errp)
+int inet_nonblocking_connect(const char *str, ConnectHandler *callback,
+                             void *opaque, bool *in_progress, Error **errp)
 {
     QemuOpts *opts;
     int sock = -1;
@@ -544,6 +625,8 @@ int inet_nonblocking_connect(const char *str, bool *in_progress,
     opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
     if (inet_parse(opts, str) == 0) {
         qemu_opt_set(opts, "block", "off");
+        connect_state.callback = callback;
+        connect_state.opaque = opaque;
         sock = inet_connect_opts(opts, in_progress, errp);
     } else {
         error_set(errp, QERR_SOCKET_CREATE_FAILED);
diff --git a/qemu_socket.h b/qemu_socket.h
index c47f2b0..c8431eb 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -38,14 +38,19 @@ void socket_set_block(int fd);
 void socket_set_nonblock(int fd);
 int send_all(int fd, const void *buf, int len1);
 
+/* callback function for nonblocking connect
+ * vaild fd on success, negative error code on failure
+ */
+typedef void ConnectHandler(int fd, void *opaque);
+
 /* New, ipv6-ready socket helper functions, see qemu-sockets.c */
 int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp);
 int inet_listen(const char *str, char *ostr, int olen,
                 int socktype, int port_offset, Error **errp);
 int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp);
 int inet_connect(const char *str, Error **errp);
-int inet_nonblocking_connect(const char *str, bool *in_progress,
-                             Error **errp);
+int inet_nonblocking_connect(const char *str, ConnectHandler *callback,
+                             void *opaque, bool *in_progress, Error **errp);
 int inet_dgram_opts(QemuOpts *opts);
 const char *inet_strfamily(int family);
 
-- 
1.7.7.6