* [Qemu-devel] [PATCH v5 1/4] Refactor inet_connect_opts function
2012-09-24 11:11 [Qemu-devel] [PATCH v5 0/4] non-blocking connect address handling cleanup Orit Wasserman
@ 2012-09-24 11:11 ` Orit Wasserman
2012-09-24 11:11 ` [Qemu-devel] [PATCH v5 2/4] Separate inet_connect into inet_connect (blocking) and inet_nonblocking_connect Orit Wasserman
` (6 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Orit Wasserman @ 2012-09-24 11:11 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, aliguori, quintela, armbru, mst, mdroth, lcapitulino,
Orit Wasserman, pbonzini, akong
From: "Michael S. Tsirkin" <mst@redhat.com>
refactor address resolution code to fix nonblocking connect
remove getnameinfo call
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Amos Kong <akong@redhat.com>
Signed-off-by: Orit Wasserman <owasserm@redhat.com>
---
qemu-sockets.c | 148 ++++++++++++++++++++++++++++++++------------------------
1 files changed, 85 insertions(+), 63 deletions(-)
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 361d890..7ca0ac9 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -209,95 +209,117 @@ listen:
return slisten;
}
-int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp)
+#ifdef _WIN32
+#define QEMU_SOCKET_RC_INPROGRESS(rc) \
+ ((rc) == -EINPROGRESS || (rc) == -EWOULDBLOCK || (rc) == -WSAEALREADY)
+#else
+#define QEMU_SOCKET_RC_INPROGRESS(rc) \
+ ((rc) == -EINPROGRESS)
+#endif
+
+static int inet_connect_addr(struct addrinfo *addr, bool block,
+ bool *in_progress)
{
- struct addrinfo ai,*res,*e;
+ int sock, rc;
+
+ if (in_progress) {
+ *in_progress = false;
+ }
+
+ sock = qemu_socket(addr->ai_family, addr->ai_socktype, addr->ai_protocol);
+ if (sock < 0) {
+ fprintf(stderr, "%s: socket(%s): %s\n", __func__,
+ inet_strfamily(addr->ai_family), strerror(errno));
+ return -1;
+ }
+ setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
+ if (!block) {
+ socket_set_nonblock(sock);
+ }
+ /* connect to peer */
+ do {
+ rc = 0;
+ if (connect(sock, addr->ai_addr, addr->ai_addrlen) < 0) {
+ rc = -socket_error();
+ }
+ } while (rc == -EINTR);
+
+ if (!block && QEMU_SOCKET_RC_INPROGRESS(rc)) {
+ if (in_progress) {
+ *in_progress = true;
+ }
+ } else if (rc < 0) {
+ closesocket(sock);
+ return -1;
+ }
+ return sock;
+}
+
+static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp)
+{
+ struct addrinfo ai, *res;
+ int rc;
const char *addr;
const char *port;
- char uaddr[INET6_ADDRSTRLEN+1];
- char uport[33];
- int sock,rc;
- bool block;
- memset(&ai,0, sizeof(ai));
+ memset(&ai, 0, sizeof(ai));
ai.ai_flags = AI_CANONNAME | AI_ADDRCONFIG;
ai.ai_family = PF_UNSPEC;
ai.ai_socktype = SOCK_STREAM;
- if (in_progress) {
- *in_progress = false;
- }
-
addr = qemu_opt_get(opts, "host");
port = qemu_opt_get(opts, "port");
- block = qemu_opt_get_bool(opts, "block", 0);
if (addr == NULL || port == NULL) {
- fprintf(stderr, "inet_connect: host and/or port not specified\n");
+ fprintf(stderr,
+ "inet_parse_connect_opts: host and/or port not specified\n");
error_set(errp, QERR_SOCKET_CREATE_FAILED);
- return -1;
+ return NULL;
}
- if (qemu_opt_get_bool(opts, "ipv4", 0))
+ if (qemu_opt_get_bool(opts, "ipv4", 0)) {
ai.ai_family = PF_INET;
- if (qemu_opt_get_bool(opts, "ipv6", 0))
+ }
+ if (qemu_opt_get_bool(opts, "ipv6", 0)) {
ai.ai_family = PF_INET6;
+ }
/* lookup */
- if (0 != (rc = getaddrinfo(addr, port, &ai, &res))) {
- fprintf(stderr,"getaddrinfo(%s,%s): %s\n", addr, port,
+ 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 -1;
+ return NULL;
+ }
+ return res;
+}
+
+int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp)
+{
+ struct addrinfo *res, *e;
+ int sock = -1;
+ bool block = qemu_opt_get_bool(opts, "block", 0);
+
+ res = inet_parse_connect_opts(opts, errp);
+ if (!res) {
+ return -1;
+ }
+
+ if (in_progress) {
+ *in_progress = false;
}
for (e = res; e != NULL; e = e->ai_next) {
- if (getnameinfo((struct sockaddr*)e->ai_addr,e->ai_addrlen,
- uaddr,INET6_ADDRSTRLEN,uport,32,
- NI_NUMERICHOST | NI_NUMERICSERV) != 0) {
- fprintf(stderr,"%s: getnameinfo: oops\n", __FUNCTION__);
- continue;
- }
- sock = qemu_socket(e->ai_family, e->ai_socktype, e->ai_protocol);
- if (sock < 0) {
- fprintf(stderr,"%s: socket(%s): %s\n", __FUNCTION__,
- inet_strfamily(e->ai_family), strerror(errno));
- continue;
- }
- setsockopt(sock,SOL_SOCKET,SO_REUSEADDR,(void*)&on,sizeof(on));
- if (!block) {
- socket_set_nonblock(sock);
- }
- /* connect to peer */
- do {
- rc = 0;
- if (connect(sock, e->ai_addr, e->ai_addrlen) < 0) {
- rc = -socket_error();
- }
- } while (rc == -EINTR);
-
- #ifdef _WIN32
- if (!block && (rc == -EINPROGRESS || rc == -EWOULDBLOCK
- || rc == -WSAEALREADY)) {
- #else
- if (!block && (rc == -EINPROGRESS)) {
- #endif
- if (in_progress) {
- *in_progress = true;
- }
- } else if (rc < 0) {
- if (NULL == e->ai_next)
- fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n", __FUNCTION__,
- inet_strfamily(e->ai_family),
- e->ai_canonname, uaddr, uport, strerror(errno));
- closesocket(sock);
- continue;
+ sock = inet_connect_addr(e, block, in_progress);
+ if (sock >= 0) {
+ break;
}
- freeaddrinfo(res);
- return sock;
}
- error_set(errp, QERR_SOCKET_CONNECT_FAILED);
+ if (sock < 0) {
+ error_set(errp, QERR_SOCKET_CONNECT_FAILED);
+ }
freeaddrinfo(res);
- return -1;
+ return sock;
}
int inet_dgram_opts(QemuOpts *opts)
--
1.7.7.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCH v5 2/4] Separate inet_connect into inet_connect (blocking) and inet_nonblocking_connect
2012-09-24 11:11 [Qemu-devel] [PATCH v5 0/4] non-blocking connect address handling cleanup Orit Wasserman
2012-09-24 11:11 ` [Qemu-devel] [PATCH v5 1/4] Refactor inet_connect_opts function Orit Wasserman
@ 2012-09-24 11:11 ` Orit Wasserman
2012-09-24 11:11 ` [Qemu-devel] [PATCH v5 3/4] Fix address handling in inet_nonblocking_connect Orit Wasserman
` (5 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Orit Wasserman @ 2012-09-24 11:11 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, aliguori, quintela, armbru, mst, mdroth, lcapitulino,
Orit Wasserman, pbonzini, akong
No need to add non blocking parameters to the blocking inet_connect
add block parameter for inet_connect_opts instead of using QemuOpt "block".
Signed-off-by: Orit Wasserman <owasserm@redhat.com>
---
migration-tcp.c | 2 +-
nbd.c | 2 +-
qemu-char.c | 2 +-
qemu-sockets.c | 58 +++++++++++++++++++++++++++++++++++++++++++++---------
qemu_socket.h | 7 ++++-
ui/vnc.c | 2 +-
6 files changed, 57 insertions(+), 16 deletions(-)
diff --git a/migration-tcp.c b/migration-tcp.c
index ac891c3..7f6ad98 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -88,7 +88,7 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
s->write = socket_write;
s->close = tcp_close;
- s->fd = inet_connect(host_port, false, &in_progress, errp);
+ s->fd = inet_nonblocking_connect(host_port, &in_progress, errp);
if (error_is_set(errp)) {
migrate_fd_error(s);
return -1;
diff --git a/nbd.c b/nbd.c
index 0dd60c5..206f75c 100644
--- a/nbd.c
+++ b/nbd.c
@@ -162,7 +162,7 @@ int tcp_socket_outgoing(const char *address, uint16_t port)
int tcp_socket_outgoing_spec(const char *address_and_port)
{
- return inet_connect(address_and_port, true, NULL, NULL);
+ return inet_connect(address_and_port, NULL);
}
int tcp_socket_incoming(const char *address, uint16_t port)
diff --git a/qemu-char.c b/qemu-char.c
index 7f0f895..13b87b5 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2456,7 +2456,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, NULL, NULL);
+ fd = inet_connect_opts(opts, true, NULL, NULL);
}
}
if (fd < 0) {
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 7ca0ac9..20bcb5e 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -54,9 +54,6 @@ static QemuOptsList dummy_opts = {
},{
.name = "ipv6",
.type = QEMU_OPT_BOOL,
- },{
- .name = "block",
- .type = QEMU_OPT_BOOL,
},
{ /* end if list */ }
},
@@ -294,11 +291,22 @@ static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp)
return res;
}
-int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp)
+/**
+ * Create a socket and connect it to an address.
+ *
+ * @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
+ *
+ * Returns: -1 on error, file descriptor on success.
+ */
+int inet_connect_opts(QemuOpts *opts, bool block, bool *in_progress,
+ Error **errp)
{
struct addrinfo *res, *e;
int sock = -1;
- bool block = qemu_opt_get_bool(opts, "block", 0);
res = inet_parse_connect_opts(opts, errp);
if (!res) {
@@ -515,17 +523,47 @@ int inet_listen(const char *str, char *ostr, int olen,
return sock;
}
-int inet_connect(const char *str, bool block, bool *in_progress, Error **errp)
+/**
+ * Create a blocking socket and connect it to an address.
+ *
+ * @str: address string
+ * @errp: set in case of an error
+ *
+ * Returns -1 in case of error, file descriptor on success
+ **/
+int inet_connect(const char *str, Error **errp)
{
QemuOpts *opts;
int sock = -1;
opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
if (inet_parse(opts, str) == 0) {
- if (block) {
- qemu_opt_set(opts, "block", "on");
- }
- sock = inet_connect_opts(opts, in_progress, errp);
+ sock = inet_connect_opts(opts, true, NULL, errp);
+ } else {
+ error_set(errp, QERR_SOCKET_CREATE_FAILED);
+ }
+ qemu_opts_del(opts);
+ return sock;
+}
+
+/**
+ * Create a non-blocking socket and connect it to an address.
+ *
+ * @str: address string
+ * @in_progress: set to true in case of ongoing connect
+ * @errp: set in case of an error
+ *
+ * Returns: -1 on error, file descriptor on success.
+ **/
+int inet_nonblocking_connect(const char *str, bool *in_progress,
+ Error **errp)
+{
+ QemuOpts *opts;
+ int sock = -1;
+
+ opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
+ if (inet_parse(opts, str) == 0) {
+ sock = inet_connect_opts(opts, false, in_progress, errp);
} else {
error_set(errp, QERR_SOCKET_CREATE_FAILED);
}
diff --git a/qemu_socket.h b/qemu_socket.h
index 30ae6af..80696aa 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -42,8 +42,11 @@ int send_all(int fd, const void *buf, int len1);
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, bool block, bool *in_progress, Error **errp);
+int inet_connect_opts(QemuOpts *opts, bool block, 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_dgram_opts(QemuOpts *opts);
const char *inet_strfamily(int family);
diff --git a/ui/vnc.c b/ui/vnc.c
index 385e345..01b2daf 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3061,7 +3061,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
if (strncmp(display, "unix:", 5) == 0)
vs->lsock = unix_connect(display+5);
else
- vs->lsock = inet_connect(display, true, NULL, NULL);
+ vs->lsock = inet_connect(display, NULL);
if (-1 == vs->lsock) {
g_free(vs->display);
vs->display = NULL;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCH v5 3/4] Fix address handling in inet_nonblocking_connect
2012-09-24 11:11 [Qemu-devel] [PATCH v5 0/4] non-blocking connect address handling cleanup Orit Wasserman
2012-09-24 11:11 ` [Qemu-devel] [PATCH v5 1/4] Refactor inet_connect_opts function Orit Wasserman
2012-09-24 11:11 ` [Qemu-devel] [PATCH v5 2/4] Separate inet_connect into inet_connect (blocking) and inet_nonblocking_connect Orit Wasserman
@ 2012-09-24 11:11 ` Orit Wasserman
2012-09-24 11:11 ` [Qemu-devel] [PATCH v5 4/4] Clear handler only for valid fd Orit Wasserman
` (4 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Orit Wasserman @ 2012-09-24 11:11 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, aliguori, quintela, armbru, mst, mdroth, lcapitulino,
Orit Wasserman, pbonzini, akong
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 | 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 7f6ad98..a15c2b8 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -53,54 +53,35 @@ 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);
}
}
int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
Error **errp)
{
- bool in_progress;
-
s->get_error = socket_errno;
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,
+ errp);
if (error_is_set(errp)) {
migrate_fd_error(s);
return -1;
}
- 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 0;
}
diff --git a/qemu-char.c b/qemu-char.c
index 13b87b5..b082bae 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2456,7 +2456,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 20bcb5e..3c568a4 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
@@ -214,14 +215,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) {
@@ -230,7 +283,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 */
@@ -241,10 +294,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;
@@ -260,6 +314,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;
@@ -296,36 +351,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;
}
@@ -538,7 +612,7 @@ int inet_connect(const char *str, Error **errp)
opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
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);
}
@@ -548,22 +622,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, NULL);
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 80696aa..3e8aee9 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -38,15 +38,21 @@ void socket_set_block(int fd);
void socket_set_nonblock(int fd);
int send_all(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.7.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCH v5 4/4] Clear handler only for valid fd
2012-09-24 11:11 [Qemu-devel] [PATCH v5 0/4] non-blocking connect address handling cleanup Orit Wasserman
` (2 preceding siblings ...)
2012-09-24 11:11 ` [Qemu-devel] [PATCH v5 3/4] Fix address handling in inet_nonblocking_connect Orit Wasserman
@ 2012-09-24 11:11 ` Orit Wasserman
2012-09-24 14:22 ` [Qemu-devel] [PATCH v5 0/4] non-blocking connect address handling cleanup Markus Armbruster
` (3 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Orit Wasserman @ 2012-09-24 11:11 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, aliguori, quintela, armbru, mst, mdroth, lcapitulino,
Orit Wasserman, pbonzini, akong
Signed-off-by: Orit Wasserman <owasserm@redhat.com>
---
migration.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/migration.c b/migration.c
index 1edeec5..22a05c4 100644
--- a/migration.c
+++ b/migration.c
@@ -240,7 +240,9 @@ static int migrate_fd_cleanup(MigrationState *s)
{
int ret = 0;
- qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+ if (s->fd != -1) {
+ qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+ }
if (s->file) {
DPRINTF("closing file\n");
--
1.7.7.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH v5 0/4] non-blocking connect address handling cleanup
2012-09-24 11:11 [Qemu-devel] [PATCH v5 0/4] non-blocking connect address handling cleanup Orit Wasserman
` (3 preceding siblings ...)
2012-09-24 11:11 ` [Qemu-devel] [PATCH v5 4/4] Clear handler only for valid fd Orit Wasserman
@ 2012-09-24 14:22 ` Markus Armbruster
2012-09-24 14:36 ` Michael S. Tsirkin
` (2 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2012-09-24 14:22 UTC (permalink / raw)
To: Orit Wasserman
Cc: kwolf, aliguori, akong, mst, quintela, mdroth, qemu-devel,
pbonzini, lcapitulino
Orit Wasserman <owasserm@redhat.com> writes:
> Changes from v4:
> - Rename ConnectHandler to NonBlockingConnectHandler
> - move function comments to functions definitions
> - move connect_state allocation to outside of the loop
> - fix comments text
>
> Changes from v3:
> - add missing parenthesis QEMU_SOCKET_RC_INPROGRESS macro
> - remove "block" from dummy_opts
> - remove in_progress from external API (inet_connect_opts and
> inet_nonblocking_connect)
> - Allocate ConnectState inside inet_connect_opts, this make the
> structure internal to qemu-sockets.c
> - fix migrate_fd_cleanup to handle invalid fd.
>
> Changes from v2:
> - remove the use of getnameinfo
> - remove errp for inet_connect_addr
> - remove QemuOpt "block"
> - fix errors in wait_for_connect
> - pass ConnectState as a parameter to allow concurrent connect ops
>
> 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.
> A simple way to reproduce the problem is migration:
> for the destination use -incoming tcp:0:4444, run migrate -d tcp:localhost:4444
> migration will fail on hosts that have both IPv4 and IPV6 address for localhost.
>
> To fix this, refactor address resolution code and make inet_nonblocking_connect
> retry connection with a different address.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH v5 0/4] non-blocking connect address handling cleanup
2012-09-24 11:11 [Qemu-devel] [PATCH v5 0/4] non-blocking connect address handling cleanup Orit Wasserman
` (4 preceding siblings ...)
2012-09-24 14:22 ` [Qemu-devel] [PATCH v5 0/4] non-blocking connect address handling cleanup Markus Armbruster
@ 2012-09-24 14:36 ` Michael S. Tsirkin
2012-09-24 14:37 ` Orit Wasserman
2012-09-25 1:54 ` Amos Kong
2012-09-26 12:37 ` Anthony Liguori
7 siblings, 1 reply; 10+ messages in thread
From: Michael S. Tsirkin @ 2012-09-24 14:36 UTC (permalink / raw)
To: Orit Wasserman
Cc: kwolf, aliguori, mdroth, quintela, armbru, qemu-devel,
lcapitulino, pbonzini, akong
On Mon, Sep 24, 2012 at 01:11:06PM +0200, Orit Wasserman wrote:
> Changes from v4:
> - Rename ConnectHandler to NonBlockingConnectHandler
> - move function comments to functions definitions
> - move connect_state allocation to outside of the loop
> - fix comments text
>
> Changes from v3:
> - add missing parenthesis QEMU_SOCKET_RC_INPROGRESS macro
> - remove "block" from dummy_opts
> - remove in_progress from external API (inet_connect_opts and
> inet_nonblocking_connect)
> - Allocate ConnectState inside inet_connect_opts, this make the
> structure internal to qemu-sockets.c
> - fix migrate_fd_cleanup to handle invalid fd.
>
> Changes from v2:
> - remove the use of getnameinfo
> - remove errp for inet_connect_addr
> - remove QemuOpt "block"
> - fix errors in wait_for_connect
> - pass ConnectState as a parameter to allow concurrent connect ops
>
> 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.
> A simple way to reproduce the problem is migration:
> for the destination use -incoming tcp:0:4444, run migrate -d tcp:localhost:4444
> migration will fail on hosts that have both IPv4 and IPV6 address for localhost.
>
> To fix this, refactor address resolution code and make inet_nonblocking_connect
> retry connection with a different address.
Acked-by: Michael S. Tsirkin <mst@redhat.com>
More ideas if you feel like touching this code some more:
- fix listen side: drop ADDRCONF hint and ideally make
it address independent
- enhance error handling so async connect errors propagate to
monitor somehow
> Michael S. Tsirkin (1):
> Refactor inet_connect_opts function
>
> Orit Wasserman (3):
> Separate inet_connect into inet_connect (blocking) and
> inet_nonblocking_connect
> Fix address handling in inet_nonblocking_connect
> Clear handler only for valid fd
>
> migration-tcp.c | 37 ++------
> migration.c | 4 +-
> nbd.c | 2 +-
> qemu-char.c | 2 +-
> qemu-sockets.c | 279 +++++++++++++++++++++++++++++++++++++++++--------------
> qemu_socket.h | 15 +++-
> ui/vnc.c | 2 +-
> 7 files changed, 237 insertions(+), 104 deletions(-)
>
> --
> 1.7.7.6
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH v5 0/4] non-blocking connect address handling cleanup
2012-09-24 14:36 ` Michael S. Tsirkin
@ 2012-09-24 14:37 ` Orit Wasserman
0 siblings, 0 replies; 10+ messages in thread
From: Orit Wasserman @ 2012-09-24 14:37 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: kwolf, aliguori, mdroth, quintela, armbru, qemu-devel,
lcapitulino, pbonzini, akong
On 09/24/2012 04:36 PM, Michael S. Tsirkin wrote:
> On Mon, Sep 24, 2012 at 01:11:06PM +0200, Orit Wasserman wrote:
>> Changes from v4:
>> - Rename ConnectHandler to NonBlockingConnectHandler
>> - move function comments to functions definitions
>> - move connect_state allocation to outside of the loop
>> - fix comments text
>>
>> Changes from v3:
>> - add missing parenthesis QEMU_SOCKET_RC_INPROGRESS macro
>> - remove "block" from dummy_opts
>> - remove in_progress from external API (inet_connect_opts and
>> inet_nonblocking_connect)
>> - Allocate ConnectState inside inet_connect_opts, this make the
>> structure internal to qemu-sockets.c
>> - fix migrate_fd_cleanup to handle invalid fd.
>>
>> Changes from v2:
>> - remove the use of getnameinfo
>> - remove errp for inet_connect_addr
>> - remove QemuOpt "block"
>> - fix errors in wait_for_connect
>> - pass ConnectState as a parameter to allow concurrent connect ops
>>
>> 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.
>> A simple way to reproduce the problem is migration:
>> for the destination use -incoming tcp:0:4444, run migrate -d tcp:localhost:4444
>> migration will fail on hosts that have both IPv4 and IPV6 address for localhost.
>>
>> To fix this, refactor address resolution code and make inet_nonblocking_connect
>> retry connection with a different address.
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> More ideas if you feel like touching this code some more:
> - fix listen side: drop ADDRCONF hint and ideally make
> it address independent
> - enhance error handling so async connect errors propagate to
> monitor somehow
I working on the second :)
>
>> Michael S. Tsirkin (1):
>> Refactor inet_connect_opts function
>>
>> Orit Wasserman (3):
>> Separate inet_connect into inet_connect (blocking) and
>> inet_nonblocking_connect
>> Fix address handling in inet_nonblocking_connect
>> Clear handler only for valid fd
>>
>> migration-tcp.c | 37 ++------
>> migration.c | 4 +-
>> nbd.c | 2 +-
>> qemu-char.c | 2 +-
>> qemu-sockets.c | 279 +++++++++++++++++++++++++++++++++++++++++--------------
>> qemu_socket.h | 15 +++-
>> ui/vnc.c | 2 +-
>> 7 files changed, 237 insertions(+), 104 deletions(-)
>>
>> --
>> 1.7.7.6
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v5 0/4] non-blocking connect address handling cleanup
2012-09-24 11:11 [Qemu-devel] [PATCH v5 0/4] non-blocking connect address handling cleanup Orit Wasserman
` (5 preceding siblings ...)
2012-09-24 14:36 ` Michael S. Tsirkin
@ 2012-09-25 1:54 ` Amos Kong
2012-09-26 12:37 ` Anthony Liguori
7 siblings, 0 replies; 10+ messages in thread
From: Amos Kong @ 2012-09-25 1:54 UTC (permalink / raw)
To: Orit Wasserman
Cc: kwolf, aliguori, mdroth, mst, quintela, armbru, qemu-devel,
pbonzini, lcapitulino
On 24/09/12 19:11, Orit Wasserman wrote:
> Changes from v4:
> - Rename ConnectHandler to NonBlockingConnectHandler
> - move function comments to functions definitions
> - move connect_state allocation to outside of the loop
> - fix comments text
>
> Changes from v3:
> - add missing parenthesis QEMU_SOCKET_RC_INPROGRESS macro
> - remove "block" from dummy_opts
> - remove in_progress from external API (inet_connect_opts and
> inet_nonblocking_connect)
> - Allocate ConnectState inside inet_connect_opts, this make the
> structure internal to qemu-sockets.c
> - fix migrate_fd_cleanup to handle invalid fd.
>
> Changes from v2:
> - remove the use of getnameinfo
> - remove errp for inet_connect_addr
> - remove QemuOpt "block"
> - fix errors in wait_for_connect
> - pass ConnectState as a parameter to allow concurrent connect ops
>
> 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.
> A simple way to reproduce the problem is migration:
> for the destination use -incoming tcp:0:4444, run migrate -d tcp:localhost:4444
> migration will fail on hosts that have both IPv4 and IPV6 address for localhost.
>
> To fix this, refactor address resolution code and make inet_nonblocking_connect
> retry connection with a different address.
Reviewed-by: Amos Kong <akong@redhat.com>
Tested-by: Amos Kong <akong@redhat.com>
--
Amos.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH v5 0/4] non-blocking connect address handling cleanup
2012-09-24 11:11 [Qemu-devel] [PATCH v5 0/4] non-blocking connect address handling cleanup Orit Wasserman
` (6 preceding siblings ...)
2012-09-25 1:54 ` Amos Kong
@ 2012-09-26 12:37 ` Anthony Liguori
7 siblings, 0 replies; 10+ messages in thread
From: Anthony Liguori @ 2012-09-26 12:37 UTC (permalink / raw)
To: Orit Wasserman, qemu-devel
Cc: kwolf, quintela, armbru, mst, mdroth, lcapitulino, pbonzini,
akong
Orit Wasserman <owasserm@redhat.com> writes:
> Changes from v4:
> - Rename ConnectHandler to NonBlockingConnectHandler
> - move function comments to functions definitions
> - move connect_state allocation to outside of the loop
> - fix comments text
>
> Changes from v3:
> - add missing parenthesis QEMU_SOCKET_RC_INPROGRESS macro
> - remove "block" from dummy_opts
> - remove in_progress from external API (inet_connect_opts and
> inet_nonblocking_connect)
> - Allocate ConnectState inside inet_connect_opts, this make the
> structure internal to qemu-sockets.c
> - fix migrate_fd_cleanup to handle invalid fd.
>
> Changes from v2:
> - remove the use of getnameinfo
> - remove errp for inet_connect_addr
> - remove QemuOpt "block"
> - fix errors in wait_for_connect
> - pass ConnectState as a parameter to allow concurrent connect ops
>
> 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.
> A simple way to reproduce the problem is migration:
> for the destination use -incoming tcp:0:4444, run migrate -d tcp:localhost:4444
> migration will fail on hosts that have both IPv4 and IPV6 address for localhost.
>
> To fix this, refactor address resolution code and make inet_nonblocking_connect
> retry connection with a different address.
>
> Michael S. Tsirkin (1):
> Refactor inet_connect_opts function
>
> Orit Wasserman (3):
> Separate inet_connect into inet_connect (blocking) and
> inet_nonblocking_connect
> Fix address handling in inet_nonblocking_connect
> Clear handler only for valid fd
>
> migration-tcp.c | 37 ++------
> migration.c | 4 +-
> nbd.c | 2 +-
> qemu-char.c | 2 +-
> qemu-sockets.c | 279 +++++++++++++++++++++++++++++++++++++++++--------------
> qemu_socket.h | 15 +++-
> ui/vnc.c | 2 +-
> 7 files changed, 237 insertions(+), 104 deletions(-)
Applied. Thanks.
Regards,
Anthony Liguori
>
> --
> 1.7.7.6
^ permalink raw reply [flat|nested] 10+ messages in thread