The non-blocking connect mechanism is obsolete, and it doesn't work well in inet connection, because it will call getaddrinfo first and getaddrinfo will blocks on DNS lookups. Since commit e65c67e4 & d984464e, the non-blocking connect of migration goes through QIOChannel in a different manner(using a thread), and nobody use this old non-blocking connect anymore.
Any newly written code which needs a non-blocking connect should use the QIOChannel code, so we can just rip out NonBlockingConnectHandler as a concept entirely. Suggested-by: Daniel P. Berrange <[email protected]> Cc: Daniel P. Berrange <[email protected]> Cc: Gerd Hoffmann <[email protected]> Cc: Paolo Bonzini <[email protected]> Signed-off-by: Cao jin <[email protected]> --- include/qemu/sockets.h | 7 +-- io/channel-socket.c | 2 +- net/socket.c | 2 +- util/qemu-sockets.c | 163 +++++-------------------------------------------- 4 files changed, 19 insertions(+), 155 deletions(-) diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h index 9eb2470..9e7c322 100644 --- a/include/qemu/sockets.h +++ b/include/qemu/sockets.h @@ -27,10 +27,6 @@ int socket_set_fast_reuse(int fd); #define SHUT_RDWR 2 #endif -/* callback function for nonblocking connect - * valid fd on success, negative error code on failure - */ -typedef void NonBlockingConnectHandler(int fd, Error *err, void *opaque); InetSocketAddress *inet_parse(const char *str, Error **errp); int inet_connect(const char *str, Error **errp); @@ -41,8 +37,7 @@ int unix_listen(const char *path, char *ostr, int olen, Error **errp); int unix_connect(const char *path, Error **errp); SocketAddress *socket_parse(const char *str, Error **errp); -int socket_connect(SocketAddress *addr, Error **errp, - NonBlockingConnectHandler *callback, void *opaque); +int socket_connect(SocketAddress *addr, Error **errp); int socket_listen(SocketAddress *addr, Error **errp); void socket_listen_cleanup(int fd, Error **errp); int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp); diff --git a/io/channel-socket.c b/io/channel-socket.c index 196a4f1..6aa0ad2 100644 --- a/io/channel-socket.c +++ b/io/channel-socket.c @@ -147,7 +147,7 @@ int qio_channel_socket_connect_sync(QIOChannelSocket *ioc, int fd; trace_qio_channel_socket_connect_sync(ioc, addr); - fd = socket_connect(addr, errp, NULL, NULL); + fd = socket_connect(addr, errp); if (fd < 0) { trace_qio_channel_socket_connect_fail(ioc); return -1; diff --git a/net/socket.c b/net/socket.c index ae6f921..8037b3c 100644 --- a/net/socket.c +++ b/net/socket.c @@ -541,7 +541,7 @@ static int net_socket_connect_init(NetClientState *peer, qemu_set_nonblock(fd); connected = 0; for(;;) { - ret = socket_connect(saddr, &local_error, NULL, NULL); + ret = socket_connect(saddr, &local_error); if (ret < 0) { if (errno == EINTR || errno == EWOULDBLOCK) { /* continue */ diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index b4314ca..cd4ed55 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -239,96 +239,18 @@ listen: return slisten; } -#ifdef _WIN32 -#define QEMU_SOCKET_RC_INPROGRESS(rc) \ - ((rc) == -EINPROGRESS || (rc) == -EWOULDBLOCK || (rc) == -WSAEALREADY) -#else -#define QEMU_SOCKET_RC_INPROGRESS(rc) \ - ((rc) == -EINPROGRESS) -#endif - -/* 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, Error **errp); - -static void wait_for_connect(void *opaque) -{ - ConnectState *s = opaque; - int val = 0, rc = 0; - socklen_t valsize = sizeof(val); - bool in_progress; - Error *err = NULL; - - qemu_set_fd_handler(s->fd, NULL, NULL, NULL); - - do { - rc = qemu_getsockopt(s->fd, SOL_SOCKET, SO_ERROR, &val, &valsize); - } while (rc == -1 && errno == EINTR); - - /* update rc to contain error */ - if (!rc && val) { - rc = -1; - errno = val; - } - - /* connect error */ - if (rc < 0) { - error_setg_errno(&err, errno, "Error connecting to socket"); - closesocket(s->fd); - s->fd = rc; - } - - /* try to connect to the next address on the list */ - if (s->current_addr) { - 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, NULL); - if (s->fd < 0) { - error_free(err); - err = NULL; - error_setg_errno(&err, errno, "Unable to start socket connect"); - } - /* connect in progress */ - if (in_progress) { - goto out; - } - } - - freeaddrinfo(s->addr_list); - } - if (s->callback) { - s->callback(s->fd, err, s->opaque); - } - g_free(s); -out: - error_free(err); -} - -static int inet_connect_addr(struct addrinfo *addr, bool *in_progress, - ConnectState *connect_state, Error **errp) +static int inet_connect_addr(struct addrinfo *addr, Error **errp) { int sock, rc; - *in_progress = false; - sock = qemu_socket(addr->ai_family, addr->ai_socktype, addr->ai_protocol); if (sock < 0) { error_setg_errno(errp, errno, "Failed to create socket"); return -1; } socket_set_fast_reuse(sock); - if (connect_state != NULL) { - qemu_set_nonblock(sock); - } + /* connect to peer */ do { rc = 0; @@ -337,15 +259,12 @@ static int inet_connect_addr(struct addrinfo *addr, bool *in_progress, } } while (rc == -EINTR); - if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) { - connect_state->fd = sock; - qemu_set_fd_handler(sock, NULL, wait_for_connect, connect_state); - *in_progress = true; - } else if (rc < 0) { + if (rc < 0) { error_setg_errno(errp, errno, "Failed to connect socket"); closesocket(sock); return -1; } + return sock; } @@ -403,43 +322,25 @@ static struct addrinfo *inet_parse_connect_saddr(InetSocketAddress *saddr, * * @saddr: Inet socket address specification * @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. */ -static int inet_connect_saddr(InetSocketAddress *saddr, Error **errp, - NonBlockingConnectHandler *callback, void *opaque) +static int inet_connect_saddr(InetSocketAddress *saddr, Error **errp) { Error *local_err = NULL; struct addrinfo *res, *e; int sock = -1; - bool in_progress; - ConnectState *connect_state = NULL; res = inet_parse_connect_saddr(saddr, errp); if (!res) { return -1; } - 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) { error_free(local_err); local_err = NULL; - if (connect_state != NULL) { - connect_state->current_addr = e; - } - sock = inet_connect_addr(e, &in_progress, connect_state, &local_err); + + sock = inet_connect_addr(e, &local_err); if (sock >= 0) { break; } @@ -447,16 +348,10 @@ static int inet_connect_saddr(InetSocketAddress *saddr, Error **errp, if (sock < 0) { error_propagate(errp, local_err); - } else if (in_progress) { - /* wait_for_connect() will do the rest */ - return sock; - } else { - if (callback) { - callback(sock, NULL, opaque); - } } - g_free(connect_state); + freeaddrinfo(res); + return sock; } @@ -640,7 +535,7 @@ int inet_connect(const char *str, Error **errp) addr = inet_parse(str, errp); if (addr != NULL) { - sock = inet_connect_saddr(addr, errp, NULL, NULL); + sock = inet_connect_saddr(addr, errp); qapi_free_InetSocketAddress(addr); } return sock; @@ -716,11 +611,9 @@ err: return -1; } -static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp, - NonBlockingConnectHandler *callback, void *opaque) +static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp) { struct sockaddr_un un; - ConnectState *connect_state = NULL; int sock, rc; if (saddr->path == NULL) { @@ -733,12 +626,6 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp, error_setg_errno(errp, errno, "Failed to create socket"); return -1; } - if (callback != NULL) { - connect_state = g_malloc0(sizeof(*connect_state)); - connect_state->callback = callback; - connect_state->opaque = opaque; - qemu_set_nonblock(sock); - } memset(&un, 0, sizeof(un)); un.sun_family = AF_UNIX; @@ -752,24 +639,12 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp, } } while (rc == -EINTR); - if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) { - connect_state->fd = sock; - qemu_set_fd_handler(sock, NULL, wait_for_connect, connect_state); - return sock; - } else if (rc >= 0) { - /* non blocking socket immediate success, call callback */ - if (callback != NULL) { - callback(sock, NULL, opaque); - } - } - if (rc < 0) { error_setg_errno(errp, -rc, "Failed to connect socket"); close(sock); sock = -1; } - g_free(connect_state); return sock; } @@ -784,8 +659,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr, return -1; } -static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp, - NonBlockingConnectHandler *callback, void *opaque) +static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp) { error_setg(errp, "unix sockets are not available on windows"); errno = ENOTSUP; @@ -829,7 +703,7 @@ int unix_connect(const char *path, Error **errp) saddr = g_new0(UnixSocketAddress, 1); saddr->path = g_strdup(path); - sock = unix_connect_saddr(saddr, errp, NULL, NULL); + sock = unix_connect_saddr(saddr, errp); qapi_free_UnixSocketAddress(saddr); return sock; } @@ -872,26 +746,21 @@ fail: return NULL; } -int socket_connect(SocketAddress *addr, Error **errp, - NonBlockingConnectHandler *callback, void *opaque) +int socket_connect(SocketAddress *addr, Error **errp) { int fd; switch (addr->type) { case SOCKET_ADDRESS_KIND_INET: - fd = inet_connect_saddr(addr->u.inet.data, errp, callback, opaque); + fd = inet_connect_saddr(addr->u.inet.data, errp); break; case SOCKET_ADDRESS_KIND_UNIX: - fd = unix_connect_saddr(addr->u.q_unix.data, errp, callback, opaque); + fd = unix_connect_saddr(addr->u.q_unix.data, errp); break; case SOCKET_ADDRESS_KIND_FD: fd = monitor_get_fd(cur_mon, addr->u.fd.data->str, errp); - if (fd >= 0 && callback) { - qemu_set_nonblock(fd); - callback(fd, NULL, opaque); - } break; default: -- 2.1.0
