From nobody Sun May 12 20:01:35 2024 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; dkim=fail; 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=gmail.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 15415085503081014.0402189515203; Tue, 6 Nov 2018 04:49:10 -0800 (PST) Received: from localhost ([::1]:40757 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gK0n7-0008Rv-42 for importer@patchew.org; Tue, 06 Nov 2018 07:49:09 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49499) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gK0kq-0004uJ-S4 for qemu-devel@nongnu.org; Tue, 06 Nov 2018 07:46:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gK0fL-00012e-94 for qemu-devel@nongnu.org; Tue, 06 Nov 2018 07:41:11 -0500 Received: from mail-lf1-x143.google.com ([2a00:1450:4864:20::143]:46190) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gK0fJ-0000ih-HZ for qemu-devel@nongnu.org; Tue, 06 Nov 2018 07:41:07 -0500 Received: by mail-lf1-x143.google.com with SMTP id f23so4166460lfc.13 for ; Tue, 06 Nov 2018 04:41:01 -0800 (PST) Received: from localhost.localdomain ([77.221.221.49]) by smtp.gmail.com with ESMTPSA id r69sm829190lfi.15.2018.11.06.04.40.58 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 06 Nov 2018 04:40:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :in-reply-to:references; bh=ATLsoUmOa0dstFxKw3sO0Lgat1XPnXnRj9CqDUcnDhU=; b=JakNITxhtCikp95rFz4noSFRX6gjTXiYO5oblte+gqnd/prQgLSvQqqtBIwB6tf44s EbruAMhsYBPNQ6LHkB3QF16UdmT30RTx+vzedeFV+XF4bU92OVzXeWCEyI5f9/Zv8W81 0hzO7hheRkWnQ3o1GWyfO6kaeh1C55qPceV3L8WWA2Vgb0PSu+SpLj2yQZVGl9ptlxeY ccl7hltRM77ARUE9NNY6uYj9RqLlEksnlSDB1TkPKh1GuyS1sR99RhxW+DJFZLqPS0K1 4F8SeGy3rtRl5V4qCl0npdsuXaam9EhN2ayI4mD/acDcN2F9Jq/bt2rct6PGaWjv4TVk UIPg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:in-reply-to:references; bh=ATLsoUmOa0dstFxKw3sO0Lgat1XPnXnRj9CqDUcnDhU=; b=CadPDoEJXPwXjLsQrE6pbAUfcWze1kZGbpZyzlbU7iWFpT9e0oNFI0Xj+CqihZ+y05 H0pPRlHyn+FOJ9R5YpHy8iJ4/qDCOO6/xEi4AcSVlovNGp+tC1oBszswjGftP2QGu2xU WqNCtcCWhd54zX7UJ4Ag/uyum7p7VswJmxTgIHOlfmv54HLCtvjSAcQmo2BhCGYH3gBp 7m3IgVj0TBxMplwxD6fbYp3vYhhqC6h2Patob8NYSoHZ/+s+N/s4EEmk0+3/IzBd/UaF uhM1scKEhaJic/hScWPF0qER7xpadl+cPri6Sh+GLRKC2RTJQ5/DDPtSZjcSjOzm+mpu Tc9A== X-Gm-Message-State: AGRZ1gKT1LF0uISpc5jzOfKAdCHzT7nMG950MUoZoLVkd7GM9fig7sMj CBI392/hmJKSOQcSH4bKAWXWM11v X-Google-Smtp-Source: AJdET5fHQdlQ0kWkGvLCEJ4b6O5NaFcjyapQiuU6lZBLpZ3fkc3iuweSI+jq2qNRPy4GyGYpAASaFA== X-Received: by 2002:a19:4c02:: with SMTP id z2-v6mr14584203lfa.48.1541508059820; Tue, 06 Nov 2018 04:40:59 -0800 (PST) From: Artem Pisarenko To: qemu-devel@nongnu.org Date: Tue, 6 Nov 2018 18:40:51 +0600 Message-Id: <7dde6abbd21682857f8294644013173c0b9949b3.1541507990.git.artem.k.pisarenko@gmail.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: References: In-Reply-To: References: X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:1450:4864:20::143 Subject: [Qemu-devel] [PATCH v3 1/2] 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: Paolo Bonzini , Artem Pisarenko , =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" 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 --- Notes: v3: - extended and improved commit message with 'why' and 'how' explanation chardev/char-fe.c | 33 ++++++++++++++++++++++++--------- chardev/char-mux.c | 16 ++++++++-------- include/chardev/char-fe.h | 18 +++++++++++++++++- 3 files changed, 49 insertions(+), 18 deletions(-) diff --git a/chardev/char-fe.c b/chardev/char-fe.c index a8931f7..b7bcbd5 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 6055e76..1199d32 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; } diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h index 46c997d..c1b7fd9 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, --=20 2.7.4 From nobody Sun May 12 20:01:35 2024 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; dkim=fail; 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=gmail.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 154150871242492.90200095968555; Tue, 6 Nov 2018 04:51:52 -0800 (PST) Received: from localhost ([::1]:40777 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gK0pj-0002ZM-4I for importer@patchew.org; Tue, 06 Nov 2018 07:51:51 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49383) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gK0kr-0004nZ-5p for qemu-devel@nongnu.org; Tue, 06 Nov 2018 07:46:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gK0fL-00012U-7h for qemu-devel@nongnu.org; Tue, 06 Nov 2018 07:41:11 -0500 Received: from mail-lf1-x143.google.com ([2a00:1450:4864:20::143]:46054) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gK0fJ-0000k8-Ft for qemu-devel@nongnu.org; Tue, 06 Nov 2018 07:41:06 -0500 Received: by mail-lf1-x143.google.com with SMTP id b20so8695142lfa.12 for ; Tue, 06 Nov 2018 04:41:03 -0800 (PST) Received: from localhost.localdomain ([77.221.221.49]) by smtp.gmail.com with ESMTPSA id r69sm829190lfi.15.2018.11.06.04.40.59 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 06 Nov 2018 04:41:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :in-reply-to:references; bh=9yPWeS0puioAFhW75zifQm7ZQFapUItZ159YEl59KMw=; b=az1IiRn3sFcwMhYe0wZO/wCH+Zk9dbzvXcWc7oltQ2RP2ZgSJ8Bx0TXLc6ydN0Iu4b QcdJehbj1EY5ro8/h0nbJHr+NFlllmr6WfQnxanRxDfA8MBs169FLiP5MtAO7DBES74S 0qUvzmvQ8ocF1wlIHuzqIQsc2plOzdRVN2EO3vHcQvDPjv019Qe+6UWCUxPaZxrP8PQX 6S/YylrwxE5tN5LrHLDd5rB5Fec+mBEVleFElFXCg/kvTQTjb8ytSu0TzULbFY7NRamw YuqhtVkOyNVjRoqnYT0z15e4iq38Z45NeDQ8N5r2LmT3wh7y63y8Dpeqn/x8/nipHbM+ 12bA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:in-reply-to:references; bh=9yPWeS0puioAFhW75zifQm7ZQFapUItZ159YEl59KMw=; b=PcXhk4iMWt03KuLeI9sM3MMWQFzF4gPNA7bk2pG+CCNBOdV1laBAn39R9+Glq64cvF 57RGT/I/L7Za03ZImMRGesyi01iQrmGIHbqP2+4UdvvS6JG5O37eHnvboW7yN3x/3YwM SRrRj3Ew1BHqRGNsa+KplB+Mrf0aaj4kTs/AOMCThhUn+UZsMc66mhk4VsoeKIPNA9FY taFB1nzGi1eLwbmFJtAxkhHYY/qt/O39qGdPXDxbC4yyAsVVelMUDLaExfhrLRZFnhN5 VZVIz5H7gee+UrcZqNIo1vjCDf5EJw8fEui2Li6gnxjt+QJSnY4IQITED3MAosBnyys8 tb0Q== X-Gm-Message-State: AGRZ1gIJpZCCesKvxBMo3GOy0/JmP+WwuNPD8biuXEC7jUdfcvJjQNac eqoGnitWZzEV4YBsBOliHoftJUL1 X-Google-Smtp-Source: AJdET5fwplZjmS+I1HUL6CYUQf5dx0R1973a70ODhnd82jRuZQBYdwEr8SppuOmFVOyF/0e9Ya4vEg== X-Received: by 2002:a19:1f46:: with SMTP id f67mr14341745lff.1.1541508061483; Tue, 06 Nov 2018 04:41:01 -0800 (PST) From: Artem Pisarenko To: qemu-devel@nongnu.org Date: Tue, 6 Nov 2018 18:40:52 +0600 Message-Id: X-Mailer: git-send-email 2.7.4 In-Reply-To: References: In-Reply-To: References: X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:1450:4864:20::143 Subject: [Qemu-devel] [PATCH v3 2/2] tests/test-char: add muxed chardev testing for open/close 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: Paolo Bonzini , Artem Pisarenko , =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Validate that frontend callbacks for CHR_EVENT_OPENED/CHR_EVENT_CLOSED events are being issued when expected and in strictly pairing order. Signed-off-by: Artem Pisarenko Reviewed-by: Marc-Andr=C3=A9 Lureau --- tests/test-char.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++= ++-- 1 file changed, 78 insertions(+), 2 deletions(-) diff --git a/tests/test-char.c b/tests/test-char.c index 831e37f..657cb4c 100644 --- a/tests/test-char.c +++ b/tests/test-char.c @@ -16,6 +16,9 @@ static bool quit; =20 typedef struct FeHandler { int read_count; + bool is_open; + int openclose_count; + bool openclose_mismatch; int last_event; char read_buf[128]; } FeHandler; @@ -49,10 +52,24 @@ static void fe_read(void *opaque, const uint8_t *buf, i= nt size) static void fe_event(void *opaque, int event) { FeHandler *h =3D opaque; + bool new_open_state; =20 h->last_event =3D event; - if (event !=3D CHR_EVENT_BREAK) { + switch (event) { + case CHR_EVENT_BREAK: + break; + case CHR_EVENT_OPENED: + case CHR_EVENT_CLOSED: + h->openclose_count++; + new_open_state =3D (event =3D=3D CHR_EVENT_OPENED); + if (h->is_open =3D=3D new_open_state) { + h->openclose_mismatch =3D true; + } + h->is_open =3D new_open_state; + /* no break */ + default: quit =3D true; + break; } } =20 @@ -161,7 +178,7 @@ static void char_mux_test(void) QemuOpts *opts; Chardev *chr, *base; char *data; - FeHandler h1 =3D { 0, }, h2 =3D { 0, }; + FeHandler h1 =3D { 0, false, 0, false, }, h2 =3D { 0, false, 0, false,= }; CharBackend chr_be1, chr_be2; =20 opts =3D qemu_opts_create(qemu_find_opts("chardev"), "mux-label", @@ -233,6 +250,65 @@ static void char_mux_test(void) g_assert_cmpint(h1.last_event, =3D=3D, CHR_EVENT_BREAK); g_assert_cmpint(h2.last_event, =3D=3D, CHR_EVENT_MUX_OUT); =20 + /* open/close state and corresponding events */ + g_assert_true(qemu_chr_fe_backend_open(&chr_be1)); + g_assert_true(qemu_chr_fe_backend_open(&chr_be2)); + g_assert_true(h1.is_open); + g_assert_false(h1.openclose_mismatch); + g_assert_true(h2.is_open); + g_assert_false(h2.openclose_mismatch); + + h1.openclose_count =3D h2.openclose_count =3D 0; + + qemu_chr_fe_set_handlers(&chr_be1, NULL, NULL, NULL, NULL, + NULL, NULL, false); + qemu_chr_fe_set_handlers(&chr_be2, NULL, NULL, NULL, NULL, + NULL, NULL, false); + g_assert_cmpint(h1.openclose_count, =3D=3D, 0); + g_assert_cmpint(h2.openclose_count, =3D=3D, 0); + + h1.is_open =3D h2.is_open =3D false; + qemu_chr_fe_set_handlers(&chr_be1, + NULL, + NULL, + fe_event, + NULL, + &h1, + NULL, false); + qemu_chr_fe_set_handlers(&chr_be2, + NULL, + NULL, + fe_event, + NULL, + &h2, + NULL, false); + g_assert_cmpint(h1.openclose_count, =3D=3D, 1); + g_assert_false(h1.openclose_mismatch); + g_assert_cmpint(h2.openclose_count, =3D=3D, 1); + g_assert_false(h2.openclose_mismatch); + + qemu_chr_be_event(base, CHR_EVENT_CLOSED); + qemu_chr_be_event(base, CHR_EVENT_OPENED); + g_assert_cmpint(h1.openclose_count, =3D=3D, 3); + g_assert_false(h1.openclose_mismatch); + g_assert_cmpint(h2.openclose_count, =3D=3D, 3); + g_assert_false(h2.openclose_mismatch); + + qemu_chr_fe_set_handlers(&chr_be2, + fe_can_read, + fe_read, + fe_event, + NULL, + &h2, + NULL, false); + qemu_chr_fe_set_handlers(&chr_be1, + fe_can_read, + fe_read, + fe_event, + NULL, + &h1, + NULL, false); + /* remove first handler */ qemu_chr_fe_set_handlers(&chr_be1, NULL, NULL, NULL, NULL, NULL, NULL, true); --=20 2.7.4