[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 00/13] monitor: Optionally run handlers in coroutines
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v7 00/13] monitor: Optionally run handlers in coroutines |
Date: |
Mon, 28 Sep 2020 11:30:55 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
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?
- Re: [PATCH v7 13/13] block: Convert 'block_resize' to coroutine, (continued)
Re: [PATCH v7 00/13] monitor: Optionally run handlers in coroutines, no-reply, 2020/09/09
Re: [PATCH v7 00/13] monitor: Optionally run handlers in coroutines, Stefan Hajnoczi, 2020/09/10