From 942ab6865ab217f370dc2e81415774aabe8b1ea8 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 9 Mar 2020 15:26:38 +0100 Subject: docs/devel/qapi-code-gen: Fix typo in grammar An ALTERNATIVE's value can only be a type name. Arrays are not supported, yet. The text gets it right: "The form STRING is shorthand for { 'type': STRING }." The grammar doesn't. Fix it. Fixes: b6c37ebaaf074cac8fe8a4781dc3e79db23e914e Reported-by: John Snow Signed-off-by: Markus Armbruster Message-Id: <20200309142638.19988-1-armbru@redhat.com> Reviewed-by: John Snow --- docs/devel/qapi-code-gen.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt index 59d6973..e73979e 100644 --- a/docs/devel/qapi-code-gen.txt +++ b/docs/devel/qapi-code-gen.txt @@ -399,7 +399,7 @@ Syntax: 'data': ALTERNATIVES, '*if': COND } ALTERNATIVES = { ALTERNATIVE, ... } - ALTERNATIVE = STRING : TYPE-REF + ALTERNATIVE = STRING : STRING | STRING : { 'type': STRING, '*if': COND } Member 'alternate' names the alternate type. -- cgit v1.1 From 73756ae3e31814b3feb14e6385538679a1cc189c Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:26 +0100 Subject: qemu-doc: Belatedly document QMP command arg & result deprecation A number of deprecated QMP arguments and results were missed in commit eb22aeca65 "docs: document deprecation policy & deprecated features in appendix" (v2.10.0): * Commit b33945cfff "block: Accept device model name for blockdev-open/close-tray" (v2.8.0) deprecated blockdev-open-tray, blockdev-close-tray argument @device. * Commit fbe2d8163e "block: Accept device model name for eject" (v2.8.0) deprecated eject argument @device. * Commit 70e2cb3bd7 "block: Accept device model name for blockdev-change-medium" (v2.8.0) deprecated blockdev-change-medium argument @device. * Commit 7a9877a026 "block: Accept device model name for block_set_io_throttle" (v2.8.0) deprecated block_set_io_throttle argument @device. * Commit c01c214b69 "block: remove all encryption handling APIs" (v2.10.0) deprecated query-named-block-nodes result @encryption_key_missing and query-block result @inserted member @encryption_key_missing. * Commit c42e8742f5 "block: Use JSON null instead of "" to disable backing file" (v2.10.0) deprecated blockdev-add empty string argument @backing. Since then, we missed a few more: * Commit 3c605f4074 "commit: Add top-node/base-node options" (v3.1.0) deprecated block-commit arguments @base and @top. * Commit 4db6ceb0b5 "block/dirty-bitmap: add recording and busy properties" (v4.0.0) deprecated query-named-block-nodes result @dirty-bitmaps member @status, not just query-block. Make up for all that. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200317115459.31821-2-armbru@redhat.com> --- docs/system/deprecated.rst | 48 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 0838338..bfc693e 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -180,27 +180,67 @@ QEMU Machine Protocol (QMP) commands Use ``blockdev-change-medium`` or ``change-vnc-password`` instead. +``blockdev-open-tray``, ``blockdev-close-tray`` argument ``device`` (since 2.8.0) +''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +Use argument ``id`` instead. + +``eject`` argument ``device`` (since 2.8.0) +''''''''''''''''''''''''''''''''''''''''''' + +Use argument ``id`` instead. + +``blockdev-change-medium`` argument ``device`` (since 2.8.0) +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +Use argument ``id`` instead. + +``block_set_io_throttle`` argument ``device`` (since 2.8.0) +''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +Use argument ``id`` instead. + ``migrate_set_downtime`` and ``migrate_set_speed`` (since 2.8.0) '''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' Use ``migrate-set-parameters`` instead. +``query-named-block-nodes`` result ``encryption_key_missing`` (since 2.10.0) +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +Always false. + +``query-block`` result ``inserted.encryption_key_missing`` (since 2.10.0) +''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +Always false. + +``blockdev-add`` empty string argument ``backing`` (since 2.10.0) +''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +Use argument value ``null`` instead. + ``migrate-set-cache-size`` and ``query-migrate-cache-size`` (since 2.11.0) '''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' Use ``migrate-set-parameters`` and ``query-migrate-parameters`` instead. +``block-commit`` arguments ``base`` and ``top`` (since 3.1.0) +''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +Use arguments ``base-node`` and ``top-node`` instead. + ``object-add`` option ``props`` (since 5.0) ''''''''''''''''''''''''''''''''''''''''''' Specify the properties for the object as top-level arguments instead. -``query-block`` result field ``dirty-bitmaps[i].status`` (since 4.0) -'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' +``query-named-block-nodes`` and ``query-block`` result dirty-bitmaps[i].status (since 4.0) +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' The ``status`` field of the ``BlockDirtyInfo`` structure, returned by -the query-block command is deprecated. Two new boolean fields, -``recording`` and ``busy`` effectively replace it. +these commands is deprecated. Two new boolean fields, ``recording`` and +``busy`` effectively replace it. ``query-block`` result field ``dirty-bitmaps`` (Since 4.2) '''''''''''''''''''''''''''''''''''''''''''''''''''''''''' -- cgit v1.1 From 0f365e3332a92c9f96f04691afb24a471368d33e Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:27 +0100 Subject: qapi: Belatedly update doc comment for @wait deprecation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit a9b305ba29 "socket: allow wait=false for client socket" deprecated use of @wait for client socket chardevs, but neglected to update char.json's doc comment. Make up for that. Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Reviewed-by: Eric Blake Message-Id: <20200317115459.31821-3-armbru@redhat.com> --- qapi/char.json | 1 + 1 file changed, 1 insertion(+) diff --git a/qapi/char.json b/qapi/char.json index 6907b2b..daceb20 100644 --- a/qapi/char.json +++ b/qapi/char.json @@ -258,6 +258,7 @@ # @server: create server socket (default: true) # @wait: wait for incoming connection on server # sockets (default: false). +# Silently ignored with server: false. This use is deprecated. # @nodelay: set TCP_NODELAY socket option (default: false) # @telnet: enable telnet protocol on server # sockets (default: false) -- cgit v1.1 From ad52292ea14f20b5ad296e0dee8a2a801c77717e Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:28 +0100 Subject: docs/devel/qapi-code-gen: Clarify allow-oob introspection Mention SchemaInfo variant member "allow-oob" defaults to false. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200317115459.31821-4-armbru@redhat.com> --- docs/devel/qapi-code-gen.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt index e73979e..3ce03e6 100644 --- a/docs/devel/qapi-code-gen.txt +++ b/docs/devel/qapi-code-gen.txt @@ -988,9 +988,9 @@ The SchemaInfo for a command has meta-type "command", and variant members "arg-type", "ret-type" and "allow-oob". On the wire, the "arguments" member of a client's "execute" command must conform to the object type named by "arg-type". The "return" member that the server -passes in a success response conforms to the type named by -"ret-type". When "allow-oob" is set, it means the command supports -out-of-band execution. +passes in a success response conforms to the type named by "ret-type". +When "allow-oob" is true, it means the command supports out-of-band +execution. It defaults to false. If the command takes no arguments, "arg-type" names an object type without members. Likewise, if the command returns nothing, "ret-type" -- cgit v1.1 From 86014c64f9a1509ab2a99d864b606882584e1f58 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:29 +0100 Subject: docs/devel/qapi-code-gen: Document 'features' introspection Commit 6a8c0b5102 "qapi: Add feature flags to struct types" neglected to update section "Client JSON Protocol introspection", and commit 23394b4c39 "qapi: Add feature flags to commands" didn't either. Make up for that. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200317115459.31821-5-armbru@redhat.com> --- docs/devel/qapi-code-gen.txt | 43 ++++++++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt index 3ce03e6..300f277 100644 --- a/docs/devel/qapi-code-gen.txt +++ b/docs/devel/qapi-code-gen.txt @@ -642,13 +642,8 @@ that previously resulted in an error). QMP clients may still need to know whether the extension is available. For this purpose, a list of features can be specified for a command or -struct type. This is exposed to the client as a list of strings, -where each string signals that this build of QEMU shows a certain -behaviour. - -Each member of the 'features' array defines a feature. It can either -be { 'name': STRING, '*if': COND }, or STRING, which is shorthand for -{ 'name': STRING }. +struct type. Each list member can either be { 'name': STRING, '*if': +COND }, or STRING, which is shorthand for { 'name': STRING }. The optional 'if' member specifies a conditional. See "Configuring the schema" below for more on this. @@ -659,6 +654,12 @@ Example: 'data': { 'number': 'int' }, 'features': [ 'allow-negative-numbers' ] } +The feature strings are exposed to clients in introspection, as +explained in section "Client JSON Protocol introspection". + +Intended use is to have each feature string signal that this build of +QEMU shows a certain behaviour. + === Naming rules and reserved names === @@ -965,7 +966,7 @@ schema, along with the SchemaInfo type. This text attempts to give an overview how things work. For details you need to consult the QAPI schema. -SchemaInfo objects have common members "name" and "meta-type", and +SchemaInfo objects have common members "name", "meta-type", and additional variant members depending on the value of meta-type. Each SchemaInfo object describes a wire ABI entity of a certain @@ -985,12 +986,13 @@ references by name. QAPI schema definitions not reachable that way are omitted. The SchemaInfo for a command has meta-type "command", and variant -members "arg-type", "ret-type" and "allow-oob". On the wire, the -"arguments" member of a client's "execute" command must conform to the -object type named by "arg-type". The "return" member that the server -passes in a success response conforms to the type named by "ret-type". -When "allow-oob" is true, it means the command supports out-of-band -execution. It defaults to false. +members "arg-type", "ret-type", "allow-oob", and "features". On the +wire, the "arguments" member of a client's "execute" command must +conform to the object type named by "arg-type". The "return" member +that the server passes in a success response conforms to the type +named by "ret-type". When "allow-oob" is true, it means the command +supports out-of-band execution. It defaults to false. "features" +exposes the command's feature strings as a JSON array of strings. If the command takes no arguments, "arg-type" names an object type without members. Likewise, if the command returns nothing, "ret-type" @@ -1025,7 +1027,8 @@ Example: the SchemaInfo for EVENT_C from section Events The SchemaInfo for struct and union types has meta-type "object". -The SchemaInfo for a struct type has variant member "members". +The SchemaInfo for a struct type has variant members "members" and +"features". The SchemaInfo for a union type additionally has variant members "tag" and "variants". @@ -1047,6 +1050,16 @@ Example: the SchemaInfo for MyType from section Struct types { "name": "member2", "type": "int" }, { "name": "member3", "type": "str", "default": null } ] } +"features" exposes the command's feature strings as a JSON array of +strings. + +Example: the SchemaInfo for TestType from section Features: + + { "name": "TestType", "meta-type": "object", + "members": [ + { "name": "number", "type": "int" } ], + "features": ["allow-negative-numbers"] } + "tag" is the name of the common member serving as type tag. "variants" is a JSON array describing the object's variant members. Each element is a JSON object with members "case" (the value of type -- cgit v1.1 From 3306459a78b210290583d47639ad37e6e0556bac Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:30 +0100 Subject: tests/test-qmp-cmds: Factor out qmp_dispatch() test helpers Checking the value of qmp_dispatch() is repetitive. Factor out helpers do_qmp_dispatch() and do_qmp_dispatch_error(). Without this, the next commit would make things even more repetitive. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200317115459.31821-6-armbru@redhat.com> --- tests/test-qmp-cmds.c | 70 +++++++++++++++++++++++++-------------------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c index 79507d9..fb18475 100644 --- a/tests/test-qmp-cmds.c +++ b/tests/test-qmp-cmds.c @@ -145,34 +145,55 @@ __org_qemu_x_Union1 *qmp___org_qemu_x_command(__org_qemu_x_EnumList *a, } +static QObject *do_qmp_dispatch(QDict *req, bool allow_oob) +{ + QDict *resp; + QObject *ret; + + resp = qmp_dispatch(&qmp_commands, QOBJECT(req), allow_oob); + g_assert(resp && !qdict_haskey(resp, "error")); + ret = qdict_get(resp, "return"); + g_assert(ret); + + qobject_ref(ret); + qobject_unref(resp); + return ret; +} + +static void do_qmp_dispatch_error(QDict *req, bool allow_oob, ErrorClass cls) +{ + QDict *resp; + + resp = qmp_dispatch(&qmp_commands, QOBJECT(req), allow_oob); + g_assert(resp && qdict_haskey(resp, "error")); + + qobject_unref(resp); +} + /* test commands with no input and no return value */ static void test_dispatch_cmd(void) { QDict *req = qdict_new(); - QDict *resp; + QObject *ret; qdict_put_str(req, "execute", "user_def_cmd"); - resp = qmp_dispatch(&qmp_commands, QOBJECT(req), false); - assert(resp != NULL); - assert(!qdict_haskey(resp, "error")); + ret = do_qmp_dispatch(req, false); - qobject_unref(resp); + qobject_unref(ret); qobject_unref(req); } static void test_dispatch_cmd_oob(void) { QDict *req = qdict_new(); - QDict *resp; + QObject *ret; qdict_put_str(req, "exec-oob", "test-flags-command"); - resp = qmp_dispatch(&qmp_commands, QOBJECT(req), true); - assert(resp != NULL); - assert(!qdict_haskey(resp, "error")); + ret = do_qmp_dispatch(req, true); - qobject_unref(resp); + qobject_unref(ret); qobject_unref(req); } @@ -181,15 +202,11 @@ static void test_dispatch_cmd_failure(void) { QDict *req = qdict_new(); QDict *args = qdict_new(); - QDict *resp; qdict_put_str(req, "execute", "user_def_cmd2"); - resp = qmp_dispatch(&qmp_commands, QOBJECT(req), false); - assert(resp != NULL); - assert(qdict_haskey(resp, "error")); + do_qmp_dispatch_error(req, false, ERROR_CLASS_GENERIC_ERROR); - qobject_unref(resp); qobject_unref(req); /* check that with extra arguments it throws an error */ @@ -199,11 +216,8 @@ static void test_dispatch_cmd_failure(void) qdict_put_str(req, "execute", "user_def_cmd"); - resp = qmp_dispatch(&qmp_commands, QOBJECT(req), false); - assert(resp != NULL); - assert(qdict_haskey(resp, "error")); + do_qmp_dispatch_error(req, false, ERROR_CLASS_GENERIC_ERROR); - qobject_unref(resp); qobject_unref(req); } @@ -218,20 +232,6 @@ static void test_dispatch_cmd_success_response(void) qobject_unref(req); } -static QObject *test_qmp_dispatch(QDict *req) -{ - QDict *resp; - QObject *ret; - - resp = qmp_dispatch(&qmp_commands, QOBJECT(req), false); - assert(resp && !qdict_haskey(resp, "error")); - ret = qdict_get(resp, "return"); - assert(ret); - qobject_ref(ret); - qobject_unref(resp); - return ret; -} - /* test commands that involve both input parameters and return values */ static void test_dispatch_cmd_io(void) { @@ -254,7 +254,7 @@ static void test_dispatch_cmd_io(void) qdict_put(req, "arguments", args); qdict_put_str(req, "execute", "user_def_cmd2"); - ret = qobject_to(QDict, test_qmp_dispatch(req)); + ret = qobject_to(QDict, do_qmp_dispatch(req, false)); assert(!strcmp(qdict_get_str(ret, "string0"), "blah1")); ret_dict = qdict_get_qdict(ret, "dict1"); @@ -275,7 +275,7 @@ static void test_dispatch_cmd_io(void) qdict_put(req, "arguments", args3); qdict_put_str(req, "execute", "guest-get-time"); - ret3 = qobject_to(QNum, test_qmp_dispatch(req)); + ret3 = qobject_to(QNum, do_qmp_dispatch(req, false)); g_assert(qnum_get_try_int(ret3, &val)); g_assert_cmpint(val, ==, 66); qobject_unref(ret3); -- cgit v1.1 From ef9f5f0d59471dbd5c0764fec67c982d95f4de20 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:31 +0100 Subject: tests/test-qmp-cmds: Check responses more thoroughly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Reviewed-by: Eric Blake Message-Id: <20200317115459.31821-7-armbru@redhat.com> --- tests/test-qmp-cmds.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c index fb18475..1563556 100644 --- a/tests/test-qmp-cmds.c +++ b/tests/test-qmp-cmds.c @@ -151,9 +151,10 @@ static QObject *do_qmp_dispatch(QDict *req, bool allow_oob) QObject *ret; resp = qmp_dispatch(&qmp_commands, QOBJECT(req), allow_oob); - g_assert(resp && !qdict_haskey(resp, "error")); + g_assert(resp); ret = qdict_get(resp, "return"); g_assert(ret); + g_assert(qdict_size(resp) == 1); qobject_ref(ret); qobject_unref(resp); @@ -163,9 +164,17 @@ static QObject *do_qmp_dispatch(QDict *req, bool allow_oob) static void do_qmp_dispatch_error(QDict *req, bool allow_oob, ErrorClass cls) { QDict *resp; + QDict *error; resp = qmp_dispatch(&qmp_commands, QOBJECT(req), allow_oob); - g_assert(resp && qdict_haskey(resp, "error")); + g_assert(resp); + error = qdict_get_qdict(resp, "error"); + g_assert(error); + g_assert_cmpstr(qdict_get_try_str(error, "class"), + ==, QapiErrorClass_str(cls)); + g_assert(qdict_get_try_str(error, "desc")); + g_assert(qdict_size(error) == 2); + g_assert(qdict_size(resp) == 1); qobject_unref(resp); } @@ -174,11 +183,12 @@ static void do_qmp_dispatch_error(QDict *req, bool allow_oob, ErrorClass cls) static void test_dispatch_cmd(void) { QDict *req = qdict_new(); - QObject *ret; + QDict *ret; qdict_put_str(req, "execute", "user_def_cmd"); - ret = do_qmp_dispatch(req, false); + ret = qobject_to(QDict, do_qmp_dispatch(req, false)); + assert(ret && qdict_size(ret) == 0); qobject_unref(ret); qobject_unref(req); @@ -187,11 +197,12 @@ static void test_dispatch_cmd(void) static void test_dispatch_cmd_oob(void) { QDict *req = qdict_new(); - QObject *ret; + QDict *ret; qdict_put_str(req, "exec-oob", "test-flags-command"); - ret = do_qmp_dispatch(req, true); + ret = qobject_to(QDict, do_qmp_dispatch(req, true)); + assert(ret && qdict_size(ret) == 0); qobject_unref(ret); qobject_unref(req); -- cgit v1.1 From 3d16042c9208c80b984b65cf90d1738a4c295bcc Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:32 +0100 Subject: tests/test-qmp-cmds: Simplify test data setup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Building requests with qdict_put() & friends is tedious to write and hard to read. Parse them from string literals with qdict_from_vjsonf_nofail() instead. Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Message-Id: <20200317115459.31821-8-armbru@redhat.com> --- tests/test-qmp-cmds.c | 95 +++++++++++++++++++++------------------------------ 1 file changed, 38 insertions(+), 57 deletions(-) diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c index 1563556..99013ff 100644 --- a/tests/test-qmp-cmds.c +++ b/tests/test-qmp-cmds.c @@ -1,5 +1,6 @@ #include "qemu/osdep.h" #include "qapi/qmp/qdict.h" +#include "qapi/qmp/qjson.h" #include "qapi/qmp/qnum.h" #include "qapi/qmp/qstring.h" #include "qapi/error.h" @@ -145,11 +146,16 @@ __org_qemu_x_Union1 *qmp___org_qemu_x_command(__org_qemu_x_EnumList *a, } -static QObject *do_qmp_dispatch(QDict *req, bool allow_oob) +static QObject *do_qmp_dispatch(bool allow_oob, const char *template, ...) { - QDict *resp; + va_list ap; + QDict *req, *resp; QObject *ret; + va_start(ap, template); + req = qdict_from_vjsonf_nofail(template, ap); + va_end(ap); + resp = qmp_dispatch(&qmp_commands, QOBJECT(req), allow_oob); g_assert(resp); ret = qdict_get(resp, "return"); @@ -158,14 +164,21 @@ static QObject *do_qmp_dispatch(QDict *req, bool allow_oob) qobject_ref(ret); qobject_unref(resp); + qobject_unref(req); return ret; } -static void do_qmp_dispatch_error(QDict *req, bool allow_oob, ErrorClass cls) +static void do_qmp_dispatch_error(bool allow_oob, ErrorClass cls, + const char *template, ...) { - QDict *resp; + va_list ap; + QDict *req, *resp; QDict *error; + va_start(ap, template); + req = qdict_from_vjsonf_nofail(template, ap); + va_end(ap); + resp = qmp_dispatch(&qmp_commands, QOBJECT(req), allow_oob); g_assert(resp); error = qdict_get_qdict(resp, "error"); @@ -177,59 +190,43 @@ static void do_qmp_dispatch_error(QDict *req, bool allow_oob, ErrorClass cls) g_assert(qdict_size(resp) == 1); qobject_unref(resp); + qobject_unref(req); } /* test commands with no input and no return value */ static void test_dispatch_cmd(void) { - QDict *req = qdict_new(); QDict *ret; - qdict_put_str(req, "execute", "user_def_cmd"); - - ret = qobject_to(QDict, do_qmp_dispatch(req, false)); + ret = qobject_to(QDict, + do_qmp_dispatch(false, + "{ 'execute': 'user_def_cmd' }")); assert(ret && qdict_size(ret) == 0); - qobject_unref(ret); - qobject_unref(req); } static void test_dispatch_cmd_oob(void) { - QDict *req = qdict_new(); QDict *ret; - qdict_put_str(req, "exec-oob", "test-flags-command"); - - ret = qobject_to(QDict, do_qmp_dispatch(req, true)); + ret = qobject_to(QDict, + do_qmp_dispatch(true, + "{ 'exec-oob': 'test-flags-command' }")); assert(ret && qdict_size(ret) == 0); - qobject_unref(ret); - qobject_unref(req); } /* test commands that return an error due to invalid parameters */ static void test_dispatch_cmd_failure(void) { - QDict *req = qdict_new(); - QDict *args = qdict_new(); - - qdict_put_str(req, "execute", "user_def_cmd2"); - - do_qmp_dispatch_error(req, false, ERROR_CLASS_GENERIC_ERROR); - - qobject_unref(req); - - /* check that with extra arguments it throws an error */ - req = qdict_new(); - qdict_put_int(args, "a", 66); - qdict_put(req, "arguments", args); - - qdict_put_str(req, "execute", "user_def_cmd"); - - do_qmp_dispatch_error(req, false, ERROR_CLASS_GENERIC_ERROR); - - qobject_unref(req); + /* missing arguments */ + do_qmp_dispatch_error(false, ERROR_CLASS_GENERIC_ERROR, + "{ 'execute': 'user_def_cmd2' }"); + + /* extra arguments */ + do_qmp_dispatch_error(false, ERROR_CLASS_GENERIC_ERROR, + "{ 'execute': 'user_def_cmd'," + " 'arguments': { 'a': 66 } }"); } static void test_dispatch_cmd_success_response(void) @@ -246,26 +243,15 @@ static void test_dispatch_cmd_success_response(void) /* test commands that involve both input parameters and return values */ static void test_dispatch_cmd_io(void) { - QDict *req = qdict_new(); - QDict *args = qdict_new(); - QDict *args3 = qdict_new(); - QDict *ud1a = qdict_new(); - QDict *ud1b = qdict_new(); QDict *ret, *ret_dict, *ret_dict_dict, *ret_dict_dict_userdef; QDict *ret_dict_dict2, *ret_dict_dict2_userdef; QNum *ret3; int64_t val; - qdict_put_int(ud1a, "integer", 42); - qdict_put_str(ud1a, "string", "hello"); - qdict_put_int(ud1b, "integer", 422); - qdict_put_str(ud1b, "string", "hello2"); - qdict_put(args, "ud1a", ud1a); - qdict_put(args, "ud1b", ud1b); - qdict_put(req, "arguments", args); - qdict_put_str(req, "execute", "user_def_cmd2"); - - ret = qobject_to(QDict, do_qmp_dispatch(req, false)); + ret = qobject_to(QDict, do_qmp_dispatch(false, + "{ 'execute': 'user_def_cmd2', 'arguments': {" + " 'ud1a': { 'integer': 42, 'string': 'hello' }," + " 'ud1b': { 'integer': 422, 'string': 'hello2' } } }")); assert(!strcmp(qdict_get_str(ret, "string0"), "blah1")); ret_dict = qdict_get_qdict(ret, "dict1"); @@ -282,16 +268,11 @@ static void test_dispatch_cmd_io(void) assert(!strcmp(qdict_get_str(ret_dict_dict2, "string"), "blah4")); qobject_unref(ret); - qdict_put_int(args3, "a", 66); - qdict_put(req, "arguments", args3); - qdict_put_str(req, "execute", "guest-get-time"); - - ret3 = qobject_to(QNum, do_qmp_dispatch(req, false)); + ret3 = qobject_to(QNum, do_qmp_dispatch(false, + "{ 'execute': 'guest-get-time', 'arguments': { 'a': 66 } }")); g_assert(qnum_get_try_int(ret3, &val)); g_assert_cmpint(val, ==, 66); qobject_unref(ret3); - - qobject_unref(req); } /* test generated dealloc functions for generated types */ -- cgit v1.1 From 3ecc3932cce98d12dfd0e5341b1b554aee977e66 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:33 +0100 Subject: tests/test-qmp-event: Simplify test data setup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Building expected data with qdict_put() & friends is tedious to write and hard to read. Parse them from string literals with qdict_from_jsonf_nofail() instead. While there, use initializers instead of assignments for initializing aggregate event arguments. Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Message-Id: <20200317115459.31821-9-armbru@redhat.com> --- tests/test-qmp-event.c | 95 +++++++++++++++----------------------------------- 1 file changed, 28 insertions(+), 67 deletions(-) diff --git a/tests/test-qmp-event.c b/tests/test-qmp-event.c index eee7e08..430001e 100644 --- a/tests/test-qmp-event.c +++ b/tests/test-qmp-event.c @@ -17,6 +17,7 @@ #include "qapi/error.h" #include "qapi/qmp/qbool.h" #include "qapi/qmp/qdict.h" +#include "qapi/qmp/qjson.h" #include "qapi/qmp/qnum.h" #include "qapi/qmp/qstring.h" #include "qapi/qmp-event.h" @@ -124,17 +125,13 @@ static void event_prepare(TestEventData *data, /* Global variable test_event_data was used to pass the expectation, so test cases can't be executed at same time. */ g_mutex_lock(&test_event_lock); - - data->expect = qdict_new(); test_event_data = data; } static void event_teardown(TestEventData *data, const void *unused) { - qobject_unref(data->expect); test_event_data = NULL; - g_mutex_unlock(&test_event_lock); } @@ -152,90 +149,54 @@ static void event_test_add(const char *testpath, static void test_event_a(TestEventData *data, const void *unused) { - QDict *d; - d = data->expect; - qdict_put_str(d, "event", "EVENT_A"); + data->expect = qdict_from_jsonf_nofail("{ 'event': 'EVENT_A' }"); qapi_event_send_event_a(); + qobject_unref(data->expect); } static void test_event_b(TestEventData *data, const void *unused) { - QDict *d; - d = data->expect; - qdict_put_str(d, "event", "EVENT_B"); + data->expect = qdict_from_jsonf_nofail("{ 'event': 'EVENT_B' }"); qapi_event_send_event_b(); + qobject_unref(data->expect); } static void test_event_c(TestEventData *data, const void *unused) { - QDict *d, *d_data, *d_b; - - UserDefOne b; - b.integer = 2; - b.string = g_strdup("test1"); - b.has_enum1 = false; - - d_b = qdict_new(); - qdict_put_int(d_b, "integer", 2); - qdict_put_str(d_b, "string", "test1"); - - d_data = qdict_new(); - qdict_put_int(d_data, "a", 1); - qdict_put(d_data, "b", d_b); - qdict_put_str(d_data, "c", "test2"); - - d = data->expect; - qdict_put_str(d, "event", "EVENT_C"); - qdict_put(d, "data", d_data); + UserDefOne b = { .integer = 2, .string = (char *)"test1" }; + data->expect = qdict_from_jsonf_nofail( + "{ 'event': 'EVENT_C', 'data': {" + " 'a': 1, 'b': { 'integer': 2, 'string': 'test1' }, 'c': 'test2' } }"); qapi_event_send_event_c(true, 1, true, &b, "test2"); - - g_free(b.string); + qobject_unref(data->expect); } /* Complex type */ static void test_event_d(TestEventData *data, const void *unused) { - UserDefOne struct1; - EventStructOne a; - QDict *d, *d_data, *d_a, *d_struct1; - - struct1.integer = 2; - struct1.string = g_strdup("test1"); - struct1.has_enum1 = true; - struct1.enum1 = ENUM_ONE_VALUE1; - - a.struct1 = &struct1; - a.string = g_strdup("test2"); - a.has_enum2 = true; - a.enum2 = ENUM_ONE_VALUE2; - - d_struct1 = qdict_new(); - qdict_put_int(d_struct1, "integer", 2); - qdict_put_str(d_struct1, "string", "test1"); - qdict_put_str(d_struct1, "enum1", "value1"); - - d_a = qdict_new(); - qdict_put(d_a, "struct1", d_struct1); - qdict_put_str(d_a, "string", "test2"); - qdict_put_str(d_a, "enum2", "value2"); - - d_data = qdict_new(); - qdict_put(d_data, "a", d_a); - qdict_put_str(d_data, "b", "test3"); - qdict_put_str(d_data, "enum3", "value3"); - - d = data->expect; - qdict_put_str(d, "event", "EVENT_D"); - qdict_put(d, "data", d_data); - + UserDefOne struct1 = { + .integer = 2, .string = (char *)"test1", + .has_enum1 = true, .enum1 = ENUM_ONE_VALUE1, + }; + EventStructOne a = { + .struct1 = &struct1, + .string = (char *)"test2", + .has_enum2 = true, + .enum2 = ENUM_ONE_VALUE2, + }; + + data->expect = qdict_from_jsonf_nofail( + "{ 'event': 'EVENT_D', 'data': {" + " 'a': {" + " 'struct1': { 'integer': 2, 'string': 'test1', 'enum1': 'value1' }," + " 'string': 'test2', 'enum2': 'value2' }," + " 'b': 'test3', 'enum3': 'value3' } }"); qapi_event_send_event_d(&a, "test3", false, NULL, true, ENUM_ONE_VALUE3); - - g_free(struct1.string); - g_free(a.string); + qobject_unref(data->expect); } int main(int argc, char **argv) -- cgit v1.1 From 052be50cf4621583bb2c33bdcd0f89b4ae873382 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:34 +0100 Subject: tests/test-qmp-event: Use qobject_is_equal() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Locally defined helper qdict_cmp_simple() implements just enough of a comparison to serve here. Replace it by qobject_is_equal(), which implements all of it. Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Message-Id: <20200317115459.31821-10-armbru@redhat.com> --- tests/test-qmp-event.c | 66 +------------------------------------------------- 1 file changed, 1 insertion(+), 65 deletions(-) diff --git a/tests/test-qmp-event.c b/tests/test-qmp-event.c index 430001e..d640661 100644 --- a/tests/test-qmp-event.c +++ b/tests/test-qmp-event.c @@ -28,73 +28,9 @@ typedef struct TestEventData { QDict *expect; } TestEventData; -typedef struct QDictCmpData { - QDict *expect; - bool result; -} QDictCmpData; - TestEventData *test_event_data; static GMutex test_event_lock; -/* Only compares bool, int, string */ -static -void qdict_cmp_do_simple(const char *key, QObject *obj1, void *opaque) - -{ - QObject *obj2; - QDictCmpData d_new, *d = opaque; - int64_t val1, val2; - - if (!d->result) { - return; - } - - obj2 = qdict_get(d->expect, key); - if (!obj2) { - d->result = false; - return; - } - - if (qobject_type(obj1) != qobject_type(obj2)) { - d->result = false; - return; - } - - switch (qobject_type(obj1)) { - case QTYPE_QBOOL: - d->result = (qbool_get_bool(qobject_to(QBool, obj1)) == - qbool_get_bool(qobject_to(QBool, obj2))); - return; - case QTYPE_QNUM: - g_assert(qnum_get_try_int(qobject_to(QNum, obj1), &val1)); - g_assert(qnum_get_try_int(qobject_to(QNum, obj2), &val2)); - d->result = val1 == val2; - return; - case QTYPE_QSTRING: - d->result = g_strcmp0(qstring_get_str(qobject_to(QString, obj1)), - qstring_get_str(qobject_to(QString, obj2))) == 0; - return; - case QTYPE_QDICT: - d_new.expect = qobject_to(QDict, obj2); - d_new.result = true; - qdict_iter(qobject_to(QDict, obj1), qdict_cmp_do_simple, &d_new); - d->result = d_new.result; - return; - default: - abort(); - } -} - -static bool qdict_cmp_simple(QDict *a, QDict *b) -{ - QDictCmpData d; - - d.expect = b; - d.result = true; - qdict_iter(a, qdict_cmp_do_simple, &d); - return d.result; -} - void test_qapi_event_emit(test_QAPIEvent event, QDict *d) { QDict *t; @@ -115,7 +51,7 @@ void test_qapi_event_emit(test_QAPIEvent event, QDict *d) qdict_del(d, "timestamp"); - g_assert(qdict_cmp_simple(d, test_event_data->expect)); + g_assert(qobject_is_equal(QOBJECT(d), QOBJECT(test_event_data->expect))); } -- cgit v1.1 From 11deae8cd2ce1e1f4240ec7d880940f25ebd65b1 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:35 +0100 Subject: tests/test-qmp-event: Check event is actually emitted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Message-Id: <20200317115459.31821-11-armbru@redhat.com> --- tests/test-qmp-event.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/test-qmp-event.c b/tests/test-qmp-event.c index d640661..7dd0053 100644 --- a/tests/test-qmp-event.c +++ b/tests/test-qmp-event.c @@ -26,6 +26,7 @@ typedef struct TestEventData { QDict *expect; + bool emitted; } TestEventData; TestEventData *test_event_data; @@ -52,7 +53,7 @@ void test_qapi_event_emit(test_QAPIEvent event, QDict *d) qdict_del(d, "timestamp"); g_assert(qobject_is_equal(QOBJECT(d), QOBJECT(test_event_data->expect))); - + test_event_data->emitted = true; } static void event_prepare(TestEventData *data, @@ -87,6 +88,7 @@ static void test_event_a(TestEventData *data, { data->expect = qdict_from_jsonf_nofail("{ 'event': 'EVENT_A' }"); qapi_event_send_event_a(); + g_assert(data->emitted); qobject_unref(data->expect); } @@ -95,6 +97,7 @@ static void test_event_b(TestEventData *data, { data->expect = qdict_from_jsonf_nofail("{ 'event': 'EVENT_B' }"); qapi_event_send_event_b(); + g_assert(data->emitted); qobject_unref(data->expect); } @@ -107,6 +110,7 @@ static void test_event_c(TestEventData *data, "{ 'event': 'EVENT_C', 'data': {" " 'a': 1, 'b': { 'integer': 2, 'string': 'test1' }, 'c': 'test2' } }"); qapi_event_send_event_c(true, 1, true, &b, "test2"); + g_assert(data->emitted); qobject_unref(data->expect); } @@ -132,6 +136,7 @@ static void test_event_d(TestEventData *data, " 'string': 'test2', 'enum2': 'value2' }," " 'b': 'test3', 'enum3': 'value3' } }"); qapi_event_send_event_d(&a, "test3", false, NULL, true, ENUM_ONE_VALUE3); + g_assert(data->emitted); qobject_unref(data->expect); } -- cgit v1.1 From e4405b30695cda6fad69a4411c05b73d538c7992 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:36 +0100 Subject: qapi/schema: Clean up around QAPISchemaEntity.connect_doc() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QAPISchemaEntity calls doc.connect_feature() in .check(). Improper since commit ee1e6a1f6c8 split .connect_doc() off .check(). Move the call. Requires making the children call super().connect_doc() as they should. Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Reviewed-by: Eric Blake Message-Id: <20200317115459.31821-12-armbru@redhat.com> --- scripts/qapi/schema.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index d759308..2a2b495 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -53,13 +53,13 @@ class QAPISchemaEntity: seen = {} for f in self.features: f.check_clash(self.info, seen) - if self.doc: - self.doc.connect_feature(f) - self._checked = True def connect_doc(self, doc=None): - pass + doc = doc or self.doc + if doc: + for f in self.features: + doc.connect_feature(f) def check_doc(self): if self.doc: @@ -250,6 +250,7 @@ class QAPISchemaEnumType(QAPISchemaType): m.check_clash(self.info, seen) def connect_doc(self, doc=None): + super().connect_doc(doc) doc = doc or self.doc if doc: for m in self.members: @@ -392,6 +393,7 @@ class QAPISchemaObjectType(QAPISchemaType): m.check_clash(info, seen) def connect_doc(self, doc=None): + super().connect_doc(doc) doc = doc or self.doc if doc: if self.base and self.base.is_implicit(): @@ -667,6 +669,7 @@ class QAPISchemaAlternateType(QAPISchemaType): types_seen[qt] = v.name def connect_doc(self, doc=None): + super().connect_doc(doc) doc = doc or self.doc if doc: for v in self.variants.variants: @@ -733,6 +736,7 @@ class QAPISchemaCommand(QAPISchemaEntity): % self.ret_type.describe()) def connect_doc(self, doc=None): + super().connect_doc(doc) doc = doc or self.doc if doc: if self.arg_type and self.arg_type.is_implicit(): @@ -775,6 +779,7 @@ class QAPISchemaEvent(QAPISchemaEntity): % self.arg_type.describe()) def connect_doc(self, doc=None): + super().connect_doc(doc) doc = doc or self.doc if doc: if self.arg_type and self.arg_type.is_implicit(): -- cgit v1.1 From 013b4efc9be9af8276bd891cd52267d409f1d712 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:37 +0100 Subject: qapi: Add feature flags to remaining definitions In v4.1.0, we added feature flags just to struct types (commit 6a8c0b5102^..f3ed93d545), to satisfy an immediate need (commit c9d4070991 "file-posix: Add dynamic-auto-read-only QAPI feature"). In v4.2.0, we added them to commands (commit 23394b4c39 "qapi: Add feature flags to commands") to satisfy another immediate need (commit d76744e65e "qapi: Allow introspecting fix for savevm's cooperation with blockdev"). Add them to the remaining definitions: enumeration types, union types, alternate types, and events. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200317115459.31821-13-armbru@redhat.com> --- docs/devel/qapi-code-gen.txt | 54 +++++++++++++------ qapi/introspect.json | 20 ++++--- scripts/qapi/doc.py | 6 +-- scripts/qapi/events.py | 2 +- scripts/qapi/expr.py | 11 ++-- scripts/qapi/introspect.py | 31 +++++------ scripts/qapi/schema.py | 96 ++++++++++++++++++--------------- scripts/qapi/types.py | 4 +- scripts/qapi/visit.py | 4 +- tests/qapi-schema/alternate-base.err | 2 +- tests/qapi-schema/doc-good.json | 17 ++++++ tests/qapi-schema/doc-good.out | 15 ++++++ tests/qapi-schema/doc-good.texi | 30 +++++++++++ tests/qapi-schema/qapi-schema-test.json | 29 +++++++--- tests/qapi-schema/qapi-schema-test.out | 27 ++++++++-- tests/qapi-schema/test-qapi.py | 9 ++-- tests/test-qmp-cmds.c | 6 +-- 17 files changed, 242 insertions(+), 121 deletions(-) diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt index 300f277..43f31b4 100644 --- a/docs/devel/qapi-code-gen.txt +++ b/docs/devel/qapi-code-gen.txt @@ -172,7 +172,8 @@ Syntax: ENUM = { 'enum': STRING, 'data': [ ENUM-VALUE, ... ], '*prefix': STRING, - '*if': COND } + '*if': COND, + '*features': FEATURES } ENUM-VALUE = STRING | { 'name': STRING, '*if': COND } @@ -207,6 +208,9 @@ the job satisfactorily. The optional 'if' member specifies a conditional. See "Configuring the schema" below for more on this. +The optional 'features' member specifies features. See "Features" +below for more on this. + === Type references and array types === @@ -279,12 +283,14 @@ below for more on this. Syntax: UNION = { 'union': STRING, 'data': BRANCHES, - '*if': COND } + '*if': COND, + '*features': FEATURES } | { 'union': STRING, 'data': BRANCHES, 'base': ( MEMBERS | STRING ), 'discriminator': STRING, - '*if': COND } + '*if': COND, + '*features': FEATURES } BRANCHES = { BRANCH, ... } BRANCH = STRING : TYPE-REF | STRING : { 'type': TYPE-REF, '*if': COND } @@ -391,13 +397,17 @@ is identical on the wire to: The optional 'if' member specifies a conditional. See "Configuring the schema" below for more on this. +The optional 'features' member specifies features. See "Features" +below for more on this. + === Alternate types === Syntax: ALTERNATE = { 'alternate': STRING, 'data': ALTERNATIVES, - '*if': COND } + '*if': COND, + '*features': FEATURES } ALTERNATIVES = { ALTERNATIVE, ... } ALTERNATIVE = STRING : STRING | STRING : { 'type': STRING, '*if': COND } @@ -441,6 +451,9 @@ following example objects: The optional 'if' member specifies a conditional. See "Configuring the schema" below for more on this. +The optional 'features' member specifies features. See "Features" +below for more on this. + === Commands === @@ -584,6 +597,9 @@ started with --preconfig. The optional 'if' member specifies a conditional. See "Configuring the schema" below for more on this. +The optional 'features' member specifies features. See "Features" +below for more on this. + === Events === @@ -595,7 +611,8 @@ Syntax: 'data': STRING, 'boxed': true, ) - '*if': COND } + '*if': COND, + '*features': FEATURES } Member 'event' names the event. This is the event name used in the Client JSON Protocol. @@ -628,6 +645,9 @@ complex type. See section "Code generated for events" for examples. The optional 'if' member specifies a conditional. See "Configuring the schema" below for more on this. +The optional 'features' member specifies features. See "Features" +below for more on this. + === Features === @@ -966,8 +986,9 @@ schema, along with the SchemaInfo type. This text attempts to give an overview how things work. For details you need to consult the QAPI schema. -SchemaInfo objects have common members "name", "meta-type", and -additional variant members depending on the value of meta-type. +SchemaInfo objects have common members "name", "meta-type", +"features", and additional variant members depending on the value of +meta-type. Each SchemaInfo object describes a wire ABI entity of a certain meta-type: a command, event or one of several kinds of type. @@ -980,19 +1001,21 @@ not. Therefore, the SchemaInfo for types have auto-generated meaningless names. For readability, the examples in this section use meaningful type names instead. +Optional member "features" exposes the entity's feature strings as a +JSON array of strings. + To examine a type, start with a command or event using it, then follow references by name. QAPI schema definitions not reachable that way are omitted. The SchemaInfo for a command has meta-type "command", and variant -members "arg-type", "ret-type", "allow-oob", and "features". On the -wire, the "arguments" member of a client's "execute" command must -conform to the object type named by "arg-type". The "return" member -that the server passes in a success response conforms to the type -named by "ret-type". When "allow-oob" is true, it means the command -supports out-of-band execution. It defaults to false. "features" -exposes the command's feature strings as a JSON array of strings. +members "arg-type", "ret-type" and "allow-oob". On the wire, the +"arguments" member of a client's "execute" command must conform to the +object type named by "arg-type". The "return" member that the server +passes in a success response conforms to the type named by "ret-type". +When "allow-oob" is true, it means the command supports out-of-band +execution. It defaults to false. If the command takes no arguments, "arg-type" names an object type without members. Likewise, if the command returns nothing, "ret-type" @@ -1027,8 +1050,7 @@ Example: the SchemaInfo for EVENT_C from section Events The SchemaInfo for struct and union types has meta-type "object". -The SchemaInfo for a struct type has variant members "members" and -"features". +The SchemaInfo for a struct type has variant member "members". The SchemaInfo for a union type additionally has variant members "tag" and "variants". diff --git a/qapi/introspect.json b/qapi/introspect.json index 8756e79..da3e176 100644 --- a/qapi/introspect.json +++ b/qapi/introspect.json @@ -89,12 +89,18 @@ # # @meta-type: the entity's meta type, inherited from @base. # +# @features: names of features associated with the entity, in no +# particular order. +# (since 4.1 for object types, 4.2 for commands, 5.0 for +# the rest) +# # Additional members depend on the value of @meta-type. # # Since: 2.5 ## { 'union': 'SchemaInfo', - 'base': { 'name': 'str', 'meta-type': 'SchemaMetaType' }, + 'base': { 'name': 'str', 'meta-type': 'SchemaMetaType', + '*features': [ 'str' ] }, 'discriminator': 'meta-type', 'data': { 'builtin': 'SchemaInfoBuiltin', @@ -174,9 +180,6 @@ # and may even differ from the order of the values of the # enum type of the @tag. # -# @features: names of features associated with the type, in no particular -# order. (since: 4.1) -# # Values of this type are JSON object on the wire. # # Since: 2.5 @@ -184,8 +187,7 @@ { 'struct': 'SchemaInfoObject', 'data': { 'members': [ 'SchemaInfoObjectMember' ], '*tag': 'str', - '*variants': [ 'SchemaInfoObjectVariant' ], - '*features': [ 'str' ] } } + '*variants': [ 'SchemaInfoObjectVariant' ] } } ## # @SchemaInfoObjectMember: @@ -266,17 +268,13 @@ # @allow-oob: whether the command allows out-of-band execution, # defaults to false (Since: 2.12) # -# @features: names of features associated with the command, in no particular -# order. (since 4.2) -# # TODO: @success-response (currently irrelevant, because it's QGA, not QMP) # # Since: 2.5 ## { 'struct': 'SchemaInfoCommand', 'data': { 'arg-type': 'str', 'ret-type': 'str', - '*allow-oob': 'bool', - '*features': [ 'str' ] } } + '*allow-oob': 'bool' } } ## # @SchemaInfoEvent: diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py index 1787a53..36e8233 100644 --- a/scripts/qapi/doc.py +++ b/scripts/qapi/doc.py @@ -243,7 +243,7 @@ class QAPISchemaGenDocVisitor(QAPISchemaVisitor): def write(self, output_dir): self._gen.write(output_dir) - def visit_enum_type(self, name, info, ifcond, members, prefix): + def visit_enum_type(self, name, info, ifcond, features, members, prefix): doc = self.cur_doc self._gen.add(texi_type('Enum', doc, ifcond, texi_members(doc, 'Values', @@ -257,7 +257,7 @@ class QAPISchemaGenDocVisitor(QAPISchemaVisitor): self._gen.add(texi_type('Object', doc, ifcond, texi_members(doc, 'Members', base, variants))) - def visit_alternate_type(self, name, info, ifcond, variants): + def visit_alternate_type(self, name, info, ifcond, features, variants): doc = self.cur_doc self._gen.add(texi_type('Alternate', doc, ifcond, texi_members(doc, 'Members'))) @@ -270,7 +270,7 @@ class QAPISchemaGenDocVisitor(QAPISchemaVisitor): texi_arguments(doc, arg_type if boxed else None))) - def visit_event(self, name, info, ifcond, arg_type, boxed): + def visit_event(self, name, info, ifcond, features, arg_type, boxed): doc = self.cur_doc self._gen.add(texi_msg('Event', doc, ifcond, texi_arguments(doc, diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py index a98b9f5..b544af5 100644 --- a/scripts/qapi/events.py +++ b/scripts/qapi/events.py @@ -189,7 +189,7 @@ void %(event_emit)s(%(event_enum)s event, QDict *qdict); event_emit=self._event_emit_name, event_enum=self._event_enum_name)) - def visit_event(self, name, info, ifcond, arg_type, boxed): + def visit_event(self, name, info, ifcond, features, arg_type, boxed): with ifcontext(ifcond, self._genh, self._genc): self._genh.add(gen_event_send_decl(name, arg_type, boxed)) self._genc.add(gen_event_send(name, arg_type, boxed, diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index fecf466..f9c4448 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -219,7 +219,6 @@ def check_struct(expr, info): check_type(members, info, "'data'", allow_dict=name) check_type(expr.get('base'), info, "'base'") - check_features(expr.get('features'), info) def check_union(expr, info): @@ -267,7 +266,6 @@ def check_command(expr, info): raise QAPISemError(info, "'boxed': true requires 'data'") check_type(args, info, "'data'", allow_dict=not boxed) check_type(rets, info, "'returns'", allow_array=True) - check_features(expr.get('features'), info) def check_event(expr, info): @@ -319,18 +317,18 @@ def check_exprs(exprs): if meta == 'enum': check_keys(expr, info, meta, - ['enum', 'data'], ['if', 'prefix']) + ['enum', 'data'], ['if', 'features', 'prefix']) check_enum(expr, info) elif meta == 'union': check_keys(expr, info, meta, ['union', 'data'], - ['base', 'discriminator', 'if']) + ['base', 'discriminator', 'if', 'features']) normalize_members(expr.get('base')) normalize_members(expr['data']) check_union(expr, info) elif meta == 'alternate': check_keys(expr, info, meta, - ['alternate', 'data'], ['if']) + ['alternate', 'data'], ['if', 'features']) normalize_members(expr['data']) check_alternate(expr, info) elif meta == 'struct': @@ -348,13 +346,14 @@ def check_exprs(exprs): check_command(expr, info) elif meta == 'event': check_keys(expr, info, meta, - ['event'], ['data', 'boxed', 'if']) + ['event'], ['data', 'boxed', 'if', 'features']) normalize_members(expr.get('data')) check_event(expr, info) else: assert False, 'unexpected meta type' check_if(expr, info, meta) + check_features(expr.get('features'), info) check_flags(expr, info) return exprs diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index b5537ed..2e9e00a 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -144,7 +144,7 @@ const QLitObject %(c_name)s = %(c_string)s; return '[' + self._use_type(typ.element_type) + ']' return self._name(typ.name) - def _gen_qlit(self, name, mtype, obj, ifcond): + def _gen_qlit(self, name, mtype, obj, ifcond, features): extra = {} if mtype not in ('command', 'event', 'builtin', 'array'): if not self._unmask: @@ -154,6 +154,8 @@ const QLitObject %(c_name)s = %(c_string)s; name = self._name(name) obj['name'] = name obj['meta-type'] = mtype + if features: + obj['features'] = [(f.name, {'if': f.ifcond}) for f in features] if ifcond: extra['if'] = ifcond if extra: @@ -178,18 +180,18 @@ const QLitObject %(c_name)s = %(c_string)s; {'if': variant.ifcond}) def visit_builtin_type(self, name, info, json_type): - self._gen_qlit(name, 'builtin', {'json-type': json_type}, []) + self._gen_qlit(name, 'builtin', {'json-type': json_type}, [], None) - def visit_enum_type(self, name, info, ifcond, members, prefix): + def visit_enum_type(self, name, info, ifcond, features, members, prefix): self._gen_qlit(name, 'enum', {'values': [(m.name, {'if': m.ifcond}) for m in members]}, - ifcond) + ifcond, features) def visit_array_type(self, name, info, ifcond, element_type): element = self._use_type(element_type) self._gen_qlit('[' + element + ']', 'array', {'element-type': element}, - ifcond) + ifcond, None) def visit_object_type_flat(self, name, info, ifcond, members, variants, features): @@ -197,16 +199,15 @@ const QLitObject %(c_name)s = %(c_string)s; if variants: obj.update(self._gen_variants(variants.tag_member.name, variants.variants)) - if features: - obj['features'] = [(f.name, {'if': f.ifcond}) for f in features] - self._gen_qlit(name, 'object', obj, ifcond) + self._gen_qlit(name, 'object', obj, ifcond, features) - def visit_alternate_type(self, name, info, ifcond, variants): + def visit_alternate_type(self, name, info, ifcond, features, variants): self._gen_qlit(name, 'alternate', {'members': [ ({'type': self._use_type(m.type)}, {'if': m.ifcond}) - for m in variants.variants]}, ifcond) + for m in variants.variants]}, + ifcond, features) def visit_command(self, name, info, ifcond, arg_type, ret_type, gen, success_response, boxed, allow_oob, allow_preconfig, @@ -217,16 +218,12 @@ const QLitObject %(c_name)s = %(c_string)s; 'ret-type': self._use_type(ret_type)} if allow_oob: obj['allow-oob'] = allow_oob + self._gen_qlit(name, 'command', obj, ifcond, features) - if features: - obj['features'] = [(f.name, {'if': f.ifcond}) for f in features] - - self._gen_qlit(name, 'command', obj, ifcond) - - def visit_event(self, name, info, ifcond, arg_type, boxed): + def visit_event(self, name, info, ifcond, features, arg_type, boxed): arg_type = arg_type or self._schema.the_empty_object_type self._gen_qlit(name, 'event', {'arg-type': self._use_type(arg_type)}, - ifcond) + ifcond, features) def gen_introspect(schema, output_dir, prefix, opt_unmask): diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 2a2b495..2223800 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -109,7 +109,7 @@ class QAPISchemaVisitor: def visit_builtin_type(self, name, info, json_type): pass - def visit_enum_type(self, name, info, ifcond, members, prefix): + def visit_enum_type(self, name, info, ifcond, features, members, prefix): pass def visit_array_type(self, name, info, ifcond, element_type): @@ -123,7 +123,7 @@ class QAPISchemaVisitor: features): pass - def visit_alternate_type(self, name, info, ifcond, variants): + def visit_alternate_type(self, name, info, ifcond, features, variants): pass def visit_command(self, name, info, ifcond, arg_type, ret_type, gen, @@ -131,7 +131,7 @@ class QAPISchemaVisitor: features): pass - def visit_event(self, name, info, ifcond, arg_type, boxed): + def visit_event(self, name, info, ifcond, features, arg_type, boxed): pass @@ -234,8 +234,8 @@ class QAPISchemaBuiltinType(QAPISchemaType): class QAPISchemaEnumType(QAPISchemaType): meta = 'enum' - def __init__(self, name, info, doc, ifcond, members, prefix): - super().__init__(name, info, doc, ifcond) + def __init__(self, name, info, doc, ifcond, features, members, prefix): + super().__init__(name, info, doc, ifcond, features) for m in members: assert isinstance(m, QAPISchemaEnumMember) m.set_defined_in(name) @@ -271,15 +271,16 @@ class QAPISchemaEnumType(QAPISchemaType): def visit(self, visitor): super().visit(visitor) - visitor.visit_enum_type(self.name, self.info, self.ifcond, - self.members, self.prefix) + visitor.visit_enum_type( + self.name, self.info, self.ifcond, self.features, + self.members, self.prefix) class QAPISchemaArrayType(QAPISchemaType): meta = 'array' def __init__(self, name, info, element_type): - super().__init__(name, info, None, None) + super().__init__(name, info, None) assert isinstance(element_type, str) self._element_type_name = element_type self.element_type = None @@ -325,8 +326,8 @@ class QAPISchemaArrayType(QAPISchemaType): class QAPISchemaObjectType(QAPISchemaType): - def __init__(self, name, info, doc, ifcond, - base, local_members, variants, features): + def __init__(self, name, info, doc, ifcond, features, + base, local_members, variants): # struct has local_members, optional base, and no variants # flat union has base, variants, and no local_members # simple union has local_members, variants, and no base @@ -622,8 +623,8 @@ class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): class QAPISchemaAlternateType(QAPISchemaType): meta = 'alternate' - def __init__(self, name, info, doc, ifcond, variants): - super().__init__(name, info, doc, ifcond) + def __init__(self, name, info, doc, ifcond, features, variants): + super().__init__(name, info, doc, ifcond, features) assert isinstance(variants, QAPISchemaObjectTypeVariants) assert variants.tag_member variants.set_defined_in(name) @@ -683,16 +684,16 @@ class QAPISchemaAlternateType(QAPISchemaType): def visit(self, visitor): super().visit(visitor) - visitor.visit_alternate_type(self.name, self.info, self.ifcond, - self.variants) + visitor.visit_alternate_type( + self.name, self.info, self.ifcond, self.features, self.variants) class QAPISchemaCommand(QAPISchemaEntity): meta = 'command' - def __init__(self, name, info, doc, ifcond, arg_type, ret_type, - gen, success_response, boxed, allow_oob, allow_preconfig, - features): + def __init__(self, name, info, doc, ifcond, features, + arg_type, ret_type, + gen, success_response, boxed, allow_oob, allow_preconfig): 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) @@ -755,8 +756,8 @@ class QAPISchemaCommand(QAPISchemaEntity): class QAPISchemaEvent(QAPISchemaEntity): meta = 'event' - def __init__(self, name, info, doc, ifcond, arg_type, boxed): - super().__init__(name, info, doc, ifcond) + def __init__(self, name, info, doc, ifcond, features, arg_type, boxed): + super().__init__(name, info, doc, ifcond, features) assert not arg_type or isinstance(arg_type, str) self._arg_type_name = arg_type self.arg_type = None @@ -787,8 +788,9 @@ class QAPISchemaEvent(QAPISchemaEntity): def visit(self, visitor): super().visit(visitor) - visitor.visit_event(self.name, self.info, self.ifcond, - self.arg_type, self.boxed) + visitor.visit_event( + self.name, self.info, self.ifcond, self.features, + self.arg_type, self.boxed) class QAPISchema: @@ -893,7 +895,7 @@ class QAPISchema: ('null', 'null', 'QNull' + pointer_suffix)]: self._def_builtin_type(*t) self.the_empty_object_type = QAPISchemaObjectType( - 'q_empty', None, None, None, None, [], None, []) + 'q_empty', None, None, None, None, None, [], None) self._def_entity(self.the_empty_object_type) qtypes = ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', @@ -901,10 +903,11 @@ class QAPISchema: qtype_values = self._make_enum_members( [{'name': n} for n in qtypes], None) - self._def_entity(QAPISchemaEnumType('QType', None, None, None, + self._def_entity(QAPISchemaEnumType('QType', None, None, None, None, qtype_values, 'QTYPE')) - def _make_features(self, features, info): + def _make_features(self, expr, info): + features = expr.get('features', []) return [QAPISchemaFeature(f['name'], info, f.get('if')) for f in features] @@ -916,7 +919,8 @@ class QAPISchema: # See also QAPISchemaObjectTypeMember.describe() name = name + 'Kind' # reserved by check_defn_name_str() self._def_entity(QAPISchemaEnumType( - name, info, None, ifcond, self._make_enum_members(values, info), + name, info, None, ifcond, None, + self._make_enum_members(values, info), None)) return name @@ -944,8 +948,8 @@ class QAPISchema: # TODO kill simple unions or implement the disjunction assert (ifcond or []) == typ._ifcond # pylint: disable=protected-access else: - self._def_entity(QAPISchemaObjectType(name, info, None, ifcond, - None, members, None, [])) + self._def_entity(QAPISchemaObjectType( + name, info, None, ifcond, None, None, members, None)) return name def _def_enum_type(self, expr, info, doc): @@ -953,8 +957,9 @@ class QAPISchema: data = expr['data'] prefix = expr.get('prefix') ifcond = expr.get('if') + features = self._make_features(expr, info) self._def_entity(QAPISchemaEnumType( - name, info, doc, ifcond, + name, info, doc, ifcond, features, self._make_enum_members(data, info), prefix)) def _make_member(self, name, typ, ifcond, info): @@ -976,12 +981,11 @@ class QAPISchema: base = expr.get('base') data = expr['data'] ifcond = expr.get('if') - features = expr.get('features', []) + features = self._make_features(expr, info) self._def_entity(QAPISchemaObjectType( - name, info, doc, ifcond, base, + name, info, doc, ifcond, features, base, self._make_members(data, info), - None, - self._make_features(features, info))) + None)) def _make_variant(self, case, typ, ifcond, info): return QAPISchemaObjectTypeVariant(case, info, typ, ifcond) @@ -1000,6 +1004,7 @@ class QAPISchema: data = expr['data'] base = expr.get('base') ifcond = expr.get('if') + features = self._make_features(expr, info) tag_name = expr.get('discriminator') tag_member = None if isinstance(base, dict): @@ -1020,21 +1025,22 @@ class QAPISchema: tag_member = QAPISchemaObjectTypeMember('type', info, typ, False) members = [tag_member] self._def_entity( - QAPISchemaObjectType(name, info, doc, ifcond, base, members, + QAPISchemaObjectType(name, info, doc, ifcond, features, + base, members, QAPISchemaObjectTypeVariants( - tag_name, info, tag_member, variants), - [])) + tag_name, info, tag_member, variants))) def _def_alternate_type(self, expr, info, doc): name = expr['alternate'] data = expr['data'] ifcond = expr.get('if') + features = self._make_features(expr, info) variants = [self._make_variant(key, value['type'], value.get('if'), info) for (key, value) in data.items()] tag_member = QAPISchemaObjectTypeMember('type', info, 'QType', False) self._def_entity( - QAPISchemaAlternateType(name, info, doc, ifcond, + QAPISchemaAlternateType(name, info, doc, ifcond, features, QAPISchemaObjectTypeVariants( None, info, tag_member, variants))) @@ -1048,27 +1054,31 @@ class QAPISchema: allow_oob = expr.get('allow-oob', False) allow_preconfig = expr.get('allow-preconfig', False) ifcond = expr.get('if') - features = expr.get('features', []) + features = self._make_features(expr, info) if isinstance(data, OrderedDict): data = self._make_implicit_object_type( - name, info, ifcond, 'arg', self._make_members(data, info)) + name, info, ifcond, + 'arg', self._make_members(data, info)) if isinstance(rets, list): assert len(rets) == 1 rets = self._make_array_type(rets[0], info) - self._def_entity(QAPISchemaCommand(name, info, doc, ifcond, data, rets, + self._def_entity(QAPISchemaCommand(name, info, doc, ifcond, features, + data, rets, gen, success_response, - boxed, allow_oob, allow_preconfig, - self._make_features(features, info))) + boxed, allow_oob, allow_preconfig)) def _def_event(self, expr, info, doc): name = expr['event'] data = expr.get('data') boxed = expr.get('boxed', False) ifcond = expr.get('if') + features = self._make_features(expr, info) if isinstance(data, OrderedDict): data = self._make_implicit_object_type( - name, info, ifcond, 'arg', self._make_members(data, info)) - self._def_entity(QAPISchemaEvent(name, info, doc, ifcond, data, boxed)) + name, info, ifcond, + 'arg', self._make_members(data, info)) + self._def_entity(QAPISchemaEvent(name, info, doc, ifcond, features, + data, boxed)) def _def_exprs(self, exprs): for expr_elem in exprs: diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py index 3c83b6e..d0d5c03 100644 --- a/scripts/qapi/types.py +++ b/scripts/qapi/types.py @@ -278,7 +278,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor): self._genh.add(gen_type_cleanup_decl(name)) self._genc.add(gen_type_cleanup(name)) - def visit_enum_type(self, name, info, ifcond, members, prefix): + def visit_enum_type(self, name, info, ifcond, features, members, prefix): with ifcontext(ifcond, self._genh, self._genc): self._genh.preamble_add(gen_enum(name, members, prefix)) self._genc.add(gen_enum_lookup(name, members, prefix)) @@ -306,7 +306,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor): # implicit types won't be directly allocated/freed self._gen_type_cleanup(name) - def visit_alternate_type(self, name, info, ifcond, variants): + def visit_alternate_type(self, name, info, ifcond, features, variants): with ifcontext(ifcond, self._genh): self._genh.preamble_add(gen_fwd_object_or_array(name)) self._genh.add(gen_object(name, ifcond, None, diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py index 421e5bd..6e5ed78 100644 --- a/scripts/qapi/visit.py +++ b/scripts/qapi/visit.py @@ -316,7 +316,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor): ''', types=types)) - def visit_enum_type(self, name, info, ifcond, members, prefix): + def visit_enum_type(self, name, info, ifcond, features, members, prefix): with ifcontext(ifcond, self._genh, self._genc): self._genh.add(gen_visit_decl(name, scalar=True)) self._genc.add(gen_visit_enum(name)) @@ -342,7 +342,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor): self._genh.add(gen_visit_decl(name)) self._genc.add(gen_visit_object(name, base, members, variants)) - def visit_alternate_type(self, name, info, ifcond, variants): + def visit_alternate_type(self, name, info, ifcond, features, variants): with ifcontext(ifcond, self._genh, self._genc): self._genh.add(gen_visit_decl(name)) self._genc.add(gen_visit_alternate(name, variants)) diff --git a/tests/qapi-schema/alternate-base.err b/tests/qapi-schema/alternate-base.err index 31ebe56..970a08a 100644 --- a/tests/qapi-schema/alternate-base.err +++ b/tests/qapi-schema/alternate-base.err @@ -1,3 +1,3 @@ alternate-base.json: In alternate 'Alt': alternate-base.json:4: alternate has unknown key 'base' -Valid keys are 'alternate', 'data', 'if'. +Valid keys are 'alternate', 'data', 'features', 'if'. diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json index d992e71..457b8b2 100644 --- a/tests/qapi-schema/doc-good.json +++ b/tests/qapi-schema/doc-good.json @@ -53,10 +53,14 @@ # @Enum: # @one: The _one_ {and only} # +# Features: +# @enum-feat: Also _one_ {and only} +# # @two is undocumented ## { 'enum': 'Enum', 'data': [ { 'name': 'one', 'if': 'defined(IFONE)' }, 'two' ], + 'features': [ 'enum-feat' ], 'if': 'defined(IFCOND)' } ## @@ -86,24 +90,34 @@ ## # @Object: +# Features: +# @union-feat1: a feature ## { 'union': 'Object', + 'features': [ 'union-feat1' ], 'base': 'Base', 'discriminator': 'base1', 'data': { 'one': 'Variant1', 'two': { 'type': 'Variant2', 'if': 'IFTWO' } } } ## # @SugaredUnion: +# Features: +# @union-feat2: a feature ## { 'union': 'SugaredUnion', + 'features': [ 'union-feat2' ], 'data': { 'one': 'Variant1', 'two': { 'type': 'Variant2', 'if': 'IFTWO' } } } ## # @Alternate: # @i: an integer # @b is undocumented +# +# Features: +# @alt-feat: a feature ## { 'alternate': 'Alternate', + 'features': [ 'alt-feat' ], 'data': { 'i': 'int', 'b': 'bool' } } ## @@ -160,6 +174,9 @@ ## # @EVT-BOXED: +# Features: +# @feat3: a feature ## { 'event': 'EVT-BOXED', 'boxed': true, + 'features': [ 'feat3' ], 'data': 'Object' } diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out index 4c9406a..9bcb2b3 100644 --- a/tests/qapi-schema/doc-good.out +++ b/tests/qapi-schema/doc-good.out @@ -15,6 +15,7 @@ enum Enum if ['defined(IFONE)'] member two if ['defined(IFCOND)'] + feature enum-feat object Base member base1: Enum optional=False object Variant1 @@ -28,6 +29,7 @@ object Object case one: Variant1 case two: Variant2 if ['IFTWO'] + feature union-feat1 object q_obj_Variant1-wrapper member data: Variant1 optional=False object q_obj_Variant2-wrapper @@ -42,10 +44,12 @@ object SugaredUnion case one: q_obj_Variant1-wrapper case two: q_obj_Variant2-wrapper if ['IFTWO'] + feature union-feat2 alternate Alternate tag type case i: int case b: bool + feature alt-feat object q_obj_cmd-arg member arg1: int optional=False member arg2: str optional=True @@ -60,6 +64,7 @@ command cmd-boxed Object -> None feature cmd-feat2 event EVT-BOXED Object boxed=True + feature feat3 doc freeform body= = Section @@ -112,6 +117,8 @@ doc symbol=Enum The _one_ {and only} arg=two + feature=enum-feat +Also _one_ {and only} section=None @two is undocumented doc symbol=Base @@ -134,11 +141,15 @@ doc symbol=Variant2 doc symbol=Object body= + feature=union-feat1 +a feature doc symbol=SugaredUnion body= arg=type + feature=union-feat2 +a feature doc symbol=Alternate body= @@ -147,6 +158,8 @@ an integer @b is undocumented arg=b + feature=alt-feat +a feature doc freeform body= == Another subsection @@ -197,3 +210,5 @@ another feature doc symbol=EVT-BOXED body= + feature=feat3 +a feature diff --git a/tests/qapi-schema/doc-good.texi b/tests/qapi-schema/doc-good.texi index d4b15da..76b396d 100644 --- a/tests/qapi-schema/doc-good.texi +++ b/tests/qapi-schema/doc-good.texi @@ -88,6 +88,12 @@ The @emph{one} @{and only@} @item @code{two} Not documented @end table + +@b{Features:} +@table @asis +@item @code{enum-feat} +Also @emph{one} @{and only@} +@end table @code{two} is undocumented @b{If:} @code{defined(IFCOND)} @@ -151,6 +157,12 @@ a feature @item The members of @code{Variant2} when @code{base1} is @t{"two"} (@b{If:} @code{IFTWO}) @end table +@b{Features:} +@table @asis +@item @code{union-feat1} +a feature +@end table + @end deftp @@ -167,6 +179,12 @@ One of @t{"one"}, @t{"two"} @item @code{data: Variant2} when @code{type} is @t{"two"} (@b{If:} @code{IFTWO}) @end table +@b{Features:} +@table @asis +@item @code{union-feat2} +a feature +@end table + @end deftp @@ -184,6 +202,12 @@ an integer Not documented @end table +@b{Features:} +@table @asis +@item @code{alt-feat} +a feature +@end table + @end deftp @@ -283,5 +307,11 @@ another feature @b{Arguments:} the members of @code{Object} +@b{Features:} +@table @asis +@item @code{feat3} +a feature +@end table + @end deftypefn diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index 9abf175..fa4f3a1 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -252,7 +252,7 @@ 'bar': { 'type': ['TestIfEnum'], 'if': 'defined(TEST_IF_EVT_BAR)' } }, 'if': 'defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)' } -# test 'features' for structs +# test 'features' { 'struct': 'FeatureStruct0', 'data': { 'foo': 'int' }, @@ -281,7 +281,22 @@ 'data': { 'foo': 'int' }, 'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'] } ] } -{ 'command': 'test-features', + +{ 'enum': 'FeatureEnum1', + 'data': [ 'eins', 'zwei', 'drei' ], + 'features': [ 'feature1' ] } + +{ 'union': 'FeatureUnion1', + 'base': { 'tag': 'FeatureEnum1' }, + 'discriminator': 'tag', + 'data': { 'eins': 'FeatureStruct1' }, + 'features': [ 'feature1' ] } + +{ 'alternate': 'FeatureAlternate1', + 'data': { 'eins': 'FeatureStruct1' }, + 'features': [ 'feature1' ] } + +{ 'command': 'test-features0', 'data': { 'fs0': 'FeatureStruct0', 'fs1': 'FeatureStruct1', 'fs2': 'FeatureStruct2', @@ -289,12 +304,9 @@ 'fs4': 'FeatureStruct4', 'cfs1': 'CondFeatureStruct1', 'cfs2': 'CondFeatureStruct2', - 'cfs3': 'CondFeatureStruct3' } } - -# test 'features' for command - -{ 'command': 'test-command-features0', + 'cfs3': 'CondFeatureStruct3' }, 'features': [] } + { 'command': 'test-command-features1', 'features': [ 'feature1' ] } { 'command': 'test-command-features3', @@ -308,3 +320,6 @@ { 'command': 'test-command-cond-features3', 'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'] } ] } + +{ 'event': 'TEST-EVENT-FEATURES1', + 'features': [ 'feature1' ] } diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index 9bd3c4a..1cbd080 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -387,7 +387,25 @@ object CondFeatureStruct3 member foo: int optional=False feature feature1 if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'] -object q_obj_test-features-arg +enum FeatureEnum1 + member eins + member zwei + member drei + feature feature1 +object q_obj_FeatureUnion1-base + member tag: FeatureEnum1 optional=False +object FeatureUnion1 + base q_obj_FeatureUnion1-base + tag tag + case eins: FeatureStruct1 + case zwei: q_empty + case drei: q_empty + feature feature1 +alternate FeatureAlternate1 + tag type + case eins: FeatureStruct1 + feature feature1 +object q_obj_test-features0-arg member fs0: FeatureStruct0 optional=False member fs1: FeatureStruct1 optional=False member fs2: FeatureStruct2 optional=False @@ -396,9 +414,7 @@ object q_obj_test-features-arg member cfs1: CondFeatureStruct1 optional=False member cfs2: CondFeatureStruct2 optional=False member cfs3: CondFeatureStruct3 optional=False -command test-features q_obj_test-features-arg -> None - gen=True success_response=True boxed=False oob=False preconfig=False -command test-command-features0 None -> None +command test-features0 q_obj_test-features0-arg -> None gen=True success_response=True boxed=False oob=False preconfig=False command test-command-features1 None -> None gen=True success_response=True boxed=False oob=False preconfig=False @@ -421,6 +437,9 @@ command test-command-cond-features3 None -> None gen=True success_response=True boxed=False oob=False preconfig=False feature feature1 if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'] +event TEST-EVENT-FEATURES1 None + boxed=False + feature feature1 module include/sub-module.json include sub-sub-module.json object SecondArrayRef diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py index bee18ee..af5b57a 100755 --- a/tests/qapi-schema/test-qapi.py +++ b/tests/qapi-schema/test-qapi.py @@ -30,7 +30,7 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor): def visit_include(self, name, info): print('include %s' % name) - def visit_enum_type(self, name, info, ifcond, members, prefix): + def visit_enum_type(self, name, info, ifcond, features, members, prefix): print('enum %s' % name) if prefix: print(' prefix %s' % prefix) @@ -38,6 +38,7 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor): print(' member %s' % m.name) self._print_if(m.ifcond, indent=8) self._print_if(ifcond) + self._print_features(features) def visit_array_type(self, name, info, ifcond, element_type): if not info: @@ -58,10 +59,11 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor): self._print_if(ifcond) self._print_features(features) - def visit_alternate_type(self, name, info, ifcond, variants): + def visit_alternate_type(self, name, info, ifcond, features, variants): print('alternate %s' % name) self._print_variants(variants) self._print_if(ifcond) + self._print_features(features) def visit_command(self, name, info, ifcond, arg_type, ret_type, gen, success_response, boxed, allow_oob, allow_preconfig, @@ -74,10 +76,11 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor): self._print_if(ifcond) self._print_features(features) - def visit_event(self, name, info, ifcond, arg_type, boxed): + def visit_event(self, name, info, ifcond, features, arg_type, boxed): print('event %s %s' % (name, arg_type and arg_type.name)) print(' boxed=%s' % boxed) self._print_if(ifcond) + self._print_features(features) @staticmethod def _print_variants(variants): diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c index 99013ff..d12ff47 100644 --- a/tests/test-qmp-cmds.c +++ b/tests/test-qmp-cmds.c @@ -45,7 +45,7 @@ void qmp_user_def_cmd1(UserDefOne * ud1, Error **errp) { } -void qmp_test_features(FeatureStruct0 *fs0, FeatureStruct1 *fs1, +void qmp_test_features0(FeatureStruct0 *fs0, FeatureStruct1 *fs1, FeatureStruct2 *fs2, FeatureStruct3 *fs3, FeatureStruct4 *fs4, CondFeatureStruct1 *cfs1, CondFeatureStruct2 *cfs2, CondFeatureStruct3 *cfs3, @@ -53,10 +53,6 @@ void qmp_test_features(FeatureStruct0 *fs0, FeatureStruct1 *fs1, { } -void qmp_test_command_features0(Error **errp) -{ -} - void qmp_test_command_features1(Error **errp) { } -- cgit v1.1 From 7b3bc9e28f366e591ae6da0d0c58d05d9f487ced Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:38 +0100 Subject: qapi: Consistently put @features parameter right after @ifcond MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Reviewed-by: Eric Blake Message-Id: <20200317115459.31821-14-armbru@redhat.com> --- scripts/qapi/commands.py | 6 +++--- scripts/qapi/doc.py | 10 +++++----- scripts/qapi/introspect.py | 10 +++++----- scripts/qapi/schema.py | 36 +++++++++++++++++------------------- scripts/qapi/types.py | 4 ++-- scripts/qapi/visit.py | 4 ++-- tests/qapi-schema/test-qapi.py | 10 +++++----- 7 files changed, 39 insertions(+), 41 deletions(-) diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py index 0e13e82..bc30876 100644 --- a/scripts/qapi/commands.py +++ b/scripts/qapi/commands.py @@ -283,9 +283,9 @@ void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds); prefix=self._prefix)) self._genc.add(gen_registry(self._regy.get_content(), self._prefix)) - def visit_command(self, name, info, ifcond, arg_type, ret_type, gen, - success_response, boxed, allow_oob, allow_preconfig, - features): + def visit_command(self, name, info, ifcond, features, + arg_type, ret_type, gen, success_response, boxed, + allow_oob, allow_preconfig): if not gen: return # FIXME: If T is a user-defined type, the user is responsible diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py index 36e8233..92f584e 100644 --- a/scripts/qapi/doc.py +++ b/scripts/qapi/doc.py @@ -249,8 +249,8 @@ class QAPISchemaGenDocVisitor(QAPISchemaVisitor): texi_members(doc, 'Values', member_func=texi_enum_value))) - def visit_object_type(self, name, info, ifcond, base, members, variants, - features): + def visit_object_type(self, name, info, ifcond, features, + base, members, variants): doc = self.cur_doc if base and base.is_implicit(): base = None @@ -262,9 +262,9 @@ class QAPISchemaGenDocVisitor(QAPISchemaVisitor): self._gen.add(texi_type('Alternate', doc, ifcond, texi_members(doc, 'Members'))) - def visit_command(self, name, info, ifcond, arg_type, ret_type, gen, - success_response, boxed, allow_oob, allow_preconfig, - features): + def visit_command(self, name, info, ifcond, features, + arg_type, ret_type, gen, success_response, boxed, + allow_oob, allow_preconfig): doc = self.cur_doc self._gen.add(texi_msg('Command', doc, ifcond, texi_arguments(doc, diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index 2e9e00a..b549105 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -193,8 +193,8 @@ const QLitObject %(c_name)s = %(c_string)s; self._gen_qlit('[' + element + ']', 'array', {'element-type': element}, ifcond, None) - def visit_object_type_flat(self, name, info, ifcond, members, variants, - features): + def visit_object_type_flat(self, name, info, ifcond, features, + members, variants): obj = {'members': [self._gen_member(m) for m in members]} if variants: obj.update(self._gen_variants(variants.tag_member.name, @@ -209,9 +209,9 @@ const QLitObject %(c_name)s = %(c_string)s; for m in variants.variants]}, ifcond, features) - def visit_command(self, name, info, ifcond, arg_type, ret_type, gen, - success_response, boxed, allow_oob, allow_preconfig, - features): + def visit_command(self, name, info, ifcond, features, + arg_type, ret_type, gen, success_response, boxed, + allow_oob, allow_preconfig): 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 2223800..958756e 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -115,20 +115,20 @@ class QAPISchemaVisitor: def visit_array_type(self, name, info, ifcond, element_type): pass - def visit_object_type(self, name, info, ifcond, base, members, variants, - features): + def visit_object_type(self, name, info, ifcond, features, + base, members, variants): pass - def visit_object_type_flat(self, name, info, ifcond, members, variants, - features): + def visit_object_type_flat(self, name, info, ifcond, features, + members, variants): pass def visit_alternate_type(self, name, info, ifcond, features, variants): pass - def visit_command(self, name, info, ifcond, arg_type, ret_type, gen, - success_response, boxed, allow_oob, allow_preconfig, - features): + def visit_command(self, name, info, ifcond, features, + arg_type, ret_type, gen, success_response, boxed, + allow_oob, allow_preconfig): pass def visit_event(self, name, info, ifcond, features, arg_type, boxed): @@ -436,12 +436,12 @@ class QAPISchemaObjectType(QAPISchemaType): def visit(self, visitor): super().visit(visitor) - visitor.visit_object_type(self.name, self.info, self.ifcond, - self.base, self.local_members, self.variants, - self.features) - visitor.visit_object_type_flat(self.name, self.info, self.ifcond, - self.members, self.variants, - self.features) + visitor.visit_object_type( + self.name, self.info, self.ifcond, self.features, + self.base, self.local_members, self.variants) + visitor.visit_object_type_flat( + self.name, self.info, self.ifcond, self.features, + self.members, self.variants) class QAPISchemaMember: @@ -745,12 +745,10 @@ class QAPISchemaCommand(QAPISchemaEntity): def visit(self, visitor): super().visit(visitor) - visitor.visit_command(self.name, self.info, self.ifcond, - self.arg_type, self.ret_type, - self.gen, self.success_response, - self.boxed, self.allow_oob, - self.allow_preconfig, - self.features) + 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) class QAPISchemaEvent(QAPISchemaEntity): diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py index d0d5c03..3ad33af 100644 --- a/scripts/qapi/types.py +++ b/scripts/qapi/types.py @@ -289,8 +289,8 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor): self._genh.add(gen_array(name, element_type)) self._gen_type_cleanup(name) - def visit_object_type(self, name, info, ifcond, base, members, variants, - features): + def visit_object_type(self, name, info, ifcond, features, + base, members, variants): # Nothing to do for the special empty builtin if name == 'q_empty': return diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py index 6e5ed78..23d9194 100644 --- a/scripts/qapi/visit.py +++ b/scripts/qapi/visit.py @@ -326,8 +326,8 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor): self._genh.add(gen_visit_decl(name)) self._genc.add(gen_visit_list(name, element_type)) - def visit_object_type(self, name, info, ifcond, base, members, variants, - features): + def visit_object_type(self, name, info, ifcond, features, + base, members, variants): # Nothing to do for the special empty builtin if name == 'q_empty': return diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py index af5b57a..8e09e54 100755 --- a/tests/qapi-schema/test-qapi.py +++ b/tests/qapi-schema/test-qapi.py @@ -46,8 +46,8 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor): print('array %s %s' % (name, element_type.name)) self._print_if(ifcond) - def visit_object_type(self, name, info, ifcond, base, members, variants, - features): + def visit_object_type(self, name, info, ifcond, features, + base, members, variants): print('object %s' % name) if base: print(' base %s' % base.name) @@ -65,9 +65,9 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor): self._print_if(ifcond) self._print_features(features) - def visit_command(self, name, info, ifcond, arg_type, ret_type, gen, - success_response, boxed, allow_oob, allow_preconfig, - features): + def visit_command(self, name, info, ifcond, features, + arg_type, ret_type, gen, success_response, boxed, + allow_oob, allow_preconfig): print('command %s %s -> %s' % (name, arg_type and arg_type.name, ret_type and ret_type.name)) -- cgit v1.1 From 2e8a843d19fb563b0a4cc7e8a8df8e60df3e97d5 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:39 +0100 Subject: qapi/introspect: Rename *qlit* to reduce confusion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We generate the value of qmp_schema_qlit from an expression tree. The function doing that is named to_qlit(), and its inputs are accumulated in QAPISchemaGenIntrospectVisitor._qlits. We call both its input and its output "qlit". This is confusing. Use "tree" for input, and "qlit" only for output: rename to_qlit() to _tree_to_qlit(), ._qlits to ._trees, ._gen_qlit() to ._gen_tree(). Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Message-Id: <20200317115459.31821-15-armbru@redhat.com> --- scripts/qapi/introspect.py | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index b549105..e4fc9d9 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -16,7 +16,7 @@ from qapi.schema import (QAPISchemaArrayType, QAPISchemaBuiltinType, QAPISchemaType) -def to_qlit(obj, level=0, suppress_first_indent=False): +def _tree_to_qlit(obj, level=0, suppress_first_indent=False): def indent(level): return level * 4 * ' ' @@ -30,7 +30,7 @@ def to_qlit(obj, level=0, suppress_first_indent=False): ret += indent(level) + '/* %s */\n' % comment if ifcond: ret += gen_if(ifcond) - ret += to_qlit(ifobj, level) + ret += _tree_to_qlit(ifobj, level) if ifcond: ret += '\n' + gen_endif(ifcond) return ret @@ -43,7 +43,7 @@ def to_qlit(obj, level=0, suppress_first_indent=False): elif isinstance(obj, str): ret += 'QLIT_QSTR(' + to_c_string(obj) + ')' elif isinstance(obj, list): - elts = [to_qlit(elt, level + 1).strip('\n') + elts = [_tree_to_qlit(elt, level + 1).strip('\n') for elt in obj] elts.append(indent(level + 1) + "{}") ret += 'QLIT_QLIST(((QLitObject[]) {\n' @@ -53,7 +53,8 @@ def to_qlit(obj, level=0, suppress_first_indent=False): elts = [] for key, value in sorted(obj.items()): elts.append(indent(level + 1) + '{ %s, %s }' % - (to_c_string(key), to_qlit(value, level + 1, True))) + (to_c_string(key), + _tree_to_qlit(value, level + 1, True))) elts.append(indent(level + 1) + '{}') ret += 'QLIT_QDICT(((QLitDictEntry[]) {\n' ret += ',\n'.join(elts) + '\n' @@ -79,7 +80,7 @@ class QAPISchemaGenIntrospectVisitor(QAPISchemaMonolithicCVisitor): ' * QAPI/QMP schema introspection', __doc__) self._unmask = unmask self._schema = None - self._qlits = [] + self._trees = [] self._used_types = [] self._name_map = {} self._genc.add(mcgen(''' @@ -108,9 +109,9 @@ extern const QLitObject %(c_name)s; const QLitObject %(c_name)s = %(c_string)s; ''', c_name=c_name(name), - c_string=to_qlit(self._qlits))) + c_string=_tree_to_qlit(self._trees))) self._schema = None - self._qlits = [] + self._trees = [] self._used_types = [] self._name_map = {} @@ -144,7 +145,7 @@ const QLitObject %(c_name)s = %(c_string)s; return '[' + self._use_type(typ.element_type) + ']' return self._name(typ.name) - def _gen_qlit(self, name, mtype, obj, ifcond, features): + def _gen_tree(self, name, mtype, obj, ifcond, features): extra = {} if mtype not in ('command', 'event', 'builtin', 'array'): if not self._unmask: @@ -159,9 +160,9 @@ const QLitObject %(c_name)s = %(c_string)s; if ifcond: extra['if'] = ifcond if extra: - self._qlits.append((obj, extra)) + self._trees.append((obj, extra)) else: - self._qlits.append(obj) + self._trees.append(obj) def _gen_member(self, member): ret = {'name': member.name, 'type': self._use_type(member.type)} @@ -180,17 +181,17 @@ const QLitObject %(c_name)s = %(c_string)s; {'if': variant.ifcond}) def visit_builtin_type(self, name, info, json_type): - self._gen_qlit(name, 'builtin', {'json-type': json_type}, [], None) + self._gen_tree(name, 'builtin', {'json-type': json_type}, [], None) def visit_enum_type(self, name, info, ifcond, features, members, prefix): - self._gen_qlit(name, 'enum', + self._gen_tree(name, 'enum', {'values': [(m.name, {'if': m.ifcond}) for m in members]}, ifcond, features) def visit_array_type(self, name, info, ifcond, element_type): element = self._use_type(element_type) - self._gen_qlit('[' + element + ']', 'array', {'element-type': element}, + self._gen_tree('[' + element + ']', 'array', {'element-type': element}, ifcond, None) def visit_object_type_flat(self, name, info, ifcond, features, @@ -200,10 +201,10 @@ const QLitObject %(c_name)s = %(c_string)s; obj.update(self._gen_variants(variants.tag_member.name, variants.variants)) - self._gen_qlit(name, 'object', obj, ifcond, features) + self._gen_tree(name, 'object', obj, ifcond, features) def visit_alternate_type(self, name, info, ifcond, features, variants): - self._gen_qlit(name, 'alternate', + self._gen_tree(name, 'alternate', {'members': [ ({'type': self._use_type(m.type)}, {'if': m.ifcond}) for m in variants.variants]}, @@ -218,11 +219,11 @@ const QLitObject %(c_name)s = %(c_string)s; 'ret-type': self._use_type(ret_type)} if allow_oob: obj['allow-oob'] = allow_oob - self._gen_qlit(name, 'command', obj, ifcond, features) + self._gen_tree(name, 'command', obj, ifcond, features) def visit_event(self, name, info, ifcond, features, arg_type, boxed): arg_type = arg_type or self._schema.the_empty_object_type - self._gen_qlit(name, 'event', {'arg-type': self._use_type(arg_type)}, + self._gen_tree(name, 'event', {'arg-type': self._use_type(arg_type)}, ifcond, features) -- cgit v1.1 From 24cfd6adddb6308eb36dbe55e541718f71a67637 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:40 +0100 Subject: qapi/introspect: Factor out _make_tree() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The value of @qmp_schema_qlit is generated from an expression tree. Tree nodes are created in several places. Factor out the common code into _make_tree(). This isn't much of a win now. It will pay off when we add feature flags in the next few commits. Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Message-Id: <20200317115459.31821-16-armbru@redhat.com> --- scripts/qapi/introspect.py | 44 ++++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index e4fc9d9..a3fa986 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -16,6 +16,18 @@ from qapi.schema import (QAPISchemaArrayType, QAPISchemaBuiltinType, QAPISchemaType) +def _make_tree(obj, ifcond, features, extra=None): + if extra is None: + extra = {} + if ifcond: + extra['if'] = ifcond + if features: + obj['features'] = [(f.name, {'if': f.ifcond}) for f in features] + if extra: + return (obj, extra) + return obj + + def _tree_to_qlit(obj, level=0, suppress_first_indent=False): def indent(level): @@ -146,47 +158,38 @@ const QLitObject %(c_name)s = %(c_string)s; return self._name(typ.name) def _gen_tree(self, name, mtype, obj, ifcond, features): - extra = {} + extra = None if mtype not in ('command', 'event', 'builtin', 'array'): if not self._unmask: # Output a comment to make it easy to map masked names # back to the source when reading the generated output. - extra['comment'] = '"%s" = %s' % (self._name(name), name) + extra = {'comment': '"%s" = %s' % (self._name(name), name)} name = self._name(name) obj['name'] = name obj['meta-type'] = mtype - if features: - obj['features'] = [(f.name, {'if': f.ifcond}) for f in features] - if ifcond: - extra['if'] = ifcond - if extra: - self._trees.append((obj, extra)) - else: - self._trees.append(obj) + self._trees.append(_make_tree(obj, ifcond, features, extra)) def _gen_member(self, member): - ret = {'name': member.name, 'type': self._use_type(member.type)} + obj = {'name': member.name, 'type': self._use_type(member.type)} if member.optional: - ret['default'] = None - if member.ifcond: - ret = (ret, {'if': member.ifcond}) - return ret + obj['default'] = None + return _make_tree(obj, member.ifcond, None) def _gen_variants(self, tag_name, variants): return {'tag': tag_name, 'variants': [self._gen_variant(v) for v in variants]} def _gen_variant(self, variant): - return ({'case': variant.name, 'type': self._use_type(variant.type)}, - {'if': variant.ifcond}) + obj = {'case': variant.name, 'type': self._use_type(variant.type)} + return _make_tree(obj, variant.ifcond, None) def visit_builtin_type(self, name, info, json_type): self._gen_tree(name, 'builtin', {'json-type': json_type}, [], None) def visit_enum_type(self, name, info, ifcond, features, members, prefix): self._gen_tree(name, 'enum', - {'values': - [(m.name, {'if': m.ifcond}) for m in members]}, + {'values': [_make_tree(m.name, m.ifcond, None) + for m in members]}, ifcond, features) def visit_array_type(self, name, info, ifcond, element_type): @@ -206,7 +209,8 @@ const QLitObject %(c_name)s = %(c_string)s; def visit_alternate_type(self, name, info, ifcond, features, variants): self._gen_tree(name, 'alternate', {'members': [ - ({'type': self._use_type(m.type)}, {'if': m.ifcond}) + _make_tree({'type': self._use_type(m.type)}, + m.ifcond, None) for m in variants.variants]}, ifcond, features) -- cgit v1.1 From ed30f58ddef369b6053fda99a22ea0c79476fc5e Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:41 +0100 Subject: qapi/schema: Change _make_features() to a take feature list QAPISchema._make_features() takes a definition expression, and extracts its 'features' member. The other ._make_FOO() leave destructuring expressions to their callers. Change ._make_features() to match them. Signed-off-by: Markus Armbruster Message-Id: <20200317115459.31821-17-armbru@redhat.com> Reviewed-by: Eric Blake --- scripts/qapi/schema.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 958756e..4d8ad67 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -904,8 +904,9 @@ class QAPISchema: self._def_entity(QAPISchemaEnumType('QType', None, None, None, None, qtype_values, 'QTYPE')) - def _make_features(self, expr, info): - features = expr.get('features', []) + def _make_features(self, features, info): + if features is None: + return [] return [QAPISchemaFeature(f['name'], info, f.get('if')) for f in features] @@ -955,7 +956,7 @@ class QAPISchema: data = expr['data'] prefix = expr.get('prefix') ifcond = expr.get('if') - features = self._make_features(expr, info) + features = self._make_features(expr.get('features'), info) self._def_entity(QAPISchemaEnumType( name, info, doc, ifcond, features, self._make_enum_members(data, info), prefix)) @@ -979,7 +980,7 @@ class QAPISchema: base = expr.get('base') data = expr['data'] ifcond = expr.get('if') - features = self._make_features(expr, info) + features = self._make_features(expr.get('features'), info) self._def_entity(QAPISchemaObjectType( name, info, doc, ifcond, features, base, self._make_members(data, info), @@ -1002,7 +1003,7 @@ class QAPISchema: data = expr['data'] base = expr.get('base') ifcond = expr.get('if') - features = self._make_features(expr, info) + features = self._make_features(expr.get('features'), info) tag_name = expr.get('discriminator') tag_member = None if isinstance(base, dict): @@ -1032,7 +1033,7 @@ class QAPISchema: name = expr['alternate'] data = expr['data'] ifcond = expr.get('if') - features = self._make_features(expr, info) + features = self._make_features(expr.get('features'), info) variants = [self._make_variant(key, value['type'], value.get('if'), info) for (key, value) in data.items()] @@ -1052,7 +1053,7 @@ class QAPISchema: allow_oob = expr.get('allow-oob', False) allow_preconfig = expr.get('allow-preconfig', False) ifcond = expr.get('if') - features = self._make_features(expr, info) + features = self._make_features(expr.get('features'), info) if isinstance(data, OrderedDict): data = self._make_implicit_object_type( name, info, ifcond, @@ -1070,7 +1071,7 @@ class QAPISchema: data = expr.get('data') boxed = expr.get('boxed', False) ifcond = expr.get('if') - features = self._make_features(expr, info) + features = self._make_features(expr.get('features'), info) if isinstance(data, OrderedDict): data = self._make_implicit_object_type( name, info, ifcond, -- cgit v1.1 From 226b5be6d434a3f139436d19227121266d8729d2 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:42 +0100 Subject: qapi/schema: Reorder classes so related ones are together MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move QAPISchemaAlternateType up some, so that all QAPISchemaFOOType are together. Move QAPISchemaObjectTypeVariants right behind its users. Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Message-Id: <20200317115459.31821-18-armbru@redhat.com> --- scripts/qapi/schema.py | 268 ++++++++++++++++++++++++------------------------- 1 file changed, 134 insertions(+), 134 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 4d8ad67..0acf8b4 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -444,82 +444,72 @@ class QAPISchemaObjectType(QAPISchemaType): self.members, self.variants) -class QAPISchemaMember: - """ Represents object members, enum members and features """ - role = 'member' - - def __init__(self, name, info, ifcond=None): - assert isinstance(name, str) - self.name = name - self.info = info - self.ifcond = ifcond or [] - self.defined_in = None - - def set_defined_in(self, name): - assert not self.defined_in - self.defined_in = name - - def check_clash(self, info, seen): - cname = c_name(self.name) - if cname in seen: - raise QAPISemError( - info, - "%s collides with %s" - % (self.describe(info), seen[cname].describe(info))) - seen[cname] = self - - def describe(self, info): - role = self.role - defined_in = self.defined_in - assert defined_in - - if defined_in.startswith('q_obj_'): - # See QAPISchema._make_implicit_object_type() - reverse the - # mapping there to create a nice human-readable description - defined_in = defined_in[6:] - if defined_in.endswith('-arg'): - # Implicit type created for a command's dict 'data' - assert role == 'member' - role = 'parameter' - elif defined_in.endswith('-base'): - # Implicit type created for a flat union's dict 'base' - role = 'base ' + role - else: - # Implicit type created for a simple union's branch - assert defined_in.endswith('-wrapper') - # Unreachable and not implemented - assert False - elif defined_in.endswith('Kind'): - # See QAPISchema._make_implicit_enum_type() - # Implicit enum created for simple union's branches - assert role == 'value' - role = 'branch' - elif defined_in != info.defn_name: - return "%s '%s' of type '%s'" % (role, self.name, defined_in) - return "%s '%s'" % (role, self.name) - +class QAPISchemaAlternateType(QAPISchemaType): + meta = 'alternate' -class QAPISchemaEnumMember(QAPISchemaMember): - role = 'value' + def __init__(self, name, info, doc, ifcond, features, variants): + super().__init__(name, info, doc, ifcond, features) + assert isinstance(variants, QAPISchemaObjectTypeVariants) + assert variants.tag_member + variants.set_defined_in(name) + variants.tag_member.set_defined_in(self.name) + self.variants = variants + def check(self, schema): + super().check(schema) + self.variants.tag_member.check(schema) + # Not calling self.variants.check_clash(), because there's nothing + # to clash with + self.variants.check(schema, {}) + # Alternate branch names have no relation to the tag enum values; + # so we have to check for potential name collisions ourselves. + seen = {} + types_seen = {} + for v in self.variants.variants: + v.check_clash(self.info, seen) + qtype = v.type.alternate_qtype() + if not qtype: + raise QAPISemError( + self.info, + "%s cannot use %s" + % (v.describe(self.info), v.type.describe())) + conflicting = set([qtype]) + if qtype == 'QTYPE_QSTRING': + if isinstance(v.type, QAPISchemaEnumType): + for m in v.type.members: + if m.name in ['on', 'off']: + conflicting.add('QTYPE_QBOOL') + if re.match(r'[-+0-9.]', m.name): + # lazy, could be tightened + conflicting.add('QTYPE_QNUM') + else: + conflicting.add('QTYPE_QNUM') + conflicting.add('QTYPE_QBOOL') + for qt in conflicting: + if qt in types_seen: + raise QAPISemError( + self.info, + "%s can't be distinguished from '%s'" + % (v.describe(self.info), types_seen[qt])) + types_seen[qt] = v.name -class QAPISchemaFeature(QAPISchemaMember): - role = 'feature' + def connect_doc(self, doc=None): + super().connect_doc(doc) + doc = doc or self.doc + if doc: + for v in self.variants.variants: + doc.connect_member(v) + def c_type(self): + return c_name(self.name) + pointer_suffix -class QAPISchemaObjectTypeMember(QAPISchemaMember): - def __init__(self, name, info, typ, optional, ifcond=None): - super().__init__(name, info, ifcond) - assert isinstance(typ, str) - assert isinstance(optional, bool) - self._type_name = typ - self.type = None - self.optional = optional + def json_type(self): + return 'value' - def check(self, schema): - assert self.defined_in - self.type = schema.resolve_type(self._type_name, self.info, - self.describe) + def visit(self, visitor): + super().visit(visitor) + visitor.visit_alternate_type( + self.name, self.info, self.ifcond, self.features, self.variants) class QAPISchemaObjectTypeVariants: @@ -613,79 +603,89 @@ class QAPISchemaObjectTypeVariants: v.type.check_clash(info, dict(seen)) -class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): - role = 'branch' +class QAPISchemaMember: + """ Represents object members, enum members and features """ + role = 'member' - def __init__(self, name, info, typ, ifcond=None): - super().__init__(name, info, typ, False, ifcond) + def __init__(self, name, info, ifcond=None): + assert isinstance(name, str) + self.name = name + self.info = info + self.ifcond = ifcond or [] + self.defined_in = None + def set_defined_in(self, name): + assert not self.defined_in + self.defined_in = name -class QAPISchemaAlternateType(QAPISchemaType): - meta = 'alternate' + def check_clash(self, info, seen): + cname = c_name(self.name) + if cname in seen: + raise QAPISemError( + info, + "%s collides with %s" + % (self.describe(info), seen[cname].describe(info))) + seen[cname] = self - def __init__(self, name, info, doc, ifcond, features, variants): - super().__init__(name, info, doc, ifcond, features) - assert isinstance(variants, QAPISchemaObjectTypeVariants) - assert variants.tag_member - variants.set_defined_in(name) - variants.tag_member.set_defined_in(self.name) - self.variants = variants + def describe(self, info): + role = self.role + defined_in = self.defined_in + assert defined_in - def check(self, schema): - super().check(schema) - self.variants.tag_member.check(schema) - # Not calling self.variants.check_clash(), because there's nothing - # to clash with - self.variants.check(schema, {}) - # Alternate branch names have no relation to the tag enum values; - # so we have to check for potential name collisions ourselves. - seen = {} - types_seen = {} - for v in self.variants.variants: - v.check_clash(self.info, seen) - qtype = v.type.alternate_qtype() - if not qtype: - raise QAPISemError( - self.info, - "%s cannot use %s" - % (v.describe(self.info), v.type.describe())) - conflicting = set([qtype]) - if qtype == 'QTYPE_QSTRING': - if isinstance(v.type, QAPISchemaEnumType): - for m in v.type.members: - if m.name in ['on', 'off']: - conflicting.add('QTYPE_QBOOL') - if re.match(r'[-+0-9.]', m.name): - # lazy, could be tightened - conflicting.add('QTYPE_QNUM') - else: - conflicting.add('QTYPE_QNUM') - conflicting.add('QTYPE_QBOOL') - for qt in conflicting: - if qt in types_seen: - raise QAPISemError( - self.info, - "%s can't be distinguished from '%s'" - % (v.describe(self.info), types_seen[qt])) - types_seen[qt] = v.name + if defined_in.startswith('q_obj_'): + # See QAPISchema._make_implicit_object_type() - reverse the + # mapping there to create a nice human-readable description + defined_in = defined_in[6:] + if defined_in.endswith('-arg'): + # Implicit type created for a command's dict 'data' + assert role == 'member' + role = 'parameter' + elif defined_in.endswith('-base'): + # Implicit type created for a flat union's dict 'base' + role = 'base ' + role + else: + # Implicit type created for a simple union's branch + assert defined_in.endswith('-wrapper') + # Unreachable and not implemented + assert False + elif defined_in.endswith('Kind'): + # See QAPISchema._make_implicit_enum_type() + # Implicit enum created for simple union's branches + assert role == 'value' + role = 'branch' + elif defined_in != info.defn_name: + return "%s '%s' of type '%s'" % (role, self.name, defined_in) + return "%s '%s'" % (role, self.name) - def connect_doc(self, doc=None): - super().connect_doc(doc) - doc = doc or self.doc - if doc: - for v in self.variants.variants: - doc.connect_member(v) - def c_type(self): - return c_name(self.name) + pointer_suffix +class QAPISchemaEnumMember(QAPISchemaMember): + role = 'value' - def json_type(self): - return 'value' - def visit(self, visitor): - super().visit(visitor) - visitor.visit_alternate_type( - self.name, self.info, self.ifcond, self.features, self.variants) +class QAPISchemaFeature(QAPISchemaMember): + role = 'feature' + + +class QAPISchemaObjectTypeMember(QAPISchemaMember): + def __init__(self, name, info, typ, optional, ifcond=None): + super().__init__(name, info, ifcond) + assert isinstance(typ, str) + assert isinstance(optional, bool) + self._type_name = typ + self.type = None + self.optional = optional + + def check(self, schema): + assert self.defined_in + self.type = schema.resolve_type(self._type_name, self.info, + self.describe) + + +class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): + role = 'branch' + + def __init__(self, name, info, typ, ifcond=None): + super().__init__(name, info, typ, False, ifcond) class QAPISchemaCommand(QAPISchemaEntity): -- cgit v1.1 From 5858fd1a023f434f30136bdb2b617834561784bd Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:43 +0100 Subject: qapi/schema: Rename QAPISchemaObjectType{Variant,Variants} MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QAPISchemaObjectTypeVariants represents both object type and alternate type variants. Rename to QAPISchemaVariants. Rename QAPISchemaObjectTypeVariant the same way. Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Message-Id: <20200317115459.31821-19-armbru@redhat.com> --- scripts/qapi/schema.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 0acf8b4..033c84c 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -338,7 +338,7 @@ class QAPISchemaObjectType(QAPISchemaType): assert isinstance(m, QAPISchemaObjectTypeMember) m.set_defined_in(name) if variants is not None: - assert isinstance(variants, QAPISchemaObjectTypeVariants) + assert isinstance(variants, QAPISchemaVariants) variants.set_defined_in(name) self._base_name = base self.base = None @@ -449,7 +449,7 @@ class QAPISchemaAlternateType(QAPISchemaType): def __init__(self, name, info, doc, ifcond, features, variants): super().__init__(name, info, doc, ifcond, features) - assert isinstance(variants, QAPISchemaObjectTypeVariants) + assert isinstance(variants, QAPISchemaVariants) assert variants.tag_member variants.set_defined_in(name) variants.tag_member.set_defined_in(self.name) @@ -512,7 +512,7 @@ class QAPISchemaAlternateType(QAPISchemaType): self.name, self.info, self.ifcond, self.features, self.variants) -class QAPISchemaObjectTypeVariants: +class QAPISchemaVariants: def __init__(self, tag_name, info, tag_member, variants): # Flat unions pass tag_name but not tag_member. # Simple unions and alternates pass tag_member but not tag_name. @@ -522,7 +522,7 @@ class QAPISchemaObjectTypeVariants: assert (isinstance(tag_name, str) or isinstance(tag_member, QAPISchemaObjectTypeMember)) for v in variants: - assert isinstance(v, QAPISchemaObjectTypeVariant) + assert isinstance(v, QAPISchemaVariant) self._tag_name = tag_name self.info = info self.tag_member = tag_member @@ -572,8 +572,8 @@ class QAPISchemaObjectTypeVariants: cases = {v.name for v in self.variants} for m in self.tag_member.type.members: if m.name not in cases: - v = QAPISchemaObjectTypeVariant(m.name, self.info, - 'q_empty', m.ifcond) + v = QAPISchemaVariant(m.name, self.info, + 'q_empty', m.ifcond) v.set_defined_in(self.tag_member.defined_in) self.variants.append(v) if not self.variants: @@ -681,7 +681,7 @@ class QAPISchemaObjectTypeMember(QAPISchemaMember): self.describe) -class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): +class QAPISchemaVariant(QAPISchemaObjectTypeMember): role = 'branch' def __init__(self, name, info, typ, ifcond=None): @@ -987,7 +987,7 @@ class QAPISchema: None)) def _make_variant(self, case, typ, ifcond, info): - return QAPISchemaObjectTypeVariant(case, info, typ, ifcond) + return QAPISchemaVariant(case, info, typ, ifcond) def _make_simple_variant(self, case, typ, ifcond, info): if isinstance(typ, list): @@ -996,7 +996,7 @@ class QAPISchema: typ = self._make_implicit_object_type( typ, info, self.lookup_type(typ), 'wrapper', [self._make_member('data', typ, None, info)]) - return QAPISchemaObjectTypeVariant(case, info, typ, ifcond) + return QAPISchemaVariant(case, info, typ, ifcond) def _def_union_type(self, expr, info, doc): name = expr['union'] @@ -1026,7 +1026,7 @@ class QAPISchema: self._def_entity( QAPISchemaObjectType(name, info, doc, ifcond, features, base, members, - QAPISchemaObjectTypeVariants( + QAPISchemaVariants( tag_name, info, tag_member, variants))) def _def_alternate_type(self, expr, info, doc): @@ -1040,7 +1040,7 @@ class QAPISchema: tag_member = QAPISchemaObjectTypeMember('type', info, 'QType', False) self._def_entity( QAPISchemaAlternateType(name, info, doc, ifcond, features, - QAPISchemaObjectTypeVariants( + QAPISchemaVariants( None, info, tag_member, variants))) def _def_command(self, expr, info, doc): -- cgit v1.1 From 645178c0697fb0a7805c090745de9925d935cd1b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:44 +0100 Subject: qapi/schema: Call QAPIDoc.connect_member() in just one place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The .connect_doc() of classes that have QAPISchemaMember connect them to their documentation. Change them to delegate the actual work to new QAPISchemaMember.connect_doc(). Matches the .connect_doc() that already exist. Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Message-Id: <20200317115459.31821-20-armbru@redhat.com> --- scripts/qapi/schema.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 033c84c..59e1f5a 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -252,9 +252,8 @@ class QAPISchemaEnumType(QAPISchemaType): def connect_doc(self, doc=None): super().connect_doc(doc) doc = doc or self.doc - if doc: - for m in self.members: - doc.connect_member(m) + for m in self.members: + m.connect_doc(doc) def is_implicit(self): # See QAPISchema._make_implicit_enum_type() and ._def_predefineds() @@ -396,11 +395,10 @@ class QAPISchemaObjectType(QAPISchemaType): def connect_doc(self, doc=None): super().connect_doc(doc) doc = doc or self.doc - if doc: - if self.base and self.base.is_implicit(): - self.base.connect_doc(doc) - for m in self.local_members: - doc.connect_member(m) + if self.base and self.base.is_implicit(): + self.base.connect_doc(doc) + for m in self.local_members: + m.connect_doc(doc) @property def ifcond(self): @@ -496,9 +494,8 @@ class QAPISchemaAlternateType(QAPISchemaType): def connect_doc(self, doc=None): super().connect_doc(doc) doc = doc or self.doc - if doc: - for v in self.variants.variants: - doc.connect_member(v) + for v in self.variants.variants: + v.connect_doc(doc) def c_type(self): return c_name(self.name) + pointer_suffix @@ -627,6 +624,10 @@ class QAPISchemaMember: % (self.describe(info), seen[cname].describe(info))) seen[cname] = self + def connect_doc(self, doc): + if doc: + doc.connect_member(self) + def describe(self, info): role = self.role defined_in = self.defined_in -- cgit v1.1 From 84ab00868798a65e19d76d3cb5f1552c6b25ceb4 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:45 +0100 Subject: qapi: Add feature flags to struct members MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Message-Id: <20200317115459.31821-21-armbru@redhat.com> --- docs/devel/qapi-code-gen.txt | 4 +++- qapi/introspect.json | 6 +++++- scripts/qapi/expr.py | 3 ++- scripts/qapi/introspect.py | 2 +- scripts/qapi/schema.py | 25 ++++++++++++++++++++----- tests/qapi-schema/doc-good.json | 5 ++++- tests/qapi-schema/doc-good.out | 3 +++ tests/qapi-schema/doc-good.texi | 2 ++ tests/qapi-schema/qapi-schema-test.json | 2 +- tests/qapi-schema/qapi-schema-test.out | 1 + tests/qapi-schema/test-qapi.py | 7 ++++--- 11 files changed, 46 insertions(+), 14 deletions(-) diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt index 43f31b4..39ae2ca 100644 --- a/docs/devel/qapi-code-gen.txt +++ b/docs/devel/qapi-code-gen.txt @@ -234,7 +234,9 @@ Syntax: '*features': FEATURES } MEMBERS = { MEMBER, ... } MEMBER = STRING : TYPE-REF - | STRING : { 'type': TYPE-REF, '*if': COND } + | STRING : { 'type': TYPE-REF, + '*if': COND, + '*features': FEATURES } Member 'struct' names the struct type. diff --git a/qapi/introspect.json b/qapi/introspect.json index da3e176..b1aabd4 100644 --- a/qapi/introspect.json +++ b/qapi/introspect.json @@ -206,11 +206,15 @@ # Future extension: if present and non-null, the parameter # is optional, and defaults to this value. # +# @features: names of features associated with the member, in no +# particular order. (since 5.0) +# # Since: 2.5 ## { 'struct': 'SchemaInfoObjectMember', - 'data': { 'name': 'str', 'type': 'str', '*default': 'any' } } + 'data': { 'name': 'str', 'type': 'str', '*default': 'any', # @default's type must be null or match @type + '*features': [ 'str' ] } } ## # @SchemaInfoObjectVariant: diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index f9c4448..2942520 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -167,8 +167,9 @@ def check_type(value, info, source, allow_optional=True, permit_upper=permit_upper) if c_name(key, False) == 'u' or c_name(key, False).startswith('has_'): raise QAPISemError(info, "%s uses reserved name" % key_source) - check_keys(arg, info, key_source, ['type'], ['if']) + check_keys(arg, info, key_source, ['type'], ['if', 'features']) check_if(arg, info, key_source) + check_features(arg.get('features'), info) check_type(arg['type'], info, key_source, allow_array=True) diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index a3fa986..23652be 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -173,7 +173,7 @@ const QLitObject %(c_name)s = %(c_string)s; obj = {'name': member.name, 'type': self._use_type(member.type)} if member.optional: obj['default'] = None - return _make_tree(obj, member.ifcond, None) + return _make_tree(obj, member.ifcond, member.features) def _gen_variants(self, tag_name, variants): return {'tag': tag_name, diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 59e1f5a..6ee3677 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -668,18 +668,31 @@ class QAPISchemaFeature(QAPISchemaMember): class QAPISchemaObjectTypeMember(QAPISchemaMember): - def __init__(self, name, info, typ, optional, ifcond=None): + def __init__(self, name, info, typ, optional, ifcond=None, features=None): super().__init__(name, info, ifcond) assert isinstance(typ, str) assert isinstance(optional, bool) + for f in features or []: + assert isinstance(f, QAPISchemaFeature) + f.set_defined_in(name) self._type_name = typ self.type = None self.optional = optional + self.features = features or [] def check(self, schema): assert self.defined_in self.type = schema.resolve_type(self._type_name, self.info, self.describe) + seen = {} + for f in self.features: + f.check_clash(self.info, seen) + + def connect_doc(self, doc): + super().connect_doc(doc) + if doc: + for f in self.features: + doc.connect_feature(f) class QAPISchemaVariant(QAPISchemaObjectTypeMember): @@ -962,7 +975,7 @@ class QAPISchema: name, info, doc, ifcond, features, self._make_enum_members(data, info), prefix)) - def _make_member(self, name, typ, ifcond, info): + def _make_member(self, name, typ, ifcond, features, info): optional = False if name.startswith('*'): name = name[1:] @@ -970,10 +983,12 @@ class QAPISchema: if isinstance(typ, list): assert len(typ) == 1 typ = self._make_array_type(typ[0], info) - return QAPISchemaObjectTypeMember(name, info, typ, optional, ifcond) + return QAPISchemaObjectTypeMember(name, info, typ, optional, ifcond, + self._make_features(features, info)) def _make_members(self, data, info): - return [self._make_member(key, value['type'], value.get('if'), info) + return [self._make_member(key, value['type'], value.get('if'), + value.get('features'), info) for (key, value) in data.items()] def _def_struct_type(self, expr, info, doc): @@ -996,7 +1011,7 @@ class QAPISchema: typ = self._make_array_type(typ[0], info) typ = self._make_implicit_object_type( typ, info, self.lookup_type(typ), - 'wrapper', [self._make_member('data', typ, None, info)]) + 'wrapper', [self._make_member('data', typ, None, None, info)]) return QAPISchemaVariant(case, info, typ, ifcond) def _def_union_type(self, expr, info, doc): diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json index 457b8b2..ddd89d1 100644 --- a/tests/qapi-schema/doc-good.json +++ b/tests/qapi-schema/doc-good.json @@ -78,10 +78,13 @@ # # Features: # @variant1-feat: a feature +# @member-feat: a member feature ## { 'struct': 'Variant1', 'features': [ 'variant1-feat' ], - 'data': { 'var1': { 'type': 'str', 'if': 'defined(IFSTR)' } } } + 'data': { 'var1': { 'type': 'str', + 'features': [ 'member-feat' ], + 'if': 'defined(IFSTR)' } } } ## # @Variant2: diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out index 9bcb2b3..6757dd2 100644 --- a/tests/qapi-schema/doc-good.out +++ b/tests/qapi-schema/doc-good.out @@ -21,6 +21,7 @@ object Base object Variant1 member var1: str optional=False if ['defined(IFSTR)'] + feature member-feat feature variant1-feat object Variant2 object Object @@ -135,6 +136,8 @@ Another paragraph (but no @var: line) feature=variant1-feat a feature + feature=member-feat +a member feature doc symbol=Variant2 body= diff --git a/tests/qapi-schema/doc-good.texi b/tests/qapi-schema/doc-good.texi index 76b396d..7f28fb7 100644 --- a/tests/qapi-schema/doc-good.texi +++ b/tests/qapi-schema/doc-good.texi @@ -132,6 +132,8 @@ Not documented @table @asis @item @code{variant1-feat} a feature +@item @code{member-feat} +a member feature @end table @end deftp diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index fa4f3a1..f576c33 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -258,7 +258,7 @@ 'data': { 'foo': 'int' }, 'features': [] } { 'struct': 'FeatureStruct1', - 'data': { 'foo': 'int' }, + 'data': { 'foo': { 'type': 'int', 'features': [ 'member-feature1' ] } }, 'features': [ 'feature1' ] } { 'struct': 'FeatureStruct2', 'data': { 'foo': 'int' }, diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index 1cbd080..cd863ae 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -359,6 +359,7 @@ object FeatureStruct0 member foo: int optional=False object FeatureStruct1 member foo: int optional=False + feature member-feature1 feature feature1 object FeatureStruct2 member foo: int optional=False diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py index 8e09e54..f396b47 100755 --- a/tests/qapi-schema/test-qapi.py +++ b/tests/qapi-schema/test-qapi.py @@ -55,6 +55,7 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor): print(' member %s: %s optional=%s' % (m.name, m.type.name, m.optional)) self._print_if(m.ifcond, 8) + self._print_features(m.features, indent=8) self._print_variants(variants) self._print_if(ifcond) self._print_features(features) @@ -96,11 +97,11 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor): print('%sif %s' % (' ' * indent, ifcond)) @classmethod - def _print_features(cls, features): + def _print_features(cls, features, indent=4): if features: for f in features: - print(' feature %s' % f.name) - cls._print_if(f.ifcond, 8) + print('%sfeature %s' % (' ' * indent, f.name)) + cls._print_if(f.ifcond, indent + 4) def test_frontend(fname): -- cgit v1.1 From cf4a0643c8ab7d503465bd3f725369b630ea5001 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:46 +0100 Subject: qapi: Inline do_qmp_dispatch() into qmp_dispatch() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both functions check @request is a QDict, and both have code for QCO_NO_SUCCESS_RESP. This wasn't the case back when they were created. It's a sign of muddled responsibilities. Inline. The next commits will clean up some more. Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Message-Id: <20200317115459.31821-22-armbru@redhat.com> --- qapi/qmp-dispatch.c | 90 ++++++++++++++++++++++++----------------------------- 1 file changed, 41 insertions(+), 49 deletions(-) diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c index bc264b3..a588072 100644 --- a/qapi/qmp-dispatch.c +++ b/qapi/qmp-dispatch.c @@ -75,19 +75,42 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, bool allow_oob, return dict; } -static QObject *do_qmp_dispatch(QmpCommandList *cmds, QObject *request, - bool allow_oob, Error **errp) +QDict *qmp_error_response(Error *err) +{ + QDict *rsp; + + rsp = qdict_from_jsonf_nofail("{ 'error': { 'class': %s, 'desc': %s } }", + QapiErrorClass_str(error_get_class(err)), + error_get_pretty(err)); + error_free(err); + return rsp; +} + +/* + * Does @qdict look like a command to be run out-of-band? + */ +bool qmp_is_oob(const QDict *dict) { - Error *local_err = NULL; + return qdict_haskey(dict, "exec-oob") + && !qdict_haskey(dict, "execute"); +} + +QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request, + bool allow_oob) +{ + Error *err = NULL; bool oob; const char *command; - QDict *args, *dict; + QDict *args; QmpCommand *cmd; + QDict *dict = qobject_to(QDict, request); + QObject *id = dict ? qdict_get(dict, "id") : NULL; QObject *ret = NULL; + QDict *rsp; - dict = qmp_dispatch_check_obj(request, allow_oob, errp); + dict = qmp_dispatch_check_obj(request, allow_oob, &err); if (!dict) { - return NULL; + goto out; } command = qdict_get_try_str(dict, "execute"); @@ -99,27 +122,27 @@ static QObject *do_qmp_dispatch(QmpCommandList *cmds, QObject *request, } cmd = qmp_find_command(cmds, command); if (cmd == NULL) { - error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND, + error_set(&err, ERROR_CLASS_COMMAND_NOT_FOUND, "The command %s has not been found", command); - return NULL; + goto out; } if (!cmd->enabled) { - error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND, + error_set(&err, ERROR_CLASS_COMMAND_NOT_FOUND, "The command %s has been disabled for this instance", command); - return NULL; + goto out; } if (oob && !(cmd->options & QCO_ALLOW_OOB)) { - error_setg(errp, "The command %s does not support OOB", + error_setg(&err, "The command %s does not support OOB", command); - return NULL; + goto out; } if (runstate_check(RUN_STATE_PRECONFIG) && !(cmd->options & QCO_ALLOW_PRECONFIG)) { - error_setg(errp, "The command '%s' isn't permitted in '%s' state", + error_setg(&err, "The command '%s' isn't permitted in '%s' state", cmd->name, RunState_str(RUN_STATE_PRECONFIG)); - return NULL; + goto out; } if (!qdict_haskey(dict, "arguments")) { @@ -129,9 +152,9 @@ static QObject *do_qmp_dispatch(QmpCommandList *cmds, QObject *request, qobject_ref(args); } - cmd->fn(args, &ret, &local_err); - if (local_err) { - error_propagate(errp, local_err); + cmd->fn(args, &ret, &err); + if (err) { + ; } else if (cmd->options & QCO_NO_SUCCESS_RESP) { g_assert(!ret); } else if (!ret) { @@ -141,38 +164,7 @@ static QObject *do_qmp_dispatch(QmpCommandList *cmds, QObject *request, qobject_unref(args); - return ret; -} - -QDict *qmp_error_response(Error *err) -{ - QDict *rsp; - - rsp = qdict_from_jsonf_nofail("{ 'error': { 'class': %s, 'desc': %s } }", - QapiErrorClass_str(error_get_class(err)), - error_get_pretty(err)); - error_free(err); - return rsp; -} - -/* - * Does @qdict look like a command to be run out-of-band? - */ -bool qmp_is_oob(const QDict *dict) -{ - return qdict_haskey(dict, "exec-oob") - && !qdict_haskey(dict, "execute"); -} - -QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request, - bool allow_oob) -{ - Error *err = NULL; - QDict *dict = qobject_to(QDict, request); - QObject *ret, *id = dict ? qdict_get(dict, "id") : NULL; - QDict *rsp; - - ret = do_qmp_dispatch(cmds, request, allow_oob, &err); +out: if (err) { rsp = qmp_error_response(err); } else if (ret) { -- cgit v1.1 From d3226035630c6f0805ed26c77011e49565029cb0 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:47 +0100 Subject: qapi: Simplify how qmp_dispatch() deals with QCO_NO_SUCCESS_RESP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Message-Id: <20200317115459.31821-23-armbru@redhat.com> --- qapi/qmp-dispatch.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c index a588072..550d1fe 100644 --- a/qapi/qmp-dispatch.c +++ b/qapi/qmp-dispatch.c @@ -106,7 +106,7 @@ QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request, QDict *dict = qobject_to(QDict, request); QObject *id = dict ? qdict_get(dict, "id") : NULL; QObject *ret = NULL; - QDict *rsp; + QDict *rsp = NULL; dict = qmp_dispatch_check_obj(request, allow_oob, &err); if (!dict) { @@ -151,31 +151,32 @@ QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request, args = qdict_get_qdict(dict, "arguments"); qobject_ref(args); } - cmd->fn(args, &ret, &err); + qobject_unref(args); if (err) { - ; - } else if (cmd->options & QCO_NO_SUCCESS_RESP) { + goto out; + } + + if (cmd->options & QCO_NO_SUCCESS_RESP) { g_assert(!ret); + return NULL; } else if (!ret) { /* TODO turn into assertion */ ret = QOBJECT(qdict_new()); } - qobject_unref(args); + rsp = qdict_new(); + qdict_put_obj(rsp, "return", ret); out: if (err) { + assert(!rsp); rsp = qmp_error_response(err); - } else if (ret) { - rsp = qdict_new(); - qdict_put_obj(rsp, "return", ret); - } else { - /* Can only happen for commands with QCO_NO_SUCCESS_RESP */ - rsp = NULL; } - if (rsp && id) { + assert(rsp); + + if (id) { qdict_put_obj(rsp, "id", qobject_ref(id)); } -- cgit v1.1 From a62c61747fc0934e0f42a37aa078a21c50565fe6 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:48 +0100 Subject: qapi: Simplify how qmp_dispatch() gets the request ID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We convert the request object to a QDict twice: first in qmp_dispatch() to get the request ID, and then again in qmp_dispatch_check_obj(), which converts to QDict, then checks and returns it. We can't get the request ID from the latter, because it's null when the qdict flunks the checks. Move the checked conversion to QDict from qmp_dispatch_check_obj() to qmp_dispatch(), and drop the duplicate there. Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau Message-Id: <20200317115459.31821-24-armbru@redhat.com> --- qapi/qmp-dispatch.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c index 550d1fe..91e50fa 100644 --- a/qapi/qmp-dispatch.c +++ b/qapi/qmp-dispatch.c @@ -19,20 +19,13 @@ #include "sysemu/runstate.h" #include "qapi/qmp/qbool.h" -static QDict *qmp_dispatch_check_obj(const QObject *request, bool allow_oob, +static QDict *qmp_dispatch_check_obj(QDict *dict, bool allow_oob, Error **errp) { const char *exec_key = NULL; const QDictEntry *ent; const char *arg_name; const QObject *arg_obj; - QDict *dict; - - dict = qobject_to(QDict, request); - if (!dict) { - error_setg(errp, "QMP input must be a JSON object"); - return NULL; - } for (ent = qdict_first(dict); ent; ent = qdict_next(dict, ent)) { @@ -103,13 +96,21 @@ QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request, const char *command; QDict *args; QmpCommand *cmd; - QDict *dict = qobject_to(QDict, request); - QObject *id = dict ? qdict_get(dict, "id") : NULL; + QDict *dict; + QObject *id; QObject *ret = NULL; QDict *rsp = NULL; - dict = qmp_dispatch_check_obj(request, allow_oob, &err); + dict = qobject_to(QDict, request); if (!dict) { + id = NULL; + error_setg(&err, "QMP input must be a JSON object"); + goto out; + } + + id = qdict_get(dict, "id"); + + if (!qmp_dispatch_check_obj(dict, allow_oob, &err)) { goto out; } -- cgit v1.1 From 4a8837389ef28554a57cdad8e2fc90ae1362dcb2 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:49 +0100 Subject: qapi: Replace qmp_dispatch()'s TODO comment by an explanation Signed-off-by: Markus Armbruster Message-Id: <20200317115459.31821-25-armbru@redhat.com> Reviewed-by: Eric Blake --- qapi/qmp-dispatch.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c index 91e50fa..44fc368 100644 --- a/qapi/qmp-dispatch.c +++ b/qapi/qmp-dispatch.c @@ -162,7 +162,11 @@ QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request, g_assert(!ret); return NULL; } else if (!ret) { - /* TODO turn into assertion */ + /* + * When the command's schema has no 'returns', cmd->fn() + * leaves @ret null. The QMP spec calls for an empty object + * then; supply it. + */ ret = QOBJECT(qdict_new()); } -- cgit v1.1 From f965e8fea6a915343d160ba6043deb75710d8df1 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:50 +0100 Subject: qapi: New special feature flag "deprecated" Unlike regular feature flags, the new special feature flag "deprecated" is recognized by the QAPI generator. For now, it's only permitted with commands, events, and struct members. It will be put to use shortly. Signed-off-by: Markus Armbruster Message-Id: <20200317115459.31821-26-armbru@redhat.com> Reviewed-by: Eric Blake [Doc typo fixed] --- docs/devel/qapi-code-gen.txt | 6 ++++++ scripts/qapi/schema.py | 6 ++++++ tests/Makefile.include | 1 + tests/qapi-schema/features-deprecated-type.err | 2 ++ tests/qapi-schema/features-deprecated-type.json | 3 +++ tests/qapi-schema/features-deprecated-type.out | 0 tests/qapi-schema/qapi-schema-test.json | 6 +++--- tests/qapi-schema/qapi-schema-test.out | 6 +++--- 8 files changed, 24 insertions(+), 6 deletions(-) create mode 100644 tests/qapi-schema/features-deprecated-type.err create mode 100644 tests/qapi-schema/features-deprecated-type.json create mode 100644 tests/qapi-schema/features-deprecated-type.out diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt index 39ae2ca..1967adf 100644 --- a/docs/devel/qapi-code-gen.txt +++ b/docs/devel/qapi-code-gen.txt @@ -683,6 +683,12 @@ Intended use is to have each feature string signal that this build of QEMU shows a certain behaviour. +==== Special features ==== + +Feature "deprecated" marks a command, event, or struct member as +deprecated. It is not supported elsewhere so far. + + === Naming rules and reserved names === All names must begin with a letter, and contain only ASCII letters, diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 6ee3677..78309a00f 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -193,6 +193,12 @@ class QAPISchemaType(QAPISchemaEntity): return None return self.name + def check(self, schema): + QAPISchemaEntity.check(self, schema) + if 'deprecated' in [f.name for f in self.features]: + raise QAPISemError( + self.info, "feature 'deprecated' is not supported for types") + def describe(self): assert self.meta return "%s type '%s'" % (self.meta, self.name) diff --git a/tests/Makefile.include b/tests/Makefile.include index 67e8fcd..d134030 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -242,6 +242,7 @@ qapi-schema += event-case.json qapi-schema += event-member-invalid-dict.json qapi-schema += event-nest-struct.json qapi-schema += features-bad-type.json +qapi-schema += features-deprecated-type.json qapi-schema += features-duplicate-name.json qapi-schema += features-if-invalid.json qapi-schema += features-missing-name.json diff --git a/tests/qapi-schema/features-deprecated-type.err b/tests/qapi-schema/features-deprecated-type.err new file mode 100644 index 0000000..af4ffe2 --- /dev/null +++ b/tests/qapi-schema/features-deprecated-type.err @@ -0,0 +1,2 @@ +features-deprecated-type.json: In struct 'S': +features-deprecated-type.json:2: feature 'deprecated' is not supported for types diff --git a/tests/qapi-schema/features-deprecated-type.json b/tests/qapi-schema/features-deprecated-type.json new file mode 100644 index 0000000..4b5bf5b --- /dev/null +++ b/tests/qapi-schema/features-deprecated-type.json @@ -0,0 +1,3 @@ +# Feature 'deprecated' is not supported for types +{ 'struct': 'S', 'data': {}, + 'features': [ 'deprecated' ] } diff --git a/tests/qapi-schema/features-deprecated-type.out b/tests/qapi-schema/features-deprecated-type.out new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index f576c33..6b1f05a 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -258,7 +258,7 @@ 'data': { 'foo': 'int' }, 'features': [] } { 'struct': 'FeatureStruct1', - 'data': { 'foo': { 'type': 'int', 'features': [ 'member-feature1' ] } }, + 'data': { 'foo': { 'type': 'int', 'features': [ 'deprecated' ] } }, 'features': [ 'feature1' ] } { 'struct': 'FeatureStruct2', 'data': { 'foo': 'int' }, @@ -308,7 +308,7 @@ 'features': [] } { 'command': 'test-command-features1', - 'features': [ 'feature1' ] } + 'features': [ 'deprecated' ] } { 'command': 'test-command-features3', 'features': [ 'feature1', 'feature2' ] } @@ -322,4 +322,4 @@ 'defined(TEST_IF_COND_2)'] } ] } { 'event': 'TEST-EVENT-FEATURES1', - 'features': [ 'feature1' ] } + 'features': [ 'deprecated' ] } diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index cd863ae..891b410 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -359,7 +359,7 @@ object FeatureStruct0 member foo: int optional=False object FeatureStruct1 member foo: int optional=False - feature member-feature1 + feature deprecated feature feature1 object FeatureStruct2 member foo: int optional=False @@ -419,7 +419,7 @@ command test-features0 q_obj_test-features0-arg -> None gen=True success_response=True boxed=False oob=False preconfig=False command test-command-features1 None -> None gen=True success_response=True boxed=False oob=False preconfig=False - feature feature1 + feature deprecated command test-command-features3 None -> None gen=True success_response=True boxed=False oob=False preconfig=False feature feature1 @@ -440,7 +440,7 @@ command test-command-cond-features3 None -> None if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'] event TEST-EVENT-FEATURES1 None boxed=False - feature feature1 + feature deprecated module include/sub-module.json include sub-sub-module.json object SecondArrayRef -- cgit v1.1 From df4097aeaf71e1ff0574222760821467a7c86c0f Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Mar 2020 12:54:51 +0100 Subject: qapi: Mark deprecated QMP parts with feature 'deprecated' Add feature 'deprecated' to the deprecated QMP commands, so their deprecation becomes visible in output of query-qmp-schema. Looks like this: {"name": "query-cpus", "ret-type": "[164]", "meta-type": "command", "arg-type": "0", ---> "features": ["deprecated"]} Management applications could conceivably use this for static checking. The deprecated commands are change, cpu-add, migrate-set-cache-size, migrate_set_downtime, migrate_set_speed, query-cpus, query-events, query-migrate-cache-size. The deprecated command arguments are block-commit arguments @base and @top, and block_set_io_throttle, blockdev-change-medium, blockdev-close-tray, blockdev-open-tray, eject argument @device. The deprecated command results are query-cpus-fast result @arch, query-block result @dirty-bitmaps, query-named-block-nodes result @encryption_key_missing and result @dirty-bitmaps's member @status. Same for query-block result @inserted, which mirrors query-named-block-nodes. Signed-off-by: Markus Armbruster Message-Id: <20200317115459.31821-27-armbru@redhat.com> Reviewed-by: Eric Blake --- qapi/block-core.json | 48 +++++++++++++++++++++++++++++++++++++----------- qapi/block.json | 30 +++++++++++++++++++++--------- qapi/control.json | 11 +++++++---- qapi/machine.json | 34 +++++++++++++++++++--------------- qapi/migration.json | 36 ++++++++++++++++++++++++------------ qapi/misc.json | 13 +++++++------ 6 files changed, 115 insertions(+), 57 deletions(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index 91586fb..943df19 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -297,7 +297,7 @@ # # @encrypted: true if the backing device is encrypted # -# @encryption_key_missing: Deprecated; always false +# @encryption_key_missing: always false # # @detect_zeroes: detect and optimize zero writes (Since 2.1) # @@ -363,13 +363,19 @@ # @dirty-bitmaps: dirty bitmaps information (only present if node # has one or more dirty bitmaps) (Since 4.2) # +# Features: +# @deprecated: Member @encryption_key_missing is deprecated. It is +# always false. +# # Since: 0.14.0 # ## { 'struct': 'BlockDeviceInfo', 'data': { 'file': 'str', '*node-name': 'str', 'ro': 'bool', 'drv': 'str', '*backing_file': 'str', 'backing_file_depth': 'int', - 'encrypted': 'bool', 'encryption_key_missing': 'bool', + 'encrypted': 'bool', + 'encryption_key_missing': { 'type': 'bool', + 'features': [ 'deprecated' ] }, 'detect_zeroes': 'BlockdevDetectZeroesOptions', 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int', @@ -475,7 +481,7 @@ # # @granularity: granularity of the dirty bitmap in bytes (since 1.4) # -# @status: Deprecated in favor of @recording and @locked. (since 2.4) +# @status: current status of the dirty bitmap (since 2.4) # # @recording: true if the bitmap is recording new writes from the guest. # Replaces `active` and `disabled` statuses. (since 4.0) @@ -492,11 +498,17 @@ # @busy to be false. This bitmap cannot be used. To remove # it, use @block-dirty-bitmap-remove. (Since 4.0) # +# Features: +# @deprecated: Member @status is deprecated. Use @recording and +# @locked instead. +# # Since: 1.3 ## { 'struct': 'BlockDirtyInfo', 'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32', - 'recording': 'bool', 'busy': 'bool', 'status': 'DirtyBitmapStatus', + 'recording': 'bool', 'busy': 'bool', + 'status': { 'type': 'DirtyBitmapStatus', + 'features': [ 'deprecated' ] }, 'persistent': 'bool', '*inconsistent': 'bool' } } ## @@ -587,7 +599,6 @@ # # @dirty-bitmaps: dirty bitmaps information (only present if the # driver has one or more dirty bitmaps) (Since 2.0) -# Deprecated in 4.2; see BlockDeviceInfo instead. # # @io-status: @BlockDeviceIoStatus. Only present if the device # supports it and the VM is configured to stop on errors @@ -597,13 +608,18 @@ # @inserted: @BlockDeviceInfo describing the device if media is # present # +# Features: +# @deprecated: Member @dirty-bitmaps is deprecated. Use @inserted +# member @dirty-bitmaps instead. +# # Since: 0.14.0 ## { 'struct': 'BlockInfo', 'data': {'device': 'str', '*qdev': 'str', 'type': 'str', 'removable': 'bool', 'locked': 'bool', '*inserted': 'BlockDeviceInfo', '*tray_open': 'bool', '*io-status': 'BlockDeviceIoStatus', - '*dirty-bitmaps': ['BlockDirtyInfo'] } } + '*dirty-bitmaps': { 'type': ['BlockDirtyInfo'], + 'features': [ 'deprecated' ] } } } ## # @BlockMeasureInfo: @@ -1551,7 +1567,7 @@ # @base: Same as @base-node, except that it is a file name rather than a node # name. This must be the exact filename string that was used to open the # node; other strings, even if addressing the same file, are not -# accepted (deprecated, use @base-node instead) +# accepted # # @top-node: The node name of the backing image within the image chain # which contains the topmost data to be committed down. If @@ -1560,7 +1576,7 @@ # @top: Same as @top-node, except that it is a file name rather than a node # name. This must be the exact filename string that was used to open the # node; other strings, even if addressing the same file, are not -# accepted (deprecated, use @base-node instead) +# accepted # # @backing-file: The backing file string to write into the overlay # image of 'top'. If 'top' is the active layer, @@ -1614,6 +1630,10 @@ # list without user intervention. # Defaults to true. (Since 3.1) # +# Features: +# @deprecated: Members @base and @top are deprecated. Use @base-node +# and @top-node instead. +# # Returns: - Nothing on success # - If @device does not exist, DeviceNotFound # - Any other error returns a GenericError. @@ -1630,7 +1650,9 @@ ## { 'command': 'block-commit', 'data': { '*job-id': 'str', 'device': 'str', '*base-node': 'str', - '*base': 'str', '*top-node': 'str', '*top': 'str', + '*base': { 'type': 'str', 'features': [ 'deprecated' ] }, + '*top-node': 'str', + '*top': { 'type': 'str', 'features': [ 'deprecated' ] }, '*backing-file': 'str', '*speed': 'int', '*on-error': 'BlockdevOnError', '*filter-node-name': 'str', @@ -2296,7 +2318,7 @@ # # A set of parameters describing block throttling. # -# @device: Block device name (deprecated, use @id instead) +# @device: Block device name # # @id: The name or QOM path of the guest device (since: 2.8) # @@ -2364,10 +2386,14 @@ # # @group: throttle group name (Since 2.4) # +# Features: +# @deprecated: Member @device is deprecated. Use @id instead. +# # Since: 1.1 ## { 'struct': 'BlockIOThrottle', - 'data': { '*device': 'str', '*id': 'str', 'bps': 'int', 'bps_rd': 'int', + 'data': { '*device': { 'type': 'str', 'features': [ 'deprecated' ] }, + '*id': 'str', 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int', '*bps_max': 'int', '*bps_rd_max': 'int', '*bps_wr_max': 'int', '*iops_max': 'int', diff --git a/qapi/block.json b/qapi/block.json index 97bf52b..2ddbfa8 100644 --- a/qapi/block.json +++ b/qapi/block.json @@ -90,15 +90,18 @@ ## # @eject: # -# Ejects a device from a removable drive. +# Ejects the medium from a removable drive. # -# @device: Block device name (deprecated, use @id instead) +# @device: Block device name # # @id: The name or QOM path of the guest device (since: 2.8) # # @force: If true, eject regardless of whether the drive is locked. # If not specified, the default value is false. # +# Features: +# @deprecated: Member @device is deprecated. Use @id instead. +# # Returns: - Nothing on success # - If @device is not a valid block device, DeviceNotFound # Notes: Ejecting a device with no media results in success @@ -111,7 +114,7 @@ # <- { "return": {} } ## { 'command': 'eject', - 'data': { '*device': 'str', + 'data': { '*device': { 'type': 'str', 'features': [ 'deprecated' ] }, '*id': 'str', '*force': 'bool' } } @@ -134,7 +137,7 @@ # to it # - if the guest device does not have an actual tray # -# @device: Block device name (deprecated, use @id instead) +# @device: Block device name # # @id: The name or QOM path of the guest device (since: 2.8) # @@ -143,6 +146,9 @@ # immediately); if true, the tray will be opened regardless of whether # it is locked # +# Features: +# @deprecated: Member @device is deprecated. Use @id instead. +# # Since: 2.5 # # Example: @@ -161,7 +167,7 @@ # ## { 'command': 'blockdev-open-tray', - 'data': { '*device': 'str', + 'data': { '*device': { 'type': 'str', 'features': [ 'deprecated' ] }, '*id': 'str', '*force': 'bool' } } @@ -174,10 +180,13 @@ # # If the tray was already closed before, this will be a no-op. # -# @device: Block device name (deprecated, use @id instead) +# @device: Block device name # # @id: The name or QOM path of the guest device (since: 2.8) # +# Features: +# @deprecated: Member @device is deprecated. Use @id instead. +# # Since: 2.5 # # Example: @@ -196,7 +205,7 @@ # ## { 'command': 'blockdev-close-tray', - 'data': { '*device': 'str', + 'data': { '*device': { 'type': 'str', 'features': [ 'deprecated' ] }, '*id': 'str' } } ## @@ -303,7 +312,7 @@ # combines blockdev-open-tray, blockdev-remove-medium, blockdev-insert-medium # and blockdev-close-tray). # -# @device: Block device name (deprecated, use @id instead) +# @device: Block device name # # @id: The name or QOM path of the guest device # (since: 2.8) @@ -316,6 +325,9 @@ # @read-only-mode: change the read-only mode of the device; defaults # to 'retain' # +# Features: +# @deprecated: Member @device is deprecated. Use @id instead. +# # Since: 2.5 # # Examples: @@ -350,7 +362,7 @@ # ## { 'command': 'blockdev-change-medium', - 'data': { '*device': 'str', + 'data': { '*device': { 'type': 'str', 'features': [ 'deprecated' ] }, '*id': 'str', 'filename': 'str', '*format': 'str', diff --git a/qapi/control.json b/qapi/control.json index 85b12fe..6b816bb 100644 --- a/qapi/control.json +++ b/qapi/control.json @@ -174,13 +174,15 @@ # # Return information on QMP events. # +# Features: +# @deprecated: This command is deprecated, because its output doesn't +# reflect compile-time configuration. Use 'query-qmp-schema' +# instead. +# # Returns: A list of @EventInfo. # # Since: 1.2.0 # -# Note: This command is deprecated, because its output doesn't reflect -# compile-time configuration. Use query-qmp-schema instead. -# # Example: # # -> { "execute": "query-events" } @@ -198,7 +200,8 @@ # Note: This example has been shortened as the real response is too long. # ## -{ 'command': 'query-events', 'returns': ['EventInfo'] } +{ 'command': 'query-events', 'returns': ['EventInfo'], + 'features': [ 'deprecated' ] } ## # @quit: diff --git a/qapi/machine.json b/qapi/machine.json index 6c11e3c..0da3f3a 100644 --- a/qapi/machine.json +++ b/qapi/machine.json @@ -184,8 +184,11 @@ # This command causes vCPU threads to exit to userspace, which causes # a small interruption to guest CPU execution. This will have a negative # impact on realtime guests and other latency sensitive guest workloads. -# It is recommended to use @query-cpus-fast instead of this command to -# avoid the vCPU interruption. +# +# Features: +# @deprecated: This command is deprecated, because it interferes with +# the guest. Use 'query-cpus-fast' instead to avoid the vCPU +# interruption. # # Returns: a list of @CpuInfo for each virtual CPU # @@ -216,12 +219,9 @@ # ] # } # -# Notes: This interface is deprecated (since 2.12.0), and it is strongly -# recommended that you avoid using it. Use @query-cpus-fast to -# obtain information about virtual CPUs. -# ## -{ 'command': 'query-cpus', 'returns': ['CpuInfo'] } +{ 'command': 'query-cpus', 'returns': ['CpuInfo'], + 'features': [ 'deprecated' ] } ## # @CpuInfoFast: @@ -237,12 +237,14 @@ # @props: properties describing to which node/socket/core/thread # virtual CPU belongs to, provided if supported by board # -# @arch: base architecture of the cpu; deprecated since 3.0.0 in favor -# of @target +# @arch: base architecture of the cpu # # @target: the QEMU system emulation target, which determines which # additional fields will be listed (since 3.0) # +# Features: +# @deprecated: Member @arch is deprecated. Use @target instead. +# # Since: 2.12 # ## @@ -251,7 +253,8 @@ 'qom-path' : 'str', 'thread-id' : 'int', '*props' : 'CpuInstanceProperties', - 'arch' : 'CpuInfoArch', + 'arch' : { 'type': 'CpuInfoArch', + 'features': [ 'deprecated' ] }, 'target' : 'SysEmuTarget' }, 'discriminator' : 'target', 'data' : { 's390x' : 'CpuInfoS390' } } @@ -307,21 +310,22 @@ # # @id: ID of CPU to be created, valid values [0..max_cpus) # +# Features: +# @deprecated: This command is deprecated. Use `device_add` instead. +# See the `query-hotpluggable-cpus` command for details. +# # Returns: Nothing on success # # Since: 1.5 # -# Note: This command is deprecated. The `device_add` command should be -# used instead. See the `query-hotpluggable-cpus` command for -# details. -# # Example: # # -> { "execute": "cpu-add", "arguments": { "id": 2 } } # <- { "return": {} } # ## -{ 'command': 'cpu-add', 'data': {'id': 'int'} } +{ 'command': 'cpu-add', 'data': {'id': 'int'}, + 'features': [ 'deprecated' ] } ## # @MachineInfo: diff --git a/qapi/migration.json b/qapi/migration.json index 0d1c071..eca2981 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -1210,9 +1210,11 @@ # # @value: maximum downtime in seconds # -# Returns: nothing on success +# Features: +# @deprecated: This command is deprecated. Use +# 'migrate-set-parameters' instead. # -# Notes: This command is deprecated in favor of 'migrate-set-parameters' +# Returns: nothing on success # # Since: 0.14.0 # @@ -1222,7 +1224,8 @@ # <- { "return": {} } # ## -{ 'command': 'migrate_set_downtime', 'data': {'value': 'number'} } +{ 'command': 'migrate_set_downtime', 'data': {'value': 'number'}, + 'features': [ 'deprecated' ] } ## # @migrate_set_speed: @@ -1231,9 +1234,11 @@ # # @value: maximum speed in bytes per second. # -# Returns: nothing on success +# Features: +# @deprecated: This command is deprecated. Use +# 'migrate-set-parameters' instead. # -# Notes: This command is deprecated in favor of 'migrate-set-parameters' +# Returns: nothing on success # # Since: 0.14.0 # @@ -1243,7 +1248,8 @@ # <- { "return": {} } # ## -{ 'command': 'migrate_set_speed', 'data': {'value': 'int'} } +{ 'command': 'migrate_set_speed', 'data': {'value': 'int'}, + 'features': [ 'deprecated' ] } ## # @migrate-set-cache-size: @@ -1252,13 +1258,15 @@ # # @value: cache size in bytes # +# Features: +# @deprecated: This command is deprecated. Use +# 'migrate-set-parameters' instead. +# # The size will be rounded down to the nearest power of 2. # The cache size can be modified before and during ongoing migration # # Returns: nothing on success # -# Notes: This command is deprecated in favor of 'migrate-set-parameters' -# # Since: 1.2 # # Example: @@ -1268,16 +1276,19 @@ # <- { "return": {} } # ## -{ 'command': 'migrate-set-cache-size', 'data': {'value': 'int'} } +{ 'command': 'migrate-set-cache-size', 'data': {'value': 'int'}, + 'features': [ 'deprecated' ] } ## # @query-migrate-cache-size: # # Query migration XBZRLE cache size # -# Returns: XBZRLE cache size in bytes +# Features: +# @deprecated: This command is deprecated. Use +# 'query-migrate-parameters' instead. # -# Notes: This command is deprecated in favor of 'query-migrate-parameters' +# Returns: XBZRLE cache size in bytes # # Since: 1.2 # @@ -1287,7 +1298,8 @@ # <- { "return": 67108864 } # ## -{ 'command': 'query-migrate-cache-size', 'returns': 'int' } +{ 'command': 'query-migrate-cache-size', 'returns': 'int', + 'features': [ 'deprecated' ] } ## # @migrate: diff --git a/qapi/misc.json b/qapi/misc.json index c18fe68..99b90ac 100644 --- a/qapi/misc.json +++ b/qapi/misc.json @@ -872,14 +872,14 @@ # If @device is 'vnc' and @target is 'password', this is the new VNC # password to set. See change-vnc-password for additional notes. # +# Features: +# @deprecated: This command is deprecated. For changing block +# devices, use 'blockdev-change-medium' instead; for changing VNC +# parameters, use 'change-vnc-password' instead. +# # Returns: - Nothing on success. # - If @device is not a valid block device, DeviceNotFound # -# Notes: This interface is deprecated, and it is strongly recommended that you -# avoid using it. For changing block devices, use -# blockdev-change-medium; for changing VNC parameters, use -# change-vnc-password. -# # Since: 0.14.0 # # Example: @@ -900,7 +900,8 @@ # ## { 'command': 'change', - 'data': {'device': 'str', 'target': 'str', '*arg': 'str'} } + 'data': {'device': 'str', 'target': 'str', '*arg': 'str'}, + 'features': [ 'deprecated' ] } ## # @xen-set-global-dirty-log: -- cgit v1.1 From f0ccc00be16e6f4c925f90305e2d4ed9dd11c8fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Mon, 16 Mar 2020 18:18:24 +0100 Subject: qmp: constify QmpCommand and list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since 0b69f6f72ce47a37a749b056b6d5ec64c61f11e8 "qapi: remove qmp_unregister_command()", the command list can be declared const. Signed-off-by: Marc-André Lureau Reviewed-by: Damien Hedde Message-Id: <20200316171824.2319695-1-marcandre.lureau@redhat.com> [Rebased] Signed-off-by: Markus Armbruster --- include/qapi/qmp/dispatch.h | 9 +++++---- monitor/monitor-internal.h | 2 +- monitor/qmp-cmds-control.c | 2 +- qapi/qmp-dispatch.c | 4 ++-- qapi/qmp-registry.c | 6 +++--- qga/commands.c | 2 +- qga/main.c | 6 +++--- 7 files changed, 16 insertions(+), 15 deletions(-) diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h index 9aa426a..5a9cf82 100644 --- a/include/qapi/qmp/dispatch.h +++ b/include/qapi/qmp/dispatch.h @@ -39,7 +39,8 @@ typedef QTAILQ_HEAD(QmpCommandList, QmpCommand) QmpCommandList; void qmp_register_command(QmpCommandList *cmds, const char *name, QmpCommandFunc *fn, QmpCommandOptions options); -QmpCommand *qmp_find_command(QmpCommandList *cmds, const char *name); +const QmpCommand *qmp_find_command(const QmpCommandList *cmds, + const char *name); void qmp_disable_command(QmpCommandList *cmds, const char *name); void qmp_enable_command(QmpCommandList *cmds, const char *name); @@ -47,13 +48,13 @@ bool qmp_command_is_enabled(const QmpCommand *cmd); const char *qmp_command_name(const QmpCommand *cmd); bool qmp_has_success_response(const QmpCommand *cmd); QDict *qmp_error_response(Error *err); -QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request, +QDict *qmp_dispatch(const QmpCommandList *cmds, QObject *request, bool allow_oob); bool qmp_is_oob(const QDict *dict); -typedef void (*qmp_cmd_callback_fn)(QmpCommand *cmd, void *opaque); +typedef void (*qmp_cmd_callback_fn)(const QmpCommand *cmd, void *opaque); -void qmp_for_each_command(QmpCommandList *cmds, qmp_cmd_callback_fn fn, +void qmp_for_each_command(const QmpCommandList *cmds, qmp_cmd_callback_fn fn, void *opaque); #endif diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h index 3e6baba..8f60ccc 100644 --- a/monitor/monitor-internal.h +++ b/monitor/monitor-internal.h @@ -133,7 +133,7 @@ typedef struct { * qmp_capabilities succeeds, we go into command mode, and * @command becomes &qmp_commands. */ - QmpCommandList *commands; + const QmpCommandList *commands; bool capab_offered[QMP_CAPABILITY__MAX]; /* capabilities offered */ bool capab[QMP_CAPABILITY__MAX]; /* offered and accepted */ /* diff --git a/monitor/qmp-cmds-control.c b/monitor/qmp-cmds-control.c index 5cd9bb8..8f04cfa 100644 --- a/monitor/qmp-cmds-control.c +++ b/monitor/qmp-cmds-control.c @@ -101,7 +101,7 @@ VersionInfo *qmp_query_version(Error **errp) return info; } -static void query_commands_cb(QmpCommand *cmd, void *opaque) +static void query_commands_cb(const QmpCommand *cmd, void *opaque) { CommandInfoList *info, **list = opaque; diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c index 44fc368..c30c7ff 100644 --- a/qapi/qmp-dispatch.c +++ b/qapi/qmp-dispatch.c @@ -88,14 +88,14 @@ bool qmp_is_oob(const QDict *dict) && !qdict_haskey(dict, "execute"); } -QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request, +QDict *qmp_dispatch(const QmpCommandList *cmds, QObject *request, bool allow_oob) { Error *err = NULL; bool oob; const char *command; QDict *args; - QmpCommand *cmd; + const QmpCommand *cmd; QDict *dict; QObject *id; QObject *ret = NULL; diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c index ca00f74..d0f9a1d 100644 --- a/qapi/qmp-registry.c +++ b/qapi/qmp-registry.c @@ -27,7 +27,7 @@ void qmp_register_command(QmpCommandList *cmds, const char *name, QTAILQ_INSERT_TAIL(cmds, cmd, node); } -QmpCommand *qmp_find_command(QmpCommandList *cmds, const char *name) +const QmpCommand *qmp_find_command(const QmpCommandList *cmds, const char *name) { QmpCommand *cmd; @@ -77,10 +77,10 @@ bool qmp_has_success_response(const QmpCommand *cmd) return !(cmd->options & QCO_NO_SUCCESS_RESP); } -void qmp_for_each_command(QmpCommandList *cmds, qmp_cmd_callback_fn fn, +void qmp_for_each_command(const QmpCommandList *cmds, qmp_cmd_callback_fn fn, void *opaque) { - QmpCommand *cmd; + const QmpCommand *cmd; QTAILQ_FOREACH(cmd, cmds, node) { fn(cmd, opaque); diff --git a/qga/commands.c b/qga/commands.c index 43c323c..f8852be 100644 --- a/qga/commands.c +++ b/qga/commands.c @@ -54,7 +54,7 @@ void qmp_guest_ping(Error **errp) slog("guest-ping called"); } -static void qmp_command_info(QmpCommand *cmd, void *opaque) +static void qmp_command_info(const QmpCommand *cmd, void *opaque) { GuestAgentInfo *info = opaque; GuestAgentCommandInfo *cmd_info; diff --git a/qga/main.c b/qga/main.c index e5c39c1..8ee2736 100644 --- a/qga/main.c +++ b/qga/main.c @@ -359,7 +359,7 @@ static gint ga_strcmp(gconstpointer str1, gconstpointer str2) } /* disable commands that aren't safe for fsfreeze */ -static void ga_disable_non_whitelisted(QmpCommand *cmd, void *opaque) +static void ga_disable_non_whitelisted(const QmpCommand *cmd, void *opaque) { bool whitelisted = false; int i = 0; @@ -378,7 +378,7 @@ static void ga_disable_non_whitelisted(QmpCommand *cmd, void *opaque) } /* [re-]enable all commands, except those explicitly blacklisted by user */ -static void ga_enable_non_blacklisted(QmpCommand *cmd, void *opaque) +static void ga_enable_non_blacklisted(const QmpCommand *cmd, void *opaque) { GList *blacklist = opaque; const char *name = qmp_command_name(cmd); @@ -918,7 +918,7 @@ int64_t ga_get_fd_handle(GAState *s, Error **errp) return handle; } -static void ga_print_cmd(QmpCommand *cmd, void *opaque) +static void ga_print_cmd(const QmpCommand *cmd, void *opaque) { printf("%s\n", qmp_command_name(cmd)); } -- cgit v1.1 From db2a380c84574d8c76d7193b8af8535234fe5156 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 17 Mar 2020 15:17:10 -0500 Subject: net: Complete qapi-fication of netdev_add We've had all the required pieces for doing a type-safe representation of netdev_add as a flat union for quite some time now (since 0e55c381f6 in v2.7.0, released in 2016), but did not make the final switch to using it because of concern about whether a command-line regression in accepting "1" in place of 1 for integer arguments would be problematic. Back then, we did not have the deprecation cycle to allow us to make progress. But now that we have waited so long, other problems have crept in: for example, our desire to add qemu-storage-daemon is hampered by the inability to express net objects, and we are unable to introspect what we actually accept. Additionally, our round-trip through QemuOpts silently eats any argument that expands to an array, rendering dnssearch, hostfwd, and guestfwd useless through QMP: {"execute": "netdev_add", "arguments": { "id": "netdev0", "type": "user", "dnssearch": [ { "str": "8.8.8.8" }, { "str": "8.8.4.4" } ]}} So without further ado, let's turn on proper QAPI. netdev_add() was a trivial wrapper around net_client_init(), which did a few steps prior to calling net_client_init1(); with this patch, we now skip directly to net_client_init1(). In addition to fixing array parameters, the following additional differences occur: - {"execute": "netdev_add", "arguments": {"type": "help"}} no longer attempts to print help to stdout and exit. Bug fix, broken in 547203ead4 'net: List available netdevs with "-netdev help"', v2.12.0. - {"execute": "netdev_add", "arguments': {... "ipv6-net": "..." }} no longer attempts to desugar the undocumented ipv6-net magic string into the proper "ipv6-prefix" and "ipv6-prefixlen". Undocumented misfeature, introduced in commit 7aac531ef2 "qapi-schema, qemu-options & slirp: Adding Qemu options for IPv6 addresses", v2.6.0. - {'execute':'netdev_add', 'arguments':{'id':'net2', 'type':'hubport', 'hubid':"2"}} {"error": {"class": "GenericError", "desc": "Invalid parameter type for 'hubid', expected: integer"}} Used to succeed: since our command line treats everything as strings, our not-so-round-trip conversion from QAPI -> QemuOpts -> QAPI lost the original typing and turned everything into a string; now that we skip the QemuOpts, the JSON input has to match the exact QAPI type. But this stricter QMP is desirable, and introspection is sufficient for any affected applications to make sure they use it correctly. In qmp_netdev_add(), we still have to create a QemuOpts object so that qmp_netdev_del() will be able to remove a hotplugged network device; but the opts->head remains empty since we now manage all parsing through the QAPI object rather than QemuOpts; a separate patch will address the abuse of QemuOpts as a witness for whether a NetClientState is a netdev. In the meantime, our argument that we are okay requires auditing all uses of option group "netdev": - qemu_netdev_opts: option group definition, empty .desc[] - CLI (CLI netdev parsing ends before monitors start, so while monitors can mess with CLI netdevs, CLI cannot mess with monitor netdevs): - main() case QEMU_OPTION_netdev: store CLI definition - main() case QEMU_OPTION_readconfig, case QEMU_OPTION_writeconfig: similar, dealing only with CLI - net_init_clients(): Pass CLI to net_client_init() - Monitor: - hmp_netdev_add(): straightforward parse into net_client_init() - qmp_netdev_add(): subject of this patch, used to add full object to option group, now just adds bare-bones id - qmp_netdev_del(), netdev_del_completion(): check the option group solely for id, as a 'is this a netdev' predicate Reported-by: Alex Kirillov Signed-off-by: Eric Blake Message-Id: <20200317201711.322764-2-eblake@redhat.com> Reviewed-by: Markus Armbruster [Commit message typo fixed] Signed-off-by: Markus Armbruster --- include/net/net.h | 1 - monitor/misc.c | 2 -- net/net.c | 6 +++--- qapi/net.json | 14 +------------- 4 files changed, 4 insertions(+), 19 deletions(-) diff --git a/include/net/net.h b/include/net/net.h index e175ba9..96e6eae 100644 --- a/include/net/net.h +++ b/include/net/net.h @@ -203,7 +203,6 @@ void net_cleanup(void); void hmp_host_net_add(Monitor *mon, const QDict *qdict); void hmp_host_net_remove(Monitor *mon, const QDict *qdict); void netdev_add(QemuOpts *opts, Error **errp); -void qmp_netdev_add(QDict *qdict, QObject **ret, Error **errp); int net_hub_id_for_client(NetClientState *nc, int *id); NetClientState *net_hub_port_find(int hub_id); diff --git a/monitor/misc.c b/monitor/misc.c index c3bc34c..41a86e7 100644 --- a/monitor/misc.c +++ b/monitor/misc.c @@ -247,8 +247,6 @@ static void monitor_init_qmp_commands(void) qmp_query_qmp_schema, QCO_ALLOW_PRECONFIG); qmp_register_command(&qmp_commands, "device_add", qmp_device_add, QCO_NO_OPTIONS); - qmp_register_command(&qmp_commands, "netdev_add", qmp_netdev_add, - QCO_NO_OPTIONS); qmp_register_command(&qmp_commands, "object-add", qmp_object_add, QCO_NO_OPTIONS); diff --git a/net/net.c b/net/net.c index 9e93c3f..a2065aa 100644 --- a/net/net.c +++ b/net/net.c @@ -1170,7 +1170,7 @@ void netdev_add(QemuOpts *opts, Error **errp) net_client_init(opts, true, errp); } -void qmp_netdev_add(QDict *qdict, QObject **ret, Error **errp) +void qmp_netdev_add(Netdev *netdev, Error **errp) { Error *local_err = NULL; QemuOptsList *opts_list; @@ -1181,12 +1181,12 @@ void qmp_netdev_add(QDict *qdict, QObject **ret, Error **errp) goto out; } - opts = qemu_opts_from_qdict(opts_list, qdict, &local_err); + opts = qemu_opts_create(opts_list, netdev->id, 1, &local_err); if (local_err) { goto out; } - netdev_add(opts, &local_err); + net_client_init1(netdev, true, &local_err); if (local_err) { qemu_opts_del(opts); goto out; diff --git a/qapi/net.json b/qapi/net.json index 1cb9a7d..cebb1b5 100644 --- a/qapi/net.json +++ b/qapi/net.json @@ -39,18 +39,8 @@ # # Add a network backend. # -# @type: the type of network backend. Possible values are listed in -# NetClientDriver (excluding 'none' and 'nic') -# -# @id: the name of the new network backend -# # Additional arguments depend on the type. # -# TODO: This command effectively bypasses QAPI completely due to its -# "additional arguments" business. It shouldn't have been added to -# the schema in this form. It should be qapified properly, or -# replaced by a properly qapified command. -# # Since: 0.14.0 # # Returns: Nothing on success @@ -64,9 +54,7 @@ # <- { "return": {} } # ## -{ 'command': 'netdev_add', - 'data': {'type': 'str', 'id': 'str'}, - 'gen': false } # so we can get the additional arguments +{ 'command': 'netdev_add', 'data': 'Netdev', 'boxed': true } ## # @netdev_del: -- cgit v1.1 From 08712fcb851034228b61f75bd922863a984a4f60 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 17 Mar 2020 15:17:11 -0500 Subject: net: Track netdevs in NetClientState rather than QemuOpt As mentioned in the previous patch, our use of QemuOpt group "netdev" has two purposes: collect the CLI arguments, and serve as a witness for monitor hotplug actions. As the latter didn't use anything but an id, it felt rather unclean to have to touch QemuOpts at all when going through QMP, so let's instead track things with a bool field in NetClientState. Suggested-by: Markus Armbruster Signed-off-by: Eric Blake Message-Id: <20200317201711.322764-3-eblake@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- include/net/net.h | 1 + monitor/misc.c | 4 +--- net/net.c | 37 +++++++++++-------------------------- 3 files changed, 13 insertions(+), 29 deletions(-) diff --git a/include/net/net.h b/include/net/net.h index 96e6eae..094e966 100644 --- a/include/net/net.h +++ b/include/net/net.h @@ -98,6 +98,7 @@ struct NetClientState { unsigned rxfilter_notify_enabled:1; int vring_enable; int vnet_hdr_len; + bool is_netdev; QTAILQ_HEAD(, NetFilterState) filters; }; diff --git a/monitor/misc.c b/monitor/misc.c index 41a86e7..6c45fa4 100644 --- a/monitor/misc.c +++ b/monitor/misc.c @@ -2035,13 +2035,11 @@ void netdev_del_completion(ReadLineState *rs, int nb_args, const char *str) count = qemu_find_net_clients_except(NULL, ncs, NET_CLIENT_DRIVER_NIC, MAX_QUEUE_NUM); for (i = 0; i < MIN(count, MAX_QUEUE_NUM); i++) { - QemuOpts *opts; const char *name = ncs[i]->name; if (strncmp(str, name, len)) { continue; } - opts = qemu_opts_find(qemu_find_opts_err("netdev", NULL), name); - if (opts) { + if (ncs[i]->is_netdev) { readline_add_completion(rs, name); } } diff --git a/net/net.c b/net/net.c index a2065aa..38778e8 100644 --- a/net/net.c +++ b/net/net.c @@ -1060,6 +1060,15 @@ static int net_client_init1(const void *object, bool is_netdev, Error **errp) } return -1; } + + if (is_netdev) { + NetClientState *nc; + + nc = qemu_find_netdev(netdev->id); + assert(nc); + nc->is_netdev = true; + } + return 0; } @@ -1172,34 +1181,12 @@ void netdev_add(QemuOpts *opts, Error **errp) void qmp_netdev_add(Netdev *netdev, Error **errp) { - Error *local_err = NULL; - QemuOptsList *opts_list; - QemuOpts *opts; - - opts_list = qemu_find_opts_err("netdev", &local_err); - if (local_err) { - goto out; - } - - opts = qemu_opts_create(opts_list, netdev->id, 1, &local_err); - if (local_err) { - goto out; - } - - net_client_init1(netdev, true, &local_err); - if (local_err) { - qemu_opts_del(opts); - goto out; - } - -out: - error_propagate(errp, local_err); + net_client_init1(netdev, true, errp); } void qmp_netdev_del(const char *id, Error **errp) { NetClientState *nc; - QemuOpts *opts; nc = qemu_find_netdev(id); if (!nc) { @@ -1208,14 +1195,12 @@ void qmp_netdev_del(const char *id, Error **errp) return; } - opts = qemu_opts_find(qemu_find_opts_err("netdev", NULL), id); - if (!opts) { + if (!nc->is_netdev) { error_setg(errp, "Device '%s' is not a netdev", id); return; } qemu_del_net_client(nc); - qemu_opts_del(opts); } static void netfilter_print_info(Monitor *mon, NetFilterState *nf) -- cgit v1.1