From nobody Sat Feb 7 08:23:47 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 207.211.31.120 as permitted sender) client-ip=207.211.31.120; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-1.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 207.211.31.120 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.120]) by mx.zohomail.com with SMTPS id 158021724465330.985585088991343; Tue, 28 Jan 2020 05:14:04 -0800 (PST) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-347-HEr6UX2sMamWpwf9owx8Bg-1; Tue, 28 Jan 2020 08:14:00 -0500 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8572A8010C4; Tue, 28 Jan 2020 13:13:55 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5F2F05C219; Tue, 28 Jan 2020 13:13:55 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 1793718034EF; Tue, 28 Jan 2020 13:13:55 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 00SDDn0N019512 for ; Tue, 28 Jan 2020 08:13:49 -0500 Received: by smtp.corp.redhat.com (Postfix) id BC4591001DE1; Tue, 28 Jan 2020 13:13:49 +0000 (UTC) Received: from domokun.gsslab.fab.redhat.com (unknown [10.33.8.110]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2CC431001DDE; Tue, 28 Jan 2020 13:13:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1580217243; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=TW5DTJ9LMgFNuikD40PIB7FMHkrgRKfHf9SWYeGwvJk=; b=J3qAjCq6dmiGvC6+iIKNs0EfPCGRJ4X69yeXGx4Q+LFNbrcpi4teT5vIdPVF1or71UgeJS qUQxrZ59fDhb2MS40tmtD7jq9S0Dxkm4WsbZp/xDiggjMvPzwxf12Dv7d0eUF3YHsmF3zV Djac9VZT/pVCTOXULbOitwA23/Xh+xI= From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= To: libvir-list@redhat.com Subject: [libvirt PATCH v2 34/56] rpc: convert RPC client to use GMainLoop instead of poll Date: Tue, 28 Jan 2020 13:11:15 +0000 Message-Id: <20200128131137.1762449-35-berrange@redhat.com> In-Reply-To: <20200128131137.1762449-1-berrange@redhat.com> References: <20200128131137.1762449-1-berrange@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-loop: libvir-list@redhat.com X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-MC-Unique: HEr6UX2sMamWpwf9owx8Bg-1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Content-Type: text/plain; charset="utf-8" To eliminate the dependancy on GNULIB's poll impl, we need to change the RPC client code to use GMainLoop. We don't really want to use GIOChannel, but it provides the most convenient way to do socket event watches with Windows portability. The other alternative would be to use GSocket but that is a much more complex change affecting libvirt more broadly. Signed-off-by: Daniel P. Berrang=C3=A9 --- src/rpc/virnetclient.c | 215 ++++++++++++++++++++++------------------- 1 file changed, 113 insertions(+), 102 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 031a99711f..9069c57113 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -21,7 +21,6 @@ #include =20 #include -#include #include #include =20 @@ -36,6 +35,7 @@ #include "virerror.h" #include "virprobe.h" #include "virstring.h" +#include "vireventglibwatch.h" =20 #define VIR_FROM_THIS VIR_FROM_RPC =20 @@ -83,9 +83,8 @@ struct _virNetClient { virNetSASLSessionPtr sasl; #endif =20 - /* Self-pipe to wakeup threads waiting in poll() */ - int wakeupSendFD; - int wakeupReadFD; + GMainLoop *eventLoop; + GMainContext *eventCtx; =20 /* * List of calls currently waiting for dispatch @@ -294,25 +293,18 @@ static virNetClientPtr virNetClientNew(virNetSocketPt= r sock, const char *hostname) { virNetClientPtr client =3D NULL; - int wakeupFD[2] =3D { -1, -1 }; =20 if (virNetClientInitialize() < 0) goto error; =20 - if (pipe2(wakeupFD, O_CLOEXEC) < 0) { - virReportSystemError(errno, "%s", - _("unable to make pipe")); - goto error; - } - if (!(client =3D virObjectLockableNew(virNetClientClass))) goto error; =20 client->sock =3D sock; sock =3D NULL; - client->wakeupReadFD =3D wakeupFD[0]; - client->wakeupSendFD =3D wakeupFD[1]; - wakeupFD[0] =3D wakeupFD[1] =3D -1; + + client->eventCtx =3D g_main_context_new(); + client->eventLoop =3D g_main_loop_new(client->eventCtx, FALSE); =20 client->hostname =3D g_strdup(hostname); =20 @@ -322,8 +314,6 @@ static virNetClientPtr virNetClientNew(virNetSocketPtr = sock, return client; =20 error: - VIR_FORCE_CLOSE(wakeupFD[0]); - VIR_FORCE_CLOSE(wakeupFD[1]); virObjectUnref(client); virObjectUnref(sock); return NULL; @@ -698,8 +688,8 @@ void virNetClientDispose(void *obj) virObjectUnref(client->programs[i]); VIR_FREE(client->programs); =20 - VIR_FORCE_CLOSE(client->wakeupSendFD); - VIR_FORCE_CLOSE(client->wakeupReadFD); + g_main_loop_unref(client->eventLoop); + g_main_context_unref(client->eventCtx); =20 VIR_FREE(client->hostname); =20 @@ -778,6 +768,7 @@ virNetClientCloseLocked(virNetClientPtr client) } } =20 + static void virNetClientCloseInternal(virNetClientPtr client, int reason) { @@ -800,11 +791,7 @@ static void virNetClientCloseInternal(virNetClientPtr = client, * queue and close the client because we set client->wantClose. */ if (client->haveTheBuck) { - char ignore =3D 1; - size_t len =3D sizeof(ignore); - - if (safewrite(client->wakeupSendFD, &ignore, len) !=3D len) - VIR_ERROR(_("failed to wake up polling thread")); + g_main_loop_quit(client->eventLoop); } else { virNetClientIOEventLoopPassTheBuck(client, NULL); } @@ -831,13 +818,70 @@ void virNetClientSetSASLSession(virNetClientPtr clien= t, #endif =20 =20 +static gboolean +virNetClientIOEventTLS(int fd, + GIOCondition ev, + gpointer opaque); + +static gboolean +virNetClientTLSHandshake(virNetClientPtr client) +{ + GIOCondition ev; + int ret; + + ret =3D virNetTLSSessionHandshake(client->tls); + + if (ret <=3D 0) + return FALSE; + + if (virNetTLSSessionGetHandshakeStatus(client->tls) =3D=3D + VIR_NET_TLS_HANDSHAKE_RECVING) + ev =3D G_IO_IN; + else + ev =3D G_IO_OUT; + + virEventGLibAddSocketWatch(virNetSocketGetFD(client->sock), + ev, + client->eventCtx, + virNetClientIOEventTLS, client, NULL); + + return TRUE; +} + + +static gboolean +virNetClientIOEventTLS(int fd G_GNUC_UNUSED, + GIOCondition ev G_GNUC_UNUSED, + gpointer opaque) +{ + virNetClientPtr client =3D opaque; + + if (!virNetClientTLSHandshake(client)) + g_main_loop_quit(client->eventLoop); + + return G_SOURCE_REMOVE; +} + + +static gboolean +virNetClientIOEventTLSConfirm(int fd G_GNUC_UNUSED, + GIOCondition ev G_GNUC_UNUSED, + gpointer opaque) +{ + virNetClientPtr client =3D opaque; + + g_main_loop_quit(client->eventLoop); + + return G_SOURCE_REMOVE; +} + + int virNetClientSetTLSSession(virNetClientPtr client, virNetTLSContextPtr tls) { int ret; char buf[1]; int len; - struct pollfd fds[1]; =20 #ifndef WIN32 sigset_t oldmask, blockedsigs; @@ -860,22 +904,8 @@ int virNetClientSetTLSSession(virNetClientPtr client, =20 virNetSocketSetTLSSession(client->sock, client->tls); =20 - for (;;) { - ret =3D virNetTLSSessionHandshake(client->tls); - - if (ret < 0) - goto error; - if (ret =3D=3D 0) - break; - - fds[0].fd =3D virNetSocketGetFD(client->sock); - fds[0].revents =3D 0; - if (virNetTLSSessionGetHandshakeStatus(client->tls) =3D=3D - VIR_NET_TLS_HANDSHAKE_RECVING) - fds[0].events =3D POLLIN; - else - fds[0].events =3D POLLOUT; - + virResetLastError(); + if (virNetClientTLSHandshake(client)) { #ifndef WIN32 /* Block SIGWINCH from interrupting poll in curses programs, * then restore the original signal mask again immediately @@ -885,16 +915,16 @@ int virNetClientSetTLSSession(virNetClientPtr client, ignore_value(pthread_sigmask(SIG_BLOCK, &blockedsigs, &oldmask)); #endif /* !WIN32 */ =20 - repoll: - ret =3D poll(fds, G_N_ELEMENTS(fds), -1); - if (ret < 0 && (errno =3D=3D EAGAIN || errno =3D=3D EINTR)) - goto repoll; + g_main_loop_run(client->eventLoop); =20 #ifndef WIN32 ignore_value(pthread_sigmask(SIG_SETMASK, &oldmask, NULL)); #endif /* !WIN32 */ } =20 + if (virGetLastErrorCode() !=3D VIR_ERR_OK) + goto error; + ret =3D virNetTLSContextCheckCertificate(tls, client->tls); =20 if (ret < 0) @@ -904,19 +934,17 @@ int virNetClientSetTLSSession(virNetClientPtr client, * etc. If we make the grade, it will send us a '\1' byte. */ =20 - fds[0].fd =3D virNetSocketGetFD(client->sock); - fds[0].revents =3D 0; - fds[0].events =3D POLLIN; + virEventGLibAddSocketWatch(virNetSocketGetFD(client->sock), + G_IO_IN, + client->eventCtx, + virNetClientIOEventTLSConfirm, client, NULL= ); =20 #ifndef WIN32 /* Block SIGWINCH from interrupting poll in curses programs */ ignore_value(pthread_sigmask(SIG_BLOCK, &blockedsigs, &oldmask)); #endif /* !WIN32 */ =20 - repoll2: - ret =3D poll(fds, G_N_ELEMENTS(fds), -1); - if (ret < 0 && (errno =3D=3D EAGAIN || errno =3D=3D EINTR)) - goto repoll2; + g_main_loop_run(client->eventLoop); =20 #ifndef WIN32 ignore_value(pthread_sigmask(SIG_SETMASK, &oldmask, NULL)); @@ -1451,12 +1479,12 @@ virNetClientIOHandleInput(virNetClientPtr client) static bool virNetClientIOEventLoopPollEvents(virNetClientCallPtr call, void *opaque) { - struct pollfd *fd =3D opaque; + GIOCondition *ev =3D opaque; =20 if (call->mode =3D=3D VIR_NET_CLIENT_MODE_WAIT_RX) - fd->events |=3D POLLIN; + *ev |=3D G_IO_IN; if (call->mode =3D=3D VIR_NET_CLIENT_MODE_WAIT_TX) - fd->events |=3D POLLOUT; + *ev |=3D G_IO_OUT; =20 return false; } @@ -1552,6 +1580,18 @@ virNetClientIOEventLoopPassTheBuck(virNetClientPtr c= lient, } =20 =20 +static gboolean +virNetClientIOEventFD(int fd G_GNUC_UNUSED, + GIOCondition ev, + gpointer opaque) +{ + GIOCondition *rev =3D opaque; + *rev =3D ev; + + return G_SOURCE_REMOVE; +} + + /* * Process all calls pending dispatch/receive until we * get a reply to our own call. Then quit and pass the buck @@ -1563,21 +1603,17 @@ virNetClientIOEventLoopPassTheBuck(virNetClientPtr = client, static int virNetClientIOEventLoop(virNetClientPtr client, virNetClientCallPtr thiscall) { - struct pollfd fds[2]; bool error =3D false; int closeReason; - int ret; - - fds[0].fd =3D virNetSocketGetFD(client->sock); - fds[1].fd =3D client->wakeupReadFD; =20 for (;;) { - char ignore; #ifndef WIN32 sigset_t oldmask, blockedsigs; #endif /* !WIN32 */ int timeout =3D -1; virNetMessagePtr msg =3D NULL; + GIOCondition ev =3D 0; + GIOCondition rev =3D 0; =20 /* If we have existing SASL decoded data we don't want to sleep in * the poll(), just check if any other FDs are also ready. @@ -1595,22 +1631,22 @@ static int virNetClientIOEventLoop(virNetClientPtr = client, if (timeout =3D=3D -1) timeout =3D virKeepAliveTimeout(client->keepalive); =20 - fds[0].events =3D fds[0].revents =3D 0; - fds[1].events =3D fds[1].revents =3D 0; - - fds[1].events =3D POLLIN; - /* Calculate poll events for calls */ virNetClientCallMatchPredicate(client->waitDispatch, virNetClientIOEventLoopPollEvents, - &fds[0]); + &ev); =20 /* We have to be prepared to receive stream data * regardless of whether any of the calls waiting * for dispatch are for streams. */ if (client->nstreams) - fds[0].events |=3D POLLIN; + ev |=3D G_IO_IN; + + virEventGLibAddSocketWatch(virNetSocketGetFD(client->sock), + ev, + client->eventCtx, + virNetClientIOEventFD, &rev, NULL); =20 /* Release lock while poll'ing so other threads * can stuff themselves on the queue */ @@ -1630,13 +1666,12 @@ static int virNetClientIOEventLoop(virNetClientPtr = client, sigaddset(&blockedsigs, SIGCHLD); # endif sigaddset(&blockedsigs, SIGPIPE); + ignore_value(pthread_sigmask(SIG_BLOCK, &blockedsigs, &oldmask)); #endif /* !WIN32 */ =20 - repoll: - ret =3D poll(fds, G_N_ELEMENTS(fds), timeout); - if (ret < 0 && (errno =3D=3D EAGAIN || errno =3D=3D EINTR)) - goto repoll; + while (!rev) + g_main_context_iteration(client->eventCtx, TRUE); =20 #ifndef WIN32 ignore_value(pthread_sigmask(SIG_SETMASK, &oldmask, NULL)); @@ -1644,12 +1679,6 @@ static int virNetClientIOEventLoop(virNetClientPtr c= lient, =20 virObjectLock(client); =20 - if (ret < 0) { - virReportSystemError(errno, - "%s", _("poll on socket failed")); - goto error; - } - if (virKeepAliveTrigger(client->keepalive, &msg)) { virNetClientMarkClose(client, VIR_CONNECT_CLOSE_REASON_KEEPALI= VE); } else if (msg && virNetClientQueueNonBlocking(client, msg) < 0) { @@ -1661,7 +1690,7 @@ static int virNetClientIOEventLoop(virNetClientPtr cl= ient, * the socket became readable so we consume it */ if (virNetSocketHasCachedData(client->sock)) - fds[0].revents |=3D POLLIN; + rev |=3D G_IO_IN; =20 /* If wantClose flag is set, pretend there was an error on the soc= ket, * but still read and process any data we received so far. @@ -1669,23 +1698,12 @@ static int virNetClientIOEventLoop(virNetClientPtr = client, if (client->wantClose) error =3D true; =20 - if (fds[1].revents) { - VIR_DEBUG("Woken up from poll by other thread"); - if (saferead(client->wakeupReadFD, &ignore, sizeof(ignore)) != =3D sizeof(ignore)) { - virReportSystemError(errno, "%s", - _("read on wakeup fd failed")); - virNetClientMarkClose(client, VIR_CONNECT_CLOSE_REASON_ERR= OR); - error =3D true; - /* Fall through to process any pending data. */ - } - } - - if (fds[0].revents & POLLHUP) + if (rev & G_IO_HUP) closeReason =3D VIR_CONNECT_CLOSE_REASON_EOF; else closeReason =3D VIR_CONNECT_CLOSE_REASON_ERROR; =20 - if (fds[0].revents & POLLOUT) { + if (rev & G_IO_OUT) { if (virNetClientIOHandleOutput(client) < 0) { virNetClientMarkClose(client, closeReason); error =3D true; @@ -1693,7 +1711,7 @@ static int virNetClientIOEventLoop(virNetClientPtr cl= ient, } } =20 - if (fds[0].revents & POLLIN) { + if (rev & G_IO_IN) { if (virNetClientIOHandleInput(client) < 0) { virNetClientMarkClose(client, closeReason); error =3D true; @@ -1725,13 +1743,13 @@ static int virNetClientIOEventLoop(virNetClientPtr = client, if (error) goto error; =20 - if (fds[0].revents & POLLHUP) { + if (rev & G_IO_HUP) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("received hangup event on socket")); virNetClientMarkClose(client, closeReason); goto error; } - if (fds[0].revents & POLLERR) { + if (rev & G_IO_ERR) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("received error event on socket")); virNetClientMarkClose(client, closeReason); @@ -1858,15 +1876,8 @@ static int virNetClientIO(virNetClientPtr client, =20 /* Check to see if another thread is dispatching */ if (client->haveTheBuck) { - char ignore =3D 1; - /* Force other thread to wakeup from poll */ - if (safewrite(client->wakeupSendFD, &ignore, sizeof(ignore)) !=3D = sizeof(ignore)) { - virNetClientCallRemove(&client->waitDispatch, thiscall); - virReportSystemError(errno, "%s", - _("failed to wake up polling thread")); - return -1; - } + g_main_loop_quit(client->eventLoop); =20 /* If we are non-blocking, detach the thread and keep the call in = the * queue. */ --=20 2.24.1