From 5d5326a03528f4aa22c9d78862cda307736c3f6a Mon Sep 17 00:00:00 2001 Message-Id: <5d5326a03528f4aa22c9d78862cda307736c3f6a.1349175436.git.minovotn@redhat.com> In-Reply-To: <94968b7fa9b14e71f004474d7ce77e189e6a2bf3.1349175436.git.minovotn@redhat.com> References: <94968b7fa9b14e71f004474d7ce77e189e6a2bf3.1349175436.git.minovotn@redhat.com> From: Amos Kong Date: Mon, 1 Oct 2012 14:12:46 +0200 Subject: [PATCH 33/34] Fix address handling in inet_nonblocking_connect RH-Author: Amos Kong Message-id: <1349100767-9066-14-git-send-email-akong@redhat.com> Patchwork-id: 42572 O-Subject: [RHEL-6.4 qemu-kvm PATCH v7 13/14] Fix address handling in inet_nonblocking_connect Bugzilla: 680356 RH-Acked-by: Paolo Bonzini RH-Acked-by: Markus Armbruster RH-Acked-by: Orit Wasserman From: Orit Wasserman 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 (Cherry-picked from commit 233aa5c2d1cf4655ffe335025a68cf5454f87dad) Conflicts: migration-tcp.c qemu-sockets.c Signed-off-by: Orit Wasserman Signed-off-by: Michael S. Tsirkin Signed-off-by: Anthony Liguori Signed-off-by: Amos Kong --- migration-tcp.c | 37 ++++------------ qemu-char.c | 2 +- qemu-sockets.c | 129 ++++++++++++++++++++++++++++++++++++++++++++---------- qemu_socket.h | 16 +++++-- 4 files changed, 126 insertions(+), 58 deletions(-) Signed-off-by: Michal Novotny --- migration-tcp.c | 37 ++++------------ qemu-char.c | 2 +- qemu-sockets.c | 129 +++++++++++++++++++++++++++++++++++++++++++++----------- qemu_socket.h | 16 ++++--- 4 files changed, 126 insertions(+), 58 deletions(-) diff --git a/migration-tcp.c b/migration-tcp.c index dc218c8..7e46755 100644 --- a/migration-tcp.c +++ b/migration-tcp.c @@ -49,30 +49,18 @@ static int tcp_close(FdMigrationState *s) return 0; } - -static void tcp_wait_for_connect(void *opaque) +static void tcp_wait_for_connect(int fd, void *opaque) { FdMigrationState *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 && (s->get_error(s)) == 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); } } @@ -85,7 +73,6 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon, Error **errp) { FdMigrationState *s; - bool in_progress; s = qemu_mallocz(sizeof(*s)); @@ -107,20 +94,14 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon, migrate_fd_monitor_suspend(s, mon); } - s->fd = inet_nonblocking_connect(host_port, &in_progress, errp); + s->fd = inet_nonblocking_connect(host_port, tcp_wait_for_connect, s, + errp); if (error_is_set(errp)) { migrate_fd_error(s); qemu_free(s); return NULL; } - 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); - } - return &s->mig_state; } diff --git a/qemu-char.c b/qemu-char.c index 8dc1cda..0e42bdb 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2350,7 +2350,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts) if (is_listen) { fd = inet_listen_opts(opts, 0, NULL); } else { - fd = inet_connect_opts(opts, true, NULL, NULL); + fd = inet_connect_opts(opts, NULL, NULL, NULL); } } if (fd < 0) diff --git a/qemu-sockets.c b/qemu-sockets.c index 4a28e9e..a2a1e68 100644 --- a/qemu-sockets.c +++ b/qemu-sockets.c @@ -21,6 +21,7 @@ #include "qemu_socket.h" #include "qemu-common.h" /* for qemu_isdigit */ +#include "qemu-char.h" #ifndef AI_ADDRCONFIG # define AI_ADDRCONFIG 0 @@ -211,14 +212,66 @@ listen: ((rc) == -EINPROGRESS) #endif -static int inet_connect_addr(struct addrinfo *addr, bool block, - bool *in_progress) +/* Struct to store connect state for non blocking connect */ +typedef struct ConnectState { + int fd; + struct addrinfo *addr_list; + struct addrinfo *current_addr; + NonBlockingConnectHandler *callback; + void *opaque; +} ConnectState; + +static int inet_connect_addr(struct addrinfo *addr, bool *in_progress, + ConnectState *connect_state); + +static void wait_for_connect(void *opaque) { - int sock, rc; + ConnectState *s = opaque; + int val = 0, rc = 0; + socklen_t valsize = sizeof(val); + bool in_progress; + + qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL); + + do { + rc = getsockopt(s->fd, SOL_SOCKET, SO_ERROR, (void *) &val, &valsize); + } while (rc == -1 && socket_error() == EINTR); + + /* update rc to contain error */ + if (!rc && val) { + rc = -1; + } + + /* connect error */ + if (rc < 0) { + closesocket(s->fd); + s->fd = rc; + } + + /* try to connect to the next address on the list */ + while (s->current_addr->ai_next != NULL && s->fd < 0) { + s->current_addr = s->current_addr->ai_next; + s->fd = inet_connect_addr(s->current_addr, &in_progress, s); + /* connect in progress */ + if (in_progress) { + return; + } + } - if (in_progress) { - *in_progress = false; + freeaddrinfo(s->addr_list); + if (s->callback) { + s->callback(s->fd, s->opaque); } + g_free(s); + return; +} + +static int inet_connect_addr(struct addrinfo *addr, bool *in_progress, + ConnectState *connect_state) +{ + int sock, rc; + + *in_progress = false; sock = qemu_socket(addr->ai_family, addr->ai_socktype, addr->ai_protocol); if (sock < 0) { @@ -227,7 +280,7 @@ static int inet_connect_addr(struct addrinfo *addr, bool block, return -1; } setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on)); - if (!block) { + if (connect_state != NULL) { socket_set_nonblock(sock); } /* connect to peer */ @@ -238,10 +291,11 @@ static int inet_connect_addr(struct addrinfo *addr, bool block, } } while (rc == -EINTR); - if (!block && QEMU_SOCKET_RC_INPROGRESS(rc)) { - if (in_progress) { - *in_progress = true; - } + if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) { + connect_state->fd = sock; + qemu_set_fd_handler2(sock, NULL, NULL, wait_for_connect, + connect_state); + *in_progress = true; } else if (rc < 0) { closesocket(sock); return -1; @@ -257,6 +311,7 @@ static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp) 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; @@ -293,36 +348,55 @@ static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp) * * @opts: QEMU options, recognized parameters strings "host" and "port", * bools "ipv4" and "ipv6". - * @block: set true for blocking socket - * @in_progress: set to true in case of ongoing connect * @errp: set on error + * @callback: callback function for non-blocking connect + * @opaque: opaque for callback function * * Returns: -1 on error, file descriptor on success. + * + * If @callback is non-null, the connect is non-blocking. If this + * function succeeds, callback will be called when the connection + * completes, with the file descriptor on success, or -1 on error. */ -int inet_connect_opts(QemuOpts *opts, bool block, bool *in_progress, - Error **errp) +int inet_connect_opts(QemuOpts *opts, Error **errp, + NonBlockingConnectHandler *callback, void *opaque) { struct addrinfo *res, *e; int sock = -1; + bool in_progress; + ConnectState *connect_state = NULL; res = inet_parse_connect_opts(opts, errp); if (!res) { return -1; } - if (in_progress) { - *in_progress = false; + if (callback != NULL) { + connect_state = g_malloc0(sizeof(*connect_state)); + connect_state->addr_list = res; + connect_state->callback = callback; + connect_state->opaque = opaque; } for (e = res; e != NULL; e = e->ai_next) { - sock = inet_connect_addr(e, block, in_progress); - if (sock >= 0) { + if (connect_state != NULL) { + connect_state->current_addr = e; + } + sock = inet_connect_addr(e, &in_progress, connect_state); + if (in_progress) { + return sock; + } else if (sock >= 0) { + /* non blocking socket immediate success, call callback */ + if (callback != NULL) { + callback(sock, opaque); + } break; } } if (sock < 0) { error_set(errp, QERR_SOCKET_CONNECT_FAILED); } + g_free(connect_state); freeaddrinfo(res); return sock; } @@ -535,7 +609,7 @@ int inet_connect(const char *str, Error **errp) opts = qemu_opts_create(&dummy_opts, NULL, 0); if (inet_parse(opts, str) == 0) { - sock = inet_connect_opts(opts, true, NULL, errp); + sock = inet_connect_opts(opts, errp, NULL, NULL); } else { error_set(errp, QERR_SOCKET_CREATE_FAILED); } @@ -545,22 +619,29 @@ int inet_connect(const char *str, Error **errp) /** * Create a non-blocking socket and connect it to an address. + * Calls the callback function with fd in case of success or -1 in case of + * error. * * @str: address string - * @in_progress: set to true in case of ongoing connect + * @callback: callback function that is called when connect completes, + * cannot be NULL. + * @opaque: opaque for callback function * @errp: set in case of an error * - * Returns: -1 on error, file descriptor on success. + * Returns: -1 on immediate error, file descriptor on success. **/ -int inet_nonblocking_connect(const char *str, bool *in_progress, - Error **errp) +int inet_nonblocking_connect(const char *str, + NonBlockingConnectHandler *callback, + void *opaque, Error **errp) { QemuOpts *opts; int sock = -1; + g_assert(callback != NULL); + opts = qemu_opts_create(&dummy_opts, NULL, 0); if (inet_parse(opts, str) == 0) { - sock = inet_connect_opts(opts, false, in_progress, errp); + sock = inet_connect_opts(opts, errp, callback, opaque); } else { error_set(errp, QERR_SOCKET_CREATE_FAILED); } diff --git a/qemu_socket.h b/qemu_socket.h index 898b2a5..3c07fad 100644 --- a/qemu_socket.h +++ b/qemu_socket.h @@ -39,15 +39,21 @@ int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen); void socket_set_nonblock(int fd); int send_all(CharDriverState *chr, int fd, const void *buf, int len1); -/* New, ipv6-ready socket helper functions, see qemu-sockets.c */ +/* callback function for nonblocking connect + * valid fd on success, negative error code on failure + */ +typedef void NonBlockingConnectHandler(int fd, void *opaque); + 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 block, bool *in_progress, - Error **errp); +int inet_connect_opts(QemuOpts *opts, Error **errp, + NonBlockingConnectHandler *callback, void *opaque); 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, + NonBlockingConnectHandler *callback, + void *opaque, Error **errp); + int inet_dgram_opts(QemuOpts *opts); const char *inet_strfamily(int family); -- 1.7.11.4