From nobody Wed Apr 24 21:59:17 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=fail(p=none dis=none) header.from=virtuozzo.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1549544372847884.8547985279217; Thu, 7 Feb 2019 04:59:32 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2ECC22CD7FA; Thu, 7 Feb 2019 12:59:30 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id CF6F46294C; Thu, 7 Feb 2019 12:59:29 +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 7B8F83F60C; Thu, 7 Feb 2019 12:59:29 +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 x17CxHjV018582 for ; Thu, 7 Feb 2019 07:59:17 -0500 Received: by smtp.corp.redhat.com (Postfix) id DB38F104810F; Thu, 7 Feb 2019 12:59:17 +0000 (UTC) Received: from mx1.redhat.com (ext-mx09.extmail.prod.ext.phx2.redhat.com [10.5.110.38]) by smtp.corp.redhat.com (Postfix) with ESMTPS id CC0461048102 for ; Thu, 7 Feb 2019 12:59:11 +0000 (UTC) Received: from relay.sw.ru (relay.sw.ru [185.231.240.75]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 149BBA4050 for ; Thu, 7 Feb 2019 12:59:09 +0000 (UTC) Received: from [10.94.3.220] (helo=dim-vz7.qa.sw.ru) by relay.sw.ru with esmtp (Exim 4.91) (envelope-from ) id 1grjGk-0006TA-Mb for libvir-list@redhat.com; Thu, 07 Feb 2019 15:59:06 +0300 From: Nikolay Shirokovskiy To: libvir-list@redhat.com Date: Thu, 7 Feb 2019 15:58:39 +0300 Message-Id: <1549544327-743817-2-git-send-email-nshirokovskiy@virtuozzo.com> In-Reply-To: <1549544327-743817-1-git-send-email-nshirokovskiy@virtuozzo.com> References: <1549544327-743817-1-git-send-email-nshirokovskiy@virtuozzo.com> X-Greylist: Sender passed SPF test, ACL 242 matched, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Thu, 07 Feb 2019 12:59:09 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Thu, 07 Feb 2019 12:59:09 +0000 (UTC) for IP:'185.231.240.75' DOMAIN:'relay.sw.ru' HELO:'relay.sw.ru' FROM:'nshirokovskiy@virtuozzo.com' RCPT:'' X-RedHat-Spam-Score: -0.001 (SPF_PASS) 185.231.240.75 relay.sw.ru 185.231.240.75 relay.sw.ru X-Scanned-By: MIMEDefang 2.78 on 10.5.110.38 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 1/9] rpc: fix race on stream abort/finish and server side abort 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: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Thu, 07 Feb 2019 12:59:31 +0000 (UTC) Content-Type: text/plain; charset="utf-8" Stream abort/finish can hang because we can receive abort message from server and yet sent abort/finish message to server. The latter will not be answered ever because after server sends abort message it forgets the stream and messages for unknown stream are simply ignored. We check for stream error at the very beginning of remoteStreamFinish/remot= eStreamAbort but stream error can be set after the check in another thread operating on stream. Let's check for stream error under client lock similar to what's done in [1]. [1] 833b901cb: stream: Check for stream EOF Signed-off-by: Nikolay Shirokovskiy --- src/rpc/virnetclient.c | 30 ++++++++++++++++++------------ src/rpc/virnetclientstream.c | 2 +- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 40d45b9..7aa5223 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -2216,20 +2216,26 @@ int virNetClientSendWithReplyStream(virNetClientPtr= client, virNetMessagePtr msg, virNetClientStreamPtr st) { - int ret; + int ret =3D -1; + virObjectLock(client); - /* Other thread might have already received - * stream EOF so we don't want sent anything. - * Server won't respond anyway. - */ - if (virNetClientStreamEOF(st)) { - virObjectUnlock(client); - return 0; + + if (virNetClientStreamRaiseError(st)) + goto cleanup; + + /* Check for EOF only if we are going to wait for incoming data */ + if (!msg->bufferLength && virNetClientStreamEOF(st)) { + ret =3D 0; + goto cleanup; } =20 - ret =3D virNetClientSendInternal(client, msg, true, false); + if (virNetClientSendInternal(client, msg, true, false) < 0) + goto cleanup; + + ret =3D 0; + + cleanup: virObjectUnlock(client); - if (ret < 0) - return -1; - return 0; + + return ret; } diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c index a17da31..3b0db52 100644 --- a/src/rpc/virnetclientstream.c +++ b/src/rpc/virnetclientstream.c @@ -350,7 +350,7 @@ int virNetClientStreamSendPacket(virNetClientStreamPtr = st, if (virNetMessageEncodePayloadRaw(msg, NULL, 0) < 0) goto error; =20 - if (virNetClientSendWithReply(client, msg) < 0) + if (virNetClientSendWithReplyStream(client, msg, st) < 0) goto error; } =20 --=20 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Wed Apr 24 21:59:17 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=fail(p=none dis=none) header.from=virtuozzo.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1549544365721966.5111773178803; Thu, 7 Feb 2019 04:59:25 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 244E0C01DE0B; Thu, 7 Feb 2019 12:59:23 +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 D964C165FD; Thu, 7 Feb 2019 12:59:22 +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 92214180BAAC; Thu, 7 Feb 2019 12:59:22 +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 x17CxEHS018521 for ; Thu, 7 Feb 2019 07:59:14 -0500 Received: by smtp.corp.redhat.com (Postfix) id 316951048109; Thu, 7 Feb 2019 12:59:14 +0000 (UTC) Received: from mx1.redhat.com (ext-mx01.extmail.prod.ext.phx2.redhat.com [10.5.110.25]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 263931048102 for ; Thu, 7 Feb 2019 12:59:11 +0000 (UTC) Received: from relay.sw.ru (relay.sw.ru [185.231.240.75]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1AF8F7AEA6 for ; Thu, 7 Feb 2019 12:59:09 +0000 (UTC) Received: from [10.94.3.220] (helo=dim-vz7.qa.sw.ru) by relay.sw.ru with esmtp (Exim 4.91) (envelope-from ) id 1grjGk-0006TA-UO for libvir-list@redhat.com; Thu, 07 Feb 2019 15:59:07 +0300 From: Nikolay Shirokovskiy To: libvir-list@redhat.com Date: Thu, 7 Feb 2019 15:58:40 +0300 Message-Id: <1549544327-743817-3-git-send-email-nshirokovskiy@virtuozzo.com> In-Reply-To: <1549544327-743817-1-git-send-email-nshirokovskiy@virtuozzo.com> References: <1549544327-743817-1-git-send-email-nshirokovskiy@virtuozzo.com> X-Greylist: Sender passed SPF test, ACL 242 matched, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Thu, 07 Feb 2019 12:59:09 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Thu, 07 Feb 2019 12:59:09 +0000 (UTC) for IP:'185.231.240.75' DOMAIN:'relay.sw.ru' HELO:'relay.sw.ru' FROM:'nshirokovskiy@virtuozzo.com' RCPT:'' X-RedHat-Spam-Score: -0.001 (SPF_PASS) 185.231.240.75 relay.sw.ru 185.231.240.75 relay.sw.ru X-Scanned-By: MIMEDefang 2.83 on 10.5.110.25 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 2/9] rpc: use single function to send stream messages 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: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Thu, 07 Feb 2019 12:59:24 +0000 (UTC) Content-Type: text/plain; charset="utf-8" In next patches we'll add stream state checks to this function that applicable to all call paths. This is handy place because we hold client lock here. Signed-off-by: Nikolay Shirokovskiy --- src/libvirt_remote.syms | 2 +- src/rpc/virnetclient.c | 13 ++++++++----- src/rpc/virnetclient.h | 6 +++--- src/rpc/virnetclientstream.c | 12 ++++-------- 4 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index 9a33626..704f7ea 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -39,8 +39,8 @@ virNetClientRemoteAddrStringSASL; virNetClientRemoveStream; virNetClientSendNonBlock; virNetClientSendNoReply; +virNetClientSendStream; virNetClientSendWithReply; -virNetClientSendWithReplyStream; virNetClientSetCloseCallback; virNetClientSetTLSSession; =20 diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 7aa5223..29c4dc5 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -2205,18 +2205,21 @@ int virNetClientSendNonBlock(virNetClientPtr client, /* * @msg: a message allocated on heap or stack * - * Send a message synchronously, and wait for the reply synchronously + * Send a message synchronously, and wait for the reply synchronously if + * message is dummy (just to wait for incoming data) or abort/finish messa= ge. * * The caller is responsible for free'ing @msg if it was allocated * on the heap * * Returns 0 on success, -1 on failure */ -int virNetClientSendWithReplyStream(virNetClientPtr client, - virNetMessagePtr msg, - virNetClientStreamPtr st) +int virNetClientSendStream(virNetClientPtr client, + virNetMessagePtr msg, + virNetClientStreamPtr st) { int ret =3D -1; + bool expectReply =3D !msg->bufferLength || + msg->header.status !=3D VIR_NET_CONTINUE; =20 virObjectLock(client); =20 @@ -2229,7 +2232,7 @@ int virNetClientSendWithReplyStream(virNetClientPtr c= lient, goto cleanup; } =20 - if (virNetClientSendInternal(client, msg, true, false) < 0) + if (virNetClientSendInternal(client, msg, expectReply, false) < 0) goto cleanup; =20 ret =3D 0; diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h index 39a6176..12ac2b5 100644 --- a/src/rpc/virnetclient.h +++ b/src/rpc/virnetclient.h @@ -115,9 +115,9 @@ int virNetClientSendNoReply(virNetClientPtr client, int virNetClientSendNonBlock(virNetClientPtr client, virNetMessagePtr msg); =20 -int virNetClientSendWithReplyStream(virNetClientPtr client, - virNetMessagePtr msg, - virNetClientStreamPtr st); +int virNetClientSendStream(virNetClientPtr client, + virNetMessagePtr msg, + virNetClientStreamPtr st); =20 # ifdef WITH_SASL void virNetClientSetSASLSession(virNetClientPtr client, diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c index 3b0db52..65aa583 100644 --- a/src/rpc/virnetclientstream.c +++ b/src/rpc/virnetclientstream.c @@ -343,17 +343,13 @@ int virNetClientStreamSendPacket(virNetClientStreamPt= r st, if (status =3D=3D VIR_NET_CONTINUE) { if (virNetMessageEncodePayloadRaw(msg, data, nbytes) < 0) goto error; - - if (virNetClientSendNoReply(client, msg) < 0) - goto error; } else { if (virNetMessageEncodePayloadRaw(msg, NULL, 0) < 0) goto error; - - if (virNetClientSendWithReplyStream(client, msg, st) < 0) - goto error; } =20 + if (virNetClientSendStream(client, msg, st) < 0) + goto error; =20 virNetMessageFree(msg); =20 @@ -500,7 +496,7 @@ int virNetClientStreamRecvPacket(virNetClientStreamPtr = st, =20 VIR_DEBUG("Dummy packet to wait for stream data"); virObjectUnlock(st); - ret =3D virNetClientSendWithReplyStream(client, msg, st); + ret =3D virNetClientSendStream(client, msg, st); virObjectLock(st); virNetMessageFree(msg); =20 @@ -627,7 +623,7 @@ virNetClientStreamSendHole(virNetClientStreamPtr st, &data) < 0) goto cleanup; =20 - if (virNetClientSendNoReply(client, msg) < 0) + if (virNetClientSendStream(client, msg, st) < 0) goto cleanup; =20 ret =3D 0; --=20 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Wed Apr 24 21:59:17 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=fail(p=none dis=none) header.from=virtuozzo.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1549544363400212.28598714499788; Thu, 7 Feb 2019 04:59:23 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 396127AEA4; Thu, 7 Feb 2019 12:59:21 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id F31DC272BF; Thu, 7 Feb 2019 12:59:20 +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 A46213F607; Thu, 7 Feb 2019 12:59:20 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x17CxE8X018538 for ; Thu, 7 Feb 2019 07:59:14 -0500 Received: by smtp.corp.redhat.com (Postfix) id 9D3AE7010E; Thu, 7 Feb 2019 12:59:14 +0000 (UTC) Received: from mx1.redhat.com (ext-mx05.extmail.prod.ext.phx2.redhat.com [10.5.110.29]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 90ABE6FECE for ; Thu, 7 Feb 2019 12:59:11 +0000 (UTC) Received: from relay.sw.ru (relay.sw.ru [185.231.240.75]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1224E83F93 for ; Thu, 7 Feb 2019 12:59:09 +0000 (UTC) Received: from [10.94.3.220] (helo=dim-vz7.qa.sw.ru) by relay.sw.ru with esmtp (Exim 4.91) (envelope-from ) id 1grjGl-0006TA-4Q for libvir-list@redhat.com; Thu, 07 Feb 2019 15:59:07 +0300 From: Nikolay Shirokovskiy To: libvir-list@redhat.com Date: Thu, 7 Feb 2019 15:58:41 +0300 Message-Id: <1549544327-743817-4-git-send-email-nshirokovskiy@virtuozzo.com> In-Reply-To: <1549544327-743817-1-git-send-email-nshirokovskiy@virtuozzo.com> References: <1549544327-743817-1-git-send-email-nshirokovskiy@virtuozzo.com> X-Greylist: Sender passed SPF test, ACL 242 matched, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Thu, 07 Feb 2019 12:59:09 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Thu, 07 Feb 2019 12:59:09 +0000 (UTC) for IP:'185.231.240.75' DOMAIN:'relay.sw.ru' HELO:'relay.sw.ru' FROM:'nshirokovskiy@virtuozzo.com' RCPT:'' X-RedHat-Spam-Score: -0.001 (SPF_PASS) 185.231.240.75 relay.sw.ru 185.231.240.75 relay.sw.ru X-Scanned-By: MIMEDefang 2.78 on 10.5.110.29 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 3/9] rpc: remove unused virNetClientSendNoReply 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: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Thu, 07 Feb 2019 12:59:22 +0000 (UTC) Content-Type: text/plain; charset="utf-8" Signed-off-by: Nikolay Shirokovskiy --- src/libvirt_remote.syms | 1 - src/rpc/virnetclient.c | 22 ---------------------- 2 files changed, 23 deletions(-) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index 704f7ea..88745f2 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -38,7 +38,6 @@ virNetClientRegisterKeepAlive; virNetClientRemoteAddrStringSASL; virNetClientRemoveStream; virNetClientSendNonBlock; -virNetClientSendNoReply; virNetClientSendStream; virNetClientSendWithReply; virNetClientSetCloseCallback; diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 29c4dc5..c102cdc 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -2160,28 +2160,6 @@ int virNetClientSendWithReply(virNetClientPtr client, =20 =20 /* - * @msg: a message allocated on heap or stack - * - * Send a message synchronously, without any reply - * - * The caller is responsible for free'ing @msg if it was allocated - * on the heap - * - * Returns 0 on success, -1 on failure - */ -int virNetClientSendNoReply(virNetClientPtr client, - virNetMessagePtr msg) -{ - int ret; - virObjectLock(client); - ret =3D virNetClientSendInternal(client, msg, false, false); - virObjectUnlock(client); - if (ret < 0) - return -1; - return 0; -} - -/* * @msg: a message allocated on the heap. * * Send a message asynchronously, without any reply --=20 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Wed Apr 24 21:59:17 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=fail(p=none dis=none) header.from=virtuozzo.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1549544358730953.8048610859278; Thu, 7 Feb 2019 04:59:18 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 82FDE110C; Thu, 7 Feb 2019 12:59:16 +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 3491B1048102; Thu, 7 Feb 2019 12:59:16 +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 C8A5A18033A9; Thu, 7 Feb 2019 12:59:15 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x17CxEBd018528 for ; Thu, 7 Feb 2019 07:59:14 -0500 Received: by smtp.corp.redhat.com (Postfix) id 45F2962952; Thu, 7 Feb 2019 12:59:14 +0000 (UTC) Received: from mx1.redhat.com (ext-mx04.extmail.prod.ext.phx2.redhat.com [10.5.110.28]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 3800162941 for ; Thu, 7 Feb 2019 12:59:11 +0000 (UTC) Received: from relay.sw.ru (relay.sw.ru [185.231.240.75]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 19D758F4FD for ; Thu, 7 Feb 2019 12:59:09 +0000 (UTC) Received: from [10.94.3.220] (helo=dim-vz7.qa.sw.ru) by relay.sw.ru with esmtp (Exim 4.91) (envelope-from ) id 1grjGl-0006TA-BW for libvir-list@redhat.com; Thu, 07 Feb 2019 15:59:07 +0300 From: Nikolay Shirokovskiy To: libvir-list@redhat.com Date: Thu, 7 Feb 2019 15:58:42 +0300 Message-Id: <1549544327-743817-5-git-send-email-nshirokovskiy@virtuozzo.com> In-Reply-To: <1549544327-743817-1-git-send-email-nshirokovskiy@virtuozzo.com> References: <1549544327-743817-1-git-send-email-nshirokovskiy@virtuozzo.com> X-Greylist: Sender passed SPF test, ACL 242 matched, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Thu, 07 Feb 2019 12:59:09 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Thu, 07 Feb 2019 12:59:09 +0000 (UTC) for IP:'185.231.240.75' DOMAIN:'relay.sw.ru' HELO:'relay.sw.ru' FROM:'nshirokovskiy@virtuozzo.com' RCPT:'' X-RedHat-Spam-Score: -0.001 (SPF_PASS) 185.231.240.75 relay.sw.ru 185.231.240.75 relay.sw.ru X-Scanned-By: MIMEDefang 2.78 on 10.5.110.28 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 4/9] rpc: fix propagation of errors from server 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: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 07 Feb 2019 12:59:17 +0000 (UTC) Content-Type: text/plain; charset="utf-8" Stream server error is not propagated if thread does not have the buck. In case we have the buck we are ok due to the code added in [1]. Let's check for stream error on all paths. Now we don't need to raise error in virNetClientCallDispatchStream. Old code reported error only if the first message in wait queue awaits reply. It is odd as depends on wait queue situation. For example if we have only TX message in queue and in one iteration loop both send the message and receive error then thread sending TX message did not receive the error. Next if we have RX message (first) and TX message (second) in queue and in one iteration loop both send the TX message and receive error then thread sending TX message received error. In short it was inconsistent. Let's report error whenever we received it and for every type of message as it makes sense to report errors as early as possible. [1] 16c6e2b41: Fix propagation of RPC errors from streams Signed-off-by: Nikolay Shirokovskiy --- src/rpc/virnetclient.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index c102cdc..fcc2e80 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -1230,9 +1230,6 @@ static int virNetClientCallDispatchStream(virNetClien= tPtr client) =20 if (thecall && thecall->expectReply) { VIR_DEBUG("Got a synchronous error"); - /* Raise error now, so that this call will see it immediately = */ - if (!virNetClientStreamRaiseError(st)) - VIR_DEBUG("unable to raise synchronous error"); thecall->mode =3D VIR_NET_CLIENT_MODE_COMPLETE; } return 0; @@ -1947,16 +1944,11 @@ static int virNetClientIO(virNetClientPtr client, */ virNetClientIOUpdateCallback(client, false); =20 - virResetLastError(); rv =3D virNetClientIOEventLoop(client, thiscall); =20 if (client->sock) virNetClientIOUpdateCallback(client, true); =20 - if (rv =3D=3D 0 && - virGetLastErrorCode()) - rv =3D -1; - cleanup: VIR_DEBUG("All done with our call head=3D%p call=3D%p rv=3D%d", client->waitDispatch, thiscall, rv); @@ -2213,6 +2205,9 @@ int virNetClientSendStream(virNetClientPtr client, if (virNetClientSendInternal(client, msg, expectReply, false) < 0) goto cleanup; =20 + if (virNetClientStreamRaiseError(st)) + goto cleanup; + ret =3D 0; =20 cleanup: --=20 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Wed Apr 24 21:59:17 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=fail(p=none dis=none) header.from=virtuozzo.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1549544363521792.6711112514294; Thu, 7 Feb 2019 04:59:23 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8A6DD3D945; Thu, 7 Feb 2019 12:59:21 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 497DC70113; Thu, 7 Feb 2019 12:59:20 +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 E20883F606; Thu, 7 Feb 2019 12:59:19 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x17CxFXX018549 for ; Thu, 7 Feb 2019 07:59:15 -0500 Received: by smtp.corp.redhat.com (Postfix) id 143EB60C61; Thu, 7 Feb 2019 12:59:15 +0000 (UTC) Received: from mx1.redhat.com (ext-mx06.extmail.prod.ext.phx2.redhat.com [10.5.110.30]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 92A7770113 for ; Thu, 7 Feb 2019 12:59:10 +0000 (UTC) Received: from relay.sw.ru (relay.sw.ru [185.231.240.75]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 34DDE369A0 for ; Thu, 7 Feb 2019 12:59:09 +0000 (UTC) Received: from [10.94.3.220] (helo=dim-vz7.qa.sw.ru) by relay.sw.ru with esmtp (Exim 4.91) (envelope-from ) id 1grjGl-0006TA-Ie for libvir-list@redhat.com; Thu, 07 Feb 2019 15:59:07 +0300 From: Nikolay Shirokovskiy To: libvir-list@redhat.com Date: Thu, 7 Feb 2019 15:58:43 +0300 Message-Id: <1549544327-743817-6-git-send-email-nshirokovskiy@virtuozzo.com> In-Reply-To: <1549544327-743817-1-git-send-email-nshirokovskiy@virtuozzo.com> References: <1549544327-743817-1-git-send-email-nshirokovskiy@virtuozzo.com> X-Greylist: Sender passed SPF test, ACL 242 matched, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 07 Feb 2019 12:59:09 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 07 Feb 2019 12:59:09 +0000 (UTC) for IP:'185.231.240.75' DOMAIN:'relay.sw.ru' HELO:'relay.sw.ru' FROM:'nshirokovskiy@virtuozzo.com' RCPT:'' X-RedHat-Spam-Score: -0.001 (SPF_PASS) 185.231.240.75 relay.sw.ru 185.231.240.75 relay.sw.ru X-Scanned-By: MIMEDefang 2.78 on 10.5.110.30 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 5/9] rpc: add mising locking in virNetClientStreamRecvHole 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: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 07 Feb 2019 12:59:22 +0000 (UTC) Content-Type: text/plain; charset="utf-8" Signed-off-by: Nikolay Shirokovskiy --- src/rpc/virnetclientstream.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c index 65aa583..136ed16 100644 --- a/src/rpc/virnetclientstream.c +++ b/src/rpc/virnetclientstream.c @@ -644,8 +644,12 @@ virNetClientStreamRecvHole(virNetClientPtr client ATTR= IBUTE_UNUSED, return -1; } =20 + virObjectLock(st); + *length =3D st->holeLength; st->holeLength =3D 0; + + virObjectUnlock(st); return 0; } =20 --=20 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Wed Apr 24 21:59:17 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=fail(p=none dis=none) header.from=virtuozzo.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1549544358385219.9944625665904; Thu, 7 Feb 2019 04:59:18 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8054E7AE9E; Thu, 7 Feb 2019 12:59:16 +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 3BC3E7C825; Thu, 7 Feb 2019 12:59:15 +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 D30D718033A6; Thu, 7 Feb 2019 12:59:14 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x17CxEfo018522 for ; Thu, 7 Feb 2019 07:59:14 -0500 Received: by smtp.corp.redhat.com (Postfix) id 3C20D660C9; Thu, 7 Feb 2019 12:59:14 +0000 (UTC) Received: from mx1.redhat.com (ext-mx02.extmail.prod.ext.phx2.redhat.com [10.5.110.26]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 2F674794AE for ; Thu, 7 Feb 2019 12:59:11 +0000 (UTC) Received: from relay.sw.ru (relay.sw.ru [185.231.240.75]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4C0CD87623 for ; Thu, 7 Feb 2019 12:59:09 +0000 (UTC) Received: from [10.94.3.220] (helo=dim-vz7.qa.sw.ru) by relay.sw.ru with esmtp (Exim 4.91) (envelope-from ) id 1grjGl-0006TA-O0 for libvir-list@redhat.com; Thu, 07 Feb 2019 15:59:07 +0300 From: Nikolay Shirokovskiy To: libvir-list@redhat.com Date: Thu, 7 Feb 2019 15:58:44 +0300 Message-Id: <1549544327-743817-7-git-send-email-nshirokovskiy@virtuozzo.com> In-Reply-To: <1549544327-743817-1-git-send-email-nshirokovskiy@virtuozzo.com> References: <1549544327-743817-1-git-send-email-nshirokovskiy@virtuozzo.com> X-Greylist: Sender passed SPF test, ACL 242 matched, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Thu, 07 Feb 2019 12:59:09 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Thu, 07 Feb 2019 12:59:09 +0000 (UTC) for IP:'185.231.240.75' DOMAIN:'relay.sw.ru' HELO:'relay.sw.ru' FROM:'nshirokovskiy@virtuozzo.com' RCPT:'' X-RedHat-Spam-Score: -0.001 (SPF_PASS) 185.231.240.75 relay.sw.ru 185.231.240.75 relay.sw.ru X-Scanned-By: MIMEDefang 2.78 on 10.5.110.26 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 6/9] rpc: client: incapsulate error checks 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: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@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 (mx1.redhat.com [10.5.110.25]); Thu, 07 Feb 2019 12:59:17 +0000 (UTC) Content-Type: text/plain; charset="utf-8" Checking virNetClientStreamRaiseError without client lock is racy which is fixed in [1] for example. Thus let's remove such checks when we are sending message to server. And in other cases (like virNetClientStreamRecvHole for example) let's move the check into client stream code. virNetClientStreamRecvPacket already have stream lock so we could introduce another error checking function like virNetClientStreamRaiseError= Locked but as error is set when both client and stream lock are hold we can remove locking from virNetClientStreamRaiseError because all callers hold either client or stream lock. Also let's split virNetClientStreamRaiseErrorLocked into checking state function and checking message send status function. They are same yet. [1] 1b6a29c21: rpc: fix race on stream abort/finish and server side abort Signed-off-by: Nikolay Shirokovskiy --- src/libvirt_remote.syms | 3 ++- src/remote/remote_driver.c | 16 ---------------- src/rpc/virnetclient.c | 4 ++-- src/rpc/virnetclientstream.c | 45 ++++++++++++++++++++++++++++++++++------= ---- src/rpc/virnetclientstream.h | 5 ++++- 5 files changed, 43 insertions(+), 30 deletions(-) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index 88745f2..98586d1 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -54,6 +54,8 @@ virNetClientProgramNew; =20 =20 # rpc/virnetclientstream.h +virNetClientStreamCheckSendStatus; +virNetClientStreamCheckState; virNetClientStreamEOF; virNetClientStreamEventAddCallback; virNetClientStreamEventRemoveCallback; @@ -61,7 +63,6 @@ virNetClientStreamEventUpdateCallback; virNetClientStreamMatches; virNetClientStreamNew; virNetClientStreamQueuePacket; -virNetClientStreamRaiseError; virNetClientStreamRecvHole; virNetClientStreamRecvPacket; virNetClientStreamSendHole; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 058e4c9..1ff55e2 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -5600,9 +5600,6 @@ remoteStreamSend(virStreamPtr st, virNetClientStreamPtr privst =3D st->privateData; int rv; =20 - if (virNetClientStreamRaiseError(privst)) - return -1; - remoteDriverLock(priv); priv->localUses++; remoteDriverUnlock(priv); @@ -5634,9 +5631,6 @@ remoteStreamRecvFlags(virStreamPtr st, =20 virCheckFlags(VIR_STREAM_RECV_STOP_AT_HOLE, -1); =20 - if (virNetClientStreamRaiseError(privst)) - return -1; - remoteDriverLock(priv); priv->localUses++; remoteDriverUnlock(priv); @@ -5676,9 +5670,6 @@ remoteStreamSendHole(virStreamPtr st, virNetClientStreamPtr privst =3D st->privateData; int rv; =20 - if (virNetClientStreamRaiseError(privst)) - return -1; - remoteDriverLock(priv); priv->localUses++; remoteDriverUnlock(priv); @@ -5709,9 +5700,6 @@ remoteStreamRecvHole(virStreamPtr st, =20 virCheckFlags(0, -1); =20 - if (virNetClientStreamRaiseError(privst)) - return -1; - remoteDriverLock(priv); priv->localUses++; remoteDriverUnlock(priv); @@ -5834,9 +5822,6 @@ remoteStreamCloseInt(virStreamPtr st, bool streamAbor= t) =20 remoteDriverLock(priv); =20 - if (virNetClientStreamRaiseError(privst)) - goto cleanup; - priv->localUses++; remoteDriverUnlock(priv); =20 @@ -5849,7 +5834,6 @@ remoteStreamCloseInt(virStreamPtr st, bool streamAbor= t) remoteDriverLock(priv); priv->localUses--; =20 - cleanup: virNetClientRemoveStream(priv->client, privst); virObjectUnref(privst); st->privateData =3D NULL; diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index fcc2e80..70192a9 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -2193,7 +2193,7 @@ int virNetClientSendStream(virNetClientPtr client, =20 virObjectLock(client); =20 - if (virNetClientStreamRaiseError(st)) + if (virNetClientStreamCheckState(st) < 0) goto cleanup; =20 /* Check for EOF only if we are going to wait for incoming data */ @@ -2205,7 +2205,7 @@ int virNetClientSendStream(virNetClientPtr client, if (virNetClientSendInternal(client, msg, expectReply, false) < 0) goto cleanup; =20 - if (virNetClientStreamRaiseError(st)) + if (virNetClientStreamCheckSendStatus(st, msg) < 0) goto cleanup; =20 ret =3D 0; diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c index 136ed16..a7a7824 100644 --- a/src/rpc/virnetclientstream.c +++ b/src/rpc/virnetclientstream.c @@ -184,14 +184,9 @@ bool virNetClientStreamMatches(virNetClientStreamPtr s= t, } =20 =20 -bool virNetClientStreamRaiseError(virNetClientStreamPtr st) +static +void virNetClientStreamRaiseError(virNetClientStreamPtr st) { - virObjectLock(st); - if (st->err.code =3D=3D VIR_ERR_OK) { - virObjectUnlock(st); - return false; - } - virRaiseErrorFull(__FILE__, __FUNCTION__, __LINE__, st->err.domain, st->err.code, @@ -202,8 +197,31 @@ bool virNetClientStreamRaiseError(virNetClientStreamPt= r st) st->err.int1, st->err.int2, "%s", st->err.message ? st->err.message : _("Unknown= error")); - virObjectUnlock(st); - return true; +} + + +/* MUST be called under stream or client lock */ +int virNetClientStreamCheckState(virNetClientStreamPtr st) +{ + if (st->err.code !=3D VIR_ERR_OK) { + virNetClientStreamRaiseError(st); + return -1; + } + + return 0; +} + + +/* MUST be called under stream or client lock */ +int virNetClientStreamCheckSendStatus(virNetClientStreamPtr st, + virNetMessagePtr msg ATTRIBUTE_UNUSE= D) +{ + if (st->err.code !=3D VIR_ERR_OK) { + virNetClientStreamRaiseError(st); + return -1; + } + + return 0; } =20 =20 @@ -474,7 +492,9 @@ int virNetClientStreamRecvPacket(virNetClientStreamPtr = st, virObjectLock(st); =20 reread: - if (!st->rx && !st->incomingEOF) { + if (virNetClientStreamCheckState(st) < 0) { + goto cleanup; + } else if (!st->rx && !st->incomingEOF) { virNetMessagePtr msg; int ret; =20 @@ -646,6 +666,11 @@ virNetClientStreamRecvHole(virNetClientPtr client ATTR= IBUTE_UNUSED, =20 virObjectLock(st); =20 + if (virNetClientStreamCheckState(st) < 0) { + virObjectUnlock(st); + return -1; + } + *length =3D st->holeLength; st->holeLength =3D 0; =20 diff --git a/src/rpc/virnetclientstream.h b/src/rpc/virnetclientstream.h index d137932..d81ec60 100644 --- a/src/rpc/virnetclientstream.h +++ b/src/rpc/virnetclientstream.h @@ -36,7 +36,10 @@ virNetClientStreamPtr virNetClientStreamNew(virStreamPtr= stream, unsigned serial, bool allowSkip); =20 -bool virNetClientStreamRaiseError(virNetClientStreamPtr st); +int virNetClientStreamCheckState(virNetClientStreamPtr st); + +int virNetClientStreamCheckSendStatus(virNetClientStreamPtr st, + virNetMessagePtr msg); =20 int virNetClientStreamSetError(virNetClientStreamPtr st, virNetMessagePtr msg); --=20 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Wed Apr 24 21:59:17 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=fail(p=none dis=none) header.from=virtuozzo.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1549544354982513.769425132323; Thu, 7 Feb 2019 04:59:14 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1BFF0E6A93; Thu, 7 Feb 2019 12:59:13 +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 C7F8E608C2; Thu, 7 Feb 2019 12:59:12 +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 78D2A18005AF; Thu, 7 Feb 2019 12:59:12 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x17CxAja018498 for ; Thu, 7 Feb 2019 07:59:11 -0500 Received: by smtp.corp.redhat.com (Postfix) id E6A24272B8; Thu, 7 Feb 2019 12:59:10 +0000 (UTC) Received: from mx1.redhat.com (ext-mx06.extmail.prod.ext.phx2.redhat.com [10.5.110.30]) by smtp.corp.redhat.com (Postfix) with ESMTPS id DE23D165FD for ; Thu, 7 Feb 2019 12:59:10 +0000 (UTC) Received: from relay.sw.ru (relay.sw.ru [185.231.240.75]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6C4E2356F5 for ; Thu, 7 Feb 2019 12:59:09 +0000 (UTC) Received: from [10.94.3.220] (helo=dim-vz7.qa.sw.ru) by relay.sw.ru with esmtp (Exim 4.91) (envelope-from ) id 1grjGl-0006TA-T9 for libvir-list@redhat.com; Thu, 07 Feb 2019 15:59:07 +0300 From: Nikolay Shirokovskiy To: libvir-list@redhat.com Date: Thu, 7 Feb 2019 15:58:45 +0300 Message-Id: <1549544327-743817-8-git-send-email-nshirokovskiy@virtuozzo.com> In-Reply-To: <1549544327-743817-1-git-send-email-nshirokovskiy@virtuozzo.com> References: <1549544327-743817-1-git-send-email-nshirokovskiy@virtuozzo.com> X-Greylist: Sender passed SPF test, ACL 242 matched, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 07 Feb 2019 12:59:09 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 07 Feb 2019 12:59:09 +0000 (UTC) for IP:'185.231.240.75' DOMAIN:'relay.sw.ru' HELO:'relay.sw.ru' FROM:'nshirokovskiy@virtuozzo.com' RCPT:'' X-RedHat-Spam-Score: -0.001 (SPF_PASS) 185.231.240.75 relay.sw.ru 185.231.240.75 relay.sw.ru X-Scanned-By: MIMEDefang 2.78 on 10.5.110.30 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 7/9] rpc: client: don't set incomingEOF on errors 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: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@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 (mx1.redhat.com [10.5.110.38]); Thu, 07 Feb 2019 12:59:13 +0000 (UTC) Content-Type: text/plain; charset="utf-8" This mixing errors and EOF condition in one flag is odd. Instead let's check st->err.code where appropriate. Signed-off-by: Nikolay Shirokovskiy --- src/rpc/virnetclientstream.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c index a7a7824..713307c 100644 --- a/src/rpc/virnetclientstream.c +++ b/src/rpc/virnetclientstream.c @@ -86,7 +86,7 @@ virNetClientStreamEventTimerUpdate(virNetClientStreamPtr = st) =20 VIR_DEBUG("Check timer rx=3D%p cbEvents=3D%d", st->rx, st->cbEvents); =20 - if (((st->rx || st->incomingEOF) && + if (((st->rx || st->incomingEOF || st->err.code !=3D VIR_ERR_OK) && (st->cbEvents & VIR_STREAM_EVENT_READABLE)) || (st->cbEvents & VIR_STREAM_EVENT_WRITABLE)) { VIR_DEBUG("Enabling event timer"); @@ -108,7 +108,7 @@ virNetClientStreamEventTimer(int timer ATTRIBUTE_UNUSED= , void *opaque) =20 if (st->cb && (st->cbEvents & VIR_STREAM_EVENT_READABLE) && - (st->rx || st->incomingEOF)) + (st->rx || st->incomingEOF || st->err.code !=3D VIR_ERR_OK)) events |=3D VIR_STREAM_EVENT_READABLE; if (st->cb && (st->cbEvents & VIR_STREAM_EVENT_WRITABLE)) @@ -272,7 +272,6 @@ int virNetClientStreamSetError(virNetClientStreamPtr st, st->err.int1 =3D err.int1; st->err.int2 =3D err.int2; =20 - st->incomingEOF =3D true; virNetClientStreamEventTimerUpdate(st); =20 ret =3D 0; --=20 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Wed Apr 24 21:59:17 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=fail(p=none dis=none) header.from=virtuozzo.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1549544363539618.0948993589293; Thu, 7 Feb 2019 04:59:23 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DCCF78F50B; Thu, 7 Feb 2019 12:59:19 +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 92DE5627DE; Thu, 7 Feb 2019 12:59:18 +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 3212D18033AB; Thu, 7 Feb 2019 12:59:17 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x17CxBlM018506 for ; Thu, 7 Feb 2019 07:59:11 -0500 Received: by smtp.corp.redhat.com (Postfix) id 781D0608DA; Thu, 7 Feb 2019 12:59:11 +0000 (UTC) Received: from mx1.redhat.com (ext-mx09.extmail.prod.ext.phx2.redhat.com [10.5.110.38]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 6F50E608C2 for ; Thu, 7 Feb 2019 12:59:11 +0000 (UTC) Received: from relay.sw.ru (relay.sw.ru [185.231.240.75]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B1E47C4EB3 for ; Thu, 7 Feb 2019 12:59:09 +0000 (UTC) Received: from [10.94.3.220] (helo=dim-vz7.qa.sw.ru) by relay.sw.ru with esmtp (Exim 4.91) (envelope-from ) id 1grjGm-0006TA-1K for libvir-list@redhat.com; Thu, 07 Feb 2019 15:59:08 +0300 From: Nikolay Shirokovskiy To: libvir-list@redhat.com Date: Thu, 7 Feb 2019 15:58:46 +0300 Message-Id: <1549544327-743817-9-git-send-email-nshirokovskiy@virtuozzo.com> In-Reply-To: <1549544327-743817-1-git-send-email-nshirokovskiy@virtuozzo.com> References: <1549544327-743817-1-git-send-email-nshirokovskiy@virtuozzo.com> X-Greylist: Sender passed SPF test, ACL 242 matched, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Thu, 07 Feb 2019 12:59:10 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Thu, 07 Feb 2019 12:59:10 +0000 (UTC) for IP:'185.231.240.75' DOMAIN:'relay.sw.ru' HELO:'relay.sw.ru' FROM:'nshirokovskiy@virtuozzo.com' RCPT:'' X-RedHat-Spam-Score: -0.001 (SPF_PASS) 185.231.240.75 relay.sw.ru 185.231.240.75 relay.sw.ru X-Scanned-By: MIMEDefang 2.78 on 10.5.110.38 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 8/9] rpc: client stream: dispose private data on stream dispose 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: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Thu, 07 Feb 2019 12:59:22 +0000 (UTC) Content-Type: text/plain; charset="utf-8" If we call virStreamFinish and virStreamAbort from 2 distinct threads for example we can have access to freed memory. Because when virStreamFinish finishes for example virStreamAbort yet to be finished and it access virNetClientStreamPtr object in stream->privateData. Also it does not make sense to clear @driver field. After stream is finished/aborted it is better to have appropriate error message instead of "unsupported error". This commit reverts [1] or virNetClientStreamPtr and virStreamPtr will never be unrefed due to cyclic dependency. Before this patch we don't have leaks because all execution paths we call virStreamFinish or virStreamAbort. [1] 8b6ffe40 : virNetClientStreamNew: Track origin stream Signed-off-by: Nikolay Shirokovskiy --- src/datatypes.c | 2 ++ src/datatypes.h | 1 + src/remote/remote_driver.c | 11 ++++------- src/rpc/gendispatch.pl | 3 ++- src/rpc/virnetclientstream.c | 7 +------ src/rpc/virnetclientstream.h | 3 +-- 6 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/datatypes.c b/src/datatypes.c index 09f63d9..be9b528 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -763,6 +763,8 @@ virStreamDispose(void *obj) virStreamPtr st =3D obj; VIR_DEBUG("release dev %p", st); =20 + if (st->ff) + st->ff(st->privateData); virObjectUnref(st->conn); } =20 diff --git a/src/datatypes.h b/src/datatypes.h index 529b340..1201567 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -665,6 +665,7 @@ struct _virStream { =20 virStreamDriverPtr driver; void *privateData; + virFreeCallback ff; }; =20 /** diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 1ff55e2..2861ee6 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -5835,9 +5835,6 @@ remoteStreamCloseInt(virStreamPtr st, bool streamAbor= t) priv->localUses--; =20 virNetClientRemoveStream(priv->client, privst); - virObjectUnref(privst); - st->privateData =3D NULL; - st->driver =3D NULL; =20 remoteDriverUnlock(priv); return ret; @@ -6177,8 +6174,7 @@ remoteDomainMigratePrepareTunnel3(virConnectPtr dconn, memset(&args, 0, sizeof(args)); memset(&ret, 0, sizeof(ret)); =20 - if (!(netst =3D virNetClientStreamNew(st, - priv->remoteProgram, + if (!(netst =3D virNetClientStreamNew(priv->remoteProgram, REMOTE_PROC_DOMAIN_MIGRATE_PREPARE= _TUNNEL3, priv->counter, false))) @@ -6191,6 +6187,7 @@ remoteDomainMigratePrepareTunnel3(virConnectPtr dconn, =20 st->driver =3D &remoteStreamDrv; st->privateData =3D netst; + st->ff =3D virObjectFreeCallback; =20 args.cookie_in.cookie_in_val =3D (char *)cookiein; args.cookie_in.cookie_in_len =3D cookieinlen; @@ -7142,8 +7139,7 @@ remoteDomainMigratePrepareTunnel3Params(virConnectPtr= dconn, goto cleanup; } =20 - if (!(netst =3D virNetClientStreamNew(st, - priv->remoteProgram, + if (!(netst =3D virNetClientStreamNew(priv->remoteProgram, REMOTE_PROC_DOMAIN_MIGRATE_PREPARE= _TUNNEL3_PARAMS, priv->counter, false))) @@ -7156,6 +7152,7 @@ remoteDomainMigratePrepareTunnel3Params(virConnectPtr= dconn, =20 st->driver =3D &remoteStreamDrv; st->privateData =3D netst; + st->ff =3D virObjectFreeCallback; =20 if (call(dconn, priv, 0, REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL3_PA= RAMS, (xdrproc_t) xdr_remote_domain_migrate_prepare_tunnel3_params_= args, diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index ce4db5d..ae3a42c 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -1804,7 +1804,7 @@ elsif ($mode eq "client") { =20 if ($call->{streamflag} ne "none") { print "\n"; - print " if (!(netst =3D virNetClientStreamNew(st, priv->rem= oteProgram, $call->{constname}, priv->counter, sparse)))\n"; + print " if (!(netst =3D virNetClientStreamNew(priv->remoteP= rogram, $call->{constname}, priv->counter, sparse)))\n"; print " goto done;\n"; print "\n"; print " if (virNetClientAddStream(priv->client, netst) < 0)= {\n"; @@ -1814,6 +1814,7 @@ elsif ($mode eq "client") { print "\n"; print " st->driver =3D &remoteStreamDrv;\n"; print " st->privateData =3D netst;\n"; + print " st->ff =3D virObjectFreeCallback;\n"; } =20 if ($call->{ProcName} eq "SupportsFeature") { diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c index 713307c..cfdaa74 100644 --- a/src/rpc/virnetclientstream.c +++ b/src/rpc/virnetclientstream.c @@ -34,8 +34,6 @@ VIR_LOG_INIT("rpc.netclientstream"); struct _virNetClientStream { virObjectLockable parent; =20 - virStreamPtr stream; /* Reverse pointer to parent stream */ - virNetClientProgramPtr prog; int proc; unsigned serial; @@ -133,8 +131,7 @@ virNetClientStreamEventTimer(int timer ATTRIBUTE_UNUSED= , void *opaque) } =20 =20 -virNetClientStreamPtr virNetClientStreamNew(virStreamPtr stream, - virNetClientProgramPtr prog, +virNetClientStreamPtr virNetClientStreamNew(virNetClientProgramPtr prog, int proc, unsigned serial, bool allowSkip) @@ -147,7 +144,6 @@ virNetClientStreamPtr virNetClientStreamNew(virStreamPt= r stream, if (!(st =3D virObjectLockableNew(virNetClientStreamClass))) return NULL; =20 - st->stream =3D virObjectRef(stream); st->prog =3D virObjectRef(prog); st->proc =3D proc; st->serial =3D serial; @@ -167,7 +163,6 @@ void virNetClientStreamDispose(void *obj) virNetMessageFree(msg); } virObjectUnref(st->prog); - virObjectUnref(st->stream); } =20 bool virNetClientStreamMatches(virNetClientStreamPtr st, diff --git a/src/rpc/virnetclientstream.h b/src/rpc/virnetclientstream.h index d81ec60..49b74bc 100644 --- a/src/rpc/virnetclientstream.h +++ b/src/rpc/virnetclientstream.h @@ -30,8 +30,7 @@ typedef virNetClientStream *virNetClientStreamPtr; typedef void (*virNetClientStreamEventCallback)(virNetClientStreamPtr stre= am, int events, void *opaque); =20 -virNetClientStreamPtr virNetClientStreamNew(virStreamPtr stream, - virNetClientProgramPtr prog, +virNetClientStreamPtr virNetClientStreamNew(virNetClientProgramPtr prog, int proc, unsigned serial, bool allowSkip); --=20 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Wed Apr 24 21:59:17 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=fail(p=none dis=none) header.from=virtuozzo.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1549544368382624.9758661322116; Thu, 7 Feb 2019 04:59:28 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5E950B1E0; Thu, 7 Feb 2019 12:59:26 +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 ECAA57C82B; Thu, 7 Feb 2019 12:59:25 +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 8893718033CC; Thu, 7 Feb 2019 12:59:25 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x17CxFQN018544 for ; Thu, 7 Feb 2019 07:59:15 -0500 Received: by smtp.corp.redhat.com (Postfix) id 006C9627DE; Thu, 7 Feb 2019 12:59:15 +0000 (UTC) Received: from mx1.redhat.com (ext-mx06.extmail.prod.ext.phx2.redhat.com [10.5.110.30]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 3806A62943 for ; Thu, 7 Feb 2019 12:59:11 +0000 (UTC) Received: from relay.sw.ru (relay.sw.ru [185.231.240.75]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C2656356E5 for ; Thu, 7 Feb 2019 12:59:09 +0000 (UTC) Received: from [10.94.3.220] (helo=dim-vz7.qa.sw.ru) by relay.sw.ru with esmtp (Exim 4.91) (envelope-from ) id 1grjGm-0006TA-7m for libvir-list@redhat.com; Thu, 07 Feb 2019 15:59:08 +0300 From: Nikolay Shirokovskiy To: libvir-list@redhat.com Date: Thu, 7 Feb 2019 15:58:47 +0300 Message-Id: <1549544327-743817-10-git-send-email-nshirokovskiy@virtuozzo.com> In-Reply-To: <1549544327-743817-1-git-send-email-nshirokovskiy@virtuozzo.com> References: <1549544327-743817-1-git-send-email-nshirokovskiy@virtuozzo.com> X-Greylist: Sender passed SPF test, ACL 242 matched, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 07 Feb 2019 12:59:10 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 07 Feb 2019 12:59:10 +0000 (UTC) for IP:'185.231.240.75' DOMAIN:'relay.sw.ru' HELO:'relay.sw.ru' FROM:'nshirokovskiy@virtuozzo.com' RCPT:'' X-RedHat-Spam-Score: -0.001 (SPF_PASS) 185.231.240.75 relay.sw.ru 185.231.240.75 relay.sw.ru X-Scanned-By: MIMEDefang 2.78 on 10.5.110.30 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 9/9] rpc: client: stream: fix multi thread abort/finish 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: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@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 (mx1.redhat.com [10.5.110.28]); Thu, 07 Feb 2019 12:59:27 +0000 (UTC) Content-Type: text/plain; charset="utf-8" If 2 threads call abort for example then one of them will hang because client will send 2 abort messages and server will reply only on first of them, the second will be ignored. And on server reply client changes the state only one of abort message to complete, the second will hang forever. There are other similar issues. We should complete all messages waiting reply if we got error or expected abort/finish reply from server. Also if one thread send finish and another abort one of them will win the race and server will either abort or finish stream. If stream is aborted then thread requested finishing should report error. In order to archive this let's keep stream closing reason in @closed field. If we receive VIR_NET_OK message for stream then stream is finished if oldest (closest to queue end) message in stream queue is finish message and stream is aborted if oldest message is abort message. Otherwise it is protocol error. By the way we need to fix case of receiving VIR_NET_CONTINUE message. Now we take oldest message in queue and check if this is dummy message. If one thread first sends abort and second thread then receives data then oldest message is abort message and second thread won't be notified when data arrives. Let's find oldest dummy message instead. Signed-off-by: Nikolay Shirokovskiy --- src/rpc/virnetclient.c | 74 ++++++++++++++++++++++++++++------------= ---- src/rpc/virnetclientstream.c | 47 +++++++++++++++++++++++++--- src/rpc/virnetclientstream.h | 9 ++++++ 3 files changed, 100 insertions(+), 30 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 70192a9..64855fb 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -1158,6 +1158,19 @@ static int virNetClientCallDispatchMessage(virNetCli= entPtr client) return 0; } =20 +static void virNetClientCallCompleteAllWaitingReply(virNetClientPtr client) +{ + virNetClientCallPtr call; + + for (call =3D client->waitDispatch; call; call =3D call->next) { + if (call->msg->header.prog =3D=3D client->msg.header.prog && + call->msg->header.vers =3D=3D client->msg.header.vers && + call->msg->header.serial =3D=3D client->msg.header.serial && + call->expectReply) + call->mode =3D VIR_NET_CLIENT_MODE_COMPLETE; + } +} + static int virNetClientCallDispatchStream(virNetClientPtr client) { size_t i; @@ -1181,16 +1194,6 @@ static int virNetClientCallDispatchStream(virNetClie= ntPtr client) return 0; } =20 - /* Finish/Abort are synchronous, so also see if there's an - * (optional) call waiting for this stream packet */ - thecall =3D client->waitDispatch; - while (thecall && - !(thecall->msg->header.prog =3D=3D client->msg.header.prog && - thecall->msg->header.vers =3D=3D client->msg.header.vers && - thecall->msg->header.serial =3D=3D client->msg.header.serial)) - thecall =3D thecall->next; - - VIR_DEBUG("Found call %p", thecall); =20 /* Status is either * - VIR_NET_OK - no payload for streams @@ -1202,25 +1205,47 @@ static int virNetClientCallDispatchStream(virNetCli= entPtr client) if (virNetClientStreamQueuePacket(st, &client->msg) < 0) return -1; =20 - if (thecall && thecall->expectReply) { - if (thecall->msg->header.status =3D=3D VIR_NET_CONTINUE) { - VIR_DEBUG("Got a synchronous confirm"); - thecall->mode =3D VIR_NET_CLIENT_MODE_COMPLETE; - } else { - VIR_DEBUG("Not completing call with status %d", thecall->m= sg->header.status); - } + /* Find oldest dummy message waiting for incoming data. */ + for (thecall =3D client->waitDispatch; thecall; thecall =3D thecal= l->next) { + if (thecall->msg->header.prog =3D=3D client->msg.header.prog && + thecall->msg->header.vers =3D=3D client->msg.header.vers && + thecall->msg->header.serial =3D=3D client->msg.header.seri= al && + thecall->expectReply && + thecall->msg->header.status =3D=3D VIR_NET_CONTINUE) + break; + } + + if (thecall) { + VIR_DEBUG("Got a new incoming stream data"); + thecall->mode =3D VIR_NET_CLIENT_MODE_COMPLETE; } return 0; } =20 case VIR_NET_OK: - if (thecall && thecall->expectReply) { - VIR_DEBUG("Got a synchronous confirm"); - thecall->mode =3D VIR_NET_CLIENT_MODE_COMPLETE; - } else { + /* Find oldest abort/finish message. */ + for (thecall =3D client->waitDispatch; thecall; thecall =3D thecal= l->next) { + if (thecall->msg->header.prog =3D=3D client->msg.header.prog && + thecall->msg->header.vers =3D=3D client->msg.header.vers && + thecall->msg->header.serial =3D=3D client->msg.header.seri= al && + thecall->expectReply && + thecall->msg->header.status !=3D VIR_NET_CONTINUE) + break; + } + + if (!thecall) { VIR_DEBUG("Got unexpected async stream finish confirmation"); return -1; } + + VIR_DEBUG("Got a synchronous abort/finish confirm"); + + virNetClientStreamSetClosed(st, + thecall->msg->header.status =3D=3D VIR= _NET_OK ? + VIR_NET_CLIENT_STREAM_CLOSED_FINIS= HED : + VIR_NET_CLIENT_STREAM_CLOSED_ABORT= ED); + + virNetClientCallCompleteAllWaitingReply(client); return 0; =20 case VIR_NET_ERROR: @@ -1228,10 +1253,7 @@ static int virNetClientCallDispatchStream(virNetClie= ntPtr client) if (virNetClientStreamSetError(st, &client->msg) < 0) return -1; =20 - if (thecall && thecall->expectReply) { - VIR_DEBUG("Got a synchronous error"); - thecall->mode =3D VIR_NET_CLIENT_MODE_COMPLETE; - } + virNetClientCallCompleteAllWaitingReply(client); return 0; =20 default: @@ -2205,7 +2227,7 @@ int virNetClientSendStream(virNetClientPtr client, if (virNetClientSendInternal(client, msg, expectReply, false) < 0) goto cleanup; =20 - if (virNetClientStreamCheckSendStatus(st, msg) < 0) + if (expectReply && virNetClientStreamCheckSendStatus(st, msg) < 0) goto cleanup; =20 ret =3D 0; diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c index cfdaa74..583cd369 100644 --- a/src/rpc/virnetclientstream.c +++ b/src/rpc/virnetclientstream.c @@ -49,6 +49,7 @@ struct _virNetClientStream { */ virNetMessagePtr rx; bool incomingEOF; + int closed; /* enum virNetClientStreamClosed */ =20 bool allowSkip; long long holeLength; /* Size of incoming hole in stream. */ @@ -84,7 +85,7 @@ virNetClientStreamEventTimerUpdate(virNetClientStreamPtr = st) =20 VIR_DEBUG("Check timer rx=3D%p cbEvents=3D%d", st->rx, st->cbEvents); =20 - if (((st->rx || st->incomingEOF || st->err.code !=3D VIR_ERR_OK) && + if (((st->rx || st->incomingEOF || st->err.code !=3D VIR_ERR_OK || st-= >closed) && (st->cbEvents & VIR_STREAM_EVENT_READABLE)) || (st->cbEvents & VIR_STREAM_EVENT_WRITABLE)) { VIR_DEBUG("Enabling event timer"); @@ -106,7 +107,7 @@ virNetClientStreamEventTimer(int timer ATTRIBUTE_UNUSED= , void *opaque) =20 if (st->cb && (st->cbEvents & VIR_STREAM_EVENT_READABLE) && - (st->rx || st->incomingEOF || st->err.code !=3D VIR_ERR_OK)) + (st->rx || st->incomingEOF || st->err.code !=3D VIR_ERR_OK || st->= closed)) events |=3D VIR_STREAM_EVENT_READABLE; if (st->cb && (st->cbEvents & VIR_STREAM_EVENT_WRITABLE)) @@ -203,23 +204,61 @@ int virNetClientStreamCheckState(virNetClientStreamPt= r st) return -1; } =20 + if (st->closed) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("stream is closed")); + return -1; + } + return 0; } =20 =20 -/* MUST be called under stream or client lock */ +/* MUST be called under stream or client lock. This should + * be called only for message that expect reply. */ int virNetClientStreamCheckSendStatus(virNetClientStreamPtr st, - virNetMessagePtr msg ATTRIBUTE_UNUSE= D) + virNetMessagePtr msg) { if (st->err.code !=3D VIR_ERR_OK) { virNetClientStreamRaiseError(st); return -1; } =20 + /* We can not check if the message is dummy in a usual way + * by checking msg->bufferLength because at this point message payload + * is cleared. As caller must not call this function for messages + * not expecting reply we can check for dummy messages just by status. + */ + if (msg->header.status =3D=3D VIR_NET_CONTINUE) { + if (st->closed) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("stream is closed")); + return -1; + } + return 0; + } else if (msg->header.status =3D=3D VIR_NET_OK && + st->closed !=3D VIR_NET_CLIENT_STREAM_CLOSED_FINISHED) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("stream aborted by another thread")); + return -1; + } + return 0; } =20 =20 +void virNetClientStreamSetClosed(virNetClientStreamPtr st, + int closed) +{ + virObjectLock(st); + + st->closed =3D closed; + virNetClientStreamEventTimerUpdate(st); + + virObjectUnlock(st); +} + + int virNetClientStreamSetError(virNetClientStreamPtr st, virNetMessagePtr msg) { diff --git a/src/rpc/virnetclientstream.h b/src/rpc/virnetclientstream.h index 49b74bc..cb28428 100644 --- a/src/rpc/virnetclientstream.h +++ b/src/rpc/virnetclientstream.h @@ -27,6 +27,12 @@ typedef struct _virNetClientStream virNetClientStream; typedef virNetClientStream *virNetClientStreamPtr; =20 +typedef enum { + VIR_NET_CLIENT_STREAM_CLOSED_NOT =3D 0, + VIR_NET_CLIENT_STREAM_CLOSED_FINISHED, + VIR_NET_CLIENT_STREAM_CLOSED_ABORTED, +} virNetClientStreamClosed; + typedef void (*virNetClientStreamEventCallback)(virNetClientStreamPtr stre= am, int events, void *opaque); =20 @@ -43,6 +49,9 @@ int virNetClientStreamCheckSendStatus(virNetClientStreamP= tr st, int virNetClientStreamSetError(virNetClientStreamPtr st, virNetMessagePtr msg); =20 +void virNetClientStreamSetClosed(virNetClientStreamPtr st, + int closed); + bool virNetClientStreamMatches(virNetClientStreamPtr st, virNetMessagePtr msg); =20 --=20 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list