Re: [PATCH v7 00/13] monitor: Optionally run handlers in coroutines

[Markus Armbruster]

Kevin Wolf <kwolf@redhat.com> writes:

> Am 10.09.2020 um 15:24 hat Stefan Hajnoczi geschrieben:
>> On Wed, Sep 09, 2020 at 05:11:36PM +0200, Kevin Wolf wrote:
>> > Some QMP command handlers can block the main loop for a relatively long
>> > time, for example because they perform some I/O. This is quite nasty.
>> > Allowing such handlers to run in a coroutine where they can yield (and
>> > therefore release the BQL) while waiting for an event such as I/O
>> > completion solves the problem.
>> > 
>> > This series adds the infrastructure to allow this and switches
>> > block_resize to run in a coroutine as a first example.
>> > 
>> > This is an alternative solution to Marc-André's "monitor: add
>> > asynchronous command type" series.
>> 
>> Please clarify the following in the QAPI documentation:
>>  * Is the QMP monitor suspended while the command is pending?
>
> Suspended as in monitor_suspend()? No.

A suspended monitor doesn't read monitor input.

We suspend

* a QMP monitor while the request queue is full

* an HMP monitor while it executes a command

* a multiplexed HMP monitor while the "mux-focus" is elsewhere

* an HMP monitor when it executes command "quit", forever

* an HMP monitor while it executes command "migrate" without -d

Let me explain the first item in a bit more detail.  Before OOB, a QMP
monitor was also suspended while it executed a command.  To make OOB
work, we moved the QMP monitors to an I/O thread and added a request
queue, drained by the main loop.  QMP monitors continue to read
commands, execute right away if OOB, else queue, suspend when queue gets
full, resume when it gets non-full.

The "run command in coroutine context" feature does not affect any of
this.

qapi-code-gen.txt does not talk about monitor suspension at all.  It's
instead discussed in qmp-spec.txt section 2.3.1 Out-of-band execution.

Stefan, what would you like us to clarify, and where?

>>  * Are QMP events reported while the command is pending?
>
> Hm, I don't know to be honest. But I think so.

Yes, events should be reported while a command is being executed.

Sending events takes locks.  Their critical sections are all
short-lived.  Another possible delay is the underlying character device
failing the send with EAGAIN.  That's all.

Fine print: qapi_event_emit() takes @monitor_lock.  It sends to each QMP
monitor with qmp_send_response(), which uses monitor_puts(), which takes
the monitor's @mon_lock.

The "run command in coroutine context" feature does not affect any of
this.

> Does it matter, though? I don't think events have a defined order
> compared to command results, and the client can't respond to the event
> anyway until the current command has completed.

Stefan, what would you like us to clarify, and where?