Re: [PATCH v5 3/4] qmp: Move dispatcher to a coroutine

[Markus Armbruster <armbru@redhat.com>]

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

> Hi
>
> On Mon, Mar 23, 2020 at 6:41 PM Kevin Wolf <kwolf@redhat.com> wrote:
>>
>> Am 18.03.2020 um 16:36 hat Markus Armbruster geschrieben:
>> > Kevin Wolf <kwolf@redhat.com> writes:
>> >
>> > > This moves the QMP dispatcher to a coroutine and runs all QMP command
>> > > handlers that declare 'coroutine': true in coroutine context so they
>> > > can avoid blocking the main loop while doing I/O or waiting for other
>> > > events.
>> > >
>> > > For commands that are not declared safe to run in a coroutine, the
>> > > dispatcher drops out of coroutine context by calling the QMP command
>> > > handler from a bottom half.
>> > >
>> > > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> >
>> > Uh, what about @cur_mon?
>> >
>> > @cur_mon points to the current monitor while a command executes.
>> > Initial value is null.  It is set in three places (not counting unit
>> > tests), and all three save, set, do something that may use @cur_mon,
>> > restore:
>> >
>> > * monitor_qmp_dispatch(), for use within qmp_dispatch()
>> > * monitor_read(), for use within handle_hmp_command()
>> > * qmp_human_monitor_command(), also for use within handle_hmp_command()
>> >
>> > Therefore, @cur_mon is null unless we're running within qmp_dispatch()
>> > or handle_hmp_command().
>>
>> Can we make it NULL for coroutine-enabled handlers?
>
> fwiw, I think /dev/fdset doesn't care about cur_mon. However, qmp
> handlers that use monitor_get_fd() usually depend on cur_mon.
>
> Note: I wonder if add-fd (fdsets) and getfd (monitor fds) deserve to co-exist.

Beats me.

If one of them is more general, we could consider deprecating the other
one.