{"id":810047,"url":"http://patchwork.ozlabs.org/api/patches/810047/?format=json","web_url":"http://patchwork.ozlabs.org/project/qemu-devel/patch/20170905092230.8243-7-berrange@redhat.com/","project":{"id":14,"url":"http://patchwork.ozlabs.org/api/projects/14/?format=json","name":"QEMU Development","link_name":"qemu-devel","list_id":"qemu-devel.nongnu.org","list_email":"qemu-devel@nongnu.org","web_url":"","scm_url":"","webscm_url":"","list_archive_url":"","list_archive_url_format":"","commit_url_format":""},"msgid":"<20170905092230.8243-7-berrange@redhat.com>","list_archive_url":null,"date":"2017-09-05T09:22:27","name":"[PULL,v1,6/9] util: remove the obsolete non-blocking connect","commit_ref":null,"pull_url":null,"state":"new","archived":false,"hash":"aa0a121d23fd37f40e816a5ffba5449617d21517","submitter":{"id":2694,"url":"http://patchwork.ozlabs.org/api/people/2694/?format=json","name":"Daniel P. Berrangé","email":"berrange@redhat.com"},"delegate":null,"mbox":"http://patchwork.ozlabs.org/project/qemu-devel/patch/20170905092230.8243-7-berrange@redhat.com/mbox/","series":[{"id":1527,"url":"http://patchwork.ozlabs.org/api/series/1527/?format=json","web_url":"http://patchwork.ozlabs.org/project/qemu-devel/list/?series=1527","date":"2017-09-05T09:22:21","name":"[PULL,v1,1/9] io: fix temp directory used by test-io-channel-tls test","version":1,"mbox":"http://patchwork.ozlabs.org/series/1527/mbox/"}],"comments":"http://patchwork.ozlabs.org/api/patches/810047/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/810047/checks/","tags":{},"related":[],"headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=berrange@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xmhGV0b1Dz9s75\n\tfor ;\n\tTue, 5 Sep 2017 19:28:16 +1000 (AEST)","from localhost ([::1]:57684 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dpA9W-0005VC-7a\n\tfor incoming@patchwork.ozlabs.org; Tue, 05 Sep 2017 05:28:14 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:41236)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dpA4H-0001EE-NN\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:22:55 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dpA4B-0002lO-Kj\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:22:49 -0400","from mx1.redhat.com ([209.132.183.28]:2282)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dpA4B-0002kR-Bi\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:22:43 -0400","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 64F7CC04B924;\n\tTue, 5 Sep 2017 09:22:42 +0000 (UTC)","from t460.redhat.com (unknown [10.33.36.63])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 0254C60BEC;\n\tTue, 5 Sep 2017 09:22:40 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 64F7CC04B924","From":"\"Daniel P. Berrange\" ","To":"qemu-devel@nongnu.org","Date":"Tue, 5 Sep 2017 10:22:27 +0100","Message-Id":"<20170905092230.8243-7-berrange@redhat.com>","In-Reply-To":"<20170905092230.8243-1-berrange@redhat.com>","References":"<20170905092230.8243-1-berrange@redhat.com>","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.31]);\n\tTue, 05 Sep 2017 09:22:42 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"[Qemu-devel] [PULL v1 6/9] util: remove the obsolete non-blocking\n\tconnect","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Peter Maydell ,\n\tCao jin ,\n\tMao Zhongyi ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"},"content":"From: Cao jin \n\nThe non-blocking connect mechanism is obsolete, and it doesn't\nwork well in inet connection, because it will call getaddrinfo\nfirst and getaddrinfo will blocks on DNS lookups. Since commit\ne65c67e4 & d984464e, the non-blocking connect of migration goes\nthrough QIOChannel in a different manner(using a thread), and\nnobody use this old non-blocking connect anymore.\n\nAny newly written code which needs a non-blocking connect should\nuse the QIOChannel code, so we can drop NonBlockingConnectHandler\nas a concept entirely.\n\nSuggested-by: Daniel P. Berrange \nSigned-off-by: Cao jin \nSigned-off-by: Mao Zhongyi \nReviewed-by: Juan Quintela \nSigned-off-by: Daniel P. Berrange \n---\n block/sheepdog.c | 2 +-\n block/ssh.c | 2 +-\n include/qemu/sockets.h | 12 +--\n io/channel-socket.c | 2 +-\n util/qemu-sockets.c | 205 ++++++-------------------------------------------\n 5 files changed, 28 insertions(+), 195 deletions(-)","diff":"diff --git a/block/sheepdog.c b/block/sheepdog.c\nindex abb2e79065..64ab07f3b7 100644\n--- a/block/sheepdog.c\n+++ b/block/sheepdog.c\n@@ -591,7 +591,7 @@ static int connect_to_sdog(BDRVSheepdogState *s, Error **errp)\n {\n int fd;\n \n- fd = socket_connect(s->addr, NULL, NULL, errp);\n+ fd = socket_connect(s->addr, errp);\n \n if (s->addr->type == SOCKET_ADDRESS_TYPE_INET && fd >= 0) {\n int ret = socket_set_nodelay(fd);\ndiff --git a/block/ssh.c b/block/ssh.c\nindex e8f0404c03..b049a16eb9 100644\n--- a/block/ssh.c\n+++ b/block/ssh.c\n@@ -678,7 +678,7 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,\n }\n \n /* Open the socket and connect. */\n- s->sock = inet_connect_saddr(s->inet, NULL, NULL, errp);\n+ s->sock = inet_connect_saddr(s->inet, errp);\n if (s->sock < 0) {\n ret = -EIO;\n goto err;\ndiff --git a/include/qemu/sockets.h b/include/qemu/sockets.h\nindex ef6b5591f7..639cc079d9 100644\n--- a/include/qemu/sockets.h\n+++ b/include/qemu/sockets.h\n@@ -27,18 +27,11 @@ int socket_set_fast_reuse(int fd);\n #define SHUT_RDWR 2\n #endif\n \n-/* callback function for nonblocking connect\n- * valid fd on success, negative error code on failure\n- */\n-typedef void NonBlockingConnectHandler(int fd, Error *err, void *opaque);\n-\n int inet_ai_family_from_address(InetSocketAddress *addr,\n Error **errp);\n int inet_parse(InetSocketAddress *addr, const char *str, Error **errp);\n int inet_connect(const char *str, Error **errp);\n-int inet_connect_saddr(InetSocketAddress *saddr,\n- NonBlockingConnectHandler *callback, void *opaque,\n- Error **errp);\n+int inet_connect_saddr(InetSocketAddress *saddr, Error **errp);\n \n NetworkAddressFamily inet_netfamily(int family);\n \n@@ -46,8 +39,7 @@ int unix_listen(const char *path, char *ostr, int olen, Error **errp);\n int unix_connect(const char *path, Error **errp);\n \n SocketAddress *socket_parse(const char *str, Error **errp);\n-int socket_connect(SocketAddress *addr, NonBlockingConnectHandler *callback,\n- void *opaque, Error **errp);\n+int socket_connect(SocketAddress *addr, Error **errp);\n int socket_listen(SocketAddress *addr, Error **errp);\n void socket_listen_cleanup(int fd, Error **errp);\n int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp);\ndiff --git a/io/channel-socket.c b/io/channel-socket.c\nindex 591d27e8c3..563e297357 100644\n--- a/io/channel-socket.c\n+++ b/io/channel-socket.c\n@@ -140,7 +140,7 @@ int qio_channel_socket_connect_sync(QIOChannelSocket *ioc,\n int fd;\n \n trace_qio_channel_socket_connect_sync(ioc, addr);\n- fd = socket_connect(addr, NULL, NULL, errp);\n+ fd = socket_connect(addr, errp);\n if (fd < 0) {\n trace_qio_channel_socket_connect_fail(ioc);\n return -1;\ndiff --git a/util/qemu-sockets.c b/util/qemu-sockets.c\nindex 6a511fbf76..b47fb45885 100644\n--- a/util/qemu-sockets.c\n+++ b/util/qemu-sockets.c\n@@ -342,88 +342,19 @@ listen_ok:\n ((rc) == -EINPROGRESS)\n #endif\n \n-/* Struct to store connect state for non blocking connect */\n-typedef struct ConnectState {\n- int fd;\n- struct addrinfo *addr_list;\n- struct addrinfo *current_addr;\n- NonBlockingConnectHandler *callback;\n- void *opaque;\n-} ConnectState;\n-\n-static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,\n- ConnectState *connect_state, Error **errp);\n+static int inet_connect_addr(struct addrinfo *addr, Error **errp);\n \n-static void wait_for_connect(void *opaque)\n-{\n- ConnectState *s = opaque;\n- int val = 0, rc = 0;\n- socklen_t valsize = sizeof(val);\n- bool in_progress;\n- Error *err = NULL;\n-\n- qemu_set_fd_handler(s->fd, NULL, NULL, NULL);\n-\n- do {\n- rc = qemu_getsockopt(s->fd, SOL_SOCKET, SO_ERROR, &val, &valsize);\n- } while (rc == -1 && errno == EINTR);\n-\n- /* update rc to contain error */\n- if (!rc && val) {\n- rc = -1;\n- errno = val;\n- }\n-\n- /* connect error */\n- if (rc < 0) {\n- error_setg_errno(&err, errno, \"Error connecting to socket\");\n- closesocket(s->fd);\n- s->fd = rc;\n- }\n-\n- /* try to connect to the next address on the list */\n- if (s->current_addr) {\n- while (s->current_addr->ai_next != NULL && s->fd < 0) {\n- s->current_addr = s->current_addr->ai_next;\n- s->fd = inet_connect_addr(s->current_addr, &in_progress, s, NULL);\n- if (s->fd < 0) {\n- error_free(err);\n- err = NULL;\n- error_setg_errno(&err, errno, \"Unable to start socket connect\");\n- }\n- /* connect in progress */\n- if (in_progress) {\n- goto out;\n- }\n- }\n-\n- freeaddrinfo(s->addr_list);\n- }\n-\n- if (s->callback) {\n- s->callback(s->fd, err, s->opaque);\n- }\n- g_free(s);\n-out:\n- error_free(err);\n-}\n-\n-static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,\n- ConnectState *connect_state, Error **errp)\n+static int inet_connect_addr(struct addrinfo *addr, Error **errp)\n {\n int sock, rc;\n \n- *in_progress = false;\n-\n sock = qemu_socket(addr->ai_family, addr->ai_socktype, addr->ai_protocol);\n if (sock < 0) {\n error_setg_errno(errp, errno, \"Failed to create socket\");\n return -1;\n }\n socket_set_fast_reuse(sock);\n- if (connect_state != NULL) {\n- qemu_set_nonblock(sock);\n- }\n+\n /* connect to peer */\n do {\n rc = 0;\n@@ -432,15 +363,12 @@ static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,\n }\n } while (rc == -EINTR);\n \n- if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) {\n- connect_state->fd = sock;\n- qemu_set_fd_handler(sock, NULL, wait_for_connect, connect_state);\n- *in_progress = true;\n- } else if (rc < 0) {\n+ if (rc < 0) {\n error_setg_errno(errp, errno, \"Failed to connect socket\");\n closesocket(sock);\n return -1;\n }\n+\n return sock;\n }\n \n@@ -498,44 +426,24 @@ static struct addrinfo *inet_parse_connect_saddr(InetSocketAddress *saddr,\n *\n * @saddr: Inet socket address specification\n * @errp: set on error\n- * @callback: callback function for non-blocking connect\n- * @opaque: opaque for callback function\n *\n * Returns: -1 on error, file descriptor on success.\n- *\n- * If @callback is non-null, the connect is non-blocking. If this\n- * function succeeds, callback will be called when the connection\n- * completes, with the file descriptor on success, or -1 on error.\n */\n-int inet_connect_saddr(InetSocketAddress *saddr,\n- NonBlockingConnectHandler *callback, void *opaque,\n- Error **errp)\n+int inet_connect_saddr(InetSocketAddress *saddr, Error **errp)\n {\n Error *local_err = NULL;\n struct addrinfo *res, *e;\n int sock = -1;\n- bool in_progress;\n- ConnectState *connect_state = NULL;\n \n res = inet_parse_connect_saddr(saddr, errp);\n if (!res) {\n return -1;\n }\n \n- if (callback != NULL) {\n- connect_state = g_malloc0(sizeof(*connect_state));\n- connect_state->addr_list = res;\n- connect_state->callback = callback;\n- connect_state->opaque = opaque;\n- }\n-\n for (e = res; e != NULL; e = e->ai_next) {\n error_free(local_err);\n local_err = NULL;\n- if (connect_state != NULL) {\n- connect_state->current_addr = e;\n- }\n- sock = inet_connect_addr(e, &in_progress, connect_state, &local_err);\n+ sock = inet_connect_addr(e, &local_err);\n if (sock >= 0) {\n break;\n }\n@@ -543,15 +451,8 @@ int inet_connect_saddr(InetSocketAddress *saddr,\n \n if (sock < 0) {\n error_propagate(errp, local_err);\n- } else if (in_progress) {\n- /* wait_for_connect() will do the rest */\n- return sock;\n- } else {\n- if (callback) {\n- callback(sock, NULL, opaque);\n- }\n }\n- g_free(connect_state);\n+\n freeaddrinfo(res);\n return sock;\n }\n@@ -730,7 +631,7 @@ int inet_connect(const char *str, Error **errp)\n InetSocketAddress *addr = g_new(InetSocketAddress, 1);\n \n if (!inet_parse(addr, str, errp)) {\n- sock = inet_connect_saddr(addr, NULL, NULL, errp);\n+ sock = inet_connect_saddr(addr, errp);\n }\n qapi_free_InetSocketAddress(addr);\n return sock;\n@@ -763,21 +664,16 @@ static bool vsock_parse_vaddr_to_sockaddr(const VsockSocketAddress *vaddr,\n return true;\n }\n \n-static int vsock_connect_addr(const struct sockaddr_vm *svm, bool *in_progress,\n- ConnectState *connect_state, Error **errp)\n+static int vsock_connect_addr(const struct sockaddr_vm *svm, Error **errp)\n {\n int sock, rc;\n \n- *in_progress = false;\n-\n sock = qemu_socket(AF_VSOCK, SOCK_STREAM, 0);\n if (sock < 0) {\n error_setg_errno(errp, errno, \"Failed to create socket\");\n return -1;\n }\n- if (connect_state != NULL) {\n- qemu_set_nonblock(sock);\n- }\n+\n /* connect to peer */\n do {\n rc = 0;\n@@ -786,50 +682,26 @@ static int vsock_connect_addr(const struct sockaddr_vm *svm, bool *in_progress,\n }\n } while (rc == -EINTR);\n \n- if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) {\n- connect_state->fd = sock;\n- qemu_set_fd_handler(sock, NULL, wait_for_connect, connect_state);\n- *in_progress = true;\n- } else if (rc < 0) {\n+ if (rc < 0) {\n error_setg_errno(errp, errno, \"Failed to connect socket\");\n closesocket(sock);\n return -1;\n }\n+\n return sock;\n }\n \n-static int vsock_connect_saddr(VsockSocketAddress *vaddr,\n- NonBlockingConnectHandler *callback,\n- void *opaque,\n- Error **errp)\n+static int vsock_connect_saddr(VsockSocketAddress *vaddr, Error **errp)\n {\n struct sockaddr_vm svm;\n int sock = -1;\n- bool in_progress;\n- ConnectState *connect_state = NULL;\n \n if (!vsock_parse_vaddr_to_sockaddr(vaddr, &svm, errp)) {\n return -1;\n }\n \n- if (callback != NULL) {\n- connect_state = g_malloc0(sizeof(*connect_state));\n- connect_state->callback = callback;\n- connect_state->opaque = opaque;\n- }\n+ sock = vsock_connect_addr(&svm, errp);\n \n- sock = vsock_connect_addr(&svm, &in_progress, connect_state, errp);\n- if (sock < 0) {\n- /* do nothing */\n- } else if (in_progress) {\n- /* wait_for_connect() will do the rest */\n- return sock;\n- } else {\n- if (callback) {\n- callback(sock, NULL, opaque);\n- }\n- }\n- g_free(connect_state);\n return sock;\n }\n \n@@ -889,9 +761,7 @@ static void vsock_unsupported(Error **errp)\n error_setg(errp, \"socket family AF_VSOCK unsupported\");\n }\n \n-static int vsock_connect_saddr(VsockSocketAddress *vaddr,\n- NonBlockingConnectHandler *callback,\n- void *opaque, Error **errp)\n+static int vsock_connect_saddr(VsockSocketAddress *vaddr, Error **errp)\n {\n vsock_unsupported(errp);\n return -1;\n@@ -994,12 +864,9 @@ err:\n return -1;\n }\n \n-static int unix_connect_saddr(UnixSocketAddress *saddr,\n- NonBlockingConnectHandler *callback, void *opaque,\n- Error **errp)\n+static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)\n {\n struct sockaddr_un un;\n- ConnectState *connect_state = NULL;\n int sock, rc;\n \n if (saddr->path == NULL) {\n@@ -1012,12 +879,6 @@ static int unix_connect_saddr(UnixSocketAddress *saddr,\n error_setg_errno(errp, errno, \"Failed to create socket\");\n return -1;\n }\n- if (callback != NULL) {\n- connect_state = g_malloc0(sizeof(*connect_state));\n- connect_state->callback = callback;\n- connect_state->opaque = opaque;\n- qemu_set_nonblock(sock);\n- }\n \n if (strlen(saddr->path) > sizeof(un.sun_path)) {\n error_setg(errp, \"UNIX socket path '%s' is too long\", saddr->path);\n@@ -1038,29 +899,16 @@ static int unix_connect_saddr(UnixSocketAddress *saddr,\n }\n } while (rc == -EINTR);\n \n- if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) {\n- connect_state->fd = sock;\n- qemu_set_fd_handler(sock, NULL, wait_for_connect, connect_state);\n- return sock;\n- } else if (rc >= 0) {\n- /* non blocking socket immediate success, call callback */\n- if (callback != NULL) {\n- callback(sock, NULL, opaque);\n- }\n- }\n-\n if (rc < 0) {\n error_setg_errno(errp, -rc, \"Failed to connect socket %s\",\n saddr->path);\n goto err;\n }\n \n- g_free(connect_state);\n return sock;\n \n err:\n close(sock);\n- g_free(connect_state);\n return -1;\n }\n \n@@ -1075,9 +923,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,\n return -1;\n }\n \n-static int unix_connect_saddr(UnixSocketAddress *saddr,\n- NonBlockingConnectHandler *callback, void *opaque,\n- Error **errp)\n+static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)\n {\n error_setg(errp, \"unix sockets are not available on windows\");\n errno = ENOTSUP;\n@@ -1123,7 +969,7 @@ int unix_connect(const char *path, Error **errp)\n \n saddr = g_new0(UnixSocketAddress, 1);\n saddr->path = g_strdup(path);\n- sock = unix_connect_saddr(saddr, NULL, NULL, errp);\n+ sock = unix_connect_saddr(saddr, errp);\n qapi_free_UnixSocketAddress(saddr);\n return sock;\n }\n@@ -1168,30 +1014,25 @@ fail:\n return NULL;\n }\n \n-int socket_connect(SocketAddress *addr, NonBlockingConnectHandler *callback,\n- void *opaque, Error **errp)\n+int socket_connect(SocketAddress *addr, Error **errp)\n {\n int fd;\n \n switch (addr->type) {\n case SOCKET_ADDRESS_TYPE_INET:\n- fd = inet_connect_saddr(&addr->u.inet, callback, opaque, errp);\n+ fd = inet_connect_saddr(&addr->u.inet, errp);\n break;\n \n case SOCKET_ADDRESS_TYPE_UNIX:\n- fd = unix_connect_saddr(&addr->u.q_unix, callback, opaque, errp);\n+ fd = unix_connect_saddr(&addr->u.q_unix, errp);\n break;\n \n case SOCKET_ADDRESS_TYPE_FD:\n fd = monitor_get_fd(cur_mon, addr->u.fd.str, errp);\n- if (fd >= 0 && callback) {\n- qemu_set_nonblock(fd);\n- callback(fd, NULL, opaque);\n- }\n break;\n \n case SOCKET_ADDRESS_TYPE_VSOCK:\n- fd = vsock_connect_saddr(&addr->u.vsock, callback, opaque, errp);\n+ fd = vsock_connect_saddr(&addr->u.vsock, errp);\n break;\n \n default:\n","prefixes":["PULL","v1","6/9"]}