[PATCH v5 0/8] Configurable policy for handling deprecated interfaces

[PATCH v5 0/8] Configurable policy for handling deprecated interfaces

[Markus Armbruster]

New option -compat lets you configure what to do when deprecated
interfaces get used.  This is intended for testing users of the
management interfaces.  It is experimental.

-compat deprecated-input=<in-policy> configures what to do when
deprecated input is received.  Available policies:

* accept: Accept deprecated commands and arguments (default)
* reject: Reject them
* crash: Crash

-compat deprecated-output=<out-policy> configures what to do when
deprecated output is sent.  Available output policies:

* accept: Emit deprecated command results and events (default)
* hide: Suppress them

For now, -compat covers only deprecated syntactic aspects of QMP.  We
may want to extend it to cover semantic aspects, CLI, and experimental
features.

v5:
* Old PATCH 01-26 merged in commit f57587c7d47.
* Rebased, non-trivial conflicts in PATCH 1 due to Meson, and in PATCH
  7 due to visitor changes
* PATCH 1: Comments updated for 5.2 [Eric]
* PATCH 2: Harmless missing initialization fixed [Eric]
* PATCH 3+4: Harmless missing has_FOO = true fixed [Eric]
* PATCH 6+7: Commit message tweaked

v4:
* PATCH 05+07: Temporary memory leak plugged [Marc-André]
* PATCH 23: Rewritten [Marc-André]
* PATCH 24: Comment typo [Marc-André]
* PATCH 30: Memory leaks plugged

v3:
* Rebased, non-trivial conflicts in PATCH 01+26+27+34 due to RST
  conversion and code motion
* PATCH 28-29: Old PATCH 28 split up to ease review
* PATCH 30-31: New
* PATCH 32-33: Old PATCH 29 split up to ease review

Comparison to RFC (24 Oct 2019):
* Cover arguments and results in addition to commands and events
* Half-baked "[RFC PATCH 18/19] qapi: Include a warning in the
  response to a deprecated command" dropped

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

Cc: Lukáš Doktor <ldoktor@redhat.com>
Cc: libguestfs@redhat.com
Cc: libvir-list@redhat.com
Cc: Daniel P. Berrange <berrange@redhat.com>
Cc: Peter Krempa <pkrempa@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>

Markus Armbruster (8):
  qemu-options: New -compat to set policy for deprecated interfaces
  qapi: Implement deprecated-output=hide for QMP command results
  qapi: Implement deprecated-output=hide for QMP events
  qapi: Implement deprecated-output=hide for QMP event data
  qapi: Implement deprecated-output=hide for QMP introspection
  qapi: Implement deprecated-input=reject for QMP commands
  qapi: Implement deprecated-input=reject for QMP command arguments
  qapi: New -compat deprecated-input=crash

 qapi/compat.json                        |  52 ++++++++++++
 qapi/introspect.json                    |   2 +-
 qapi/qapi-schema.json                   |   1 +
 include/qapi/compat-policy.h            |  20 +++++
 include/qapi/qmp/dispatch.h             |   1 +
 include/qapi/qobject-input-visitor.h    |   9 +++
 include/qapi/qobject-output-visitor.h   |   9 +++
 include/qapi/visitor-impl.h             |   6 ++
 include/qapi/visitor.h                  |  18 +++++
 monitor/monitor-internal.h              |   3 -
 monitor/misc.c                          |   2 -
 monitor/qmp-cmds-control.c              | 100 +++++++++++++++++++++---
 qapi/qapi-visit-core.c                  |  18 +++++
 qapi/qmp-dispatch.c                     |  17 ++++
 qapi/qobject-input-visitor.c            |  29 +++++++
 qapi/qobject-output-visitor.c           |  19 +++++
 softmmu/vl.c                            |  17 ++++
 storage-daemon/qemu-storage-daemon.c    |   2 -
 tests/test-qmp-cmds.c                   |  91 +++++++++++++++++++--
 tests/test-qmp-event.c                  |  41 ++++++++++
 qapi/meson.build                        |   1 +
 qapi/trace-events                       |   2 +
 qemu-options.hx                         |  22 ++++++
 scripts/qapi/commands.py                |  14 ++--
 scripts/qapi/events.py                  |  22 +++++-
 scripts/qapi/visit.py                   |  15 ++++
 tests/qapi-schema/qapi-schema-test.json |  20 +++--
 tests/qapi-schema/qapi-schema-test.out  |  20 ++---
 28 files changed, 522 insertions(+), 51 deletions(-)
 create mode 100644 qapi/compat.json
 create mode 100644 include/qapi/compat-policy.h

-- 
2.26.2

[PATCH v5 1/8] qemu-options: New -compat to set policy for deprecated interfaces

[Markus Armbruster]

Policy is separate for input and output.

Input policy can be "accept" (accept silently), or "reject" (reject
the request with an error).

Output policy can be "accept" (pass on unchanged), or "hide" (filter
out the deprecated parts).

Default is "accept".  Policies other than "accept" are implemented
later in this series.

For now, -compat covers only syntactic aspects of QMP, i.e. stuff
tagged with feature 'deprecated'.  We may want to extend it to cover
semantic aspects, CLI, and experimental features.

The option is experimental.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/compat.json             | 51 ++++++++++++++++++++++++++++++++++++
 qapi/qapi-schema.json        |  1 +
 include/qapi/compat-policy.h | 20 ++++++++++++++
 qapi/qmp-dispatch.c          |  3 +++
 softmmu/vl.c                 | 17 ++++++++++++
 qapi/meson.build             |  1 +
 qemu-options.hx              | 20 ++++++++++++++
 7 files changed, 113 insertions(+)
 create mode 100644 qapi/compat.json
 create mode 100644 include/qapi/compat-policy.h

diff --git a/qapi/compat.json b/qapi/compat.json
new file mode 100644
index 0000000000..d2c02a21aa
--- /dev/null
+++ b/qapi/compat.json
@@ -0,0 +1,51 @@
+# -*- Mode: Python -*-
+
+##
+# = Compatibility policy
+##
+
+##
+# @CompatPolicyInput:
+#
+# Policy for handling "funny" input.
+#
+# @accept: Accept silently
+# @reject: Reject with an error
+#
+# Since: 5.2
+##
+{ 'enum': 'CompatPolicyInput',
+  'data': [ 'accept', 'reject' ] }
+
+##
+# @CompatPolicyOutput:
+#
+# Policy for handling "funny" output.
+#
+# @accept: Pass on unchanged
+# @hide: Filter out
+#
+# Since: 5.2
+##
+{ 'enum': 'CompatPolicyOutput',
+  'data': [ 'accept', 'hide' ] }
+
+##
+# @CompatPolicy:
+#
+# Policy for handling deprecated management interfaces.
+#
+# This is intended for testing users of the management interfaces.
+#
+# Limitation: covers only syntactic aspects of QMP, i.e. stuff tagged
+# with feature 'deprecated'.  We may want to extend it to cover
+# semantic aspects, CLI, and experimental features.
+#
+# @deprecated-input: how to handle deprecated input (default 'accept')
+# @deprecated-output: how to handle deprecated output (default 'accept')
+#
+# Since: 5.2
+##
+{ 'struct': 'CompatPolicy',
+  'data': { '*deprecated-input': 'CompatPolicyInput',
+            '*deprecated-output': 'CompatPolicyOutput' } }
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index f03ff91ceb..2550b16028 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -76,6 +76,7 @@
 { 'include': 'migration.json' }
 { 'include': 'transaction.json' }
 { 'include': 'trace.json' }
+{ 'include': 'compat.json' }
 { 'include': 'control.json' }
 { 'include': 'introspect.json' }
 { 'include': 'qom.json' }
diff --git a/include/qapi/compat-policy.h b/include/qapi/compat-policy.h
new file mode 100644
index 0000000000..b8c6638156
--- /dev/null
+++ b/include/qapi/compat-policy.h
@@ -0,0 +1,20 @@
+/*
+ * Policy for handling "funny" management interfaces
+ *
+ * Copyright (C) 2020 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-compat.h"
+
+extern CompatPolicy compat_policy;
+
+#endif
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 79347e0864..f65b8cf000 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 compat_policy;
+
 static QDict *qmp_dispatch_check_obj(QDict *dict, bool allow_oob,
                                      Error **errp)
 {
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 0cc86b0766..dbe9dc06f0 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -27,6 +27,7 @@
 #include "qemu/units.h"
 #include "hw/boards.h"
 #include "hw/qdev-properties.h"
+#include "qapi/compat-policy.h"
 #include "qapi/error.h"
 #include "qemu-version.h"
 #include "qemu/cutils.h"
@@ -106,6 +107,7 @@
 #include "sysemu/replay.h"
 #include "qapi/qapi-events-run-state.h"
 #include "qapi/qapi-visit-block-core.h"
+#include "qapi/qapi-visit-compat.h"
 #include "qapi/qapi-visit-ui.h"
 #include "qapi/qapi-commands-block-core.h"
 #include "qapi/qapi-commands-run-state.h"
@@ -3771,6 +3773,21 @@ void qemu_init(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, &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/qapi/meson.build b/qapi/meson.build
index 2b2872a41d..c72fcd9616 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -20,6 +20,7 @@ qapi_all_modules = [
   'block',
   'char',
   'common',
+  'compat',
   'control',
   'crypto',
   'dump',
diff --git a/qemu-options.hx b/qemu-options.hx
index b0f020594e..bb0c6bb70e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3391,6 +3391,26 @@ DEFHEADING()
 
 DEFHEADING(Debug/Expert options:)
 
+DEF("compat", HAS_ARG, QEMU_OPTION_compat,
+    "-compat [deprecated-input=accept|reject][,deprecated-output=accept|hide]\n"
+    "                Policy for handling deprecated management interfaces\n",
+    QEMU_ARCH_ALL)
+SRST
+``-compat [deprecated-input=@var{input-policy}][,deprecated-output=@var{output-policy}]``
+    Set policy for handling deprecated management interfaces (experimental):
+
+    ``deprecated-input=accept`` (default)
+        Accept deprecated commands and arguments
+    ``deprecated-input=reject``
+        Reject deprecated commands and arguments
+    ``deprecated-output=accept`` (default)
+        Emit deprecated command results and events
+    ``deprecated-output=hide``
+        Suppress deprecated command results and events
+
+    Limitation: covers only syntactic aspects of QMP.
+ERST
+
 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.26.2

[PATCH v5 2/8] qapi: Implement deprecated-output=hide for QMP command results

[Markus Armbruster]

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>
Reviewed-by: Eric Blake <eblake@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 7362c043be..2d853255a3 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -113,6 +113,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 ebc19ede7f..4d23b59853 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -459,6 +459,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 7e5f40e7f0..d9726ecaa1 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -135,6 +135,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 ba6f6ac8a7..5c4aa0f64d 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 */
@@ -207,6 +210,13 @@ static bool qobject_output_type_null(Visitor *v, const char *name,
     return true;
 }
 
+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.  */
@@ -256,6 +266,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;
 
@@ -264,3 +275,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..bca89f8e88 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_new0(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 3cf9e1110b..14662515f3 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,
 {
     Visitor *v;
 
-    v = qobject_output_visitor_new(ret_out);
+    v = qobject_output_visitor_new_qmp(ret_out);
     if (visit_type_%(c_name)s(v, "unused", &ret_in, errp)) {
         visit_complete(v, ret_out);
     }
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index cdabc5fa28..110563e56f 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -53,6 +53,7 @@ bool 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('''
@@ -60,6 +61,12 @@ bool 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('''
     if (!visit_type_%(c_type)s(v, "%(name)s", &obj->%(c_name)s, errp)) {
         return false;
@@ -67,6 +74,11 @@ bool 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 3a9f2cbb33..309e60566d 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -298,14 +298,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.26.2

[PATCH v5 3/8] qapi: Implement deprecated-output=hide for QMP events

[Markus Armbruster]

This policy suppresses deprecated bits in output, and thus permits
"testing the future".  Implement it for QMP events: suppress
deprecated ones.

No QMP event is deprecated right now.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/test-qmp-event.c | 20 ++++++++++++++++++++
 scripts/qapi/events.py | 14 ++++++++++++--
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/tests/test-qmp-event.c b/tests/test-qmp-event.c
index 7dd0053190..ab059fb5c2 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,24 @@ 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' }");
+
+    memset(&compat_policy, 0, sizeof(compat_policy));
+
+    qapi_event_send_test_event_features1();
+    g_assert(data->emitted);
+
+    compat_policy.has_deprecated_output = true;
+    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 +167,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/scripts/qapi/events.py b/scripts/qapi/events.py
index b544af5a1c..95ca4b4753 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 (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.26.2

[PATCH v5 4/8] qapi: Implement deprecated-output=hide for QMP event data

[Markus Armbruster]

This policy suppresses deprecated bits in output, and thus permits
"testing the future".  Implement it for QMP event data: suppress
deprecated members.

No QMP event data is deprecated right now.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/test-qmp-event.c                  | 21 +++++++++++++++++++++
 scripts/qapi/events.py                  |  8 ++++++--
 tests/qapi-schema/qapi-schema-test.json |  3 +++
 tests/qapi-schema/qapi-schema-test.out  |  2 ++
 4 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/tests/test-qmp-event.c b/tests/test-qmp-event.c
index ab059fb5c2..047f44ff9a 100644
--- a/tests/test-qmp-event.c
+++ b/tests/test-qmp-event.c
@@ -159,6 +159,26 @@ static void test_event_deprecated(TestEventData *data, const void *unused)
     qobject_unref(data->expect);
 }
 
+static void test_event_deprecated_data(TestEventData *data, const void *unused)
+{
+    memset(&compat_policy, 0, sizeof(compat_policy));
+
+    data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES0',"
+                                           " 'data': { 'foo': 42 } }");
+    qapi_event_send_test_event_features0(42);
+    g_assert(data->emitted);
+
+    qobject_unref(data->expect);
+
+    compat_policy.has_deprecated_output = true;
+    compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE;
+    data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES0' }");
+    qapi_event_send_test_event_features0(42);
+    g_assert(data->emitted);
+
+    qobject_unref(data->expect);
+}
+
 int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
@@ -168,6 +188,7 @@ int main(int argc, char **argv)
     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);
+    event_test_add("/event/deprecated_data", test_event_deprecated_data);
     g_test_run();
 
     return 0;
diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index 95ca4b4753..f03c825cc1 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -104,7 +104,7 @@ def gen_event_send(name, arg_type, features, boxed,
 
     if have_args:
         ret += mcgen('''
-    v = qobject_output_visitor_new(&obj);
+    v = qobject_output_visitor_new_qmp(&obj);
 ''')
         if not arg_type.is_implicit():
             ret += mcgen('''
@@ -123,7 +123,11 @@ def gen_event_send(name, arg_type, features, boxed,
         ret += mcgen('''
 
     visit_complete(v, &obj);
-    qdict_put_obj(qmp, "data", obj);
+    if (qdict_size(qobject_to(QDict, obj))) {
+        qdict_put_obj(qmp, "data", obj);
+    } else {
+        qobject_unref(obj);
+    }
 ''')
 
     ret += mcgen('''
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 309e60566d..3f2943341e 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -323,5 +323,8 @@
   'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)',
                                               'defined(TEST_IF_COND_2)'] } ] }
 
+{ 'event': 'TEST-EVENT-FEATURES0',
+  'data': 'FeatureStruct1' }
+
 { 'event': 'TEST-EVENT-FEATURES1',
   'features': [ 'deprecated' ] }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index cd53323abd..1a63d3bca7 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -438,6 +438,8 @@ 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-FEATURES0 FeatureStruct1
+    boxed=False
 event TEST-EVENT-FEATURES1 None
     boxed=False
     feature deprecated
-- 
2.26.2

[PATCH v5 5/8] qapi: Implement deprecated-output=hide for QMP introspection

[Markus Armbruster]

This policy suppresses deprecated bits in output, and thus permits
"testing the future".  Implement it for QMP command query-qmp-schema:
suppress information on deprecated commands, events and object type
members, i.e. anything that has the special feature flag "deprecated".

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/introspect.json                 |   2 +-
 monitor/monitor-internal.h           |   3 -
 monitor/misc.c                       |   2 -
 monitor/qmp-cmds-control.c           | 100 +++++++++++++++++++++++----
 storage-daemon/qemu-storage-daemon.c |   2 -
 5 files changed, 89 insertions(+), 20 deletions(-)

diff --git a/qapi/introspect.json b/qapi/introspect.json
index 944bb87a20..39bd303778 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -49,7 +49,7 @@
 ##
 { 'command': 'query-qmp-schema',
   'returns': [ 'SchemaInfo' ],
-  'gen': false }                # just to simplify qmp_query_json()
+  'allow-preconfig': true }
 
 ##
 # @SchemaMetaType:
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index b39e03b744..8833afaa65 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -180,7 +180,4 @@ void help_cmd(Monitor *mon, const char *name);
 void handle_hmp_command(MonitorHMP *mon, const char *cmdline);
 int hmp_compare_cmd(const char *name, const char *list);
 
-void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
-                                 Error **errp);
-
 #endif
diff --git a/monitor/misc.c b/monitor/misc.c
index e847b58a8c..3fd335e737 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -242,8 +242,6 @@ static void monitor_init_qmp_commands(void)
 
     qmp_init_marshal(&qmp_commands);
 
-    qmp_register_command(&qmp_commands, "query-qmp-schema",
-                         qmp_query_qmp_schema, QCO_ALLOW_PRECONFIG);
     qmp_register_command(&qmp_commands, "device_add", qmp_device_add,
                          QCO_NO_OPTIONS);
     qmp_register_command(&qmp_commands, "object-add", qmp_object_add,
diff --git a/monitor/qmp-cmds-control.c b/monitor/qmp-cmds-control.c
index 8f04cfa6e6..5b07c7e765 100644
--- a/monitor/qmp-cmds-control.c
+++ b/monitor/qmp-cmds-control.c
@@ -26,10 +26,14 @@
 
 #include "monitor-internal.h"
 #include "qemu-version.h"
+#include "qapi/compat-policy.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-control.h"
+#include "qapi/qapi-commands-introspect.h"
 #include "qapi/qapi-emit-events.h"
 #include "qapi/qapi-introspect.h"
+#include "qapi/qapi-visit-introspect.h"
+#include "qapi/qobject-input-visitor.h"
 
 /*
  * Accept QMP capabilities in @list for @mon.
@@ -153,17 +157,89 @@ EventInfoList *qmp_query_events(Error **errp)
     return ev_list;
 }
 
-/*
- * Minor hack: generated marshalling suppressed for this command
- * ('gen': false in the schema) so we can parse the JSON string
- * directly into QObject instead of first parsing it with
- * visit_type_SchemaInfoList() into a SchemaInfoList, then marshal it
- * to QObject with generated output marshallers, every time.  Instead,
- * we do it in test-qobject-input-visitor.c, just to make sure
- * qapi-gen.py's output actually conforms to the schema.
- */
-void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
-                                 Error **errp)
+static void *split_off_generic_list(void *list,
+                                    bool (*splitp)(void *elt),
+                                    void **part)
 {
-    *ret_data = qobject_from_qlit(&qmp_schema_qlit);
+    GenericList *keep = NULL, **keep_tailp = &keep;
+    GenericList *split = NULL, **split_tailp = &split;
+    GenericList *tail;
+
+    for (tail = list; tail; tail = tail->next) {
+        if (splitp(tail)) {
+            *split_tailp = tail;
+            split_tailp = &tail->next;
+        } else {
+            *keep_tailp = tail;
+            keep_tailp = &tail->next;
+        }
+    }
+
+    *keep_tailp = *split_tailp = NULL;
+    *part = split;
+    return keep;
+}
+
+static bool is_in(const char *s, strList *list)
+{
+    strList *tail;
+
+    for (tail = list; tail; tail = tail->next) {
+        if (!strcmp(tail->value, s)) {
+            return true;
+        }
+    }
+    return false;
+}
+
+static bool is_entity_deprecated(void *link)
+{
+    return is_in("deprecated", ((SchemaInfoList *)link)->value->features);
+}
+
+static bool is_member_deprecated(void *link)
+{
+    return is_in("deprecated",
+                 ((SchemaInfoObjectMemberList *)link)->value->features);
+}
+
+static SchemaInfoList *zap_deprecated(SchemaInfoList *schema)
+{
+    void *to_zap;
+    SchemaInfoList *tail;
+    SchemaInfo *ent;
+
+    schema = split_off_generic_list(schema, is_entity_deprecated, &to_zap);
+    qapi_free_SchemaInfoList(to_zap);
+
+    for (tail = schema; tail; tail = tail->next) {
+        ent = tail->value;
+        if (ent->meta_type == SCHEMA_META_TYPE_OBJECT) {
+            ent->u.object.members
+                = split_off_generic_list(ent->u.object.members,
+                                         is_member_deprecated, &to_zap);
+            qapi_free_SchemaInfoObjectMemberList(to_zap);
+        }
+    }
+
+    return schema;
+}
+
+SchemaInfoList *qmp_query_qmp_schema(Error **errp)
+{
+    QObject *obj = qobject_from_qlit(&qmp_schema_qlit);
+    Visitor *v = qobject_input_visitor_new(obj);
+    SchemaInfoList *schema = NULL;
+
+    /* test_visitor_in_qmp_introspect() ensures this can't fail */
+    visit_type_SchemaInfoList(v, NULL, &schema, &error_abort);
+    g_assert(schema);
+
+    qobject_unref(obj);
+    visit_free(v);
+
+    if (compat_policy.deprecated_output == COMPAT_POLICY_OUTPUT_HIDE) {
+        return zap_deprecated(schema);
+    }
+    return schema;
 }
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index 7e9b0e0d3f..9490099036 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -142,8 +142,6 @@ static QemuOptsList qemu_object_opts = {
 static void init_qmp_commands(void)
 {
     qmp_init_marshal(&qmp_commands);
-    qmp_register_command(&qmp_commands, "query-qmp-schema",
-                         qmp_query_qmp_schema, QCO_ALLOW_PRECONFIG);
 
     QTAILQ_INIT(&qmp_cap_negotiation_commands);
     qmp_register_command(&qmp_cap_negotiation_commands, "qmp_capabilities",
-- 
2.26.2

[PATCH v5 6/8] qapi: Implement deprecated-input=reject for QMP commands

[Markus Armbruster]

This policy rejects deprecated input, and thus permits "testing the
future".  Implement it for QMP commands: make deprecated ones fail.
Example: when QEMU is run with -compat deprecated-input=reject, then

    {"execute": "query-cpus"}

fails like this

    {"error": {"class": "CommandNotFound", "desc": "Deprecated command query-cpus disabled by policy"}}

When the deprecated command is removed, the error will change to

    {"error": {"class": "CommandNotFound", "desc": "The command query-cpus has not been found"}}

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/dispatch.h |  1 +
 qapi/qmp-dispatch.c         | 13 +++++++++++++
 tests/test-qmp-cmds.c       | 24 ++++++++++++++++++++++++
 scripts/qapi/commands.py    | 10 +++++++---
 4 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 5a9cf82472..3307b57a96 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 f65b8cf000..634e9c0a7d 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -130,6 +130,19 @@ QDict *qmp_dispatch(const QmpCommandList *cmds, QObject *request,
                   "The command %s has not been found", command);
         goto out;
     }
+    if (cmd->options & QCO_DEPRECATED) {
+        switch (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 bca89f8e88..033d5dae9d 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -277,6 +277,28 @@ static void test_dispatch_cmd_io(void)
     qobject_unref(ret3);
 }
 
+static void test_dispatch_cmd_deprecated(void)
+{
+    const char *cmd = "{ 'execute': 'test-command-features1' }";
+    QDict *ret;
+
+    memset(&compat_policy, 0, sizeof(compat_policy));
+
+    /* accept */
+    ret = qobject_to(QDict, do_qmp_dispatch(false, cmd));
+    assert(ret && qdict_size(ret) == 0);
+    qobject_unref(ret);
+
+    compat_policy.has_deprecated_input = true;
+    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);
+
+    compat_policy.deprecated_input = COMPAT_POLICY_INPUT_REJECT;
+    do_qmp_dispatch_error(false, ERROR_CLASS_COMMAND_NOT_FOUND, cmd);
+}
+
 static void test_dispatch_cmd_ret_deprecated(void)
 {
     const char *cmd = "{ 'execute': 'test-features0' }";
@@ -375,6 +397,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/dispatch_cmd_ret_deprecated",
                     test_dispatch_cmd_ret_deprecated);
     g_test_add_func("/qmp/dealloc_types", test_dealloc_types);
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 14662515f3..5f4b16fed0 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -176,9 +176,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:
@@ -284,8 +288,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.26.2

[PATCH v5 7/8] qapi: Implement deprecated-input=reject for QMP command arguments

[Markus Armbruster]

This policy rejects deprecated input, and thus permits "testing the
future".  Implement it for QMP command arguments: reject commands with
deprecated ones.  Example: when QEMU is run with -compat
deprecated-input=reject, then

    {"execute": "eject", "arguments": {"device": "cd"}}

fails like this

    {"error": {"class": "GenericError", "desc": "Deprecated parameter 'device' disabled by policy"}}

When the deprecated parameter is removed, the error will change to

    {"error": {"class": "GenericError", "desc": "Parameter 'device' is unexpected"}}

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qobject-input-visitor.h |  9 +++++++++
 include/qapi/visitor-impl.h          |  3 +++
 include/qapi/visitor.h               |  9 +++++++++
 qapi/qapi-visit-core.c               |  9 +++++++++
 qapi/qobject-input-visitor.c         | 28 ++++++++++++++++++++++++++++
 tests/test-qmp-cmds.c                | 25 +++++++++++++++++++++++++
 qapi/trace-events                    |  1 +
 scripts/qapi/commands.py             |  2 +-
 scripts/qapi/visit.py                |  3 +++
 9 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/include/qapi/qobject-input-visitor.h b/include/qapi/qobject-input-visitor.h
index 95985e25e5..cbc54de4ac 100644
--- a/include/qapi/qobject-input-visitor.h
+++ b/include/qapi/qobject-input-visitor.h
@@ -58,6 +58,15 @@ typedef struct QObjectInputVisitor QObjectInputVisitor;
  */
 Visitor *qobject_input_visitor_new(QObject *obj);
 
+/*
+ * Create a QObject input visitor for @obj for use with QMP
+ *
+ * This is like qobject_input_visitor_new(), except it obeys the
+ * policy for handling deprecated management interfaces set with
+ * -compat.
+ */
+Visitor *qobject_input_visitor_new_qmp(QObject *obj);
+
 /*
  * Create a QObject input visitor for @obj for use with keyval_parse()
  *
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 2d853255a3..3b950f6e3d 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -113,6 +113,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_accept)(Visitor *v, const char *name, Error **errp);
+
     /* Optional */
     bool (*deprecated)(Visitor *v, const char *name);
 
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 4d23b59853..b3c9ef7a81 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -459,6 +459,15 @@ void visit_end_alternate(Visitor *v, void **obj);
  */
 bool visit_optional(Visitor *v, const char *name, bool *present);
 
+/*
+ * Should we reject 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_accept(Visitor *v, const char *name, Error **errp);
+
 /*
  * Should we visit deprecated member @name?
  *
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index d9726ecaa1..a641adec51 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -135,6 +135,15 @@ bool visit_optional(Visitor *v, const char *name, bool *present)
     return *present;
 }
 
+bool visit_deprecated_accept(Visitor *v, const char *name, Error **errp)
+{
+    trace_visit_deprecated_accept(v, name);
+    if (v->deprecated_accept) {
+        return v->deprecated_accept(v, name, errp);
+    }
+    return true;
+}
+
 bool visit_deprecated(Visitor *v, const char *name)
 {
     trace_visit_deprecated(v, name);
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index f918a05e5f..91e2d84ad2 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -14,6 +14,7 @@
 
 #include "qemu/osdep.h"
 #include <math.h>
+#include "qapi/compat-policy.h"
 #include "qapi/error.h"
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/visitor-impl.h"
@@ -43,6 +44,7 @@ typedef struct StackObject {
 
 struct QObjectInputVisitor {
     Visitor visitor;
+    CompatPolicyInput deprecated_policy;
 
     /* Root of visit at visitor creation. */
     QObject *root;
@@ -666,6 +668,23 @@ static void qobject_input_optional(Visitor *v, const char *name, bool *present)
     *present = true;
 }
 
+static bool qobject_input_deprecated_accept(Visitor *v, const char *name,
+                                            Error **errp)
+{
+    QObjectInputVisitor *qiv = to_qiv(v);
+
+    switch (qiv->deprecated_policy) {
+    case COMPAT_POLICY_INPUT_ACCEPT:
+        return true;
+    case COMPAT_POLICY_INPUT_REJECT:
+        error_setg(errp, "Deprecated parameter '%s' disabled by policy",
+                   name);
+        return false;
+    default:
+        abort();
+    }
+}
+
 static void qobject_input_free(Visitor *v)
 {
     QObjectInputVisitor *qiv = to_qiv(v);
@@ -700,6 +719,7 @@ static QObjectInputVisitor *qobject_input_visitor_base_new(QObject *obj)
     v->visitor.end_list = qobject_input_end_list;
     v->visitor.start_alternate = qobject_input_start_alternate;
     v->visitor.optional = qobject_input_optional;
+    v->visitor.deprecated_accept = qobject_input_deprecated_accept;
     v->visitor.free = qobject_input_free;
 
     v->root = qobject_ref(obj);
@@ -722,6 +742,14 @@ Visitor *qobject_input_visitor_new(QObject *obj)
     return &v->visitor;
 }
 
+Visitor *qobject_input_visitor_new_qmp(QObject *obj)
+{
+    QObjectInputVisitor *v = to_qiv(qobject_input_visitor_new(obj));
+
+    v->deprecated_policy = compat_policy.deprecated_input;
+    return &v->visitor;
+}
+
 Visitor *qobject_input_visitor_new_keyval(QObject *obj)
 {
     QObjectInputVisitor *v = qobject_input_visitor_base_new(obj);
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index 033d5dae9d..9aec9da2ce 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -299,6 +299,29 @@ static void test_dispatch_cmd_deprecated(void)
     do_qmp_dispatch_error(false, ERROR_CLASS_COMMAND_NOT_FOUND, cmd);
 }
 
+static void test_dispatch_cmd_arg_deprecated(void)
+{
+    const char *cmd = "{ 'execute': 'test-features0',"
+        " 'arguments': { 'fs1': { 'foo': 42 } } }";
+    QDict *ret;
+
+    memset(&compat_policy, 0, sizeof(compat_policy));
+
+    /* accept */
+    ret = qobject_to(QDict, do_qmp_dispatch(false, cmd));
+    assert(ret && qdict_size(ret) == 1);
+    qobject_unref(ret);
+
+    compat_policy.has_deprecated_input = true;
+    compat_policy.deprecated_input = COMPAT_POLICY_INPUT_ACCEPT;
+    ret = qobject_to(QDict, do_qmp_dispatch(false, cmd));
+    assert(ret && qdict_size(ret) == 1);
+    qobject_unref(ret);
+
+    compat_policy.deprecated_input = COMPAT_POLICY_INPUT_REJECT;
+    do_qmp_dispatch_error(false, ERROR_CLASS_GENERIC_ERROR, cmd);
+}
+
 static void test_dispatch_cmd_ret_deprecated(void)
 {
     const char *cmd = "{ 'execute': 'test-features0' }";
@@ -399,6 +422,8 @@ int main(int argc, char **argv)
                     test_dispatch_cmd_success_response);
     g_test_add_func("/qmp/dispatch_cmd_deprecated",
                     test_dispatch_cmd_deprecated);
+    g_test_add_func("/qmp/dispatch_cmd_arg_deprecated",
+                    test_dispatch_cmd_arg_deprecated);
     g_test_add_func("/qmp/dispatch_cmd_ret_deprecated",
                     test_dispatch_cmd_ret_deprecated);
     g_test_add_func("/qmp/dealloc_types", test_dealloc_types);
diff --git a/qapi/trace-events b/qapi/trace-events
index eff1fbd199..3cabe912ae 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_accept(void *v, const char *name) "v=%p name=%s"
 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"
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 5f4b16fed0..9c9b119f32 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -121,7 +121,7 @@ def gen_marshal(name, arg_type, boxed, ret_type):
 
     ret += mcgen('''
 
-    v = qobject_input_visitor_new(QOBJECT(args));
+    v = qobject_input_visitor_new_qmp(QOBJECT(args));
     if (!visit_start_struct(v, NULL, NULL, 0, errp)) {
         goto out;
     }
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 110563e56f..ad1747a84f 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -63,6 +63,9 @@ bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
             push_indent()
         if deprecated:
             ret += mcgen('''
+    if (!visit_deprecated_accept(v, "%(name)s", errp)) {
+        return false;
+    }
     if (visit_deprecated(v, "%(name)s")) {
 ''',
                          name=memb.name)
-- 
2.26.2

[PATCH v5 8/8] qapi: New -compat deprecated-input=crash

[Markus Armbruster]

Policy "crash" calls abort() when deprecated input is received.

Bugs in integration tests may mask the error from policy "reject".
Provide a larger hammer: crash outright.  Masking that seems unlikely.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/compat.json             | 3 ++-
 qapi/qmp-dispatch.c          | 1 +
 qapi/qobject-input-visitor.c | 1 +
 qemu-options.hx              | 4 +++-
 4 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/qapi/compat.json b/qapi/compat.json
index d2c02a21aa..936602dcc6 100644
--- a/qapi/compat.json
+++ b/qapi/compat.json
@@ -11,11 +11,12 @@
 #
 # @accept: Accept silently
 # @reject: Reject with an error
+# @crash: abort() the process
 #
 # Since: 5.2
 ##
 { 'enum': 'CompatPolicyInput',
-  'data': [ 'accept', 'reject' ] }
+  'data': [ 'accept', 'reject', 'crash' ] }
 
 ##
 # @CompatPolicyOutput:
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 634e9c0a7d..9954894d95 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -139,6 +139,7 @@ QDict *qmp_dispatch(const QmpCommandList *cmds, QObject *request,
                       "Deprecated command %s disabled by policy",
                       command);
             goto out;
+        case COMPAT_POLICY_INPUT_CRASH:
         default:
             abort();
         }
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 91e2d84ad2..d21ba0db8e 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -680,6 +680,7 @@ static bool qobject_input_deprecated_accept(Visitor *v, const char *name,
         error_setg(errp, "Deprecated parameter '%s' disabled by policy",
                    name);
         return false;
+    case COMPAT_POLICY_INPUT_CRASH:
     default:
         abort();
     }
diff --git a/qemu-options.hx b/qemu-options.hx
index bb0c6bb70e..f36d2073e5 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3392,7 +3392,7 @@ DEFHEADING()
 DEFHEADING(Debug/Expert options:)
 
 DEF("compat", HAS_ARG, QEMU_OPTION_compat,
-    "-compat [deprecated-input=accept|reject][,deprecated-output=accept|hide]\n"
+    "-compat [deprecated-input=accept|reject|crash][,deprecated-output=accept|hide]\n"
     "                Policy for handling deprecated management interfaces\n",
     QEMU_ARCH_ALL)
 SRST
@@ -3403,6 +3403,8 @@ SRST
         Accept deprecated commands and arguments
     ``deprecated-input=reject``
         Reject deprecated commands and arguments
+    ``deprecated-input=crash``
+        Crash on deprecated command
     ``deprecated-output=accept`` (default)
         Emit deprecated command results and events
     ``deprecated-output=hide``
-- 
2.26.2

Re: [PATCH v5 1/8] qemu-options: New -compat to set policy for deprecated interfaces

[Peter Krempa]

On Mon, Sep 14, 2020 at 10:47:55 +0200, Markus Armbruster wrote:
> Policy is separate for input and output.
> 
> Input policy can be "accept" (accept silently), or "reject" (reject
> the request with an error).
> 
> Output policy can be "accept" (pass on unchanged), or "hide" (filter
> out the deprecated parts).
> 
> Default is "accept".  Policies other than "accept" are implemented
> later in this series.
> 
> For now, -compat covers only syntactic aspects of QMP, i.e. stuff
> tagged with feature 'deprecated'.  We may want to extend it to cover
> semantic aspects, CLI, and experimental features.
> 
> The option is experimental.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---

I wasn't able to find any good anchor point which would allow me to
detect that this command line option/feature is present.

Is there anything in e.g. in query-qmp-schema or query-command-line-options
I could base this capability on?

Re: [PATCH v5 1/8] qemu-options: New -compat to set policy for deprecated interfaces

[Eric Blake]

On 9/14/20 3:47 AM, Markus Armbruster wrote:
> Policy is separate for input and output.
> 
> Input policy can be "accept" (accept silently), or "reject" (reject
> the request with an error).
> 
> Output policy can be "accept" (pass on unchanged), or "hide" (filter
> out the deprecated parts).
> 
> Default is "accept".  Policies other than "accept" are implemented
> later in this series.
> 
> For now, -compat covers only syntactic aspects of QMP, i.e. stuff
> tagged with feature 'deprecated'.  We may want to extend it to cover
> semantic aspects, CLI, and experimental features.
> 
> The option is experimental.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---

Reviewed-by: Eric Blake <eblake@redhat.com>

But as Peter points out, where is the introspection for whether this 
command line argument exists?

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

Re: [PATCH v5 3/8] qapi: Implement deprecated-output=hide for QMP events

[Eric Blake]

On 9/14/20 3:47 AM, Markus Armbruster wrote:
> This policy suppresses deprecated bits in output, and thus permits
> "testing the future".  Implement it for QMP events: suppress
> deprecated ones.
> 
> No QMP event is deprecated right now.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   tests/test-qmp-event.c | 20 ++++++++++++++++++++
>   scripts/qapi/events.py | 14 ++++++++++++--
>   2 files changed, 32 insertions(+), 2 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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

Re: [PATCH v5 4/8] qapi: Implement deprecated-output=hide for QMP event data

[Eric Blake]

On 9/14/20 3:47 AM, Markus Armbruster wrote:
> This policy suppresses deprecated bits in output, and thus permits
> "testing the future".  Implement it for QMP event data: suppress
> deprecated members.
> 
> No QMP event data is deprecated right now.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   tests/test-qmp-event.c                  | 21 +++++++++++++++++++++
>   scripts/qapi/events.py                  |  8 ++++++--
>   tests/qapi-schema/qapi-schema-test.json |  3 +++
>   tests/qapi-schema/qapi-schema-test.out  |  2 ++
>   4 files changed, 32 insertions(+), 2 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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

Re: [PATCH v5 5/8] qapi: Implement deprecated-output=hide for QMP introspection

[Eric Blake]

On 9/14/20 3:47 AM, Markus Armbruster wrote:
> This policy suppresses deprecated bits in output, and thus permits
> "testing the future".  Implement it for QMP command query-qmp-schema:
> suppress information on deprecated commands, events and object type
> members, i.e. anything that has the special feature flag "deprecated".
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   qapi/introspect.json                 |   2 +-
>   monitor/monitor-internal.h           |   3 -
>   monitor/misc.c                       |   2 -
>   monitor/qmp-cmds-control.c           | 100 +++++++++++++++++++++++----
>   storage-daemon/qemu-storage-daemon.c |   2 -
>   5 files changed, 89 insertions(+), 20 deletions(-)
> 
> diff --git a/qapi/introspect.json b/qapi/introspect.json
> index 944bb87a20..39bd303778 100644
> --- a/qapi/introspect.json
> +++ b/qapi/introspect.json
> @@ -49,7 +49,7 @@
>   ##
>   { 'command': 'query-qmp-schema',
>     'returns': [ 'SchemaInfo' ],
> -  'gen': false }                # just to simplify qmp_query_json()
> +  'allow-preconfig': true }

Interesting change. Dropping 'gen':false is explained below...

> @@ -153,17 +157,89 @@ EventInfoList *qmp_query_events(Error **errp)
>       return ev_list;
>   }
>   
> -/*
> - * Minor hack: generated marshalling suppressed for this command
> - * ('gen': false in the schema) so we can parse the JSON string
> - * directly into QObject instead of first parsing it with
> - * visit_type_SchemaInfoList() into a SchemaInfoList, then marshal it
> - * to QObject with generated output marshallers, every time.  Instead,
> - * we do it in test-qobject-input-visitor.c, just to make sure
> - * qapi-gen.py's output actually conforms to the schema.
> - */
> -void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
> -                                 Error **errp)
> +static void *split_off_generic_list(void *list,
> +                                    bool (*splitp)(void *elt),
> +                                    void **part)

...but adding 'allow-preconfig':true, while it makes sense, seems a bit 
unrelated.  Worth a better commit message?

Reviewed-by: Eric Blake <eblake@redhat.com>

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

Re: [PATCH v5 6/8] qapi: Implement deprecated-input=reject for QMP commands

[Eric Blake]

On 9/14/20 3:48 AM, Markus Armbruster wrote:
> This policy rejects deprecated input, and thus permits "testing the
> future".  Implement it for QMP commands: make deprecated ones fail.
> Example: when QEMU is run with -compat deprecated-input=reject, then
> 
>      {"execute": "query-cpus"}
> 
> fails like this
> 
>      {"error": {"class": "CommandNotFound", "desc": "Deprecated command query-cpus disabled by policy"}}
> 
> When the deprecated command is removed, the error will change to
> 
>      {"error": {"class": "CommandNotFound", "desc": "The command query-cpus has not been found"}}
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---

Reviewed-by: Eric Blake <eblake@redhat.com>

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

Re: [PATCH v5 7/8] qapi: Implement deprecated-input=reject for QMP command arguments

[Eric Blake]

On 9/14/20 3:48 AM, Markus Armbruster wrote:
> This policy rejects deprecated input, and thus permits "testing the
> future".  Implement it for QMP command arguments: reject commands with
> deprecated ones.  Example: when QEMU is run with -compat
> deprecated-input=reject, then
> 
>      {"execute": "eject", "arguments": {"device": "cd"}}
> 
> fails like this
> 
>      {"error": {"class": "GenericError", "desc": "Deprecated parameter 'device' disabled by policy"}}
> 
> When the deprecated parameter is removed, the error will change to
> 
>      {"error": {"class": "GenericError", "desc": "Parameter 'device' is unexpected"}}
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---

Reviewed-by: Eric Blake <eblake@redhat.com>

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

Re: [PATCH v5 8/8] qapi: New -compat deprecated-input=crash

[Eric Blake]

On 9/14/20 3:48 AM, Markus Armbruster wrote:
> Policy "crash" calls abort() when deprecated input is received.
> 
> Bugs in integration tests may mask the error from policy "reject".
> Provide a larger hammer: crash outright.  Masking that seems unlikely.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---

> +++ b/qemu-options.hx
> @@ -3392,7 +3392,7 @@ DEFHEADING()
>   DEFHEADING(Debug/Expert options:)
>   
>   DEF("compat", HAS_ARG, QEMU_OPTION_compat,
> -    "-compat [deprecated-input=accept|reject][,deprecated-output=accept|hide]\n"
> +    "-compat [deprecated-input=accept|reject|crash][,deprecated-output=accept|hide]\n"
>       "                Policy for handling deprecated management interfaces\n",
>       QEMU_ARCH_ALL)
>   SRST
> @@ -3403,6 +3403,8 @@ SRST
>           Accept deprecated commands and arguments
>       ``deprecated-input=reject``
>           Reject deprecated commands and arguments
> +    ``deprecated-input=crash``
> +        Crash on deprecated command

Missing 'and arguments'?

Otherwise,
Reviewed-by: Eric Blake <eblake@redhat.com>

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

Re: [PATCH v5 1/8] qemu-options: New -compat to set policy for deprecated interfaces

[Richard W.M. Jones]

On Mon, Sep 14, 2020 at 10:08:50AM -0500, Eric Blake wrote:
> On 9/14/20 3:47 AM, Markus Armbruster wrote:
> >Policy is separate for input and output.
> >
> >Input policy can be "accept" (accept silently), or "reject" (reject
> >the request with an error).
> >
> >Output policy can be "accept" (pass on unchanged), or "hide" (filter
> >out the deprecated parts).
> >
> >Default is "accept".  Policies other than "accept" are implemented
> >later in this series.
> >
> >For now, -compat covers only syntactic aspects of QMP, i.e. stuff
> >tagged with feature 'deprecated'.  We may want to extend it to cover
> >semantic aspects, CLI, and experimental features.
> >
> >The option is experimental.
> >
> >Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >---
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> But as Peter points out, where is the introspection for whether this
> command line argument exists?

FWIW libguestfs still tests some qemu features by grepping -help
output.  That's actually what I'm intending to do with this one, at
least for now.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

Re: [PATCH v5 0/8] Configurable policy for handling deprecated interfaces

[Richard W.M. Jones]

[-- Attachment #1: Type: text/plain, Size: 1238 bytes --]

Some general comments on using the patch:

* For libguestfs I chose to add

  -compat deprecated-input=reject,deprecated-output=hide

  This is only enabled in developer builds of libguestfs when we
  are running qemu directly (not via libvirt).  The patch for
  this is attached.

* What's the point/difference in having reject vs crash?

* I hope that by hiding deprecated QAPI fields we may detect
  errors in libguestfs, but I suspect what'll happen is it'll
  cause fall-back behaviour which might be harder to detect.

* Be *really* good to have this for command line parameters!

Notes on the attached patch:

* https://libguestfs.org/guestfs-building.1.html

* Simple test:

LIBGUESTFS_BACKEND=direct \
LIBGUESTFS_HV=~/d/qemu/build/x86_64-softmmu/qemu-system-x86_64 \
./run libguestfs-test-tool

* Run the full test suite:

LIBGUESTFS_HV=~/d/qemu/build/x86_64-softmmu/qemu-system-x86_64 \
make -k check-direct

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

[-- Attachment #2: 0001-lib-direct-Pass-qemu-compat-to-detect-deprecated-fea.patch --]
[-- Type: text/plain, Size: 2018 bytes --]

From 90df6dc8a3278800f9f9dc23f626df5fa00b5950 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Mon, 21 Sep 2020 13:18:05 +0100
Subject: [PATCH] lib: direct: Pass qemu -compat to detect deprecated features.

In developer versions of libguestfs only, pass the qemu -compat option
which will reject deprecated qemu features, giving us early warning if
we are using something that may be removed in future.  This does not
affect stable branch builds or old versions of qemu which did not have
this flag.
---
 lib/guestfs-internal.h |  3 +++
 lib/launch-direct.c    | 11 +++++++++++
 2 files changed, 14 insertions(+)

diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h
index d7ec7215d..4ad1cd125 100644
--- a/lib/guestfs-internal.h
+++ b/lib/guestfs-internal.h
@@ -33,6 +33,9 @@
 
 #include <pcre.h>
 
+/* Is this a developer version of libguestfs? */
+#define IS_DEVELOPER_VERSION ((PACKAGE_VERSION_MINOR & 1) == 1)
+
 /* Minimum required version of libvirt for the libvirt backend.
  *
  * This is also checked at runtime because you can dynamically link
diff --git a/lib/launch-direct.c b/lib/launch-direct.c
index b6ed9766f..3e42609ff 100644
--- a/lib/launch-direct.c
+++ b/lib/launch-direct.c
@@ -501,6 +501,17 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
   if (guestfs_int_qemu_supports (g, data->qemu_data, "-enable-fips"))
     flag ("-enable-fips");
 
+  /* In non-stable versions of libguestfs, pass the -compat option to
+   * qemu so we can look for potentially deprecated features.
+   */
+  if (IS_DEVELOPER_VERSION &&
+      guestfs_int_qemu_supports (g, data->qemu_data, "-compat")) {
+    start_list ("-compat") {
+      append_list ("deprecated-input=reject");
+      append_list ("deprecated-output=hide");
+    } end_list ();
+  }
+
   /* Newer versions of qemu (from around 2009/12) changed the
    * behaviour of monitors so that an implicit '-monitor stdio' is
    * assumed if we are in -nographic mode and there is no other
-- 
2.28.0.rc2

Re: [PATCH v5 0/8] Configurable policy for handling deprecated interfaces

[Peter Krempa]

On Mon, Sep 21, 2020 at 13:45:14 +0100, Richard W.M. Jones wrote:
> Some general comments on using the patch:
> 
> * For libguestfs I chose to add
> 
>   -compat deprecated-input=reject,deprecated-output=hide
> 
>   This is only enabled in developer builds of libguestfs when we
>   are running qemu directly (not via libvirt).  The patch for
>   this is attached.
> 
> * What's the point/difference in having reject vs crash?

I'll be adding the following documentation for the qemu.conf entry in
libvirt controling the feature:

+# The "reject" option is less harsh towards the VMs but some code paths ignore
+# errors reported by qemu and thus it may not be obvious that a deprecated
+# command/field was used, thus it's suggested to use the "crash" option instead.

Re: [PATCH v5 0/8] Configurable policy for handling deprecated interfaces

[Peter Maydell]

On Mon, 14 Sep 2020 at 09:55, Markus Armbruster <armbru@redhat.com> wrote:
>
> New option -compat lets you configure what to do when deprecated
> interfaces get used.  This is intended for testing users of the
> management interfaces.  It is experimental.
>
> -compat deprecated-input=<in-policy> configures what to do when
> deprecated input is received.  Available policies:
>
> * accept: Accept deprecated commands and arguments (default)
> * reject: Reject them
> * crash: Crash
>
> -compat deprecated-output=<out-policy> configures what to do when
> deprecated output is sent.  Available output policies:
>
> * accept: Emit deprecated command results and events (default)
> * hide: Suppress them
>
> For now, -compat covers only deprecated syntactic aspects of QMP.  We
> may want to extend it to cover semantic aspects, CLI, and experimental
> features.

Some bikeshedding on option naming...

If this only covers QMP, should we make the argument to -compat
have a name that expresses that? eg deprecated-qmp-input,
deprecated-qmp-output ?

Also, it seems a bit repetitive to say 'deprecated' here all
the time -- do you have a future use of -compat in mind which
would be to adjust something that is *not* deprecated ? If
not, maybe the 'deprecated' part should be in the option name
rather than in every argument, eg

  -deprecation-compat qmp-input=crash,qmp-output=hide,cli-option=reject

?

thanks
-- PMM

Re: [PATCH v5 0/8] Configurable policy for handling deprecated interfaces

[Richard W.M. Jones]

On Mon, Sep 21, 2020 at 02:54:15PM +0200, Peter Krempa wrote:
> On Mon, Sep 21, 2020 at 13:45:14 +0100, Richard W.M. Jones wrote:
> > Some general comments on using the patch:
> > 
> > * For libguestfs I chose to add
> > 
> >   -compat deprecated-input=reject,deprecated-output=hide
> > 
> >   This is only enabled in developer builds of libguestfs when we
> >   are running qemu directly (not via libvirt).  The patch for
> >   this is attached.
> > 
> > * What's the point/difference in having reject vs crash?
> 
> I'll be adding the following documentation for the qemu.conf entry in
> libvirt controling the feature:
> 
> +# The "reject" option is less harsh towards the VMs but some code paths ignore
> +# errors reported by qemu and thus it may not be obvious that a deprecated
> +# command/field was used, thus it's suggested to use the "crash" option instead.

I'm not sure if libguestfs should use reject or crash.  But since most
of the benefit of this is going to be in detecting deprecated CLI
parameters in future, reject should be sufficient for us.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

Re: [PATCH v5 1/8] qemu-options: New -compat to set policy for deprecated interfaces

[Markus Armbruster]

Peter Krempa <pkrempa@redhat.com> writes:

> On Mon, Sep 14, 2020 at 10:47:55 +0200, Markus Armbruster wrote:
>> Policy is separate for input and output.
>> 
>> Input policy can be "accept" (accept silently), or "reject" (reject
>> the request with an error).
>> 
>> Output policy can be "accept" (pass on unchanged), or "hide" (filter
>> out the deprecated parts).
>> 
>> Default is "accept".  Policies other than "accept" are implemented
>> later in this series.
>> 
>> For now, -compat covers only syntactic aspects of QMP, i.e. stuff
>> tagged with feature 'deprecated'.  We may want to extend it to cover
>> semantic aspects, CLI, and experimental features.
>> 
>> The option is experimental.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>
> I wasn't able to find any good anchor point which would allow me to
> detect that this command line option/feature is present.
>
> Is there anything in e.g. in query-qmp-schema or query-command-line-options
> I could base this capability on?

You asked this in review of v4.  I didn't have a ready answer then, and
forgot to figure out a solution before I post v5.  Thanks for the
reminder!

Re: [PATCH v5 5/8] qapi: Implement deprecated-output=hide for QMP introspection

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 9/14/20 3:47 AM, Markus Armbruster wrote:
>> This policy suppresses deprecated bits in output, and thus permits
>> "testing the future".  Implement it for QMP command query-qmp-schema:
>> suppress information on deprecated commands, events and object type
>> members, i.e. anything that has the special feature flag "deprecated".
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   qapi/introspect.json                 |   2 +-
>>   monitor/monitor-internal.h           |   3 -
>>   monitor/misc.c                       |   2 -
>>   monitor/qmp-cmds-control.c           | 100 +++++++++++++++++++++++----
>>   storage-daemon/qemu-storage-daemon.c |   2 -
>>   5 files changed, 89 insertions(+), 20 deletions(-)
>> diff --git a/qapi/introspect.json b/qapi/introspect.json
>> index 944bb87a20..39bd303778 100644
>> --- a/qapi/introspect.json
>> +++ b/qapi/introspect.json
>> @@ -49,7 +49,7 @@
>>   ##
>>   { 'command': 'query-qmp-schema',
>>     'returns': [ 'SchemaInfo' ],
>> -  'gen': false }                # just to simplify qmp_query_json()
>> +  'allow-preconfig': true }
>
> Interesting change. Dropping 'gen':false is explained below...
>
>> @@ -153,17 +157,89 @@ EventInfoList *qmp_query_events(Error **errp)
>>       return ev_list;
>>   }
>>   -/*
>> - * Minor hack: generated marshalling suppressed for this command
>> - * ('gen': false in the schema) so we can parse the JSON string
>> - * directly into QObject instead of first parsing it with
>> - * visit_type_SchemaInfoList() into a SchemaInfoList, then marshal it
>> - * to QObject with generated output marshallers, every time.  Instead,
>> - * we do it in test-qobject-input-visitor.c, just to make sure
>> - * qapi-gen.py's output actually conforms to the schema.
>> - */
>> -void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
>> -                                 Error **errp)
>> +static void *split_off_generic_list(void *list,
>> +                                    bool (*splitp)(void *elt),
>> +                                    void **part)
>
> ...but adding 'allow-preconfig':true, while it makes sense, seems a
> bit unrelated.

It's not, actually: query-qmp-schema has always worked in preconfig
state.  Current master:

    $ upstream-qemu -nodefaults -S -display none -qmp stdio -preconfig
    {"QMP": {"version": {"qemu": {"micro": 50, "minor": 1, "major": 5}, "package": ""}, "capabilities": ["oob"]}}
    {"execute": "qmp_capabilities"}
    {"return": {}}
    {"execute": "query-qmp-schema"}
    {"return": [{"name": "query-status", ...}}
    {"execute": "query-block"}
    {"error": {"class": "GenericError", "desc": "The command 'query-block' isn't permitted in 'preconfig' state"}}

We better keep it working there.

>                 Worth a better commit message?

Yes.

> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!

Re: [PATCH v5 8/8] qapi: New -compat deprecated-input=crash

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 9/14/20 3:48 AM, Markus Armbruster wrote:
>> Policy "crash" calls abort() when deprecated input is received.
>> Bugs in integration tests may mask the error from policy "reject".
>> Provide a larger hammer: crash outright.  Masking that seems unlikely.
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>
>> +++ b/qemu-options.hx
>> @@ -3392,7 +3392,7 @@ DEFHEADING()
>>   DEFHEADING(Debug/Expert options:)
>>     DEF("compat", HAS_ARG, QEMU_OPTION_compat,
>> -    "-compat [deprecated-input=accept|reject][,deprecated-output=accept|hide]\n"
>> +    "-compat [deprecated-input=accept|reject|crash][,deprecated-output=accept|hide]\n"
>>       "                Policy for handling deprecated management interfaces\n",
>>       QEMU_ARCH_ALL)
>>   SRST
>> @@ -3403,6 +3403,8 @@ SRST
>>           Accept deprecated commands and arguments
>>       ``deprecated-input=reject``
>>           Reject deprecated commands and arguments
>> +    ``deprecated-input=crash``
>> +        Crash on deprecated command
>
> Missing 'and arguments'?

Yes.

> Otherwise,
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!

Re: [PATCH v5 0/8] Configurable policy for handling deprecated interfaces

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 14 Sep 2020 at 09:55, Markus Armbruster <armbru@redhat.com> wrote:
>>
>> New option -compat lets you configure what to do when deprecated
>> interfaces get used.  This is intended for testing users of the
>> management interfaces.  It is experimental.
>>
>> -compat deprecated-input=<in-policy> configures what to do when
>> deprecated input is received.  Available policies:
>>
>> * accept: Accept deprecated commands and arguments (default)
>> * reject: Reject them
>> * crash: Crash
>>
>> -compat deprecated-output=<out-policy> configures what to do when
>> deprecated output is sent.  Available output policies:
>>
>> * accept: Emit deprecated command results and events (default)
>> * hide: Suppress them
>>
>> For now, -compat covers only deprecated syntactic aspects of QMP.  We
>> may want to extend it to cover semantic aspects, CLI, and experimental
>> features.
>
> Some bikeshedding on option naming...
>
> If this only covers QMP, should we make the argument to -compat
> have a name that expresses that? eg deprecated-qmp-input,
> deprecated-qmp-output ?

It's only implemented for QMP so far.  But we really want it for all
external interfaces for use by machines.  Today, that's QMP and CLI.

Naming the parameters deprecated-qmp-{input,output} leads to separate
settings for QMP and CLI.

Naming them just deprecated-{input,output} leads to a single set of
settings common to all externeal interfaces, or to sugar for setting all
the deprecated-*-{input,output} we may have.

I don't think getting it wrong now would be a big deal.  No excuse for
getting it wrong unthinkingly :)

> Also, it seems a bit repetitive to say 'deprecated' here all
> the time -- do you have a future use of -compat in mind which
> would be to adjust something that is *not* deprecated ? If
> not, maybe the 'deprecated' part should be in the option name
> rather than in every argument, eg
>
>   -deprecation-compat qmp-input=crash,qmp-output=hide,cli-option=reject
>
> ?

My cover letter hints at such future uses: "We may want to extend it to
cover [...] experimental features."  Something like

    -compat experimental-input=reject,experimental-output=hide

Re: [PATCH v5 0/8] Configurable policy for handling deprecated interfaces

[Peter Maydell]

On Mon, 21 Sep 2020 at 15:58, Markus Armbruster <armbru@redhat.com> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
> > If this only covers QMP, should we make the argument to -compat
> > have a name that expresses that? eg deprecated-qmp-input,
> > deprecated-qmp-output ?
>
> It's only implemented for QMP so far.  But we really want it for all
> external interfaces for use by machines.  Today, that's QMP and CLI.
>
> Naming the parameters deprecated-qmp-{input,output} leads to separate
> settings for QMP and CLI.

I think that's a good thing. I might have fixed up my handling
of QMP to avoid deprecated behaviours but not yet got round to
doing that for my command line option choices (or vice-versa).

> > Also, it seems a bit repetitive to say 'deprecated' here all
> > the time -- do you have a future use of -compat in mind which
> > would be to adjust something that is *not* deprecated ? If
> > not, maybe the 'deprecated' part should be in the option name
> > rather than in every argument, eg
> >
> >   -deprecation-compat qmp-input=crash,qmp-output=hide,cli-option=reject
> >
> > ?
>
> My cover letter hints at such future uses: "We may want to extend it to
> cover [...] experimental features."

Ah, I read "-compat covers only deprecated syntactic aspects of QMP.  We
may want to extend it to cover semantic aspects, CLI, and experimental
features." as implying "deprecated semantic aspects, deprecated CLI,
and deprecated experimental features"...

thanks
-- PMM