[PATCH v5 0/5] qapi: Add detection for the 'savevm' fix for blockdev

[PATCH v5 0/5] qapi: Add detection for the 'savevm' fix for blockdev

[Markus Armbruster]

Add 'features' field in the schema for commands and add a feature flag
to advertise that the fix for savevm [1] is present.

[1] https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg03487.html

Based-on: <20191018074345.24034-1-armbru@redhat.com>

v5:
* PATCH 2:
  - qapi-code-gen.txt grammar updated
  - Doc generation for boxed commands fixed
  - Commit message tweaked
* PATCH 3:
  - Command names in qapi-schema-test.json tweaked
  - Trivial pycodestyle-3 fix
* PATCH 4: New
* PATCH 5: Whitespace tweaked

v4:
* PATCH 1: New
* PATCH 2: Factor out check_features()
* PATCH 3: Factor out _print_features(), drop duplicated test
* PATCH 4
  - Shorten savevm-blockdev-monitor-nodes to just savevm-monitor-nodes
  - Tweak commit message and documentation

Markus Armbruster (2):
  tests/qapi-schema: Tidy up test output indentation
  tests/qapi-schema: Cover feature documentation comments

Peter Krempa (3):
  qapi: Add feature flags to commands
  tests: qapi: Test 'features' of commands
  qapi: Allow introspecting fix for savevm's cooperation with blockdev

 docs/devel/qapi-code-gen.txt            | 10 ++--
 tests/qapi-schema/doc-good.texi         | 22 ++++++++
 qapi/introspect.json                    |  6 +-
 qapi/misc.json                          |  9 ++-
 tests/test-qmp-cmds.c                   | 24 ++++++++
 scripts/qapi/commands.py                |  3 +-
 scripts/qapi/doc.py                     |  4 +-
 scripts/qapi/expr.py                    | 35 +++++++-----
 scripts/qapi/introspect.py              |  7 ++-
 scripts/qapi/schema.py                  | 22 ++++++--
 tests/qapi-schema/doc-good.json         | 17 +++++-
 tests/qapi-schema/doc-good.out          |  9 ++-
 tests/qapi-schema/event-case.out        |  2 +-
 tests/qapi-schema/indented-expr.out     |  4 +-
 tests/qapi-schema/qapi-schema-test.json | 18 ++++++
 tests/qapi-schema/qapi-schema-test.out  | 75 ++++++++++++++++---------
 tests/qapi-schema/test-qapi.py          | 20 ++++---
 17 files changed, 219 insertions(+), 68 deletions(-)

-- 
2.21.0

[PATCH v5 1/5] tests/qapi-schema: Tidy up test output indentation

[Markus Armbruster]

Command and event details are indented three spaces, everything else
four.  Messed up in commit 156402e5042.  Use four spaces consistently.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
---
 tests/qapi-schema/doc-good.out         |  4 +-
 tests/qapi-schema/event-case.out       |  2 +-
 tests/qapi-schema/indented-expr.out    |  4 +-
 tests/qapi-schema/qapi-schema-test.out | 52 +++++++++++++-------------
 tests/qapi-schema/test-qapi.py         |  4 +-
 5 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index d3bca343eb..6562e1f412 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -46,9 +46,9 @@ object q_obj_cmd-arg
     member arg2: str optional=True
     member arg3: bool optional=False
 command cmd q_obj_cmd-arg -> Object
-   gen=True success_response=True boxed=False oob=False preconfig=False
+    gen=True success_response=True boxed=False oob=False preconfig=False
 command cmd-boxed Object -> None
-   gen=True success_response=True boxed=True oob=False preconfig=False
+    gen=True success_response=True boxed=True oob=False preconfig=False
 doc freeform
     body=
 = Section
diff --git a/tests/qapi-schema/event-case.out b/tests/qapi-schema/event-case.out
index ec8a1406e4..42ae519656 100644
--- a/tests/qapi-schema/event-case.out
+++ b/tests/qapi-schema/event-case.out
@@ -11,4 +11,4 @@ enum QType
     member qbool
 module event-case.json
 event oops None
-   boxed=False
+    boxed=False
diff --git a/tests/qapi-schema/indented-expr.out b/tests/qapi-schema/indented-expr.out
index bffdf6756d..04356775cd 100644
--- a/tests/qapi-schema/indented-expr.out
+++ b/tests/qapi-schema/indented-expr.out
@@ -11,6 +11,6 @@ enum QType
     member qbool
 module indented-expr.json
 command eins None -> None
-   gen=True success_response=True boxed=False oob=False preconfig=False
+    gen=True success_response=True boxed=False oob=False preconfig=False
 command zwei None -> None
-   gen=True success_response=True boxed=False oob=False preconfig=False
+    gen=True success_response=True boxed=False oob=False preconfig=False
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 98031da96f..aca43186a9 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -33,7 +33,7 @@ object Union
     case value3: q_empty
     case value4: q_empty
 command user_def_cmd0 Empty2 -> Empty2
-   gen=True success_response=True boxed=False oob=False preconfig=False
+    gen=True success_response=True boxed=False oob=False preconfig=False
 enum QEnumTwo
     prefix QENUM_TWO
     member value1
@@ -205,35 +205,35 @@ object SecondArrayRef
     member s: StatusList optional=False
 module qapi-schema-test.json
 command user_def_cmd None -> None
-   gen=True success_response=True boxed=False oob=False preconfig=False
+    gen=True success_response=True boxed=False oob=False preconfig=False
 object q_obj_user_def_cmd1-arg
     member ud1a: UserDefOne optional=False
 command user_def_cmd1 q_obj_user_def_cmd1-arg -> None
-   gen=True success_response=True boxed=False oob=False preconfig=False
+    gen=True success_response=True boxed=False oob=False preconfig=False
 object q_obj_user_def_cmd2-arg
     member ud1a: UserDefOne optional=False
     member ud1b: UserDefOne optional=True
 command user_def_cmd2 q_obj_user_def_cmd2-arg -> UserDefTwo
-   gen=True success_response=True boxed=False oob=False preconfig=False
+    gen=True success_response=True boxed=False oob=False preconfig=False
 command cmd-success-response None -> None
-   gen=True success_response=False boxed=False oob=False preconfig=False
+    gen=True success_response=False boxed=False oob=False preconfig=False
 object q_obj_guest-get-time-arg
     member a: int optional=False
     member b: int optional=True
 command guest-get-time q_obj_guest-get-time-arg -> int
-   gen=True success_response=True boxed=False oob=False preconfig=False
+    gen=True success_response=True boxed=False oob=False preconfig=False
 object q_obj_guest-sync-arg
     member arg: any optional=False
 command guest-sync q_obj_guest-sync-arg -> any
-   gen=True success_response=True boxed=False oob=False preconfig=False
+    gen=True success_response=True boxed=False oob=False preconfig=False
 command boxed-struct UserDefZero -> None
-   gen=True success_response=True boxed=True oob=False preconfig=False
+    gen=True success_response=True boxed=True oob=False preconfig=False
 command boxed-union UserDefListUnion -> None
-   gen=True success_response=True boxed=True oob=False preconfig=False
+    gen=True success_response=True boxed=True oob=False preconfig=False
 command boxed-empty Empty1 -> None
-   gen=True success_response=True boxed=True oob=False preconfig=False
+    gen=True success_response=True boxed=True oob=False preconfig=False
 command test-flags-command None -> None
-   gen=True success_response=True boxed=False oob=True preconfig=True
+    gen=True success_response=True boxed=False oob=True preconfig=True
 object UserDefOptions
     member i64: intList optional=True
     member u64: uint64List optional=True
@@ -245,28 +245,28 @@ object EventStructOne
     member string: str optional=False
     member enum2: EnumOne optional=True
 event EVENT_A None
-   boxed=False
+    boxed=False
 event EVENT_B None
-   boxed=False
+    boxed=False
 object q_obj_EVENT_C-arg
     member a: int optional=True
     member b: UserDefOne optional=True
     member c: str optional=False
 event EVENT_C q_obj_EVENT_C-arg
-   boxed=False
+    boxed=False
 object q_obj_EVENT_D-arg
     member a: EventStructOne optional=False
     member b: str optional=False
     member c: str optional=True
     member enum3: EnumOne optional=True
 event EVENT_D q_obj_EVENT_D-arg
-   boxed=False
+    boxed=False
 event EVENT_E UserDefZero
-   boxed=True
+    boxed=True
 event EVENT_F UserDefFlatUnion
-   boxed=True
+    boxed=True
 event EVENT_G Empty1
-   boxed=True
+    boxed=True
 enum __org.qemu_x-Enum
     member __org.qemu_x-value
 object __org.qemu_x-Base
@@ -297,7 +297,7 @@ alternate __org.qemu_x-Alt
     tag type
     case __org.qemu_x-branch: __org.qemu_x-Base
 event __ORG.QEMU_X-EVENT __org.qemu_x-Struct
-   boxed=False
+    boxed=False
 array __org.qemu_x-EnumList __org.qemu_x-Enum
 array __org.qemu_x-StructList __org.qemu_x-Struct
 object q_obj___org.qemu_x-command-arg
@@ -306,7 +306,7 @@ object q_obj___org.qemu_x-command-arg
     member c: __org.qemu_x-Union2 optional=False
     member d: __org.qemu_x-Alt optional=False
 command __org.qemu_x-command q_obj___org.qemu_x-command-arg -> __org.qemu_x-Union1
-   gen=True success_response=True boxed=False oob=False preconfig=False
+    gen=True success_response=True boxed=False oob=False preconfig=False
 object TestIfStruct
     member foo: int optional=False
     member bar: int optional=False
@@ -335,7 +335,7 @@ object q_obj_TestIfUnionCmd-arg
     member union_cmd_arg: TestIfUnion optional=False
     if ['defined(TEST_IF_UNION)']
 command TestIfUnionCmd q_obj_TestIfUnionCmd-arg -> None
-   gen=True success_response=True boxed=False oob=False preconfig=False
+    gen=True success_response=True boxed=False oob=False preconfig=False
     if ['defined(TEST_IF_UNION)']
 alternate TestIfAlternate
     tag type
@@ -347,7 +347,7 @@ object q_obj_TestIfAlternateCmd-arg
     member alt_cmd_arg: TestIfAlternate optional=False
     if ['defined(TEST_IF_ALT)']
 command TestIfAlternateCmd q_obj_TestIfAlternateCmd-arg -> None
-   gen=True success_response=True boxed=False oob=False preconfig=False
+    gen=True success_response=True boxed=False oob=False preconfig=False
     if ['defined(TEST_IF_ALT)']
 object q_obj_TestIfCmd-arg
     member foo: TestIfStruct optional=False
@@ -355,10 +355,10 @@ object q_obj_TestIfCmd-arg
         if ['defined(TEST_IF_CMD_BAR)']
     if ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)']
 command TestIfCmd q_obj_TestIfCmd-arg -> UserDefThree
-   gen=True success_response=True boxed=False oob=False preconfig=False
+    gen=True success_response=True boxed=False oob=False preconfig=False
     if ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)']
 command TestCmdReturnDefThree None -> UserDefThree
-   gen=True success_response=True boxed=False oob=False preconfig=False
+    gen=True success_response=True boxed=False oob=False preconfig=False
 array TestIfEnumList TestIfEnum
     if ['defined(TEST_IF_ENUM)']
 object q_obj_TestIfEvent-arg
@@ -367,7 +367,7 @@ object q_obj_TestIfEvent-arg
         if ['defined(TEST_IF_EVT_BAR)']
     if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)']
 event TestIfEvent q_obj_TestIfEvent-arg
-   boxed=False
+    boxed=False
     if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)']
 object FeatureStruct0
     member foo: int optional=False
@@ -411,4 +411,4 @@ object q_obj_test-features-arg
     member cfs2: CondFeatureStruct2 optional=False
     member cfs3: CondFeatureStruct3 optional=False
 command test-features q_obj_test-features-arg -> None
-   gen=True success_response=True boxed=False oob=False preconfig=False
+    gen=True success_response=True boxed=False oob=False preconfig=False
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index 664254618a..29d9435bf7 100755
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -76,13 +76,13 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
         print('command %s %s -> %s'
               % (name, arg_type and arg_type.name,
                  ret_type and ret_type.name))
-        print('   gen=%s success_response=%s boxed=%s oob=%s preconfig=%s'
+        print('    gen=%s success_response=%s boxed=%s oob=%s preconfig=%s'
               % (gen, success_response, boxed, allow_oob, allow_preconfig))
         self._print_if(ifcond)
 
     def visit_event(self, name, info, ifcond, arg_type, boxed):
         print('event %s %s' % (name, arg_type and arg_type.name))
-        print('   boxed=%s' % boxed)
+        print('    boxed=%s' % boxed)
         self._print_if(ifcond)
 
     @staticmethod
-- 
2.21.0

[PATCH v5 2/5] qapi: Add feature flags to commands

[Markus Armbruster]

From: Peter Krempa <pkrempa@redhat.com>

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 <pkrempa@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 docs/devel/qapi-code-gen.txt   | 10 ++++++----
 qapi/introspect.json           |  6 +++++-
 scripts/qapi/commands.py       |  3 ++-
 scripts/qapi/doc.py            |  4 +++-
 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, 62 insertions(+), 28 deletions(-)

diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 64d9e4c6a9..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.
 
@@ -640,9 +641,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..6d5726cf6e 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -249,12 +249,14 @@ 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)
             body += ('\n@b{Arguments:} the members of @code{%s}\n'
                      % arg_type.name)
+            body += texi_features(doc)
             body += texi_sections(doc, ifcond)
         else:
             body = texi_entity(doc, 'Arguments', ifcond)
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'", allow_dict=name)
     check_type(expr.get('base'), info, "'base'")
-
-    if features:
-        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)
+    check_features(expr.get('features'), info)
 
 
 def check_union(expr, info):
@@ -282,6 +285,7 @@ def check_command(expr, info):
         raise QAPISemError(info, "'boxed': true requires 'data'")
     check_type(args, info, "'data'", allow_dict=not boxed)
     check_type(rets, info, "'returns'", allow_array=True)
+    check_features(expr.get('features'), info)
 
 
 def check_event(expr, info):
@@ -357,10 +361,11 @@ def check_exprs(exprs):
         elif meta == 'command':
             check_keys(expr, info, meta,
                        ['command'],
-                       ['data', 'returns', 'boxed', 'if',
+                       ['data', 'returns', 'boxed', 'if', 'features',
                         'gen', 'success-response', 'allow-oob',
                         'allow-preconfig'])
             normalize_members(expr.get('data'))
+            normalize_features(expr.get('features'))
             check_command(expr, info)
         elif meta == 'event':
             check_keys(expr, info, meta,
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 4f257591de..b3a463dd8b 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -211,13 +211,18 @@ const QLitObject %(c_name)s = %(c_string)s;
                            for m in variants.variants]}, 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):
         arg_type = arg_type or self._schema.the_empty_object_type
         ret_type = ret_type or self._schema.the_empty_object_type
         obj = {'arg-type': self._use_type(arg_type),
                'ret-type': self._use_type(ret_type)}
         if allow_oob:
             obj['allow-oob'] = allow_oob
+
+        if features:
+            obj['features'] = [(f.name, {'if': f.ifcond}) for f in features]
+
         self._gen_qlit(name, 'command', obj, ifcond)
 
     def visit_event(self, name, info, ifcond, arg_type, boxed):
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 38041098bd..8a48231766 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -109,7 +109,8 @@ class QAPISchemaVisitor(object):
         pass
 
     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):
         pass
 
     def visit_event(self, name, info, ifcond, arg_type, boxed):
@@ -658,10 +659,14 @@ class QAPISchemaCommand(QAPISchemaEntity):
     meta = 'command'
 
     def __init__(self, name, info, doc, ifcond, arg_type, ret_type,
-                 gen, success_response, boxed, allow_oob, allow_preconfig):
+                 gen, success_response, boxed, allow_oob, allow_preconfig,
+                 features):
         QAPISchemaEntity.__init__(self, name, info, doc, ifcond)
         assert not arg_type or isinstance(arg_type, str)
         assert not ret_type or isinstance(ret_type, str)
+        for f in features:
+            assert isinstance(f, QAPISchemaFeature)
+            f.set_defined_in(name)
         self._arg_type_name = arg_type
         self.arg_type = None
         self._ret_type_name = ret_type
@@ -671,6 +676,7 @@ class QAPISchemaCommand(QAPISchemaEntity):
         self.boxed = boxed
         self.allow_oob = allow_oob
         self.allow_preconfig = allow_preconfig
+        self.features = features
 
     def check(self, schema):
         QAPISchemaEntity.check(self, schema)
@@ -700,13 +706,19 @@ class QAPISchemaCommand(QAPISchemaEntity):
                         "command's 'returns' cannot take %s"
                         % self.ret_type.describe())
 
+        # Features are in a name space separate from members
+        seen = {}
+        for f in self.features:
+            f.check_clash(self.info, seen)
+
     def visit(self, visitor):
         QAPISchemaEntity.visit(self, visitor)
         visitor.visit_command(self.name, self.info, self.ifcond,
                               self.arg_type, self.ret_type,
                               self.gen, self.success_response,
                               self.boxed, self.allow_oob,
-                              self.allow_preconfig)
+                              self.allow_preconfig,
+                              self.features)
 
 
 class QAPISchemaEvent(QAPISchemaEntity):
@@ -983,6 +995,7 @@ class QAPISchema(object):
         allow_oob = expr.get('allow-oob', False)
         allow_preconfig = expr.get('allow-preconfig', False)
         ifcond = expr.get('if')
+        features = expr.get('features', [])
         if isinstance(data, OrderedDict):
             data = self._make_implicit_object_type(
                 name, info, doc, ifcond, 'arg', self._make_members(data, info))
@@ -991,7 +1004,8 @@ class QAPISchema(object):
             rets = self._make_array_type(rets[0], info)
         self._def_entity(QAPISchemaCommand(name, info, doc, ifcond, data, rets,
                                            gen, success_response,
-                                           boxed, allow_oob, allow_preconfig))
+                                           boxed, allow_oob, allow_preconfig,
+                                           self._make_features(features, info)))
 
     def _def_event(self, expr, info, doc):
         name = expr['event']
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index 29d9435bf7..d31ac4bbb7 100755
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -72,7 +72,8 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
         self._print_if(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):
         print('command %s %s -> %s'
               % (name, arg_type and arg_type.name,
                  ret_type and ret_type.name))
-- 
2.21.0

[PATCH v5 3/5] tests: qapi: Test 'features' of commands

[Markus Armbruster]

From: Peter Krempa <pkrempa@redhat.com>

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/test-qmp-cmds.c                   | 24 ++++++++++++++++++++++++
 tests/qapi-schema/qapi-schema-test.json | 18 ++++++++++++++++++
 tests/qapi-schema/qapi-schema-test.out  | 23 +++++++++++++++++++++++
 tests/qapi-schema/test-qapi.py          | 13 +++++++++----
 4 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index 36fdf5b115..27b0afe55a 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -51,6 +51,30 @@ void qmp_test_features(FeatureStruct0 *fs0, FeatureStruct1 *fs1,
 {
 }
 
+void qmp_test_command_features0(Error **errp)
+{
+}
+
+void qmp_test_command_features1(Error **errp)
+{
+}
+
+void qmp_test_command_features3(Error **errp)
+{
+}
+
+void qmp_test_command_cond_features1(Error **errp)
+{
+}
+
+void qmp_test_command_cond_features2(Error **errp)
+{
+}
+
+void qmp_test_command_cond_features3(Error **errp)
+{
+}
+
 UserDefTwo *qmp_user_def_cmd2(UserDefOne *ud1a,
                               bool has_udb1, UserDefOne *ud1b,
                               Error **errp)
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 75c42eb0e3..9abf175fe0 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -290,3 +290,21 @@
             'cfs1': 'CondFeatureStruct1',
             'cfs2': 'CondFeatureStruct2',
             'cfs3': 'CondFeatureStruct3' } }
+
+# test 'features' for command
+
+{ 'command': 'test-command-features0',
+  'features': [] }
+{ 'command': 'test-command-features1',
+  'features': [ 'feature1' ] }
+{ 'command': 'test-command-features3',
+  'features': [ 'feature1', 'feature2' ] }
+
+{ 'command': 'test-command-cond-features1',
+  'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'} ] }
+{ 'command': 'test-command-cond-features2',
+  'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'},
+                { 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }
+{ 'command': 'test-command-cond-features3',
+  'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)',
+                                              'defined(TEST_IF_COND_2)'] } ] }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index aca43186a9..3660e75a48 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -412,3 +412,26 @@ object q_obj_test-features-arg
     member cfs3: CondFeatureStruct3 optional=False
 command test-features q_obj_test-features-arg -> None
     gen=True success_response=True boxed=False oob=False preconfig=False
+command test-command-features0 None -> None
+    gen=True success_response=True boxed=False oob=False preconfig=False
+command test-command-features1 None -> None
+    gen=True success_response=True boxed=False oob=False preconfig=False
+    feature feature1
+command test-command-features3 None -> None
+    gen=True success_response=True boxed=False oob=False preconfig=False
+    feature feature1
+    feature feature2
+command test-command-cond-features1 None -> None
+    gen=True success_response=True boxed=False oob=False preconfig=False
+    feature feature1
+        if ['defined(TEST_IF_FEATURE_1)']
+command test-command-cond-features2 None -> None
+    gen=True success_response=True boxed=False oob=False preconfig=False
+    feature feature1
+        if ['defined(TEST_IF_FEATURE_1)']
+    feature feature2
+        if ['defined(TEST_IF_FEATURE_2)']
+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)']
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index d31ac4bbb7..2bd9fd8742 100755
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -61,10 +61,7 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
             self._print_if(m.ifcond, 8)
         self._print_variants(variants)
         self._print_if(ifcond)
-        if features:
-            for f in features:
-                print('    feature %s' % f.name)
-                self._print_if(f.ifcond, 8)
+        self._print_features(features)
 
     def visit_alternate_type(self, name, info, ifcond, variants):
         print('alternate %s' % name)
@@ -80,6 +77,7 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
         print('    gen=%s success_response=%s boxed=%s oob=%s preconfig=%s'
               % (gen, success_response, boxed, allow_oob, allow_preconfig))
         self._print_if(ifcond)
+        self._print_features(features)
 
     def visit_event(self, name, info, ifcond, arg_type, boxed):
         print('event %s %s' % (name, arg_type and arg_type.name))
@@ -99,6 +97,13 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
         if ifcond:
             print('%sif %s' % (' ' * indent, ifcond))
 
+    @classmethod
+    def _print_features(cls, features):
+        if features:
+            for f in features:
+                print('    feature %s' % f.name)
+                cls._print_if(f.ifcond, 8)
+
 
 def test_frontend(fname):
     schema = QAPISchema(fname)
-- 
2.21.0

[PATCH v5 4/5] tests/qapi-schema: Cover feature documentation comments

[Markus Armbruster]

Commit 8aa3a33e44 "tests/qapi-schema: Test for good feature lists in
structs" neglected to cover documentation comments, and the previous
commit followed its example.  Make up for them.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/qapi-schema/doc-good.texi | 22 ++++++++++++++++++++++
 tests/qapi-schema/doc-good.json | 17 +++++++++++++++--
 tests/qapi-schema/doc-good.out  |  5 +++++
 3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/tests/qapi-schema/doc-good.texi b/tests/qapi-schema/doc-good.texi
index 2526abc6d9..2ce8b883c9 100644
--- a/tests/qapi-schema/doc-good.texi
+++ b/tests/qapi-schema/doc-good.texi
@@ -122,6 +122,12 @@ Not documented
 @*@b{If:} @code{defined(IFSTR)}
 @end table
 
+@b{Features:}
+@table @asis
+@item @code{variant1-feat}
+a feature
+@end table
+
 @end deftp
 
 
@@ -182,6 +188,14 @@ argument
 Not documented
 @end table
 
+@b{Features:}
+@table @asis
+@item @code{cmd-feat1}
+a feature
+@item @code{cmd-feat2}
+another feature
+@end table
+
 @b{Note:}
 @code{arg3} is undocumented
 
@@ -227,6 +241,14 @@ If you're bored enough to read this, go see a video of boxed cats
 
 @b{Arguments:} the members of @code{Object}
 
+@b{Features:}
+@table @asis
+@item @code{cmd-feat1}
+a feature
+@item @code{cmd-feat2}
+another feature
+@end table
+
 @b{Example:}
 @example
 -> in
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index f7fb48af38..7dc21e58a3 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -71,8 +71,12 @@
 # A paragraph
 #
 # Another paragraph (but no @var: line)
+#
+# Features:
+# @variant1-feat: a feature
 ##
 { 'struct': 'Variant1',
+  'features': [ 'variant1-feat' ],
   'data': { 'var1': { 'type': 'str', 'if': 'defined(IFSTR)' } } }
 
 ##
@@ -104,6 +108,10 @@
 #
 # @arg2: the second
 # argument
+#
+# Features:
+# @cmd-feat1: a feature
+# @cmd-feat2: another feature
 # Note: @arg3 is undocumented
 # Returns: @Object
 # TODO: frobnicate
@@ -123,11 +131,15 @@
 ##
 { 'command': 'cmd',
   'data': { 'arg1': 'int', '*arg2': 'str', 'arg3': 'bool' },
-  'returns': 'Object' }
+  'returns': 'Object',
+  'features': [ 'cmd-feat1', 'cmd-feat2' ] }
 
 ##
 # @cmd-boxed:
 # If you're bored enough to read this, go see a video of boxed cats
+# Features:
+# @cmd-feat1: a feature
+# @cmd-feat2: another feature
 # Example:
 #
 # -> in
@@ -135,4 +147,5 @@
 # <- out
 ##
 { 'command': 'cmd-boxed', 'boxed': true,
-  'data': 'Object' }
+  'data': 'Object',
+  'features': [ 'cmd-feat1', 'cmd-feat2' ] }
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 6562e1f412..f78fdef6a9 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -20,6 +20,7 @@ object Base
 object Variant1
     member var1: str optional=False
         if ['defined(IFSTR)']
+    feature variant1-feat
 object Variant2
 object Object
     base Base
@@ -47,8 +48,12 @@ object q_obj_cmd-arg
     member arg3: bool optional=False
 command cmd q_obj_cmd-arg -> Object
     gen=True success_response=True boxed=False oob=False preconfig=False
+    feature cmd-feat1
+    feature cmd-feat2
 command cmd-boxed Object -> None
     gen=True success_response=True boxed=True oob=False preconfig=False
+    feature cmd-feat1
+    feature cmd-feat2
 doc freeform
     body=
 = Section
-- 
2.21.0

[PATCH v5 5/5] qapi: Allow introspecting fix for savevm's cooperation with blockdev

[Markus Armbruster]

From: Peter Krempa <pkrempa@redhat.com>

'savevm' was buggy as it considered all monitor-owned block device
nodes for snapshot. With the introduction of -blockdev, the common
usage made all nodes including protocol and backing file nodes be
monitor-owned and thus considered for snapshot.

This is a problem since the 'file' protocol nodes can't have internal
snapshots and it does not make sense to take snapshot of nodes
representing backing files.

This was fixed by commit 05f4aced658a02b02. Clients need to be able to
detect whether this fix is present.

Since savevm does not have an QMP alternative, add the feature for the
'human-monitor-command' backdoor which is used to call this command in
modern use.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/misc.json | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/qapi/misc.json b/qapi/misc.json
index 6bd11f50e6..33b94e3589 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -1020,6 +1020,12 @@
 #
 # @cpu-index: The CPU to use for commands that require an implicit CPU
 #
+# Features:
+# @savevm-monitor-nodes: If present, HMP command savevm only snapshots
+#                        monitor-owned nodes if they have no parents.
+#                        This allows the use of 'savevm' with
+#                        -blockdev. (since 4.2)
+#
 # Returns: the output of the command as a string
 #
 # Since: 0.14.0
@@ -1047,7 +1053,8 @@
 ##
 { 'command': 'human-monitor-command',
   'data': {'command-line': 'str', '*cpu-index': 'int'},
-  'returns': 'str' }
+  'returns': 'str',
+  'features': [ 'savevm-monitor-nodes' ] }
 
 ##
 # @change:
-- 
2.21.0

Re: [PATCH v5 4/5] tests/qapi-schema: Cover feature documentation comments

[Eric Blake]

On 10/18/19 3:14 AM, Markus Armbruster wrote:
> Commit 8aa3a33e44 "tests/qapi-schema: Test for good feature lists in
> structs" neglected to cover documentation comments, and the previous
> commit followed its example.  Make up for them.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---

> @@ -227,6 +241,14 @@ If you're bored enough to read this, go see a video of boxed cats

Meow.

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

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