qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/2] chardev: fix mess in OPENED/CLOSED events when muxed
@ 2018-11-05 12:45 Artem Pisarenko
  2018-11-05 12:45 ` [Qemu-devel] [PATCH v2 1/2] " Artem Pisarenko
  2018-11-05 12:45 ` [Qemu-devel] [PATCH v2 2/2] tests/test-char: add muxed chardev testing for open/close Artem Pisarenko
  0 siblings, 2 replies; 6+ messages in thread
From: Artem Pisarenko @ 2018-11-05 12:45 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Marc-André Lureau, Artem Pisarenko

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 first patch doesn't fix any bug (known to me), its main effect is optimization of emulation performance by avoiding extra activity.
Added testing demonstrates issue and prevents potential bugs in future.

v2 changes:
 - fix failed unit test
 - 'mux_chr_set_handlers()' hack rewritten (as supposed by Marc-André Lureau)
 - added testing of issue to unit test (new patch)

Artem Pisarenko (2):
  chardev: fix mess in OPENED/CLOSED events when muxed
  tests/test-char: add muxed chardev testing for open/close

 chardev/char-fe.c         | 33 +++++++++++++------
 chardev/char-mux.c        | 16 +++++-----
 include/chardev/char-fe.h | 18 ++++++++++-
 tests/test-char.c         | 80 +++++++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 127 insertions(+), 20 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH v2 1/2] chardev: fix mess in OPENED/CLOSED events when muxed
  2018-11-05 12:45 [Qemu-devel] [PATCH v2 0/2] chardev: fix mess in OPENED/CLOSED events when muxed Artem Pisarenko
@ 2018-11-05 12:45 ` Artem Pisarenko
  2018-11-06 11:35   ` Marc-André Lureau
  2018-11-05 12:45 ` [Qemu-devel] [PATCH v2 2/2] tests/test-char: add muxed chardev testing for open/close Artem Pisarenko
  1 sibling, 1 reply; 6+ messages in thread
From: Artem Pisarenko @ 2018-11-05 12:45 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Marc-André Lureau, 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 <artem.k.pisarenko@gmail.com>
---
 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)
     }
 }
 
-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 sure 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,
     }
 }
 
+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_change,
+                                  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 = 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;
 }
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);
 
 /**
- * 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 = true.
+ */
 void qemu_chr_fe_set_handlers(CharBackend *b,
                               IOCanReadHandler *fd_can_read,
                               IOReadHandler *fd_read,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH v2 2/2] tests/test-char: add muxed chardev testing for open/close
  2018-11-05 12:45 [Qemu-devel] [PATCH v2 0/2] chardev: fix mess in OPENED/CLOSED events when muxed Artem Pisarenko
  2018-11-05 12:45 ` [Qemu-devel] [PATCH v2 1/2] " Artem Pisarenko
@ 2018-11-05 12:45 ` Artem Pisarenko
  2018-11-06 11:22   ` Marc-André Lureau
  1 sibling, 1 reply; 6+ messages in thread
From: Artem Pisarenko @ 2018-11-05 12:45 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Marc-André Lureau, Artem Pisarenko

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 <artem.k.pisarenko@gmail.com>
---
 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;
 
 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, int size)
 static void fe_event(void *opaque, int event)
 {
     FeHandler *h = opaque;
+    bool new_open_state;
 
     h->last_event = event;
-    if (event != CHR_EVENT_BREAK) {
+    switch (event) {
+    case CHR_EVENT_BREAK:
+        break;
+    case CHR_EVENT_OPENED:
+    case CHR_EVENT_CLOSED:
+        h->openclose_count++;
+        new_open_state = (event == CHR_EVENT_OPENED);
+        if (h->is_open == new_open_state) {
+            h->openclose_mismatch = true;
+        }
+        h->is_open = new_open_state;
+        /* no break */
+    default:
         quit = true;
+        break;
     }
 }
 
@@ -161,7 +178,7 @@ static void char_mux_test(void)
     QemuOpts *opts;
     Chardev *chr, *base;
     char *data;
-    FeHandler h1 = { 0, }, h2 = { 0, };
+    FeHandler h1 = { 0, false, 0, false, }, h2 = { 0, false, 0, false, };
     CharBackend chr_be1, chr_be2;
 
     opts = 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, ==, CHR_EVENT_BREAK);
     g_assert_cmpint(h2.last_event, ==, CHR_EVENT_MUX_OUT);
 
+    /* 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 = h2.openclose_count = 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, ==, 0);
+    g_assert_cmpint(h2.openclose_count, ==, 0);
+
+    h1.is_open = h2.is_open = 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, ==, 1);
+    g_assert_false(h1.openclose_mismatch);
+    g_assert_cmpint(h2.openclose_count, ==, 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, ==, 3);
+    g_assert_false(h1.openclose_mismatch);
+    g_assert_cmpint(h2.openclose_count, ==, 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);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] tests/test-char: add muxed chardev testing for open/close
  2018-11-05 12:45 ` [Qemu-devel] [PATCH v2 2/2] tests/test-char: add muxed chardev testing for open/close Artem Pisarenko
@ 2018-11-06 11:22   ` Marc-André Lureau
  2018-11-06 12:46     ` Artem Pisarenko
  0 siblings, 1 reply; 6+ messages in thread
From: Marc-André Lureau @ 2018-11-06 11:22 UTC (permalink / raw)
  To: artem.k.pisarenko; +Cc: QEMU, Paolo Bonzini

Hi

On Mon, Nov 5, 2018 at 4:47 PM Artem Pisarenko
<artem.k.pisarenko@gmail.com> wrote:
>
> 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 <artem.k.pisarenko@gmail.com>
> ---
>  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;
>
>  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, int size)
>  static void fe_event(void *opaque, int event)
>  {
>      FeHandler *h = opaque;
> +    bool new_open_state;
>
>      h->last_event = event;
> -    if (event != CHR_EVENT_BREAK) {
> +    switch (event) {
> +    case CHR_EVENT_BREAK:
> +        break;
> +    case CHR_EVENT_OPENED:
> +    case CHR_EVENT_CLOSED:
> +        h->openclose_count++;
> +        new_open_state = (event == CHR_EVENT_OPENED);
> +        if (h->is_open == new_open_state) {
> +            h->openclose_mismatch = true;
> +        }
> +        h->is_open = new_open_state;
> +        /* no break */
> +    default:
>          quit = true;
> +        break;
>      }
>  }
>
> @@ -161,7 +178,7 @@ static void char_mux_test(void)
>      QemuOpts *opts;
>      Chardev *chr, *base;
>      char *data;
> -    FeHandler h1 = { 0, }, h2 = { 0, };
> +    FeHandler h1 = { 0, false, 0, false, }, h2 = { 0, false, 0, false, };

this is unnecessary change, I can drop on commit

>      CharBackend chr_be1, chr_be2;
>
>      opts = 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, ==, CHR_EVENT_BREAK);
>      g_assert_cmpint(h2.last_event, ==, CHR_EVENT_MUX_OUT);
>
> +    /* 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 = h2.openclose_count = 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, ==, 0);
> +    g_assert_cmpint(h2.openclose_count, ==, 0);
> +
> +    h1.is_open = h2.is_open = 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, ==, 1);
> +    g_assert_false(h1.openclose_mismatch);
> +    g_assert_cmpint(h2.openclose_count, ==, 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, ==, 3);
> +    g_assert_false(h1.openclose_mismatch);
> +    g_assert_cmpint(h2.openclose_count, ==, 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);
> +

otherwise, looks good to me
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


>      /* remove first handler */
>      qemu_chr_fe_set_handlers(&chr_be1, NULL, NULL, NULL, NULL,
>                               NULL, NULL, true);
> --
> 2.7.4
>
>


-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/2] chardev: fix mess in OPENED/CLOSED events when muxed
  2018-11-05 12:45 ` [Qemu-devel] [PATCH v2 1/2] " Artem Pisarenko
@ 2018-11-06 11:35   ` Marc-André Lureau
  0 siblings, 0 replies; 6+ messages in thread
From: Marc-André Lureau @ 2018-11-06 11:35 UTC (permalink / raw)
  To: artem.k.pisarenko; +Cc: QEMU, Paolo Bonzini

Hi

On Mon, Nov 5, 2018 at 4:46 PM Artem Pisarenko
<artem.k.pisarenko@gmail.com> wrote:
>
> 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 <artem.k.pisarenko@gmail.com>

I can see that this fixes the mismatch you describe, and the changes
look reasonably simple that I can understand how it works. But if you
could also describe with your own words the "why" it happened and the
"how" you fixed it in the commit message, that would be really helpful
for the future readers (including ourself).

otherwise, patch looks good to me.

Since it's not for 3.1, after you send v3, I will queue those patches
for merge early in 3.2, so we get enough testing.

thanks!

> ---
>  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)
>      }
>  }
>
> -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 sure 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,
>      }
>  }
>
> +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_change,
> +                                  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 = 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;
>  }
> 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);
>
>  /**
> - * 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 = true.
> + */
>  void qemu_chr_fe_set_handlers(CharBackend *b,
>                                IOCanReadHandler *fd_can_read,
>                                IOReadHandler *fd_read,
> --
> 2.7.4
>
>


-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/2] tests/test-char: add muxed chardev testing for open/close
  2018-11-06 11:22   ` Marc-André Lureau
@ 2018-11-06 12:46     ` Artem Pisarenko
  0 siblings, 0 replies; 6+ messages in thread
From: Artem Pisarenko @ 2018-11-06 12:46 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: QEMU, Paolo Bonzini

> this is unnecessary change, I can drop on commit

Oops, didn't noticed your message before sent v3.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-11-06 12:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-05 12:45 [Qemu-devel] [PATCH v2 0/2] chardev: fix mess in OPENED/CLOSED events when muxed Artem Pisarenko
2018-11-05 12:45 ` [Qemu-devel] [PATCH v2 1/2] " Artem Pisarenko
2018-11-06 11:35   ` Marc-André Lureau
2018-11-05 12:45 ` [Qemu-devel] [PATCH v2 2/2] tests/test-char: add muxed chardev testing for open/close Artem Pisarenko
2018-11-06 11:22   ` Marc-André Lureau
2018-11-06 12:46     ` Artem Pisarenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).