diff mbox series

[v4,27/34] qemu-options: New -compat to set policy for deprecated interfaces

Message ID 20200317115459.31821-28-armbru@redhat.com (mailing list archive)
State New, archived
Headers show
Series Configurable policy for handling deprecated interfaces | expand

Commit Message

Markus Armbruster March 17, 2020, 11:54 a.m. UTC
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/Makefile.objs           |  8 +++---
 qemu-options.hx              | 20 ++++++++++++++
 7 files changed, 116 insertions(+), 4 deletions(-)
 create mode 100644 qapi/compat.json
 create mode 100644 include/qapi/compat-policy.h

Comments

Eric Blake March 17, 2020, 9:05 p.m. UTC | #1
On 3/17/20 6:54 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.

On IRC, we decided that it's probably not worth shoe-horning this (and 
the rest of the series) into 5.0, given the experimental nature.  Still, 
I'll go ahead and review, so we can settle on things early in 5.1.

> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---

> +++ 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.0

Of course, now that we're slipping this, you'll have to s/5.0/5.1/g over 
the remaining patches.  I won't point it out further.

> +##
> +# @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.

Hiding/rejecting x- interfaces is probably the easiest of these, but I 
agree that leaving this open-ended to add further coverage (or even 
additional modes) is still reasonable.

> +#
> +# @deprecated-input: how to handle deprecated input (default 'accept')
> +# @deprecated-output: how to handle deprecated output (default 'accept')
> +#
> +# Since: 5.0
> +##
> +{ 'struct': 'CompatPolicy',
> +  'data': { '*deprecated-input': 'CompatPolicyInput',
> +            '*deprecated-output': 'CompatPolicyOutput' } }

For example, adding
'*experimental-input': 'CompatPolicyInput'
would make it easy to hard-code failure on attempt to use x-* commands.


> +++ b/include/qapi/compat-policy.h
> @@ -0,0 +1,20 @@
> +/*
> + * Policy for handling "funny" management interfaces
> + *
> + * Copyright (C) 2019 Red Hat, Inc.

You've had this in-tree for a while. I'll leave it up to you if you want 
to add 2020.

> + *
> + * Authors:
> + *  Markus Armbruster <armbru@redhat.com>,
> + *

Ending with a comma is odd.  Is the Authors: snippet even necessary, or 
are we better off relying on git history (which tends to be more 
accurate anyway)?

> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later.  See the COPYING file in the top-level directory.
> + */

Are we trying to use SPDX tags in more files?

> +++ b/qemu-options.hx
> @@ -3357,6 +3357,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):

We'll eventually want to drop (experimental), especially if we get all 
the rest of this into 5.1.

But for now this looks like a good start.
Markus Armbruster March 18, 2020, 6:59 a.m. UTC | #2
Eric Blake <eblake@redhat.com> writes:

> On 3/17/20 6:54 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.
>
> On IRC, we decided that it's probably not worth shoe-horning this (and
> the rest of the series) into 5.0, given the experimental nature.

Yes.  I can't wait for having test suites try this, but releases and
impatience are not a good mix.

> Still, I'll go ahead and review, so we can settle on things early in
> 5.1.

Appreciated!

>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>
>> +++ 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.0
>
> Of course, now that we're slipping this, you'll have to s/5.0/5.1/g
> over the remaining patches.  I won't point it out further.

Acknowledged.

>> +##
>> +# @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.
>
> Hiding/rejecting x- interfaces is probably the easiest of these, but I
> agree that leaving this open-ended to add further coverage (or even
> additional modes) is still reasonable.

Apropos experimental: we may choose to move from the x- naming
convention to a special feature flag "experimental".  That way, we don't
have to rename when we declare the thing stable.

>> +#
>> +# @deprecated-input: how to handle deprecated input (default 'accept')
>> +# @deprecated-output: how to handle deprecated output (default 'accept')
>> +#
>> +# Since: 5.0
>> +##
>> +{ 'struct': 'CompatPolicy',
>> +  'data': { '*deprecated-input': 'CompatPolicyInput',
>> +            '*deprecated-output': 'CompatPolicyOutput' } }
>
> For example, adding
> '*experimental-input': 'CompatPolicyInput'
> would make it easy to hard-code failure on attempt to use x-* commands.

Exactly.

>> +++ b/include/qapi/compat-policy.h
>> @@ -0,0 +1,20 @@
>> +/*
>> + * Policy for handling "funny" management interfaces
>> + *
>> + * Copyright (C) 2019 Red Hat, Inc.
>
> You've had this in-tree for a while. I'll leave it up to you if you
> want to add 2020.

I want to.  Hope I remember when I respin.

>> + *
>> + * Authors:
>> + *  Markus Armbruster <armbru@redhat.com>,
>> + *
>
> Ending with a comma is odd.  Is the Authors: snippet even necessary,
> or are we better off relying on git history (which tends to be more
> accurate anyway)?

The comma is an accident.

The Authors section is custom, not necessity.

>> + * This work is licensed under the terms of the GNU GPL, version 2 or
>> + * later.  See the COPYING file in the top-level directory.
>> + */
>
> Are we trying to use SPDX tags in more files?

No idea :)

>> +++ b/qemu-options.hx
>> @@ -3357,6 +3357,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):
>
> We'll eventually want to drop (experimental), especially if we get all
> the rest of this into 5.1.
>
> But for now this looks like a good start.

Thanks!
diff mbox series

Patch

diff --git a/qapi/compat.json b/qapi/compat.json
new file mode 100644
index 0000000000..fd6f8e932c
--- /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.0
+##
+{ 'enum': 'CompatPolicyInput',
+  'data': [ 'accept', 'reject' ] }
+
+##
+# @CompatPolicyOutput:
+#
+# Policy for handling "funny" output.
+#
+# @accept: Pass on unchanged
+# @hide: Filter out
+#
+# Since: 5.0
+##
+{ '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.0
+##
+{ 'struct': 'CompatPolicy',
+  'data': { '*deprecated-input': 'CompatPolicyInput',
+            '*deprecated-output': 'CompatPolicyOutput' } }
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index 43b0ba0dea..f575b76d81 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -75,6 +75,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..8efb2c58aa
--- /dev/null
+++ b/include/qapi/compat-policy.h
@@ -0,0 +1,20 @@ 
+/*
+ * Policy for handling "funny" management interfaces
+ *
+ * Copyright (C) 2019 Red Hat, Inc.
+ *
+ * Authors:
+ *  Markus Armbruster <armbru@redhat.com>,
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#ifndef QAPI_COMPAT_POLICY_H
+#define QAPI_COMPAT_POLICY_H
+
+#include "qapi/qapi-types-compat.h"
+
+extern CompatPolicy compat_policy;
+
+#endif
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 44fc368d61..57d823c8e1 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 ff2685dff8..74eb43d114 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"
@@ -105,6 +106,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"
@@ -3749,6 +3751,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/Makefile.objs b/qapi/Makefile.objs
index 4673ab7490..a3de2e2756 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -5,10 +5,10 @@  util-obj-y += opts-visitor.o qapi-clone-visitor.o
 util-obj-y += qmp-event.o
 util-obj-y += qapi-util.o
 
-QAPI_COMMON_MODULES = audio authz block-core block char common control crypto
-QAPI_COMMON_MODULES += dump error introspect job machine migration misc
-QAPI_COMMON_MODULES += net pragma qdev qom rdma rocker run-state sockets tpm
-QAPI_COMMON_MODULES += trace transaction ui
+QAPI_COMMON_MODULES = audio authz block-core block char common compat
+QAPI_COMMON_MODULES += control crypto dump error introspect job
+QAPI_COMMON_MODULES += machine migration misc net pragma qdev qom rdma
+QAPI_COMMON_MODULES += rocker run-state sockets tpm trace transaction ui
 QAPI_TARGET_MODULES = machine-target misc-target
 QAPI_MODULES = $(QAPI_COMMON_MODULES) $(QAPI_TARGET_MODULES)
 
diff --git a/qemu-options.hx b/qemu-options.hx
index 962a5ebaa6..263d18d63a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3357,6 +3357,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"