Am 14.09.2020 um 17:15 hat Markus Armbruster geschrieben: > Kevin Wolf <kw...@redhat.com> writes: > > > This patch adds a new 'coroutine' flag to QMP command definitions that > > tells the QMP dispatcher that the command handler is safe to be run in a > > coroutine. > > > > The documentation of the new flag pretends that this flag is already > > used as intended, which it isn't yet after this patch. We'll implement > > this in another patch in this series. > > > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > > Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com> > > Reviewed-by: Markus Armbruster <arm...@redhat.com> > > --- > > docs/devel/qapi-code-gen.txt | 12 ++++++++++++ > > include/qapi/qmp/dispatch.h | 1 + > > tests/test-qmp-cmds.c | 4 ++++ > > scripts/qapi/commands.py | 10 +++++++--- > > scripts/qapi/doc.py | 2 +- > > scripts/qapi/expr.py | 10 ++++++++-- > > scripts/qapi/introspect.py | 2 +- > > scripts/qapi/schema.py | 12 ++++++++---- > > tests/qapi-schema/test-qapi.py | 7 ++++--- > > tests/qapi-schema/meson.build | 1 + > > tests/qapi-schema/oob-coroutine.err | 2 ++ > > tests/qapi-schema/oob-coroutine.json | 2 ++ > > tests/qapi-schema/oob-coroutine.out | 0 > > tests/qapi-schema/qapi-schema-test.json | 1 + > > tests/qapi-schema/qapi-schema-test.out | 2 ++ > > 15 files changed, 54 insertions(+), 14 deletions(-) > > create mode 100644 tests/qapi-schema/oob-coroutine.err > > create mode 100644 tests/qapi-schema/oob-coroutine.json > > create mode 100644 tests/qapi-schema/oob-coroutine.out > > > > diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt > > index f3e7ced212..36daa9b5f8 100644 > > --- a/docs/devel/qapi-code-gen.txt > > +++ b/docs/devel/qapi-code-gen.txt > > @@ -472,6 +472,7 @@ Syntax: > > '*gen': false, > > '*allow-oob': true, > > '*allow-preconfig': true, > > + '*coroutine': true, > > '*if': COND, > > '*features': FEATURES } > > > > @@ -596,6 +597,17 @@ before the machine is built. It defaults to false. > > For example: > > QMP is available before the machine is built only when QEMU was > > started with --preconfig. > > > > +Member 'coroutine' tells the QMP dispatcher whether the command handler > > +is safe to be run in a coroutine. > > We need to document what exactly makes a command handler coroutine safe > / unsafe. We discussed this at some length in review of PATCH v4 1/4: > > Message-ID: <874kwnvgad....@dusky.pond.sub.org> > https://lists.nongnu.org/archive/html/qemu-devel/2020-01/msg05015.html > > I'd be willing to accept a follow-up patch, if that's more convenient > for you.
Did we ever arrive at a conclusion for a good definition? I mean I can give a definition like "it's coroutine safe if it results in the right behaviour even when called in coroutine context", but that's not really helpful. FWIW, I would also have a hard time giving a much better definition than this for thread safety. > > It defaults to false. If it is true, > > +the command handler is called from coroutine context and may yield while > > Is it *always* called from coroutine context, or could it be called > outside coroutine context, too? I guess the former, thanks to PATCH 10, > and as long as we diligently mark HMP commands that such call QMP > handlers, like you do in PATCH 13. Yes, it must *always* be called from coroutine context. This is the reason why I included HMP after all, even though I'm really mostly just interested in QMP. > What's the worst than can happen when we neglect to mark such an HMP > command? When the command handler tries to yield and it's not in coroutine context, it will abort(). If it never tries to yield, I think it would just work - but of course, the ability to yield is the only reason why you would want to run a command handler in a coroutine. Kevin
