* [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces
@ 2019-10-24 12:34 Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 01/19] tests/test-qmp-cmds: Factor out qmp_dispatch() test helpers Markus Armbruster
` (18 more replies)
0 siblings, 19 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
This series is RFC because it's incomplete, and the warning idea in
PATCH 18 is half-baked. I'm soliciting feed back from the management
application crowd: is this going into a useful direction?
The series adresses only deprecated commands and events. Good enough
to demonstrate the ideas, I think. A complete solution should
additionally cover arguments and return values. Feels feasible to me.
New option -compat deprecated-input=<in-policy>,deprecated-output=out-policy
configures the policy. Available input policies:
* accept: Accept deprecated commands with a warning (default)
* reject: Reject deprecated commands
* crash: Crash on deprecated command
Available output policies:
* accept: Emit deprecated events (default)
* hide: Suppress deprecated events
See also last item of
Subject: Minutes of KVM Forum BoF on deprecating stuff
Date: Fri, 26 Oct 2018 16:03:51 +0200
Message-ID: <87mur0ls8o.fsf@dusky.pond.sub.org>
https://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg05828.html
Markus Armbruster (19):
tests/test-qmp-cmds: Factor out qmp_dispatch() test helpers
tests/test-qmp-cmds: Check responses more thoroughly
tests/test-qmp-cmds: Simplify test data setup
tests/test-qmp-event: Simplify test data setup
tests/test-qmp-event: Use qobject_is_equal()
tests/test-qmp-event: Check event is actually emitted
qapi: Add feature flags to remaining definitions
qapi: Consistently put @features parameter right after @ifcond
qapi: Inline do_qmp_dispatch() into qmp_dispatch()
qapi: Simplify how qmp_dispatch() deals with QCO_NO_SUCCESS_RESP
qapi: Simplify how qmp_dispatch() gets the request ID
qapi: Replace qmp_dispatch()'s TODO comment by an explanation
qapi: New special feature flag "deprecated"
qemu-options: New -compat to set policy for "funny" interfaces
qapi: Mark deprecated QMP commands with feature 'deprecated'
qapi: Implement -compat deprecated-input=reject for commands
qapi: Implement -compat deprecated-input=crash for commands
qapi: Include a warning in the response to a deprecated command
qapi: Implement -compat deprecated-output=hide for events
docs/devel/qapi-code-gen.txt | 21 +-
tests/qapi-schema/doc-good.texi | 32 ++-
qapi/common.json | 48 ++++
qapi/introspect.json | 28 ++-
qapi/machine.json | 24 +-
qapi/migration.json | 36 ++-
qapi/misc.json | 25 +-
include/qapi/compat-policy.h | 20 ++
include/qapi/qmp/dispatch.h | 1 +
qapi/qmp-dispatch.c | 140 +++++++-----
tests/test-qmp-cmds.c | 216 +++++++++++-------
tests/test-qmp-event.c | 181 ++++-----------
vl.c | 17 ++
qemu-options.hx | 24 ++
scripts/qapi/commands.py | 16 +-
scripts/qapi/doc.py | 16 +-
scripts/qapi/events.py | 16 +-
scripts/qapi/expr.py | 11 +-
scripts/qapi/introspect.py | 41 ++--
scripts/qapi/schema.py | 138 ++++++-----
scripts/qapi/types.py | 8 +-
scripts/qapi/visit.py | 8 +-
tests/Makefile.include | 1 +
tests/qapi-schema/alternate-base.err | 2 +-
tests/qapi-schema/doc-good.json | 18 +-
tests/qapi-schema/doc-good.out | 20 +-
.../qapi-schema/features-deprecated-type.err | 2 +
.../qapi-schema/features-deprecated-type.json | 3 +
.../qapi-schema/features-deprecated-type.out | 0
tests/qapi-schema/qapi-schema-test.json | 31 ++-
tests/qapi-schema/qapi-schema-test.out | 29 ++-
tests/qapi-schema/test-qapi.py | 19 +-
32 files changed, 731 insertions(+), 461 deletions(-)
create mode 100644 include/qapi/compat-policy.h
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
--
2.21.0
^ permalink raw reply [flat|nested] 24+ messages in thread
* [RFC PATCH 01/19] tests/test-qmp-cmds: Factor out qmp_dispatch() test helpers
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
@ 2019-10-24 12:34 ` Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 02/19] tests/test-qmp-cmds: Check responses more thoroughly Markus Armbruster
` (17 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
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 <armbru@redhat.com>
---
tests/test-qmp-cmds.c | 72 +++++++++++++++++++++----------------------
1 file changed, 35 insertions(+), 37 deletions(-)
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index 27b0afe55a..e738bead86 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -144,34 +144,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);
}
@@ -180,15 +201,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 */
@@ -198,11 +215,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);
}
@@ -217,20 +231,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)
{
@@ -253,7 +253,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");
@@ -274,12 +274,10 @@ 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);
-
- qobject_unref(req);
}
/* test generated dealloc functions for generated types */
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [RFC PATCH 02/19] tests/test-qmp-cmds: Check responses more thoroughly
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 01/19] tests/test-qmp-cmds: Factor out qmp_dispatch() test helpers Markus Armbruster
@ 2019-10-24 12:34 ` Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 03/19] tests/test-qmp-cmds: Simplify test data setup Markus Armbruster
` (16 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
Signed-off-by: Markus Armbruster <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 e738bead86..667e03cb1b 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -150,9 +150,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);
@@ -162,9 +163,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);
}
@@ -173,11 +182,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);
@@ -186,11 +196,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);
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [RFC PATCH 03/19] tests/test-qmp-cmds: Simplify test data setup
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 01/19] tests/test-qmp-cmds: Factor out qmp_dispatch() test helpers Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 02/19] tests/test-qmp-cmds: Check responses more thoroughly Markus Armbruster
@ 2019-10-24 12:34 ` Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 04/19] tests/test-qmp-event: " Markus Armbruster
` (15 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
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 <armbru@redhat.com>
---
tests/test-qmp-cmds.c | 93 ++++++++++++++++++-------------------------
1 file changed, 38 insertions(+), 55 deletions(-)
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index 667e03cb1b..3798ba1b16 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"
@@ -144,11 +145,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");
@@ -157,14 +163,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");
@@ -176,59 +189,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)
@@ -245,26 +242,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");
@@ -281,11 +267,8 @@ 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);
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [RFC PATCH 04/19] tests/test-qmp-event: Simplify test data setup
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
` (2 preceding siblings ...)
2019-10-24 12:34 ` [RFC PATCH 03/19] tests/test-qmp-cmds: Simplify test data setup Markus Armbruster
@ 2019-10-24 12:34 ` Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 05/19] tests/test-qmp-event: Use qobject_is_equal() Markus Armbruster
` (14 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
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 <armbru@redhat.com>
---
tests/test-qmp-event.c | 93 ++++++++++++------------------------------
1 file changed, 27 insertions(+), 66 deletions(-)
diff --git a/tests/test-qmp-event.c b/tests/test-qmp-event.c
index eee7e08ab6..430001e622 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)
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [RFC PATCH 05/19] tests/test-qmp-event: Use qobject_is_equal()
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
` (3 preceding siblings ...)
2019-10-24 12:34 ` [RFC PATCH 04/19] tests/test-qmp-event: " Markus Armbruster
@ 2019-10-24 12:34 ` Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 06/19] tests/test-qmp-event: Check event is actually emitted Markus Armbruster
` (13 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
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 <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 430001e622..d64066139c 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)));
}
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [RFC PATCH 06/19] tests/test-qmp-event: Check event is actually emitted
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
` (4 preceding siblings ...)
2019-10-24 12:34 ` [RFC PATCH 05/19] tests/test-qmp-event: Use qobject_is_equal() Markus Armbruster
@ 2019-10-24 12:34 ` Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 07/19] qapi: Add feature flags to remaining definitions Markus Armbruster
` (12 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
Signed-off-by: Markus Armbruster <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 d64066139c..7dd0053190 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);
}
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [RFC PATCH 07/19] qapi: Add feature flags to remaining definitions
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
` (5 preceding siblings ...)
2019-10-24 12:34 ` [RFC PATCH 06/19] tests/test-qmp-event: Check event is actually emitted Markus Armbruster
@ 2019-10-24 12:34 ` Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 08/19] qapi: Consistently put @features parameter right after @ifcond Markus Armbruster
` (11 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
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"). We
just 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 <armbru@redhat.com>
---
docs/devel/qapi-code-gen.txt | 15 ++--
tests/qapi-schema/doc-good.texi | 32 ++++++++-
qapi/introspect.json | 28 +++++---
tests/test-qmp-cmds.c | 6 +-
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 | 18 ++++-
tests/qapi-schema/doc-good.out | 20 +++++-
tests/qapi-schema/qapi-schema-test.json | 29 ++++++--
tests/qapi-schema/qapi-schema-test.out | 27 +++++--
tests/qapi-schema/test-qapi.py | 9 ++-
17 files changed, 226 insertions(+), 114 deletions(-)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 45c93a43cc..eaeedc7bd3 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 }
@@ -279,12 +280,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 }
@@ -397,7 +400,8 @@ the schema" below for more on this.
Syntax:
ALTERNATE = { 'alternate': STRING,
'data': ALTERNATIVES,
- '*if': COND }
+ '*if': COND,
+ '*features': FEATURES }
ALTERNATIVES = { ALTERNATIVE, ... }
ALTERNATIVE = STRING : TYPE-REF
| STRING : { 'type': STRING, '*if': COND }
@@ -595,7 +599,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.
diff --git a/tests/qapi-schema/doc-good.texi b/tests/qapi-schema/doc-good.texi
index d4b15dabf0..5ef7fea436 100644
--- a/tests/qapi-schema/doc-good.texi
+++ b/tests/qapi-schema/doc-good.texi
@@ -84,11 +84,17 @@ Examples:
@table @asis
@item @code{one}
The @emph{one} @{and only@}
+@code{two} is undocumented
@*@b{If:} @code{defined(IFONE)}
@item @code{two}
Not documented
@end table
-@code{two} is undocumented
+
+@b{Features:}
+@table @asis
+@item @code{enum-feat}
+Also @emph{one} @{and only@}
+@end table
@b{If:} @code{defined(IFCOND)}
@end deftp
@@ -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/qapi/introspect.json b/qapi/introspect.json
index 031a954fa9..7322ab3f59 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -105,6 +105,17 @@
'command': 'SchemaInfoCommand',
'event': 'SchemaInfoEvent' } }
+##
+# @SchemaInfoFeatures:
+#
+# @features: names of features associated with the entity, in no particular
+# order.
+#
+# Since: 4.2
+##
+{ 'struct': 'SchemaInfoFeatures',
+ 'data': { '*features': [ 'str' ] } }
+
##
# @SchemaInfoBuiltin:
#
@@ -142,6 +153,7 @@
# Since: 2.5
##
{ 'struct': 'SchemaInfoEnum',
+ 'base': 'SchemaInfoFeatures',
'data': { 'values': ['str'] } }
##
@@ -174,18 +186,15 @@
# 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
##
{ 'struct': 'SchemaInfoObject',
+ 'base': 'SchemaInfoFeatures',
'data': { 'members': [ 'SchemaInfoObjectMember' ],
'*tag': 'str',
- '*variants': [ 'SchemaInfoObjectVariant' ],
- '*features': [ 'str' ] } }
+ '*variants': [ 'SchemaInfoObjectVariant' ] } }
##
# @SchemaInfoObjectMember:
@@ -239,6 +248,7 @@
# Since: 2.5
##
{ 'struct': 'SchemaInfoAlternate',
+ 'base': 'SchemaInfoFeatures',
'data': { 'members': [ 'SchemaInfoAlternateMember' ] } }
##
@@ -266,17 +276,14 @@
# @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',
+ 'base': 'SchemaInfoFeatures',
'data': { 'arg-type': 'str', 'ret-type': 'str',
- '*allow-oob': 'bool',
- '*features': [ 'str' ] } }
+ '*allow-oob': 'bool' } }
##
# @SchemaInfoEvent:
@@ -289,4 +296,5 @@
# Since: 2.5
##
{ 'struct': 'SchemaInfoEvent',
+ 'base': 'SchemaInfoFeatures',
'data': { 'arg-type': 'str' } }
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index 3798ba1b16..cf4fa1a091 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -44,7 +44,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,
@@ -52,10 +52,6 @@ void qmp_test_features(FeatureStruct0 *fs0, FeatureStruct1 *fs1,
{
}
-void qmp_test_command_features0(Error **errp)
-{
-}
-
void qmp_test_command_features1(Error **errp)
{
}
diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 6f1c17f71f..53a1f8e952 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -244,7 +244,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',
@@ -258,7 +258,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')))
@@ -271,7 +271,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 10fc509fa9..f64e61076e 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 d7a289eded..92b2407315 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -220,7 +220,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):
@@ -268,7 +267,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):
@@ -320,18 +318,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':
@@ -349,13 +347,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 b3a463dd8b..ba493977cf 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -146,7 +146,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:
@@ -156,6 +156,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:
@@ -180,18 +182,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):
@@ -199,16 +201,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,
@@ -219,16 +220,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 cf0045f34e..f13f442896 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -109,7 +109,7 @@ class QAPISchemaVisitor(object):
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(object):
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(object):
features):
pass
- def visit_event(self, name, info, ifcond, arg_type, boxed):
+ def visit_event(self, name, info, ifcond, features, arg_type, boxed):
pass
@@ -220,8 +220,8 @@ class QAPISchemaBuiltinType(QAPISchemaType):
class QAPISchemaEnumType(QAPISchemaType):
meta = 'enum'
- def __init__(self, name, info, doc, ifcond, members, prefix):
- QAPISchemaType.__init__(self, name, info, doc, ifcond)
+ def __init__(self, name, info, doc, ifcond, features, members, prefix):
+ QAPISchemaType.__init__(self, name, info, doc, ifcond, features)
for m in members:
assert isinstance(m, QAPISchemaEnumMember)
m.set_defined_in(name)
@@ -256,15 +256,16 @@ class QAPISchemaEnumType(QAPISchemaType):
def visit(self, visitor):
QAPISchemaType.visit(self, 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):
- QAPISchemaType.__init__(self, name, info, None, None)
+ QAPISchemaType.__init__(self, name, info, None)
assert isinstance(element_type, str)
self._element_type_name = element_type
self.element_type = None
@@ -312,8 +313,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
@@ -609,8 +610,8 @@ class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
class QAPISchemaAlternateType(QAPISchemaType):
meta = 'alternate'
- def __init__(self, name, info, doc, ifcond, variants):
- QAPISchemaType.__init__(self, name, info, doc, ifcond)
+ def __init__(self, name, info, doc, ifcond, features, variants):
+ QAPISchemaType.__init__(self, name, info, doc, ifcond, features)
assert isinstance(variants, QAPISchemaObjectTypeVariants)
assert variants.tag_member
variants.set_defined_in(name)
@@ -669,16 +670,16 @@ class QAPISchemaAlternateType(QAPISchemaType):
def visit(self, visitor):
QAPISchemaType.visit(self, 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):
QAPISchemaEntity.__init__(self, name, info, doc, ifcond, features)
assert not arg_type or isinstance(arg_type, str)
assert not ret_type or isinstance(ret_type, str)
@@ -739,8 +740,8 @@ class QAPISchemaCommand(QAPISchemaEntity):
class QAPISchemaEvent(QAPISchemaEntity):
meta = 'event'
- def __init__(self, name, info, doc, ifcond, arg_type, boxed):
- QAPISchemaEntity.__init__(self, name, info, doc, ifcond)
+ def __init__(self, name, info, doc, ifcond, features, arg_type, boxed):
+ QAPISchemaEntity.__init__(self, name, info, doc, ifcond, features)
assert not arg_type or isinstance(arg_type, str)
self._arg_type_name = arg_type
self.arg_type = None
@@ -770,8 +771,9 @@ class QAPISchemaEvent(QAPISchemaEntity):
def visit(self, visitor):
QAPISchemaEntity.visit(self, 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(object):
@@ -860,7 +862,7 @@ class QAPISchema(object):
('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',
@@ -868,10 +870,11 @@ class QAPISchema(object):
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]
@@ -883,7 +886,8 @@ class QAPISchema(object):
# 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
@@ -911,8 +915,8 @@ class QAPISchema(object):
# 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):
@@ -920,8 +924,9 @@ class QAPISchema(object):
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):
@@ -943,12 +948,11 @@ class QAPISchema(object):
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)
@@ -967,6 +971,7 @@ class QAPISchema(object):
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):
@@ -987,21 +992,22 @@ class QAPISchema(object):
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)))
@@ -1015,27 +1021,31 @@ class QAPISchema(object):
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 d8751daa04..2a108b6911 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -277,7 +277,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))
@@ -305,7 +305,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 c72f2bc5c0..b21e1340a2 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 31ebe56bbf..970a08ab26 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 d992e713d9..01c930c474 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -52,11 +52,14 @@
##
# @Enum:
# @one: The _one_ {and only}
-#
# @two is undocumented
+#
+# Features:
+# @enum-feat: Also _one_ {and only}
##
{ 'enum': 'Enum', 'data':
[ { 'name': 'one', 'if': 'defined(IFONE)' }, 'two' ],
+ 'features': [ 'enum-feat' ],
'if': 'defined(IFCOND)' }
##
@@ -86,24 +89,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 +173,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 4c9406a464..f5d9dc969c 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
@@ -110,10 +115,11 @@ doc symbol=Enum
arg=one
The _one_ {and only}
- arg=two
-
- section=None
@two is undocumented
+ arg=two
+
+ feature=enum-feat
+Also _one_ {and only}
doc symbol=Base
body=
@@ -134,11 +140,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 +157,8 @@ an integer
@b is undocumented
arg=b
+ feature=alt-feat
+a feature
doc freeform
body=
== Another subsection
@@ -197,3 +209,5 @@ another feature
doc symbol=EVT-BOXED
body=
+ feature=feat3
+a feature
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 9abf175fe0..fa4f3a15da 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 3660e75a48..1ece836d9b 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -401,7 +401,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
@@ -410,9 +428,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
@@ -435,3 +451,6 @@ 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
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index bad14edb47..078fc63f97 100755
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -35,7 +35,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)
@@ -43,6 +43,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:
@@ -63,10 +64,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,
@@ -79,10 +81,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):
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [RFC PATCH 08/19] qapi: Consistently put @features parameter right after @ifcond
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
` (6 preceding siblings ...)
2019-10-24 12:34 ` [RFC PATCH 07/19] qapi: Add feature flags to remaining definitions Markus Armbruster
@ 2019-10-24 12:34 ` Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 09/19] qapi: Inline do_qmp_dispatch() into qmp_dispatch() Markus Armbruster
` (10 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
Signed-off-by: Markus Armbruster <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 ab98e504f3..11e9a6c095 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -276,9 +276,9 @@ void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
c_prefix=c_name(self._prefix, protect=False)))
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 53a1f8e952..55c4892a1e 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -250,8 +250,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
@@ -263,9 +263,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 ba493977cf..f71b984d3b 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -195,8 +195,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,
@@ -211,9 +211,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 f13f442896..639140fceb 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -115,20 +115,20 @@ class QAPISchemaVisitor(object):
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, features, ifcond,
+ 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):
@@ -422,12 +422,12 @@ class QAPISchemaObjectType(QAPISchemaType):
def visit(self, visitor):
QAPISchemaType.visit(self, 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(object):
@@ -729,12 +729,10 @@ class QAPISchemaCommand(QAPISchemaEntity):
def visit(self, visitor):
QAPISchemaEntity.visit(self, 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 2a108b6911..9dc07f6308 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -288,8 +288,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 b21e1340a2..4db94e5e80 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 078fc63f97..9ee8993f8d 100755
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -51,8 +51,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)
@@ -70,9 +70,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))
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [RFC PATCH 09/19] qapi: Inline do_qmp_dispatch() into qmp_dispatch()
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
` (7 preceding siblings ...)
2019-10-24 12:34 ` [RFC PATCH 08/19] qapi: Consistently put @features parameter right after @ifcond Markus Armbruster
@ 2019-10-24 12:34 ` Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 10/19] qapi: Simplify how qmp_dispatch() deals with QCO_NO_SUCCESS_RESP Markus Armbruster
` (9 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
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 <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 bc264b3c9b..a69d5b5a96 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)
{
- Error *local_err = NULL;
+ 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;
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) {
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [RFC PATCH 10/19] qapi: Simplify how qmp_dispatch() deals with QCO_NO_SUCCESS_RESP
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
` (8 preceding siblings ...)
2019-10-24 12:34 ` [RFC PATCH 09/19] qapi: Inline do_qmp_dispatch() into qmp_dispatch() Markus Armbruster
@ 2019-10-24 12:34 ` Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 11/19] qapi: Simplify how qmp_dispatch() gets the request ID Markus Armbruster
` (8 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/qmp-dispatch.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index a69d5b5a96..d1643fe37a 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -151,31 +151,31 @@ 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);
-
out:
+ assert(!err != !ret);
+
if (err) {
rsp = qmp_error_response(err);
- } else if (ret) {
+ } else {
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) {
+ if (id) {
qdict_put_obj(rsp, "id", qobject_ref(id));
}
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [RFC PATCH 11/19] qapi: Simplify how qmp_dispatch() gets the request ID
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
` (9 preceding siblings ...)
2019-10-24 12:34 ` [RFC PATCH 10/19] qapi: Simplify how qmp_dispatch() deals with QCO_NO_SUCCESS_RESP Markus Armbruster
@ 2019-10-24 12:34 ` Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 12/19] qapi: Replace qmp_dispatch()'s TODO comment by an explanation Markus Armbruster
` (7 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
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 getting the request ID into qmp_dispatch_check_obj().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/qmp-dispatch.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index d1643fe37a..0cbb663097 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -20,7 +20,7 @@
#include "qapi/qmp/qbool.h"
static QDict *qmp_dispatch_check_obj(const QObject *request, bool allow_oob,
- Error **errp)
+ QObject **id, Error **errp)
{
const char *exec_key = NULL;
const QDictEntry *ent;
@@ -30,10 +30,13 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, bool allow_oob,
dict = qobject_to(QDict, request);
if (!dict) {
+ *id = NULL;
error_setg(errp, "QMP input must be a JSON object");
return NULL;
}
+ *id = qdict_get(dict, "id");
+
for (ent = qdict_first(dict); ent;
ent = qdict_next(dict, ent)) {
arg_name = qdict_entry_key(ent);
@@ -103,12 +106,12 @@ 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;
- dict = qmp_dispatch_check_obj(request, allow_oob, &err);
+ dict = qmp_dispatch_check_obj(request, allow_oob, &id, &err);
if (!dict) {
goto out;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [RFC PATCH 12/19] qapi: Replace qmp_dispatch()'s TODO comment by an explanation
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
` (10 preceding siblings ...)
2019-10-24 12:34 ` [RFC PATCH 11/19] qapi: Simplify how qmp_dispatch() gets the request ID Markus Armbruster
@ 2019-10-24 12:34 ` Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 13/19] qapi: New special feature flag "deprecated" Markus Armbruster
` (6 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
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 0cbb663097..55bc224c61 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -164,7 +164,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 the empty
+ * object then; supply it.
+ */
ret = QOBJECT(qdict_new());
}
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [RFC PATCH 13/19] qapi: New special feature flag "deprecated"
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
` (11 preceding siblings ...)
2019-10-24 12:34 ` [RFC PATCH 12/19] qapi: Replace qmp_dispatch()'s TODO comment by an explanation Markus Armbruster
@ 2019-10-24 12:34 ` Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 14/19] qemu-options: New -compat to set policy for "funny" interfaces Markus Armbruster
` (5 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
Unlike regular feature flags, the new special feature flag
"deprecated" is recognized by the QAPI generator. For now, it's only
permitted with commands and events. It will be put to use shortly.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
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 | 4 ++--
tests/qapi-schema/qapi-schema-test.out | 4 ++--
8 files changed, 22 insertions(+), 4 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 eaeedc7bd3..e6ef93544e 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -665,6 +665,12 @@ Example:
'features': [ 'allow-negative-numbers' ] }
+==== Special features ====
+
+Feature "deprecated" makes a command or event 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 639140fceb..840d119c6d 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -179,6 +179,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 ea35cd54cc..9da831b882 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -388,6 +388,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 0000000000..af4ffe20aa
--- /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 0000000000..4b5bf5b86e
--- /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 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index fa4f3a15da..6862d8ba2c 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -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 1ece836d9b..494d4c25c4 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -432,7 +432,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
@@ -453,4 +453,4 @@ 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
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [RFC PATCH 14/19] qemu-options: New -compat to set policy for "funny" interfaces
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
` (12 preceding siblings ...)
2019-10-24 12:34 ` [RFC PATCH 13/19] qapi: New special feature flag "deprecated" Markus Armbruster
@ 2019-10-24 12:34 ` Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 15/19] qapi: Mark deprecated QMP commands with feature 'deprecated' Markus Armbruster
` (4 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
"Funny" interface policy is separate for input and output.
Input policy can be "accept" (accept silently; default), "reject"
(reject the request with an error), or "crash" (abort() the process).
Output policy can be "accept" (pass on unchanged), or "hide" (filter
out the deprecated parts).
Policies other than "accept" are implemented later in this series.
For now, the "funny" interfaces are just QMP commands and events with
feature "deprecated". We'll want to extend this to arguments,
returns, and the command line. Extending it to experimental
interfaces may make sense.
FIXME Documentation and help need some work
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/common.json | 48 ++++++++++++++++++++++++++++++++++++
include/qapi/compat-policy.h | 20 +++++++++++++++
qapi/qmp-dispatch.c | 3 +++
vl.c | 17 +++++++++++++
qemu-options.hx | 18 ++++++++++++++
5 files changed, 106 insertions(+)
create mode 100644 include/qapi/compat-policy.h
diff --git a/qapi/common.json b/qapi/common.json
index 7b9cbcd97b..9fc3e6400c 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -144,3 +144,51 @@
##
{ 'enum': 'PCIELinkWidth',
'data': [ '1', '2', '4', '8', '12', '16', '32' ] }
+
+##
+# @CompatPolicyInput:
+#
+# Policy for handling "funny" input.
+#
+# @accept: Accept silently
+# TODO @reject: Reject with an error
+# TODO @crash: abort() the process
+#
+# FIXME Guidance on intended use.
+#
+# Since: 4.2
+##
+{ 'enum': 'CompatPolicyInput',
+ 'data': [ 'accept' ] }
+
+##
+# @CompatPolicyOutput:
+#
+# Policy for handling "funny" output.
+#
+# @accept: Pass on unchanged
+# TODO @hide: Filter out
+#
+# FIXME Guidance on intended use.
+#
+# Since: 4.2
+##
+{ 'enum': 'CompatPolicyOutput',
+ 'data': [ 'accept' ] }
+
+##
+# @CompatPolicy:
+#
+# Policy for handling "funny" management interfaces.
+#
+# Limitation: covers only QMP commands and events. Argument support
+# is not yet implemented. CLI is not yet implemented.
+#
+# @deprecated-input: how to handle deprecated input (default 'accept')
+# @deprecated-output: how to handle deprecated output (default 'accept')
+##
+{ 'struct': 'CompatPolicy',
+ 'data': { '*deprecated-input': 'CompatPolicyInput',
+ '*deprecated-output': 'CompatPolicyOutput' } }
+
+# FIXME CompatPolicy & friends don't belong here
diff --git a/include/qapi/compat-policy.h b/include/qapi/compat-policy.h
new file mode 100644
index 0000000000..c491d74aac
--- /dev/null
+++ b/include/qapi/compat-policy.h
@@ -0,0 +1,20 @@
+/*
+ * Policy for handling "funny" management interfaces
+ *
+ * Copyright (C) 2019 Red Hat, Inc.
+ *
+ * Authors:
+ * Markus Armbruster <armbru@redhat.com>,
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+
+#ifndef QAPI_COMPAT_POLICY_H
+#define QAPI_COMPAT_POLICY_H
+
+#include "qapi/qapi-types-common.h"
+
+extern CompatPolicy qapi_compat_policy;
+
+#endif
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 55bc224c61..8fe59cf54d 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -12,6 +12,7 @@
*/
#include "qemu/osdep.h"
+#include "qapi/compat-policy.h"
#include "qapi/error.h"
#include "qapi/qmp/dispatch.h"
#include "qapi/qmp/qdict.h"
@@ -19,6 +20,8 @@
#include "sysemu/runstate.h"
#include "qapi/qmp/qbool.h"
+CompatPolicy qapi_compat_policy;
+
static QDict *qmp_dispatch_check_obj(const QObject *request, bool allow_oob,
QObject **id, Error **errp)
{
diff --git a/vl.c b/vl.c
index 4489cfb2bb..300f2a01de 100644
--- a/vl.c
+++ b/vl.c
@@ -26,6 +26,7 @@
#include "qemu-common.h"
#include "qemu/units.h"
#include "hw/qdev-properties.h"
+#include "qapi/compat-policy.h"
#include "qapi/error.h"
#include "qemu-version.h"
#include "qemu/cutils.h"
@@ -3783,6 +3784,22 @@ int main(int argc, char **argv, char **envp)
qemu_opt_get_bool(opts, "mem-lock", false);
enable_cpu_pm = qemu_opt_get_bool(opts, "cpu-pm", false);
break;
+ case QEMU_OPTION_compat:
+ {
+ CompatPolicy *opts;
+ Visitor *v;
+
+ v = qobject_input_visitor_new_str(optarg, NULL,
+ &error_fatal);
+
+ visit_type_CompatPolicy(v, NULL, &opts, &error_fatal);
+ QAPI_CLONE_MEMBERS(CompatPolicy, &qapi_compat_policy,
+ opts);
+
+ qapi_free_CompatPolicy(opts);
+ visit_free(v);
+ break;
+ }
case QEMU_OPTION_msg:
opts = qemu_opts_parse_noisily(qemu_find_opts("msg"), optarg,
false);
diff --git a/qemu-options.hx b/qemu-options.hx
index 996b6fba74..c43f768a15 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3319,6 +3319,24 @@ STEXI
@table @option
ETEXI
+DEF("compat", HAS_ARG, QEMU_OPTION_compat,
+ "-compat [deprecated-input=accept][,deprecated-output=accept]\n"
+ " Policy for handling deprecated management interfaces\n",
+ QEMU_ARCH_ALL)
+STEXI
+@item -compat [deprecated-input=@var{input-policy}][,deprecated-output=@var{output-policy}]
+@findex -compat
+
+Set policy for handling deprecated management interfaces:
+@table @option
+@item deprecated-input=accept (default)
+Accept deprecated commands
+@item deprecated-output=accept (default)
+Emit deprecated events
+@end table
+FIXME Guidance on intended use
+ETEXI
+
DEF("fw_cfg", HAS_ARG, QEMU_OPTION_fwcfg,
"-fw_cfg [name=]<name>,file=<file>\n"
" add named fw_cfg entry with contents from file\n"
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [RFC PATCH 15/19] qapi: Mark deprecated QMP commands with feature 'deprecated'
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
` (13 preceding siblings ...)
2019-10-24 12:34 ` [RFC PATCH 14/19] qemu-options: New -compat to set policy for "funny" interfaces Markus Armbruster
@ 2019-10-24 12:34 ` Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 16/19] qapi: Implement -compat deprecated-input=reject for commands Markus Armbruster
` (3 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
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"]}
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.
Management applications can use -compat deprecated-input=... to set
policy for these commands. So far, the only available policy is
"accept", which doesn't change behavior. The next few commits will
provide more interesting policies.
Command deprecation becomes visible in introspection. Management
applications could conceivably use this for static checking.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/machine.json | 24 ++++++++++++++----------
qapi/migration.json | 36 ++++++++++++++++++++++++------------
qapi/misc.json | 25 +++++++++++++++----------
3 files changed, 53 insertions(+), 32 deletions(-)
diff --git a/qapi/machine.json b/qapi/machine.json
index ca26779f1a..3913ef2138 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -189,6 +189,11 @@
# It is recommended to use @query-cpus-fast instead of this command to
# avoid the vCPU interruption.
#
+# Features:
+# @deprecated: 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.
+#
# Returns: a list of @CpuInfo for each virtual CPU
#
# Since: 0.14.0
@@ -218,12 +223,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:
@@ -309,21 +311,23 @@
#
# @id: ID of CPU to be created, valid values [0..max_cpus)
#
+# Features:
+# @deprecated: This command is deprecated. The `device_add` command
+# should be used 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 82feb5bd39..a110948bfe 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1119,9 +1119,11 @@
#
# @value: maximum downtime in seconds
#
-# Returns: nothing on success
+# Features:
+# @deprecated: This command is deprecated in favor of
+# 'migrate-set-parameters'.
#
-# Notes: This command is deprecated in favor of 'migrate-set-parameters'
+# Returns: nothing on success
#
# Since: 0.14.0
#
@@ -1131,7 +1133,8 @@
# <- { "return": {} }
#
##
-{ 'command': 'migrate_set_downtime', 'data': {'value': 'number'} }
+{ 'command': 'migrate_set_downtime', 'data': {'value': 'number'},
+ 'features': [ 'deprecated' ] }
##
# @migrate_set_speed:
@@ -1140,9 +1143,11 @@
#
# @value: maximum speed in bytes per second.
#
-# Returns: nothing on success
+# Features:
+# @deprecated: This command is deprecated in favor of
+# 'migrate-set-parameters'.
#
-# Notes: This command is deprecated in favor of 'migrate-set-parameters'
+# Returns: nothing on success
#
# Since: 0.14.0
#
@@ -1152,7 +1157,8 @@
# <- { "return": {} }
#
##
-{ 'command': 'migrate_set_speed', 'data': {'value': 'int'} }
+{ 'command': 'migrate_set_speed', 'data': {'value': 'int'},
+ 'features': [ 'deprecated' ] }
##
# @migrate-set-cache-size:
@@ -1161,13 +1167,15 @@
#
# @value: cache size in bytes
#
+# Features:
+# @deprecated: This command is deprecated in favor of
+# 'migrate-set-parameters'.
+#
# 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:
@@ -1177,17 +1185,20 @@
# <- { "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
#
+# Features:
+# @deprecated: This command is deprecated in favor of
+# 'query-migrate-parameters'.
+#
# Returns: XBZRLE cache size in bytes
#
-# Notes: This command is deprecated in favor of 'query-migrate-parameters'
-#
# Since: 1.2
#
# Example:
@@ -1196,7 +1207,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 33b94e3589..abd2e5dc6e 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -316,13 +316,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" }
@@ -340,7 +342,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' ] }
##
# @IOThreadInfo:
@@ -1074,14 +1077,15 @@
# 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, 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.
+#
# 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:
@@ -1102,7 +1106,8 @@
#
##
{ 'command': 'change',
- 'data': {'device': 'str', 'target': 'str', '*arg': 'str'} }
+ 'data': {'device': 'str', 'target': 'str', '*arg': 'str'},
+ 'features': [ 'deprecated' ] }
##
# @xen-set-global-dirty-log:
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [RFC PATCH 16/19] qapi: Implement -compat deprecated-input=reject for commands
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
` (14 preceding siblings ...)
2019-10-24 12:34 ` [RFC PATCH 15/19] qapi: Mark deprecated QMP commands with feature 'deprecated' Markus Armbruster
@ 2019-10-24 12:34 ` Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 17/19] qapi: Implement -compat deprecated-input=crash " Markus Armbruster
` (2 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
This policy makes deprecated commands fail like this:
---> {"execute": "query-cpus"}
<--- {"error": {"class": "CommandNotFound", "desc": "Deprecated command query-cpus disabled by policy"}}
When the command is removed, the error will change to
<--- {"error": {"class": "CommandNotFound", "desc": "The command query-cpus has not been found"}}
The policy thus permits "testing the future".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/common.json | 4 ++--
include/qapi/qmp/dispatch.h | 1 +
qapi/qmp-dispatch.c | 13 +++++++++++++
tests/test-qmp-cmds.c | 23 +++++++++++++++++++++++
qemu-options.hx | 4 +++-
scripts/qapi/commands.py | 10 +++++++---
6 files changed, 49 insertions(+), 6 deletions(-)
diff --git a/qapi/common.json b/qapi/common.json
index 9fc3e6400c..3e9d12c90f 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -151,7 +151,7 @@
# Policy for handling "funny" input.
#
# @accept: Accept silently
-# TODO @reject: Reject with an error
+# @reject: Reject with an error
# TODO @crash: abort() the process
#
# FIXME Guidance on intended use.
@@ -159,7 +159,7 @@
# Since: 4.2
##
{ 'enum': 'CompatPolicyInput',
- 'data': [ 'accept' ] }
+ 'data': [ 'accept', 'reject' ] }
##
# @CompatPolicyOutput:
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 9aa426a398..ef256f2bb7 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -24,6 +24,7 @@ typedef enum QmpCommandOptions
QCO_NO_SUCCESS_RESP = (1U << 0),
QCO_ALLOW_OOB = (1U << 1),
QCO_ALLOW_PRECONFIG = (1U << 2),
+ QCO_DEPRECATED = (1U << 3),
} QmpCommandOptions;
typedef struct QmpCommand
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 8fe59cf54d..b079db85d2 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -132,6 +132,19 @@ QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request,
"The command %s has not been found", command);
goto out;
}
+ if (cmd->options & QCO_DEPRECATED) {
+ switch (qapi_compat_policy.deprecated_input) {
+ case COMPAT_POLICY_INPUT_ACCEPT:
+ break;
+ case COMPAT_POLICY_INPUT_REJECT:
+ error_set(&err, ERROR_CLASS_COMMAND_NOT_FOUND,
+ "Deprecated command %s disabled by policy",
+ command);
+ goto out;
+ default:
+ abort();
+ }
+ }
if (!cmd->enabled) {
error_set(&err, ERROR_CLASS_COMMAND_NOT_FOUND,
"The command %s has been disabled for this instance",
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index cf4fa1a091..005ea24a27 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -1,4 +1,5 @@
#include "qemu/osdep.h"
+#include "qapi/compat-policy.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qjson.h"
#include "qapi/qmp/qnum.h"
@@ -235,6 +236,26 @@ static void test_dispatch_cmd_success_response(void)
qobject_unref(req);
}
+static void test_dispatch_cmd_deprecated(void)
+{
+ const char *cmd = "{ 'execute': 'test-command-features1' }";
+ QDict *ret;
+
+ /* accept */
+ ret = qobject_to(QDict, do_qmp_dispatch(false, cmd));
+ assert(ret && qdict_size(ret) == 0);
+ qobject_unref(ret);
+
+ qapi_compat_policy.has_deprecated_input = true;
+ qapi_compat_policy.deprecated_input = COMPAT_POLICY_INPUT_ACCEPT;
+ ret = qobject_to(QDict, do_qmp_dispatch(false, cmd));
+ assert(ret && qdict_size(ret) == 0);
+ qobject_unref(ret);
+
+ qapi_compat_policy.deprecated_input = COMPAT_POLICY_INPUT_REJECT;
+ do_qmp_dispatch_error(false, ERROR_CLASS_COMMAND_NOT_FOUND, cmd);
+}
+
/* test commands that involve both input parameters and return values */
static void test_dispatch_cmd_io(void)
{
@@ -344,6 +365,8 @@ int main(int argc, char **argv)
g_test_add_func("/qmp/dispatch_cmd_io", test_dispatch_cmd_io);
g_test_add_func("/qmp/dispatch_cmd_success_response",
test_dispatch_cmd_success_response);
+ g_test_add_func("/qmp/dispatch_cmd_deprecated",
+ test_dispatch_cmd_deprecated);
g_test_add_func("/qmp/dealloc_types", test_dealloc_types);
g_test_add_func("/qmp/dealloc_partial", test_dealloc_partial);
diff --git a/qemu-options.hx b/qemu-options.hx
index c43f768a15..f107a57c81 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3320,7 +3320,7 @@ STEXI
ETEXI
DEF("compat", HAS_ARG, QEMU_OPTION_compat,
- "-compat [deprecated-input=accept][,deprecated-output=accept]\n"
+ "-compat [deprecated-input=accept|reject][,deprecated-output=accept]\n"
" Policy for handling deprecated management interfaces\n",
QEMU_ARCH_ALL)
STEXI
@@ -3331,6 +3331,8 @@ Set policy for handling deprecated management interfaces:
@table @option
@item deprecated-input=accept (default)
Accept deprecated commands
+@item deprecated-input=reject
+Reject deprecated commands
@item deprecated-output=accept (default)
Emit deprecated events
@end table
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 11e9a6c095..df2a132915 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -194,9 +194,13 @@ out:
return ret
-def gen_register_command(name, success_response, allow_oob, allow_preconfig):
+def gen_register_command(name, features,
+ success_response, allow_oob, allow_preconfig):
options = []
+ if 'deprecated' in [f.name for f in features]:
+ options += ['QCO_DEPRECATED']
+
if not success_response:
options += ['QCO_NO_SUCCESS_RESP']
if allow_oob:
@@ -295,8 +299,8 @@ void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
self._genh.add(gen_command_decl(name, arg_type, boxed, ret_type))
self._genh.add(gen_marshal_decl(name))
self._genc.add(gen_marshal(name, arg_type, boxed, ret_type))
- self._regy.add(gen_register_command(name, success_response,
- allow_oob, allow_preconfig))
+ self._regy.add(gen_register_command(
+ name, features, success_response, allow_oob, allow_preconfig))
def gen_commands(schema, output_dir, prefix):
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [RFC PATCH 17/19] qapi: Implement -compat deprecated-input=crash for commands
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
` (15 preceding siblings ...)
2019-10-24 12:34 ` [RFC PATCH 16/19] qapi: Implement -compat deprecated-input=reject for commands Markus Armbruster
@ 2019-10-24 12:34 ` Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 18/19] qapi: Include a warning in the response to a deprecated command Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 19/19] qapi: Implement -compat deprecated-output=hide for events Markus Armbruster
18 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
This policy calls abort() when a deprecated command is received.
Crashing should reliably[*] fail existing integration tests with very
little additional work (just pass the option).
[*] Bugs in tests can conceivably mask even crashes, but that seems
unlikely.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/common.json | 4 ++--
qapi/qmp-dispatch.c | 1 +
qemu-options.hx | 4 +++-
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/qapi/common.json b/qapi/common.json
index 3e9d12c90f..06e54642bb 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -152,14 +152,14 @@
#
# @accept: Accept silently
# @reject: Reject with an error
-# TODO @crash: abort() the process
+# @crash: abort() the process
#
# FIXME Guidance on intended use.
#
# Since: 4.2
##
{ 'enum': 'CompatPolicyInput',
- 'data': [ 'accept', 'reject' ] }
+ 'data': [ 'accept', 'reject', 'crash' ] }
##
# @CompatPolicyOutput:
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index b079db85d2..6436417844 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -141,6 +141,7 @@ QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request,
"Deprecated command %s disabled by policy",
command);
goto out;
+ case COMPAT_POLICY_INPUT_CRASH:
default:
abort();
}
diff --git a/qemu-options.hx b/qemu-options.hx
index f107a57c81..3a740ea7b1 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3320,7 +3320,7 @@ STEXI
ETEXI
DEF("compat", HAS_ARG, QEMU_OPTION_compat,
- "-compat [deprecated-input=accept|reject][,deprecated-output=accept]\n"
+ "-compat [deprecated-input=accept|reject|crash][,deprecated-output=accept]\n"
" Policy for handling deprecated management interfaces\n",
QEMU_ARCH_ALL)
STEXI
@@ -3333,6 +3333,8 @@ Set policy for handling deprecated management interfaces:
Accept deprecated commands
@item deprecated-input=reject
Reject deprecated commands
+@item deprecated-input=crash
+Crash on deprecated command
@item deprecated-output=accept (default)
Emit deprecated events
@end table
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [RFC PATCH 18/19] qapi: Include a warning in the response to a deprecated command
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
` (16 preceding siblings ...)
2019-10-24 12:34 ` [RFC PATCH 17/19] qapi: Implement -compat deprecated-input=crash " Markus Armbruster
@ 2019-10-24 12:34 ` Markus Armbruster
2019-10-24 14:01 ` [libvirt] " Daniel P. Berrangé
2019-10-24 12:34 ` [RFC PATCH 19/19] qapi: Implement -compat deprecated-output=hide for events Markus Armbruster
18 siblings, 1 reply; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
Looks like this
---> {"execute": "query-cpus"}
<--- {"return": [...], "warnings": [{"class": "CommandNotFound", "desc": "command is deprecated"}]}
Management applications may want to log such warnings.
This commit is not for merging as is, because
* docs/interop/qmp-spec.txt needs an update for the new success
response member "warnings".
* I'd like to see a prospective user before I extend the QMP protocol.
If you have specific plans to put them to use, let me know.
* The same warning should be included in a deprecated event.
* Emitting the same warning over and over again might be annoying or
slow. Perhaps warning just once would be better.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/qmp-dispatch.c | 8 ++++++++
tests/test-qmp-cmds.c | 43 +++++++++++++++++++++++++++++++++++++++++--
qemu-options.hx | 2 +-
3 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 6436417844..9c17a59f31 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -197,6 +197,14 @@ out:
} else {
rsp = qdict_new();
qdict_put_obj(rsp, "return", ret);
+ if (cmd->options & QCO_DEPRECATED) {
+ qdict_put_obj(
+ rsp, "warnings",
+ qobject_from_jsonf_nofail(
+ "[ { 'class': %s, 'desc': %s } ]",
+ QapiErrorClass_str(ERROR_CLASS_COMMAND_NOT_FOUND),
+ "command is deprecated"));
+ }
}
if (id) {
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index 005ea24a27..38d2e5b4a7 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -2,6 +2,7 @@
#include "qapi/compat-policy.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qlist.h"
#include "qapi/qmp/qnum.h"
#include "qapi/qmp/qstring.h"
#include "qapi/error.h"
@@ -164,6 +165,40 @@ static QObject *do_qmp_dispatch(bool allow_oob, const char *template, ...)
return ret;
}
+static QObject *do_qmp_dispatch_warning(bool allow_oob, ErrorClass cls,
+ const char *template, ...)
+{
+ va_list ap;
+ QDict *req, *resp;
+ QObject *ret;
+ QList *warnings;
+ QDict *warning;
+
+ 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");
+ g_assert(ret);
+ warnings = qdict_get_qlist(resp, "warnings");
+ g_assert(warnings);
+ warning = qobject_to(QDict, qlist_peek(warnings));
+ g_assert(warning);
+ g_assert_cmpstr(qdict_get_try_str(warning, "class"),
+ ==, QapiErrorClass_str(cls));
+ g_assert(qdict_get_try_str(warning, "desc"));
+ g_assert(qdict_size(warning) == 2);
+ g_assert(qlist_size(warnings) == 1);
+ g_assert(qdict_size(resp) == 2);
+
+ qobject_ref(ret);
+ qobject_unref(resp);
+ qobject_unref(req);
+ return ret;
+}
+
static void do_qmp_dispatch_error(bool allow_oob, ErrorClass cls,
const char *template, ...)
{
@@ -242,13 +277,17 @@ static void test_dispatch_cmd_deprecated(void)
QDict *ret;
/* accept */
- ret = qobject_to(QDict, do_qmp_dispatch(false, cmd));
+ ret = qobject_to(QDict, do_qmp_dispatch_warning(
+ false, ERROR_CLASS_COMMAND_NOT_FOUND,
+ "{ 'execute': 'test-command-features1' }"));
assert(ret && qdict_size(ret) == 0);
qobject_unref(ret);
qapi_compat_policy.has_deprecated_input = true;
qapi_compat_policy.deprecated_input = COMPAT_POLICY_INPUT_ACCEPT;
- ret = qobject_to(QDict, do_qmp_dispatch(false, cmd));
+ ret = qobject_to(QDict, do_qmp_dispatch_warning(
+ false, ERROR_CLASS_COMMAND_NOT_FOUND,
+ "{ 'execute': 'test-command-features1' }"));
assert(ret && qdict_size(ret) == 0);
qobject_unref(ret);
diff --git a/qemu-options.hx b/qemu-options.hx
index 3a740ea7b1..645629457a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3330,7 +3330,7 @@ STEXI
Set policy for handling deprecated management interfaces:
@table @option
@item deprecated-input=accept (default)
-Accept deprecated commands
+Accept deprecated commands with a warning
@item deprecated-input=reject
Reject deprecated commands
@item deprecated-input=crash
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [RFC PATCH 19/19] qapi: Implement -compat deprecated-output=hide for events
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
` (17 preceding siblings ...)
2019-10-24 12:34 ` [RFC PATCH 18/19] qapi: Include a warning in the response to a deprecated command Markus Armbruster
@ 2019-10-24 12:34 ` Markus Armbruster
2019-10-24 14:16 ` [libvirt] " Daniel P. Berrangé
18 siblings, 1 reply; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: libvir-list, mdroth
This policy suppresses deprecated events, and thus permits "testing
the future".
No QMP event is deprecated right now.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/common.json | 4 ++--
tests/test-qmp-event.c | 17 +++++++++++++++++
qemu-options.hx | 4 +++-
scripts/qapi/events.py | 14 ++++++++++++--
4 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/qapi/common.json b/qapi/common.json
index 06e54642bb..4e3da4beee 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -167,14 +167,14 @@
# Policy for handling "funny" output.
#
# @accept: Pass on unchanged
-# TODO @hide: Filter out
+# @hide: Filter out
#
# FIXME Guidance on intended use.
#
# Since: 4.2
##
{ 'enum': 'CompatPolicyOutput',
- 'data': [ 'accept' ] }
+ 'data': [ 'accept', 'hide' ] }
##
# @CompatPolicy:
diff --git a/tests/test-qmp-event.c b/tests/test-qmp-event.c
index 7dd0053190..303c8d6382 100644
--- a/tests/test-qmp-event.c
+++ b/tests/test-qmp-event.c
@@ -14,6 +14,7 @@
#include "qemu/osdep.h"
#include "qemu-common.h"
+#include "qapi/compat-policy.h"
#include "qapi/error.h"
#include "qapi/qmp/qbool.h"
#include "qapi/qmp/qdict.h"
@@ -140,6 +141,21 @@ static void test_event_d(TestEventData *data,
qobject_unref(data->expect);
}
+static void test_event_deprecated(TestEventData *data, const void *unused)
+{
+ data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES1' }");
+
+ qapi_event_send_test_event_features1();
+ g_assert(data->emitted);
+
+ qapi_compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE;
+ data->emitted = false;
+ qapi_event_send_test_event_features1();
+ g_assert(!data->emitted);
+
+ qobject_unref(data->expect);
+}
+
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
@@ -148,6 +164,7 @@ int main(int argc, char **argv)
event_test_add("/event/event_b", test_event_b);
event_test_add("/event/event_c", test_event_c);
event_test_add("/event/event_d", test_event_d);
+ event_test_add("/event/deprecated", test_event_deprecated);
g_test_run();
return 0;
diff --git a/qemu-options.hx b/qemu-options.hx
index 645629457a..c0128813c6 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3320,7 +3320,7 @@ STEXI
ETEXI
DEF("compat", HAS_ARG, QEMU_OPTION_compat,
- "-compat [deprecated-input=accept|reject|crash][,deprecated-output=accept]\n"
+ "-compat [deprecated-input=accept|reject|crash][,deprecated-output=accept|hide]\n"
" Policy for handling deprecated management interfaces\n",
QEMU_ARCH_ALL)
STEXI
@@ -3337,6 +3337,8 @@ Reject deprecated commands
Crash on deprecated command
@item deprecated-output=accept (default)
Emit deprecated events
+@item deprecated-output=hide
+Suppress deprecated events
@end table
FIXME Guidance on intended use
ETEXI
diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index f64e61076e..5778fa1a0d 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -61,7 +61,8 @@ def gen_param_var(typ):
return ret
-def gen_event_send(name, arg_type, boxed, event_enum_name, event_emit):
+def gen_event_send(name, arg_type, features, boxed,
+ event_enum_name, event_emit):
# FIXME: Our declaration of local variables (and of 'errp' in the
# parameter list) can collide with exploded members of the event's
# data type passed in as parameters. If this collision ever hits in
@@ -86,6 +87,14 @@ def gen_event_send(name, arg_type, boxed, event_enum_name, event_emit):
if not boxed:
ret += gen_param_var(arg_type)
+ if 'deprecated' in [f.name for f in features]:
+ ret += mcgen('''
+
+ if (qapi_compat_policy.deprecated_output == COMPAT_POLICY_OUTPUT_HIDE) {
+ return;
+ }
+''')
+
ret += mcgen('''
qmp = qmp_event_build_dict("%(name)s");
@@ -154,6 +163,7 @@ class QAPISchemaGenEventVisitor(QAPISchemaModularCVisitor):
#include "%(prefix)sqapi-emit-events.h"
#include "%(events)s.h"
#include "%(visit)s.h"
+#include "qapi/compat-policy.h"
#include "qapi/error.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qobject-output-visitor.h"
@@ -192,7 +202,7 @@ void %(event_emit)s(%(event_enum)s event, QDict *qdict);
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,
+ self._genc.add(gen_event_send(name, arg_type, features, boxed,
self._event_enum_name,
self._event_emit_name))
# Note: we generate the enum member regardless of @ifcond, to
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [libvirt] [RFC PATCH 18/19] qapi: Include a warning in the response to a deprecated command
2019-10-24 12:34 ` [RFC PATCH 18/19] qapi: Include a warning in the response to a deprecated command Markus Armbruster
@ 2019-10-24 14:01 ` Daniel P. Berrangé
2019-10-24 19:35 ` Markus Armbruster
0 siblings, 1 reply; 24+ messages in thread
From: Daniel P. Berrangé @ 2019-10-24 14:01 UTC (permalink / raw)
To: Markus Armbruster; +Cc: libvir-list, qemu-devel
On Thu, Oct 24, 2019 at 02:34:57PM +0200, Markus Armbruster wrote:
> Looks like this
>
> ---> {"execute": "query-cpus"}
> <--- {"return": [...], "warnings": [{"class": "CommandNotFound", "desc": "command is deprecated"}]}
>
> Management applications may want to log such warnings.
>
> This commit is not for merging as is, because
>
> * docs/interop/qmp-spec.txt needs an update for the new success
> response member "warnings".
>
> * I'd like to see a prospective user before I extend the QMP protocol.
> If you have specific plans to put them to use, let me know.
Thinking about libvirt's usage of QMP
- A public API call may result in many QMP commands being run.
- Public APIs don't have any convenient way to report deprecated
usage synchronously at runtime.
- The set of QMP comamnds used by libvirt is a private impl
detail that a mgmt app shouldn't know about
Some (most) deprecations will be things targetted at libvirt
developers, where libvirt just needs fixing to use some new
alternative instead.
Other deprecations where there's no replacement provided by QEMU
are things where an application might need to be told to stop
using the feature. From libvirt's public API POV the feature
likely won't be deprecated, only the specific usage of that
feature with the QEMU driver. eg consider QEMU decides to
stop POSTCOPY migration for some reason. Its deprecated from
POV of QEMU & QMP commands. If Xen or ESX support POSTCOPY
though, its not deprecated from libvirt's API POV. In many
ways this becomes a capabilities reporting problem between
libvirt & the application. Libvirt needs to tell the app which
features they can use, given their curent open libvirt connection
and VM instance(s).
So, either way, I don't think the QMP deprecations are something
we would want to expose to applications 'as is', since they're
either not something an app dev can fix, or they need rephrasing
in terms of the libvirt API/config feature the app is using, or
translating into a way for libvirt to expose capabilities to apps.
Libvirt could potentially log the deprecation warning in the per
QEMU VM log file. If end users see such log messages they'll
probably file support tickets / bug reports against libvirt and/or
the mgmt app, which will alert their maintainers to the fact. THis
could be useful if the maintainers missed the QEMU documentation
update listing the deprecation. It could be annoying if libvirt
knows it is deprecated though, and intentionally is still using
it in this particular version, with plans already present to fix
it in future. So if libvirt does log the deprecations to the
VM log file, we'll probably want to /not/ log certain deprecations
that we're intentionally ignoring (temporarily).
In theory libvirt could see the deprecation reply and take
different action, but I don't much like that idea. It is too
late becasue we've already run the command, and its providing
a second way to deal with capabilities. We should be able to
query/probe the right way to invoke commands upfront, so that
we avoid using deprecated stuff in the first place.
> * The same warning should be included in a deprecated event.
>
> * Emitting the same warning over and over again might be annoying or
> slow. Perhaps warning just once would be better.
If written to a log file, any single deprecation definitely
needs to be limited to once only per QEMU process lifetime.
Once a libvirt/qemu pair is deployed on a host it may be a
long time before an upgrade is done that pulls in the new
libvirt to avoid the deprecation. So we don't want to be
spamming logs of an otherwise fully functional VM.
In summary, it is probably reasonable to include this info in the QMP
command reply, but don't expect much to be done with it beyond possibly
writing it to a log file.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [libvirt] [RFC PATCH 19/19] qapi: Implement -compat deprecated-output=hide for events
2019-10-24 12:34 ` [RFC PATCH 19/19] qapi: Implement -compat deprecated-output=hide for events Markus Armbruster
@ 2019-10-24 14:16 ` Daniel P. Berrangé
2019-10-24 19:56 ` Markus Armbruster
0 siblings, 1 reply; 24+ messages in thread
From: Daniel P. Berrangé @ 2019-10-24 14:16 UTC (permalink / raw)
To: Markus Armbruster; +Cc: libvir-list, qemu-devel
On Thu, Oct 24, 2019 at 02:34:58PM +0200, Markus Armbruster wrote:
> This policy suppresses deprecated events, and thus permits "testing
> the future".
One thing that occurs to me is that this is a fairly passive impact
on libvirt. eg it may well be not at all obvious if libvirt is behaving
in a broken way due to an event not being emitted, as the code in
question simply won't be triggered.
With the current QMP this situation is unavoidable since QEMU doesn't
know which events the client (libvirt) is actually using. QEMU just
unconditionally emits all events.
I've often wondered if we should have the client explicitly tell
QEMU which events it wants to receive as part of the QMP greeting
handshake.
ie, libvirt knows which events it can handle. QEMU knows which
events it can emit, and reports this via capabilities which
libvirt probes.
So on connecting libvirt can tell QEMU exactly which evnets it
wants to get back. QEMU is now able to explicitly tell libvirt
it has asked for a deprecated event, and so the logic from the
"deprecated-input" option can take effect.
We'd not need "deprecated-output" at that point.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [libvirt] [RFC PATCH 18/19] qapi: Include a warning in the response to a deprecated command
2019-10-24 14:01 ` [libvirt] " Daniel P. Berrangé
@ 2019-10-24 19:35 ` Markus Armbruster
0 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 19:35 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: libvir-list, qemu-devel
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Thu, Oct 24, 2019 at 02:34:57PM +0200, Markus Armbruster wrote:
>> Looks like this
>>
>> ---> {"execute": "query-cpus"}
>> <--- {"return": [...], "warnings": [{"class": "CommandNotFound", "desc": "command is deprecated"}]}
>>
>> Management applications may want to log such warnings.
>>
>> This commit is not for merging as is, because
>>
>> * docs/interop/qmp-spec.txt needs an update for the new success
>> response member "warnings".
>>
>> * I'd like to see a prospective user before I extend the QMP protocol.
>> If you have specific plans to put them to use, let me know.
>
> Thinking about libvirt's usage of QMP
>
> - A public API call may result in many QMP commands being run.
> - Public APIs don't have any convenient way to report deprecated
> usage synchronously at runtime.
> - The set of QMP comamnds used by libvirt is a private impl
> detail that a mgmt app shouldn't know about
>
> Some (most) deprecations will be things targetted at libvirt
> developers, where libvirt just needs fixing to use some new
> alternative instead.
Based on what we've deprecated so far: most, for a large value of
"most".
> Other deprecations where there's no replacement provided by QEMU
> are things where an application might need to be told to stop
> using the feature. From libvirt's public API POV the feature
> likely won't be deprecated, only the specific usage of that
> feature with the QEMU driver. eg consider QEMU decides to
> stop POSTCOPY migration for some reason. Its deprecated from
> POV of QEMU & QMP commands. If Xen or ESX support POSTCOPY
> though, its not deprecated from libvirt's API POV. In many
> ways this becomes a capabilities reporting problem between
> libvirt & the application. Libvirt needs to tell the app which
> features they can use, given their curent open libvirt connection
> and VM instance(s).
Makes sense.
> So, either way, I don't think the QMP deprecations are something
> we would want to expose to applications 'as is', since they're
> either not something an app dev can fix, or they need rephrasing
> in terms of the libvirt API/config feature the app is using, or
> translating into a way for libvirt to expose capabilities to apps.
Makes sense, too.
> Libvirt could potentially log the deprecation warning in the per
> QEMU VM log file. If end users see such log messages they'll
> probably file support tickets / bug reports against libvirt and/or
> the mgmt app, which will alert their maintainers to the fact. THis
> could be useful if the maintainers missed the QEMU documentation
> update listing the deprecation. It could be annoying if libvirt
> knows it is deprecated though, and intentionally is still using
> it in this particular version, with plans already present to fix
> it in future. So if libvirt does log the deprecations to the
> VM log file, we'll probably want to /not/ log certain deprecations
> that we're intentionally ignoring (temporarily).
Makes sense, too.
Logging the complete QMP traffic can be invaluable when troubleshooting,
and is unlikely to make users report the warnings to libvirt developers.
But that's a different log / a higher debug level.
> In theory libvirt could see the deprecation reply and take
> different action, but I don't much like that idea. It is too
That way is madness :)
> late becasue we've already run the command, and its providing
> a second way to deal with capabilities. We should be able to
> query/probe the right way to invoke commands upfront, so that
> we avoid using deprecated stuff in the first place.
PATCH 15 makes deprecation visible in introspection. Like all of this
series, it's limited to commands and events. Extending to arguments and
return values feels feasible, and I'm willing to do the work.
Argument *values* are a different ballgame. Schema support for "this
argument is deprecated" is straightforward (tack feature "deprecated" to
it). Support for "this argument value is deprecated" is not (except for
enumerations, where we can tack feature "deprecated" to the enumeration
value). Same for return values, combinations of arguments, and so
forth. Not sure how relevant these are in practice.
I'm not sure how useful the "deprecated" feature will be for guiding
decisions on which interface to use. I imagine there's typically a list
of interfaces libvirt can use, ordered by "desirability", and the most
desirable interface known to work gets used. If $new_way is workable,
you use it, else you fall back to $old_way. Whether $old_way is
deprecated is immaterial.
The "deprecated" feature could be used for dynamic checking, i.e. check
the commands sent to QEMU against the output of query-qmp-schema. But
that merely duplicates the check QEMU does when it receives it.
Static checking would be more interesting, if we can pull it off.
>> * The same warning should be included in a deprecated event.
>>
>> * Emitting the same warning over and over again might be annoying or
>> slow. Perhaps warning just once would be better.
>
> If written to a log file, any single deprecation definitely
> needs to be limited to once only per QEMU process lifetime.
> Once a libvirt/qemu pair is deployed on a host it may be a
> long time before an upgrade is done that pulls in the new
> libvirt to avoid the deprecation. So we don't want to be
> spamming logs of an otherwise fully functional VM.
>
> In summary, it is probably reasonable to include this info in the QMP
> command reply, but don't expect much to be done with it beyond possibly
> writing it to a log file.
Understood.
Thanks!
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [libvirt] [RFC PATCH 19/19] qapi: Implement -compat deprecated-output=hide for events
2019-10-24 14:16 ` [libvirt] " Daniel P. Berrangé
@ 2019-10-24 19:56 ` Markus Armbruster
0 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2019-10-24 19:56 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: libvir-list, qemu-devel
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Thu, Oct 24, 2019 at 02:34:58PM +0200, Markus Armbruster wrote:
>> This policy suppresses deprecated events, and thus permits "testing
>> the future".
>
> One thing that occurs to me is that this is a fairly passive impact
> on libvirt. eg it may well be not at all obvious if libvirt is behaving
> in a broken way due to an event not being emitted, as the code in
> question simply won't be triggered.
Intented use of -compat deprecated-input=error,deprecated-output=hide is
"testing the future": make QEMU behave as if the deprecated features
were already gone. Can be useful when you want to test code that deals
with the anticipated future *now*.
It can also be used to ferret out unknown uses of deprecated interfaces:
run test suite with it, see what fails. But as you note, the
deprecated-output=hide part is somewhat problematic in that role.
> With the current QMP this situation is unavoidable since QEMU doesn't
> know which events the client (libvirt) is actually using. QEMU just
> unconditionally emits all events.
>
> I've often wondered if we should have the client explicitly tell
> QEMU which events it wants to receive as part of the QMP greeting
> handshake.
>
> ie, libvirt knows which events it can handle. QEMU knows which
> events it can emit, and reports this via capabilities which
> libvirt probes.
>
> So on connecting libvirt can tell QEMU exactly which evnets it
> wants to get back. QEMU is now able to explicitly tell libvirt
> it has asked for a deprecated event, and so the logic from the
> "deprecated-input" option can take effect.
QEMU already reports its events via introspection. What's missing is an
event subscription mechanism. Should be feasible.
Additional benefit: can reduce I/O.
> We'd not need "deprecated-output" at that point.
If deprecated-input=error makes subscribing to a deprecated event fail,
we don't need deprecated-output=hide for events.
But events are not the only output: there's also command returns.
Consider query-cpus-fast. Returns list of CpuInfoFast. CpuInfoFast
member @arch is deprecated. deprecated-output=hide should hide it,
except it's not implemented in this series.
This is also "a fairly passive impact on libvirt", I'm afraid.
We have some 40 events, and having libvirt subscribe to the ones it
actually uses is obviously practical.
I doubt the subscription idea scales up to return values.
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2019-10-24 19:58 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-24 12:34 [RFC PATCH 00/19] Configurable policy for handling deprecated interfaces Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 01/19] tests/test-qmp-cmds: Factor out qmp_dispatch() test helpers Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 02/19] tests/test-qmp-cmds: Check responses more thoroughly Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 03/19] tests/test-qmp-cmds: Simplify test data setup Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 04/19] tests/test-qmp-event: " Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 05/19] tests/test-qmp-event: Use qobject_is_equal() Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 06/19] tests/test-qmp-event: Check event is actually emitted Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 07/19] qapi: Add feature flags to remaining definitions Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 08/19] qapi: Consistently put @features parameter right after @ifcond Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 09/19] qapi: Inline do_qmp_dispatch() into qmp_dispatch() Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 10/19] qapi: Simplify how qmp_dispatch() deals with QCO_NO_SUCCESS_RESP Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 11/19] qapi: Simplify how qmp_dispatch() gets the request ID Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 12/19] qapi: Replace qmp_dispatch()'s TODO comment by an explanation Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 13/19] qapi: New special feature flag "deprecated" Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 14/19] qemu-options: New -compat to set policy for "funny" interfaces Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 15/19] qapi: Mark deprecated QMP commands with feature 'deprecated' Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 16/19] qapi: Implement -compat deprecated-input=reject for commands Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 17/19] qapi: Implement -compat deprecated-input=crash " Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 18/19] qapi: Include a warning in the response to a deprecated command Markus Armbruster
2019-10-24 14:01 ` [libvirt] " Daniel P. Berrangé
2019-10-24 19:35 ` Markus Armbruster
2019-10-24 12:34 ` [RFC PATCH 19/19] qapi: Implement -compat deprecated-output=hide for events Markus Armbruster
2019-10-24 14:16 ` [libvirt] " Daniel P. Berrangé
2019-10-24 19:56 ` Markus Armbruster
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.