Re: [Qemu-devel] [PATCH v8 17/23] qapi: introduce new cmd option "allow-oob"

[Eric Blake]

On 03/09/2018 03:00 AM, Peter Xu wrote:
> Here "oob" stands for "Out-Of-Band".  When "allow-oob" is set, it means
> the command allows out-of-band execution.
>
> The "oob" idea is proposed by Markus Armbruster in following thread:
>
>    https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg02057.html
>
> This new "allow-oob" boolean will be exposed by "query-qmp-schema" as
> well for command entries, so that QMP clients can know which command can

s/command can/commands/can/

> be used as out-of-band calls. For example the command "migrate"
> originally looks like:
>
>    {"name": "migrate", "ret-type": "17", "meta-type": "command",
>     "arg-type": "86"}
>
> And it'll be changed into:
>
>    {"name": "migrate", "ret-type": "17", "allow-oob": false,
>     "meta-type": "command", "arg-type": "86"}
>
> This patch only provides the QMP interface level changes.  It does not
> contains the real out-of-band execution implementation yet.

s/contains/contain/

> Suggested-by: Markus Armbruster <address@hidden>
> Reviewed-by: Stefan Hajnoczi <address@hidden>
> Reviewed-by: Fam Zheng <address@hidden>
> Signed-off-by: Peter Xu <address@hidden>
> ---
>  include/qapi/qmp/dispatch.h    |  5 +++--
>  qapi/introspect.json           |  6 +++++-
>  scripts/qapi/commands.py       | 18 +++++++++++++-----
>  scripts/qapi/common.py         | 15 ++++++++++-----
>  scripts/qapi/doc.py            |  2 +-
>  scripts/qapi/introspect.py     | 10 ++++++++--
>  tests/qapi-schema/test-qapi.py |  2 +-
>  7 files changed, 41 insertions(+), 17 deletions(-)

I'm a bit disappointed that tests/qapi-schema/qapi-schema-test.json 
doesn't have any "allow-oob":true commands at any point in the series. 
I consider that to be a bug, so it can be fixed by followup patch even 
after soft freeze; but won't make the lack of good testing hold up this 
series from making the release.

> +++ b/scripts/qapi/introspect.py
> @@ -29,6 +29,11 @@ def to_json(obj, level=0):
>                                 to_json(obj[key], level + 1))
>                   for key in sorted(obj.keys())]
>           ret = '{' + ', '.join(elts) + '}'
> +    elif isinstance(obj, bool):
> +        if obj:
> +            ret = 'true'
> +        else:
> +            ret = 'false'

Conflicts with Marc-Andre's work to perform introspection by QLIT_*; but 
it's easy enough to adjust:

+    elif isinstance(obj, bool):
+        ret += 'QLIT_QBOOL(%s)' % ('true' if obj else 'false')

Reviewed-by: Eric Blake <address@hidden>

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org