To fix these, let's just drop the BH.
Since the initial reasoning for using it still applies to an extent,
work around that by deferring the delivery of CHR_EVENT_OPENED until
after the chardevs have been fully initialized, toward the end of
qmp_chardev_add() (or some cases, qemu_chr_new_from_opts()). This
defers delivery long enough that we can be assured a CharDriverState
is fully initialized before CHR_EVENT_OPENED is sent.
Also, rather than requiring each chardev to do an explicit open, do it
automatically, and allow the small few who don't desire such behavior to
suppress the OPENED-on-init behavior by setting a
'suppress_be_open_on_init' flag.
We additionally add missing OPENED events for stdio backends on w32,
which were previously not being issued, causing us to not recieve the
banner and initial prompts for qmp/hmp.
Reported-by: Stefan Priebe <address@hidden>
Cc: address@hidden
Signed-off-by: Michael Roth <address@hidden>
---
v2->v3:
* removed artifact in from v1 in backends/baum.c, test build with
BRLAPI=y (Anthony)
* rebased on latest origin/master
v1->v2:
* default to sending OPENED on backend init, add flag to suppress
it (Anthony)
* fix missing OPENED for stdio backends on w32
* fix missing OPENED when qemu_chr_new_from_opts() doesn't use
qmp_chardev_add()
* clean up/update commit message
backends/baum.c | 2 --
include/sysemu/char.h | 2 +-
qemu-char.c | 38 +++++++++++++++++---------------------
ui/console.c | 1 -
ui/gtk.c | 1 -
5 files changed, 18 insertions(+), 26 deletions(-)
diff --git a/backends/baum.c b/backends/baum.c
index 4cba79f..62aa784 100644
--- a/backends/baum.c
+++ b/backends/baum.c
@@ -611,8 +611,6 @@ CharDriverState *chr_baum_init(void)
qemu_set_fd_handler(baum->brlapi_fd, baum_chr_read, NULL, baum);
- qemu_chr_be_generic_open(chr);
-
return chr;
fail:
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 5e42c90..b0ae749 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -70,13 +70,13 @@ struct CharDriverState {
void (*chr_set_echo)(struct CharDriverState *chr, bool echo);
void (*chr_set_fe_open)(struct CharDriverState *chr, int fe_open);
void *opaque;
- int idle_tag;
char *label;
char *filename;
int be_open;
int fe_open;
int explicit_fe_open;
int avail_connections;
+ bool suppress_be_open_on_init;
QemuOpts *opts;
QTAILQ_ENTRY(CharDriverState) next;
};
diff --git a/qemu-char.c b/qemu-char.c
index d04b429..8043f86 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -110,19 +110,9 @@ void qemu_chr_be_event(CharDriverState *s, int event)
s->chr_event(s->handler_opaque, event);
}
-static gboolean qemu_chr_be_generic_open_bh(gpointer opaque)
-{
- CharDriverState *s = opaque;
- qemu_chr_be_event(s, CHR_EVENT_OPENED);
- s->idle_tag = 0;
- return FALSE;
-}
-
void qemu_chr_be_generic_open(CharDriverState *s)
{
- if (s->idle_tag == 0) {
- s->idle_tag = g_idle_add(qemu_chr_be_generic_open_bh, s);
- }
+ qemu_chr_be_event(s, CHR_EVENT_OPENED);
}
int qemu_chr_fe_write(CharDriverState *s, const uint8_t *buf, int len)
@@ -247,6 +237,7 @@ static CharDriverState *qemu_chr_open_null(void)
chr = g_malloc0(sizeof(CharDriverState));
chr->chr_write = null_chr_write;
+ chr->suppress_be_open_on_init = true;
return chr;
}
@@ -504,9 +495,6 @@ static CharDriverState *qemu_chr_open_mux(CharDriverState
*drv)
/* Frontend guest-open / -close notification is not support with muxes */
chr->chr_set_fe_open = NULL;
- /* Muxes are always open on creation */
- qemu_chr_be_generic_open(chr);
-
return chr;
}
@@ -883,8 +871,6 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int
fd_out)
chr->chr_update_read_handler = fd_chr_update_read_handler;
chr->chr_close = fd_chr_close;
- qemu_chr_be_generic_open(chr);
-
return chr;
}
@@ -1243,6 +1229,7 @@ static CharDriverState *qemu_chr_open_pty(const char *id,
chr->chr_update_read_handler = pty_chr_update_read_handler;
chr->chr_close = pty_chr_close;
chr->chr_add_watch = pty_chr_add_watch;
+ chr->suppress_be_open_on_init = true;
s->fd = io_channel_from_fd(master_fd);
s->timer_tag = 0;
@@ -1595,8 +1582,6 @@ static CharDriverState *qemu_chr_open_pp_fd(int fd)
chr->chr_close = pp_close;
chr->opaque = drv;
- qemu_chr_be_generic_open(chr);
-
return chr;
}
#endif /* __linux__ */
@@ -1650,6 +1635,7 @@ static CharDriverState *qemu_chr_open_pp_fd(int fd)
chr->opaque = (void *)(intptr_t)fd;
chr->chr_write = null_chr_write;
chr->chr_ioctl = pp_ioctl;
+ chr->suppress_be_open_on_init = true;
return chr;
}
#endif
@@ -1880,7 +1866,6 @@ static CharDriverState *qemu_chr_open_win_path(const char
*filename)
g_free(chr);
return NULL;
}
- qemu_chr_be_generic_open(chr);
return chr;
}
@@ -1980,7 +1965,6 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev
*opts)
g_free(chr);
return NULL;
}
- qemu_chr_be_generic_open(chr);
return chr;
}
@@ -1994,7 +1978,6 @@ static CharDriverState *qemu_chr_open_win_file(HANDLE
fd_out)
s->hcom = fd_out;
chr->opaque = s;
chr->chr_write = win_chr_write;
- qemu_chr_be_generic_open(chr);
return chr;
}
@@ -2329,6 +2312,8 @@ static CharDriverState *qemu_chr_open_udp_fd(int fd)
chr->chr_write = udp_chr_write;
chr->chr_update_read_handler = udp_chr_update_read_handler;
chr->chr_close = udp_chr_close;
+ /* be isn't opened until we get a connection */
+ chr->suppress_be_open_on_init = true;
return chr;
}
@@ -2731,6 +2716,8 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd,
bool do_nodelay,
chr->get_msgfd = tcp_get_msgfd;
chr->chr_add_client = tcp_chr_add_client;
chr->chr_add_watch = tcp_chr_add_watch;
+ /* be isn't opened until we get a connection */
+ chr->suppress_be_open_on_init = true;
if (is_listen) {
s->listen_fd = fd;
@@ -3325,6 +3312,12 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
if (!chr->filename)
chr->filename = g_strdup(qemu_opt_get(opts, "backend"));
chr->init = init;
+ /* if we didn't create the chardev via qmp_chardev_add, we
+ * need to send the OPENED event here
+ */
+ if (!chr->suppress_be_open_on_init) {
+ qemu_chr_be_event(chr, CHR_EVENT_OPENED);
+ }
QTAILQ_INSERT_TAIL(&chardevs, chr, next);
if (qemu_opt_get_bool(opts, "mux", 0)) {
@@ -3804,6 +3797,9 @@ ChardevReturn *qmp_chardev_add(const char *id,
ChardevBackend *backend,
if (!chr->filename) {
chr->filename =
g_strdup(ChardevBackendKind_lookup[backend->kind]);
}
+ if (!chr->suppress_be_open_on_init) {
+ qemu_chr_be_event(chr, CHR_EVENT_OPENED);
+ }
QTAILQ_INSERT_TAIL(&chardevs, chr, next);
return ret;
} else {
diff --git a/ui/console.c b/ui/console.c
index b30853f..54b192d 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1746,7 +1746,6 @@ static void text_console_do_init(CharDriverState *chr,
DisplayState *ds)
s->t_attrib = s->t_attrib_default;
}
- qemu_chr_be_generic_open(chr);
if (chr->init)
chr->init(chr);
}
diff --git a/ui/gtk.c b/ui/gtk.c
index 52c3f95..71b2497 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1214,7 +1214,6 @@ static GSList *gd_vc_init(GtkDisplayState *s,
VirtualConsole *vc, int index, GSL
gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), vc->menu_item);
- qemu_chr_be_generic_open(vc->chr);
if (vc->chr->init) {
vc->chr->init(vc->chr);
}