[Qemu-devel] [PATCH] chardev: fix mess in OPENED/CLOSED events when muxed

[Artem Pisarenko]

When chardev is multiplexed (mux=on) there are a lot of cases, when
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 (when it does with mux=off).
Fix that.

Signed-off-by: Artem Pisarenko <address@hidden>
---

Notes:
    This issue actually more complex. Idea of generating events from inside 
function called '*_set_handlers' isn't good, at least its implicit nature, and 
especially a fact, that function decides about open state (see 'fe_open' 
variable), but generates event only in one direction. Combined with 
'mux_chr_set_handlers()' hack this makes things even worse.
    Better solution is to change fe interface and rewrite all frontends code (a 
lot of stuff in hw/char/* and somewhere else).
    Although this patch doesn't fix any issue/bug (known to me), it prevents 
them in future. Also it optimizes emulation performance by avoiding extra 
activity.
    I did several trivial tests on x86_64 target and seems like nothing broken.

 chardev/char-fe.c          |  9 ++++++---
 chardev/char-mux.c         | 13 ++++++++-----
 include/chardev/char-mux.h |  2 +-
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index a8931f7..31cf7f0 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -257,6 +257,7 @@ void qemu_chr_fe_set_handlers(CharBackend *b,
 {
     Chardev *s;
     int fe_open;
+    static __thread bool mux_reentered;
 
     s = b->chr;
     if (!s) {
@@ -284,14 +285,16 @@ void qemu_chr_fe_set_handlers(CharBackend *b,
     if (fe_open) {
         qemu_chr_fe_take_focus(b);
         /* We're connecting to an already opened device, so let's make sure we
-           also get the open event */
-        if (s->be_open) {
+           also get the open event (hack: except when chardev is muxed) */
+        if (s->be_open && !mux_reentered) {
             qemu_chr_be_event(s, CHR_EVENT_OPENED);
         }
     }
 
     if (CHARDEV_IS_MUX(s)) {
-        mux_chr_set_handlers(s, context);
+        mux_reentered = true;
+        mux_chr_set_handlers(s, fe_open, context);
+        mux_reentered = false;
     }
 }
 
diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index 6055e76..9244802 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -272,21 +272,24 @@ static void char_mux_finalize(Object *obj)
     for (i = 0; i < d->mux_cnt; i++) {
         CharBackend *be = d->backends[i];
         if (be) {
+            if (be->chr && be->chr->be_open) {
+                qemu_chr_be_event(be->chr, CHR_EVENT_CLOSED);
+            }
             be->chr = NULL;
         }
     }
     qemu_chr_fe_deinit(&d->chr, false);
 }
 
-void mux_chr_set_handlers(Chardev *chr, GMainContext *context)
+void mux_chr_set_handlers(Chardev *chr, bool is_open, 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,
+                             is_open ? mux_chr_can_read : NULL,
+                             is_open ? mux_chr_read : NULL,
+                             is_open ? mux_chr_event : NULL,
                              NULL,
                              chr,
                              context, true);
@@ -367,7 +370,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;
 }
diff --git a/include/chardev/char-mux.h b/include/chardev/char-mux.h
index 1e13187..4b4df6e 100644
--- a/include/chardev/char-mux.h
+++ b/include/chardev/char-mux.h
@@ -55,7 +55,7 @@ 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_chr_set_handlers(Chardev *chr, bool is_open, GMainContext *context);
 void mux_set_focus(Chardev *chr, int focus);
 void mux_chr_send_all_event(Chardev *chr, int event);
 
-- 
2.7.4