From nobody Sun Nov 2 11:47:01 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (208.118.235.17 [208.118.235.17]) by mx.zohomail.com with SMTPS id 1528099665489646.1516980787943; Mon, 4 Jun 2018 01:07:45 -0700 (PDT) Received: from localhost ([::1]:38246 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fPkWc-0004wR-EK for importer@patchew.org; Mon, 04 Jun 2018 04:07:34 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51930) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fPkVm-0004f1-9z for qemu-devel@nongnu.org; Mon, 04 Jun 2018 04:06:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fPkVi-0003tB-Aa for qemu-devel@nongnu.org; Mon, 04 Jun 2018 04:06:42 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:56068 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fPkVi-0003sl-5f for qemu-devel@nongnu.org; Mon, 04 Jun 2018 04:06:38 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 714DD401EF05 for ; Mon, 4 Jun 2018 08:06:37 +0000 (UTC) Received: from xz-mi.nay.redhat.com (dhcp-14-151.nay.redhat.com [10.66.14.151]) by smtp.corp.redhat.com (Postfix) with ESMTP id 248AC217B400; Mon, 4 Jun 2018 08:06:31 +0000 (UTC) From: Peter Xu To: qemu-devel@nongnu.org Date: Mon, 4 Jun 2018 16:06:30 +0800 Message-Id: <20180604080630.10946-1-peterx@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Mon, 04 Jun 2018 08:06:37 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Mon, 04 Jun 2018 08:06:37 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'peterx@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH] monitor: postpone monitor_qmp_cleanup_queues X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , John Snow , Markus Armbruster , peterx@redhat.com, Max Reitz , Stefan Hajnoczi , =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= , Paolo Bonzini , "Dr . David Alan Gilbert" Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Previously we cleanup the queues when we got CLOSED event. It was used to make sure we won't leftover replies/events of a old client to a new client. Now this patch postpones that until OPENED. In most cases, it does not really matter much since either way will make sure that the new client won't receive unexpected old events/responses. However there can be a very rare race condition with the old way when we use QMP with stdio and pipelines (which is quite common in iotests). The problem is that, we can easily have something like this in scripts: cat $QMP_COMMANDS | qemu -qmp stdio ... | filter_commands In most cases, a QMP session will be based on a bidirectional channel (read/write to a TCP port, for example), so IN and OUT are sharing some fundamentally same thing. However that's untrue for pipes above - the IN is the "cat" program, while the "OUT" is directed to the "filter_commands" which is actually another process. Now when we received the CLOSED event, we cleanup the queues (including the QMP response queue). That means, if we kill/stop the "cat" process faster than the filter_commands process, the filter_commands process is possible to miss some responses/events that should belong to it. In practise, I encountered a very strange SHUTDOWN event missing when running test with iotest 087. Without this patch, iotest 078 will have ~10% chance to miss the SHUTDOWN event and fail when with Out-Of-Band enabled: 087 8s ... - output mismatch (see 087.out.bad) --- /home/peterx/git/qemu/tests/qemu-iotests/087.out 2018-06-01 18:44:22= .378982462 +0800 +++ /home/peterx/git/qemu/bin/tests/qemu-iotests/087.out.bad 2018-06-01 = 18:53:44.267840928 +0800 @@ -8,7 +8,6 @@ {"return": {}} {"error": {"class": "GenericError", "desc": "'node-name' must be specified= for the root node"}} {"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event"= : "SHUTDOWN", "data": {"guest": false}} =3D=3D=3D Duplicate ID =3D=3D=3D @@ -53,7 +52,6 @@ {"return": {}} {"return": {}} {"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event"= : "SHUTDOWN", "data": {"guest": false}} This patch fixes the problem. Fixes: 6d2d563f8c ("qmp: cleanup qmp queues properly", 2018-03-27) Signed-off-by: Peter Xu Signed-off-by: Peter Xu --- monitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monitor.c b/monitor.c index 46814af533..6f26108108 100644 --- a/monitor.c +++ b/monitor.c @@ -4354,6 +4354,7 @@ static void monitor_qmp_event(void *opaque, int event) =20 switch (event) { case CHR_EVENT_OPENED: + monitor_qmp_cleanup_queues(mon); mon->qmp.commands =3D &qmp_cap_negotiation_commands; monitor_qmp_caps_reset(mon); data =3D get_qmp_greeting(mon); @@ -4362,7 +4363,6 @@ static void monitor_qmp_event(void *opaque, int event) mon_refcount++; break; case CHR_EVENT_CLOSED: - monitor_qmp_cleanup_queues(mon); json_message_parser_destroy(&mon->qmp.parser); json_message_parser_init(&mon->qmp.parser, handle_qmp_command); mon_refcount--; --=20 2.17.0