From nobody Thu May 2 15:09:08 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 1550239834154529.8506773133257; Fri, 15 Feb 2019 06:10:34 -0800 (PST) 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 mx1.redhat.com (Postfix) with ESMTPS id 6139F7AEAF; Fri, 15 Feb 2019 14:10: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 DC2355F9C0; Fri, 15 Feb 2019 14:10: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 21B7E3F7D5; Fri, 15 Feb 2019 14:10: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 x1FEAQG8029191 for ; Fri, 15 Feb 2019 09:10:26 -0500 Received: by smtp.corp.redhat.com (Postfix) id B0EEC1024904; Fri, 15 Feb 2019 14:10:26 +0000 (UTC) Received: from mx1.redhat.com (ext-mx10.extmail.prod.ext.phx2.redhat.com [10.5.110.39]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A6931101E843 for ; Fri, 15 Feb 2019 14:10:23 +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 B3B3C1301D5 for ; Fri, 15 Feb 2019 14:10:21 +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 1gueC3-00053q-BE for libvir-list@redhat.com; Fri, 15 Feb 2019 17:10:19 +0300 From: Nikolay Shirokovskiy To: libvir-list@redhat.com Date: Fri, 15 Feb 2019 17:10:12 +0300 Message-Id: <1550239813-870626-2-git-send-email-nshirokovskiy@virtuozzo.com> In-Reply-To: <1550239813-870626-1-git-send-email-nshirokovskiy@virtuozzo.com> References: <1550239813-870626-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.39]); Fri, 15 Feb 2019 14:10:22 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Fri, 15 Feb 2019 14:10:22 +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.39 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 1/2] rpc: client: fix race on stream error and stream creation 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.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Fri, 15 Feb 2019 14:10:32 +0000 (UTC) Content-Type: text/plain; charset="utf-8" Message of API call that creates stream and stream itself have same rpc serial. This can lead to issues. If stream got error immediately after creation then notification can be delivered before API call reply arrived. This is possible because the reply and the error message are sent from different threads (rpc worker thread and event loop thread respectively). As we don't check for message type in virNetClientCallCompleteAllWaitingReply we complete the API call which leads to API call error [1] as there is no actual reply. Later when reply arrives connection will be closed due to protocol error (see check in virNetClientCallDispatch= Reply). Let's fix virNetClientCallCompleteAllWaitingReply. Queue inspection on arriving VIR_NET_CONTINUE message in virNetClientCallDispatchStream is safe because there we check for status field also. Queue inspection on arriving VIR_NET_OK is safe too as this message can not arrive before we call virFinishAbort(Finish) which is not possible before API call that creates streams returns. But just to be sure let's add checking message type in these places too. [1] error: internal error: Unexpected message type 0 Signed-off-by: Nikolay Shirokovskiy --- src/rpc/virnetclient.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 64855fb..0393587 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -1163,7 +1163,8 @@ static void virNetClientCallCompleteAllWaitingReply(v= irNetClientPtr client) virNetClientCallPtr call; =20 for (call =3D client->waitDispatch; call; call =3D call->next) { - if (call->msg->header.prog =3D=3D client->msg.header.prog && + if (call->msg->header.type =3D=3D VIR_NET_STREAM && + 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) @@ -1207,7 +1208,8 @@ static int virNetClientCallDispatchStream(virNetClien= tPtr client) =20 /* 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 && + if (thecall->msg->header.type =3D=3D VIR_NET_STREAM && + 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 && @@ -1225,7 +1227,8 @@ static int virNetClientCallDispatchStream(virNetClien= tPtr client) case VIR_NET_OK: /* 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 && + if (thecall->msg->header.type =3D=3D VIR_NET_STREAM && + 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 && --=20 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Thu May 2 15:09:08 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 1550239835216648.5771223424191; Fri, 15 Feb 2019 06:10:35 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 15EA3315FCC; Fri, 15 Feb 2019 14:10:31 +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 D5661600C1; Fri, 15 Feb 2019 14:10:30 +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 674D0181A00C; Fri, 15 Feb 2019 14:10:30 +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 x1FEATHc029211 for ; Fri, 15 Feb 2019 09:10:29 -0500 Received: by smtp.corp.redhat.com (Postfix) id CBD6460C69; Fri, 15 Feb 2019 14:10:29 +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 C191260C6B for ; Fri, 15 Feb 2019 14:10:23 +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 A911F8E37A for ; Fri, 15 Feb 2019 14:10:21 +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 1gueC3-00053q-HB for libvir-list@redhat.com; Fri, 15 Feb 2019 17:10:19 +0300 From: Nikolay Shirokovskiy To: libvir-list@redhat.com Date: Fri, 15 Feb 2019 17:10:13 +0300 Message-Id: <1550239813-870626-3-git-send-email-nshirokovskiy@virtuozzo.com> In-Reply-To: <1550239813-870626-1-git-send-email-nshirokovskiy@virtuozzo.com> References: <1550239813-870626-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]); Fri, 15 Feb 2019 14:10:22 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Fri, 15 Feb 2019 14:10:22 +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.12 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 2/2] rpc: client: stream: notify streams of closing connection 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.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Fri, 15 Feb 2019 14:10:34 +0000 (UTC) Content-Type: text/plain; charset="utf-8" It not done yet. As a result if we doing 'virsh console' and libvirtd is killed we get [1] message as virsh tracks connection status but virsh itself won't exit because it won't get notification that stream is broken. Only after we press a key and virsh tries to write that key code to stream we get an error. So let's add notification that stream's underlying connection is closed. [1] error: Disconnected from qemu:///system due to end of file Signed-off-by: Nikolay Shirokovskiy --- src/rpc/virnetclient.c | 4 ++++ src/rpc/virnetclientstream.c | 30 ++++++++++++++++++++++++++++-- src/rpc/virnetclientstream.h | 2 ++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 0393587..3fa0320 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -785,6 +785,7 @@ static void virNetClientCloseLocked(virNetClientPtr client) { virKeepAlivePtr ka; + size_t i; =20 VIR_DEBUG("client=3D%p, sock=3D%p, reason=3D%d", client, client->sock,= client->closeReason); =20 @@ -825,6 +826,9 @@ virNetClientCloseLocked(virNetClientPtr client) virObjectLock(client); virObjectUnref(client); } + + for (i =3D 0; i < client->nstreams; i++) + virNetClientStreamSetClientClosed(client->streams[i]); } =20 static void virNetClientCloseInternal(virNetClientPtr client, diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c index 834c448..de0c961 100644 --- a/src/rpc/virnetclientstream.c +++ b/src/rpc/virnetclientstream.c @@ -50,6 +50,7 @@ struct _virNetClientStream { virNetMessagePtr rx; bool incomingEOF; virNetClientStreamClosed closed; + bool clientClosed; =20 bool allowSkip; long long holeLength; /* Size of incoming hole in stream. */ @@ -85,7 +86,11 @@ 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 || st-= >closed) && + if (((st->rx || + st->incomingEOF || + st->err.code !=3D VIR_ERR_OK || + st->closed || + st->clientClosed) && (st->cbEvents & VIR_STREAM_EVENT_READABLE)) || (st->cbEvents & VIR_STREAM_EVENT_WRITABLE)) { VIR_DEBUG("Enabling event timer"); @@ -107,7 +112,11 @@ virNetClientStreamEventTimer(int timer ATTRIBUTE_UNUSE= D, void *opaque) =20 if (st->cb && (st->cbEvents & VIR_STREAM_EVENT_READABLE) && - (st->rx || st->incomingEOF || st->err.code !=3D VIR_ERR_OK || st->= closed)) + (st->rx || + st->incomingEOF || + st->err.code !=3D VIR_ERR_OK || + st->closed || + st->clientClosed)) events |=3D VIR_STREAM_EVENT_READABLE; if (st->cb && (st->cbEvents & VIR_STREAM_EVENT_WRITABLE)) @@ -204,6 +213,12 @@ int virNetClientStreamCheckState(virNetClientStreamPtr= st) return -1; } =20 + if (st->clientClosed) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("client socket is closed")); + return -1; + } + if (st->closed) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("stream is closed")); @@ -247,6 +262,17 @@ int virNetClientStreamCheckSendStatus(virNetClientStre= amPtr st, } =20 =20 +void virNetClientStreamSetClientClosed(virNetClientStreamPtr st) +{ + virObjectLock(st); + + st->clientClosed =3D true; + virNetClientStreamEventTimerUpdate(st); + + virObjectUnlock(st); +} + + void virNetClientStreamSetClosed(virNetClientStreamPtr st, virNetClientStreamClosed closed) { diff --git a/src/rpc/virnetclientstream.h b/src/rpc/virnetclientstream.h index 33a2af3..f1df4e2 100644 --- a/src/rpc/virnetclientstream.h +++ b/src/rpc/virnetclientstream.h @@ -49,6 +49,8 @@ int virNetClientStreamCheckSendStatus(virNetClientStreamP= tr st, int virNetClientStreamSetError(virNetClientStreamPtr st, virNetMessagePtr msg); =20 +void virNetClientStreamSetClientClosed(virNetClientStreamPtr st); + void virNetClientStreamSetClosed(virNetClientStreamPtr st, virNetClientStreamClosed closed); =20 --=20 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list