From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, marcandre.lureau@gmail.com, armbru@redhat.com,
qemu-devel@nongnu.org
Subject: [PATCH v6 07/12] qapi: Add a 'coroutine' flag for commands
Date: Thu, 28 May 2020 17:37:37 +0200 [thread overview]
Message-ID: <20200528153742.274164-8-kwolf@redhat.com> (raw)
In-Reply-To: <20200528153742.274164-1-kwolf@redhat.com>
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 <kwolf@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@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/Makefile.include | 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 a7794ef658..b8eb370a13 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. It defaults to false. If it is true,
+the command handler is called from coroutine context and may yield while
+waiting for an external event (such as I/O completion) in order to avoid
+blocking the guest and other background operations.
+
+It is an error to specify both 'coroutine': true and 'allow-oob': true
+for a command. We don't currently have a use case for both together and
+without a use case, it's not entirely clear what the semantics should
+be.
+
The optional 'if' member specifies a conditional. See "Configuring
the schema" below for more on this.
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 0c2f467028..9fd2b720a7 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -25,6 +25,7 @@ typedef enum QmpCommandOptions
QCO_NO_SUCCESS_RESP = (1U << 0),
QCO_ALLOW_OOB = (1U << 1),
QCO_ALLOW_PRECONFIG = (1U << 2),
+ QCO_COROUTINE = (1U << 3),
} QmpCommandOptions;
typedef struct QmpCommand
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index 5f1b245e19..d3413bfef0 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -36,6 +36,10 @@ void qmp_cmd_success_response(Error **errp)
{
}
+void qmp_coroutine_cmd(Error **errp)
+{
+}
+
Empty2 *qmp_user_def_cmd0(Error **errp)
{
return g_new0(Empty2, 1);
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 6809b0fb6e..f50710306d 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -180,7 +180,8 @@ out:
return ret
-def gen_register_command(name, success_response, allow_oob, allow_preconfig):
+def gen_register_command(name, success_response, allow_oob, allow_preconfig,
+ coroutine):
options = []
if not success_response:
@@ -189,6 +190,8 @@ def gen_register_command(name, success_response, allow_oob, allow_preconfig):
options += ['QCO_ALLOW_OOB']
if allow_preconfig:
options += ['QCO_ALLOW_PRECONFIG']
+ if coroutine:
+ options += ['QCO_COROUTINE']
if not options:
options = ['QCO_NO_OPTIONS']
@@ -271,7 +274,7 @@ void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
def visit_command(self, name, info, ifcond, features,
arg_type, ret_type, gen, success_response, boxed,
- allow_oob, allow_preconfig):
+ allow_oob, allow_preconfig, coroutine):
if not gen:
return
# FIXME: If T is a user-defined type, the user is responsible
@@ -289,7 +292,8 @@ void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
self._genh.add(gen_marshal_decl(name))
self._genc.add(gen_marshal(name, arg_type, boxed, ret_type))
self._regy.add(gen_register_command(name, success_response,
- allow_oob, allow_preconfig))
+ allow_oob, allow_preconfig,
+ coroutine))
def gen_commands(schema, output_dir, prefix):
diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 92f584edcf..11771de923 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -264,7 +264,7 @@ class QAPISchemaGenDocVisitor(QAPISchemaVisitor):
def visit_command(self, name, info, ifcond, features,
arg_type, ret_type, gen, success_response, boxed,
- allow_oob, allow_preconfig):
+ allow_oob, allow_preconfig, coroutine):
doc = self.cur_doc
self._gen.add(texi_msg('Command', doc, ifcond,
texi_arguments(doc,
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 2942520399..928cd1eb5c 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -88,10 +88,16 @@ def check_flags(expr, info):
if key in expr and expr[key] is not False:
raise QAPISemError(
info, "flag '%s' may only use false value" % key)
- for key in ['boxed', 'allow-oob', 'allow-preconfig']:
+ for key in ['boxed', 'allow-oob', 'allow-preconfig', 'coroutine']:
if key in expr and expr[key] is not True:
raise QAPISemError(
info, "flag '%s' may only use true value" % key)
+ if 'allow-oob' in expr and 'coroutine' in expr:
+ # This is not necessarily a fundamental incompatibility, but we don't
+ # have a use case and the desired semantics isn't obvious. The simplest
+ # solution is to forbid it until we get a use case for it.
+ raise QAPISemError(info, "flags 'allow-oob' and 'coroutine' "
+ "are incompatible")
def check_if(expr, info, source):
@@ -342,7 +348,7 @@ def check_exprs(exprs):
['command'],
['data', 'returns', 'boxed', 'if', 'features',
'gen', 'success-response', 'allow-oob',
- 'allow-preconfig'])
+ 'allow-preconfig', 'coroutine'])
normalize_members(expr.get('data'))
check_command(expr, info)
elif meta == 'event':
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 23652be810..5907b09cd5 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -216,7 +216,7 @@ const QLitObject %(c_name)s = %(c_string)s;
def visit_command(self, name, info, ifcond, features,
arg_type, ret_type, gen, success_response, boxed,
- allow_oob, allow_preconfig):
+ allow_oob, allow_preconfig, coroutine):
arg_type = arg_type or self._schema.the_empty_object_type
ret_type = ret_type or self._schema.the_empty_object_type
obj = {'arg-type': self._use_type(arg_type),
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 78309a00f0..c44d391c3f 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -128,7 +128,7 @@ class QAPISchemaVisitor:
def visit_command(self, name, info, ifcond, features,
arg_type, ret_type, gen, success_response, boxed,
- allow_oob, allow_preconfig):
+ allow_oob, allow_preconfig, coroutine):
pass
def visit_event(self, name, info, ifcond, features, arg_type, boxed):
@@ -713,7 +713,8 @@ class QAPISchemaCommand(QAPISchemaEntity):
def __init__(self, name, info, doc, ifcond, features,
arg_type, ret_type,
- gen, success_response, boxed, allow_oob, allow_preconfig):
+ gen, success_response, boxed, allow_oob, allow_preconfig,
+ coroutine):
super().__init__(name, info, doc, ifcond, features)
assert not arg_type or isinstance(arg_type, str)
assert not ret_type or isinstance(ret_type, str)
@@ -726,6 +727,7 @@ class QAPISchemaCommand(QAPISchemaEntity):
self.boxed = boxed
self.allow_oob = allow_oob
self.allow_preconfig = allow_preconfig
+ self.coroutine = coroutine
def check(self, schema):
super().check(schema)
@@ -768,7 +770,7 @@ class QAPISchemaCommand(QAPISchemaEntity):
visitor.visit_command(
self.name, self.info, self.ifcond, self.features,
self.arg_type, self.ret_type, self.gen, self.success_response,
- self.boxed, self.allow_oob, self.allow_preconfig)
+ self.boxed, self.allow_oob, self.allow_preconfig, self.coroutine)
class QAPISchemaEvent(QAPISchemaEntity):
@@ -1074,6 +1076,7 @@ class QAPISchema:
boxed = expr.get('boxed', False)
allow_oob = expr.get('allow-oob', False)
allow_preconfig = expr.get('allow-preconfig', False)
+ coroutine = expr.get('coroutine', False)
ifcond = expr.get('if')
features = self._make_features(expr.get('features'), info)
if isinstance(data, OrderedDict):
@@ -1086,7 +1089,8 @@ class QAPISchema:
self._def_entity(QAPISchemaCommand(name, info, doc, ifcond, features,
data, rets,
gen, success_response,
- boxed, allow_oob, allow_preconfig))
+ boxed, allow_oob, allow_preconfig,
+ coroutine))
def _def_event(self, expr, info, doc):
name = expr['event']
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index f396b471eb..e8db9d09d9 100755
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -68,12 +68,13 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
def visit_command(self, name, info, ifcond, features,
arg_type, ret_type, gen, success_response, boxed,
- allow_oob, allow_preconfig):
+ allow_oob, allow_preconfig, coroutine):
print('command %s %s -> %s'
% (name, arg_type and arg_type.name,
ret_type and ret_type.name))
- print(' gen=%s success_response=%s boxed=%s oob=%s preconfig=%s'
- % (gen, success_response, boxed, allow_oob, allow_preconfig))
+ print(' gen=%s success_response=%s boxed=%s oob=%s preconfig=%s%s'
+ % (gen, success_response, boxed, allow_oob, allow_preconfig,
+ " coroutine=True" if coroutine else ""))
self._print_if(ifcond)
self._print_features(features)
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 03a74b60f6..bb69487436 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -290,6 +290,7 @@ qapi-schema += missing-type.json
qapi-schema += nested-struct-data.json
qapi-schema += nested-struct-data-invalid-dict.json
qapi-schema += non-objects.json
+qapi-schema += oob-coroutine.json
qapi-schema += oob-test.json
qapi-schema += allow-preconfig-test.json
qapi-schema += pragma-doc-required-crap.json
diff --git a/tests/qapi-schema/oob-coroutine.err b/tests/qapi-schema/oob-coroutine.err
new file mode 100644
index 0000000000..c01a4992bd
--- /dev/null
+++ b/tests/qapi-schema/oob-coroutine.err
@@ -0,0 +1,2 @@
+oob-coroutine.json: In command 'oob-command-1':
+oob-coroutine.json:2: flags 'allow-oob' and 'coroutine' are incompatible
diff --git a/tests/qapi-schema/oob-coroutine.json b/tests/qapi-schema/oob-coroutine.json
new file mode 100644
index 0000000000..0f67663bcd
--- /dev/null
+++ b/tests/qapi-schema/oob-coroutine.json
@@ -0,0 +1,2 @@
+# Check that incompatible flags allow-oob and coroutine are rejected
+{ 'command': 'oob-command-1', 'allow-oob': true, 'coroutine': true }
diff --git a/tests/qapi-schema/oob-coroutine.out b/tests/qapi-schema/oob-coroutine.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 6b1f05afa7..3e29fe5b40 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -147,6 +147,7 @@
'returns': 'UserDefTwo' }
{ 'command': 'cmd-success-response', 'data': {}, 'success-response': false }
+{ 'command': 'coroutine-cmd', 'data': {}, 'coroutine': true }
# Returning a non-dictionary requires a name from the whitelist
{ 'command': 'guest-get-time', 'data': {'a': 'int', '*b': 'int' },
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 891b4101e0..8868ca0dca 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -203,6 +203,8 @@ command user_def_cmd2 q_obj_user_def_cmd2-arg -> UserDefTwo
gen=True success_response=True boxed=False oob=False preconfig=False
command cmd-success-response None -> None
gen=True success_response=False boxed=False oob=False preconfig=False
+command coroutine-cmd None -> None
+ gen=True success_response=True boxed=False oob=False preconfig=False coroutine=True
object q_obj_guest-get-time-arg
member a: int optional=False
member b: int optional=True
--
2.25.4
next prev parent reply other threads:[~2020-05-28 15:43 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-28 15:37 [PATCH v6 00/12] monitor: Optionally run handlers in coroutines Kevin Wolf
2020-05-28 15:37 ` [PATCH v6 01/12] monitor: Add Monitor parameter to monitor_set_cpu() Kevin Wolf
2020-05-28 18:24 ` Eric Blake
2020-08-04 11:19 ` Markus Armbruster
2020-05-28 15:37 ` [PATCH v6 02/12] monitor: Use getter/setter functions for cur_mon Kevin Wolf
2020-05-28 18:31 ` Eric Blake
2020-06-02 13:36 ` Kevin Wolf
2020-05-28 18:36 ` Eric Blake
2020-08-04 12:46 ` Markus Armbruster
2020-08-04 16:16 ` Kevin Wolf
2020-08-05 7:19 ` Markus Armbruster
2020-08-05 8:25 ` Kevin Wolf
2020-08-05 4:45 ` Markus Armbruster
2020-05-28 15:37 ` [PATCH v6 03/12] hmp: Set cur_mon only in handle_hmp_command() Kevin Wolf
2020-05-28 18:37 ` Eric Blake
2020-08-04 12:54 ` Markus Armbruster
2020-05-28 15:37 ` [PATCH v6 04/12] qmp: Assert that no other monitor is active Kevin Wolf
2020-05-28 18:38 ` Eric Blake
2020-08-04 12:57 ` Markus Armbruster
2020-05-28 15:37 ` [PATCH v6 05/12] qmp: Call monitor_set_cur() only in qmp_dispatch() Kevin Wolf
2020-05-28 18:42 ` Eric Blake
2020-08-04 13:17 ` Markus Armbruster
2020-05-28 15:37 ` [PATCH v6 06/12] monitor: Make current monitor a per-coroutine property Kevin Wolf
2020-05-28 18:44 ` Eric Blake
2020-08-04 13:50 ` Markus Armbruster
2020-08-04 16:06 ` Kevin Wolf
2020-08-05 7:28 ` Markus Armbruster
2020-08-05 8:32 ` Kevin Wolf
2020-08-07 13:09 ` Ways to do per-coroutine properties (was: [PATCH v6 06/12] monitor: Make current monitor a per-coroutine property) Markus Armbruster
2020-08-07 13:27 ` [PATCH] Simple & stupid coroutine-aware monitor_cur() Markus Armbruster
2020-08-10 12:19 ` Kevin Wolf
2020-08-26 12:37 ` Markus Armbruster
2020-08-07 13:29 ` [PATCH] Coroutine-aware monitor_cur() with coroutine-specific data Markus Armbruster
2020-08-10 12:58 ` Kevin Wolf
2020-08-26 12:40 ` Markus Armbruster
2020-08-26 13:49 ` Kevin Wolf
2020-08-04 16:14 ` [PATCH v6 06/12] monitor: Make current monitor a per-coroutine property Daniel P. Berrangé
2020-05-28 15:37 ` Kevin Wolf [this message]
2020-05-28 15:37 ` [PATCH v6 08/12] qmp: Move dispatcher to a coroutine Kevin Wolf
2020-08-05 10:03 ` Markus Armbruster
2020-05-28 15:37 ` [PATCH v6 09/12] hmp: Add support for coroutine command handlers Kevin Wolf
2020-08-05 10:33 ` Markus Armbruster
2020-05-28 15:37 ` [PATCH v6 10/12] util/async: Add aio_co_reschedule_self() Kevin Wolf
2020-05-28 15:37 ` [PATCH v6 11/12] block: Add bdrv_co_move_to_aio_context() Kevin Wolf
2020-05-28 15:37 ` [PATCH v6 12/12] block: Convert 'block_resize' to coroutine Kevin Wolf
2020-08-04 11:16 ` [PATCH v6 00/12] monitor: Optionally run handlers in coroutines Markus Armbruster
2020-08-05 11:34 ` Markus Armbruster
2020-09-03 10:49 ` Markus Armbruster
2020-09-03 12:45 ` Kevin Wolf
2020-09-03 14:17 ` Markus Armbruster
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200528153742.274164-8-kwolf@redhat.com \
--to=kwolf@redhat.com \
--cc=armbru@redhat.com \
--cc=marcandre.lureau@gmail.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.