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.pisare...@gmail.com>
---
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
