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

[Markus Armbruster <armbru@redhat.com>]

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Hi
> On Thu, Dec 6, 2018 at 10:08 AM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> One more question...
>>
>> Marc-André Lureau <marcandre.lureau@redhat.com> 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.

That's because it makes chardev-mux implement chr_update_read_handler(),
and "[PATCH 3/7] char: add a QEMU_CHAR_FEATURE_GCONTEXT flag" assumes
that a chardev implementing that "will take the updated gcontext into
account".

Sounds to me as if "[PATCH 4/9] char: update the mux hanlders in class
callback" violates that assumption.  Why am I wrong?

> 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.

I'm afraid I can't yet see the full picture.

The goal of this series PATCH 3-5 is to catch certain thread-related
badness in chardevs before it can happen.

Apparently, there are two separate kinds of badness:

* The chardev backend may fail to cope with changed gcontext.  I don't
  understand how exactly the backends screw up, but I doubt I have to
  right now.

* The chardev frontend may fail to... what exactly?  And why is only
  chardev-mux affected?

[...]