From nobody Wed Nov 5 12:56:14 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 14976035321601003.3515986313369; Fri, 16 Jun 2017 01:58:52 -0700 (PDT) Received: from localhost ([::1]:57722 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dLn5e-0005LJ-Nw for importer@patchew.org; Fri, 16 Jun 2017 04:58:50 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47081) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dLn4U-0004WQ-3z for qemu-devel@nongnu.org; Fri, 16 Jun 2017 04:57:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dLn4S-0006BJ-52 for qemu-devel@nongnu.org; Fri, 16 Jun 2017 04:57:38 -0400 Received: from [59.151.112.132] (port=14864 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dLn4I-00065L-RB; Fri, 16 Jun 2017 04:57:28 -0400 Received: from unknown (HELO cn.fujitsu.com) ([10.167.33.5]) by heian.cn.fujitsu.com with ESMTP; 16 Jun 2017 16:57:23 +0800 Received: from G08CNEXCHPEKD02.g08.fujitsu.local (unknown [10.167.33.83]) by cn.fujitsu.com (Postfix) with ESMTP id AC89747C7C61; Fri, 16 Jun 2017 16:57:24 +0800 (CST) Received: from maozy.g08.fujitsu.local (10.167.225.76) by G08CNEXCHPEKD02.g08.fujitsu.local (10.167.33.89) with Microsoft SMTP Server (TLS) id 14.3.319.2; Fri, 16 Jun 2017 16:57:22 +0800 X-IronPort-AV: E=Sophos;i="5.22,518,1449504000"; d="scan'208";a="20114332" From: Mao Zhongyi To: Date: Fri, 16 Jun 2017 16:54:45 +0800 Message-ID: <7fb8c9ff25d183cf7c0f729a90ee4f59c0632d64.1497602554.git.maozy.fnst@cn.fujitsu.com> X-Mailer: git-send-email 2.9.3 MIME-Version: 1.0 X-Originating-IP: [10.167.225.76] X-yoursite-MailScanner-ID: AC89747C7C61.A5AA3 X-yoursite-MailScanner: Found to be clean X-yoursite-MailScanner-From: maozy.fnst@cn.fujitsu.com X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 59.151.112.132 Subject: [Qemu-devel] [PATCH v3] util: remove the obsolete non-blocking connect X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, qemu-block@nongnu.org, quintela@redhat.com, mitake.hitoshi@lab.ntt.co.jp, jcody@redhat.com, Cao jin , mreitz@redhat.com, sheepdog@lists.wpkg.org, kraxel@redhat.com, pbonzini@redhat.com, famz@redhat.com, namei.unix@gmail.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Cao jin 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 drop NonBlockingConnectHandler as a concept entirely. Suggested-by: Daniel P. Berrange Signed-off-by: Cao jin Signed-off-by: Mao Zhongyi Reviewed-by: Juan Quintela Reviewed-by: Daniel P. Berrange --- v3: 1. fix the mistake that adds a unmodified file '--help' [Fam Zheng] v2: 1. block/ssh.c is not compiled in v1 which leads to no error reported when I run make & make check, although I make a mistake in block/ssh.c, because the package libssh2-devel missed in my system. Now fixed it. 2. Write the prototype in a single line. [Juan Quintela] Because it's a minor change, so still add the r-b. v1: This patch was reviewed by Daniel about a years ago, but it has never been merged just since socket_connect() called by net_socket_connect_init() where NonBlockingConnectHandler was passed to socket_connect(). it's broken. Now this problem was worked around by Daniel's patch(commit 6701e551 'Revert "Change net/socket.c to use socket_*() functions" again'). Therefore, resend it, of course, part of related code has changed over the years, so changed the patch accordingly. The reviewed patch listed on: https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg06373.html block/sheepdog.c | 2 +- block/ssh.c | 2 +- include/qemu/sockets.h | 12 +-- io/channel-socket.c | 2 +- util/qemu-sockets.c | 205 ++++++---------------------------------------= ---- 5 files changed, 28 insertions(+), 195 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index a18315a..97e6c96 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -589,7 +589,7 @@ static int connect_to_sdog(BDRVSheepdogState *s, Error = **errp) { int fd; =20 - fd =3D socket_connect(s->addr, NULL, NULL, errp); + fd =3D socket_connect(s->addr, errp); =20 if (s->addr->type =3D=3D SOCKET_ADDRESS_TYPE_INET && fd >=3D 0) { int ret =3D socket_set_nodelay(fd); diff --git a/block/ssh.c b/block/ssh.c index 11203fc..d6a82ae 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -679,7 +679,7 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *optio= ns, } =20 /* Open the socket and connect. */ - s->sock =3D inet_connect_saddr(s->inet, NULL, NULL, errp); + s->sock =3D inet_connect_saddr(s->inet, errp); if (s->sock < 0) { ret =3D -EIO; goto err; diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h index 5c326db..dce0e41 100644 --- a/include/qemu/sockets.h +++ b/include/qemu/sockets.h @@ -27,18 +27,11 @@ int socket_set_fast_reuse(int fd); #define SHUT_RDWR 2 #endif =20 -/* callback function for nonblocking connect - * valid fd on success, negative error code on failure - */ -typedef void NonBlockingConnectHandler(int fd, Error *err, void *opaque); - int inet_ai_family_from_address(InetSocketAddress *addr, Error **errp); int inet_parse(InetSocketAddress *addr, const char *str, Error **errp); int inet_connect(const char *str, Error **errp); -int inet_connect_saddr(InetSocketAddress *saddr, - NonBlockingConnectHandler *callback, void *opaque, - Error **errp); +int inet_connect_saddr(InetSocketAddress *saddr, Error **errp); =20 NetworkAddressFamily inet_netfamily(int family); =20 @@ -46,8 +39,7 @@ int unix_listen(const char *path, char *ostr, int olen, E= rror **errp); int unix_connect(const char *path, Error **errp); =20 SocketAddress *socket_parse(const char *str, Error **errp); -int socket_connect(SocketAddress *addr, NonBlockingConnectHandler *callbac= k, - void *opaque, Error **errp); +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 53386b7..05ea120 100644 --- a/io/channel-socket.c +++ b/io/channel-socket.c @@ -140,7 +140,7 @@ int qio_channel_socket_connect_sync(QIOChannelSocket *i= oc, int fd; =20 trace_qio_channel_socket_connect_sync(ioc, addr); - fd =3D socket_connect(addr, NULL, NULL, errp); + fd =3D socket_connect(addr, errp); if (fd < 0) { trace_qio_channel_socket_connect_fail(ioc); return -1; diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index 82290cb..af14f33 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -263,88 +263,19 @@ listen: ((rc) =3D=3D -EINPROGRESS) #endif =20 -/* 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 int inet_connect_addr(struct addrinfo *addr, Error **errp); =20 -static void wait_for_connect(void *opaque) -{ - ConnectState *s =3D opaque; - int val =3D 0, rc =3D 0; - socklen_t valsize =3D sizeof(val); - bool in_progress; - Error *err =3D NULL; - - qemu_set_fd_handler(s->fd, NULL, NULL, NULL); - - do { - rc =3D qemu_getsockopt(s->fd, SOL_SOCKET, SO_ERROR, &val, &valsize= ); - } while (rc =3D=3D -1 && errno =3D=3D EINTR); - - /* update rc to contain error */ - if (!rc && val) { - rc =3D -1; - errno =3D val; - } - - /* connect error */ - if (rc < 0) { - error_setg_errno(&err, errno, "Error connecting to socket"); - closesocket(s->fd); - s->fd =3D rc; - } - - /* try to connect to the next address on the list */ - if (s->current_addr) { - while (s->current_addr->ai_next !=3D NULL && s->fd < 0) { - s->current_addr =3D s->current_addr->ai_next; - s->fd =3D inet_connect_addr(s->current_addr, &in_progress, s, = NULL); - if (s->fd < 0) { - error_free(err); - err =3D NULL; - error_setg_errno(&err, errno, "Unable to start socket conn= ect"); - } - /* 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; =20 - *in_progress =3D false; - sock =3D qemu_socket(addr->ai_family, addr->ai_socktype, addr->ai_prot= ocol); if (sock < 0) { error_setg_errno(errp, errno, "Failed to create socket"); return -1; } socket_set_fast_reuse(sock); - if (connect_state !=3D NULL) { - qemu_set_nonblock(sock); - } + /* connect to peer */ do { rc =3D 0; @@ -353,15 +284,12 @@ static int inet_connect_addr(struct addrinfo *addr, b= ool *in_progress, } } while (rc =3D=3D -EINTR); =20 - if (connect_state !=3D NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) { - connect_state->fd =3D sock; - qemu_set_fd_handler(sock, NULL, wait_for_connect, connect_state); - *in_progress =3D true; - } else if (rc < 0) { + if (rc < 0) { error_setg_errno(errp, errno, "Failed to connect socket"); closesocket(sock); return -1; } + return sock; } =20 @@ -419,44 +347,24 @@ static struct addrinfo *inet_parse_connect_saddr(Inet= SocketAddress *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. */ -int inet_connect_saddr(InetSocketAddress *saddr, - NonBlockingConnectHandler *callback, void *opaque, - Error **errp) +int inet_connect_saddr(InetSocketAddress *saddr, Error **errp) { Error *local_err =3D NULL; struct addrinfo *res, *e; int sock =3D -1; - bool in_progress; - ConnectState *connect_state =3D NULL; =20 res =3D inet_parse_connect_saddr(saddr, errp); if (!res) { return -1; } =20 - if (callback !=3D NULL) { - connect_state =3D g_malloc0(sizeof(*connect_state)); - connect_state->addr_list =3D res; - connect_state->callback =3D callback; - connect_state->opaque =3D opaque; - } - for (e =3D res; e !=3D NULL; e =3D e->ai_next) { error_free(local_err); local_err =3D NULL; - if (connect_state !=3D NULL) { - connect_state->current_addr =3D e; - } - sock =3D inet_connect_addr(e, &in_progress, connect_state, &local_= err); + sock =3D inet_connect_addr(e, &local_err); if (sock >=3D 0) { break; } @@ -464,15 +372,8 @@ int inet_connect_saddr(InetSocketAddress *saddr, =20 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; } @@ -655,7 +556,7 @@ int inet_connect(const char *str, Error **errp) InetSocketAddress *addr =3D g_new(InetSocketAddress, 1); =20 if (!inet_parse(addr, str, errp)) { - sock =3D inet_connect_saddr(addr, NULL, NULL, errp); + sock =3D inet_connect_saddr(addr, errp); } qapi_free_InetSocketAddress(addr); return sock; @@ -688,21 +589,16 @@ static bool vsock_parse_vaddr_to_sockaddr(const Vsock= SocketAddress *vaddr, return true; } =20 -static int vsock_connect_addr(const struct sockaddr_vm *svm, bool *in_prog= ress, - ConnectState *connect_state, Error **errp) +static int vsock_connect_addr(const struct sockaddr_vm *svm, Error **errp) { int sock, rc; =20 - *in_progress =3D false; - sock =3D qemu_socket(AF_VSOCK, SOCK_STREAM, 0); if (sock < 0) { error_setg_errno(errp, errno, "Failed to create socket"); return -1; } - if (connect_state !=3D NULL) { - qemu_set_nonblock(sock); - } + /* connect to peer */ do { rc =3D 0; @@ -711,50 +607,26 @@ static int vsock_connect_addr(const struct sockaddr_v= m *svm, bool *in_progress, } } while (rc =3D=3D -EINTR); =20 - if (connect_state !=3D NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) { - connect_state->fd =3D sock; - qemu_set_fd_handler(sock, NULL, wait_for_connect, connect_state); - *in_progress =3D true; - } else if (rc < 0) { + if (rc < 0) { error_setg_errno(errp, errno, "Failed to connect socket"); closesocket(sock); return -1; } + return sock; } =20 -static int vsock_connect_saddr(VsockSocketAddress *vaddr, - NonBlockingConnectHandler *callback, - void *opaque, - Error **errp) +static int vsock_connect_saddr(VsockSocketAddress *vaddr, Error **errp) { struct sockaddr_vm svm; int sock =3D -1; - bool in_progress; - ConnectState *connect_state =3D NULL; =20 if (!vsock_parse_vaddr_to_sockaddr(vaddr, &svm, errp)) { return -1; } =20 - if (callback !=3D NULL) { - connect_state =3D g_malloc0(sizeof(*connect_state)); - connect_state->callback =3D callback; - connect_state->opaque =3D opaque; - } + sock =3D vsock_connect_addr(&svm, errp); =20 - sock =3D vsock_connect_addr(&svm, &in_progress, connect_state, errp); - if (sock < 0) { - /* do nothing */ - } else if (in_progress) { - /* wait_for_connect() will do the rest */ - return sock; - } else { - if (callback) { - callback(sock, NULL, opaque); - } - } - g_free(connect_state); return sock; } =20 @@ -814,9 +686,7 @@ static void vsock_unsupported(Error **errp) error_setg(errp, "socket family AF_VSOCK unsupported"); } =20 -static int vsock_connect_saddr(VsockSocketAddress *vaddr, - NonBlockingConnectHandler *callback, - void *opaque, Error **errp) +static int vsock_connect_saddr(VsockSocketAddress *vaddr, Error **errp) { vsock_unsupported(errp); return -1; @@ -919,12 +789,9 @@ err: return -1; } =20 -static int unix_connect_saddr(UnixSocketAddress *saddr, - NonBlockingConnectHandler *callback, void *o= paque, - Error **errp) +static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp) { struct sockaddr_un un; - ConnectState *connect_state =3D NULL; int sock, rc; =20 if (saddr->path =3D=3D NULL) { @@ -937,12 +804,6 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, error_setg_errno(errp, errno, "Failed to create socket"); return -1; } - if (callback !=3D NULL) { - connect_state =3D g_malloc0(sizeof(*connect_state)); - connect_state->callback =3D callback; - connect_state->opaque =3D opaque; - qemu_set_nonblock(sock); - } =20 if (strlen(saddr->path) > sizeof(un.sun_path)) { error_setg(errp, "UNIX socket path '%s' is too long", saddr->path); @@ -963,29 +824,16 @@ static int unix_connect_saddr(UnixSocketAddress *sadd= r, } } while (rc =3D=3D -EINTR); =20 - if (connect_state !=3D NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) { - connect_state->fd =3D sock; - qemu_set_fd_handler(sock, NULL, wait_for_connect, connect_state); - return sock; - } else if (rc >=3D 0) { - /* non blocking socket immediate success, call callback */ - if (callback !=3D NULL) { - callback(sock, NULL, opaque); - } - } - if (rc < 0) { error_setg_errno(errp, -rc, "Failed to connect socket %s", saddr->path); goto err; } =20 - g_free(connect_state); return sock; =20 err: close(sock); - g_free(connect_state); return -1; } =20 @@ -1000,9 +848,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr, return -1; } =20 -static int unix_connect_saddr(UnixSocketAddress *saddr, - NonBlockingConnectHandler *callback, void *o= paque, - Error **errp) +static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp) { error_setg(errp, "unix sockets are not available on windows"); errno =3D ENOTSUP; @@ -1048,7 +894,7 @@ int unix_connect(const char *path, Error **errp) =20 saddr =3D g_new0(UnixSocketAddress, 1); saddr->path =3D g_strdup(path); - sock =3D unix_connect_saddr(saddr, NULL, NULL, errp); + sock =3D unix_connect_saddr(saddr, errp); qapi_free_UnixSocketAddress(saddr); return sock; } @@ -1093,30 +939,25 @@ fail: return NULL; } =20 -int socket_connect(SocketAddress *addr, NonBlockingConnectHandler *callbac= k, - void *opaque, Error **errp) +int socket_connect(SocketAddress *addr, Error **errp) { int fd; =20 switch (addr->type) { case SOCKET_ADDRESS_TYPE_INET: - fd =3D inet_connect_saddr(&addr->u.inet, callback, opaque, errp); + fd =3D inet_connect_saddr(&addr->u.inet, errp); break; =20 case SOCKET_ADDRESS_TYPE_UNIX: - fd =3D unix_connect_saddr(&addr->u.q_unix, callback, opaque, errp); + fd =3D unix_connect_saddr(&addr->u.q_unix, errp); break; =20 case SOCKET_ADDRESS_TYPE_FD: fd =3D monitor_get_fd(cur_mon, addr->u.fd.str, errp); - if (fd >=3D 0 && callback) { - qemu_set_nonblock(fd); - callback(fd, NULL, opaque); - } break; =20 case SOCKET_ADDRESS_TYPE_VSOCK: - fd =3D vsock_connect_saddr(&addr->u.vsock, callback, opaque, errp); + fd =3D vsock_connect_saddr(&addr->u.vsock, errp); break; =20 default: --=20 2.9.3