From nobody Tue Feb 10 00:23:19 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.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 209.51.188.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 (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1550076015889435.80480329847967; Wed, 13 Feb 2019 08:40:15 -0800 (PST) Received: from localhost ([127.0.0.1]:59837 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gtxa0-0003kj-Li for importer@patchew.org; Wed, 13 Feb 2019 11:40:12 -0500 Received: from eggs.gnu.org ([209.51.188.92]:39192) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gtxG5-000450-Q4 for qemu-devel@nongnu.org; Wed, 13 Feb 2019 11:19:40 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gtxG2-0005Js-44 for qemu-devel@nongnu.org; Wed, 13 Feb 2019 11:19:37 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48156) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gtxG1-000597-F7 for qemu-devel@nongnu.org; Wed, 13 Feb 2019 11:19:33 -0500 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 6275BC04959D; Wed, 13 Feb 2019 16:19:17 +0000 (UTC) Received: from localhost (ovpn-112-60.ams2.redhat.com [10.36.112.60]) by smtp.corp.redhat.com (Postfix) with ESMTP id B4EB960856; Wed, 13 Feb 2019 16:19:16 +0000 (UTC) From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= To: qemu-devel@nongnu.org Date: Wed, 13 Feb 2019 17:18:49 +0100 Message-Id: <20190213161913.22107-2-marcandre.lureau@redhat.com> In-Reply-To: <20190213161913.22107-1-marcandre.lureau@redhat.com> References: <20190213161913.22107-1-marcandre.lureau@redhat.com> MIME-Version: 1.0 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.31]); Wed, 13 Feb 2019 16:19:17 +0000 (UTC) Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL v2 01/25] chardev: fix mess in OPENED/CLOSED events when muxed 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: peter.maydell@linaro.org Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Type: text/plain; charset="utf-8" From: Artem Pisarenko When chardev is multiplexed (mux=3Don) there are a lot of cases where CHR_EVENT_OPENED/CHR_EVENT_CLOSED events pairing (expected from frontend side) is broken. There are either generation of multiple repeated or extra CHR_EVENT_OPENED events, or CHR_EVENT_CLOSED just isn't generated at all. This is mostly because 'qemu_chr_fe_set_handlers()' function makes its own (and often wrong) implicit decision on updated frontend state and invokes 'fd_event' callback with 'CHR_EVENT_OPENED'. And even worse, it doesn't do symmetric action in opposite direction, as someone may expect (i.e. it doesn't invoke previously set 'fd_event' with 'CHR_EVENT_CLOSED'). Muxed chardev uses trick by calling this function again to replace callback handlers with its own ones, but it doesn't account for such side effect. Fix that using extended version of this function with added argument for disabling side effect and keep original function for compatibility with lots of frontends already using this interface and being "tolerant" to its side effects. One more source of event duplication is just line of code in char-mux.c, which does far more than comment above says (obvious fix). Signed-off-by: Artem Pisarenko Reviewed-by: Marc-Andr=C3=A9 Lureau Message-Id: <7dde6abbd21682857f8294644013173c0b9949b3.1541507990.git.artem.= k.pisarenko@gmail.com> Signed-off-by: Marc-Andr=C3=A9 Lureau --- include/chardev/char-fe.h | 18 +++++++++++++++++- chardev/char-fe.c | 33 ++++++++++++++++++++++++--------- chardev/char-mux.c | 16 ++++++++-------- 3 files changed, 49 insertions(+), 18 deletions(-) diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h index 46c997d352..c1b7fd9a95 100644 --- a/include/chardev/char-fe.h +++ b/include/chardev/char-fe.h @@ -67,7 +67,7 @@ bool qemu_chr_fe_backend_connected(CharBackend *be); bool qemu_chr_fe_backend_open(CharBackend *be); =20 /** - * qemu_chr_fe_set_handlers: + * qemu_chr_fe_set_handlers_full: * @b: a CharBackend * @fd_can_read: callback to get the amount of data the frontend may * receive @@ -79,12 +79,28 @@ bool qemu_chr_fe_backend_open(CharBackend *be); * @context: a main loop context or NULL for the default * @set_open: whether to call qemu_chr_fe_set_open() implicitely when * any of the handler is non-NULL + * @sync_state: whether to issue event callback with updated state * * Set the front end char handlers. The front end takes the focus if * any of the handler is non-NULL. * * Without associated Chardev, nothing is changed. */ +void qemu_chr_fe_set_handlers_full(CharBackend *b, + IOCanReadHandler *fd_can_read, + IOReadHandler *fd_read, + IOEventHandler *fd_event, + BackendChangeHandler *be_change, + void *opaque, + GMainContext *context, + bool set_open, + bool sync_state); + +/** + * qemu_chr_fe_set_handlers: + * + * Version of qemu_chr_fe_set_handlers_full() with sync_state =3D true. + */ void qemu_chr_fe_set_handlers(CharBackend *b, IOCanReadHandler *fd_can_read, IOReadHandler *fd_read, diff --git a/chardev/char-fe.c b/chardev/char-fe.c index a8931f7afd..b7bcbd59c0 100644 --- a/chardev/char-fe.c +++ b/chardev/char-fe.c @@ -246,14 +246,15 @@ void qemu_chr_fe_deinit(CharBackend *b, bool del) } } =20 -void qemu_chr_fe_set_handlers(CharBackend *b, - IOCanReadHandler *fd_can_read, - IOReadHandler *fd_read, - IOEventHandler *fd_event, - BackendChangeHandler *be_change, - void *opaque, - GMainContext *context, - bool set_open) +void qemu_chr_fe_set_handlers_full(CharBackend *b, + IOCanReadHandler *fd_can_read, + IOReadHandler *fd_read, + IOEventHandler *fd_event, + BackendChangeHandler *be_change, + void *opaque, + GMainContext *context, + bool set_open, + bool sync_state) { Chardev *s; int fe_open; @@ -285,7 +286,7 @@ void qemu_chr_fe_set_handlers(CharBackend *b, qemu_chr_fe_take_focus(b); /* We're connecting to an already opened device, so let's make sur= e we also get the open event */ - if (s->be_open) { + if (sync_state && s->be_open) { qemu_chr_be_event(s, CHR_EVENT_OPENED); } } @@ -295,6 +296,20 @@ void qemu_chr_fe_set_handlers(CharBackend *b, } } =20 +void qemu_chr_fe_set_handlers(CharBackend *b, + IOCanReadHandler *fd_can_read, + IOReadHandler *fd_read, + IOEventHandler *fd_event, + BackendChangeHandler *be_change, + void *opaque, + GMainContext *context, + bool set_open) +{ + qemu_chr_fe_set_handlers_full(b, fd_can_read, fd_read, fd_event, be_ch= ange, + opaque, context, set_open, + true); +} + void qemu_chr_fe_take_focus(CharBackend *b) { if (!b->chr) { diff --git a/chardev/char-mux.c b/chardev/char-mux.c index 6055e76293..1199d32674 100644 --- a/chardev/char-mux.c +++ b/chardev/char-mux.c @@ -283,13 +283,13 @@ void mux_chr_set_handlers(Chardev *chr, GMainContext = *context) MuxChardev *d =3D MUX_CHARDEV(chr); =20 /* Fix up the real driver with mux routines */ - qemu_chr_fe_set_handlers(&d->chr, - mux_chr_can_read, - mux_chr_read, - mux_chr_event, - NULL, - chr, - context, true); + qemu_chr_fe_set_handlers_full(&d->chr, + mux_chr_can_read, + mux_chr_read, + mux_chr_event, + NULL, + chr, + context, true, false); } =20 void mux_set_focus(Chardev *chr, int focus) @@ -367,7 +367,7 @@ static int open_muxes(Chardev *chr) * mark mux as OPENED so any new FEs will immediately receive * OPENED event */ - qemu_chr_be_event(chr, CHR_EVENT_OPENED); + chr->be_open =3D 1; =20 return 0; } --=20 2.21.0.rc0.1.g036caf7885