Re: [Qemu-devel] [PATCH v2 4/6] monitor: check if chardev can switch gcontext for OOB

[Marc-André Lureau]

Hi

On Mon, Dec 3, 2018 at 12:23 PM Markus Armbruster <address@hidden> wrote:
>
> Marc-André Lureau <address@hidden> 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.
>
> Double-checking: the reason for forbidding MUX and the reason for
> requiring QEMU_CHAR_FEATURE_GCONTEXT are orthogonal, right?

Yes, to me the mux check is there because other front-end sides (other
than QMP monitor) may not handle dispatch from other threads.

Otoh, the GCONTEXT feature check is there to check if the chardev
(backend) handles switching context.


> > Note: this patch will conflict with Peter "[PATCH v9 3/6] monitor:
> > remove "x-oob", turn oob on by default", but can be trivially updated.
>
> I intend to merge this series first, and drop this sentence.
>
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> >  monitor.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/monitor.c b/monitor.c
> > index 511dd11d1c..fffeb27ef9 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -4560,9 +4560,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) {
>