Hi, I noticed that this patch kills the Qemu monitor for me. If I start a text-mode guest with -nographic, then I can't switch to the monitor any longer with Ctrl+a c . The guest works otherwise, e.g. I can login from the console.
Tested on s390, but I think it's a more general issue, since the patch is not arch-dependent. On 03/10/16 11:47, Marc-André Lureau wrote: > mux_chr_update_read_handler() is adding a new mux_cnt each time > mux_chr_update_read_handler() is called, it's not possible to actually > update the "child" chr callbacks that were set previously. This may lead > to crashes if the "child" chr is destroyed: > > valgrind x86_64-softmmu/qemu-system-x86_64 -chardev > stdio,mux=on,id=char0 -mon chardev=char0,mode=control,default > > when quitting: > > ==4306== Invalid read of size 8 > ==4306== at 0x8061D3: json_lexer_destroy (json-lexer.c:385) > ==4306== by 0x7E39F8: json_message_parser_destroy (json-streamer.c:134) > ==4306== by 0x3447F6: monitor_qmp_event (monitor.c:3908) > ==4306== by 0x480153: mux_chr_send_event (qemu-char.c:630) > ==4306== by 0x480694: mux_chr_event (qemu-char.c:734) > ==4306== by 0x47F1E9: qemu_chr_be_event (qemu-char.c:205) > ==4306== by 0x481207: fd_chr_close (qemu-char.c:1114) > ==4306== by 0x481659: qemu_chr_close_stdio (qemu-char.c:1221) > ==4306== by 0x486F07: qemu_chr_free (qemu-char.c:4146) > ==4306== by 0x486F97: qemu_chr_delete (qemu-char.c:4154) > ==4306== by 0x487E66: qemu_chr_cleanup (qemu-char.c:4678) > ==4306== by 0x495A98: main (vl.c:4675) > ==4306== Address 0x28439e90 is 112 bytes inside a block of size 240 free'd > ==4306== at 0x4C2CD5A: free (vg_replace_malloc.c:530) > ==4306== by 0x1E4CBF2D: g_free (in /usr/lib64/libglib-2.0.so.0.4800.2) > ==4306== by 0x344DE9: monitor_cleanup (monitor.c:4058) > ==4306== by 0x495A93: main (vl.c:4674) > ==4306== Block was alloc'd at > ==4306== at 0x4C2BBAD: malloc (vg_replace_malloc.c:299) > ==4306== by 0x1E4CBE18: g_malloc (in /usr/lib64/libglib-2.0.so.0.4800.2) > ==4306== by 0x344BF8: monitor_init (monitor.c:4021) > ==4306== by 0x49063C: mon_init_func (vl.c:2417) > ==4306== by 0x7FC6DE: qemu_opts_foreach (qemu-option.c:1116) > ==4306== by 0x4954E0: main (vl.c:4473) > > Instead, keep the "child" chr associated with a particular idx so its > handlers can be updated and removed to avoid the crash. > > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> > --- > qemu-char.c | 22 +++++++++++++++------- > include/sysemu/char.h | 1 + > 2 files changed, 16 insertions(+), 7 deletions(-) > > diff --git a/qemu-char.c b/qemu-char.c > index f7a07e6..4b330ea 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -165,6 +165,7 @@ CharDriverState *qemu_chr_alloc(ChardevCommon *backend, > Error **errp) > CharDriverState *chr = g_malloc0(sizeof(CharDriverState)); > qemu_mutex_init(&chr->chr_write_lock); > > + chr->mux_idx = -1; > if (backend->has_logfile) { > int flags = O_WRONLY | O_CREAT; > if (backend->has_logappend && > @@ -738,17 +739,25 @@ static void mux_chr_update_read_handler(CharDriverState > *chr, > GMainContext *context) > { > MuxDriver *d = chr->opaque; > + int idx; > > if (d->mux_cnt >= MAX_MUX) { > fprintf(stderr, "Cannot add I/O handlers, MUX array is full\n"); > return; > } > - d->ext_opaque[d->mux_cnt] = chr->handler_opaque; > - d->chr_can_read[d->mux_cnt] = chr->chr_can_read; > - d->chr_read[d->mux_cnt] = chr->chr_read; > - d->chr_event[d->mux_cnt] = chr->chr_event; > + > + if (chr->mux_idx == -1) { > + chr->mux_idx = d->mux_cnt++; > + } > + > + idx = chr->mux_idx; > + d->ext_opaque[idx] = chr->handler_opaque; > + d->chr_can_read[idx] = chr->chr_can_read; > + d->chr_read[idx] = chr->chr_read; > + d->chr_event[idx] = chr->chr_event; > + > /* Fix up the real driver with mux routines */ > - if (d->mux_cnt == 0) { > + if (d->mux_cnt == 1) { > qemu_chr_add_handlers_full(d->drv, mux_chr_can_read, > mux_chr_read, > mux_chr_event, > @@ -757,8 +766,7 @@ static void mux_chr_update_read_handler(CharDriverState > *chr, > if (d->focus != -1) { > mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_OUT); > } > - d->focus = d->mux_cnt; > - d->mux_cnt++; > + d->focus = idx; > mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_IN); > } > > diff --git a/include/sysemu/char.h b/include/sysemu/char.h > index 0d0465a..4593576 100644 > --- a/include/sysemu/char.h > +++ b/include/sysemu/char.h > @@ -92,6 +92,7 @@ struct CharDriverState { > int explicit_be_open; > int avail_connections; > int is_mux; > + int mux_idx; > guint fd_in_tag; > QemuOpts *opts; > bool replay; >