Re: [Qemu-devel] [PATCH V12 0/5] add pvpanic mmio support

[peng.hao2]

>v10 --> v11
>change configure interface in virt machine configure parameters.
>
>v11 --> v12
>realize pvpanic as a pci device and use the mmio of pci device.
>
>Philippe Mathieu-Daudé (2):
>hw/misc/pvpanic: Build the pvpanic device in $(common-obj)
>hw/misc/pvpanic: Cosmetic renaming
>
>Peng Hao (3):
>pvpanic : update pvpanic document
>hw/arm/virt: Use the pvpanic device
>pvpanic: add mmio interface as a pci device
Hi , I resubmit a series patches for realizeing pvpanic as a pci device.
and I want to confirm if these patches meet the requirements. 
I know there are still some imperfections in this series of patches, 
such as how to avoid x86/pvpanic using ISA/pvpanic and PCI/pvpanic at the same 
time.
 But I want to confirm if there is a problem with the overall direction of 
implementation 
as a PCI device.Then I can resubmit patches to kernel for replacing the 
previous ones.

Thanks.

[Qemu-devel] [PATCH for-4.0 v7 01/27] qapi: make sure osdep.h is included in type headers

[Marc-André Lureau]

Now that the schema can be configured, it is crucial that all types
are configured the same. Make sure config-host.h is included, by
checking osdep.h inclusion. The build-sys tracks the dependency and
rebuilds the types if the configuration changed.

Signed-off-by: Marc-André Lureau 
---
 scripts/qapi/types.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index fd7808103c..3bb9cb6d47 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -201,6 +201,9 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
 ''',
   types=types, visit=visit))
 self._genh.preamble_add(mcgen('''
+#ifndef QEMU_OSDEP_H
+#error Please include osdep.h first!
+#endif
 #include "qapi/qapi-builtin-types.h"
 '''))
 
-- 
2.20.0.rc1

[Qemu-devel] [PATCH for-4.0 v7 00/27] Hi,

[Marc-André Lureau]

This is the second part of the "add #if pre-processor conditions to
generated code" series, adding schema member conditions (roughly
16-38/49).

Members can be exploded as dictionnary with 'type'/'if' keys:

{ 'struct': 'TestIfStruct', 'data':
  { 'foo': 'int',
'bar': { 'type': 'int', 'if': 'defined(TEST_IF_STRUCT_BAR)'} } }

Enum values can be exploded as dictionnary with 'type'/'if' keys:

{ 'enum': 'TestIfEnum', 'data':
  [ 'foo',
{ 'name' : 'bar', 'if': 'defined(TEST_IF_ENUM_BAR)' } ] }

v7:
- replaced osdep.h include by a #warning check
- renamed event_names/event_enum_members, enum_name/event_enum_ename
- move normalization after checking
- simplify make_enum_members()
- some tests changes
- rebase/conflicts tweaks
- squash documentation changes in related patches
- tweak generated documentation code & output
- modify commit comments, add rb tags

v6:
- subset of v5 series: add schema member conditions
- split "qapi: factor out check_known_keys()", error report improvements
- add a patch to "qapi: include osdep.h in type headers" to avoid
  potential build configuration change issues
- rebased, on top of Markus's qapi-next branch (first 4 patches,
  included for patchew testing)

Marc-André Lureau (27):
  qapi: make sure osdep.h is included in type headers
  qapi: do not define enumeration value explicitly
  qapi: rename QAPISchemaEnumType.values to .members
  qapi: change enum visitor and gen_enum* to take QAPISchemaMember
  tests: print enum type members more like object type members
  qapi: factor out checking for keys
  qapi: improve reporting of unknown or missing keys
  qapi: add a dictionary form with 'name' key for enum members
  qapi: add 'if' to enum members
  qapi-events: add 'if' condition to implicit event enum
  qapi: pass long form enum to make_enum_members
  qapi: rename allow_dict to allow_implicit
  qapi: add a dictionary form for TYPE
  qapi: add 'if' to implicit struct members
  qapi: add an error in case a discriminator is conditional
  qapi: add 'if' to union members
  qapi: simplify make_enum_members()
  tests/qapi: add command with condition on union argument
  qapi: add 'if' to alternate members
  tests/qapi: add command with condition on alternate argument
  qapi: add #if conditions to generated code members
  qapi: add 'If:' condition to enum values documentation
  qapi: add 'If:' condition to struct members documentation
  qapi: add condition to variants documentation
  qapi: break long lines at 'data' member
  qapi: add more conditions to SPICE
  qapi: add conditions to REPLICATION type/commands on the schema

 qapi/block-core.json  |  26 +-
 qapi/char.json| 150 ++-
 qapi/migration.json   |  15 +-
 qapi/misc.json|   7 +-
 qapi/net.json |   3 +-
 qapi/tpm.json |   5 +-
 qapi/ui.json  |   3 +-
 scripts/qapi/common.py| 253 --
 scripts/qapi/doc.py   |  30 ++-
 scripts/qapi/events.py|  13 +-
 scripts/qapi/introspect.py|  16 +-
 scripts/qapi/types.py |  13 +-
 scripts/qapi/visit.py |   8 +-
 migration/colo.c  |  16 +-
 monitor.c |   5 -
 docs/devel/qapi-code-gen.txt  |  19 ++
 tests/Makefile.include|  11 +-
 tests/qapi-schema/alternate-base.err  |   1 +
 tests/qapi-schema/alternate-invalid-dict.err  |   1 +
 ...ember.exit => alternate-invalid-dict.exit} |   0
 tests/qapi-schema/alternate-invalid-dict.json |   4 +
 ...-member.out => alternate-invalid-dict.out} |   0
 tests/qapi-schema/comments.out|  14 +-
 tests/qapi-schema/doc-bad-section.out |  13 +-
 tests/qapi-schema/doc-good.json   |  11 +-
 tests/qapi-schema/doc-good.out|  22 +-
 tests/qapi-schema/doc-good.texi   |   7 +-
 tests/qapi-schema/double-type.err |   1 +
 tests/qapi-schema/empty.out   |   9 +-
 tests/qapi-schema/enum-bad-member.err |   1 +
 tests/qapi-schema/enum-bad-member.exit|   1 +
 tests/qapi-schema/enum-bad-member.json|   2 +
 tests/qapi-schema/enum-bad-member.out |   0
 .../qapi-schema/enum-dict-member-unknown.err  |   2 +
 .../qapi-schema/enum-dict-member-unknown.exit |   1 +
 .../qapi-schema/enum-dict-member-unknown.json |   2 +
 .../qapi-schema/enum-dict-member-unknown.out  |   0
 tests/qapi-schema/enum-dict-member.err|   1 -
 tests/qapi-schema/enum-dict-member.json   |   2 -
 tests/qapi-schema/enum-if-invalid.err |   1 +
 tests/qapi-schema/enum-if-invalid.exit|   1 +
 tests/qapi-schema/enum-if-invalid.json|   3 +
 tests/qapi-schema/enum-if-invalid.out |  

[Qemu-devel] [PATCH for-4.0 v7 06/27] qapi: factor out checking for keys

[Marc-André Lureau]

Introduce a new helper function to check if the given keys are known,
and if mandatory keys are present. The function will be reused in
other places in the following code changes.

Signed-off-by: Marc-André Lureau 
---
 scripts/qapi/common.py | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 1fa2f79990..18f5872808 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -873,6 +873,17 @@ def check_struct(expr, info):
allow_metas=['struct'])
 
 
+def check_known_keys(info, source, keys, required, optional):
+for key in keys:
+if key not in required and key not in optional:
+raise QAPISemError(info, "Unknown key '%s' in %s" % (key, source))
+
+for key in required:
+if key not in keys:
+raise QAPISemError(info, "Key '%s' is missing from %s"
+   % (key, source))
+
+
 def check_keys(expr_elem, meta, required, optional=[]):
 expr = expr_elem['expr']
 info = expr_elem['info']
@@ -880,10 +891,9 @@ def check_keys(expr_elem, meta, required, optional=[]):
 if not isinstance(name, str):
 raise QAPISemError(info, "'%s' key must have a string value" % meta)
 required = required + [meta]
+source = "%s '%s'" % (meta, name)
+check_known_keys(info, source, expr.keys(), required, optional)
 for (key, value) in expr.items():
-if key not in required and key not in optional:
-raise QAPISemError(info, "Unknown key '%s' in %s '%s'"
-   % (key, meta, name))
 if key in ['gen', 'success-response'] and value is not False:
 raise QAPISemError(info,
"'%s' of %s '%s' should only use false value"
@@ -895,10 +905,6 @@ def check_keys(expr_elem, meta, required, optional=[]):
% (key, meta, name))
 if key == 'if':
 check_if(expr, info)
-for key in required:
-if key not in expr:
-raise QAPISemError(info, "Key '%s' is missing from %s '%s'"
-   % (key, meta, name))
 
 
 def check_exprs(exprs):
-- 
2.20.0.rc1

[Qemu-devel] [PATCH for-4.0 v7 02/27] qapi: do not define enumeration value explicitly

[Marc-André Lureau]

The C standard has the initial value at 0 and the subsequent values
incremented by 1. No need to set this explicitely.

This will prevent from artificial "gaps" when compiling out some enum
values and having unnecessarily large MAX values & enums arrays, or
simplifying iterating over valid enum values.

Whenever config-host.h is changed, all the enum/types are recompiled.

Signed-off-by: Marc-André Lureau 
---
 scripts/qapi/common.py | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 7b62a4c7b0..7ea0cf5139 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -2045,14 +2045,11 @@ typedef enum %(c_name)s {
 ''',
 c_name=c_name(name))
 
-i = 0
 for value in enum_values:
 ret += mcgen('''
-%(c_enum)s = %(i)d,
+%(c_enum)s,
 ''',
- c_enum=c_enum_const(name, value, prefix),
- i=i)
-i += 1
+ c_enum=c_enum_const(name, value, prefix))
 
 ret += mcgen('''
 } %(c_name)s;
-- 
2.20.0.rc1

[Qemu-devel] [PATCH for-4.0 v7 04/27] qapi: change enum visitor and gen_enum* to take QAPISchemaMember

[Marc-André Lureau]

This will allow to add and access more properties associated with enum
values/members, like the associated 'if' condition. We may want to
have a specialized type QAPISchemaEnumMember, for now this will do.

Modify gen_enum() and gen_enum_lookup() for the same reason.

Suggested-by: Markus Armbruster 
Signed-off-by: Marc-André Lureau 
Reviewed-by: Markus Armbruster 
---
 scripts/qapi/common.py | 22 +++---
 scripts/qapi/doc.py|  2 +-
 scripts/qapi/events.py | 13 +++--
 scripts/qapi/introspect.py |  5 +++--
 scripts/qapi/types.py  |  6 +++---
 scripts/qapi/visit.py  |  2 +-
 tests/qapi-schema/test-qapi.py |  4 ++--
 7 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 55c914ec44..1fa2f79990 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1063,7 +1063,7 @@ class QAPISchemaVisitor(object):
 def visit_builtin_type(self, name, info, json_type):
 pass
 
-def visit_enum_type(self, name, info, ifcond, values, prefix):
+def visit_enum_type(self, name, info, ifcond, members, prefix):
 pass
 
 def visit_array_type(self, name, info, ifcond, element_type):
@@ -1193,7 +1193,7 @@ class QAPISchemaEnumType(QAPISchemaType):
 
 def visit(self, visitor):
 visitor.visit_enum_type(self.name, self.info, self.ifcond,
-self.member_names(), self.prefix)
+self.members, self.prefix)
 
 
 class QAPISchemaArrayType(QAPISchemaType):
@@ -2012,19 +2012,19 @@ def _wrap_ifcond(ifcond, before, after):
 return out
 
 
-def gen_enum_lookup(name, values, prefix=None):
+def gen_enum_lookup(name, members, prefix=None):
 ret = mcgen('''
 
 const QEnumLookup %(c_name)s_lookup = {
 .array = (const char *const[]) {
 ''',
 c_name=c_name(name))
-for value in values:
-index = c_enum_const(name, value, prefix)
+for m in members:
+index = c_enum_const(name, m.name, prefix)
 ret += mcgen('''
-[%(index)s] = "%(value)s",
+[%(index)s] = "%(name)s",
 ''',
- index=index, value=value)
+ index=index, name=m.name)
 
 ret += mcgen('''
 },
@@ -2035,9 +2035,9 @@ const QEnumLookup %(c_name)s_lookup = {
 return ret
 
 
-def gen_enum(name, values, prefix=None):
+def gen_enum(name, members, prefix=None):
 # append automatically generated _MAX value
-enum_values = values + ['_MAX']
+enum_members = members + [QAPISchemaMember('_MAX')]
 
 ret = mcgen('''
 
@@ -2045,11 +2045,11 @@ typedef enum %(c_name)s {
 ''',
 c_name=c_name(name))
 
-for value in enum_values:
+for m in enum_members:
 ret += mcgen('''
 %(c_enum)s,
 ''',
- c_enum=c_enum_const(name, value, prefix))
+ c_enum=c_enum_const(name, m.name, prefix))
 
 ret += mcgen('''
 } %(c_name)s;
diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 987fd3c943..76cb186ff9 100755
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -206,7 +206,7 @@ class 
QAPISchemaGenDocVisitor(qapi.common.QAPISchemaVisitor):
 def write(self, output_dir):
 self._gen.write(output_dir, self._prefix + 'qapi-doc.texi')
 
-def visit_enum_type(self, name, info, ifcond, values, prefix):
+def visit_enum_type(self, name, info, ifcond, members, prefix):
 doc = self.cur_doc
 self._gen.add(TYPE_FMT(type='Enum',
name=doc.symbol,
diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index 2ed7902424..f1b88d8786 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -143,8 +143,8 @@ class QAPISchemaGenEventVisitor(QAPISchemaModularCVisitor):
 QAPISchemaModularCVisitor.__init__(
 self, prefix, 'qapi-events',
 ' * Schema-defined QAPI/QMP events', __doc__)
-self._enum_name = c_name(prefix + 'QAPIEvent', protect=False)
-self._event_names = []
+self._event_enum_name = c_name(prefix + 'QAPIEvent', protect=False)
+self._event_enum_members = []
 
 def _begin_module(self, name):
 types = self._module_basename('qapi-types', name)
@@ -170,15 +170,16 @@ class 
QAPISchemaGenEventVisitor(QAPISchemaModularCVisitor):
 
 def visit_end(self):
 (genc, genh) = self._module[self._main_module]
-genh.add(gen_enum(self._enum_name, self._event_names))
-genc.add(gen_enum_lookup(self._enum_name, self._event_names))
+genh.add(gen_enum(self._event_enum_name, self._event_enum_members))
+genc.add(gen_enum_lookup(self._event_enum_name,
+ self._event_enum_members))
 
 def visit_event(self, name, info, ifcond, arg_type, boxed):
 with ifcontext(ifcond, self._genh, self._genc):
 self._genh.add(gen_event_send_decl(name, arg_type, boxed))
  

[Qemu-devel] [PATCH for-4.0 v7 03/27] qapi: rename QAPISchemaEnumType.values to .members

[Marc-André Lureau]

Rename QAPISchemaEnumType.values and related variables to members.
Makes sense ever since commit 93bda4dd4 changed .values from list of
string to list of QAPISchemaMember. Obvious no-op.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Markus Armbruster 
---
 scripts/qapi/common.py | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 7ea0cf5139..55c914ec44 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1161,22 +1161,22 @@ class QAPISchemaBuiltinType(QAPISchemaType):
 
 
 class QAPISchemaEnumType(QAPISchemaType):
-def __init__(self, name, info, doc, ifcond, values, prefix):
+def __init__(self, name, info, doc, ifcond, members, prefix):
 QAPISchemaType.__init__(self, name, info, doc, ifcond)
-for v in values:
-assert isinstance(v, QAPISchemaMember)
-v.set_owner(name)
+for m in members:
+assert isinstance(m, QAPISchemaMember)
+m.set_owner(name)
 assert prefix is None or isinstance(prefix, str)
-self.values = values
+self.members = members
 self.prefix = prefix
 
 def check(self, schema):
 QAPISchemaType.check(self, schema)
 seen = {}
-for v in self.values:
-v.check_clash(self.info, seen)
+for m in self.members:
+m.check_clash(self.info, seen)
 if self.doc:
-self.doc.connect_member(v)
+self.doc.connect_member(m)
 
 def is_implicit(self):
 # See QAPISchema._make_implicit_enum_type() and ._def_predefineds()
@@ -1186,7 +1186,7 @@ class QAPISchemaEnumType(QAPISchemaType):
 return c_name(self.name)
 
 def member_names(self):
-return [v.name for v in self.values]
+return [m.name for m in self.members]
 
 def json_type(self):
 return 'string'
@@ -1403,9 +1403,9 @@ class QAPISchemaObjectTypeVariants(object):
 if self._tag_name:# flat union
 # branches that are not explicitly covered get an empty type
 cases = set([v.name for v in self.variants])
-for val in self.tag_member.type.values:
-if val.name not in cases:
-v = QAPISchemaObjectTypeVariant(val.name, 'q_empty')
+for m in self.tag_member.type.members:
+if m.name not in cases:
+v = QAPISchemaObjectTypeVariant(m.name, 'q_empty')
 v.set_owner(self.tag_member.owner)
 self.variants.append(v)
 for v in self.variants:
-- 
2.20.0.rc1

[Qemu-devel] [PATCH for-4.0 v7 08/27] qapi: add a dictionary form with 'name' key for enum members

[Marc-André Lureau]

Desugar the enum NAME form to { 'name': NAME }. This will allow to add
new enum members, such as 'if' in the following patch.

Signed-off-by: Marc-André Lureau 
---
 scripts/qapi/common.py| 49 ---
 tests/Makefile.include|  3 +-
 tests/qapi-schema/enum-bad-member.err |  1 +
 ...-dict-member.exit => enum-bad-member.exit} |  0
 tests/qapi-schema/enum-bad-member.json|  2 +
 ...um-dict-member.out => enum-bad-member.out} |  0
 .../qapi-schema/enum-dict-member-unknown.err  |  2 +
 .../qapi-schema/enum-dict-member-unknown.exit |  1 +
 .../qapi-schema/enum-dict-member-unknown.json |  2 +
 .../qapi-schema/enum-dict-member-unknown.out  |  0
 tests/qapi-schema/enum-dict-member.err|  1 -
 tests/qapi-schema/enum-dict-member.json   |  2 -
 12 files changed, 52 insertions(+), 11 deletions(-)
 create mode 100644 tests/qapi-schema/enum-bad-member.err
 rename tests/qapi-schema/{enum-dict-member.exit => enum-bad-member.exit} (100%)
 create mode 100644 tests/qapi-schema/enum-bad-member.json
 rename tests/qapi-schema/{enum-dict-member.out => enum-bad-member.out} (100%)
 create mode 100644 tests/qapi-schema/enum-dict-member-unknown.err
 create mode 100644 tests/qapi-schema/enum-dict-member-unknown.exit
 create mode 100644 tests/qapi-schema/enum-dict-member-unknown.json
 create mode 100644 tests/qapi-schema/enum-dict-member-unknown.out
 delete mode 100644 tests/qapi-schema/enum-dict-member.err
 delete mode 100644 tests/qapi-schema/enum-dict-member.json

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index f205805751..610840d2e5 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -740,6 +740,10 @@ def check_event(expr, info):
allow_metas=meta)
 
 
+def enum_get_names(expr):
+return [e['name'] if isinstance(e, dict) else e for e in expr['data']]
+
+
 def check_union(expr, info):
 name = expr['union']
 base = expr.get('base')
@@ -799,7 +803,7 @@ def check_union(expr, info):
 # If the discriminator names an enum type, then all members
 # of 'data' must also be members of the enum type.
 if enum_define:
-if key not in enum_define['data']:
+if key not in enum_get_names(enum_define):
 raise QAPISemError(info,
"Discriminator value '%s' is not found in "
"enum '%s'"
@@ -831,7 +835,7 @@ def check_alternate(expr, info):
 if qtype == 'QTYPE_QSTRING':
 enum_expr = enum_types.get(value)
 if enum_expr:
-for v in enum_expr['data']:
+for v in enum_get_names(enum_expr):
 if v in ['on', 'off']:
 conflicting.add('QTYPE_QBOOL')
 if re.match(r'[-+0-9.]', v): # lazy, could be tightened
@@ -847,18 +851,32 @@ def check_alternate(expr, info):
 types_seen[qt] = key
 
 
+def normalize_enum(expr):
+members = expr['data']
+
+# translate short member form to dict form
+expr['data'] = [m if isinstance(m, dict) else {'name': m} for m in members]
+
+
 def check_enum(expr, info):
 name = expr['enum']
-members = expr.get('data')
+members = expr['data']
 prefix = expr.get('prefix')
 
-if not isinstance(members, list):
-raise QAPISemError(info,
-   "Enum '%s' requires an array for 'data'" % name)
 if prefix is not None and not isinstance(prefix, str):
 raise QAPISemError(info,
"Enum '%s' requires a string for 'prefix'" % name)
+
+if not isinstance(members, list):
+raise QAPISemError(info,
+   "Enum '%s' requires an array for 'data'" % name)
+
 for member in members:
+if isinstance(member, dict):
+source = "dictionary member of enum '%s'" % name
+check_known_keys(info, source, member, ['name'], [])
+member = member['name']
+
 check_name(info, "Member of enum '%s'" % name, member,
enum_member=True)
 
@@ -1011,6 +1029,15 @@ def check_exprs(exprs):
 return exprs
 
 
+def normalize_exprs(exprs):
+for expr_elem in exprs:
+expr = expr_elem['expr']
+if 'enum' in expr:
+normalize_enum(expr)
+
+return exprs
+
+
 #
 # Schema compiler frontend
 #
@@ -1567,6 +1594,7 @@ class QAPISchema(object):
 f = open(fname, 'r')
 parser = QAPISchemaParser(f)
 exprs = check_exprs(parser.exprs)
+exprs = normalize_exprs(exprs)
 self.docs = parser.docs
 self._entity_list = []
 self._entity_dict = {}
@@ -1640,7 +1668,14 @@ class QAPISchema(object):
 qtype_values, 'QTYPE'))
 
 def _make_enum_members(self, values):
-return [QAPISchemaMember(v) for v in values]
+enum = []
+for v in values:
+   

[Qemu-devel] [PATCH for-4.0 v7 10/27] qapi-events: add 'if' condition to implicit event enum

[Marc-André Lureau]

Add condition to QAPIEvent enum members based on the event 'if'.

The generated code remains unconditional for now. Later patches
generate the conditionals (also there is no additional coverage of
this change in qapi-schema-test.out since the event_names enum is an
implicit type created by qapi/events.py).

Signed-off-by: Marc-André Lureau 
Reviewed-by: Markus Armbruster 
---
 scripts/qapi/events.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index f1b88d8786..37ee5de682 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -179,7 +179,7 @@ class QAPISchemaGenEventVisitor(QAPISchemaModularCVisitor):
 self._genh.add(gen_event_send_decl(name, arg_type, boxed))
 self._genc.add(gen_event_send(name, arg_type, boxed,
   self._event_enum_name))
-self._event_enum_members.append(QAPISchemaMember(name))
+self._event_enum_members.append(QAPISchemaMember(name, ifcond))
 
 
 def gen_events(schema, output_dir, prefix):
-- 
2.20.0.rc1

[Qemu-devel] [PATCH for-4.0 v7 05/27] tests: print enum type members more like object type members

[Marc-André Lureau]

Commit 93bda4dd461 changed the internal representation of enum type
members from str to QAPISchemaMember, but we still print only a
string.  Has been good enough, as the name is the member's only
attribute of interest, but that's about to change.  To prepare, print
them more like object type members.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Markus Armbruster 
---
 tests/qapi-schema/comments.out   | 14 ++-
 tests/qapi-schema/doc-bad-section.out| 13 +-
 tests/qapi-schema/doc-good.out   | 17 ++--
 tests/qapi-schema/empty.out  |  9 -
 tests/qapi-schema/event-case.out |  9 -
 tests/qapi-schema/ident-with-escape.out  |  9 -
 tests/qapi-schema/include-relpath.out| 14 ++-
 tests/qapi-schema/include-repetition.out | 14 ++-
 tests/qapi-schema/include-simple.out | 14 ++-
 tests/qapi-schema/indented-expr.out  |  9 -
 tests/qapi-schema/qapi-schema-test.out   | 50 +++-
 tests/qapi-schema/test-qapi.py   |  4 +-
 12 files changed, 149 insertions(+), 27 deletions(-)

diff --git a/tests/qapi-schema/comments.out b/tests/qapi-schema/comments.out
index 8d2f1ce8a2..d1abc4b5a1 100644
--- a/tests/qapi-schema/comments.out
+++ b/tests/qapi-schema/comments.out
@@ -1,5 +1,15 @@
 object q_empty
-enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
+enum QType
 prefix QTYPE
+member none
+member qnull
+member qnum
+member qstring
+member qdict
+member qlist
+member qbool
 module comments.json
-enum Status ['good', 'bad', 'ugly']
+enum Status
+member good
+member bad
+member ugly
diff --git a/tests/qapi-schema/doc-bad-section.out 
b/tests/qapi-schema/doc-bad-section.out
index cd28721568..db8014eed0 100644
--- a/tests/qapi-schema/doc-bad-section.out
+++ b/tests/qapi-schema/doc-bad-section.out
@@ -1,8 +1,17 @@
 object q_empty
-enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
+enum QType
 prefix QTYPE
+member none
+member qnull
+member qnum
+member qstring
+member qdict
+member qlist
+member qbool
 module doc-bad-section.json
-enum Enum ['one', 'two']
+enum Enum
+member one
+member two
 doc symbol=Enum
 body=
 == Produces *invalid* texinfo
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 35f3f1164c..c2fc5c774a 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -1,8 +1,17 @@
 object q_empty
-enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
+enum QType
 prefix QTYPE
+member none
+member qnull
+member qnum
+member qstring
+member qdict
+member qlist
+member qbool
 module doc-good.json
-enum Enum ['one', 'two']
+enum Enum
+member one
+member two
 if ['defined(IFCOND)']
 object Base
 member base1: Enum optional=False
@@ -18,7 +27,9 @@ object q_obj_Variant1-wrapper
 member data: Variant1 optional=False
 object q_obj_Variant2-wrapper
 member data: Variant2 optional=False
-enum SugaredUnionKind ['one', 'two']
+enum SugaredUnionKind
+member one
+member two
 object SugaredUnion
 member type: SugaredUnionKind optional=False
 tag type
diff --git a/tests/qapi-schema/empty.out b/tests/qapi-schema/empty.out
index 0ec234eec4..5483cb7bc6 100644
--- a/tests/qapi-schema/empty.out
+++ b/tests/qapi-schema/empty.out
@@ -1,3 +1,10 @@
 object q_empty
-enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
+enum QType
 prefix QTYPE
+member none
+member qnull
+member qnum
+member qstring
+member qdict
+member qlist
+member qbool
diff --git a/tests/qapi-schema/event-case.out b/tests/qapi-schema/event-case.out
index 88c0964917..f69d4ffe4e 100644
--- a/tests/qapi-schema/event-case.out
+++ b/tests/qapi-schema/event-case.out
@@ -1,6 +1,13 @@
 object q_empty
-enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
+enum QType
 prefix QTYPE
+member none
+member qnull
+member qnum
+member qstring
+member qdict
+member qlist
+member qbool
 module event-case.json
 event oops None
boxed=False
diff --git a/tests/qapi-schema/ident-with-escape.out 
b/tests/qapi-schema/ident-with-escape.out
index 24c976f473..7f891f7e90 100644
--- a/tests/qapi-schema/ident-with-escape.out
+++ b/tests/qapi-schema/ident-with-escape.out
@@ -1,6 +1,13 @@
 object q_empty
-enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
+enum QType
 prefix QTYPE
+member none
+member qnull
+member qnum
+member qstring
+member qdict
+member qlist
+member qbool
 module ident-with-escape.json
 object q_obj_fooA-arg
 member bar1: str optional=False
diff --git a/tests/qapi-schema/include-relpath.out 
b/tests/qapi-schema/include-relpath.out
index ebbabd7a18..783ccfc855 100644
--- a/tests/qapi-schema/include-relpath.out
+++ b/tests/qapi-schem

[Qemu-devel] [PATCH for-4.0 v7 11/27] qapi: pass long form enum to make_enum_members

[Marc-André Lureau]

This will allow to get rid of short form handling in a following
patch.

Signed-off-by: Marc-André Lureau 
Suggested-by: Markus Armbruster 
---
 scripts/qapi/common.py | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index fdc0fd69ef..557b413950 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1663,9 +1663,10 @@ class QAPISchema(object):
 self.the_empty_object_type = QAPISchemaObjectType(
 'q_empty', None, None, None, None, [], None)
 self._def_entity(self.the_empty_object_type)
-qtype_values = self._make_enum_members(['none', 'qnull', 'qnum',
-'qstring', 'qdict', 'qlist',
-'qbool'])
+
+qtypes = ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 
'qbool']
+qtype_values = self._make_enum_members([{'name': n} for n in qtypes])
+
 self._def_entity(QAPISchemaEnumType('QType', None, None, None,
 qtype_values, 'QTYPE'))
 
-- 
2.20.0.rc1

[Qemu-devel] [PATCH for-4.0 v7 09/27] qapi: add 'if' to enum members

[Marc-André Lureau]

QAPISchemaMember gains .ifcond for enum members: inherited classes,
such as QAPISchemaObjectTypeMember, will thus have an ifcond member
after this (those different types will also use the .ifcond to store
the condition and generate conditional code in the following patches).

The generated code remains unconditional for now. Later patches
generate the conditionals.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Markus Armbruster 
---
 scripts/qapi/common.py | 10 +++---
 docs/devel/qapi-code-gen.txt   |  9 +
 tests/Makefile.include |  1 +
 tests/qapi-schema/enum-dict-member-unknown.err |  2 +-
 tests/qapi-schema/enum-if-invalid.err  |  1 +
 tests/qapi-schema/enum-if-invalid.exit |  1 +
 tests/qapi-schema/enum-if-invalid.json |  3 +++
 tests/qapi-schema/enum-if-invalid.out  |  0
 tests/qapi-schema/qapi-schema-test.json|  5 +++--
 tests/qapi-schema/qapi-schema-test.out |  2 ++
 tests/qapi-schema/test-qapi.py |  1 +
 11 files changed, 29 insertions(+), 6 deletions(-)
 create mode 100644 tests/qapi-schema/enum-if-invalid.err
 create mode 100644 tests/qapi-schema/enum-if-invalid.exit
 create mode 100644 tests/qapi-schema/enum-if-invalid.json
 create mode 100644 tests/qapi-schema/enum-if-invalid.out

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 610840d2e5..fdc0fd69ef 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -874,7 +874,8 @@ def check_enum(expr, info):
 for member in members:
 if isinstance(member, dict):
 source = "dictionary member of enum '%s'" % name
-check_known_keys(info, source, member, ['name'], [])
+check_known_keys(info, source, member, ['name'], ['if'])
+check_if(member, info)
 member = member['name']
 
 check_name(info, "Member of enum '%s'" % name, member,
@@ -1358,9 +1359,10 @@ class QAPISchemaObjectType(QAPISchemaType):
 class QAPISchemaMember(object):
 role = 'member'
 
-def __init__(self, name):
+def __init__(self, name, ifcond=None):
 assert isinstance(name, str)
 self.name = name
+self.ifcond = listify_cond(ifcond)
 self.owner = None
 
 def set_owner(self, name):
@@ -1672,9 +1674,11 @@ class QAPISchema(object):
 for v in values:
 if isinstance(v, dict):
 name = v['name']
+ifcond = v.get('if')
 else:
 name = v
-enum.append(QAPISchemaMember(name))
+ifcond = None
+enum.append(QAPISchemaMember(name, ifcond))
 return enum
 
 def _make_implicit_enum_type(self, name, info, ifcond, values):
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 53eaf01f34..08c5ef97ff 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -752,6 +752,15 @@ gets its generated code guarded like this:
  #endif /* defined(HAVE_BAR) */
  #endif /* defined(CONFIG_FOO) */
 
+An enum value can be replaced by a dictionary with a 'name' and a 'if'
+key.
+
+Example: a conditional 'bar' enum member.
+
+{ 'enum': 'IfEnum', 'data':
+  [ 'foo',
+{ 'name' : 'bar', '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/tests/Makefile.include b/tests/Makefile.include
index 2e894c1037..3c9eea27fd 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -384,6 +384,7 @@ qapi-schema += enum-bad-name.json
 qapi-schema += enum-bad-prefix.json
 qapi-schema += enum-clash-member.json
 qapi-schema += enum-dict-member-unknown.json
+qapi-schema += enum-if-invalid.json
 qapi-schema += enum-int-member.json
 qapi-schema += enum-member-case.json
 qapi-schema += enum-missing-data.json
diff --git a/tests/qapi-schema/enum-dict-member-unknown.err 
b/tests/qapi-schema/enum-dict-member-unknown.err
index 76bd0471db..2aae618be0 100644
--- a/tests/qapi-schema/enum-dict-member-unknown.err
+++ b/tests/qapi-schema/enum-dict-member-unknown.err
@@ -1,2 +1,2 @@
 tests/qapi-schema/enum-dict-member-unknown.json:2: Unknown key 'bad-key' in 
dictionary member of enum 'MyEnum'
-Valid keys are 'name'.
+Valid keys are 'if', 'name'.
diff --git a/tests/qapi-schema/enum-if-invalid.err 
b/tests/qapi-schema/enum-if-invalid.err
new file mode 100644
index 00..54c3cf887b
--- /dev/null
+++ b/tests/qapi-schema/enum-if-invalid.err
@@ -0,0 +1 @@
+tests/qapi-schema/enum-if-invalid.json:2: 'if' condition must be a string or a 
list of strings
diff --git a/tests/qapi-schema/enum-if-invalid.exit 
b/tests/qapi-schema/enum-if-invalid.exit
new file mode 100644
index 00..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/enum-if-invalid.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/enum-if-invalid.js

[Qemu-devel] [PATCH for-4.0 v7 14/27] qapi: add 'if' to implicit struct members

[Marc-André Lureau]

The generated code is for now *unconditional*. Later patches generate
the conditionals.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Markus Armbruster 
---
 scripts/qapi/common.py  | 14 +++---
 docs/devel/qapi-code-gen.txt| 10 ++
 tests/qapi-schema/qapi-schema-test.json | 12 +---
 tests/qapi-schema/qapi-schema-test.out  |  5 +
 tests/qapi-schema/test-qapi.py  |  1 +
 5 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 4b3ba53dc7..aec51acb07 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -717,7 +717,7 @@ def check_type(info, source, value, allow_array=False,
 # an optional argument.
 if isinstance(arg, dict):
 check_known_keys(info, "member '%s' of %s" % (key, source),
- arg, ['type'], [])
+ arg, ['type'], ['if'])
 typ = arg['type']
 else:
 typ = arg
@@ -1438,8 +1438,8 @@ class QAPISchemaMember(object):
 
 
 class QAPISchemaObjectTypeMember(QAPISchemaMember):
-def __init__(self, name, typ, optional):
-QAPISchemaMember.__init__(self, name)
+def __init__(self, name, typ, optional, ifcond=None):
+QAPISchemaMember.__init__(self, name, ifcond)
 assert isinstance(typ, str)
 assert isinstance(optional, bool)
 self._type_name = typ
@@ -1762,7 +1762,7 @@ class QAPISchema(object):
 name, info, doc, ifcond,
 self._make_enum_members(data), prefix))
 
-def _make_member(self, name, typ, info):
+def _make_member(self, name, typ, ifcond, info):
 optional = False
 if name.startswith('*'):
 name = name[1:]
@@ -1770,10 +1770,10 @@ class QAPISchema(object):
 if isinstance(typ, list):
 assert len(typ) == 1
 typ = self._make_array_type(typ[0], info)
-return QAPISchemaObjectTypeMember(name, typ, optional)
+return QAPISchemaObjectTypeMember(name, typ, optional, ifcond)
 
 def _make_members(self, data, info):
-return [self._make_member(key, value['type'], info)
+return [self._make_member(key, value['type'], value.get('if'), info)
 for (key, value) in data.items()]
 
 def _def_struct_type(self, expr, info, doc):
@@ -1794,7 +1794,7 @@ class QAPISchema(object):
 typ = self._make_array_type(typ[0], info)
 typ = self._make_implicit_object_type(
 typ, info, None, self.lookup_type(typ),
-'wrapper', [self._make_member('data', typ, info)])
+'wrapper', [self._make_member('data', typ, None, info)])
 return QAPISchemaObjectTypeVariant(case, typ)
 
 def _def_union_type(self, expr, info, doc):
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 08c5ef97ff..e6fff32241 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -752,6 +752,16 @@ gets its generated code guarded like this:
  #endif /* defined(HAVE_BAR) */
  #endif /* defined(CONFIG_FOO) */
 
+Where a member can be defined with a single string value for its type,
+it is also possible to supply a dictionary instead with both 'type'
+and 'if' keys. (TODO: union and alternate)
+
+Example: a conditional 'bar' member
+
+{ 'struct': 'IfStruct', 'data':
+  { 'foo': 'int',
+'bar': { 'type': 'int', 'if': 'defined(IFCOND)'} } }
+
 An enum value can be replaced by a dictionary with a 'name' and a 'if'
 key.
 
diff --git a/tests/qapi-schema/qapi-schema-test.json 
b/tests/qapi-schema/qapi-schema-test.json
index 94570154c9..b8565a3913 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -201,7 +201,9 @@
 
 # test 'if' condition handling
 
-{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
+{ 'struct': 'TestIfStruct', 'data':
+  { 'foo': 'int',
+'bar': { 'type': 'int', 'if': 'defined(TEST_IF_STRUCT_BAR)'} },
   'if': 'defined(TEST_IF_STRUCT)' }
 
 { 'enum': 'TestIfEnum', 'data':
@@ -214,11 +216,15 @@
 { 'alternate': 'TestIfAlternate', 'data': { 'foo': 'int', 'bar': 'TestStruct' 
},
   'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' }
 
-{ 'command': 'TestIfCmd', 'data': { 'foo': 'TestIfStruct', 'bar': 'TestIfEnum' 
},
+{ 'command': 'TestIfCmd', 'data':
+  { 'foo': 'TestIfStruct',
+'bar': { 'type': 'TestIfEnum', 'if': 'defined(TEST_IF_CMD_BAR)' } },
   'returns': 'UserDefThree',
   'if': ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] }
 
 { 'command': 'TestCmdReturnDefThree', 'returns': 'UserDefThree' }
 
-{ 'event': 'TestIfEvent', 'data': { 'foo': 'TestIfStruct' },
+{ 'event': 'TestIfEvent', 'data':
+  { 'foo': 'TestIfStruct',
+'bar': { 'type': 'TestIfEnum', 'if': 'defined(TEST_IF_EVT_BAR)' } },
   'if': 'defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)' }
diff --git a/tests/qapi-schema/qapi-schema-test.out 
b/tests/qapi-schema/qapi-schema-test.out

[Qemu-devel] [PATCH for-4.0 v7 13/27] qapi: add a dictionary form for TYPE

[Marc-André Lureau]

Wherever a struct/union/alternate/command/event member with NAME: TYPE
form is accepted, desugar it to a NAME: { 'type': TYPE } form.

This will allow to add new member details, such as 'if' in the
following patch to introduce conditionals, or 'default' for default
values etc.

Signed-off-by: Marc-André Lureau 
---
 scripts/qapi/common.py| 71 ++-
 tests/Makefile.include|  6 ++
 tests/qapi-schema/alternate-invalid-dict.err  |  1 +
 tests/qapi-schema/alternate-invalid-dict.exit |  1 +
 tests/qapi-schema/alternate-invalid-dict.json |  4 ++
 tests/qapi-schema/alternate-invalid-dict.out  |  0
 .../qapi-schema/event-member-invalid-dict.err |  1 +
 .../event-member-invalid-dict.exit|  1 +
 .../event-member-invalid-dict.json|  2 +
 .../qapi-schema/event-member-invalid-dict.out |  0
 tests/qapi-schema/event-nest-struct.json  |  2 +-
 .../flat-union-inline-invalid-dict.err|  1 +
 .../flat-union-inline-invalid-dict.exit   |  1 +
 .../flat-union-inline-invalid-dict.json   | 11 +++
 .../flat-union-inline-invalid-dict.out|  0
 tests/qapi-schema/flat-union-inline.json  |  2 +-
 .../nested-struct-data-invalid-dict.err   |  1 +
 .../nested-struct-data-invalid-dict.exit  |  1 +
 .../nested-struct-data-invalid-dict.json  |  3 +
 .../nested-struct-data-invalid-dict.out   |  0
 tests/qapi-schema/nested-struct-data.json |  2 +-
 tests/qapi-schema/qapi-schema-test.json   | 10 +--
 .../struct-member-invalid-dict.err|  1 +
 .../struct-member-invalid-dict.exit   |  1 +
 .../struct-member-invalid-dict.json   |  3 +
 .../struct-member-invalid-dict.out|  0
 .../qapi-schema/union-branch-invalid-dict.err |  1 +
 .../union-branch-invalid-dict.exit|  1 +
 .../union-branch-invalid-dict.json|  4 ++
 .../qapi-schema/union-branch-invalid-dict.out |  0
 30 files changed, 106 insertions(+), 26 deletions(-)
 create mode 100644 tests/qapi-schema/alternate-invalid-dict.err
 create mode 100644 tests/qapi-schema/alternate-invalid-dict.exit
 create mode 100644 tests/qapi-schema/alternate-invalid-dict.json
 create mode 100644 tests/qapi-schema/alternate-invalid-dict.out
 create mode 100644 tests/qapi-schema/event-member-invalid-dict.err
 create mode 100644 tests/qapi-schema/event-member-invalid-dict.exit
 create mode 100644 tests/qapi-schema/event-member-invalid-dict.json
 create mode 100644 tests/qapi-schema/event-member-invalid-dict.out
 create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.err
 create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.exit
 create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.json
 create mode 100644 tests/qapi-schema/flat-union-inline-invalid-dict.out
 create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.err
 create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.exit
 create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.json
 create mode 100644 tests/qapi-schema/nested-struct-data-invalid-dict.out
 create mode 100644 tests/qapi-schema/struct-member-invalid-dict.err
 create mode 100644 tests/qapi-schema/struct-member-invalid-dict.exit
 create mode 100644 tests/qapi-schema/struct-member-invalid-dict.json
 create mode 100644 tests/qapi-schema/struct-member-invalid-dict.out
 create mode 100644 tests/qapi-schema/union-branch-invalid-dict.err
 create mode 100644 tests/qapi-schema/union-branch-invalid-dict.exit
 create mode 100644 tests/qapi-schema/union-branch-invalid-dict.json
 create mode 100644 tests/qapi-schema/union-branch-invalid-dict.out

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 95e55b3f44..4b3ba53dc7 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -588,11 +588,13 @@ def discriminator_find_enum_define(expr):
 if not base_members:
 return None
 
-discriminator_type = base_members.get(discriminator)
-if not discriminator_type:
+discriminator_value = base_members.get(discriminator)
+if not discriminator_value:
 return None
 
-return enum_types.get(discriminator_type)
+if isinstance(discriminator_value, dict):
+discriminator_value = discriminator_value['type']
+return enum_types.get(discriminator_value)
 
 
 # Names must be letters, numbers, -, and _.  They must start with letter,
@@ -660,6 +662,15 @@ def check_if(expr, info):
 check_if_str(ifcond, info)
 
 
+def normalize_members(expr, field):
+members = expr.get(field)
+if isinstance(members, OrderedDict):
+for key, arg in members.items():
+if isinstance(arg, dict):
+continue
+members[key] = {'type': arg}
+
+
 def check_type(info, source, value, allow_array=False,
allow_implicit=False, allow_optional=False,
allow_metas=[]):
@@ -704,8 +715,14 @@ def check_type(info, source, value, al

[Qemu-devel] [PATCH for-4.0 v7 12/27] qapi: rename allow_dict to allow_implicit

[Marc-André Lureau]

This makes it a bit clearer what is the intent of the dictionnary for
the check_type() function, since there was some confusion on a
previous iteration of this series.

Suggested-by: Markus Armbruster 
Signed-off-by: Marc-André Lureau 
---
 scripts/qapi/common.py | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 557b413950..95e55b3f44 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -661,7 +661,7 @@ def check_if(expr, info):
 
 
 def check_type(info, source, value, allow_array=False,
-   allow_dict=False, allow_optional=False,
+   allow_implicit=False, allow_optional=False,
allow_metas=[]):
 global all_names
 
@@ -688,7 +688,7 @@ def check_type(info, source, value, allow_array=False,
(source, all_names[value], value))
 return
 
-if not allow_dict:
+if not allow_implicit:
 raise QAPISemError(info, "%s should be a type name" % source)
 
 if not isinstance(value, OrderedDict):
@@ -718,7 +718,7 @@ def check_command(expr, info):
 if boxed:
 args_meta += ['union', 'alternate']
 check_type(info, "'data' for command '%s'" % name,
-   expr.get('data'), allow_dict=not boxed, allow_optional=True,
+   expr.get('data'), allow_implicit=not boxed, allow_optional=True,
allow_metas=args_meta)
 returns_meta = ['union', 'struct']
 if name in returns_whitelist:
@@ -736,7 +736,7 @@ def check_event(expr, info):
 if boxed:
 meta += ['union', 'alternate']
 check_type(info, "'data' for event '%s'" % name,
-   expr.get('data'), allow_dict=not boxed, allow_optional=True,
+   expr.get('data'), allow_implicit=not boxed, allow_optional=True,
allow_metas=meta)
 
 
@@ -764,7 +764,7 @@ def check_union(expr, info):
 else:
 # The object must have a string or dictionary 'base'.
 check_type(info, "'base' for union '%s'" % name,
-   base, allow_dict=True, allow_optional=True,
+   base, allow_implicit=True, allow_optional=True,
allow_metas=['struct'])
 if not base:
 raise QAPISemError(info, "Flat union '%s' must have a base"
@@ -887,7 +887,7 @@ def check_struct(expr, info):
 members = expr['data']
 
 check_type(info, "'data' for struct '%s'" % name, members,
-   allow_dict=True, allow_optional=True)
+   allow_implicit=True, allow_optional=True)
 check_type(info, "'base' for struct '%s'" % name, expr.get('base'),
allow_metas=['struct'])
 
-- 
2.20.0.rc1

[Qemu-devel] [PATCH for-4.0 v7 07/27] qapi: improve reporting of unknown or missing keys

[Marc-André Lureau]

Report the set of missing or unknown keys. And give a hint about the
accepted keys.

Signed-off-by: Marc-André Lureau 
---
 scripts/qapi/common.py  | 23 +++
 tests/qapi-schema/alternate-base.err|  1 +
 tests/qapi-schema/double-type.err   |  1 +
 tests/qapi-schema/unknown-expr-key.err  |  3 ++-
 tests/qapi-schema/unknown-expr-key.json |  2 +-
 5 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 18f5872808..f205805751 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -874,14 +874,21 @@ def check_struct(expr, info):
 
 
 def check_known_keys(info, source, keys, required, optional):
-for key in keys:
-if key not in required and key not in optional:
-raise QAPISemError(info, "Unknown key '%s' in %s" % (key, source))
-
-for key in required:
-if key not in keys:
-raise QAPISemError(info, "Key '%s' is missing from %s"
-   % (key, source))
+
+def pprint(elems):
+return ', '.join("'" + e + "'" for e in sorted(elems))
+
+missing = set(required) - set(keys)
+if missing:
+raise QAPISemError(info, "Key%s %s %s missing from %s"
+   % ('s' if len(missing) > 1 else '', pprint(missing),
+  'are' if len(missing) > 1 else 'is', source))
+allowed = set(required + optional)
+unknown = set(keys) - allowed
+if unknown:
+raise QAPISemError(info, "Unknown key%s %s in %s\nValid keys are %s."
+   % ('s' if len(unknown) > 1 else '', pprint(unknown),
+  source, pprint(allowed)))
 
 
 def check_keys(expr_elem, meta, required, optional=[]):
diff --git a/tests/qapi-schema/alternate-base.err 
b/tests/qapi-schema/alternate-base.err
index 30d8a34373..ebe05bc898 100644
--- a/tests/qapi-schema/alternate-base.err
+++ b/tests/qapi-schema/alternate-base.err
@@ -1 +1,2 @@
 tests/qapi-schema/alternate-base.json:4: Unknown key 'base' in alternate 'Alt'
+Valid keys are 'alternate', 'data', 'if'.
diff --git a/tests/qapi-schema/double-type.err 
b/tests/qapi-schema/double-type.err
index f9613c6d6b..799193dba1 100644
--- a/tests/qapi-schema/double-type.err
+++ b/tests/qapi-schema/double-type.err
@@ -1 +1,2 @@
 tests/qapi-schema/double-type.json:2: Unknown key 'command' in struct 'bar'
+Valid keys are 'base', 'data', 'if', 'struct'.
diff --git a/tests/qapi-schema/unknown-expr-key.err 
b/tests/qapi-schema/unknown-expr-key.err
index 12f5ed5b43..6ff8bb99c5 100644
--- a/tests/qapi-schema/unknown-expr-key.err
+++ b/tests/qapi-schema/unknown-expr-key.err
@@ -1 +1,2 @@
-tests/qapi-schema/unknown-expr-key.json:2: Unknown key 'bogus' in struct 'bar'
+tests/qapi-schema/unknown-expr-key.json:2: Unknown keys 'bogus', 'phony' in 
struct 'bar'
+Valid keys are 'base', 'data', 'if', 'struct'.
diff --git a/tests/qapi-schema/unknown-expr-key.json 
b/tests/qapi-schema/unknown-expr-key.json
index 3b2be00cc4..13292d75ed 100644
--- a/tests/qapi-schema/unknown-expr-key.json
+++ b/tests/qapi-schema/unknown-expr-key.json
@@ -1,2 +1,2 @@
 # we reject an expression with unknown top-level keys
-{ 'struct': 'bar', 'data': { 'string': 'str'}, 'bogus': { } }
+{ 'struct': 'bar', 'data': { 'string': 'str'}, 'bogus': { }, 'phony': { } }
-- 
2.20.0.rc1

[Qemu-devel] [PATCH for-4.0 v7 16/27] qapi: add 'if' to union members

[Marc-André Lureau]

Add 'if' key to union members:

{ 'union': 'TestIfUnion', 'data':
'mem': { 'type': 'str', 'if': 'COND'} }

The generated code remains unconditional for now. Later patches
generate the conditionals.

Signed-off-by: Marc-André Lureau 
---
 scripts/qapi/common.py  | 17 +
 docs/devel/qapi-code-gen.txt|  2 +-
 tests/qapi-schema/qapi-schema-test.json |  4 +++-
 tests/qapi-schema/qapi-schema-test.out  |  4 
 tests/qapi-schema/test-qapi.py  |  1 +
 5 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index f79b3fad54..fd622313cb 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -823,7 +823,7 @@ def check_union(expr, info):
 check_name(info, "Member of union '%s'" % name, key)
 if isinstance(value, dict):
 check_known_keys(info, "member '%s' of union '%s'" % (key, name),
- value, ['type'], [])
+ value, ['type'], ['if'])
 typ = value['type']
 else:
 typ = value
@@ -1513,8 +1513,8 @@ class QAPISchemaObjectTypeVariants(object):
 class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
 role = 'branch'
 
-def __init__(self, name, typ):
-QAPISchemaObjectTypeMember.__init__(self, name, typ, False)
+def __init__(self, name, typ, ifcond=None):
+QAPISchemaObjectTypeMember.__init__(self, name, typ, False, ifcond)
 
 
 class QAPISchemaAlternateType(QAPISchemaType):
@@ -1796,14 +1796,14 @@ class QAPISchema(object):
 def _make_variant(self, case, typ):
 return QAPISchemaObjectTypeVariant(case, typ)
 
-def _make_simple_variant(self, case, typ, info):
+def _make_simple_variant(self, case, typ, ifcond, info):
 if isinstance(typ, list):
 assert len(typ) == 1
 typ = self._make_array_type(typ[0], info)
 typ = self._make_implicit_object_type(
 typ, info, None, self.lookup_type(typ),
 'wrapper', [self._make_member('data', typ, None, info)])
-return QAPISchemaObjectTypeVariant(case, typ)
+return QAPISchemaObjectTypeVariant(case, typ, ifcond)
 
 def _def_union_type(self, expr, info, doc):
 name = expr['union']
@@ -1821,10 +1821,11 @@ class QAPISchema(object):
 for (key, value) in data.items()]
 members = []
 else:
-variants = [self._make_simple_variant(key, value['type'], info)
+variants = [self._make_simple_variant(key, value['type'],
+  value.get('if'), info)
 for (key, value) in data.items()]
-typ = self._make_implicit_enum_type(name, info, ifcond,
-[v.name for v in variants])
+enum = [{'name': v.name, 'if': v.ifcond} for v in variants]
+typ = self._make_implicit_enum_type(name, info, ifcond, enum)
 tag_member = QAPISchemaObjectTypeMember('type', typ, False)
 members = [tag_member]
 self._def_entity(
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index e6fff32241..6f2457a2e0 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -754,7 +754,7 @@ gets its generated code guarded like this:
 
 Where a member can be defined with a single string value for its type,
 it is also possible to supply a dictionary instead with both 'type'
-and 'if' keys. (TODO: union and alternate)
+and 'if' keys. (TODO: alternate)
 
 Example: a conditional 'bar' member
 
diff --git a/tests/qapi-schema/qapi-schema-test.json 
b/tests/qapi-schema/qapi-schema-test.json
index b8565a3913..6e9e4e14d9 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -210,7 +210,9 @@
   [ 'foo', { 'name' : 'bar', 'if': 'defined(TEST_IF_ENUM_BAR)' } ],
   'if': 'defined(TEST_IF_ENUM)' }
 
-{ 'union': 'TestIfUnion', 'data': { 'foo': 'TestStruct' },
+{ 'union': 'TestIfUnion', 'data':
+  { 'foo': 'TestStruct',
+'union_bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} },
   'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' }
 
 { 'alternate': 'TestIfAlternate', 'data': { 'foo': 'int', 'bar': 'TestStruct' 
},
diff --git a/tests/qapi-schema/qapi-schema-test.out 
b/tests/qapi-schema/qapi-schema-test.out
index 23fb078b85..a342ecfc55 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -280,11 +280,15 @@ object q_obj_TestStruct-wrapper
 member data: TestStruct optional=False
 enum TestIfUnionKind
 member foo
+member union_bar
+if ['defined(TEST_IF_UNION_BAR)']
 if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
 object TestIfUnion
 member type: TestIfUnionKind optional=False
 tag type
 case foo: q_obj_TestStruct-wrapper
+case union_bar: q_

[Qemu-devel] [PATCH for-4.0 v7 17/27] qapi: simplify make_enum_members()

[Marc-André Lureau]

The function only receives the dictionary form of enum expressions
now, so we can make it shorter.

Suggested-by: Markus Armbruster 
Signed-off-by: Marc-André Lureau 
---
 scripts/qapi/common.py | 11 +--
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index fd622313cb..44020779dd 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1714,16 +1714,7 @@ class QAPISchema(object):
 qtype_values, 'QTYPE'))
 
 def _make_enum_members(self, values):
-enum = []
-for v in values:
-if isinstance(v, dict):
-name = v['name']
-ifcond = v.get('if')
-else:
-name = v
-ifcond = None
-enum.append(QAPISchemaMember(name, ifcond))
-return enum
+return [QAPISchemaMember(v['name'], v.get('if')) for v in values]
 
 def _make_implicit_enum_type(self, name, info, ifcond, values):
 # See also QAPISchemaObjectTypeMember._pretty_owner()
-- 
2.20.0.rc1

[Qemu-devel] [PATCH for-4.0 v7 20/27] tests/qapi: add command with condition on alternate argument

[Marc-André Lureau]

For completeness.

Signed-off-by: Marc-André Lureau 
---
 tests/qapi-schema/qapi-schema-test.json | 3 +++
 tests/qapi-schema/qapi-schema-test.out  | 6 ++
 2 files changed, 9 insertions(+)

diff --git a/tests/qapi-schema/qapi-schema-test.json 
b/tests/qapi-schema/qapi-schema-test.json
index d58cc49028..cb0857df52 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -223,6 +223,9 @@
 'bar': { 'type': 'TestStruct', 'if': 'defined(TEST_IF_ALT_BAR)'} },
   'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' }
 
+{ 'command': 'TestIfAlternateCmd', 'data': { 'alt_cmd_arg': 'TestIfAlternate' 
},
+  'if': 'defined(TEST_IF_ALT)' }
+
 { 'command': 'TestIfCmd', 'data':
   { 'foo': 'TestIfStruct',
 'bar': { 'type': 'TestIfEnum', 'if': 'defined(TEST_IF_CMD_BAR)' } },
diff --git a/tests/qapi-schema/qapi-schema-test.out 
b/tests/qapi-schema/qapi-schema-test.out
index 59d7ed17a1..9464101d26 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -302,6 +302,12 @@ alternate TestIfAlternate
 case bar: TestStruct
 if ['defined(TEST_IF_ALT_BAR)']
 if ['defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)']
+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
+if ['defined(TEST_IF_ALT)']
 object q_obj_TestIfCmd-arg
 member foo: TestIfStruct optional=False
 member bar: TestIfEnum optional=False
-- 
2.20.0.rc1

[Qemu-devel] [PATCH for-4.0 v7 19/27] qapi: add 'if' to alternate members

[Marc-André Lureau]

Add 'if' key to alternate members:

{ 'alternate': 'TestIfAlternate', 'data':
  { 'alt': { 'type': 'TestStruct', 'if': 'COND' } } }

Generated code is not changed by this patch but with "qapi: add #if
conditions to generated code".

Signed-off-by: Marc-André Lureau 
---
 scripts/qapi/common.py  | 10 +-
 docs/devel/qapi-code-gen.txt|  2 +-
 tests/qapi-schema/qapi-schema-test.json |  4 +++-
 tests/qapi-schema/qapi-schema-test.out  |  1 +
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 44020779dd..a70d6dec3b 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -857,7 +857,7 @@ def check_alternate(expr, info):
 if isinstance(value, dict):
 check_known_keys(info,
  "member '%s' of alternate '%s'" % (key, name),
- value, ['type'], [])
+ value, ['type'], ['if'])
 typ = value['type']
 else:
 typ = value
@@ -1784,8 +1784,8 @@ class QAPISchema(object):
   self._make_members(data, info),
   None))
 
-def _make_variant(self, case, typ):
-return QAPISchemaObjectTypeVariant(case, typ)
+def _make_variant(self, case, typ, ifcond):
+return QAPISchemaObjectTypeVariant(case, typ, ifcond)
 
 def _make_simple_variant(self, case, typ, ifcond, info):
 if isinstance(typ, list):
@@ -1808,7 +1808,7 @@ class QAPISchema(object):
 name, info, doc, ifcond,
 'base', self._make_members(base, info))
 if tag_name:
-variants = [self._make_variant(key, value['type'])
+variants = [self._make_variant(key, value['type'], value.get('if'))
 for (key, value) in data.items()]
 members = []
 else:
@@ -1829,7 +1829,7 @@ class QAPISchema(object):
 name = expr['alternate']
 data = expr['data']
 ifcond = expr.get('if')
-variants = [self._make_variant(key, value['type'])
+variants = [self._make_variant(key, value['type'], value.get('if'))
 for (key, value) in data.items()]
 tag_member = QAPISchemaObjectTypeMember('type', 'QType', False)
 self._def_entity(
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 6f2457a2e0..5ea62cb111 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -754,7 +754,7 @@ gets its generated code guarded like this:
 
 Where a member can be defined with a single string value for its type,
 it is also possible to supply a dictionary instead with both 'type'
-and 'if' keys. (TODO: alternate)
+and 'if' keys.
 
 Example: a conditional 'bar' member
 
diff --git a/tests/qapi-schema/qapi-schema-test.json 
b/tests/qapi-schema/qapi-schema-test.json
index 0d28475f4c..d58cc49028 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -218,7 +218,9 @@
 { 'command': 'TestIfUnionCmd', 'data': { 'union_cmd_arg': 'TestIfUnion' },
   'if': 'defined(TEST_IF_UNION)' }
 
-{ 'alternate': 'TestIfAlternate', 'data': { 'foo': 'int', 'bar': 'TestStruct' 
},
+{ 'alternate': 'TestIfAlternate', 'data':
+  { 'foo': 'int',
+'bar': { 'type': 'TestStruct', 'if': 'defined(TEST_IF_ALT_BAR)'} },
   'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' }
 
 { 'command': 'TestIfCmd', 'data':
diff --git a/tests/qapi-schema/qapi-schema-test.out 
b/tests/qapi-schema/qapi-schema-test.out
index aedc668aa4..59d7ed17a1 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -300,6 +300,7 @@ alternate TestIfAlternate
 tag type
 case foo: int
 case bar: TestStruct
+if ['defined(TEST_IF_ALT_BAR)']
 if ['defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)']
 object q_obj_TestIfCmd-arg
 member foo: TestIfStruct optional=False
-- 
2.20.0.rc1

[Qemu-devel] [PATCH for-4.0 v7 26/27] qapi: add more conditions to SPICE

[Marc-André Lureau]

Now that member can be made conditional, let's make SPICE chardev
conditional:

* spiceport, spicevmc

  Before and after the patch for !CONFIG_SPICE, the error is the
  same ('spiceport' is not a valid char driver name).

Signed-off-by: Marc-André Lureau 
---
 qapi/char.json | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/qapi/char.json b/qapi/char.json
index 24628331ec..77ed847972 100644
--- a/qapi/char.json
+++ b/qapi/char.json
@@ -332,8 +332,8 @@
 ##
 { 'struct': 'ChardevSpiceChannel',
   'data': { 'type': 'str' },
-  'base': 'ChardevCommon' }
-# TODO: 'if': 'defined(CONFIG_SPICE)'
+  'base': 'ChardevCommon',
+  'if': 'defined(CONFIG_SPICE)' }
 
 ##
 # @ChardevSpicePort:
@@ -346,8 +346,8 @@
 ##
 { 'struct': 'ChardevSpicePort',
   'data': { 'fqdn': 'str' },
-  'base': 'ChardevCommon' }
-# TODO: 'if': 'defined(CONFIG_SPICE)'
+  'base': 'ChardevCommon',
+  'if': 'defined(CONFIG_SPICE)' }
 
 ##
 # @ChardevVC:
@@ -404,10 +404,10 @@
 'testdev': 'ChardevCommon',
 'stdio': 'ChardevStdio',
 'console': 'ChardevCommon',
-'spicevmc': 'ChardevSpiceChannel',
-# TODO: { 'type': 'ChardevSpiceChannel', 'if': 'defined(CONFIG_SPICE)' },
-'spiceport': 'ChardevSpicePort',
-# TODO: { 'type': 'ChardevSpicePort', 'if': 'defined(CONFIG_SPICE)' },
+'spicevmc': { 'type': 'ChardevSpiceChannel',
+  'if': 'defined(CONFIG_SPICE)' },
+'spiceport': { 'type': 'ChardevSpicePort',
+   'if': 'defined(CONFIG_SPICE)' },
 'vc': 'ChardevVC',
 'ringbuf': 'ChardevRingbuf',
 # next one is just for compatibility
-- 
2.20.0.rc1

[Qemu-devel] [PATCH for-4.0 v7 21/27] qapi: add #if conditions to generated code members

[Marc-André Lureau]

Wrap generated enum/struct members and code with #if/#endif, using the
.ifcond members added in the previous patches.

Some types generate both enum and struct members for example, so a
step-by-step is unnecessarily complicated to deal with (it would
easily generate invalid intermediary code).

Signed-off-by: Marc-André Lureau 
---
 scripts/qapi/common.py |  4 
 scripts/qapi/introspect.py | 13 +
 scripts/qapi/types.py  |  4 
 scripts/qapi/visit.py  |  6 ++
 4 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index a70d6dec3b..3a29812ef2 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -2108,11 +2108,13 @@ const QEnumLookup %(c_name)s_lookup = {
 ''',
 c_name=c_name(name))
 for m in members:
+ret += gen_if(m.ifcond)
 index = c_enum_const(name, m.name, prefix)
 ret += mcgen('''
 [%(index)s] = "%(name)s",
 ''',
  index=index, name=m.name)
+ret += gen_endif(m.ifcond)
 
 ret += mcgen('''
 },
@@ -2134,10 +2136,12 @@ typedef enum %(c_name)s {
 c_name=c_name(name))
 
 for m in enum_members:
+ret += gen_if(m.ifcond)
 ret += mcgen('''
 %(c_enum)s,
 ''',
  c_enum=c_enum_const(name, m.name, prefix))
+ret += gen_endif(m.ifcond)
 
 ret += mcgen('''
 } %(c_name)s;
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 417625d54b..77087f629b 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -162,6 +162,8 @@ const QLitObject %(c_name)s = %(c_string)s;
 ret = {'name': member.name, 'type': self._use_type(member.type)}
 if member.optional:
 ret['default'] = None
+if member.ifcond:
+ret = (ret, {'if': member.ifcond})
 return ret
 
 def _gen_variants(self, tag_name, variants):
@@ -169,14 +171,16 @@ const QLitObject %(c_name)s = %(c_string)s;
 'variants': [self._gen_variant(v) for v in variants]}
 
 def _gen_variant(self, variant):
-return {'case': variant.name, 'type': self._use_type(variant.type)}
+return ({'case': variant.name, 'type': self._use_type(variant.type)},
+{'if': variant.ifcond})
 
 def visit_builtin_type(self, name, info, json_type):
 self._gen_qlit(name, 'builtin', {'json-type': json_type}, [])
 
 def visit_enum_type(self, name, info, ifcond, members, prefix):
 self._gen_qlit(name, 'enum',
-   {'values': [m.name for m in members]}, ifcond)
+   {'values': [(m.name, {'if': m.ifcond}) for m in 
members]},
+   ifcond)
 
 def visit_array_type(self, name, info, ifcond, element_type):
 element = self._use_type(element_type)
@@ -192,8 +196,9 @@ const QLitObject %(c_name)s = %(c_string)s;
 
 def visit_alternate_type(self, name, info, ifcond, variants):
 self._gen_qlit(name, 'alternate',
-   {'members': [{'type': self._use_type(m.type)}
-for m in variants.variants]}, ifcond)
+   {'members': [
+   ({'type': self._use_type(m.type)}, {'if': m.ifcond})
+   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):
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index 0404710bbd..e2ee9f3b72 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -43,6 +43,7 @@ struct %(c_name)s {
 def gen_struct_members(members):
 ret = ''
 for memb in members:
+ret += gen_if(memb.ifcond)
 if memb.optional:
 ret += mcgen('''
 bool has_%(c_name)s;
@@ -52,6 +53,7 @@ def gen_struct_members(members):
 %(c_type)s %(c_name)s;
 ''',
  c_type=memb.type.c_type(), c_name=c_name(memb.name))
+ret += gen_endif(memb.ifcond)
 return ret
 
 
@@ -131,11 +133,13 @@ def gen_variants(variants):
 for var in variants.variants:
 if var.type.name == 'q_empty':
 continue
+ret += gen_if(var.ifcond)
 ret += mcgen('''
 %(c_type)s %(c_name)s;
 ''',
  c_type=var.type.c_unboxed_type(),
  c_name=c_name(var.name))
+ret += gen_endif(var.ifcond)
 
 ret += mcgen('''
 } u;
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 24f85a2e85..82eab72b21 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -54,6 +54,7 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s 
*obj, Error **errp)
  c_type=base.c_name())
 
 for memb in members:
+ret += gen_if(memb.ifcond)
 if memb.optional:
 ret += mcgen('''
 if (visit_optional(

[Qemu-devel] [PATCH for-4.0 v7 22/27] qapi: add 'If:' condition to enum values documentation

[Marc-André Lureau]

Use a common function to generate the "If:..." line.

While at it, get rid of the existing \n\n (no idea why it was
there). Use a line-break in member description, this seems to look
slightly better in the plaintext version.

Signed-off-by: Marc-André Lureau 
---
 scripts/qapi/doc.py | 24 +++-
 tests/qapi-schema/doc-good.json |  4 +++-
 tests/qapi-schema/doc-good.out  |  1 +
 tests/qapi-schema/doc-good.texi |  2 +-
 4 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 76cb186ff9..2133ded47e 100755
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -126,19 +126,26 @@ def texi_body(doc):
 return texi_format(doc.body.text)
 
 
-def texi_enum_value(value):
+def texi_if(ifcond, prefix='\n', suffix='\n'):
+"""Format the #if condition"""
+return '%s@b{If:} @code{%s}%s' % (
+prefix, ', '.join(ifcond), suffix) if ifcond else ''
+
+
+def texi_enum_value(value, desc, suffix=''):
 """Format a table of members item for an enumeration value"""
-return '@item @code{%s}\n' % value.name
+return '@item @code{%s}\n%s%s' % (
+value.name, desc, texi_if(value.ifcond, prefix='@*'))
 
 
-def texi_member(member, suffix=''):
+def texi_member(member, desc='', suffix=''):
 """Format a table of members item for an object type member"""
 typ = member.type.doc_type()
 membertype = ': ' + typ if typ else ''
-return '@item @code{%s%s}%s%s\n' % (
+return '@item @code{%s%s}%s%s\n%s' % (
 member.name, membertype,
 ' (optional)' if member.optional else '',
-suffix)
+suffix, desc)
 
 
 def texi_members(doc, what, base, variants, member_func):
@@ -155,7 +162,7 @@ def texi_members(doc, what, base, variants, member_func):
 desc = 'One of ' + members_text + '\n'
 else:
 desc = 'Not documented\n'
-items += member_func(section.member) + desc
+items += member_func(section.member, desc)
 if base:
 items += '@item The members of @code{%s}\n' % base.doc_type()
 if variants:
@@ -165,7 +172,7 @@ def texi_members(doc, what, base, variants, member_func):
 if v.type.is_implicit():
 assert not v.type.base and not v.type.variants
 for m in v.type.local_members:
-items += member_func(m, when)
+items += member_func(m, suffix=when)
 else:
 items += '@item The members of @code{%s}%s\n' % (
 v.type.doc_type(), when)
@@ -185,8 +192,7 @@ def texi_sections(doc, ifcond):
 body += texi_example(section.text)
 else:
 body += texi_format(section.text)
-if ifcond:
-body += '\n\n@b{If:} @code{%s}' % ", ".join(ifcond)
+body += texi_if(ifcond, suffix='')
 return body
 
 
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index 984cd8ed06..c7fe08c530 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -55,7 +55,9 @@
 #
 # @two is undocumented
 ##
-{ 'enum': 'Enum', 'data': [ 'one', 'two' ], 'if': 'defined(IFCOND)' }
+{ 'enum': 'Enum', 'data':
+  [ { 'name': 'one', 'if': 'defined(IFENUM)' }, 'two' ],
+  'if': 'defined(IFCOND)' }
 
 ##
 # @Base:
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index c2fc5c774a..a05535b69b 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -11,6 +11,7 @@ enum QType
 module doc-good.json
 enum Enum
 member one
+if ['defined(IFENUM)']
 member two
 if ['defined(IFCOND)']
 object Base
diff --git a/tests/qapi-schema/doc-good.texi b/tests/qapi-schema/doc-good.texi
index e42eace474..5c0231e0e6 100644
--- a/tests/qapi-schema/doc-good.texi
+++ b/tests/qapi-schema/doc-good.texi
@@ -84,12 +84,12 @@ Examples:
 @table @asis
 @item @code{one}
 The @emph{one} @{and only@}
+@*@b{If:} @code{defined(IFENUM)}
 @item @code{two}
 Not documented
 @end table
 @code{two} is undocumented
 
-
 @b{If:} @code{defined(IFCOND)}
 @end deftp
 
-- 
2.20.0.rc1

Re: [Qemu-devel] [PATCH v6 22/27] docs: document schema configuration

[Marc-André Lureau]

On Thu, Dec 6, 2018 at 10:14 PM Markus Armbruster  wrote:
>
> Marc-André Lureau  writes:
>
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  docs/devel/qapi-code-gen.txt | 19 +++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
> > index c2e11465f0..799aef7728 100644
> > --- a/docs/devel/qapi-code-gen.txt
> > +++ b/docs/devel/qapi-code-gen.txt
> > @@ -752,6 +752,25 @@ gets its generated code guarded like this:
> >   #endif /* defined(HAVE_BAR) */
> >   #endif /* defined(CONFIG_FOO) */
> >
> > +Where a member can be defined with a single string value for its type,
> > +it is also possible to supply a dictionary instead with both 'type'
> > +and 'if' keys.
> > +
> > +Example: a conditional 'bar' member
> > +
> > +{ 'struct': 'IfStruct', 'data':
> > +  { 'foo': 'int',
> > +'bar': { 'type': 'int', 'if': 'defined(IFCOND)'} } }
> > +
> > +An enum value can be replaced by a dictionary with a 'name' and a 'if'
> > +key.
> > +
> > +Example: a conditional 'bar' enum member.
> > +
> > +{ 'enum': 'IfEnum', 'data':
> > +  [ 'foo',
> > +{ 'name' : 'bar', '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.
>
> I'd prefer to update qapi-code-gen.txt right when we extend the schema
> language, like you did in part 1 (commit 967c885108f qapi: add 'if' to
> top-level expressions).  I understand it's a bit more churn, since you
> need four steps (enum, struct, union, alternate members).  I think the
> following would be the least work for you that still satisfies me:
>
> * Add the part about enum values in PATCH 13.
>
> * Add the rest in PATCH 17 (the first patch that implements a part of
>   it), with a TODO implementpatches>.  Then just update the TODO as you go.
>
> Okay?

Easy enough, done.

[Qemu-devel] [PATCH for-4.0 v7 24/27] qapi: add condition to variants documentation

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 scripts/qapi/doc.py | 4 ++--
 tests/qapi-schema/doc-good.json | 4 ++--
 tests/qapi-schema/doc-good.out  | 3 +++
 tests/qapi-schema/doc-good.texi | 4 ++--
 4 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index e0cf94421c..4008054eea 100755
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -167,8 +167,8 @@ def texi_members(doc, what, base, variants, member_func):
 items += '@item The members of @code{%s}\n' % base.doc_type()
 if variants:
 for v in variants.variants:
-when = ' when @code{%s} is @t{"%s"}' % (
-variants.tag_member.name, v.name)
+when = ' when @code{%s} is @t{"%s"}%s' % (
+variants.tag_member.name, v.name, texi_if(v.ifcond, " (", ")"))
 if v.type.is_implicit():
 assert not v.type.base and not v.type.variants
 for m in v.type.local_members:
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index 158443b1a3..afe46d93f0 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -86,13 +86,13 @@
 { 'union': 'Object',
   'base': 'Base',
   'discriminator': 'base1',
-  'data': { 'one': 'Variant1', 'two': 'Variant2' } }
+  'data': { 'one': 'Variant1', 'two': { 'type': 'Variant2', 'if': 'IFTWO' } } }
 
 ##
 # @SugaredUnion:
 ##
 { 'union': 'SugaredUnion',
-  'data': { 'one': 'Variant1', 'two': 'Variant2' } }
+  'data': { 'one': 'Variant1', 'two': { 'type': 'Variant2', 'if': 'IFTWO' } } }
 
 ##
 # == Another subsection
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index c310b47be2..ed647b82be 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -25,6 +25,7 @@ object Object
 tag base1
 case one: Variant1
 case two: Variant2
+if ['IFTWO']
 object q_obj_Variant1-wrapper
 member data: Variant1 optional=False
 object q_obj_Variant2-wrapper
@@ -32,11 +33,13 @@ object q_obj_Variant2-wrapper
 enum SugaredUnionKind
 member one
 member two
+if ['IFTWO']
 object SugaredUnion
 member type: SugaredUnionKind optional=False
 tag type
 case one: q_obj_Variant1-wrapper
 case two: q_obj_Variant2-wrapper
+if ['IFTWO']
 object q_obj_cmd-arg
 member arg1: int optional=False
 member arg2: str optional=True
diff --git a/tests/qapi-schema/doc-good.texi b/tests/qapi-schema/doc-good.texi
index 0f8d69a378..df08a89521 100644
--- a/tests/qapi-schema/doc-good.texi
+++ b/tests/qapi-schema/doc-good.texi
@@ -142,7 +142,7 @@ Not documented
 @table @asis
 @item The members of @code{Base}
 @item The members of @code{Variant1} when @code{base1} is @t{"one"}
-@item The members of @code{Variant2} when @code{base1} is @t{"two"}
+@item The members of @code{Variant2} when @code{base1} is @t{"two"} (@b{If:} 
@code{IFTWO})
 @end table
 
 @end deftp
@@ -158,7 +158,7 @@ Not documented
 @item @code{type}
 One of @t{"one"}, @t{"two"}
 @item @code{data: Variant1} when @code{type} is @t{"one"}
-@item @code{data: Variant2} when @code{type} is @t{"two"}
+@item @code{data: Variant2} when @code{type} is @t{"two"} (@b{If:} 
@code{IFTWO})
 @end table
 
 @end deftp
-- 
2.20.0.rc1

[Qemu-devel] [PATCH for-4.0 v7 23/27] qapi: add 'If:' condition to struct members documentation

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 scripts/qapi/doc.py | 4 ++--
 tests/qapi-schema/doc-good.json | 3 ++-
 tests/qapi-schema/doc-good.out  | 1 +
 tests/qapi-schema/doc-good.texi | 1 +
 4 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 2133ded47e..e0cf94421c 100755
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -142,10 +142,10 @@ def texi_member(member, desc='', suffix=''):
 """Format a table of members item for an object type member"""
 typ = member.type.doc_type()
 membertype = ': ' + typ if typ else ''
-return '@item @code{%s%s}%s%s\n%s' % (
+return '@item @code{%s%s}%s%s\n%s%s' % (
 member.name, membertype,
 ' (optional)' if member.optional else '',
-suffix, desc)
+suffix, desc, texi_if(member.ifcond, prefix='@*'))
 
 
 def texi_members(doc, what, base, variants, member_func):
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index c7fe08c530..158443b1a3 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -72,7 +72,8 @@
 #
 # Another paragraph (but no @var: line)
 ##
-{ 'struct': 'Variant1', 'data': { 'var1': 'str' } }
+{ 'struct': 'Variant1',
+  'data': { 'var1': { 'type': 'str', 'if': 'defined(IFSTR)' } } }
 
 ##
 # @Variant2:
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index a05535b69b..c310b47be2 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -18,6 +18,7 @@ object Base
 member base1: Enum optional=False
 object Variant1
 member var1: str optional=False
+if ['defined(IFSTR)']
 object Variant2
 object Object
 base Base
diff --git a/tests/qapi-schema/doc-good.texi b/tests/qapi-schema/doc-good.texi
index 5c0231e0e6..0f8d69a378 100644
--- a/tests/qapi-schema/doc-good.texi
+++ b/tests/qapi-schema/doc-good.texi
@@ -119,6 +119,7 @@ Another paragraph (but no @code{var}: line)
 @table @asis
 @item @code{var1: string}
 Not documented
+@*@b{If:} @code{defined(IFSTR)}
 @end table
 
 @end deftp
-- 
2.20.0.rc1

[Qemu-devel] [PATCH for-4.0 v7 27/27] qapi: add conditions to REPLICATION type/commands on the schema

[Marc-André Lureau]

Add #if defined(CONFIG_REPLICATION) in generated code, and adjust the
code accordingly.

Made conditional:

* xen-set-replication, query-xen-replication-status,
  xen-colo-do-checkpoint

  Before the patch, we first register the commands unconditionally in
  generated code (requires a stub), then conditionally unregister in
  qmp_unregister_commands_hack().

  Afterwards, we register only when CONFIG_REPLICATION.  The command
  fails exactly the same, with CommandNotFound.

  Improvement, because now query-qmp-schema is accurate, and we're one
  step closer to killing qmp_unregister_commands_hack().

* enum BlockdevDriver value "replication" in command blockdev-add

* BlockdevOptions variant @replication

and related structures.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Markus Armbruster 
---
 qapi/block-core.json | 13 +
 qapi/migration.json  | 12 
 migration/colo.c | 16 
 monitor.c|  5 -
 4 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 92e0205d91..762000f31f 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2623,7 +2623,9 @@
 'copy-on-read', 'dmg', 'file', 'ftp', 'ftps', 'gluster',
 'host_cdrom', 'host_device', 'http', 'https', 'iscsi', 'luks',
 'nbd', 'nfs', 'null-aio', 'null-co', 'nvme', 'parallels', 'qcow',
-'qcow2', 'qed', 'quorum', 'raw', 'rbd', 'replication', 'sheepdog',
+'qcow2', 'qed', 'quorum', 'raw', 'rbd',
+{ 'name': 'replication', 'if': 'defined(CONFIG_REPLICATION)' },
+'sheepdog',
 'ssh', 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] 
}
 
 ##
@@ -3380,7 +3382,8 @@
 #
 # Since: 2.9
 ##
-{ 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ] }
+{ 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ],
+  'if': 'defined(CONFIG_REPLICATION)' }
 
 ##
 # @BlockdevOptionsReplication:
@@ -3398,7 +3401,8 @@
 { 'struct': 'BlockdevOptionsReplication',
   'base': 'BlockdevOptionsGenericFormat',
   'data': { 'mode': 'ReplicationMode',
-'*top-id': 'str' } }
+'*top-id': 'str' },
+  'if': 'defined(CONFIG_REPLICATION)' }
 
 ##
 # @NFSTransport:
@@ -3714,7 +3718,8 @@
   'quorum': 'BlockdevOptionsQuorum',
   'raw':'BlockdevOptionsRaw',
   'rbd':'BlockdevOptionsRbd',
-  'replication':'BlockdevOptionsReplication',
+  'replication': { 'type': 'BlockdevOptionsReplication',
+   'if': 'defined(CONFIG_REPLICATION)' },
   'sheepdog':   'BlockdevOptionsSheepdog',
   'ssh':'BlockdevOptionsSsh',
   'throttle':   'BlockdevOptionsThrottle',
diff --git a/qapi/migration.json b/qapi/migration.json
index 5fd33316a0..31b589ec26 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1257,7 +1257,8 @@
 # Since: 2.9
 ##
 { 'command': 'xen-set-replication',
-  'data': { 'enable': 'bool', 'primary': 'bool', '*failover' : 'bool' } }
+  'data': { 'enable': 'bool', 'primary': 'bool', '*failover' : 'bool' },
+  'if': 'defined(CONFIG_REPLICATION)' }
 
 ##
 # @ReplicationStatus:
@@ -1272,7 +1273,8 @@
 # Since: 2.9
 ##
 { 'struct': 'ReplicationStatus',
-  'data': { 'error': 'bool', '*desc': 'str' } }
+  'data': { 'error': 'bool', '*desc': 'str' },
+  'if': 'defined(CONFIG_REPLICATION)' }
 
 ##
 # @query-xen-replication-status:
@@ -1289,7 +1291,8 @@
 # Since: 2.9
 ##
 { 'command': 'query-xen-replication-status',
-  'returns': 'ReplicationStatus' }
+  'returns': 'ReplicationStatus',
+  'if': 'defined(CONFIG_REPLICATION)' }
 
 ##
 # @xen-colo-do-checkpoint:
@@ -1305,7 +1308,8 @@
 #
 # Since: 2.9
 ##
-{ 'command': 'xen-colo-do-checkpoint' }
+{ 'command': 'xen-colo-do-checkpoint',
+  'if': 'defined(CONFIG_REPLICATION)' }
 
 ##
 # @COLOStatus:
diff --git a/migration/colo.c b/migration/colo.c
index fcff04c78c..398b239d1c 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -24,7 +24,9 @@
 #include "trace.h"
 #include "qemu/error-report.h"
 #include "migration/failover.h"
+#ifdef CONFIG_REPLICATION
 #include "replication.h"
+#endif
 #include "net/colo-compare.h"
 #include "net/colo.h"
 #include "block/block.h"
@@ -201,11 +203,11 @@ void colo_do_failover(MigrationState *s)
 }
 }
 
+#ifdef CONFIG_REPLICATION
 void qmp_xen_set_replication(bool enable, bool primary,
  bool has_failover, bool failover,
  Error **errp)
 {
-#ifdef CONFIG_REPLICATION
 ReplicationMode mode = primary ?
REPLICATION_MODE_PRIMARY :
REPLICATION_MODE_SECONDARY;
@@ -224,14 +226,10 @@ void qmp_xen_set_replication(bool enable, bool primary,
 }
 replication_stop_all(failover, failover ? NULL : errp);
 }
-#else
-abort();
-#endif
 }
 
 ReplicationStatus *qmp_query_xen_replication_status(Error **errp)
 {
-#ifdef CONFIG_REPLICATION
 Error *err = NULL;
 

Re: [Qemu-devel] [PATCH v6 20/27] qapi: add 'if' to alternate members

[Marc-André Lureau]

Hi
On Thu, Dec 6, 2018 at 8:41 PM Markus Armbruster  wrote:
>
> Marc-André Lureau  writes:
>
> > Add 'if' key to alternate members:
> >
> > { 'alternate': 'TestIfAlternate', 'data':
> >   { 'alt': { 'type': 'TestStruct', 'if': 'COND' } } }
> >
> > Generated code is not changed by this patch but with "qapi: add #if
> > conditions to generated code".
> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  scripts/qapi/common.py  | 10 +-
> >  tests/qapi-schema/qapi-schema-test.json |  6 +-
> >  tests/qapi-schema/qapi-schema-test.out  |  9 -
> >  3 files changed, 18 insertions(+), 7 deletions(-)
> >
> > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > index e1bd9a22ba..b3b64a60bf 100644
> > --- a/scripts/qapi/common.py
> > +++ b/scripts/qapi/common.py
> > @@ -843,7 +843,7 @@ def check_alternate(expr, info):
> >  for (key, value) in members.items():
> >  check_name(info, "Member of alternate '%s'" % name, key)
> >  source = "member '%s' of alternate '%s'" % (key, name)
> > -check_known_keys(info, source, value, ['type'], [])
> > +check_known_keys(info, source, value, ['type'], ['if'])
> >  typ = value['type']
> >
> >  # Ensure alternates have no type conflicts.
> > @@ -1774,8 +1774,8 @@ class QAPISchema(object):
> >self._make_members(data, 
> > info),
> >None))
> >
> > -def _make_variant(self, case, typ):
> > -return QAPISchemaObjectTypeVariant(case, typ)
> > +def _make_variant(self, case, typ, ifcond):
> > +return QAPISchemaObjectTypeVariant(case, typ, ifcond)
> >
> >  def _make_simple_variant(self, case, typ, ifcond, info):
> >  if isinstance(typ, list):
> > @@ -1798,7 +1798,7 @@ class QAPISchema(object):
> >  name, info, doc, ifcond,
> >  'base', self._make_members(base, info))
> >  if tag_name:
> > -variants = [self._make_variant(key, value['type'])
> > +variants = [self._make_variant(key, value['type'], 
> > value.get('if'))
> >  for (key, value) in data.items()]
> >  members = []
> >  else:
> > @@ -1819,7 +1819,7 @@ class QAPISchema(object):
> >  name = expr['alternate']
> >  data = expr['data']
> >  ifcond = expr.get('if')
> > -variants = [self._make_variant(key, value['type'])
> > +variants = [self._make_variant(key, value['type'], value.get('if'))
> >  for (key, value) in data.items()]
> >  tag_member = QAPISchemaObjectTypeMember('type', 'QType', False)
> >  self._def_entity(
> > diff --git a/tests/qapi-schema/qapi-schema-test.json 
> > b/tests/qapi-schema/qapi-schema-test.json
> > index 6d3c6c0b53..df3edf9d89 100644
> > --- a/tests/qapi-schema/qapi-schema-test.json
> > +++ b/tests/qapi-schema/qapi-schema-test.json
> > @@ -216,9 +216,13 @@
> >  { 'command': 'TestIfUnionCmd', 'data': { 'union_cmd_arg': 'TestIfUnion' },
> >'if': 'defined(TEST_IF_UNION)' }
> >
> > -{ 'alternate': 'TestIfAlternate', 'data': { 'foo': 'int', 'bar': 
> > 'TestStruct' },
> > +{ 'alternate': 'TestIfAlternate', 'data':
> > +  { 'foo': 'int', 'alt_bar': { 'type': 'TestStruct', 'if': 
> > 'defined(TEST_IF_ALT_BAR)'} },
>
> Let's break the long line betwen the members.

ok

>
> Why rename member 'bar' to 'alt_bar'?

no reason I remember, dropped

>
> >'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' }
> >
> > +{ 'command': 'TestIfAlternateCmd', 'data': { 'alt_cmd_arg': 
> > 'TestIfAlternate' },
>
> Another long line.  And I'm feeling dense again: why does this change
> belong to this patch?

No reason I remember, let's split it

>
> > +  'if': 'defined(TEST_IF_ALT)' }
> > +
> >  { 'command': 'TestIfCmd', 'data':
> >{ 'foo': 'TestIfStruct',
> >  'bar': { 'type': 'TestIfEnum', 'if': 'defined(TEST_IF_CMD_BAR)' } },
> > diff --git a/tests/qapi-schema/qapi-schema-test.out 
> > b/tests/qapi-schema/qapi-schema-test.out
> > index ac1069cf1f..cdbd5b87cc 100644
> > --- a/tests/qapi-schema/qapi-schema-test.out
> > +++ b/tests/qapi-schema/qapi-schema-test.out
> > @@ -297,8 +297,15 @@ command TestIfUnionCmd q_obj_TestIfUnionCmd-arg -> None
> >  alternate TestIfAlternate
> >  tag type
> >  case foo: int
> > -case bar: TestStruct
> > +case alt_bar: TestStruct
> > +if ['defined(TEST_IF_ALT_BAR)']
> >  if ['defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)']
> > +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
> > +if ['defined(TEST_IF_ALT)']
> >  object q_obj_TestIfCmd-arg
> >  member foo: TestIfStruct optional=False
> >  member bar: TestIfEnum optio

Re: [Qemu-devel] [PATCH v6 19/27] qapi: add 'if' on union members

[Marc-André Lureau]

Hi
On Thu, Dec 6, 2018 at 8:37 PM Markus Armbruster  wrote:
>
> In the subject, s/ on / to /.
>
> Marc-André Lureau  writes:
>
> > Add 'if' key to union members:
> >
> > { 'union': 'TestIfUnion', 'data':
> > 'mem': { 'type': 'str', 'if': 'COND'} }
> >
> > Generated code is not changed by this patch but with "qapi: add #if
> > conditions to generated code".
>
> My suggestion on PATCH 13's commit message applies.

ok

>
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  scripts/qapi/common.py  | 17 +
> >  tests/qapi-schema/qapi-schema-test.json |  7 ++-
> >  tests/qapi-schema/qapi-schema-test.out  | 10 ++
> >  tests/qapi-schema/test-qapi.py  |  1 +
> >  4 files changed, 26 insertions(+), 9 deletions(-)
> >
> > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > index 13fbb28493..e1bd9a22ba 100644
> > --- a/scripts/qapi/common.py
> > +++ b/scripts/qapi/common.py
> > @@ -813,7 +813,7 @@ def check_union(expr, info):
> >  for (key, value) in members.items():
> >  check_name(info, "Member of union '%s'" % name, key)
> >  source = "member '%s' of union '%s'" % (key, name)
> > -check_known_keys(info, source, value, ['type'], [])
> > +check_known_keys(info, source, value, ['type'], ['if'])
> >  typ = value['type']
> >
> >  # Each value must name a known type
> > @@ -1496,8 +1496,8 @@ class QAPISchemaObjectTypeVariants(object):
> >  class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
> >  role = 'branch'
> >
> > -def __init__(self, name, typ):
> > -QAPISchemaObjectTypeMember.__init__(self, name, typ, False)
> > +def __init__(self, name, typ, ifcond=None):
> > +QAPISchemaObjectTypeMember.__init__(self, name, typ, False, ifcond)
> >
> >
> >  class QAPISchemaAlternateType(QAPISchemaType):
> > @@ -1777,14 +1777,14 @@ class QAPISchema(object):
> >  def _make_variant(self, case, typ):
> >  return QAPISchemaObjectTypeVariant(case, typ)
> >
> > -def _make_simple_variant(self, case, typ, info):
> > +def _make_simple_variant(self, case, typ, ifcond, info):
> >  if isinstance(typ, list):
> >  assert len(typ) == 1
> >  typ = self._make_array_type(typ[0], info)
> >  typ = self._make_implicit_object_type(
> >  typ, info, None, self.lookup_type(typ),
> >  'wrapper', [self._make_member('data', typ, None, info)])
> > -return QAPISchemaObjectTypeVariant(case, typ)
> > +return QAPISchemaObjectTypeVariant(case, typ, ifcond)
> >
> >  def _def_union_type(self, expr, info, doc):
> >  name = expr['union']
> > @@ -1802,10 +1802,11 @@ class QAPISchema(object):
> >  for (key, value) in data.items()]
> >  members = []
> >  else:
> > -variants = [self._make_simple_variant(key, value['type'], info)
> > +variants = [self._make_simple_variant(key, value['type'],
> > +  value.get('if'), info)
> >  for (key, value) in data.items()]
> > -typ = self._make_implicit_enum_type(name, info, ifcond,
> > -[v.name for v in variants])
> > +enum = [{'name': v.name, 'if': v.ifcond} for v in variants]
> > +typ = self._make_implicit_enum_type(name, info, ifcond, enum)
> >  tag_member = QAPISchemaObjectTypeMember('type', typ, False)
> >  members = [tag_member]
> >  self._def_entity(
> > diff --git a/tests/qapi-schema/qapi-schema-test.json 
> > b/tests/qapi-schema/qapi-schema-test.json
> > index 3bf440aab4..6d3c6c0b53 100644
> > --- a/tests/qapi-schema/qapi-schema-test.json
> > +++ b/tests/qapi-schema/qapi-schema-test.json
> > @@ -208,9 +208,14 @@
> >[ 'foo', { 'name' : 'bar', 'if': 'defined(TEST_IF_ENUM_BAR)' } ],
> >'if': 'defined(TEST_IF_ENUM)' }
> >
> > -{ 'union': 'TestIfUnion', 'data': { 'foo': 'TestStruct' },
> > +{ 'union': 'TestIfUnion', 'data':
> > +  { 'foo': 'TestStruct',
> > +'union_bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} },
>
> "Union Bar" sounds like a cocktail lounge in northeast US :)
>
> Back to serious: this is a simple union.  I'd prefer to test flat
> unions.  Changing TestIfUnion accordingly could be done either before
> this patch, or on top of this series, the latter not necessarily by
> you.  Your choice.

I prefer to defer

>
> >'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' }
> >
> > +{ 'command': 'TestIfUnionCmd', 'data': { 'union_cmd_arg': 'TestIfUnion' },
> > +  'if': 'defined(TEST_IF_UNION)' }
> > +
>
> I'm feeling dense... why does this change belong to this patch?

Hmm, no strong reason, I'll make it a separate commit.
>
> >  { 'alternate': 'TestIfAlternate', 'data': { 'foo': 'int', 'bar': 
> > 'TestStruct' },
> >'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)'

Re: [Qemu-devel] [PATCH for-4.0 v7 00/27] Hi,

[Marc-André Lureau]

Hi,

The cover letter title should be "qapi: add #if pre-processor
conditions to generated code (part 2)"


On Sat, Dec 8, 2018 at 3:16 PM Marc-André Lureau
 wrote:
>
> This is the second part of the "add #if pre-processor conditions to
> generated code" series, adding schema member conditions (roughly
> 16-38/49).
>
> Members can be exploded as dictionnary with 'type'/'if' keys:
>
> { 'struct': 'TestIfStruct', 'data':
>   { 'foo': 'int',
> 'bar': { 'type': 'int', 'if': 'defined(TEST_IF_STRUCT_BAR)'} } }
>
> Enum values can be exploded as dictionnary with 'type'/'if' keys:
>
> { 'enum': 'TestIfEnum', 'data':
>   [ 'foo',
> { 'name' : 'bar', 'if': 'defined(TEST_IF_ENUM_BAR)' } ] }
>
> v7:
> - replaced osdep.h include by a #warning check
> - renamed event_names/event_enum_members, enum_name/event_enum_ename
> - move normalization after checking
> - simplify make_enum_members()
> - some tests changes
> - rebase/conflicts tweaks
> - squash documentation changes in related patches
> - tweak generated documentation code & output
> - modify commit comments, add rb tags
>
> v6:
> - subset of v5 series: add schema member conditions
> - split "qapi: factor out check_known_keys()", error report improvements
> - add a patch to "qapi: include osdep.h in type headers" to avoid
>   potential build configuration change issues
> - rebased, on top of Markus's qapi-next branch (first 4 patches,
>   included for patchew testing)
>
> Marc-André Lureau (27):
>   qapi: make sure osdep.h is included in type headers
>   qapi: do not define enumeration value explicitly
>   qapi: rename QAPISchemaEnumType.values to .members
>   qapi: change enum visitor and gen_enum* to take QAPISchemaMember
>   tests: print enum type members more like object type members
>   qapi: factor out checking for keys
>   qapi: improve reporting of unknown or missing keys
>   qapi: add a dictionary form with 'name' key for enum members
>   qapi: add 'if' to enum members
>   qapi-events: add 'if' condition to implicit event enum
>   qapi: pass long form enum to make_enum_members
>   qapi: rename allow_dict to allow_implicit
>   qapi: add a dictionary form for TYPE
>   qapi: add 'if' to implicit struct members
>   qapi: add an error in case a discriminator is conditional
>   qapi: add 'if' to union members
>   qapi: simplify make_enum_members()
>   tests/qapi: add command with condition on union argument
>   qapi: add 'if' to alternate members
>   tests/qapi: add command with condition on alternate argument
>   qapi: add #if conditions to generated code members
>   qapi: add 'If:' condition to enum values documentation
>   qapi: add 'If:' condition to struct members documentation
>   qapi: add condition to variants documentation
>   qapi: break long lines at 'data' member
>   qapi: add more conditions to SPICE
>   qapi: add conditions to REPLICATION type/commands on the schema
>
>  qapi/block-core.json  |  26 +-
>  qapi/char.json| 150 ++-
>  qapi/migration.json   |  15 +-
>  qapi/misc.json|   7 +-
>  qapi/net.json |   3 +-
>  qapi/tpm.json |   5 +-
>  qapi/ui.json  |   3 +-
>  scripts/qapi/common.py| 253 --
>  scripts/qapi/doc.py   |  30 ++-
>  scripts/qapi/events.py|  13 +-
>  scripts/qapi/introspect.py|  16 +-
>  scripts/qapi/types.py |  13 +-
>  scripts/qapi/visit.py |   8 +-
>  migration/colo.c  |  16 +-
>  monitor.c |   5 -
>  docs/devel/qapi-code-gen.txt  |  19 ++
>  tests/Makefile.include|  11 +-
>  tests/qapi-schema/alternate-base.err  |   1 +
>  tests/qapi-schema/alternate-invalid-dict.err  |   1 +
>  ...ember.exit => alternate-invalid-dict.exit} |   0
>  tests/qapi-schema/alternate-invalid-dict.json |   4 +
>  ...-member.out => alternate-invalid-dict.out} |   0
>  tests/qapi-schema/comments.out|  14 +-
>  tests/qapi-schema/doc-bad-section.out |  13 +-
>  tests/qapi-schema/doc-good.json   |  11 +-
>  tests/qapi-schema/doc-good.out|  22 +-
>  tests/qapi-schema/doc-good.texi   |   7 +-
>  tests/qapi-schema/double-type.err |   1 +
>  tests/qapi-schema/empty.out   |   9 +-
>  tests/qapi-schema/enum-bad-member.err |   1 +
>  tests/qapi-schema/enum-bad-member.exit|   1 +
>  tests/qapi-schema/enum-bad-member.json|   2 +
>  tests/qapi-schema/enum-bad-member.out |   0
>  .../qapi-schema/enum-dict-member-unknown.err  |   2 +
>  .../qapi-schema/enum-dict-member-unknown.exit |   1 +
>  .../qapi-schema/enum-dict-member-unknown.json |   2 +
>  .../qapi-schema/

[Qemu-devel] [PATCH for-4.0 v7 25/27] qapi: break long lines at 'data' member

[Marc-André Lureau]

Let's break the line before 'data'. While at it, improve a bit
indentation/spacing. (I removed some alignment which are not helping
much readability and become quickly inconsistent)

Suggested-by: Markus Armbruster 
Signed-off-by: Marc-André Lureau 
---
 qapi/block-core.json |  13 ++--
 qapi/char.json   | 138 +--
 qapi/migration.json  |   3 +-
 qapi/misc.json   |   7 ++-
 qapi/net.json|   3 +-
 qapi/tpm.json|   5 +-
 qapi/ui.json |   3 +-
 7 files changed, 101 insertions(+), 71 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index d4fe710836..92e0205d91 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1143,8 +1143,10 @@
 # This command is now obsolete and will always return an error since 2.10
 #
 ##
-{ 'command': 'block_passwd', 'data': {'*device': 'str',
-  '*node-name': 'str', 'password': 'str'} }
+{ 'command': 'block_passwd',
+  'data': { '*device': 'str',
+'*node-name': 'str',
+'password': 'str' } }
 
 ##
 # @block_resize:
@@ -1171,9 +1173,10 @@
 # <- { "return": {} }
 #
 ##
-{ 'command': 'block_resize', 'data': { '*device': 'str',
-   '*node-name': 'str',
-   'size': 'int' }}
+{ 'command': 'block_resize',
+  'data': { '*device': 'str',
+'*node-name': 'str',
+'size': 'int' } }
 
 ##
 # @NewImageMode:
diff --git a/qapi/char.json b/qapi/char.json
index 79bac598a0..24628331ec 100644
--- a/qapi/char.json
+++ b/qapi/char.json
@@ -25,9 +25,10 @@
 #
 # Since: 0.14.0
 ##
-{ 'struct': 'ChardevInfo', 'data': {'label': 'str',
-  'filename': 'str',
-  'frontend-open': 'bool'} }
+{ 'struct': 'ChardevInfo',
+  'data': { 'label': 'str',
+'filename': 'str',
+'frontend-open': 'bool' } }
 
 ##
 # @query-chardev:
@@ -152,7 +153,8 @@
 #
 ##
 { 'command': 'ringbuf-write',
-  'data': {'device': 'str', 'data': 'str',
+  'data': { 'device': 'str',
+'data': 'str',
'*format': 'DataFormat'} }
 
 ##
@@ -202,8 +204,9 @@
 #
 # Since: 2.6
 ##
-{ 'struct': 'ChardevCommon', 'data': { '*logfile': 'str',
-   '*logappend': 'bool' } }
+{ 'struct': 'ChardevCommon',
+  'data': { '*logfile': 'str',
+'*logappend': 'bool' } }
 
 ##
 # @ChardevFile:
@@ -217,9 +220,10 @@
 #
 # Since: 1.4
 ##
-{ 'struct': 'ChardevFile', 'data': { '*in' : 'str',
-   'out' : 'str',
-   '*append': 'bool' },
+{ 'struct': 'ChardevFile',
+  'data': { '*in': 'str',
+'out': 'str',
+'*append': 'bool' },
   'base': 'ChardevCommon' }
 
 ##
@@ -232,7 +236,8 @@
 #
 # Since: 1.4
 ##
-{ 'struct': 'ChardevHostdev', 'data': { 'device' : 'str' },
+{ 'struct': 'ChardevHostdev',
+  'data': { 'device': 'str' },
   'base': 'ChardevCommon' }
 
 ##
@@ -260,15 +265,16 @@
 #
 # Since: 1.4
 ##
-{ 'struct': 'ChardevSocket', 'data': { 'addr'   : 'SocketAddressLegacy',
- '*tls-creds'  : 'str',
- '*server': 'bool',
- '*wait'  : 'bool',
- '*nodelay'   : 'bool',
- '*telnet': 'bool',
- '*tn3270': 'bool',
- '*websocket' : 'bool',
- '*reconnect' : 'int' },
+{ 'struct': 'ChardevSocket',
+  'data': { 'addr': 'SocketAddressLegacy',
+'*tls-creds': 'str',
+'*server': 'bool',
+'*wait': 'bool',
+'*nodelay': 'bool',
+'*telnet': 'bool',
+'*tn3270': 'bool',
+'*websocket': 'bool',
+'*reconnect': 'int' },
   'base': 'ChardevCommon' }
 
 ##
@@ -281,8 +287,9 @@
 #
 # Since: 1.5
 ##
-{ 'struct': 'ChardevUdp', 'data': { 'remote' : 'SocketAddressLegacy',
-  '*local' : 'SocketAddressLegacy' },
+{ 'struct': 'ChardevUdp',
+  'data': { 'remote': 'SocketAddressLegacy',
+'*local': 'SocketAddressLegacy' },
   'base': 'ChardevCommon' }
 
 ##
@@ -294,7 +301,8 @@
 #
 # Since: 1.5
 ##
-{ 'struct': 'ChardevMux', 'data': { 'chardev' : 'str' },
+{ 'struct': 'ChardevMux',
+  'data': { 'chardev': 'str' },
   'base': 'ChardevCommon' }
 
 ##
@@ -308,7 +316,8 @@
 #
 # Since: 1.5
 ##
-{ 'struct': 'ChardevStdio', 'data': { '*signal' : 'bool' },
+{ 'struct': 'ChardevStdio',
+  'data': { '*signal': 'bool' },
   'base': 'ChardevCommon' }
 
 
@@ -321,7 +330,8 @@
 #
 # Since: 1.5
 ##
-{ 'struct': 'ChardevSpiceChannel', 'data': { 'type'  : 'str' },
+{ 'struct': 'ChardevSpiceChannel',
+  'data': { 'type': 'str' },
   'base': 'ChardevCommon' }
 # TODO: 'if': 'defined(CONFIG_SPICE)'
 
@@ -334,7 +344

Re: [Qemu-devel] [PATCH v6 06/27] qapi: do not define enumeration value explicitly

[Marc-André Lureau]

Hi
On Wed, Dec 5, 2018 at 5:19 PM Markus Armbruster  wrote:
>
> Marc-André Lureau  writes:
>
> > The C standard has the initial value at 0 and the subsequent values
> > incremented by 1. No need to set this explicitely.
> >
> > This will prevent from artificial "gaps" when compiling out some enum
> > values and having unnecessarily large MAX values & enums arrays, or
> > simplifying iterating over valid enum values.
>
> Yes, gaps can be annoying, e.g. in lookup tables where gaps get confused
> with sentinels.
>
> Avoiding gaps scares me.  You have to religiously compile the enum with
> the exact same configuration everywhere in the program, or else you end
> up with bugs that are hard to spot.  Therefore:
>
> > Whenever config-host.h is changed, all the enum/types are recompiled.
> >
> > (a subsequent patch will split the schema. Target-specific poisoined
> > conditionals will be added. They cannot be mixed with the common
> > schema: it is not possible to end up with enums of different values in
> > common and target builds)
>
> Should we mention config-target.h here?
>

My memory isn't clear. I'll drop that comment from here, we will
discuss the poisoned conditionals again with the patch splitting the
schema.

> > Signed-off-by: Marc-André Lureau 
> > ---
> >  scripts/qapi/common.py | 7 ++-
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > index 02c5c6767a..6f9498566e 100644
> > --- a/scripts/qapi/common.py
> > +++ b/scripts/qapi/common.py
> > @@ -2045,14 +2045,11 @@ typedef enum %(c_name)s {
> >  ''',
> >  c_name=c_name(name))
> >
> > -i = 0
> >  for value in enum_values:
> >  ret += mcgen('''
> > -%(c_enum)s = %(i)d,
> > +%(c_enum)s,
> >  ''',
> > - c_enum=c_enum_const(name, value, prefix),
> > - i=i)
> > -i += 1
> > + c_enum=c_enum_const(name, value, prefix))
> >
> >  ret += mcgen('''
> >  } %(c_name)s;
>
> I need to consider the whole series to decide whether avoiding gaps is a
> good idea.  If it is, then this patch is fine.

Re: [Qemu-devel] [PATCH v6 15/27] qapi: rename allow_dict to allow_implicit

[Marc-André Lureau]

On Wed, Dec 5, 2018 at 10:41 PM Markus Armbruster  wrote:
>
> Marc-André Lureau  writes:
>
> > This makes it a bit clearer what is the intent of the dictionnary for
>
> dictionary

sigh, this must be a very common misspell (dictionnaire in french)

>
> > the check_type() function, since there was some confusion on a
> > previous iteration of this series.
>
> I don't remember... got a pointer to that confusion handy?

https://lists.nongnu.org/archive/html/qemu-devel/2017-12/msg01584.html

>
> > Suggested-by: Markus Armbruster 
> > Signed-off-by: Marc-André Lureau 

Re: [Qemu-devel] [PATCH v6 21/27] qapi: add #if conditions to generated code members

[Marc-André Lureau]

Hi
On Thu, Dec 6, 2018 at 9:42 PM Markus Armbruster  wrote:
>
> Marc-André Lureau  writes:
>
> > Wrap generated enum/struct members and code with #if/#endif, using the
>
> enum and struct members

ok

>
> > .ifcond members added in the previous patches.
> >
> > Some types generate both enum and struct members for example, so a
> > step-by-step is unnecessarily complicated to deal with (it would
> > easily generate invalid intermediary code).
>
> Can you give an example of a schema definition that would lead to
> complications?
>

Honestly, I don't remember well (it's been a while I wrote that code).
It must be related to implicit enums, such as union kind... If there
is no strong need to split this patch, I would rather not do that
extra work.

> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  scripts/qapi/common.py |  4 
> >  scripts/qapi/introspect.py | 13 +
> >  scripts/qapi/types.py  |  4 
> >  scripts/qapi/visit.py  |  6 ++
> >  4 files changed, 23 insertions(+), 4 deletions(-)
> >
> > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > index b3b64a60bf..a66c035b91 100644
> > --- a/scripts/qapi/common.py
> > +++ b/scripts/qapi/common.py
> > @@ -2098,11 +2098,13 @@ const QEnumLookup %(c_name)s_lookup = {
> >  ''',
> >  c_name=c_name(name))
> >  for m in members:
> > +ret += gen_if(m.ifcond)
> >  index = c_enum_const(name, m.name, prefix)
> >  ret += mcgen('''
> >  [%(index)s] = "%(name)s",
> >  ''',
> >   index=index, name=m.name)
> > +ret += gen_endif(m.ifcond)
> >
> >  ret += mcgen('''
> >  },
> > @@ -2124,10 +2126,12 @@ typedef enum %(c_name)s {
> >  c_name=c_name(name))
> >
> >  for m in enum_members:
> > +ret += gen_if(m.ifcond)
> >  ret += mcgen('''
> >  %(c_enum)s,
> >  ''',
> >   c_enum=c_enum_const(name, m.name, prefix))
> > +ret += gen_endif(m.ifcond)
> >
> >  ret += mcgen('''
> >  } %(c_name)s;
> > diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
> > index 3f1ca99f6d..bf5db51f4a 100644
> > --- a/scripts/qapi/introspect.py
> > +++ b/scripts/qapi/introspect.py
> > @@ -148,6 +148,8 @@ const QLitObject %(c_name)s = %(c_string)s;
> >  ret = {'name': member.name, 'type': self._use_type(member.type)}
> >  if member.optional:
> >  ret['default'] = None
> > +if member.ifcond:
> > +ret = (ret, member.ifcond)
> >  return ret
> >
> >  def _gen_variants(self, tag_name, variants):
> > @@ -155,14 +157,16 @@ const QLitObject %(c_name)s = %(c_string)s;
> >  'variants': [self._gen_variant(v) for v in variants]}
> >
> >  def _gen_variant(self, variant):
> > -return {'case': variant.name, 'type': self._use_type(variant.type)}
> > +return ({'case': variant.name, 'type': 
> > self._use_type(variant.type)},
> > +variant.ifcond)
>
> Looks different in your rebased version at
> https://github.com/elmarco/qemu.git branch qapi-if.  I'm only skimming
> this version.
>
> Note to self: always creates the tuple form, even when there's no
> if-condition.  Differs from _gen_qlit(), which creates a tuple only when
> there's a if-condition.  Not wrong, just a bit inconsistent.
>
> >
> >  def visit_builtin_type(self, name, info, json_type):
> >  self._gen_qlit(name, 'builtin', {'json-type': json_type}, [])
> >
> >  def visit_enum_type(self, name, info, ifcond, members, prefix):
> >  self._gen_qlit(name, 'enum',
> > -   {'values': [m.name for m in members]}, ifcond)
> > +   {'values': [(m.name, m.ifcond) for m in members]},
> > +   ifcond)
>
> Likewise.
>
> >
> >  def visit_array_type(self, name, info, ifcond, element_type):
> >  element = self._use_type(element_type)
> > @@ -178,8 +182,9 @@ const QLitObject %(c_name)s = %(c_string)s;
> >
> >  def visit_alternate_type(self, name, info, ifcond, variants):
> >  self._gen_qlit(name, 'alternate',
> > -   {'members': [{'type': self._use_type(m.type)}
> > -for m in variants.variants]}, ifcond)
> > +   {'members': [
> > +   ({'type': self._use_type(m.type)}, m.ifcond)
> > +   for m in variants.variants]}, ifcond)
>
> Likewise.
>
> >
> >  def visit_command(self, name, info, ifcond, arg_type, ret_type, gen,
> >success_response, boxed, allow_oob, allow_preconfig):
> > diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
> > index 2d4a70f810..7d9eef6320 100644
> > --- a/scripts/qapi/types.py
> > +++ b/scripts/qapi/types.py
> > @@ -43,6 +43,7 @@ struct %(c_name)s {
> >  def gen_struct_members(members):
> >  ret = ''
> >  for memb in members:
> > +ret += gen_if(memb.ifcond)

Re: [Qemu-devel] [PATCH v6 12/27] qapi: add a dictionnary form with 'name' key for enum members

[Marc-André Lureau]

Hi
On Wed, Dec 5, 2018 at 10:02 PM Markus Armbruster  wrote:
>
> In subject, s/dictionnary/dictionary/
>
> Marc-André Lureau  writes:
>
> > Desugar the enum NAME form to { 'name': NAME }. This will allow to add
> > new enum members, such as 'if' in the following patch.
> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  scripts/qapi/common.py| 47 ---
> >  tests/Makefile.include|  3 +-
> >  tests/qapi-schema/enum-bad-member.err |  1 +
> >  ...-dict-member.exit => enum-bad-member.exit} |  0
> >  tests/qapi-schema/enum-bad-member.json|  2 +
> >  ...um-dict-member.out => enum-bad-member.out} |  0
> >  .../qapi-schema/enum-dict-member-unknown.err  |  2 +
> >  .../qapi-schema/enum-dict-member-unknown.exit |  1 +
> >  .../qapi-schema/enum-dict-member-unknown.json |  2 +
> >  .../qapi-schema/enum-dict-member-unknown.out  |  0
> >  tests/qapi-schema/enum-dict-member.err|  1 -
> >  tests/qapi-schema/enum-dict-member.json   |  2 -
> >  tests/qapi-schema/enum-missing-data.err   |  2 +-
> >  13 files changed, 51 insertions(+), 12 deletions(-)
> >  create mode 100644 tests/qapi-schema/enum-bad-member.err
> >  rename tests/qapi-schema/{enum-dict-member.exit => enum-bad-member.exit} 
> > (100%)
> >  create mode 100644 tests/qapi-schema/enum-bad-member.json
> >  rename tests/qapi-schema/{enum-dict-member.out => enum-bad-member.out} 
> > (100%)
> >  create mode 100644 tests/qapi-schema/enum-dict-member-unknown.err
> >  create mode 100644 tests/qapi-schema/enum-dict-member-unknown.exit
> >  create mode 100644 tests/qapi-schema/enum-dict-member-unknown.json
> >  create mode 100644 tests/qapi-schema/enum-dict-member-unknown.out
> >  delete mode 100644 tests/qapi-schema/enum-dict-member.err
> >  delete mode 100644 tests/qapi-schema/enum-dict-member.json
> >
> > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > index cf8dab2866..e9fb736d46 100644
> > --- a/scripts/qapi/common.py
> > +++ b/scripts/qapi/common.py
> > @@ -740,6 +740,10 @@ def check_event(expr, info):
> > allow_metas=meta)
> >
> >
> > +def enum_get_names(expr):
> > +return [e['name'] for e in expr['data']]
> > +
> > +
> >  def check_union(expr, info):
> >  name = expr['union']
> >  base = expr.get('base')
> > @@ -799,7 +803,7 @@ def check_union(expr, info):
> >  # If the discriminator names an enum type, then all members
> >  # of 'data' must also be members of the enum type.
> >  if enum_define:
> > -if key not in enum_define['data']:
> > +if key not in enum_get_names(enum_define):
> >  raise QAPISemError(info,
> > "Discriminator value '%s' is not found 
> > in "
> > "enum '%s'"
> > @@ -831,10 +835,10 @@ def check_alternate(expr, info):
> >  if qtype == 'QTYPE_QSTRING':
> >  enum_expr = enum_types.get(value)
> >  if enum_expr:
> > -for v in enum_expr['data']:
> > +for v in enum_get_names(enum_expr):
> >  if v in ['on', 'off']:
> >  conflicting.add('QTYPE_QBOOL')
> > -if re.match(r'[-+0-9.]', v): # lazy, could be tightened
> > +if re.match(r'[-+0-9.]', v):  # lazy, could be 
> > tightened
>
> Unrelated change.
>

ok

> >  conflicting.add('QTYPE_QNUM')
> >  else:
> >  conflicting.add('QTYPE_QNUM')
>
> The helper enum_get_names() is used just twice.
>
> The second instance
>
> for v in enum_get_names(enum_expr):
>
> could just as well iterate over enum_expr
>
> for d in enum_expr['data']:
> v = d['name']
>
> The first instance could simply be inlined then.

well, with unnormalized form (your request below), this becomes even
less readable. I'll keep it, even if it's a one-liner, I much prefer
an explicit function name and common code.

>
> > @@ -847,19 +851,34 @@ def check_alternate(expr, info):
> >  types_seen[qt] = key
> >
> >
> > -def check_enum(expr, info):
> > +def normalize_enum(expr, info):
> >  name = expr['enum']
> >  members = expr.get('data')
> > -prefix = expr.get('prefix')
> >
> >  if not isinstance(members, list):
> >  raise QAPISemError(info,
> > "Enum '%s' requires an array for 'data'" % name)
> > +
> > +# translate short member form to dict form
> > +for i, member in enumerate(members):
> > +if not isinstance(member, dict):
> > +member = {'name': member}
> > +members[i] = member
>
> Simpler:
>
>members = [m if isinstance(m, dict) else {'name': m} for m in members]
>

Yes, except that this doesn't change the dict in place. No pb, updated.

> > +
> > +
> > +def check_enum(expr, info):
> > +name = expr['enum']
> > +m

Re: [Qemu-devel] [PATCH v6 16/27] qapi: add a dictionary form with 'type' key for members

[Marc-André Lureau]

Hi

On Thu, Dec 6, 2018 at 7:56 PM Markus Armbruster  wrote:
>
> Marc-André Lureau  writes:
>
> > Wherever a struct/union/alternate/command/event member with NAME: TYPE
> > form is accepted, desugar it to a NAME: { 'type': TYPE } form.
> >
> > This will allow to add new member details, such as 'if' in the
> > following patch to introduce conditionals, or 'default' for default
> > values etc.
> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  scripts/qapi/common.py| 56 +--
> >  tests/Makefile.include|  3 +
> >  tests/qapi-schema/alternate-invalid-dict.err  |  1 +
> >  tests/qapi-schema/alternate-invalid-dict.exit |  1 +
> >  tests/qapi-schema/alternate-invalid-dict.json |  4 ++
> >  tests/qapi-schema/alternate-invalid-dict.out  |  0
> >  tests/qapi-schema/event-nest-struct.err   |  2 +-
> >  tests/qapi-schema/flat-union-inline.err   |  2 +-
> >  tests/qapi-schema/nested-struct-data.err  |  2 +-
> >  tests/qapi-schema/qapi-schema-test.json   | 10 ++--
> >  .../struct-member-invalid-dict.err|  1 +
> >  .../struct-member-invalid-dict.exit   |  1 +
> >  .../struct-member-invalid-dict.json   |  3 +
> >  .../struct-member-invalid-dict.out|  0
> >  .../qapi-schema/union-branch-invalid-dict.err |  1 +
> >  .../union-branch-invalid-dict.exit|  1 +
> >  .../union-branch-invalid-dict.json|  4 ++
> >  .../qapi-schema/union-branch-invalid-dict.out |  0
> >  18 files changed, 66 insertions(+), 26 deletions(-)
> >  create mode 100644 tests/qapi-schema/alternate-invalid-dict.err
> >  create mode 100644 tests/qapi-schema/alternate-invalid-dict.exit
> >  create mode 100644 tests/qapi-schema/alternate-invalid-dict.json
> >  create mode 100644 tests/qapi-schema/alternate-invalid-dict.out
> >  create mode 100644 tests/qapi-schema/struct-member-invalid-dict.err
> >  create mode 100644 tests/qapi-schema/struct-member-invalid-dict.exit
> >  create mode 100644 tests/qapi-schema/struct-member-invalid-dict.json
> >  create mode 100644 tests/qapi-schema/struct-member-invalid-dict.out
> >  create mode 100644 tests/qapi-schema/union-branch-invalid-dict.err
> >  create mode 100644 tests/qapi-schema/union-branch-invalid-dict.exit
> >  create mode 100644 tests/qapi-schema/union-branch-invalid-dict.json
> >  create mode 100644 tests/qapi-schema/union-branch-invalid-dict.out
> >
> > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > index d83fa1900e..fc68bb472e 100644
> > --- a/scripts/qapi/common.py
> > +++ b/scripts/qapi/common.py
> > @@ -588,11 +588,11 @@ def discriminator_find_enum_define(expr):
> >  if not base_members:
> >  return None
> >
> > -discriminator_type = base_members.get(discriminator)
> > -if not discriminator_type:
> > +discriminator_member = base_members.get(discriminator)
> > +if not discriminator_member:
> >  return None
> >
> > -return enum_types.get(discriminator_type)
> > +return enum_types.get(discriminator_member['type'])
> >
> >
>
> The name discriminator_member is confusing.  It suggests its value is
> the (desugared) member definition, i.e. something like
>
>  {'enum1': {'type': 'EnumOne'}}
>
> it's actually only the definition's value part, i.e. something like
>
>  {'type': 'EnumOne'}
>
> Naming is hard... discriminator_value?  discriminator_def_rhs?
>
> Same in check_union() below.
>

discriminator_value, ok

> >  # Names must be letters, numbers, -, and _.  They must start with letter,
> > @@ -660,6 +660,15 @@ def check_if(expr, info):
> >  check_if_str(ifcond, info)
> >
> >
> > +def normalize_members(expr, field):
> > +members = expr.get(field)
> > +if isinstance(members, OrderedDict):
> > +for key, arg in members.items():
> > +if isinstance(arg, dict):
> > +continue
> > +members[key] = {'type': arg}
> > +
> > +
>
> PATCH 12's normalize_enum() conflates normalization and checking.  This
> one doesn't.  Better.

I fixed patch 12

>
> >  def check_type(info, source, value, allow_array=False,
> > allow_implicit=False, allow_optional=False,
> > allow_metas=[]):
> > @@ -704,8 +713,10 @@ def check_type(info, source, value, allow_array=False,
> > % (source, key))
> >  # Todo: allow dictionaries to represent default values of
> >  # an optional argument.
> > -check_type(info, "Member '%s' of %s" % (key, source), arg,
> > -   allow_array=True,
> > +check_known_keys(info, "member '%s' of %s" % (key, source),
> > + arg, ['type'], [])
> > +check_type(info, "Member '%s' of %s" % (key, source),
> > +   arg['type'], allow_array=True,
> > allow_metas=['built-in', 'union', 'alternate', 'struct',
> >  'enum'])
> >
> > @@ -776,13 +787,13 @@ def c

Re: [Qemu-devel] [PATCH v2 14/18] xen: add implementations of xen-block connect and disconnect functions...

[Paul Durrant]

> -Original Message-
> From: Anthony PERARD [mailto:anthony.per...@citrix.com]
> Sent: 07 December 2018 18:21
> To: Paul Durrant 
> Cc: qemu-devel@nongnu.org; qemu-bl...@nongnu.org; xen-
> de...@lists.xenproject.org; Stefano Stabellini ;
> Kevin Wolf ; Max Reitz 
> Subject: Re: [PATCH v2 14/18] xen: add implementations of xen-block
> connect and disconnect functions...
> 
> On Thu, Dec 06, 2018 at 03:08:40PM +, Paul Durrant wrote:
> > ...and wire in the dataplane.
> >
> > This patch adds the remaining code to make the xen-block XenDevice
> > functional. The parameters that a block frontend expects to find are
> > populated in the backend xenstore area, and the 'ring-ref' and
> > 'event-channel' values specified in the frontend xenstore area are
> > mapped/bound and used to set up the dataplane.
> >
> > Signed-off-by: Paul Durrant 
> 
> With this patch, we should be able to have QEMU instantiate a new
> backend for a guest, right ? (via command line or QMP)
> 
> I've tried, and that doesn't work, the xenstore path for the frontend is
> wrong. In the qemu trace, I have:
> xs_node_create /local/domain/0/backend/xen-disk/23/268572709
> Which is probably fine, even if not described in xenstore-paths.markdown.
> xs_node_create /local/domain/23/device/xen-disk/268572709
> Which is not, instead of "xen-disk", we should have "vbd".
> 
> I know that this is fixed in "xen: automatically create
> XenBlockDevice-s", but at least the "vbd" type couldn't be added in this
> patch, or maybe a previous one.


Yes, I guess I should move the name overrides into an earlier patch.

> 
> 
> Another issue seems to be error handling. I've done a very simple test,
> I've added '-device xen-disk,vdev=d536p37,id=mydisk' to the command
> line (which is obvious wrong), and QEMU abort with:
> qemu-system-i386: hw/block/xen-block.c:174: xen_block_realize:
> Assertion `conf->blk' failed.
> But I've pointed out the error in the code below.
> 
> 
> And just for fun, adding then removing a xen-disk via QMP. Adding works
> fine (once I've fixed the frontend name). I've run the following with
> ./scripts/qmp/qmp-shell:
> blockdev-add driver=file  filename=/root/vm/disk/testing-disk.qcow2 node-
> name=emptyfile
> blockdev-add driver=qcow2 node-name=emptyqcow2 file=emptyfile
> device_add driver=xen-disk vdev=xvdn id=fromqmp drive=emptyqcow2
> 
> But, then, remove doesn't work, running "device_del id=fromqmp" doesn't
> do anything. I guess we can try to fix that later if you don't find
> what's missing.

Hmm, that's weird. I guess the name lookup must be failing somewhere.

> 
> > @@ -76,6 +151,7 @@ static void xen_block_realize(XenDevice *xendev,
> Error **errp)
> >  const char *type = object_get_typename(OBJECT(blockdev));
> >  XenBlockVdev *vdev = &blockdev->vdev;
> >  Error *local_err = NULL;
> > +BlockConf *conf = &blockdev->conf;
> >
> >  if (vdev->type == XEN_BLOCK_VDEV_TYPE_INVALID) {
> >  error_setg(errp, "vdev property not set");
> > @@ -90,6 +166,59 @@ static void xen_block_realize(XenDevice *xendev,
> Error **errp)
> >  error_propagate(errp, local_err);
> 
> You probably want to add a return here, this is when
> `blockdev_class->realize' fails.
> 

Yes, I do.

> >  }
> >  }
> > +
> > +/*
> > + * The blkif protocol does not deal with removable media, so it
> must
> > + * always be present, even for CDRom devices.
> > + */
> > +assert(conf->blk);
> 
> That assert should probably not be there, as a missing conf->blk isn't a
> programming error, but a user error, I think.
> 
> Actually, the issue is the missing return abrove, and the assert is
> probably fine.
> 

Yes, the assert is intentional and caught the programming error :-)

  Paul

> --
> Anthony PERARD

[Qemu-devel] [PATCH for-4.0 v7 15/27] qapi: add an error in case a discriminator is conditional

[Marc-André Lureau]

Making a discriminator conditional doesn't make much sense. Instead,
the union could be made conditional.

Signed-off-by: Marc-André Lureau 
---
 scripts/qapi/common.py  |  8 
 tests/Makefile.include  |  1 +
 .../flat-union-invalid-if-discriminator.err |  1 +
 .../flat-union-invalid-if-discriminator.exit|  1 +
 .../flat-union-invalid-if-discriminator.json| 17 +
 .../flat-union-invalid-if-discriminator.out |  0
 6 files changed, 28 insertions(+)
 create mode 100644 tests/qapi-schema/flat-union-invalid-if-discriminator.err
 create mode 100644 tests/qapi-schema/flat-union-invalid-if-discriminator.exit
 create mode 100644 tests/qapi-schema/flat-union-invalid-if-discriminator.json
 create mode 100644 tests/qapi-schema/flat-union-invalid-if-discriminator.out

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index aec51acb07..f79b3fad54 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -578,6 +578,7 @@ def find_alternate_member_qtype(qapi_type):
 # Return the discriminator enum define if discriminator is specified as an
 # enum type, otherwise return None.
 def discriminator_find_enum_define(expr):
+name = expr['union']
 base = expr.get('base')
 discriminator = expr.get('discriminator')
 
@@ -594,6 +595,7 @@ def discriminator_find_enum_define(expr):
 
 if isinstance(discriminator_value, dict):
 discriminator_value = discriminator_value['type']
+
 return enum_types.get(discriminator_value)
 
 
@@ -800,7 +802,12 @@ def check_union(expr, info):
"struct '%s'"
% (discriminator, base))
 if isinstance(discriminator_value, dict):
+if discriminator_value.get('if'):
+raise QAPISemError(info, 'The discriminator %s.%s for union %s 
'
+   'must not be conditional' %
+   (base, discriminator, name))
 discriminator_value = discriminator_value['type']
+
 enum_define = enum_types.get(discriminator_value)
 allow_metas = ['struct']
 # Do not allow string discriminator
@@ -1023,6 +1030,7 @@ def check_exprs(exprs):
 
 if 'include' in expr:
 continue
+info = expr_elem['info']
 if 'union' in expr and not discriminator_find_enum_define(expr):
 name = '%sKind' % expr['union']
 elif 'alternate' in expr:
diff --git a/tests/Makefile.include b/tests/Makefile.include
index ea5d1e8787..3f5a1d0c30 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -409,6 +409,7 @@ qapi-schema += flat-union-inline-invalid-dict.json
 qapi-schema += flat-union-int-branch.json
 qapi-schema += flat-union-invalid-branch-key.json
 qapi-schema += flat-union-invalid-discriminator.json
+qapi-schema += flat-union-invalid-if-discriminator.json
 qapi-schema += flat-union-no-base.json
 qapi-schema += flat-union-optional-discriminator.json
 qapi-schema += flat-union-string-discriminator.json
diff --git a/tests/qapi-schema/flat-union-invalid-if-discriminator.err 
b/tests/qapi-schema/flat-union-invalid-if-discriminator.err
new file mode 100644
index 00..0c94c9860d
--- /dev/null
+++ b/tests/qapi-schema/flat-union-invalid-if-discriminator.err
@@ -0,0 +1 @@
+tests/qapi-schema/flat-union-invalid-if-discriminator.json:13: The 
discriminator TestBase.enum1 for union TestUnion must not be conditional
diff --git a/tests/qapi-schema/flat-union-invalid-if-discriminator.exit 
b/tests/qapi-schema/flat-union-invalid-if-discriminator.exit
new file mode 100644
index 00..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/flat-union-invalid-if-discriminator.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/flat-union-invalid-if-discriminator.json 
b/tests/qapi-schema/flat-union-invalid-if-discriminator.json
new file mode 100644
index 00..618ec36396
--- /dev/null
+++ b/tests/qapi-schema/flat-union-invalid-if-discriminator.json
@@ -0,0 +1,17 @@
+{ 'enum': 'TestEnum',
+  'data': [ 'value1', 'value2' ] }
+
+{ 'struct': 'TestBase',
+  'data': { 'enum1': { 'type': 'TestEnum', 'if': 'FOO' } } }
+
+{ 'struct': 'TestTypeA',
+  'data': { 'string': 'str' } }
+
+{ 'struct': 'TestTypeB',
+  'data': { 'integer': 'int' } }
+
+{ 'union': 'TestUnion',
+  'base': 'TestBase',
+  'discriminator': 'enum1',
+  'data': { 'value1': 'TestTypeA',
+'value2': 'TestTypeB' } }
diff --git a/tests/qapi-schema/flat-union-invalid-if-discriminator.out 
b/tests/qapi-schema/flat-union-invalid-if-discriminator.out
new file mode 100644
index 00..e69de29bb2
-- 
2.20.0.rc1

[Qemu-devel] [PATCH for-4.0 v7 18/27] tests/qapi: add command with condition on union argument

[Marc-André Lureau]

For completeness.

Signed-off-by: Marc-André Lureau 
---
 tests/qapi-schema/qapi-schema-test.json | 3 +++
 tests/qapi-schema/qapi-schema-test.out  | 6 ++
 2 files changed, 9 insertions(+)

diff --git a/tests/qapi-schema/qapi-schema-test.json 
b/tests/qapi-schema/qapi-schema-test.json
index 6e9e4e14d9..0d28475f4c 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -215,6 +215,9 @@
 'union_bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} },
   'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' }
 
+{ 'command': 'TestIfUnionCmd', 'data': { 'union_cmd_arg': 'TestIfUnion' },
+  'if': 'defined(TEST_IF_UNION)' }
+
 { 'alternate': 'TestIfAlternate', 'data': { 'foo': 'int', 'bar': 'TestStruct' 
},
   'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' }
 
diff --git a/tests/qapi-schema/qapi-schema-test.out 
b/tests/qapi-schema/qapi-schema-test.out
index a342ecfc55..aedc668aa4 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -290,6 +290,12 @@ object TestIfUnion
 case union_bar: q_obj_str-wrapper
 if ['defined(TEST_IF_UNION_BAR)']
 if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
+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
+if ['defined(TEST_IF_UNION)']
 alternate TestIfAlternate
 tag type
 case foo: int
-- 
2.20.0.rc1

[Qemu-devel] [BUG]Unassigned mem write during pci device hot-plug

[xuyandong]

Hi all,

In our test, we configured VM with several pci-bridges and a virtio-net nic 
been attached with bus 4,
After VM is startup, We ping this nic from host to judge if it is working 
normally. Then, we hot add pci devices to this VM with bus 0.
We  found the virtio-net NIC in bus 4 is not working (can not connect) 
occasionally, as it kick virtio backend failure with error below:
Unassigned mem write fc803004 = 0x1

memory-region: pci_bridge_pci
  - (prio 0, RW): pci_bridge_pci
fc80-fc803fff (prio 1, RW): virtio-pci
  fc80-fc800fff (prio 0, RW): virtio-pci-common
  fc801000-fc801fff (prio 0, RW): virtio-pci-isr
  fc802000-fc802fff (prio 0, RW): virtio-pci-device
  fc803000-fc803fff (prio 0, RW): virtio-pci-notify  <- io 
mem unassigned
  …

We caught an exceptional address changing while this problem happened, show as 
follow:
Before pci_bridge_update_mappings：
  fc00-fc1f (prio 1, RW): alias pci_bridge_pref_mem 
@pci_bridge_pci fc00-fc1f
  fc20-fc3f (prio 1, RW): alias pci_bridge_pref_mem 
@pci_bridge_pci fc20-fc3f
  fc40-fc5f (prio 1, RW): alias pci_bridge_pref_mem 
@pci_bridge_pci fc40-fc5f
  fc60-fc7f (prio 1, RW): alias pci_bridge_pref_mem 
@pci_bridge_pci fc60-fc7f
  fc80-fc9f (prio 1, RW): alias pci_bridge_pref_mem 
@pci_bridge_pci fc80-fc9f <- correct Adress Spce
  fca0-fcbf (prio 1, RW): alias pci_bridge_pref_mem 
@pci_bridge_pci fca0-fcbf
  fcc0-fcdf (prio 1, RW): alias pci_bridge_pref_mem 
@pci_bridge_pci fcc0-fcdf
  fce0-fcff (prio 1, RW): alias pci_bridge_pref_mem 
@pci_bridge_pci fce0-fcff

After pci_bridge_update_mappings：
  fda0-fdbf (prio 1, RW): alias pci_bridge_mem 
@pci_bridge_pci fda0-fdbf
  fdc0-fddf (prio 1, RW): alias pci_bridge_mem 
@pci_bridge_pci fdc0-fddf
  fde0-fdff (prio 1, RW): alias pci_bridge_mem 
@pci_bridge_pci fde0-fdff
  fe00-fe1f (prio 1, RW): alias pci_bridge_mem 
@pci_bridge_pci fe00-fe1f
  fe20-fe3f (prio 1, RW): alias pci_bridge_mem 
@pci_bridge_pci fe20-fe3f
  fe40-fe5f (prio 1, RW): alias pci_bridge_mem 
@pci_bridge_pci fe40-fe5f
  fe60-fe7f (prio 1, RW): alias pci_bridge_mem 
@pci_bridge_pci fe60-fe7f
  fe80-fe9f (prio 1, RW): alias pci_bridge_mem 
@pci_bridge_pci fe80-fe9f
  fc80-fc80 (prio 1, RW): alias pci_bridge_pref_mem 
@pci_bridge_pci fc80-fc80   <- Exceptional Adress Space

We have figured out why this address becomes this value,  according to pci 
spec,  pci driver can get BAR address size by writing 0x to
the pci register firstly, and then read back the value from this register.
We didn't handle this value  specially while process pci write in qemu, the 
function call stack is:
Pci_bridge_dev_write_config
-> pci_bridge_write_config
-> pci_default_write_config (we update the config[address] value here to 
fc80, which should be 0xfc80 )
-> pci_bridge_update_mappings
->pci_bridge_region_del(br, br->windows);
-> pci_bridge_region_init

->pci_bridge_init_alias (here pci_bridge_get_base, we use the wrong value 
fc80)
-> 
memory_region_transaction_commit

So, as we can see, we use the wrong base address in qemu to update the memory 
regions, though, we update the base address to
The correct value after pci driver in VM write the original value back, the 
virtio NIC in bus 4 may still sends net packets concurrently with
The wrong memory region address.

We have tried to skip the memory region update action in qemu while detect pci 
write with 0x value, and it does work, but
This seems to be not gently.

diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index b2e50c3..84b405d 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -256,7 +256,8 @@ void pci_bridge_write_config(PCIDevice *d,
 pci_default_write_config(d, address, val, len);
-if (ranges_overlap(address, len, PCI_COMMAND, 2) ||
+if ( (val != 0x) &&
+(ranges_overlap(add

Re: [Qemu-devel] [PATCH] cpus.c: Fix race condition in cpu_stop_current()

[Jaap Crezee]

Hi all,

On 12/7/18 4:59 PM, Peter Maydell wrote:
> Jaap: could you test whether this patch fixes the issue you
> were seeing, please?

I have applied the patch and started my test tool against it. It will need some 
time as I have also seen cases where it only failed after 600 reboots.
My testtool logs into the VM over ssh and reboots it. Meanwhile it will check 
the device going offline and coming back online again and starts over
(including a random wait).

I will keep you updated. Thanks!

Jaap

Re: [Qemu-devel] [RFC 48/48] plugin: add a couple of very simple examples

[Emilio G. Cota]

On Thu, Nov 29, 2018 at 23:45:18 +0300, Roman Bolshakov wrote:
> On Thu, Oct 25, 2018 at 01:20:57PM -0400, Emilio G. Cota wrote:
> > +
> > +lib%.so: %.o
> > +   $(CC) -shared -Wl,-soname,$@ -o $@ $^ $(LDLIBS)
> 
> The rule should be a bit different for macOS:
> %.bundle: %.o
>$(CC) -bundle -Wl,-bundle_loader,PATH_TO_QEMU_EXE -o $@ $^ $(LDLIBS)
> 
> "-bundle" flag is needed because macOS has two kinds of Mach-O
> dynamically loaded executables:
>  - dylib (MH_DYLIB) - it's like an ELF shared object used for dynamic
>linking. Can be loaded, preloaded for sake of function interposing
>but cannot be explicitly unloaded.
>  - bundle (MH_BUNDLE) - similar to an ELF shared object used in plugin
>dlopen/dlclose scenario.
> 
> We can pick any (enabled in configure) qemu-system executable as
> bundle_loader. We specify it to check if all symbols in a bundle,
> including the ones coming from the executable are defined. FWIW, here's
> the reference for bundle_loader flag:
>   -bundle_loader executable
>   This specifies the executable that will be loading the bundle
>   output file being linked. Undefined symbols from the bundle are
>   checked against the specified executable like it was one of the
>   dynamic libraries the bundle was linked with
> 
> 
> The ".bundle" extension is not required but IMO it feels more native to
> the system than ".so".

The goal of the examples is to be target-independent, so I'm not
convinced that we want to bury $PATH_TO_QEMU_EXE in the build recipe
(or get configure involved in this).

Since you say the "bundle" business isn't a requirement, I'll leave just
the .so rule in v2.

Thanks,

Emilio