Kevin Wolf <kw...@redhat.com> writes:
> Am 14.09.2020 um 17:30 hat Markus Armbruster geschrieben:
>> Kevin Wolf <kw...@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 <kw...@redhat.com>
>> > Reviewed-by: Markus Armbruster <arm...@redhat.com>
>> > ---
[...]
>> > diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
>> > index 5677ba92ca..754f7b854c 100644
>> > --- a/qapi/qmp-dispatch.c
>> > +++ b/qapi/qmp-dispatch.c
[...]
>> > @@ -153,12 +181,35 @@ QDict *qmp_dispatch(const QmpCommandList *cmds,
>> > QObject *request,
>> > qobject_ref(args);
>> > }
>> >
>> > + assert(!(oob && qemu_in_coroutine()));
>> > assert(monitor_cur() == NULL);
>> > - monitor_set_cur(qemu_coroutine_self(), cur_mon);
>> > -
>> > - cmd->fn(args, &ret, &err);
>> > -
>> > - monitor_set_cur(qemu_coroutine_self(), NULL);
>> > + if (!!(cmd->options & QCO_COROUTINE) == qemu_in_coroutine()) {
>> > + monitor_set_cur(qemu_coroutine_self(), cur_mon);
>> > + cmd->fn(args, &ret, &err);
>> > + monitor_set_cur(qemu_coroutine_self(), NULL);
>> > + } else {
>> > + /*
>> > + * Not being in coroutine context implies that we're handling
>> > + * an OOB command, which must not have QCO_COROUTINE.
>> > + *
>> > + * This implies that we are in coroutine context, but the
>> > + * command doesn't have QCO_COROUTINE. We must drop out of
>> > + * coroutine context for this one.
>> > + */
>>
>> I had to read this several times to get it. The first sentence leads me
>> into coroutine context, and then the next sentence tells me the
>> opposite, throwing me into confusion.
>>
>> Perhaps something like this:
>>
>> /*
>> * Actual context doesn't match the one the command needs.
>> * Case 1: we are in coroutine context, but command does not
>> * have QCO_COROUTINE. We need to drop out of coroutine
>> * context for executing it.
>> * Case 2: we are outside coroutine context, but command has
>> * QCO_COROUTINE. Can't actually happen, because we get here
>> * outside coroutine context only when executing a command
>> * out of band, and OOB commands never have QCO_COROUTINE.
>> */
>
> Works for me. Can you squash this in while applying?
Sure!
