From patchwork Tue Aug 23 13:33:45 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Marc-Andr=C3=A9_Lureau?= X-Patchwork-Id: 9295647 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 50DB460574 for ; Tue, 23 Aug 2016 13:34:27 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 40D872868F for ; Tue, 23 Aug 2016 13:34:27 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 355CF286D5; Tue, 23 Aug 2016 13:34:27 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 790CE2868F for ; Tue, 23 Aug 2016 13:34:26 +0000 (UTC) Received: from localhost ([::1]:46640 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bcBqS-0000Ka-4Z for patchwork-qemu-devel@patchwork.kernel.org; Tue, 23 Aug 2016 09:34:24 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57417) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bcBq2-0000KH-9u for qemu-devel@nongnu.org; Tue, 23 Aug 2016 09:33:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bcBpx-0007Zm-8n for qemu-devel@nongnu.org; Tue, 23 Aug 2016 09:33:57 -0400 Received: from mx3-phx2.redhat.com ([209.132.183.24]:41118) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bcBpw-0007ZW-W0 for qemu-devel@nongnu.org; Tue, 23 Aug 2016 09:33:53 -0400 Received: from zmail17.collab.prod.int.phx2.redhat.com (zmail17.collab.prod.int.phx2.redhat.com [10.5.83.19]) by mx3-phx2.redhat.com (8.13.8/8.13.8) with ESMTP id u7NDXl4u009184; Tue, 23 Aug 2016 09:33:47 -0400 Date: Tue, 23 Aug 2016 09:33:45 -0400 (EDT) From: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau To: "Daniel P. Berrange" Message-ID: <1593550465.1799998.1471959225761.JavaMail.zimbra@redhat.com> In-Reply-To: <20160823131338.GA32515@redhat.com> References: <20160823094537.8782-1-marcandre.lureau@redhat.com> <20160823094537.8782-3-marcandre.lureau@redhat.com> <57BC28EA.8070700@cn.fujitsu.com> <20160823131338.GA32515@redhat.com> MIME-Version: 1.0 X-Originating-IP: [10.3.116.5] X-Mailer: Zimbra 8.0.6_GA_5922 (ZimbraWebClient - FF48 (Linux)/8.0.6_GA_5922) Thread-Topic: make socket connect non-blocking again Thread-Index: aNMCoE6RTdCATDHhrL/mSgdObEQQRw== X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 209.132.183.24 Subject: Re: [Qemu-devel] [PATCH for-2.7 2/2] net: make socket connect non-blocking again 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: jasowang@redhat.com, qemu-devel@nongnu.org, Cao jin , pbonzini@redhat.com, ashijeetacharya@gmail.com, crobinso@redhat.com, =?utf-8?Q?Marc-Andr=C3=A9?= Lureau Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP Hi ----- Original Message ----- > On Tue, Aug 23, 2016 at 06:43:54PM +0800, Cao jin wrote: > > Hi, > > I just noticed you still using the old-style non-blocking connect(which > > will > > still block when query DNS), which will be removed (suggested by Daniel), > > but the patch is still on the way. So I guess maybe you should switch to > > QIOChannel way. > > > > FYI: > > http://lists.nongnu.org/archive/html/qemu-devel/2016-07/msg06386.html > > http://lists.nongnu.org/archive/html/qemu-devel/2016-08/msg00046.html > > Yes, switching to QIOChannel would be good, but that is certainly not > something that's acceptable for 2.7.0. We need Marc-André's fix in the > short term for this release. If someone wants to do a QIOChannel > conversion for 2.8.0/2.9.0/etc that's an option... After looking at this a bit, it would need significant effort for net/socket to switch fully to qiochannel, so it's certainly not for 2.7. However, we could use it just for the connect step for now (see attached patch) (tbh, I am not sure how well that net/socket async code is being handled in case of cancellation etc..) From ad81f6d913bdb09b6f7c781c1e55ac42228f7c4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Tue, 23 Aug 2016 13:33:30 +0400 Subject: [PATCH] net: make socket connect non-blocking again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since commit 7e8449594c929, the socket connect code is blocking, because calling socket_connect() without callback is blocking. Make it non-blocking by adding a callback. Unfortunately, the callback needs many local variables that are not easy to get rid of (it can't easily create the qemu_new_net_client() earlier). By adding the callback, the socket is also made non-blocking. If the socket attempt succeeded immediately, the callback is still being called. Signed-off-by: Marc-André Lureau --- net/socket.c | 91 ++++++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 61 insertions(+), 30 deletions(-) diff --git a/net/socket.c b/net/socket.c index 645bcb0..ca0c0b7 100644 --- a/net/socket.c +++ b/net/socket.c @@ -33,6 +33,7 @@ #include "qemu/sockets.h" #include "qemu/iov.h" #include "qemu/main-loop.h" +#include "io/channel-socket.h" typedef struct NetSocketState { NetClientState nc; @@ -517,53 +518,83 @@ static int net_socket_listen_init(NetClientState *peer, return 0; } -static int net_socket_connect_init(NetClientState *peer, - const char *model, - const char *name, - const char *host_str) +typedef struct { + QIOChannelSocket *qio_socket; + NetClientState *peer; + SocketAddress *saddr; + char *model; + char *name; +} socket_connect_data; + +static void socket_connect_data_free(void *data) { + socket_connect_data *c = data; + + qapi_free_SocketAddress(c->saddr); + object_unref(OBJECT(c->qio_socket)); + g_free(c->model); + g_free(c->name); + g_free(c); +} + +static void net_socket_connect_cb(Object *source, + Error *err, + gpointer opaque) +{ + socket_connect_data *c = opaque; + int fd; NetSocketState *s; - int fd = -1, ret = -1; char *addr_str = NULL; - SocketAddress *saddr = NULL; Error *local_error = NULL; - saddr = socket_parse(host_str, &local_error); - if (saddr == NULL) { - error_report_err(local_error); - return -1; - } - - fd = socket_connect(saddr, &local_error, NULL, NULL); - if (fd < 0) { + addr_str = socket_address_to_string(c->saddr, &local_error); + if (addr_str == NULL) { error_report_err(local_error); - goto end; + return; } - qemu_set_nonblock(fd); - - s = net_socket_fd_init(peer, model, name, fd, true); + fd = c->qio_socket->fd; + c->qio_socket->fd = -1; + s = net_socket_fd_init(c->peer, c->model, c->name, fd, true); if (!s) { - goto end; - } - - addr_str = socket_address_to_string(saddr, &local_error); - if (addr_str == NULL) { - error_report_err(local_error); + closesocket(fd); goto end; } snprintf(s->nc.info_str, sizeof(s->nc.info_str), "socket: connect to %s", addr_str); - ret = 0; end: - if (ret == -1 && fd >= 0) { - closesocket(fd); - } - qapi_free_SocketAddress(saddr); g_free(addr_str); - return ret; +} + +static int net_socket_connect_init(NetClientState *peer, + const char *model, + const char *name, + const char *host_str) +{ + socket_connect_data *c = g_new0(socket_connect_data, 1); + Error *local_error = NULL; + + c->qio_socket = qio_channel_socket_new(); + c->peer = peer; + c->model = g_strdup(model); + c->name = g_strdup(name); + c->saddr = socket_parse(host_str, &local_error); + if (c->saddr == NULL) { + goto err; + } + + qio_channel_socket_connect_async(c->qio_socket, c->saddr, + net_socket_connect_cb, + c, socket_connect_data_free); + + return 0; + +err: + error_report_err(local_error); + socket_connect_data_free(c); + return -1; } static int net_socket_mcast_init(NetClientState *peer, -- 2.9.0