Re: [Qemu-devel] [PATCH v3 00/38] RFC: monitor: add asynchronous command type

[Marc-André Lureau]

Hi

On Wed, Apr 4, 2018 at 11:34 AM, Stefan Hajnoczi <address@hidden> wrote:
> On Mon, Mar 26, 2018 at 05:08:38PM +0200, Marc-André Lureau wrote:
>> One of initial design goals of QMP was to have "asynchronous command
>> completion" (http://wiki.qemu.org/Features/QAPI). Unfortunately, that
>> goal was not fully achieved, and some broken bits left were removed
>> progressively until commit 65207c59d that removed async command
>> support.
>>
>> Yet there are clear benefits allowing the command handler to re-enter
>> the main loop if the command cannot be handled synchronously, or if it
>> is long-lasting. This is currently not possible and make some bugs
>> such as rhbz#1230527 difficult to solve.  Furthermore, many QMP
>> commands do IO and could be considered 'slow' and blocking the main
>> loop today. The unwritten solution so far is to use a pair of
>> command+event. But this approach has a number of issues, in particular
>> to fix existing commands, and inadequacy since the event is
>> broadcasted and may thus have command 'id' conflict, beside being
>> rather inefficient and incorrect.
>
> This paragraph implies changing the command+event QMP idiom but none of
> the patches touch docs/interop/qmp-spec.txt.  So I guess this new
> revision doesn't modify the QMP protocol anymore and this paragraph is
> outdated?  This revision simply implements async command handlers but
> QMP clients see no protocol change?

Yes, that version of the patch series doesn't provide asynchronous
protocol. The protocol is unchanged. With asynchronous handler, you
don't have to use the cmd+event idiom, unless you want some kind of
asynchronous protocol, with concurrent commands/events - you may still
use oob anyway. I think having a better asynchronous protocol has
merits that would simplify the commands/events&oob situation, but this
is left for another series, when there will be clearer benefits.

> For more complex commands a coroutine would be cleaner than hand-written
> async code.  The handler would look something like:
>
>   QDict * coroutine_fn (*co_cmd)(Monitor *mon,
>                                  const QDict *qdict,
>                                  Error **errp)
>
> The monitor invokes it from coroutine context.  The coroutine just needs
> to return a QDict or set errp - no explicit QmpReturn API is needed.

Right, that would be nice. It should probably be built on top of the
non-coroutine version.

-- 
Marc-André Lureau