Re: [PATCH v4 2/4] qapi: Add feature flags to commands in qapi

2019-10-14 Thread Markus Armbruster
Markus Armbruster  writes:

> From: Peter Krempa 
>
> Similarly to features for struct types introduce the feature flags also
> for commands. This will allow notifying management layers of fixes and
> compatible changes in the behaviour of a command which may not be
> detectable any other way.
>
> The changes were heavily inspired by commit 6a8c0b51025.
>
> Signed-off-by: Peter Krempa 
> Reviewed-by: Markus Armbruster 
> Signed-off-by: Markus Armbruster 
> ---
>  docs/devel/qapi-code-gen.txt   |  7 ---
>  qapi/introspect.json   |  6 +-
>  scripts/qapi/commands.py   |  3 ++-
>  scripts/qapi/doc.py|  3 ++-
>  scripts/qapi/expr.py   | 35 +++---
>  scripts/qapi/introspect.py |  7 ++-
>  scripts/qapi/schema.py | 22 +
>  tests/qapi-schema/test-qapi.py |  3 ++-
>  8 files changed, 59 insertions(+), 27 deletions(-)
>
> diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
> index 64d9e4c6a9..637fa49977 100644
> --- a/docs/devel/qapi-code-gen.txt
> +++ b/docs/devel/qapi-code-gen.txt
> @@ -640,9 +640,10 @@ change in the QMP syntax (usually by allowing values or 
> operations
>  that previously resulted in an error).  QMP clients may still need to
>  know whether the extension is available.
>  
> -For this purpose, a list of features can be specified for a struct type.
> -This is exposed to the client as a list of string, where each string
> -signals that this build of QEMU shows a certain behaviour.
> +For this purpose, a list of features can be specified for a command or
> +struct type.  This is exposed to the client as a list of strings,
> +where each string signals that this build of QEMU shows a certain
> +behaviour.
>  
>  Each member of the 'features' array defines a feature.  It can either
>  be { 'name': STRING, '*if': COND }, or STRING, which is shorthand for

Squashing in missing syntax update:

diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 637fa49977..45c93a43cc 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -457,7 +457,8 @@ Syntax:
 '*gen': false,
 '*allow-oob': true,
 '*allow-preconfig': true,
-'*if': COND }
+'*if': COND,
+'*features': FEATURES }
 
 Member 'command' names the command.

[...]



[PATCH v4 2/4] qapi: Add feature flags to commands in qapi

2019-10-11 Thread Markus Armbruster
From: Peter Krempa 

Similarly to features for struct types introduce the feature flags also
for commands. This will allow notifying management layers of fixes and
compatible changes in the behaviour of a command which may not be
detectable any other way.

The changes were heavily inspired by commit 6a8c0b51025.

Signed-off-by: Peter Krempa 
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 docs/devel/qapi-code-gen.txt   |  7 ---
 qapi/introspect.json   |  6 +-
 scripts/qapi/commands.py   |  3 ++-
 scripts/qapi/doc.py|  3 ++-
 scripts/qapi/expr.py   | 35 +++---
 scripts/qapi/introspect.py |  7 ++-
 scripts/qapi/schema.py | 22 +
 tests/qapi-schema/test-qapi.py |  3 ++-
 8 files changed, 59 insertions(+), 27 deletions(-)

diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 64d9e4c6a9..637fa49977 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -640,9 +640,10 @@ change in the QMP syntax (usually by allowing values or 
operations
 that previously resulted in an error).  QMP clients may still need to
 know whether the extension is available.
 
-For this purpose, a list of features can be specified for a struct type.
-This is exposed to the client as a list of string, where each string
-signals that this build of QEMU shows a certain behaviour.
+For this purpose, a list of features can be specified for a command or
+struct type.  This is exposed to the client as a list of strings,
+where each string signals that this build of QEMU shows a certain
+behaviour.
 
 Each member of the 'features' array defines a feature.  It can either
 be { 'name': STRING, '*if': COND }, or STRING, which is shorthand for
diff --git a/qapi/introspect.json b/qapi/introspect.json
index 1843c1cb17..031a954fa9 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -266,13 +266,17 @@
 # @allow-oob: whether the command allows out-of-band execution,
 # defaults to false (Since: 2.12)
 #
+# @features: names of features associated with the command, in no particular
+#order. (since 4.2)
+#
 # TODO: @success-response (currently irrelevant, because it's QGA, not QMP)
 #
 # Since: 2.5
 ##
 { 'struct': 'SchemaInfoCommand',
   'data': { 'arg-type': 'str', 'ret-type': 'str',
-'*allow-oob': 'bool' } }
+'*allow-oob': 'bool',
+'*features': [ 'str' ] } }
 
 ##
 # @SchemaInfoEvent:
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 898516b086..ab98e504f3 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -277,7 +277,8 @@ void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
 genc.add(gen_registry(self._regy.get_content(), self._prefix))
 
 def visit_command(self, name, info, ifcond, arg_type, ret_type, gen,
-  success_response, boxed, allow_oob, allow_preconfig):
+  success_response, boxed, allow_oob, allow_preconfig,
+  features):
 if not gen:
 return
 # FIXME: If T is a user-defined type, the user is responsible
diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index dc8919bab7..8278ccbc43 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -249,7 +249,8 @@ class QAPISchemaGenDocVisitor(QAPISchemaVisitor):
body=texi_entity(doc, 'Members', ifcond)))
 
 def visit_command(self, name, info, ifcond, arg_type, ret_type, gen,
-  success_response, boxed, allow_oob, allow_preconfig):
+  success_response, boxed, allow_oob, allow_preconfig,
+  features):
 doc = self.cur_doc
 if boxed:
 body = texi_body(doc)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index da23063f57..5a7e548899 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -184,6 +184,22 @@ def normalize_features(features):
for f in features]
 
 
+def check_features(features, info):
+if features is None:
+return
+if not isinstance(features, list):
+raise QAPISemError(info, "'features' must be an array")
+for f in features:
+source = "'features' member"
+assert isinstance(f, dict)
+check_keys(f, info, source, ['name'], ['if'])
+check_name_is_str(f['name'], info, source)
+source = "%s '%s'" % (source, f['name'])
+check_name_str(f['name'], info, source)
+check_if(f, info, source)
+normalize_if(f)
+
+
 def normalize_enum(expr):
 if isinstance(expr['data'], list):
 expr['data'] = [m if isinstance(m, dict) else {'name': m}
@@ -216,23 +232,10 @@ def check_enum(expr, info):
 def check_struct(expr, info):
 name = expr['struct']
 members = expr['data']
-features = expr.get('features')
 
 check_type(members, info, "'data'",