From 7a9657ef5307e769f084bd9f7e20ebe1ee6ac044 Mon Sep 17 00:00:00 2001 From: Artem Pisarenko Date: Tue, 6 Nov 2018 18:40:51 +0600 Subject: chardev: fix mess in OPENED/CLOSED events when muxed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When chardev is multiplexed (mux=on) 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é Lureau Message-Id: <7dde6abbd21682857f8294644013173c0b9949b3.1541507990.git.artem.k.pisarenko@gmail.com> Signed-off-by: Marc-André Lureau --- chardev/char-mux.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'chardev/char-mux.c') 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 = MUX_CHARDEV(chr); /* 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); } 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 = 1; return 0; } -- cgit 1.4.1 From 3d9e232240f9d029d7255b5b11d3a2f61c53d0d0 Mon Sep 17 00:00:00 2001 From: Marc-André Lureau Date: Wed, 6 Feb 2019 18:43:23 +0100 Subject: char: update the mux handlers in class callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of handling mux chardev in a special way in qemu_chr_fe_set_handlers(), we may use the chr_update_read_handler class callback instead. Signed-off-by: Marc-André Lureau Message-Id: <20190206174328.9736-2-marcandre.lureau@redhat.com> Reviewed-by: Paolo Bonzini --- chardev/char-fe.c | 4 ---- chardev/char-mux.c | 5 +++-- include/chardev/char-mux.h | 1 - 3 files changed, 3 insertions(+), 7 deletions(-) (limited to 'chardev/char-mux.c') diff --git a/chardev/char-fe.c b/chardev/char-fe.c index b7bcbd59c0..f3530a90e6 100644 --- a/chardev/char-fe.c +++ b/chardev/char-fe.c @@ -290,10 +290,6 @@ void qemu_chr_fe_set_handlers_full(CharBackend *b, qemu_chr_be_event(s, CHR_EVENT_OPENED); } } - - if (CHARDEV_IS_MUX(s)) { - mux_chr_set_handlers(s, context); - } } void qemu_chr_fe_set_handlers(CharBackend *b, diff --git a/chardev/char-mux.c b/chardev/char-mux.c index 1199d32674..23aa82125d 100644 --- a/chardev/char-mux.c +++ b/chardev/char-mux.c @@ -278,7 +278,7 @@ static void char_mux_finalize(Object *obj) qemu_chr_fe_deinit(&d->chr, false); } -void mux_chr_set_handlers(Chardev *chr, GMainContext *context) +static void mux_chr_update_read_handlers(Chardev *chr) { MuxChardev *d = MUX_CHARDEV(chr); @@ -289,7 +289,7 @@ void mux_chr_set_handlers(Chardev *chr, GMainContext *context) mux_chr_event, NULL, chr, - context, true, false); + chr->gcontext, true, false); } void mux_set_focus(Chardev *chr, int focus) @@ -383,6 +383,7 @@ static void char_mux_class_init(ObjectClass *oc, void *data) cc->chr_add_watch = mux_chr_add_watch; cc->chr_be_event = mux_chr_be_event; cc->chr_machine_done = open_muxes; + cc->chr_update_read_handler = mux_chr_update_read_handlers; } static const TypeInfo char_mux_type_info = { diff --git a/include/chardev/char-mux.h b/include/chardev/char-mux.h index 1e13187767..572cefd517 100644 --- a/include/chardev/char-mux.h +++ b/include/chardev/char-mux.h @@ -55,7 +55,6 @@ typedef struct MuxChardev { #define CHARDEV_IS_MUX(chr) \ object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_MUX) -void mux_chr_set_handlers(Chardev *chr, GMainContext *context); void mux_set_focus(Chardev *chr, int focus); void mux_chr_send_all_event(Chardev *chr, int event); -- cgit 1.4.1