This policy suppresses deprecated bits in output, and thus permits
"testing the future". Implement it for QMP command results. Example:
when QEMU is run with -compat deprecated-output=hide, then
{"execute": "query-cpus-fast"}
yields
{"return": [{"thread-id": 9805, "props": {"core-id": 0, "thread-id": 0, "socket-id": 0}, "qom-path": "/machine/unattached/device[0]", "cpu-index": 0, "target": "x86_64"}]}
instead of
{"return": [{"arch": "x86", "thread-id": 22436, "props": {"core-id": 0, "thread-id": 0, "socket-id": 0}, "qom-path": "/machine/unattached/device[0]", "cpu-index": 0, "target": "x86_64"}]}
Note the suppression of deprecated member "arch".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
include/qapi/qobject-output-visitor.h | 9 ++++++
include/qapi/visitor-impl.h | 3 ++
include/qapi/visitor.h | 9 ++++++
qapi/qapi-visit-core.c | 9 ++++++
qapi/qobject-output-visitor.c | 19 +++++++++++
tests/test-qmp-cmds.c | 42 ++++++++++++++++++++++---
qapi/trace-events | 1 +
scripts/qapi/commands.py | 2 +-
scripts/qapi/visit.py | 12 +++++++
tests/qapi-schema/qapi-schema-test.json | 17 +++++-----
tests/qapi-schema/qapi-schema-test.out | 18 +++++------
11 files changed, 118 insertions(+), 23 deletions(-)
diff --git a/include/qapi/qobject-output-visitor.h b/include/qapi/qobject-output-visitor.h
index 2b1726baf5..29f4ea6aad 100644
--- a/include/qapi/qobject-output-visitor.h
+++ b/include/qapi/qobject-output-visitor.h
@@ -53,4 +53,13 @@ typedef struct QObjectOutputVisitor QObjectOutputVisitor;
*/
Visitor *qobject_output_visitor_new(QObject **result);
+/*
+ * Create a QObject output visitor for @obj for use with QMP
+ *
+ * This is like qobject_output_visitor_new(), except it obeys the
+ * policy for handling deprecated management interfaces set with
+ * -compat.
+ */
+Visitor *qobject_output_visitor_new_qmp(QObject **result);
+
#endif
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 8ccb3b6c20..a6b26b7a5b 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -110,6 +110,9 @@ struct Visitor
The core takes care of the return type in the public interface. */
void (*optional)(Visitor *v, const char *name, bool *present);
+ /* Optional */
+ bool (*deprecated)(Visitor *v, const char *name);
+
/* Must be set */
VisitorType type;
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index c5b23851a1..c89d51b2a4 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -449,6 +449,15 @@ void visit_end_alternate(Visitor *v, void **obj);
*/
bool visit_optional(Visitor *v, const char *name, bool *present);
+/*
+ * Should we visit deprecated member @name?
+ *
+ * @name must not be NULL. This function is only useful between
+ * visit_start_struct() and visit_end_struct(), since only objects
+ * have deprecated members.
+ */
+bool visit_deprecated(Visitor *v, const char *name);
+
/*
* Visit an enum value.
*
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 5365561b07..501b3ccdef 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -137,6 +137,15 @@ bool visit_optional(Visitor *v, const char *name, bool *present)
return *present;
}
+bool visit_deprecated(Visitor *v, const char *name)
+{
+ trace_visit_deprecated(v, name);
+ if (v->deprecated) {
+ return v->deprecated(v, name);
+ }
+ return true;
+}
+
bool visit_is_input(Visitor *v)
{
return v->type == VISITOR_INPUT;
diff --git a/qapi/qobject-output-visitor.c b/qapi/qobject-output-visitor.c
index 26d7be5ec9..84cee17596 100644
--- a/qapi/qobject-output-visitor.c
+++ b/qapi/qobject-output-visitor.c
@@ -13,6 +13,7 @@
*/
#include "qemu/osdep.h"
+#include "qapi/compat-policy.h"
#include "qapi/qobject-output-visitor.h"
#include "qapi/visitor-impl.h"
#include "qemu/queue.h"
@@ -31,6 +32,8 @@ typedef struct QStackEntry {
struct QObjectOutputVisitor {
Visitor visitor;
+ CompatPolicyOutput deprecated_policy;
+
QSLIST_HEAD(, QStackEntry) stack; /* Stack of unfinished containers */
QObject *root; /* Root of the output visit */
QObject **result; /* User's storage location for result */
@@ -198,6 +201,13 @@ static void qobject_output_type_null(Visitor *v, const char *name,
qobject_output_add(qov, name, qnull());
}
+static bool qobject_output_deprecated(Visitor *v, const char *name)
+{
+ QObjectOutputVisitor *qov = to_qov(v);
+
+ return qov->deprecated_policy != COMPAT_POLICY_OUTPUT_HIDE;
+}
+
/* Finish building, and return the root object.
* The root object is never null. The caller becomes the object's
* owner, and should use qobject_unref() when done with it. */
@@ -247,6 +257,7 @@ Visitor *qobject_output_visitor_new(QObject **result)
v->visitor.type_number = qobject_output_type_number;
v->visitor.type_any = qobject_output_type_any;
v->visitor.type_null = qobject_output_type_null;
+ v->visitor.deprecated = qobject_output_deprecated;
v->visitor.complete = qobject_output_complete;
v->visitor.free = qobject_output_free;
@@ -255,3 +266,11 @@ Visitor *qobject_output_visitor_new(QObject **result)
return &v->visitor;
}
+
+Visitor *qobject_output_visitor_new_qmp(QObject **result)
+{
+ QObjectOutputVisitor *v = to_qov(qobject_output_visitor_new(result));
+
+ v->deprecated_policy = compat_policy.deprecated_output;
+ return &v->visitor;
+}
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index d12ff47e26..82d599630c 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"
@@ -45,12 +46,17 @@ void qmp_user_def_cmd1(UserDefOne * ud1, Error **errp)
{
}
-void qmp_test_features0(FeatureStruct0 *fs0, FeatureStruct1 *fs1,
- FeatureStruct2 *fs2, FeatureStruct3 *fs3,
- FeatureStruct4 *fs4, CondFeatureStruct1 *cfs1,
- CondFeatureStruct2 *cfs2, CondFeatureStruct3 *cfs3,
- Error **errp)
+FeatureStruct1 *qmp_test_features0(bool has_fs0, FeatureStruct0 *fs0,
+ bool has_fs1, FeatureStruct1 *fs1,
+ bool has_fs2, FeatureStruct2 *fs2,
+ bool has_fs3, FeatureStruct3 *fs3,
+ bool has_fs4, FeatureStruct4 *fs4,
+ bool has_cfs1, CondFeatureStruct1 *cfs1,
+ bool has_cfs2, CondFeatureStruct2 *cfs2,
+ bool has_cfs3, CondFeatureStruct3 *cfs3,
+ Error **errp)
{
+ return g_new(FeatureStruct1, 1);
}
void qmp_test_command_features1(Error **errp)
@@ -271,6 +277,30 @@ static void test_dispatch_cmd_io(void)
qobject_unref(ret3);
}
+static void test_dispatch_cmd_ret_deprecated(void)
+{
+ const char *cmd = "{ 'execute': 'test-features0' }";
+ QDict *ret;
+
+ memset(&compat_policy, 0, sizeof(compat_policy));
+
+ /* default accept */
+ ret = qobject_to(QDict, do_qmp_dispatch(false, cmd));
+ assert(ret && qdict_size(ret) == 1);
+ qobject_unref(ret);
+
+ compat_policy.has_deprecated_output = true;
+ compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_ACCEPT;
+ ret = qobject_to(QDict, do_qmp_dispatch(false, cmd));
+ assert(ret && qdict_size(ret) == 1);
+ qobject_unref(ret);
+
+ compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE;
+ ret = qobject_to(QDict, do_qmp_dispatch(false, cmd));
+ assert(ret && qdict_size(ret) == 0);
+ qobject_unref(ret);
+}
+
/* test generated dealloc functions for generated types */
static void test_dealloc_types(void)
{
@@ -345,6 +375,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_ret_deprecated",
+ test_dispatch_cmd_ret_deprecated);
g_test_add_func("/qmp/dealloc_types", test_dealloc_types);
g_test_add_func("/qmp/dealloc_partial", test_dealloc_partial);
diff --git a/qapi/trace-events b/qapi/trace-events
index 5eb4afa110..eff1fbd199 100644
--- a/qapi/trace-events
+++ b/qapi/trace-events
@@ -17,6 +17,7 @@ visit_start_alternate(void *v, const char *name, void *obj, size_t size) "v=%p n
visit_end_alternate(void *v, void *obj) "v=%p obj=%p"
visit_optional(void *v, const char *name, bool *present) "v=%p name=%s present=%p"
+visit_deprecated(void *v, const char *name) "v=%p name=%s"
visit_type_enum(void *v, const char *name, int *obj) "v=%p name=%s obj=%p"
visit_type_int(void *v, const char *name, int64_t *obj) "v=%p name=%s obj=%p"
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index bc30876c88..35b79c554d 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -69,7 +69,7 @@ static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out,
Error *err = NULL;
Visitor *v;
- v = qobject_output_visitor_new(ret_out);
+ v = qobject_output_visitor_new_qmp(ret_out);
visit_type_%(c_name)s(v, "unused", &ret_in, &err);
if (!err) {
visit_complete(v, ret_out);
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 23d9194aa4..21df3abed2 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -56,6 +56,7 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
c_type=base.c_name())
for memb in members:
+ deprecated = 'deprecated' in [f.name for f in memb.features]
ret += gen_if(memb.ifcond)
if memb.optional:
ret += mcgen('''
@@ -63,6 +64,12 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
''',
name=memb.name, c_name=c_name(memb.name))
push_indent()
+ if deprecated:
+ ret += mcgen('''
+ if (visit_deprecated(v, "%(name)s")) {
+''',
+ name=memb.name)
+ push_indent()
ret += mcgen('''
visit_type_%(c_type)s(v, "%(name)s", &obj->%(c_name)s, &err);
if (err) {
@@ -71,6 +78,11 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
''',
c_type=memb.type.c_name(), name=memb.name,
c_name=c_name(memb.name))
+ if deprecated:
+ pop_indent()
+ ret += mcgen('''
+ }
+''')
if memb.optional:
pop_indent()
ret += mcgen('''
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 6b1f05afa7..e4cce0d5b0 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -297,14 +297,15 @@
'features': [ 'feature1' ] }
{ 'command': 'test-features0',
- 'data': { 'fs0': 'FeatureStruct0',
- 'fs1': 'FeatureStruct1',
- 'fs2': 'FeatureStruct2',
- 'fs3': 'FeatureStruct3',
- 'fs4': 'FeatureStruct4',
- 'cfs1': 'CondFeatureStruct1',
- 'cfs2': 'CondFeatureStruct2',
- 'cfs3': 'CondFeatureStruct3' },
+ 'data': { '*fs0': 'FeatureStruct0',
+ '*fs1': 'FeatureStruct1',
+ '*fs2': 'FeatureStruct2',
+ '*fs3': 'FeatureStruct3',
+ '*fs4': 'FeatureStruct4',
+ '*cfs1': 'CondFeatureStruct1',
+ '*cfs2': 'CondFeatureStruct2',
+ '*cfs3': 'CondFeatureStruct3' },
+ 'returns': 'FeatureStruct1',
'features': [] }
{ 'command': 'test-command-features1',
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 891b4101e0..cd53323abd 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -407,15 +407,15 @@ alternate FeatureAlternate1
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
- member fs3: FeatureStruct3 optional=False
- member fs4: FeatureStruct4 optional=False
- member cfs1: CondFeatureStruct1 optional=False
- member cfs2: CondFeatureStruct2 optional=False
- member cfs3: CondFeatureStruct3 optional=False
-command test-features0 q_obj_test-features0-arg -> None
+ member fs0: FeatureStruct0 optional=True
+ member fs1: FeatureStruct1 optional=True
+ member fs2: FeatureStruct2 optional=True
+ member fs3: FeatureStruct3 optional=True
+ member fs4: FeatureStruct4 optional=True
+ member cfs1: CondFeatureStruct1 optional=True
+ member cfs2: CondFeatureStruct2 optional=True
+ member cfs3: CondFeatureStruct3 optional=True
+command test-features0 q_obj_test-features0-arg -> FeatureStruct1
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
--
2.21.1
Hi
On Sun, Mar 15, 2020 at 4:11 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> This policy suppresses deprecated bits in output, and thus permits
> "testing the future". Implement it for QMP command results. Example:
> when QEMU is run with -compat deprecated-output=hide, then
>
> {"execute": "query-cpus-fast"}
>
> yields
>
> {"return": [{"thread-id": 9805, "props": {"core-id": 0, "thread-id": 0, "socket-id": 0}, "qom-path": "/machine/unattached/device[0]", "cpu-index": 0, "target": "x86_64"}]}
>
> instead of
>
> {"return": [{"arch": "x86", "thread-id": 22436, "props": {"core-id": 0, "thread-id": 0, "socket-id": 0}, "qom-path": "/machine/unattached/device[0]", "cpu-index": 0, "target": "x86_64"}]}
>
> Note the suppression of deprecated member "arch".
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> include/qapi/qobject-output-visitor.h | 9 ++++++
> include/qapi/visitor-impl.h | 3 ++
> include/qapi/visitor.h | 9 ++++++
> qapi/qapi-visit-core.c | 9 ++++++
> qapi/qobject-output-visitor.c | 19 +++++++++++
> tests/test-qmp-cmds.c | 42 ++++++++++++++++++++++---
> qapi/trace-events | 1 +
> scripts/qapi/commands.py | 2 +-
> scripts/qapi/visit.py | 12 +++++++
> tests/qapi-schema/qapi-schema-test.json | 17 +++++-----
> tests/qapi-schema/qapi-schema-test.out | 18 +++++------
> 11 files changed, 118 insertions(+), 23 deletions(-)
>
> diff --git a/include/qapi/qobject-output-visitor.h b/include/qapi/qobject-output-visitor.h
> index 2b1726baf5..29f4ea6aad 100644
> --- a/include/qapi/qobject-output-visitor.h
> +++ b/include/qapi/qobject-output-visitor.h
> @@ -53,4 +53,13 @@ typedef struct QObjectOutputVisitor QObjectOutputVisitor;
> */
> Visitor *qobject_output_visitor_new(QObject **result);
>
> +/*
> + * Create a QObject output visitor for @obj for use with QMP
> + *
> + * This is like qobject_output_visitor_new(), except it obeys the
> + * policy for handling deprecated management interfaces set with
> + * -compat.
> + */
> +Visitor *qobject_output_visitor_new_qmp(QObject **result);
> +
> #endif
> diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
> index 8ccb3b6c20..a6b26b7a5b 100644
> --- a/include/qapi/visitor-impl.h
> +++ b/include/qapi/visitor-impl.h
> @@ -110,6 +110,9 @@ struct Visitor
> The core takes care of the return type in the public interface. */
> void (*optional)(Visitor *v, const char *name, bool *present);
>
> + /* Optional */
> + bool (*deprecated)(Visitor *v, const char *name);
> +
> /* Must be set */
> VisitorType type;
>
> diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
> index c5b23851a1..c89d51b2a4 100644
> --- a/include/qapi/visitor.h
> +++ b/include/qapi/visitor.h
> @@ -449,6 +449,15 @@ void visit_end_alternate(Visitor *v, void **obj);
> */
> bool visit_optional(Visitor *v, const char *name, bool *present);
>
> +/*
> + * Should we visit deprecated member @name?
> + *
> + * @name must not be NULL. This function is only useful between
> + * visit_start_struct() and visit_end_struct(), since only objects
> + * have deprecated members.
> + */
> +bool visit_deprecated(Visitor *v, const char *name);
> +
> /*
> * Visit an enum value.
> *
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index 5365561b07..501b3ccdef 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -137,6 +137,15 @@ bool visit_optional(Visitor *v, const char *name, bool *present)
> return *present;
> }
>
> +bool visit_deprecated(Visitor *v, const char *name)
> +{
> + trace_visit_deprecated(v, name);
> + if (v->deprecated) {
> + return v->deprecated(v, name);
> + }
> + return true;
> +}
> +
> bool visit_is_input(Visitor *v)
> {
> return v->type == VISITOR_INPUT;
> diff --git a/qapi/qobject-output-visitor.c b/qapi/qobject-output-visitor.c
> index 26d7be5ec9..84cee17596 100644
> --- a/qapi/qobject-output-visitor.c
> +++ b/qapi/qobject-output-visitor.c
> @@ -13,6 +13,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qapi/compat-policy.h"
> #include "qapi/qobject-output-visitor.h"
> #include "qapi/visitor-impl.h"
> #include "qemu/queue.h"
> @@ -31,6 +32,8 @@ typedef struct QStackEntry {
>
> struct QObjectOutputVisitor {
> Visitor visitor;
> + CompatPolicyOutput deprecated_policy;
> +
> QSLIST_HEAD(, QStackEntry) stack; /* Stack of unfinished containers */
> QObject *root; /* Root of the output visit */
> QObject **result; /* User's storage location for result */
> @@ -198,6 +201,13 @@ static void qobject_output_type_null(Visitor *v, const char *name,
> qobject_output_add(qov, name, qnull());
> }
>
> +static bool qobject_output_deprecated(Visitor *v, const char *name)
> +{
> + QObjectOutputVisitor *qov = to_qov(v);
> +
> + return qov->deprecated_policy != COMPAT_POLICY_OUTPUT_HIDE;
> +}
> +
> /* Finish building, and return the root object.
> * The root object is never null. The caller becomes the object's
> * owner, and should use qobject_unref() when done with it. */
> @@ -247,6 +257,7 @@ Visitor *qobject_output_visitor_new(QObject **result)
> v->visitor.type_number = qobject_output_type_number;
> v->visitor.type_any = qobject_output_type_any;
> v->visitor.type_null = qobject_output_type_null;
> + v->visitor.deprecated = qobject_output_deprecated;
> v->visitor.complete = qobject_output_complete;
> v->visitor.free = qobject_output_free;
>
> @@ -255,3 +266,11 @@ Visitor *qobject_output_visitor_new(QObject **result)
>
> return &v->visitor;
> }
> +
> +Visitor *qobject_output_visitor_new_qmp(QObject **result)
> +{
> + QObjectOutputVisitor *v = to_qov(qobject_output_visitor_new(result));
> +
> + v->deprecated_policy = compat_policy.deprecated_output;
> + return &v->visitor;
> +}
> diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
> index d12ff47e26..82d599630c 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"
> @@ -45,12 +46,17 @@ void qmp_user_def_cmd1(UserDefOne * ud1, Error **errp)
> {
> }
>
> -void qmp_test_features0(FeatureStruct0 *fs0, FeatureStruct1 *fs1,
> - FeatureStruct2 *fs2, FeatureStruct3 *fs3,
> - FeatureStruct4 *fs4, CondFeatureStruct1 *cfs1,
> - CondFeatureStruct2 *cfs2, CondFeatureStruct3 *cfs3,
> - Error **errp)
> +FeatureStruct1 *qmp_test_features0(bool has_fs0, FeatureStruct0 *fs0,
> + bool has_fs1, FeatureStruct1 *fs1,
> + bool has_fs2, FeatureStruct2 *fs2,
> + bool has_fs3, FeatureStruct3 *fs3,
> + bool has_fs4, FeatureStruct4 *fs4,
> + bool has_cfs1, CondFeatureStruct1 *cfs1,
> + bool has_cfs2, CondFeatureStruct2 *cfs2,
> + bool has_cfs3, CondFeatureStruct3 *cfs3,
> + Error **errp)
> {
> + return g_new(FeatureStruct1, 1);
> }
>
> void qmp_test_command_features1(Error **errp)
> @@ -271,6 +277,30 @@ static void test_dispatch_cmd_io(void)
> qobject_unref(ret3);
> }
>
> +static void test_dispatch_cmd_ret_deprecated(void)
> +{
> + const char *cmd = "{ 'execute': 'test-features0' }";
> + QDict *ret;
> +
> + memset(&compat_policy, 0, sizeof(compat_policy));
> +
> + /* default accept */
> + ret = qobject_to(QDict, do_qmp_dispatch(false, cmd));
> + assert(ret && qdict_size(ret) == 1);
> + qobject_unref(ret);
> +
> + compat_policy.has_deprecated_output = true;
> + compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_ACCEPT;
> + ret = qobject_to(QDict, do_qmp_dispatch(false, cmd));
> + assert(ret && qdict_size(ret) == 1);
> + qobject_unref(ret);
> +
> + compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE;
> + ret = qobject_to(QDict, do_qmp_dispatch(false, cmd));
> + assert(ret && qdict_size(ret) == 0);
> + qobject_unref(ret);
> +}
> +
> /* test generated dealloc functions for generated types */
> static void test_dealloc_types(void)
> {
> @@ -345,6 +375,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_ret_deprecated",
> + test_dispatch_cmd_ret_deprecated);
> g_test_add_func("/qmp/dealloc_types", test_dealloc_types);
> g_test_add_func("/qmp/dealloc_partial", test_dealloc_partial);
>
> diff --git a/qapi/trace-events b/qapi/trace-events
> index 5eb4afa110..eff1fbd199 100644
> --- a/qapi/trace-events
> +++ b/qapi/trace-events
> @@ -17,6 +17,7 @@ visit_start_alternate(void *v, const char *name, void *obj, size_t size) "v=%p n
> visit_end_alternate(void *v, void *obj) "v=%p obj=%p"
>
> visit_optional(void *v, const char *name, bool *present) "v=%p name=%s present=%p"
> +visit_deprecated(void *v, const char *name) "v=%p name=%s"
>
> visit_type_enum(void *v, const char *name, int *obj) "v=%p name=%s obj=%p"
> visit_type_int(void *v, const char *name, int64_t *obj) "v=%p name=%s obj=%p"
> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
> index bc30876c88..35b79c554d 100644
> --- a/scripts/qapi/commands.py
> +++ b/scripts/qapi/commands.py
> @@ -69,7 +69,7 @@ static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out,
> Error *err = NULL;
> Visitor *v;
>
> - v = qobject_output_visitor_new(ret_out);
> + v = qobject_output_visitor_new_qmp(ret_out);
> visit_type_%(c_name)s(v, "unused", &ret_in, &err);
> if (!err) {
> visit_complete(v, ret_out);
> diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
> index 23d9194aa4..21df3abed2 100644
> --- a/scripts/qapi/visit.py
> +++ b/scripts/qapi/visit.py
> @@ -56,6 +56,7 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
> c_type=base.c_name())
>
> for memb in members:
> + deprecated = 'deprecated' in [f.name for f in memb.features]
> ret += gen_if(memb.ifcond)
> if memb.optional:
> ret += mcgen('''
> @@ -63,6 +64,12 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
> ''',
> name=memb.name, c_name=c_name(memb.name))
> push_indent()
> + if deprecated:
> + ret += mcgen('''
> + if (visit_deprecated(v, "%(name)s")) {
Do you have a compelling case where the "name" would change the
deprecation policy? I think that could be more confusing than
necessary, say if x- name wouldn't follow the policy.
> +''',
> + name=memb.name)
> + push_indent()
> ret += mcgen('''
> visit_type_%(c_type)s(v, "%(name)s", &obj->%(c_name)s, &err);
> if (err) {
> @@ -71,6 +78,11 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
> ''',
> c_type=memb.type.c_name(), name=memb.name,
> c_name=c_name(memb.name))
> + if deprecated:
> + pop_indent()
> + ret += mcgen('''
> + }
> +''')
> if memb.optional:
> pop_indent()
> ret += mcgen('''
> diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
> index 6b1f05afa7..e4cce0d5b0 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -297,14 +297,15 @@
> 'features': [ 'feature1' ] }
>
> { 'command': 'test-features0',
> - 'data': { 'fs0': 'FeatureStruct0',
> - 'fs1': 'FeatureStruct1',
> - 'fs2': 'FeatureStruct2',
> - 'fs3': 'FeatureStruct3',
> - 'fs4': 'FeatureStruct4',
> - 'cfs1': 'CondFeatureStruct1',
> - 'cfs2': 'CondFeatureStruct2',
> - 'cfs3': 'CondFeatureStruct3' },
> + 'data': { '*fs0': 'FeatureStruct0',
> + '*fs1': 'FeatureStruct1',
> + '*fs2': 'FeatureStruct2',
> + '*fs3': 'FeatureStruct3',
> + '*fs4': 'FeatureStruct4',
> + '*cfs1': 'CondFeatureStruct1',
> + '*cfs2': 'CondFeatureStruct2',
> + '*cfs3': 'CondFeatureStruct3' },
> + 'returns': 'FeatureStruct1',
> 'features': [] }
>
> { 'command': 'test-command-features1',
> diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
> index 891b4101e0..cd53323abd 100644
> --- a/tests/qapi-schema/qapi-schema-test.out
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -407,15 +407,15 @@ alternate FeatureAlternate1
> 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
> - member fs3: FeatureStruct3 optional=False
> - member fs4: FeatureStruct4 optional=False
> - member cfs1: CondFeatureStruct1 optional=False
> - member cfs2: CondFeatureStruct2 optional=False
> - member cfs3: CondFeatureStruct3 optional=False
> -command test-features0 q_obj_test-features0-arg -> None
> + member fs0: FeatureStruct0 optional=True
> + member fs1: FeatureStruct1 optional=True
> + member fs2: FeatureStruct2 optional=True
> + member fs3: FeatureStruct3 optional=True
> + member fs4: FeatureStruct4 optional=True
> + member cfs1: CondFeatureStruct1 optional=True
> + member cfs2: CondFeatureStruct2 optional=True
> + member cfs3: CondFeatureStruct3 optional=True
> +command test-features0 q_obj_test-features0-arg -> FeatureStruct1
> 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
> --
> 2.21.1
>
>
--
Marc-André Lureau
Marc-André Lureau <marcandre.lureau@gmail.com> writes:
> Hi
>
> On Sun, Mar 15, 2020 at 4:11 PM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> This policy suppresses deprecated bits in output, and thus permits
>> "testing the future". Implement it for QMP command results. Example:
>> when QEMU is run with -compat deprecated-output=hide, then
>>
>> {"execute": "query-cpus-fast"}
>>
>> yields
>>
>> {"return": [{"thread-id": 9805, "props": {"core-id": 0, "thread-id": 0, "socket-id": 0}, "qom-path": "/machine/unattached/device[0]", "cpu-index": 0, "target": "x86_64"}]}
>>
>> instead of
>>
>> {"return": [{"arch": "x86", "thread-id": 22436, "props": {"core-id": 0, "thread-id": 0, "socket-id": 0}, "qom-path": "/machine/unattached/device[0]", "cpu-index": 0, "target": "x86_64"}]}
>>
>> Note the suppression of deprecated member "arch".
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> include/qapi/qobject-output-visitor.h | 9 ++++++
>> include/qapi/visitor-impl.h | 3 ++
>> include/qapi/visitor.h | 9 ++++++
>> qapi/qapi-visit-core.c | 9 ++++++
>> qapi/qobject-output-visitor.c | 19 +++++++++++
>> tests/test-qmp-cmds.c | 42 ++++++++++++++++++++++---
>> qapi/trace-events | 1 +
>> scripts/qapi/commands.py | 2 +-
>> scripts/qapi/visit.py | 12 +++++++
>> tests/qapi-schema/qapi-schema-test.json | 17 +++++-----
>> tests/qapi-schema/qapi-schema-test.out | 18 +++++------
>> 11 files changed, 118 insertions(+), 23 deletions(-)
>>
>> diff --git a/include/qapi/qobject-output-visitor.h b/include/qapi/qobject-output-visitor.h
>> index 2b1726baf5..29f4ea6aad 100644
>> --- a/include/qapi/qobject-output-visitor.h
>> +++ b/include/qapi/qobject-output-visitor.h
>> @@ -53,4 +53,13 @@ typedef struct QObjectOutputVisitor QObjectOutputVisitor;
>> */
>> Visitor *qobject_output_visitor_new(QObject **result);
>>
>> +/*
>> + * Create a QObject output visitor for @obj for use with QMP
>> + *
>> + * This is like qobject_output_visitor_new(), except it obeys the
>> + * policy for handling deprecated management interfaces set with
>> + * -compat.
>> + */
>> +Visitor *qobject_output_visitor_new_qmp(QObject **result);
>> +
>> #endif
>> diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
>> index 8ccb3b6c20..a6b26b7a5b 100644
>> --- a/include/qapi/visitor-impl.h
>> +++ b/include/qapi/visitor-impl.h
>> @@ -110,6 +110,9 @@ struct Visitor
>> The core takes care of the return type in the public interface. */
>> void (*optional)(Visitor *v, const char *name, bool *present);
>>
>> + /* Optional */
>> + bool (*deprecated)(Visitor *v, const char *name);
>> +
>> /* Must be set */
>> VisitorType type;
>>
>> diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
>> index c5b23851a1..c89d51b2a4 100644
>> --- a/include/qapi/visitor.h
>> +++ b/include/qapi/visitor.h
>> @@ -449,6 +449,15 @@ void visit_end_alternate(Visitor *v, void **obj);
>> */
>> bool visit_optional(Visitor *v, const char *name, bool *present);
>>
>> +/*
>> + * Should we visit deprecated member @name?
>> + *
>> + * @name must not be NULL. This function is only useful between
>> + * visit_start_struct() and visit_end_struct(), since only objects
>> + * have deprecated members.
>> + */
>> +bool visit_deprecated(Visitor *v, const char *name);
>> +
>> /*
>> * Visit an enum value.
>> *
>> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
>> index 5365561b07..501b3ccdef 100644
>> --- a/qapi/qapi-visit-core.c
>> +++ b/qapi/qapi-visit-core.c
>> @@ -137,6 +137,15 @@ bool visit_optional(Visitor *v, const char *name, bool *present)
>> return *present;
>> }
>>
>> +bool visit_deprecated(Visitor *v, const char *name)
>> +{
>> + trace_visit_deprecated(v, name);
>> + if (v->deprecated) {
>> + return v->deprecated(v, name);
>> + }
>> + return true;
>> +}
>> +
>> bool visit_is_input(Visitor *v)
>> {
>> return v->type == VISITOR_INPUT;
>> diff --git a/qapi/qobject-output-visitor.c b/qapi/qobject-output-visitor.c
>> index 26d7be5ec9..84cee17596 100644
>> --- a/qapi/qobject-output-visitor.c
>> +++ b/qapi/qobject-output-visitor.c
>> @@ -13,6 +13,7 @@
>> */
>>
>> #include "qemu/osdep.h"
>> +#include "qapi/compat-policy.h"
>> #include "qapi/qobject-output-visitor.h"
>> #include "qapi/visitor-impl.h"
>> #include "qemu/queue.h"
>> @@ -31,6 +32,8 @@ typedef struct QStackEntry {
>>
>> struct QObjectOutputVisitor {
>> Visitor visitor;
>> + CompatPolicyOutput deprecated_policy;
>> +
>> QSLIST_HEAD(, QStackEntry) stack; /* Stack of unfinished containers */
>> QObject *root; /* Root of the output visit */
>> QObject **result; /* User's storage location for result */
>> @@ -198,6 +201,13 @@ static void qobject_output_type_null(Visitor *v, const char *name,
>> qobject_output_add(qov, name, qnull());
>> }
>>
>> +static bool qobject_output_deprecated(Visitor *v, const char *name)
>> +{
>> + QObjectOutputVisitor *qov = to_qov(v);
>> +
>> + return qov->deprecated_policy != COMPAT_POLICY_OUTPUT_HIDE;
>> +}
>> +
>> /* Finish building, and return the root object.
>> * The root object is never null. The caller becomes the object's
>> * owner, and should use qobject_unref() when done with it. */
>> @@ -247,6 +257,7 @@ Visitor *qobject_output_visitor_new(QObject **result)
>> v->visitor.type_number = qobject_output_type_number;
>> v->visitor.type_any = qobject_output_type_any;
>> v->visitor.type_null = qobject_output_type_null;
>> + v->visitor.deprecated = qobject_output_deprecated;
>> v->visitor.complete = qobject_output_complete;
>> v->visitor.free = qobject_output_free;
>>
>> @@ -255,3 +266,11 @@ Visitor *qobject_output_visitor_new(QObject **result)
>>
>> return &v->visitor;
>> }
>> +
>> +Visitor *qobject_output_visitor_new_qmp(QObject **result)
>> +{
>> + QObjectOutputVisitor *v = to_qov(qobject_output_visitor_new(result));
>> +
>> + v->deprecated_policy = compat_policy.deprecated_output;
>> + return &v->visitor;
>> +}
>> diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
>> index d12ff47e26..82d599630c 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"
>> @@ -45,12 +46,17 @@ void qmp_user_def_cmd1(UserDefOne * ud1, Error **errp)
>> {
>> }
>>
>> -void qmp_test_features0(FeatureStruct0 *fs0, FeatureStruct1 *fs1,
>> - FeatureStruct2 *fs2, FeatureStruct3 *fs3,
>> - FeatureStruct4 *fs4, CondFeatureStruct1 *cfs1,
>> - CondFeatureStruct2 *cfs2, CondFeatureStruct3 *cfs3,
>> - Error **errp)
>> +FeatureStruct1 *qmp_test_features0(bool has_fs0, FeatureStruct0 *fs0,
>> + bool has_fs1, FeatureStruct1 *fs1,
>> + bool has_fs2, FeatureStruct2 *fs2,
>> + bool has_fs3, FeatureStruct3 *fs3,
>> + bool has_fs4, FeatureStruct4 *fs4,
>> + bool has_cfs1, CondFeatureStruct1 *cfs1,
>> + bool has_cfs2, CondFeatureStruct2 *cfs2,
>> + bool has_cfs3, CondFeatureStruct3 *cfs3,
>> + Error **errp)
>> {
>> + return g_new(FeatureStruct1, 1);
>> }
>>
>> void qmp_test_command_features1(Error **errp)
>> @@ -271,6 +277,30 @@ static void test_dispatch_cmd_io(void)
>> qobject_unref(ret3);
>> }
>>
>> +static void test_dispatch_cmd_ret_deprecated(void)
>> +{
>> + const char *cmd = "{ 'execute': 'test-features0' }";
>> + QDict *ret;
>> +
>> + memset(&compat_policy, 0, sizeof(compat_policy));
>> +
>> + /* default accept */
>> + ret = qobject_to(QDict, do_qmp_dispatch(false, cmd));
>> + assert(ret && qdict_size(ret) == 1);
>> + qobject_unref(ret);
>> +
>> + compat_policy.has_deprecated_output = true;
>> + compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_ACCEPT;
>> + ret = qobject_to(QDict, do_qmp_dispatch(false, cmd));
>> + assert(ret && qdict_size(ret) == 1);
>> + qobject_unref(ret);
>> +
>> + compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE;
>> + ret = qobject_to(QDict, do_qmp_dispatch(false, cmd));
>> + assert(ret && qdict_size(ret) == 0);
>> + qobject_unref(ret);
>> +}
>> +
>> /* test generated dealloc functions for generated types */
>> static void test_dealloc_types(void)
>> {
>> @@ -345,6 +375,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_ret_deprecated",
>> + test_dispatch_cmd_ret_deprecated);
>> g_test_add_func("/qmp/dealloc_types", test_dealloc_types);
>> g_test_add_func("/qmp/dealloc_partial", test_dealloc_partial);
>>
>> diff --git a/qapi/trace-events b/qapi/trace-events
>> index 5eb4afa110..eff1fbd199 100644
>> --- a/qapi/trace-events
>> +++ b/qapi/trace-events
>> @@ -17,6 +17,7 @@ visit_start_alternate(void *v, const char *name, void *obj, size_t size) "v=%p n
>> visit_end_alternate(void *v, void *obj) "v=%p obj=%p"
>>
>> visit_optional(void *v, const char *name, bool *present) "v=%p name=%s present=%p"
>> +visit_deprecated(void *v, const char *name) "v=%p name=%s"
>>
>> visit_type_enum(void *v, const char *name, int *obj) "v=%p name=%s obj=%p"
>> visit_type_int(void *v, const char *name, int64_t *obj) "v=%p name=%s obj=%p"
>> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
>> index bc30876c88..35b79c554d 100644
>> --- a/scripts/qapi/commands.py
>> +++ b/scripts/qapi/commands.py
>> @@ -69,7 +69,7 @@ static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out,
>> Error *err = NULL;
>> Visitor *v;
>>
>> - v = qobject_output_visitor_new(ret_out);
>> + v = qobject_output_visitor_new_qmp(ret_out);
>> visit_type_%(c_name)s(v, "unused", &ret_in, &err);
>> if (!err) {
>> visit_complete(v, ret_out);
>> diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
>> index 23d9194aa4..21df3abed2 100644
>> --- a/scripts/qapi/visit.py
>> +++ b/scripts/qapi/visit.py
>> @@ -56,6 +56,7 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
>> c_type=base.c_name())
>>
>> for memb in members:
>> + deprecated = 'deprecated' in [f.name for f in memb.features]
>> ret += gen_if(memb.ifcond)
>> if memb.optional:
>> ret += mcgen('''
>> @@ -63,6 +64,12 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
>> ''',
>> name=memb.name, c_name=c_name(memb.name))
>> push_indent()
>> + if deprecated:
>> + ret += mcgen('''
>> + if (visit_deprecated(v, "%(name)s")) {
>
> Do you have a compelling case where the "name" would change the
> deprecation policy? I think that could be more confusing than
> necessary, say if x- name wouldn't follow the policy.
Yes, that would be a bad idea.
Intended use is error messages, just like we do elsewhere. The output
visitor's method qobject_output_deprecated() can't fail, but the input
visitor's method can; see PATCH 33.
[...]
© 2016 - 2026 Red Hat, Inc.