scripts/qapi/commands.py | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-)
Coroutine commands have to be declared as coroutine_fn, but the
marker does not show up in the qapi-comands-* headers; likewise, the
marshaling function calls the command and therefore must be coroutine_fn.
Static analysis would want coroutine_fn to match between prototype and
declaration, because in principle coroutines might be compiled to a
completely different calling convention. So we would like to add the
marker to the header.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
scripts/qapi/commands.py | 33 +++++++++++++++++++++------------
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 79c5e5c3a989..a079378d1b8d 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -41,11 +41,13 @@
def gen_command_decl(name: str,
arg_type: Optional[QAPISchemaObjectType],
boxed: bool,
- ret_type: Optional[QAPISchemaType]) -> str:
+ ret_type: Optional[QAPISchemaType],
+ coroutine: bool) -> str:
return mcgen('''
-%(c_type)s qmp_%(c_name)s(%(params)s);
+%(c_type)s %(coroutine_fn)sqmp_%(c_name)s(%(params)s);
''',
c_type=(ret_type and ret_type.c_type()) or 'void',
+ coroutine_fn='coroutine_fn ' if coroutine else '',
c_name=c_name(name),
params=build_params(arg_type, boxed, 'Error **errp'))
@@ -157,16 +159,21 @@ def gen_marshal_output(ret_type: QAPISchemaType) -> str:
c_type=ret_type.c_type(), c_name=ret_type.c_name())
-def build_marshal_proto(name: str) -> str:
- return ('void qmp_marshal_%s(QDict *args, QObject **ret, Error **errp)'
- % c_name(name))
+def build_marshal_proto(name: str,
+ coroutine: bool) -> str:
+ return ('void %(coroutine_fn)sqmp_marshal_%(c_name)s(%(params)s)' % {
+ 'coroutine_fn': 'coroutine_fn ' if coroutine else '',
+ 'c_name': c_name(name),
+ 'params': 'QDict *args, QObject **ret, Error **errp',
+ })
-def gen_marshal_decl(name: str) -> str:
+def gen_marshal_decl(name: str,
+ coroutine: bool) -> str:
return mcgen('''
%(proto)s;
''',
- proto=build_marshal_proto(name))
+ proto=build_marshal_proto(name, coroutine))
def gen_trace(name: str) -> str:
@@ -181,7 +188,8 @@ def gen_marshal(name: str,
arg_type: Optional[QAPISchemaObjectType],
boxed: bool,
ret_type: Optional[QAPISchemaType],
- gen_tracing: bool) -> str:
+ gen_tracing: bool,
+ coroutine: bool) -> str:
have_args = boxed or (arg_type and not arg_type.is_empty())
if have_args:
assert arg_type is not None
@@ -195,7 +203,7 @@ def gen_marshal(name: str,
bool ok = false;
Visitor *v;
''',
- proto=build_marshal_proto(name))
+ proto=build_marshal_proto(name, coroutine))
if ret_type:
ret += mcgen('''
@@ -387,10 +395,11 @@ def visit_command(self,
self._genh, self._genc):
self._genc.add(gen_marshal_output(ret_type))
with ifcontext(ifcond, self._genh, self._genc):
- self._genh.add(gen_command_decl(name, arg_type, boxed, ret_type))
- self._genh.add(gen_marshal_decl(name))
+ self._genh.add(gen_command_decl(name, arg_type, boxed,
+ ret_type, coroutine))
+ self._genh.add(gen_marshal_decl(name, coroutine))
self._genc.add(gen_marshal(name, arg_type, boxed, ret_type,
- self._gen_tracing))
+ self._gen_tracing, coroutine))
if self._gen_tracing:
self._gen_trace_events.add(gen_trace(name))
with self._temp_module('./init'):
--
2.39.2Paolo Bonzini <pbonzini@redhat.com> writes:
> Coroutine commands have to be declared as coroutine_fn, but the
> marker does not show up in the qapi-comands-* headers; likewise, the
> marshaling function calls the command and therefore must be coroutine_fn.
> Static analysis would want coroutine_fn to match between prototype and
> declaration, because in principle coroutines might be compiled to a
> completely different calling convention. So we would like to add the
> marker to the header.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This continues (completes?) Kevin's commit 04f22362f14 (qapi: Add a
'coroutine' flag for commands). I'm cc'ing him so he's aware; not a
request for review.
> ---
> scripts/qapi/commands.py | 33 +++++++++++++++++++++------------
> 1 file changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
> index 79c5e5c3a989..a079378d1b8d 100644
> --- a/scripts/qapi/commands.py
> +++ b/scripts/qapi/commands.py
> @@ -41,11 +41,13 @@
> def gen_command_decl(name: str,
> arg_type: Optional[QAPISchemaObjectType],
> boxed: bool,
> - ret_type: Optional[QAPISchemaType]) -> str:
> + ret_type: Optional[QAPISchemaType],
> + coroutine: bool) -> str:
> return mcgen('''
> -%(c_type)s qmp_%(c_name)s(%(params)s);
> +%(c_type)s %(coroutine_fn)sqmp_%(c_name)s(%(params)s);
> ''',
> c_type=(ret_type and ret_type.c_type()) or 'void',
> + coroutine_fn='coroutine_fn ' if coroutine else '',
> c_name=c_name(name),
> params=build_params(arg_type, boxed, 'Error **errp'))
>
> @@ -157,16 +159,21 @@ def gen_marshal_output(ret_type: QAPISchemaType) -> str:
> c_type=ret_type.c_type(), c_name=ret_type.c_name())
>
>
> -def build_marshal_proto(name: str) -> str:
> - return ('void qmp_marshal_%s(QDict *args, QObject **ret, Error **errp)'
> - % c_name(name))
> +def build_marshal_proto(name: str,
> + coroutine: bool) -> str:
No need for the line break. No big deal, and certainly not worth a
respin.
> + return ('void %(coroutine_fn)sqmp_marshal_%(c_name)s(%(params)s)' % {
> + 'coroutine_fn': 'coroutine_fn ' if coroutine else '',
> + 'c_name': c_name(name),
> + 'params': 'QDict *args, QObject **ret, Error **errp',
> + })
>
>
> -def gen_marshal_decl(name: str) -> str:
> +def gen_marshal_decl(name: str,
> + coroutine: bool) -> str:
Likewise.
> return mcgen('''
> %(proto)s;
> ''',
> - proto=build_marshal_proto(name))
> + proto=build_marshal_proto(name, coroutine))
>
>
> def gen_trace(name: str) -> str:
> @@ -181,7 +188,8 @@ def gen_marshal(name: str,
> arg_type: Optional[QAPISchemaObjectType],
> boxed: bool,
> ret_type: Optional[QAPISchemaType],
> - gen_tracing: bool) -> str:
> + gen_tracing: bool,
> + coroutine: bool) -> str:
> have_args = boxed or (arg_type and not arg_type.is_empty())
> if have_args:
> assert arg_type is not None
> @@ -195,7 +203,7 @@ def gen_marshal(name: str,
> bool ok = false;
> Visitor *v;
> ''',
> - proto=build_marshal_proto(name))
> + proto=build_marshal_proto(name, coroutine))
>
> if ret_type:
> ret += mcgen('''
> @@ -387,10 +395,11 @@ def visit_command(self,
> self._genh, self._genc):
> self._genc.add(gen_marshal_output(ret_type))
> with ifcontext(ifcond, self._genh, self._genc):
> - self._genh.add(gen_command_decl(name, arg_type, boxed, ret_type))
> - self._genh.add(gen_marshal_decl(name))
> + self._genh.add(gen_command_decl(name, arg_type, boxed,
> + ret_type, coroutine))
> + self._genh.add(gen_marshal_decl(name, coroutine))
> self._genc.add(gen_marshal(name, arg_type, boxed, ret_type,
> - self._gen_tracing))
> + self._gen_tracing, coroutine))
> if self._gen_tracing:
> self._gen_trace_events.add(gen_trace(name))
> with self._temp_module('./init'):
Since we have just three commands marked 'coroutine' so far
(block_resize, screendump, and one in tests), reviewing how the
generated code changes is quite practical. I append the diff.
Highlighting the changes confirms that we add coroutine_fn to the
prototypes and the definitions of these three commands and marshalling
helpers, and that's all.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
diff -rup qapi-gen-efcd0ec14b/commit qapi-gen-e46cbd9158/commit
--- qapi-gen-efcd0ec14b/commit 2023-04-06 07:50:53.212606061 +0200
+++ qapi-gen-e46cbd9158/commit 2023-04-06 07:49:49.254991726 +0200
@@ -1 +1 @@
-efcd0ec14b Merge tag 'misc-fixes-20230330' of https://github.com/philmd/qemu into staging
+e46cbd9158 qapi-gen: mark coroutine QMP command functions as coroutine_fn
diff -rup qapi-gen-efcd0ec14b/qapi-commands-block-core.c qapi-gen-e46cbd9158/qapi-commands-block-core.c
--- qapi-gen-efcd0ec14b/qapi-commands-block-core.c 2023-04-06 07:50:41.951850170 +0200
+++ qapi-gen-e46cbd9158/qapi-commands-block-core.c 2023-04-06 07:49:36.071277209 +0200
@@ -208,7 +208,7 @@ out:
visit_free(v);
}
-void qmp_marshal_block_resize(QDict *args, QObject **ret, Error **errp)
+void coroutine_fn qmp_marshal_block_resize(QDict *args, QObject **ret, Error **errp)
{
Error *err = NULL;
bool ok = false;
diff -rup qapi-gen-efcd0ec14b/qapi-commands-block-core.h qapi-gen-e46cbd9158/qapi-commands-block-core.h
--- qapi-gen-efcd0ec14b/qapi-commands-block-core.h 2023-04-06 07:50:41.951850170 +0200
+++ qapi-gen-e46cbd9158/qapi-commands-block-core.h 2023-04-06 07:49:36.071277209 +0200
@@ -25,8 +25,8 @@ BlockStatsList *qmp_query_blockstats(boo
void qmp_marshal_query_blockstats(QDict *args, QObject **ret, Error **errp);
BlockJobInfoList *qmp_query_block_jobs(Error **errp);
void qmp_marshal_query_block_jobs(QDict *args, QObject **ret, Error **errp);
-void qmp_block_resize(const char *device, const char *node_name, int64_t size, Error **errp);
-void qmp_marshal_block_resize(QDict *args, QObject **ret, Error **errp);
+void coroutine_fn qmp_block_resize(const char *device, const char *node_name, int64_t size, Error **errp);
+void coroutine_fn qmp_marshal_block_resize(QDict *args, QObject **ret, Error **errp);
void qmp_blockdev_snapshot_sync(const char *device, const char *node_name, const char *snapshot_file, const char *snapshot_node_name, const char *format, bool has_mode, NewImageMode mode, Error **errp);
void qmp_marshal_blockdev_snapshot_sync(QDict *args, QObject **ret, Error **errp);
void qmp_blockdev_snapshot(const char *node, const char *overlay, Error **errp);
diff -rup qapi-gen-efcd0ec14b/qapi-commands-ui.c qapi-gen-e46cbd9158/qapi-commands-ui.c
--- qapi-gen-efcd0ec14b/qapi-commands-ui.c 2023-04-06 07:50:41.953850127 +0200
+++ qapi-gen-e46cbd9158/qapi-commands-ui.c 2023-04-06 07:49:36.073277166 +0200
@@ -107,7 +107,7 @@ out:
visit_free(v);
}
-void qmp_marshal_screendump(QDict *args, QObject **ret, Error **errp)
+void coroutine_fn qmp_marshal_screendump(QDict *args, QObject **ret, Error **errp)
{
Error *err = NULL;
bool ok = false;
diff -rup qapi-gen-efcd0ec14b/qapi-commands-ui.h qapi-gen-e46cbd9158/qapi-commands-ui.h
--- qapi-gen-efcd0ec14b/qapi-commands-ui.h 2023-04-06 07:50:41.953850127 +0200
+++ qapi-gen-e46cbd9158/qapi-commands-ui.h 2023-04-06 07:49:36.073277166 +0200
@@ -21,8 +21,8 @@ void qmp_set_password(SetPasswordOptions
void qmp_marshal_set_password(QDict *args, QObject **ret, Error **errp);
void qmp_expire_password(ExpirePasswordOptions *arg, Error **errp);
void qmp_marshal_expire_password(QDict *args, QObject **ret, Error **errp);
-void qmp_screendump(const char *filename, const char *device, bool has_head, int64_t head, bool has_format, ImageFormat format, Error **errp);
-void qmp_marshal_screendump(QDict *args, QObject **ret, Error **errp);
+void coroutine_fn qmp_screendump(const char *filename, const char *device, bool has_head, int64_t head, bool has_format, ImageFormat format, Error **errp);
+void coroutine_fn qmp_marshal_screendump(QDict *args, QObject **ret, Error **errp);
#if defined(CONFIG_SPICE)
SpiceInfo *qmp_query_spice(Error **errp);
void qmp_marshal_query_spice(QDict *args, QObject **ret, Error **errp);
diff -rup qapi-gen-efcd0ec14b/test-qapi-commands.c qapi-gen-e46cbd9158/test-qapi-commands.c
--- qapi-gen-efcd0ec14b/test-qapi-commands.c 2023-04-06 07:50:42.146845943 +0200
+++ qapi-gen-e46cbd9158/test-qapi-commands.c 2023-04-06 07:49:36.286272553 +0200
@@ -213,7 +213,7 @@ out:
visit_free(v);
}
-void qmp_marshal_coroutine_cmd(QDict *args, QObject **ret, Error **errp)
+void coroutine_fn qmp_marshal_coroutine_cmd(QDict *args, QObject **ret, Error **errp)
{
Error *err = NULL;
bool ok = false;
diff -rup qapi-gen-efcd0ec14b/test-qapi-commands.h qapi-gen-e46cbd9158/test-qapi-commands.h
--- qapi-gen-efcd0ec14b/test-qapi-commands.h 2023-04-06 07:50:42.146845943 +0200
+++ qapi-gen-e46cbd9158/test-qapi-commands.h 2023-04-06 07:49:36.286272553 +0200
@@ -26,8 +26,8 @@ UserDefTwo *qmp_user_def_cmd2(UserDefOne
void qmp_marshal_user_def_cmd2(QDict *args, QObject **ret, Error **errp);
void qmp_cmd_success_response(Error **errp);
void qmp_marshal_cmd_success_response(QDict *args, QObject **ret, Error **errp);
-void qmp_coroutine_cmd(Error **errp);
-void qmp_marshal_coroutine_cmd(QDict *args, QObject **ret, Error **errp);
+void coroutine_fn qmp_coroutine_cmd(Error **errp);
+void coroutine_fn qmp_marshal_coroutine_cmd(QDict *args, QObject **ret, Error **errp);
int64_t qmp_guest_get_time(int64_t a, bool has_b, int64_t b, Error **errp);
void qmp_marshal_guest_get_time(QDict *args, QObject **ret, Error **errp);
QObject *qmp_guest_sync(QObject *arg, Error **errp);
© 2016 - 2026 Red Hat, Inc.