From patchwork Wed Sep 12 11:12:58 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Orit Wasserman X-Patchwork-Id: 183321 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 4309E2C0086 for ; Wed, 12 Sep 2012 21:14:21 +1000 (EST) Received: from localhost ([::1]:44113 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TBktf-00066x-5D for incoming@patchwork.ozlabs.org; Wed, 12 Sep 2012 07:14:19 -0400 Received: from eggs.gnu.org ([208.118.235.92]:38384) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TBktS-0005vM-CF for qemu-devel@nongnu.org; Wed, 12 Sep 2012 07:14:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TBktP-0001Yp-GA for qemu-devel@nongnu.org; Wed, 12 Sep 2012 07:14:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39903) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TBktP-0001YX-54 for qemu-devel@nongnu.org; Wed, 12 Sep 2012 07:14:03 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q8CBE1D9020948 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 12 Sep 2012 07:14:01 -0400 Received: from dhcp-1-120.tlv.redhat.com (dhcp-4-125.tlv.redhat.com [10.35.4.125]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q8CBDWra024893; Wed, 12 Sep 2012 07:13:56 -0400 From: Orit Wasserman To: qemu-devel@nongnu.org Date: Wed, 12 Sep 2012 14:12:58 +0300 Message-Id: <1347448378-23915-4-git-send-email-owasserm@redhat.com> In-Reply-To: <1347448378-23915-1-git-send-email-owasserm@redhat.com> References: <1347448378-23915-1-git-send-email-owasserm@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.25 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Cc: kwolf@redhat.com, aliguori@us.ibm.com, quintela@redhat.com, armbru@redhat.com, mst@redhat.com, mdroth@linux.vnet.ibm.com, lcapitulino@redhat.com, Orit Wasserman , pbonzini@redhat.com, akong@redhat.com Subject: [Qemu-devel] [PATCH v2 3/3] Fix address handling in inet_nonblocking_connect X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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 Signed-off-by: Michael S. Tsirkin --- 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);