Re: [Qemu-devel] [PATCH for-4.0 v4 4/7] monitor: check if chardev can switch gcontext for OOB

[Marc-André Lureau]

Hi
On Thu, Dec 6, 2018 at 10:08 AM Markus Armbruster <address@hidden> wrote:
>
> One more question...
>
> 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.
>
> These chardevs don't provide QEMU_CHAR_FEATURE_GCONTEXT.
>
> > We currently forbid MUX because not all frontends are ready to run
> > outside main loop. Extend to add a context-switching feature check.
>
> Why check CHARDEV_IS_MUX() when chardev-mux already fails the
> qemu_char_feature_gcontext(chr, QEMU_CHAR_FEATURE_GCONTEXT) check?
>


It currently fails, but with "[PATCH 4/9] char: update the mux
hanlders in class callback", it won't.

But the main reason to keep an explicit check on mux is that the
monitor frontend doesn't know if other mux frontends can be called
from any context (when you set a context, it is set on the backend
side, events are dispatched by the backend).

We may want to mix this extra frontend-side capability limitation with
FEATURE_GCONTEXT flag, but they are fundamentally different: to be
able to set a backend context VS attached mux frontends can be
dispatched from any context.


> > 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 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) {