Hi On Wed, Dec 5, 2018 at 12:43 PM Markus Armbruster <[email protected]> wrote: > > Marc-André Lureau <[email protected]> writes: > > > Not all backends are able to switch gcontext. Those backends cannot > > drive a OOB monitor (the monitor would then be blocking on main > > thread). > > > > For example, ringbuf, spice, or more esoteric input chardevs like > > braille or MUX. > > > > We currently forbid MUX because not all frontends are ready to run > > outside main loop. Extend to add a context-switching feature check. > > > > Signed-off-by: Marc-André Lureau <[email protected]> > > --- > > monitor.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/monitor.c b/monitor.c > > index 79afe99079..25cf4223e8 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -4562,9 +4562,11 @@ void monitor_init(Chardev *chr, int flags) > > bool use_oob = flags & MONITOR_USE_OOB; > > > > if (use_oob) { > > - if (CHARDEV_IS_MUX(chr)) { > > + if (CHARDEV_IS_MUX(chr) || > > + !qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT)) { > > error_report("Monitor out-of-band is not supported with " > > - "MUX typed chardev backend"); > > + "%s typed chardev backend", > > + object_get_typename(OBJECT(chr))); > > exit(1); > > } > > if (use_readline) { > > Aha, this answers my question on the previous patch: yes, it is possible > to trip the new assertion. > > Are there any ways other than this one?
Good question, if there are, we have latent bugs if switching gcontext on a non-capable chardev. Doing a quick review of qemu_chr_fe_set_handlers() for context != NULL reveals net/colo-compare.c: I think we should have the capability check added to find_and_check_chardev(). I don't see other candidates. Considering the problem is pre-existing, I can either update the patch or add a follow-up patch. > > We could squash the two patches. But I figure you kept the previous > patch separate on purpose. That's okay, but it should mention the > assertion can be tripped, and the next patch (this one) will fix it. > -- Marc-André Lureau
