* [Qemu-devel] [PATCH v4 0/6] file-posix: Add dynamic-auto-read-only QAPI feature
@ 2019-05-30 11:02 Kevin Wolf
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 1/6] qapi: Add feature flags to struct types Kevin Wolf
` (6 more replies)
0 siblings, 7 replies; 11+ messages in thread
From: Kevin Wolf @ 2019-05-30 11:02 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, pkrempa, armbru, qemu-devel
This series adds optional feature lists to struct definitions in the
QAPI schema and makes use of them to advertise the new behaviour of
auto-read-only=on in file-posix.
v4:
- Build fix for tests/test-qmp-cmds
v3:
- Added a command using the structs types to the positive test case
- Renamed QAPIDoc.SymbolPart to QAPIDoc.DocPart
- Applied requested comment changes
- Style fixes to avoid adding pycodestyle-3 complaints
- More stylistic changes to match Markus' taste better
v2:
- Check that features have well-formed names instead of just checking
that they are strings
- Use QAPISchemaFeature objects instead of dicts
- Catch duplicate feature names (and test that a feature name doesn't
conflict with an object member)
- Added patch 4 and 5 to add rudimentary support for features to
QAPIDoc so it doesn't complain about the documentation format
suggested by Markus. Used that format in the documentation of
dynamic-auto-read-only.
Kevin Wolf (6):
qapi: Add feature flags to struct types
tests/qapi-schema: Test for good feature lists in structs
tests/qapi-schema: Error case tests for features in structs
qapi: Disentangle QAPIDoc code
qapi: Allow documentation for features
file-posix: Add dynamic-auto-read-only QAPI feature
qapi/block-core.json | 13 +-
qapi/introspect.json | 6 +-
tests/qapi-schema/features-bad-type.json | 3 +
.../qapi-schema/features-duplicate-name.json | 3 +
tests/qapi-schema/features-missing-name.json | 3 +
tests/qapi-schema/features-name-bad-type.json | 3 +
tests/qapi-schema/features-no-list.json | 3 +
tests/qapi-schema/features-unknown-key.json | 3 +
tests/qapi-schema/qapi-schema-test.json | 39 +++
docs/devel/qapi-code-gen.txt | 38 +++
tests/test-qmp-cmds.c | 8 +
scripts/qapi/common.py | 228 +++++++++++++++---
scripts/qapi/doc.py | 15 +-
scripts/qapi/introspect.py | 6 +-
scripts/qapi/types.py | 3 +-
scripts/qapi/visit.py | 3 +-
tests/Makefile.include | 6 +
tests/qapi-schema/double-type.err | 2 +-
tests/qapi-schema/features-bad-type.err | 1 +
tests/qapi-schema/features-bad-type.exit | 1 +
tests/qapi-schema/features-bad-type.out | 0
tests/qapi-schema/features-duplicate-name.err | 1 +
.../qapi-schema/features-duplicate-name.exit | 1 +
tests/qapi-schema/features-duplicate-name.out | 0
tests/qapi-schema/features-missing-name.err | 1 +
tests/qapi-schema/features-missing-name.exit | 1 +
tests/qapi-schema/features-missing-name.out | 0
tests/qapi-schema/features-name-bad-type.err | 1 +
tests/qapi-schema/features-name-bad-type.exit | 1 +
tests/qapi-schema/features-name-bad-type.out | 0
tests/qapi-schema/features-no-list.err | 1 +
tests/qapi-schema/features-no-list.exit | 1 +
tests/qapi-schema/features-no-list.out | 0
tests/qapi-schema/features-unknown-key.err | 2 +
tests/qapi-schema/features-unknown-key.exit | 1 +
tests/qapi-schema/features-unknown-key.out | 0
tests/qapi-schema/qapi-schema-test.out | 43 ++++
tests/qapi-schema/test-qapi.py | 7 +-
tests/qapi-schema/unknown-expr-key.err | 2 +-
39 files changed, 403 insertions(+), 47 deletions(-)
create mode 100644 tests/qapi-schema/features-bad-type.json
create mode 100644 tests/qapi-schema/features-duplicate-name.json
create mode 100644 tests/qapi-schema/features-missing-name.json
create mode 100644 tests/qapi-schema/features-name-bad-type.json
create mode 100644 tests/qapi-schema/features-no-list.json
create mode 100644 tests/qapi-schema/features-unknown-key.json
create mode 100644 tests/qapi-schema/features-bad-type.err
create mode 100644 tests/qapi-schema/features-bad-type.exit
create mode 100644 tests/qapi-schema/features-bad-type.out
create mode 100644 tests/qapi-schema/features-duplicate-name.err
create mode 100644 tests/qapi-schema/features-duplicate-name.exit
create mode 100644 tests/qapi-schema/features-duplicate-name.out
create mode 100644 tests/qapi-schema/features-missing-name.err
create mode 100644 tests/qapi-schema/features-missing-name.exit
create mode 100644 tests/qapi-schema/features-missing-name.out
create mode 100644 tests/qapi-schema/features-name-bad-type.err
create mode 100644 tests/qapi-schema/features-name-bad-type.exit
create mode 100644 tests/qapi-schema/features-name-bad-type.out
create mode 100644 tests/qapi-schema/features-no-list.err
create mode 100644 tests/qapi-schema/features-no-list.exit
create mode 100644 tests/qapi-schema/features-no-list.out
create mode 100644 tests/qapi-schema/features-unknown-key.err
create mode 100644 tests/qapi-schema/features-unknown-key.exit
create mode 100644 tests/qapi-schema/features-unknown-key.out
--
2.20.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v4 1/6] qapi: Add feature flags to struct types
2019-05-30 11:02 [Qemu-devel] [PATCH v4 0/6] file-posix: Add dynamic-auto-read-only QAPI feature Kevin Wolf
@ 2019-05-30 11:02 ` Kevin Wolf
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 2/6] tests/qapi-schema: Test for good feature lists in structs Kevin Wolf
` (5 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2019-05-30 11:02 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, pkrempa, armbru, qemu-devel
Sometimes, the behaviour of QEMU changes without a 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
they can rely on the changed behavior.
Let's add feature flags to the QAPI schema language, so that we can make
such changes visible with schema introspection.
An example for a schema definition using feature flags looks like this:
{ 'struct': 'TestType',
'data': { 'number': 'int' },
'features': [ 'allow-negative-numbers' ] }
Introspection information then looks like this:
{ "name": "TestType", "meta-type": "object",
"members": [
{ "name": "number", "type": "int" } ],
"features": [ "allow-negative-numbers" ] }
This patch implements feature flags only for struct types. We'll
implement them more widely as needed.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
qapi/introspect.json | 6 ++-
docs/devel/qapi-code-gen.txt | 38 +++++++++++++++
scripts/qapi/common.py | 66 ++++++++++++++++++++++----
scripts/qapi/doc.py | 3 +-
scripts/qapi/introspect.py | 6 ++-
scripts/qapi/types.py | 3 +-
scripts/qapi/visit.py | 3 +-
tests/qapi-schema/double-type.err | 2 +-
tests/qapi-schema/test-qapi.py | 3 +-
tests/qapi-schema/unknown-expr-key.err | 2 +-
10 files changed, 114 insertions(+), 18 deletions(-)
diff --git a/qapi/introspect.json b/qapi/introspect.json
index 3d22166b2b..1843c1cb17 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -174,6 +174,9 @@
# and may even differ from the order of the values of the
# enum type of the @tag.
#
+# @features: names of features associated with the type, in no particular
+# order. (since: 4.1)
+#
# Values of this type are JSON object on the wire.
#
# Since: 2.5
@@ -181,7 +184,8 @@
{ 'struct': 'SchemaInfoObject',
'data': { 'members': [ 'SchemaInfoObjectMember' ],
'*tag': 'str',
- '*variants': [ 'SchemaInfoObjectVariant' ] } }
+ '*variants': [ 'SchemaInfoObjectVariant' ],
+ '*features': [ 'str' ] } }
##
# @SchemaInfoObjectMember:
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index b517b0cfbf..e8ec8ac1de 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -719,6 +719,34 @@ any non-empty complex type (struct, union, or alternate), and a
pointer to that QAPI type is passed as a single argument.
+=== Features ===
+
+Sometimes, the behaviour of QEMU changes compatibly, but without a
+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.
+
+In the schema, features can be specified as simple strings, for example:
+
+{ 'struct': 'TestType',
+ 'data': { 'number': 'int' },
+ 'features': [ 'allow-negative-numbers' ] }
+
+Another option is to specify features as dictionaries, where the key
+'name' specifies the feature string to be exposed to clients:
+
+{ 'struct': 'TestType',
+ 'data': { 'number': 'int' },
+ 'features': [ { 'name': 'allow-negative-numbers' } ] }
+
+This expanded form is necessary if you want to make the feature
+conditional (see below in "Configuring the schema").
+
+
=== Downstream extensions ===
QAPI schema names that are externally visible, say in the Client JSON
@@ -771,6 +799,16 @@ Example: a conditional 'bar' enum member.
[ 'foo',
{ 'name' : 'bar', 'if': 'defined(IFCOND)' } ] }
+Similarly, features can be specified as a dictionary with a 'name' and
+an 'if' key.
+
+Example: a conditional 'allow-negative-numbers' feature
+
+{ 'struct': 'TestType',
+ 'data': { 'number': 'int' },
+ 'features': [ { 'name': 'allow-negative-numbers',
+ 'if' 'defined(IFCOND)' } ] }
+
Please note that you are responsible to ensure that the C code will
compile with an arbitrary combination of conditions, since the
generators are unable to check it at this point.
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index f07869ec73..9e4b6c00b5 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -886,12 +886,26 @@ def check_enum(expr, info):
def check_struct(expr, info):
name = expr['struct']
members = expr['data']
+ features = expr.get('features')
check_type(info, "'data' for struct '%s'" % name, members,
allow_dict=True, allow_optional=True)
check_type(info, "'base' for struct '%s'" % name, expr.get('base'),
allow_metas=['struct'])
+ if features:
+ if not isinstance(features, list):
+ raise QAPISemError(info,
+ "Struct '%s' requires an array for 'features'" %
+ name)
+ for f in features:
+ assert isinstance(f, dict)
+ check_known_keys(info, "feature of struct %s" % name, f,
+ ['name'], ['if'])
+
+ check_if(f, info)
+ check_name(info, "Feature of struct %s" % name, f['name'])
+
def check_known_keys(info, source, keys, required, optional):
@@ -948,6 +962,12 @@ def normalize_members(members):
members[key] = {'type': arg}
+def normalize_features(features):
+ if isinstance(features, list):
+ features[:] = [f if isinstance(f, dict) else {'name': f}
+ for f in features]
+
+
def check_exprs(exprs):
global all_names
@@ -986,8 +1006,10 @@ def check_exprs(exprs):
normalize_members(expr['data'])
elif 'struct' in expr:
meta = 'struct'
- check_keys(expr_elem, 'struct', ['data'], ['base', 'if'])
+ check_keys(expr_elem, 'struct', ['data'],
+ ['base', 'if', 'features'])
normalize_members(expr['data'])
+ normalize_features(expr.get('features'))
struct_types[expr[meta]] = expr
elif 'command' in expr:
meta = 'command'
@@ -1126,10 +1148,12 @@ class QAPISchemaVisitor(object):
def visit_array_type(self, name, info, ifcond, element_type):
pass
- def visit_object_type(self, name, info, ifcond, base, members, variants):
+ def visit_object_type(self, name, info, ifcond, base, members, variants,
+ features):
pass
- def visit_object_type_flat(self, name, info, ifcond, members, variants):
+ def visit_object_type_flat(self, name, info, ifcond, members, variants,
+ features):
pass
def visit_alternate_type(self, name, info, ifcond, variants):
@@ -1290,7 +1314,7 @@ class QAPISchemaArrayType(QAPISchemaType):
class QAPISchemaObjectType(QAPISchemaType):
def __init__(self, name, info, doc, ifcond,
- base, local_members, variants):
+ base, local_members, variants, features):
# struct has local_members, optional base, and no variants
# flat union has base, variants, and no local_members
# simple union has local_members, variants, and no base
@@ -1302,11 +1326,15 @@ class QAPISchemaObjectType(QAPISchemaType):
if variants is not None:
assert isinstance(variants, QAPISchemaObjectTypeVariants)
variants.set_owner(name)
+ for f in features:
+ assert isinstance(f, QAPISchemaFeature)
+ f.set_owner(name)
self._base_name = base
self.base = None
self.local_members = local_members
self.variants = variants
self.members = None
+ self.features = features
def check(self, schema):
QAPISchemaType.check(self, schema)
@@ -1332,6 +1360,12 @@ class QAPISchemaObjectType(QAPISchemaType):
self.variants.check(schema, seen)
assert self.variants.tag_member in self.members
self.variants.check_clash(self.info, seen)
+
+ # Features are in a name space separate from members
+ seen = {}
+ for f in self.features:
+ f.check_clash(self.info, seen)
+
if self.doc:
self.doc.check()
@@ -1368,12 +1402,15 @@ class QAPISchemaObjectType(QAPISchemaType):
def visit(self, visitor):
visitor.visit_object_type(self.name, self.info, self.ifcond,
- self.base, self.local_members, self.variants)
+ self.base, self.local_members, self.variants,
+ self.features)
visitor.visit_object_type_flat(self.name, self.info, self.ifcond,
- self.members, self.variants)
+ self.members, self.variants,
+ self.features)
class QAPISchemaMember(object):
+ """ Represents object members, enum members and features """
role = 'member'
def __init__(self, name, ifcond=None):
@@ -1419,6 +1456,10 @@ class QAPISchemaMember(object):
return "'%s' %s" % (self.name, self._pretty_owner())
+class QAPISchemaFeature(QAPISchemaMember):
+ role = 'feature'
+
+
class QAPISchemaObjectTypeMember(QAPISchemaMember):
def __init__(self, name, typ, optional, ifcond=None):
QAPISchemaMember.__init__(self, name, ifcond)
@@ -1675,7 +1716,7 @@ class QAPISchema(object):
('null', 'null', 'QNull' + pointer_suffix)]:
self._def_builtin_type(*t)
self.the_empty_object_type = QAPISchemaObjectType(
- 'q_empty', None, None, None, None, [], None)
+ 'q_empty', None, None, None, None, [], None, [])
self._def_entity(self.the_empty_object_type)
qtypes = ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist',
@@ -1685,6 +1726,9 @@ class QAPISchema(object):
self._def_entity(QAPISchemaEnumType('QType', None, None, None,
qtype_values, 'QTYPE'))
+ def _make_features(self, features):
+ return [QAPISchemaFeature(f['name'], f.get('if')) for f in features]
+
def _make_enum_members(self, values):
return [QAPISchemaMember(v['name'], v.get('if')) for v in values]
@@ -1721,7 +1765,7 @@ class QAPISchema(object):
assert ifcond == typ._ifcond # pylint: disable=protected-access
else:
self._def_entity(QAPISchemaObjectType(name, info, doc, ifcond,
- None, members, None))
+ None, members, None, []))
return name
def _def_enum_type(self, expr, info, doc):
@@ -1752,9 +1796,11 @@ class QAPISchema(object):
base = expr.get('base')
data = expr['data']
ifcond = expr.get('if')
+ features = expr.get('features', [])
self._def_entity(QAPISchemaObjectType(name, info, doc, ifcond, base,
self._make_members(data, info),
- None))
+ None,
+ self._make_features(features)))
def _make_variant(self, case, typ, ifcond):
return QAPISchemaObjectTypeVariant(case, typ, ifcond)
@@ -1795,7 +1841,7 @@ class QAPISchema(object):
QAPISchemaObjectType(name, info, doc, ifcond, base, members,
QAPISchemaObjectTypeVariants(tag_name,
tag_member,
- variants)))
+ variants), []))
def _def_alternate_type(self, expr, info, doc):
name = expr['alternate']
diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 5c8c136899..433e9fcbfb 100755
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -220,7 +220,8 @@ class QAPISchemaGenDocVisitor(qapi.common.QAPISchemaVisitor):
body=texi_entity(doc, 'Values', ifcond,
member_func=texi_enum_value)))
- def visit_object_type(self, name, info, ifcond, base, members, variants):
+ def visit_object_type(self, name, info, ifcond, base, members, variants,
+ features):
doc = self.cur_doc
if base and base.is_implicit():
base = None
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index f7f2ca07e4..f62cf0a2e1 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -188,11 +188,15 @@ const QLitObject %(c_name)s = %(c_string)s;
self._gen_qlit('[' + element + ']', 'array', {'element-type': element},
ifcond)
- def visit_object_type_flat(self, name, info, ifcond, members, variants):
+ def visit_object_type_flat(self, name, info, ifcond, members, variants,
+ features):
obj = {'members': [self._gen_member(m) for m in members]}
if variants:
obj.update(self._gen_variants(variants.tag_member.name,
variants.variants))
+ if features:
+ obj['features'] = [(f.name, {'if': f.ifcond}) for f in features]
+
self._gen_qlit(name, 'object', obj, ifcond)
def visit_alternate_type(self, name, info, ifcond, variants):
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index 2bd6fcd44f..3edd9374aa 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -227,7 +227,8 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
self._genh.add(gen_array(name, element_type))
self._gen_type_cleanup(name)
- def visit_object_type(self, name, info, ifcond, base, members, variants):
+ def visit_object_type(self, name, info, ifcond, base, members, variants,
+ features):
# Nothing to do for the special empty builtin
if name == 'q_empty':
return
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 826b8066e1..c1cd675c95 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -326,7 +326,8 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor):
self._genh.add(gen_visit_decl(name))
self._genc.add(gen_visit_list(name, element_type))
- def visit_object_type(self, name, info, ifcond, base, members, variants):
+ def visit_object_type(self, name, info, ifcond, base, members, variants,
+ features):
# Nothing to do for the special empty builtin
if name == 'q_empty':
return
diff --git a/tests/qapi-schema/double-type.err b/tests/qapi-schema/double-type.err
index 799193dba1..69457173a7 100644
--- a/tests/qapi-schema/double-type.err
+++ b/tests/qapi-schema/double-type.err
@@ -1,2 +1,2 @@
tests/qapi-schema/double-type.json:2: Unknown key 'command' in struct 'bar'
-Valid keys are 'base', 'data', 'if', 'struct'.
+Valid keys are 'base', 'data', 'features', 'if', 'struct'.
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index d21fca01fc..f2d6815c86 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -38,7 +38,8 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
print('array %s %s' % (name, element_type.name))
self._print_if(ifcond)
- def visit_object_type(self, name, info, ifcond, base, members, variants):
+ def visit_object_type(self, name, info, ifcond, base, members, variants,
+ features):
print('object %s' % name)
if base:
print(' base %s' % base.name)
diff --git a/tests/qapi-schema/unknown-expr-key.err b/tests/qapi-schema/unknown-expr-key.err
index 6ff8bb99c5..4340eaf894 100644
--- a/tests/qapi-schema/unknown-expr-key.err
+++ b/tests/qapi-schema/unknown-expr-key.err
@@ -1,2 +1,2 @@
tests/qapi-schema/unknown-expr-key.json:2: Unknown keys 'bogus', 'phony' in struct 'bar'
-Valid keys are 'base', 'data', 'if', 'struct'.
+Valid keys are 'base', 'data', 'features', 'if', 'struct'.
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v4 2/6] tests/qapi-schema: Test for good feature lists in structs
2019-05-30 11:02 [Qemu-devel] [PATCH v4 0/6] file-posix: Add dynamic-auto-read-only QAPI feature Kevin Wolf
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 1/6] qapi: Add feature flags to struct types Kevin Wolf
@ 2019-05-30 11:02 ` Kevin Wolf
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 3/6] tests/qapi-schema: Error case tests for features " Kevin Wolf
` (4 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2019-05-30 11:02 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, pkrempa, armbru, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qapi-schema/qapi-schema-test.json | 39 ++++++++++++++++++++++
tests/test-qmp-cmds.c | 8 +++++
tests/qapi-schema/qapi-schema-test.out | 43 +++++++++++++++++++++++++
tests/qapi-schema/test-qapi.py | 4 +++
4 files changed, 94 insertions(+)
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 0952c68734..c6d59acc3e 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -242,3 +242,42 @@
{ 'foo': 'TestIfStruct',
'bar': { 'type': ['TestIfEnum'], 'if': 'defined(TEST_IF_EVT_BAR)' } },
'if': 'defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)' }
+
+# test 'features' for structs
+
+{ 'struct': 'FeatureStruct0',
+ 'data': { 'foo': 'int' },
+ 'features': [] }
+{ 'struct': 'FeatureStruct1',
+ 'data': { 'foo': 'int' },
+ 'features': [ 'feature1' ] }
+{ 'struct': 'FeatureStruct2',
+ 'data': { 'foo': 'int' },
+ 'features': [ { 'name': 'feature1' } ] }
+{ 'struct': 'FeatureStruct3',
+ 'data': { 'foo': 'int' },
+ 'features': [ 'feature1', 'feature2' ] }
+{ 'struct': 'FeatureStruct4',
+ 'data': { 'namespace-test': 'int' },
+ 'features': [ 'namespace-test', 'int', 'name', 'if' ] }
+
+{ 'struct': 'CondFeatureStruct1',
+ 'data': { 'foo': 'int' },
+ 'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'} ] }
+{ 'struct': 'CondFeatureStruct2',
+ 'data': { 'foo': 'int' },
+ 'features': [ { 'name': 'feature1', 'if': 'defined(TEST_IF_FEATURE_1)'},
+ { 'name': 'feature2', 'if': 'defined(TEST_IF_FEATURE_2)'} ] }
+{ 'struct': 'CondFeatureStruct3',
+ 'data': { 'foo': 'int' },
+ 'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)',
+ 'defined(TEST_IF_COND_2)'] } ] }
+{ 'command': 'test-features',
+ 'data': { 'fs0': 'FeatureStruct0',
+ 'fs1': 'FeatureStruct1',
+ 'fs2': 'FeatureStruct2',
+ 'fs3': 'FeatureStruct3',
+ 'fs4': 'FeatureStruct4',
+ 'cfs1': 'CondFeatureStruct1',
+ 'cfs2': 'CondFeatureStruct2',
+ 'cfs3': 'CondFeatureStruct3' } }
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index 630b1b9bac..1f738f12e2 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -45,6 +45,14 @@ void qmp_user_def_cmd1(UserDefOne * ud1, Error **errp)
{
}
+void qmp_test_features(FeatureStruct0 *fs0, FeatureStruct1 *fs1,
+ FeatureStruct2 *fs2, FeatureStruct3 *fs3,
+ FeatureStruct4 *fs4, CondFeatureStruct1 *cfs1,
+ CondFeatureStruct2 *cfs2, CondFeatureStruct3 *cfs3,
+ 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.out b/tests/qapi-schema/qapi-schema-test.out
index 77fb1e1aa9..85d510bc00 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -354,3 +354,46 @@ object q_obj_TestIfEvent-arg
event TestIfEvent q_obj_TestIfEvent-arg
boxed=False
if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)']
+object FeatureStruct0
+ member foo: int optional=False
+object FeatureStruct1
+ member foo: int optional=False
+ feature feature1
+object FeatureStruct2
+ member foo: int optional=False
+ feature feature1
+object FeatureStruct3
+ member foo: int optional=False
+ feature feature1
+ feature feature2
+object FeatureStruct4
+ member namespace-test: int optional=False
+ feature namespace-test
+ feature int
+ feature name
+ feature if
+object CondFeatureStruct1
+ member foo: int optional=False
+ feature feature1
+ if ['defined(TEST_IF_FEATURE_1)']
+object CondFeatureStruct2
+ member foo: int optional=False
+ feature feature1
+ if ['defined(TEST_IF_FEATURE_1)']
+ feature feature2
+ if ['defined(TEST_IF_FEATURE_2)']
+object CondFeatureStruct3
+ member foo: int optional=False
+ feature feature1
+ if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)']
+object q_obj_test-features-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-features q_obj_test-features-arg -> None
+ 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 f2d6815c86..b0f770b9bd 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -49,6 +49,10 @@ 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)
def visit_alternate_type(self, name, info, ifcond, variants):
print('alternate %s' % name)
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v4 3/6] tests/qapi-schema: Error case tests for features in structs
2019-05-30 11:02 [Qemu-devel] [PATCH v4 0/6] file-posix: Add dynamic-auto-read-only QAPI feature Kevin Wolf
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 1/6] qapi: Add feature flags to struct types Kevin Wolf
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 2/6] tests/qapi-schema: Test for good feature lists in structs Kevin Wolf
@ 2019-05-30 11:02 ` Kevin Wolf
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 4/6] qapi: Disentangle QAPIDoc code Kevin Wolf
` (3 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2019-05-30 11:02 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, pkrempa, armbru, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qapi-schema/features-bad-type.json | 3 +++
tests/qapi-schema/features-duplicate-name.json | 3 +++
tests/qapi-schema/features-missing-name.json | 3 +++
tests/qapi-schema/features-name-bad-type.json | 3 +++
tests/qapi-schema/features-no-list.json | 3 +++
tests/qapi-schema/features-unknown-key.json | 3 +++
tests/Makefile.include | 6 ++++++
tests/qapi-schema/features-bad-type.err | 1 +
tests/qapi-schema/features-bad-type.exit | 1 +
tests/qapi-schema/features-bad-type.out | 0
tests/qapi-schema/features-duplicate-name.err | 1 +
tests/qapi-schema/features-duplicate-name.exit | 1 +
tests/qapi-schema/features-duplicate-name.out | 0
tests/qapi-schema/features-missing-name.err | 1 +
tests/qapi-schema/features-missing-name.exit | 1 +
tests/qapi-schema/features-missing-name.out | 0
tests/qapi-schema/features-name-bad-type.err | 1 +
tests/qapi-schema/features-name-bad-type.exit | 1 +
tests/qapi-schema/features-name-bad-type.out | 0
tests/qapi-schema/features-no-list.err | 1 +
tests/qapi-schema/features-no-list.exit | 1 +
tests/qapi-schema/features-no-list.out | 0
tests/qapi-schema/features-unknown-key.err | 2 ++
tests/qapi-schema/features-unknown-key.exit | 1 +
tests/qapi-schema/features-unknown-key.out | 0
25 files changed, 37 insertions(+)
create mode 100644 tests/qapi-schema/features-bad-type.json
create mode 100644 tests/qapi-schema/features-duplicate-name.json
create mode 100644 tests/qapi-schema/features-missing-name.json
create mode 100644 tests/qapi-schema/features-name-bad-type.json
create mode 100644 tests/qapi-schema/features-no-list.json
create mode 100644 tests/qapi-schema/features-unknown-key.json
create mode 100644 tests/qapi-schema/features-bad-type.err
create mode 100644 tests/qapi-schema/features-bad-type.exit
create mode 100644 tests/qapi-schema/features-bad-type.out
create mode 100644 tests/qapi-schema/features-duplicate-name.err
create mode 100644 tests/qapi-schema/features-duplicate-name.exit
create mode 100644 tests/qapi-schema/features-duplicate-name.out
create mode 100644 tests/qapi-schema/features-missing-name.err
create mode 100644 tests/qapi-schema/features-missing-name.exit
create mode 100644 tests/qapi-schema/features-missing-name.out
create mode 100644 tests/qapi-schema/features-name-bad-type.err
create mode 100644 tests/qapi-schema/features-name-bad-type.exit
create mode 100644 tests/qapi-schema/features-name-bad-type.out
create mode 100644 tests/qapi-schema/features-no-list.err
create mode 100644 tests/qapi-schema/features-no-list.exit
create mode 100644 tests/qapi-schema/features-no-list.out
create mode 100644 tests/qapi-schema/features-unknown-key.err
create mode 100644 tests/qapi-schema/features-unknown-key.exit
create mode 100644 tests/qapi-schema/features-unknown-key.out
diff --git a/tests/qapi-schema/features-bad-type.json b/tests/qapi-schema/features-bad-type.json
new file mode 100644
index 0000000000..57db5540e7
--- /dev/null
+++ b/tests/qapi-schema/features-bad-type.json
@@ -0,0 +1,3 @@
+{ 'struct': 'FeatureStruct0',
+ 'data': { 'foo': 'int' },
+ 'features': [ [ 'a feature cannot be an array' ] ] }
diff --git a/tests/qapi-schema/features-duplicate-name.json b/tests/qapi-schema/features-duplicate-name.json
new file mode 100644
index 0000000000..29358e6220
--- /dev/null
+++ b/tests/qapi-schema/features-duplicate-name.json
@@ -0,0 +1,3 @@
+{ 'struct': 'FeatureStruct0',
+ 'data': { 'foo': 'int' },
+ 'features': [ 'foo', 'bar', 'foo' ] }
diff --git a/tests/qapi-schema/features-missing-name.json b/tests/qapi-schema/features-missing-name.json
new file mode 100644
index 0000000000..2314f97c00
--- /dev/null
+++ b/tests/qapi-schema/features-missing-name.json
@@ -0,0 +1,3 @@
+{ 'struct': 'FeatureStruct0',
+ 'data': { 'foo': 'int' },
+ 'features': [ { 'if': 'defined(NAMELESS_FEATURES)' } ] }
diff --git a/tests/qapi-schema/features-name-bad-type.json b/tests/qapi-schema/features-name-bad-type.json
new file mode 100644
index 0000000000..b07139978a
--- /dev/null
+++ b/tests/qapi-schema/features-name-bad-type.json
@@ -0,0 +1,3 @@
+{ 'struct': 'FeatureStruct0',
+ 'data': { 'foo': 'int' },
+ 'features': [ { 'name': { 'feature-type': 'object' } } ] }
diff --git a/tests/qapi-schema/features-no-list.json b/tests/qapi-schema/features-no-list.json
new file mode 100644
index 0000000000..9484fd94fc
--- /dev/null
+++ b/tests/qapi-schema/features-no-list.json
@@ -0,0 +1,3 @@
+{ 'struct': 'FeatureStruct0',
+ 'data': { 'foo': 'int' },
+ 'features': 'bar' }
diff --git a/tests/qapi-schema/features-unknown-key.json b/tests/qapi-schema/features-unknown-key.json
new file mode 100644
index 0000000000..134df3b503
--- /dev/null
+++ b/tests/qapi-schema/features-unknown-key.json
@@ -0,0 +1,3 @@
+{ 'struct': 'FeatureStruct0',
+ 'data': { 'foo': 'int' },
+ 'features': [ { 'name': 'bar', 'colour': 'red' } ] }
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 1865f6b322..0f6c4583c5 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -377,6 +377,12 @@ qapi-schema += event-boxed-empty.json
qapi-schema += event-case.json
qapi-schema += event-member-invalid-dict.json
qapi-schema += event-nest-struct.json
+qapi-schema += features-bad-type.json
+qapi-schema += features-duplicate-name.json
+qapi-schema += features-missing-name.json
+qapi-schema += features-name-bad-type.json
+qapi-schema += features-no-list.json
+qapi-schema += features-unknown-key.json
qapi-schema += flat-union-array-branch.json
qapi-schema += flat-union-bad-base.json
qapi-schema += flat-union-bad-discriminator.json
diff --git a/tests/qapi-schema/features-bad-type.err b/tests/qapi-schema/features-bad-type.err
new file mode 100644
index 0000000000..5fb95c2f90
--- /dev/null
+++ b/tests/qapi-schema/features-bad-type.err
@@ -0,0 +1 @@
+tests/qapi-schema/features-bad-type.json:1: Feature of struct FeatureStruct0 requires a string name
diff --git a/tests/qapi-schema/features-bad-type.exit b/tests/qapi-schema/features-bad-type.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/features-bad-type.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/features-bad-type.out b/tests/qapi-schema/features-bad-type.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/features-duplicate-name.err b/tests/qapi-schema/features-duplicate-name.err
new file mode 100644
index 0000000000..c0a4cccae6
--- /dev/null
+++ b/tests/qapi-schema/features-duplicate-name.err
@@ -0,0 +1 @@
+tests/qapi-schema/features-duplicate-name.json:1: 'foo' (feature of FeatureStruct0) collides with 'foo' (feature of FeatureStruct0)
diff --git a/tests/qapi-schema/features-duplicate-name.exit b/tests/qapi-schema/features-duplicate-name.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/features-duplicate-name.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/features-duplicate-name.out b/tests/qapi-schema/features-duplicate-name.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/features-missing-name.err b/tests/qapi-schema/features-missing-name.err
new file mode 100644
index 0000000000..4f1d2715aa
--- /dev/null
+++ b/tests/qapi-schema/features-missing-name.err
@@ -0,0 +1 @@
+tests/qapi-schema/features-missing-name.json:1: Key 'name' is missing from feature of struct FeatureStruct0
diff --git a/tests/qapi-schema/features-missing-name.exit b/tests/qapi-schema/features-missing-name.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/features-missing-name.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/features-missing-name.out b/tests/qapi-schema/features-missing-name.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/features-name-bad-type.err b/tests/qapi-schema/features-name-bad-type.err
new file mode 100644
index 0000000000..8a3eecb972
--- /dev/null
+++ b/tests/qapi-schema/features-name-bad-type.err
@@ -0,0 +1 @@
+tests/qapi-schema/features-name-bad-type.json:1: Feature of struct FeatureStruct0 requires a string name
diff --git a/tests/qapi-schema/features-name-bad-type.exit b/tests/qapi-schema/features-name-bad-type.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/features-name-bad-type.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/features-name-bad-type.out b/tests/qapi-schema/features-name-bad-type.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/features-no-list.err b/tests/qapi-schema/features-no-list.err
new file mode 100644
index 0000000000..61ed68612b
--- /dev/null
+++ b/tests/qapi-schema/features-no-list.err
@@ -0,0 +1 @@
+tests/qapi-schema/features-no-list.json:1: Struct 'FeatureStruct0' requires an array for 'features'
diff --git a/tests/qapi-schema/features-no-list.exit b/tests/qapi-schema/features-no-list.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/features-no-list.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/features-no-list.out b/tests/qapi-schema/features-no-list.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/features-unknown-key.err b/tests/qapi-schema/features-unknown-key.err
new file mode 100644
index 0000000000..a1d693030d
--- /dev/null
+++ b/tests/qapi-schema/features-unknown-key.err
@@ -0,0 +1,2 @@
+tests/qapi-schema/features-unknown-key.json:1: Unknown key 'colour' in feature of struct FeatureStruct0
+Valid keys are 'if', 'name'.
diff --git a/tests/qapi-schema/features-unknown-key.exit b/tests/qapi-schema/features-unknown-key.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/features-unknown-key.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/features-unknown-key.out b/tests/qapi-schema/features-unknown-key.out
new file mode 100644
index 0000000000..e69de29bb2
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v4 4/6] qapi: Disentangle QAPIDoc code
2019-05-30 11:02 [Qemu-devel] [PATCH v4 0/6] file-posix: Add dynamic-auto-read-only QAPI feature Kevin Wolf
` (2 preceding siblings ...)
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 3/6] tests/qapi-schema: Error case tests for features " Kevin Wolf
@ 2019-05-30 11:02 ` Kevin Wolf
2019-06-06 11:26 ` Markus Armbruster
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 5/6] qapi: Allow documentation for features Kevin Wolf
` (2 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Kevin Wolf @ 2019-05-30 11:02 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, pkrempa, armbru, qemu-devel
Documentation comments follow a certain structure: First, we have a text
with a general description (called QAPIDoc.body). After this,
descriptions of the arguments follow. Finally, we have a part that
contains various named sections.
The code doesn't show this structure, but just checks various attributes
that indicate indirectly which part is being processed, so it happens to
do the right set of things in the right phase. This is hard to follow,
and adding support for documentation of features would be even harder.
This patch restructures the code so that the three parts are clearly
separated. The code becomes a bit longer, but easier to follow. The
resulting output remains unchanged.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
scripts/qapi/common.py | 122 ++++++++++++++++++++++++++++++++---------
1 file changed, 97 insertions(+), 25 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 9e4b6c00b5..55ccd216cf 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -120,6 +120,22 @@ class QAPIDoc(object):
def connect(self, member):
self.member = member
+ class DocPart:
+ """
+ Expression documentation blocks consist of
+ * a BODY part: first line naming the expression, plus an
+ optional overview
+ * an ARGS part: description of each argument (for commands and
+ events) or member (for structs, unions and alternates),
+ * a VARIOUS part: optional tagged sections.
+
+ Free-form documentation blocks consist only of a BODY part.
+ """
+ # TODO Make it a subclass of Enum when Python 2 support is removed
+ BODY = 1
+ ARGS = 2
+ VARIOUS = 3
+
def __init__(self, parser, info):
# self._parser is used to report errors with QAPIParseError. The
# resulting error position depends on the state of the parser.
@@ -135,6 +151,7 @@ class QAPIDoc(object):
self.sections = []
# the current section
self._section = self.body
+ self._part = QAPIDoc.DocPart.BODY
def has_section(self, name):
"""Return True if we have a section with this name."""
@@ -144,7 +161,24 @@ class QAPIDoc(object):
return False
def append(self, line):
- """Parse a comment line and add it to the documentation."""
+ """
+ Parse a comment line and add it to the documentation.
+
+ The way that the line is dealt with depends on which part of the
+ documentation we're parsing right now:
+
+ BODY means that we're ready to process free-form text into self.body. A
+ symbol name is only allowed if no other text was parsed yet. It is
+ interpreted as the symbol name that describes the currently documented
+ object. On getting the second symbol name, we proceed to ARGS.
+
+ ARGS means that we're parsing the arguments section. Any symbol name is
+ interpreted as an argument and an ArgSection is created for it.
+
+ VARIOUS is the final part where free-form sections may appear. This
+ includes named sections such as "Return:" as well as unnamed
+ paragraphs. Symbols are not allowed any more in this part.
+ """
line = line[1:]
if not line:
self._append_freeform(line)
@@ -154,37 +188,85 @@ class QAPIDoc(object):
raise QAPIParseError(self._parser, "Missing space after #")
line = line[1:]
+ if self._part == QAPIDoc.DocPart.BODY:
+ self._append_body_line(line)
+ elif self._part == QAPIDoc.DocPart.ARGS:
+ self._append_args_line(line)
+ elif self._part == QAPIDoc.DocPart.VARIOUS:
+ self._append_various_line(line)
+ else:
+ assert False
+
+ def end_comment(self):
+ self._end_section()
+
+ def _check_named_section(self, name):
+ if name in ('Returns:', 'Since:',
+ # those are often singular or plural
+ 'Note:', 'Notes:',
+ 'Example:', 'Examples:',
+ 'TODO:'):
+ self._part = QAPIDoc.DocPart.VARIOUS
+ return True
+ return False
+
+ def _append_body_line(self, line):
+ name = line.split(' ', 1)[0]
# FIXME not nice: things like '# @foo:' and '# @foo: ' aren't
# recognized, and get silently treated as ordinary text
- if self.symbol:
- self._append_symbol_line(line)
- elif not self.body.text and line.startswith('@'):
+ if not self.symbol and not self.body.text and line.startswith('@'):
if not line.endswith(':'):
raise QAPIParseError(self._parser, "Line should end with :")
self.symbol = line[1:-1]
# FIXME invalid names other than the empty string aren't flagged
if not self.symbol:
raise QAPIParseError(self._parser, "Invalid name")
+ elif self.symbol:
+ # We already know that we document some symbol
+ if name.startswith('@') and name.endswith(':'):
+ self._part = QAPIDoc.DocPart.ARGS
+ self._append_args_line(line)
+ elif self.symbol and self._check_named_section(name):
+ self._append_various_line(line)
+ else:
+ self._append_freeform(line.strip())
else:
- self._append_freeform(line)
-
- def end_comment(self):
- self._end_section()
+ # This is free-form documentation without a symbol
+ self._append_freeform(line.strip())
- def _append_symbol_line(self, line):
+ def _append_args_line(self, line):
name = line.split(' ', 1)[0]
if name.startswith('@') and name.endswith(':'):
line = line[len(name)+1:]
self._start_args_section(name[1:-1])
- elif name in ('Returns:', 'Since:',
- # those are often singular or plural
- 'Note:', 'Notes:',
- 'Example:', 'Examples:',
- 'TODO:'):
+ elif self._check_named_section(name):
+ self._append_various_line(line)
+ return
+ elif (self._section.text.endswith('\n\n')
+ and line and not line[0].isspace()):
+ self._start_section()
+ self._part = QAPIDoc.DocPart.VARIOUS
+ self._append_various_line(line)
+ return
+
+ self._append_freeform(line.strip())
+
+ def _append_various_line(self, line):
+ name = line.split(' ', 1)[0]
+
+ if name.startswith('@') and name.endswith(':'):
+ raise QAPIParseError(self._parser,
+ "'%s' can't follow '%s' section"
+ % (name, self.sections[0].name))
+ elif self._check_named_section(name):
line = line[len(name)+1:]
self._start_section(name[:-1])
+ if (not self._section.name or
+ not self._section.name.startswith('Example')):
+ line = line.strip()
+
self._append_freeform(line)
def _start_args_section(self, name):
@@ -194,10 +276,7 @@ class QAPIDoc(object):
if name in self.args:
raise QAPIParseError(self._parser,
"'%s' parameter name duplicated" % name)
- if self.sections:
- raise QAPIParseError(self._parser,
- "'@%s:' can't follow '%s' section"
- % (name, self.sections[0].name))
+ assert not self.sections
self._end_section()
self._section = QAPIDoc.ArgSection(name)
self.args[name] = self._section
@@ -219,13 +298,6 @@ class QAPIDoc(object):
self._section = None
def _append_freeform(self, line):
- in_arg = isinstance(self._section, QAPIDoc.ArgSection)
- if (in_arg and self._section.text.endswith('\n\n')
- and line and not line[0].isspace()):
- self._start_section()
- if (in_arg or not self._section.name
- or not self._section.name.startswith('Example')):
- line = line.strip()
match = re.match(r'(@\S+:)', line)
if match:
raise QAPIParseError(self._parser,
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v4 5/6] qapi: Allow documentation for features
2019-05-30 11:02 [Qemu-devel] [PATCH v4 0/6] file-posix: Add dynamic-auto-read-only QAPI feature Kevin Wolf
` (3 preceding siblings ...)
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 4/6] qapi: Disentangle QAPIDoc code Kevin Wolf
@ 2019-05-30 11:02 ` Kevin Wolf
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 6/6] file-posix: Add dynamic-auto-read-only QAPI feature Kevin Wolf
2019-06-06 14:27 ` [Qemu-devel] [PATCH v4 0/6] " Markus Armbruster
6 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2019-05-30 11:02 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, pkrempa, armbru, qemu-devel
Features will be documented in a new part introduced by a "Features:"
line, after arguments and before named sections.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
scripts/qapi/common.py | 42 ++++++++++++++++++++++++++++++++++++++----
scripts/qapi/doc.py | 12 ++++++++++++
2 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 55ccd216cf..bc09e7d358 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -127,6 +127,7 @@ class QAPIDoc(object):
optional overview
* an ARGS part: description of each argument (for commands and
events) or member (for structs, unions and alternates),
+ * a FEATURES part: description of each feature,
* a VARIOUS part: optional tagged sections.
Free-form documentation blocks consist only of a BODY part.
@@ -134,7 +135,8 @@ class QAPIDoc(object):
# TODO Make it a subclass of Enum when Python 2 support is removed
BODY = 1
ARGS = 2
- VARIOUS = 3
+ FEATURES = 3
+ VARIOUS = 4
def __init__(self, parser, info):
# self._parser is used to report errors with QAPIParseError. The
@@ -147,6 +149,7 @@ class QAPIDoc(object):
self.body = QAPIDoc.Section()
# dict mapping parameter name to ArgSection
self.args = OrderedDict()
+ self.features = OrderedDict()
# a list of Section
self.sections = []
# the current section
@@ -192,6 +195,8 @@ class QAPIDoc(object):
self._append_body_line(line)
elif self._part == QAPIDoc.DocPart.ARGS:
self._append_args_line(line)
+ elif self._part == QAPIDoc.DocPart.FEATURES:
+ self._append_features_line(line)
elif self._part == QAPIDoc.DocPart.VARIOUS:
self._append_various_line(line)
else:
@@ -226,6 +231,8 @@ class QAPIDoc(object):
if name.startswith('@') and name.endswith(':'):
self._part = QAPIDoc.DocPart.ARGS
self._append_args_line(line)
+ elif line == 'Features:':
+ self._part = QAPIDoc.DocPart.FEATURES
elif self.symbol and self._check_named_section(name):
self._append_various_line(line)
else:
@@ -243,6 +250,27 @@ class QAPIDoc(object):
elif self._check_named_section(name):
self._append_various_line(line)
return
+ elif (self._section.text.endswith('\n\n')
+ and line and not line[0].isspace()):
+ if line == 'Features:':
+ self._part = QAPIDoc.DocPart.FEATURES
+ else:
+ self._start_section()
+ self._part = QAPIDoc.DocPart.VARIOUS
+ self._append_various_line(line)
+ return
+
+ self._append_freeform(line.strip())
+
+ def _append_features_line(self, line):
+ name = line.split(' ', 1)[0]
+
+ if name.startswith('@') and name.endswith(':'):
+ line = line[len(name)+1:]
+ self._start_features_section(name[1:-1])
+ elif self._check_named_section(name):
+ self._append_various_line(line)
+ return
elif (self._section.text.endswith('\n\n')
and line and not line[0].isspace()):
self._start_section()
@@ -269,17 +297,23 @@ class QAPIDoc(object):
self._append_freeform(line)
- def _start_args_section(self, name):
+ def _start_symbol_section(self, symbols_dict, name):
# FIXME invalid names other than the empty string aren't flagged
if not name:
raise QAPIParseError(self._parser, "Invalid parameter name")
- if name in self.args:
+ if name in symbols_dict:
raise QAPIParseError(self._parser,
"'%s' parameter name duplicated" % name)
assert not self.sections
self._end_section()
self._section = QAPIDoc.ArgSection(name)
- self.args[name] = self._section
+ symbols_dict[name] = self._section
+
+ def _start_args_section(self, name):
+ self._start_symbol_section(self.args, name)
+
+ def _start_features_section(self, name):
+ self._start_symbol_section(self.features, name)
def _start_section(self, name=None):
if name in ('Returns', 'Since') and self.has_section(name):
diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 433e9fcbfb..5fc0fc7e06 100755
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -182,6 +182,17 @@ def texi_members(doc, what, base, variants, member_func):
return '\n@b{%s:}\n@table @asis\n%s@end table\n' % (what, items)
+def texi_features(doc):
+ """Format the table of features"""
+ items = ''
+ for section in doc.features.values():
+ desc = texi_format(section.text)
+ items += '@item @code{%s}\n%s' % (section.name, desc)
+ if not items:
+ return ''
+ return '\n@b{Features:}\n@table @asis\n%s@end table\n' % (items)
+
+
def texi_sections(doc, ifcond):
"""Format additional sections following arguments"""
body = ''
@@ -201,6 +212,7 @@ def texi_entity(doc, what, ifcond, base=None, variants=None,
member_func=texi_member):
return (texi_body(doc)
+ texi_members(doc, what, base, variants, member_func)
+ + texi_features(doc)
+ texi_sections(doc, ifcond))
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v4 6/6] file-posix: Add dynamic-auto-read-only QAPI feature
2019-05-30 11:02 [Qemu-devel] [PATCH v4 0/6] file-posix: Add dynamic-auto-read-only QAPI feature Kevin Wolf
` (4 preceding siblings ...)
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 5/6] qapi: Allow documentation for features Kevin Wolf
@ 2019-05-30 11:02 ` Kevin Wolf
2019-06-06 14:27 ` [Qemu-devel] [PATCH v4 0/6] " Markus Armbruster
6 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2019-05-30 11:02 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, pkrempa, armbru, qemu-devel
In commit 23dece19da4 ('file-posix: Make auto-read-only dynamic') ,
auto-read-only=on changed its behaviour in file-posix for the 4.0
release. This change cannot be detected through the usual mechanisms
like schema introspection. Add a new feature flag to the schema to
allow libvirt to detect the presence of the new behaviour.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
qapi/block-core.json | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 7ccbfff9d0..e820e7dfb0 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2843,6 +2843,15 @@
# file is large, do not use in production.
# (default: off) (since: 3.0)
#
+# Features:
+# @dynamic-auto-read-only: If present, enabled auto-read-only means that the
+# driver will open the image read-only at first,
+# dynamically reopen the image file read-write when
+# the first writer is attached to the node and reopen
+# read-only when the last writer is detached. This
+# allows to give QEMU write permissions only on demand
+# when an operation actually needs write access.
+#
# Since: 2.9
##
{ 'struct': 'BlockdevOptionsFile',
@@ -2852,7 +2861,9 @@
'*aio': 'BlockdevAioOptions',
'*drop-cache': {'type': 'bool',
'if': 'defined(CONFIG_LINUX)'},
- '*x-check-cache-dropped': 'bool' } }
+ '*x-check-cache-dropped': 'bool' },
+ 'features': [ { 'name': 'dynamic-auto-read-only',
+ 'if': 'defined(CONFIG_POSIX)' } ] }
##
# @BlockdevOptionsNull:
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4 4/6] qapi: Disentangle QAPIDoc code
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 4/6] qapi: Disentangle QAPIDoc code Kevin Wolf
@ 2019-06-06 11:26 ` Markus Armbruster
2019-06-06 12:01 ` [Qemu-devel] [PATCH v4 4.5/6] qapi: Replace QAPIDoc._part by ._append_line, and rework comments Markus Armbruster
0 siblings, 1 reply; 11+ messages in thread
From: Markus Armbruster @ 2019-06-06 11:26 UTC (permalink / raw)
To: Kevin Wolf; +Cc: pkrempa, qemu-devel, qemu-block
Kevin Wolf <kwolf@redhat.com> writes:
> Documentation comments follow a certain structure: First, we have a text
> with a general description (called QAPIDoc.body). After this,
> descriptions of the arguments follow. Finally, we have a part that
> contains various named sections.
>
> The code doesn't show this structure, but just checks various attributes
> that indicate indirectly which part is being processed, so it happens to
> do the right set of things in the right phase. This is hard to follow,
> and adding support for documentation of features would be even harder.
>
> This patch restructures the code so that the three parts are clearly
> separated. The code becomes a bit longer, but easier to follow. The
> resulting output remains unchanged.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> scripts/qapi/common.py | 122 ++++++++++++++++++++++++++++++++---------
> 1 file changed, 97 insertions(+), 25 deletions(-)
>
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 9e4b6c00b5..55ccd216cf 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -120,6 +120,22 @@ class QAPIDoc(object):
> def connect(self, member):
> self.member = member
>
> + class DocPart:
> + """
Did you drop the "A documentation part" headline intentionally?
> + Expression documentation blocks consist of
> + * a BODY part: first line naming the expression, plus an
> + optional overview
> + * an ARGS part: description of each argument (for commands and
> + events) or member (for structs, unions and alternates),
> + * a VARIOUS part: optional tagged sections.
> +
> + Free-form documentation blocks consist only of a BODY part.
> + """
> + # TODO Make it a subclass of Enum when Python 2 support is removed
> + BODY = 1
> + ARGS = 2
> + VARIOUS = 3
> +
> def __init__(self, parser, info):
> # self._parser is used to report errors with QAPIParseError. The
> # resulting error position depends on the state of the parser.
> @@ -135,6 +151,7 @@ class QAPIDoc(object):
> self.sections = []
> # the current section
> self._section = self.body
> + self._part = QAPIDoc.DocPart.BODY
>
> def has_section(self, name):
> """Return True if we have a section with this name."""
> @@ -144,7 +161,24 @@ class QAPIDoc(object):
> return False
>
> def append(self, line):
> - """Parse a comment line and add it to the documentation."""
> + """
> + Parse a comment line and add it to the documentation.
> +
> + The way that the line is dealt with depends on which part of the
> + documentation we're parsing right now:
> +
> + BODY means that we're ready to process free-form text into self.body. A
> + symbol name is only allowed if no other text was parsed yet. It is
> + interpreted as the symbol name that describes the currently documented
> + object. On getting the second symbol name, we proceed to ARGS.
> +
> + ARGS means that we're parsing the arguments section. Any symbol name is
> + interpreted as an argument and an ArgSection is created for it.
> +
> + VARIOUS is the final part where free-form sections may appear. This
> + includes named sections such as "Return:" as well as unnamed
> + paragraphs. Symbols are not allowed any more in this part.
> + """
A few little things:
* The reader has to make the connection from BODY, ARGS, VARIOUS to
self._part. To help him a bit, I'd say something like "depends on
which part of the documentation we're parsing right now, according to
self._part:"
* In the paragraph on BODY: on first reading, "A symbol name is only
allowed if no other text was parsed yet" appears to contradict "On
getting the second symbol name". It doesn't: the second symbol name
doesn't belong to this part. Perhaps we could phrase this more
clearly. Not sure it's worth the trouble.
* PEP 8: "For flowing long blocks of text with fewer structural
restrictions (docstrings or comments), the line length should be
limited to 72 characters."
* PEP 8: "You should use two spaces after a sentence-ending period in
multi-sentence comments, except after the final sentence."
> line = line[1:]
> if not line:
> self._append_freeform(line)
> @@ -154,37 +188,85 @@ class QAPIDoc(object):
> raise QAPIParseError(self._parser, "Missing space after #")
> line = line[1:]
>
> + if self._part == QAPIDoc.DocPart.BODY:
> + self._append_body_line(line)
> + elif self._part == QAPIDoc.DocPart.ARGS:
> + self._append_args_line(line)
> + elif self._part == QAPIDoc.DocPart.VARIOUS:
> + self._append_various_line(line)
> + else:
> + assert False
In my review of v2, I suggested to use a function-valued variable for
the state machine. I explored this, and will send my findings
separately.
> +
> + def end_comment(self):
> + self._end_section()
> +
> + def _check_named_section(self, name):
> + if name in ('Returns:', 'Since:',
> + # those are often singular or plural
> + 'Note:', 'Notes:',
> + 'Example:', 'Examples:',
> + 'TODO:'):
> + self._part = QAPIDoc.DocPart.VARIOUS
> + return True
> + return False
The side effect hidden in this function confused me once again, so I
played with the code to see whether avoiding it would be bothersome.
Turns out it isn't, proposed fixup appended.
> +
> + def _append_body_line(self, line):
> + name = line.split(' ', 1)[0]
> # FIXME not nice: things like '# @foo:' and '# @foo: ' aren't
> # recognized, and get silently treated as ordinary text
> - if self.symbol:
> - self._append_symbol_line(line)
> - elif not self.body.text and line.startswith('@'):
> + if not self.symbol and not self.body.text and line.startswith('@'):
> if not line.endswith(':'):
> raise QAPIParseError(self._parser, "Line should end with :")
> self.symbol = line[1:-1]
> # FIXME invalid names other than the empty string aren't flagged
> if not self.symbol:
> raise QAPIParseError(self._parser, "Invalid name")
> + elif self.symbol:
> + # We already know that we document some symbol
> + if name.startswith('@') and name.endswith(':'):
> + self._part = QAPIDoc.DocPart.ARGS
> + self._append_args_line(line)
> + elif self.symbol and self._check_named_section(name):
Checking self.symbol again is redundant.
> + self._append_various_line(line)
> + else:
> + self._append_freeform(line.strip())
> else:
> - self._append_freeform(line)
> -
> - def end_comment(self):
> - self._end_section()
> + # This is free-form documentation without a symbol
> + self._append_freeform(line.strip())
>
> - def _append_symbol_line(self, line):
> + def _append_args_line(self, line):
> name = line.split(' ', 1)[0]
>
> if name.startswith('@') and name.endswith(':'):
> line = line[len(name)+1:]
> self._start_args_section(name[1:-1])
> - elif name in ('Returns:', 'Since:',
> - # those are often singular or plural
> - 'Note:', 'Notes:',
> - 'Example:', 'Examples:',
> - 'TODO:'):
> + elif self._check_named_section(name):
> + self._append_various_line(line)
> + return
> + elif (self._section.text.endswith('\n\n')
> + and line and not line[0].isspace()):
> + self._start_section()
> + self._part = QAPIDoc.DocPart.VARIOUS
> + self._append_various_line(line)
> + return
> +
> + self._append_freeform(line.strip())
> +
> + def _append_various_line(self, line):
> + name = line.split(' ', 1)[0]
> +
> + if name.startswith('@') and name.endswith(':'):
> + raise QAPIParseError(self._parser,
> + "'%s' can't follow '%s' section"
> + % (name, self.sections[0].name))
> + elif self._check_named_section(name):
> line = line[len(name)+1:]
> self._start_section(name[:-1])
>
> + if (not self._section.name or
> + not self._section.name.startswith('Example')):
> + line = line.strip()
> +
> self._append_freeform(line)
>
> def _start_args_section(self, name):
> @@ -194,10 +276,7 @@ class QAPIDoc(object):
> if name in self.args:
> raise QAPIParseError(self._parser,
> "'%s' parameter name duplicated" % name)
> - if self.sections:
> - raise QAPIParseError(self._parser,
> - "'@%s:' can't follow '%s' section"
> - % (name, self.sections[0].name))
> + assert not self.sections
> self._end_section()
> self._section = QAPIDoc.ArgSection(name)
> self.args[name] = self._section
> @@ -219,13 +298,6 @@ class QAPIDoc(object):
> self._section = None
>
> def _append_freeform(self, line):
> - in_arg = isinstance(self._section, QAPIDoc.ArgSection)
> - if (in_arg and self._section.text.endswith('\n\n')
> - and line and not line[0].isspace()):
> - self._start_section()
> - if (in_arg or not self._section.name
> - or not self._section.name.startswith('Example')):
> - line = line.strip()
> match = re.match(r'(@\S+:)', line)
> if match:
> raise QAPIParseError(self._parser,
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 55ccd216cf..b42dad9b36 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -200,15 +200,13 @@ class QAPIDoc(object):
def end_comment(self):
self._end_section()
- def _check_named_section(self, name):
- if name in ('Returns:', 'Since:',
- # those are often singular or plural
- 'Note:', 'Notes:',
- 'Example:', 'Examples:',
- 'TODO:'):
- self._part = QAPIDoc.DocPart.VARIOUS
- return True
- return False
+ @staticmethod
+ def _is_section_tag(name):
+ return name in ('Returns:', 'Since:',
+ # those are often singular or plural
+ 'Note:', 'Notes:',
+ 'Example:', 'Examples:',
+ 'TODO:')
def _append_body_line(self, line):
name = line.split(' ', 1)[0]
@@ -226,7 +224,8 @@ class QAPIDoc(object):
if name.startswith('@') and name.endswith(':'):
self._part = QAPIDoc.DocPart.ARGS
self._append_args_line(line)
- elif self.symbol and self._check_named_section(name):
+ elif self._is_section_tag(name):
+ self._part = QAPIDoc.DocPart.VARIOUS
self._append_various_line(line)
else:
self._append_freeform(line.strip())
@@ -240,7 +239,8 @@ class QAPIDoc(object):
if name.startswith('@') and name.endswith(':'):
line = line[len(name)+1:]
self._start_args_section(name[1:-1])
- elif self._check_named_section(name):
+ elif self._is_section_tag(name):
+ self._part = QAPIDoc.DocPart.VARIOUS
self._append_various_line(line)
return
elif (self._section.text.endswith('\n\n')
@@ -259,7 +259,7 @@ class QAPIDoc(object):
raise QAPIParseError(self._parser,
"'%s' can't follow '%s' section"
% (name, self.sections[0].name))
- elif self._check_named_section(name):
+ elif self._is_section_tag(name):
line = line[len(name)+1:]
self._start_section(name[:-1])
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v4 4.5/6] qapi: Replace QAPIDoc._part by ._append_line, and rework comments
2019-06-06 11:26 ` Markus Armbruster
@ 2019-06-06 12:01 ` Markus Armbruster
2019-06-06 13:21 ` Kevin Wolf
0 siblings, 1 reply; 11+ messages in thread
From: Markus Armbruster @ 2019-06-06 12:01 UTC (permalink / raw)
To: Kevin Wolf; +Cc: pkrempa, qemu-devel, qemu-block
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
This is on top of the fixup I appended to my review of v4. I'd squash
all three patches together.
The next patch needs to be updated for this.
Unsquashed branch at git://repo.or.cz/qemu/armbru.git branch
qapi-features.
Let me know what you think.
scripts/qapi/common.py | 109 ++++++++++++++++++++++++-----------------
1 file changed, 63 insertions(+), 46 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index b42dad9b36..004b68df5c 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -102,6 +102,22 @@ class QAPISemError(QAPIError):
class QAPIDoc(object):
+ """
+ A documentation comment block, either expression or free-form
+
+ Expression documentation blocks consist of
+
+ * a body section: one line naming the expression, followed by an
+ overview (any number of lines)
+
+ * argument sections: a description of each argument (for commands
+ and events) or member (for structs, unions and alternates)
+
+ * additional (non-argument) sections, possibly tagged
+
+ Free-form documentation blocks consist only of a body section.
+ """
+
class Section(object):
def __init__(self, name=None):
# optional section name (argument/member or section name)
@@ -120,22 +136,6 @@ class QAPIDoc(object):
def connect(self, member):
self.member = member
- class DocPart:
- """
- Expression documentation blocks consist of
- * a BODY part: first line naming the expression, plus an
- optional overview
- * an ARGS part: description of each argument (for commands and
- events) or member (for structs, unions and alternates),
- * a VARIOUS part: optional tagged sections.
-
- Free-form documentation blocks consist only of a BODY part.
- """
- # TODO Make it a subclass of Enum when Python 2 support is removed
- BODY = 1
- ARGS = 2
- VARIOUS = 3
-
def __init__(self, parser, info):
# self._parser is used to report errors with QAPIParseError. The
# resulting error position depends on the state of the parser.
@@ -151,7 +151,7 @@ class QAPIDoc(object):
self.sections = []
# the current section
self._section = self.body
- self._part = QAPIDoc.DocPart.BODY
+ self._append_line = self._append_body_line
def has_section(self, name):
"""Return True if we have a section with this name."""
@@ -164,20 +164,11 @@ class QAPIDoc(object):
"""
Parse a comment line and add it to the documentation.
- The way that the line is dealt with depends on which part of the
- documentation we're parsing right now:
-
- BODY means that we're ready to process free-form text into self.body. A
- symbol name is only allowed if no other text was parsed yet. It is
- interpreted as the symbol name that describes the currently documented
- object. On getting the second symbol name, we proceed to ARGS.
-
- ARGS means that we're parsing the arguments section. Any symbol name is
- interpreted as an argument and an ArgSection is created for it.
-
- VARIOUS is the final part where free-form sections may appear. This
- includes named sections such as "Return:" as well as unnamed
- paragraphs. Symbols are not allowed any more in this part.
+ The way that the line is dealt with depends on which part of
+ the documentation we're parsing right now:
+ * The body section: ._append_line is ._append_body_line
+ * An argument section: ._append_line is ._append_args_line
+ * An additional section: ._append_line is ._append_various_line
"""
line = line[1:]
if not line:
@@ -187,15 +178,7 @@ class QAPIDoc(object):
if line[0] != ' ':
raise QAPIParseError(self._parser, "Missing space after #")
line = line[1:]
-
- if self._part == QAPIDoc.DocPart.BODY:
- self._append_body_line(line)
- elif self._part == QAPIDoc.DocPart.ARGS:
- self._append_args_line(line)
- elif self._part == QAPIDoc.DocPart.VARIOUS:
- self._append_various_line(line)
- else:
- assert False
+ self._append_line(line)
def end_comment(self):
self._end_section()
@@ -209,6 +192,19 @@ class QAPIDoc(object):
'TODO:')
def _append_body_line(self, line):
+ """
+ Process a line of documentation text in the body section.
+
+ If this a symbol line and it is the section's first line, this
+ is an expression documentation block for that symbol.
+
+ If it's an expression documentation block, another symbol line
+ begins the argument section for the argument named by it, and
+ a section tag begins an additional section. Start that
+ section and append the line to it.
+
+ Else, append the line to the current section.
+ """
name = line.split(' ', 1)[0]
# FIXME not nice: things like '# @foo:' and '# @foo: ' aren't
# recognized, and get silently treated as ordinary text
@@ -220,39 +216,60 @@ class QAPIDoc(object):
if not self.symbol:
raise QAPIParseError(self._parser, "Invalid name")
elif self.symbol:
- # We already know that we document some symbol
+ # This is an expression documentation block
if name.startswith('@') and name.endswith(':'):
- self._part = QAPIDoc.DocPart.ARGS
+ self._append_line = self._append_args_line
self._append_args_line(line)
elif self._is_section_tag(name):
- self._part = QAPIDoc.DocPart.VARIOUS
+ self._append_line = self._append_various_line
self._append_various_line(line)
else:
self._append_freeform(line.strip())
else:
- # This is free-form documentation without a symbol
+ # This is a free-form documentation block
self._append_freeform(line.strip())
def _append_args_line(self, line):
+ """
+ Process a line of documentation text in an argument section.
+
+ A symbol line begins the next argument section, a section tag
+ section or a non-indented line after a blank line begins an
+ additional section. Start that section and append the line to
+ it.
+
+ Else, append the line to the current section.
+
+ """
name = line.split(' ', 1)[0]
if name.startswith('@') and name.endswith(':'):
line = line[len(name)+1:]
self._start_args_section(name[1:-1])
elif self._is_section_tag(name):
- self._part = QAPIDoc.DocPart.VARIOUS
+ self._append_line = self._append_various_line
self._append_various_line(line)
return
elif (self._section.text.endswith('\n\n')
and line and not line[0].isspace()):
self._start_section()
- self._part = QAPIDoc.DocPart.VARIOUS
+ self._append_line = self._append_various_line
self._append_various_line(line)
return
self._append_freeform(line.strip())
def _append_various_line(self, line):
+ """
+ Process a line of documentation text in an additional section.
+
+ A symbol line is an error.
+
+ A section tag begins an additional section. Start that
+ section and append the line to it.
+
+ Else, append the line to the current section.
+ """
name = line.split(' ', 1)[0]
if name.startswith('@') and name.endswith(':'):
--
2.21.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4 4.5/6] qapi: Replace QAPIDoc._part by ._append_line, and rework comments
2019-06-06 12:01 ` [Qemu-devel] [PATCH v4 4.5/6] qapi: Replace QAPIDoc._part by ._append_line, and rework comments Markus Armbruster
@ 2019-06-06 13:21 ` Kevin Wolf
0 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2019-06-06 13:21 UTC (permalink / raw)
To: Markus Armbruster; +Cc: pkrempa, qemu-devel, qemu-block
Am 06.06.2019 um 14:01 hat Markus Armbruster geschrieben:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> This is on top of the fixup I appended to my review of v4. I'd squash
> all three patches together.
>
> The next patch needs to be updated for this.
>
> Unsquashed branch at git://repo.or.cz/qemu/armbru.git branch
> qapi-features.
>
> Let me know what you think.
As you know, I don't like the self._append_line function pointer and
think it makes the code less readable. However, I also hope that I'll
never have to touch this code again, whereas you as the maintainer will
probably have to. So your taste is more imporant than mine.
So for all I care, go ahead and squash in your changes.
Kevin
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v4 0/6] file-posix: Add dynamic-auto-read-only QAPI feature
2019-05-30 11:02 [Qemu-devel] [PATCH v4 0/6] file-posix: Add dynamic-auto-read-only QAPI feature Kevin Wolf
` (5 preceding siblings ...)
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 6/6] file-posix: Add dynamic-auto-read-only QAPI feature Kevin Wolf
@ 2019-06-06 14:27 ` Markus Armbruster
6 siblings, 0 replies; 11+ messages in thread
From: Markus Armbruster @ 2019-06-06 14:27 UTC (permalink / raw)
To: Kevin Wolf; +Cc: pkrempa, qemu-devel, qemu-block
Kevin Wolf <kwolf@redhat.com> writes:
> This series adds optional feature lists to struct definitions in the
> QAPI schema and makes use of them to advertise the new behaviour of
> auto-read-only=on in file-posix.
PATCH 1-3,5-6 are ready. PATCH 4 could use a bit of love, but I think I
don't need you to respin. Let's review the issues briefly:
(a) I found a few comment nits to pick.
(b) QAPIDoc._check_named_section() confused me. I proposed to replace
it by ._is_section_tag().
(c) I proposed to replace QAPIDoc._part by ._append_line. Matter of
taste. I find it simpler.
I'd like to proceed as follows. Since my follow-up patch for (b) is
pretty trivial and you haven't expressed a dislike for it, I'll squash
it in. Since you do dislike my follow-up patch for (c), I'll keep it
separate, so you don't get blamed for it. Any comment nits that survive
the two follow-up patches I'll address in the first one. I'll post the
result as v5.
Okay?
Since PATCH 4 isn't actually wrong, series
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-06-06 14:29 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-30 11:02 [Qemu-devel] [PATCH v4 0/6] file-posix: Add dynamic-auto-read-only QAPI feature Kevin Wolf
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 1/6] qapi: Add feature flags to struct types Kevin Wolf
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 2/6] tests/qapi-schema: Test for good feature lists in structs Kevin Wolf
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 3/6] tests/qapi-schema: Error case tests for features " Kevin Wolf
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 4/6] qapi: Disentangle QAPIDoc code Kevin Wolf
2019-06-06 11:26 ` Markus Armbruster
2019-06-06 12:01 ` [Qemu-devel] [PATCH v4 4.5/6] qapi: Replace QAPIDoc._part by ._append_line, and rework comments Markus Armbruster
2019-06-06 13:21 ` Kevin Wolf
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 5/6] qapi: Allow documentation for features Kevin Wolf
2019-05-30 11:02 ` [Qemu-devel] [PATCH v4 6/6] file-posix: Add dynamic-auto-read-only QAPI feature Kevin Wolf
2019-06-06 14:27 ` [Qemu-devel] [PATCH v4 0/6] " Markus Armbruster
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.