[Qemu-devel] [PATCH RFC 01/14] qlit: use QType instead of int

[Qemu-devel] [PATCH RFC 01/14] qlit: use QType instead of int

[Markus Armbruster]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qapi/qmp/qlit.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
index 56f9d97bd9..f1ed082df8 100644
--- a/include/qapi/qmp/qlit.h
+++ b/include/qapi/qmp/qlit.h
@@ -20,7 +20,7 @@ typedef struct QLitDictEntry QLitDictEntry;
 typedef struct QLitObject QLitObject;
 
 struct QLitObject {
-    int type;
+    QType type;
     union {
         bool qbool;
         int64_t qnum;
-- 
2.13.6

[Qemu-devel] [PATCH RFC 02/14] qlit: add qobject_from_qlit()

[Markus Armbruster]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Instantiate a QObject* from a literal QLitObject.

LitObject only supports int64_t for now.  uint64_t and double aren't
implemented.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qlit.h |  2 ++
 qobject/qlit.c          | 37 +++++++++++++++++++++++++++++++++++++
 tests/check-qlit.c      | 28 ++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+)

diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
index f1ed082df8..c0676d5daf 100644
--- a/include/qapi/qmp/qlit.h
+++ b/include/qapi/qmp/qlit.h
@@ -50,4 +50,6 @@ struct QLitDictEntry {
 
 bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs);
 
+QObject *qobject_from_qlit(const QLitObject *qlit);
+
 #endif /* QLIT_H */
diff --git a/qobject/qlit.c b/qobject/qlit.c
index 948e0b860c..a921abf84a 100644
--- a/qobject/qlit.c
+++ b/qobject/qlit.c
@@ -18,6 +18,7 @@
 #include "qapi/qmp/qlit.h"
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qlist.h"
+#include "qapi/qmp/qnull.h"
 #include "qapi/qmp/qnum.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qstring.h"
@@ -86,3 +87,39 @@ bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs)
 
     return false;
 }
+
+QObject *qobject_from_qlit(const QLitObject *qlit)
+{
+    switch (qlit->type) {
+    case QTYPE_QNULL:
+        return QOBJECT(qnull());
+    case QTYPE_QNUM:
+        return QOBJECT(qnum_from_int(qlit->value.qnum));
+    case QTYPE_QSTRING:
+        return QOBJECT(qstring_from_str(qlit->value.qstr));
+    case QTYPE_QDICT: {
+        QDict *qdict = qdict_new();
+        QLitDictEntry *e;
+
+        for (e = qlit->value.qdict; e->key; e++) {
+            qdict_put_obj(qdict, e->key, qobject_from_qlit(&e->value));
+        }
+        return QOBJECT(qdict);
+    }
+    case QTYPE_QLIST: {
+        QList *qlist = qlist_new();
+        QLitObject *e;
+
+        for (e = qlit->value.qlist; e->type != QTYPE_NONE; e++) {
+            qlist_append_obj(qlist, qobject_from_qlit(e));
+        }
+        return QOBJECT(qlist);
+    }
+    case QTYPE_QBOOL:
+        return QOBJECT(qbool_from_bool(qlit->value.qbool));
+    default:
+        assert(0);
+    }
+
+    return NULL;
+}
diff --git a/tests/check-qlit.c b/tests/check-qlit.c
index 5d0f65b9c7..836f4a3090 100644
--- a/tests/check-qlit.c
+++ b/tests/check-qlit.c
@@ -9,9 +9,11 @@
 
 #include "qemu/osdep.h"
 
+#include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qlist.h"
 #include "qapi/qmp/qlit.h"
+#include "qapi/qmp/qnum.h"
 #include "qapi/qmp/qstring.h"
 
 static QLitObject qlit = QLIT_QDICT(((QLitDictEntry[]) {
@@ -63,11 +65,37 @@ static void qlit_equal_qobject_test(void)
     qobject_decref(qobj);
 }
 
+static void qobject_from_qlit_test(void)
+{
+    QObject *obj, *qobj = qobject_from_qlit(&qlit);
+    QDict *qdict;
+    QList *bee;
+
+    qdict = qobject_to_qdict(qobj);
+    g_assert_cmpint(qdict_get_int(qdict, "foo"), ==, 42);
+    g_assert_cmpstr(qdict_get_str(qdict, "bar"), ==, "hello world");
+    g_assert(qobject_type(qdict_get(qdict, "baz")) == QTYPE_QNULL);
+
+    bee = qdict_get_qlist(qdict, "bee");
+    obj = qlist_pop(bee);
+    g_assert_cmpint(qnum_get_int(qobject_to_qnum(obj)), ==, 43);
+    qobject_decref(obj);
+    obj = qlist_pop(bee);
+    g_assert_cmpint(qnum_get_int(qobject_to_qnum(obj)), ==, 44);
+    qobject_decref(obj);
+    obj = qlist_pop(bee);
+    g_assert(qbool_get_bool(qobject_to_qbool(obj)));
+    qobject_decref(obj);
+
+    qobject_decref(qobj);
+}
+
 int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
 
     g_test_add_func("/qlit/equal_qobject", qlit_equal_qobject_test);
+    g_test_add_func("/qlit/qobject_from_qlit", qobject_from_qlit_test);
 
     return g_test_run();
 }
-- 
2.13.6

[Qemu-devel] [PATCH RFC 03/14] qapi/introspect: Simplify WIP

[Markus Armbruster]

---
 scripts/qapi/introspect.py | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index f66c397fb0..6c86673a09 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -51,36 +51,32 @@ class QAPISchemaGenIntrospectVisitor(QAPISchemaMonolithicCVisitor):
         self._jsons = []
         self._used_types = []
         self._name_map = {}
+        var = c_name(self._prefix, protect=False) + 'qmp_schema_json'
         self._genc.add(mcgen('''
 #include "qemu/osdep.h"
 #include "%(prefix)sqapi-introspect.h"
 
+const char %(var)s[] = ''',
+                             var=var, prefix=prefix))
+        self._genh.add(mcgen('''
+extern const char %(var)s[];
 ''',
-                             prefix=prefix))
+                             var=var))
 
     def visit_begin(self, schema):
         self._schema = schema
 
     def visit_end(self):
         # visit the types that are actually used
-        jsons = self._jsons
-        self._jsons = []
         for typ in self._used_types:
             typ.visit(self)
         # generate C
         # TODO can generate awfully long lines
-        jsons.extend(self._jsons)
-        name = c_name(self._prefix, protect=False) + 'qmp_schema_json'
-        self._genh.add(mcgen('''
-extern const char %(c_name)s[];
-''',
-                             c_name=c_name(name)))
-        lines = to_json(jsons).split('\n')
+        lines = to_json(self._jsons).split('\n')
         c_string = '\n    '.join([to_c_string(line) for line in lines])
         self._genc.add(mcgen('''
-const char %(c_name)s[] = %(c_string)s;
+%(c_string)s;
 ''',
-                             c_name=c_name(name),
                              c_string=c_string))
         self._schema = None
         self._jsons = []
-- 
2.13.6

[Qemu-devel] [PATCH RFC 04/14] qapi: generate a literal qobject for introspection

[Markus Armbruster]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Replace the generated json string with a literal qobject. The later is
easier to deal with, at run time as well as compile time: adding #if
conditionals will be easier than in a json string.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

FIXME doc diffs

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 docs/devel/qapi-code-gen.txt       | 29 +++++++++-----
 monitor.c                          |  2 +-
 scripts/qapi/introspect.py         | 80 ++++++++++++++++++++++----------------
 tests/test-qobject-input-visitor.c | 11 ++++--
 4 files changed, 73 insertions(+), 49 deletions(-)

diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 0c4fc342fe..cb6175ba4d 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -1318,18 +1318,27 @@ Example:
     #ifndef EXAMPLE_QMP_INTROSPECT_H
     #define EXAMPLE_QMP_INTROSPECT_H
 
-    extern const char example_qmp_schema_json[];
+    extern const QLitObject qmp_schema_qlit;
 
     #endif
     $ cat qapi-generated/example-qapi-introspect.c
 [Uninteresting stuff omitted...]
 
-    const char example_qmp_schema_json[] = "["
-        "{\"arg-type\": \"0\", \"meta-type\": \"event\", \"name\": \"MY_EVENT\"}, "
-        "{\"arg-type\": \"1\", \"meta-type\": \"command\", \"name\": \"my-command\", \"ret-type\": \"2\"}, "
-        "{\"members\": [], \"meta-type\": \"object\", \"name\": \"0\"}, "
-        "{\"members\": [{\"name\": \"arg1\", \"type\": \"[2]\"}], \"meta-type\": \"object\", \"name\": \"1\"}, "
-        "{\"members\": [{\"name\": \"integer\", \"type\": \"int\"}, {\"default\": null, \"name\": \"string\", \"type\": \"str\"}], \"meta-type\": \"object\", \"name\": \"2\"}, "
-        "{\"element-type\": \"2\", \"meta-type\": \"array\", \"name\": \"[2]\"}, "
-        "{\"json-type\": \"int\", \"meta-type\": \"builtin\", \"name\": \"int\"}, "
-        "{\"json-type\": \"string\", \"meta-type\": \"builtin\", \"name\": \"str\"}]";
+    const QLitObject example_qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
+        QLIT_QDICT(((QLitDictEntry[]) {
+            { "arg-type", QLIT_QSTR("0") },
+            { "meta-type", QLIT_QSTR("event") },
+            { "name", QLIT_QSTR("Event") },
+            { }
+        })),
+        QLIT_QDICT(((QLitDictEntry[]) {
+            { "members", QLIT_QLIST(((QLitObject[]) {
+                { }
+            })) },
+            { "meta-type", QLIT_QSTR("object") },
+            { "name", QLIT_QSTR("0") },
+            { }
+        })),
+        ...
+        { }
+    }));
diff --git a/monitor.c b/monitor.c
index d0e8d350fd..877b90f16d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -956,7 +956,7 @@ EventInfoList *qmp_query_events(Error **errp)
 static void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
                                  Error **errp)
 {
-    *ret_data = qobject_from_json(qmp_schema_json, &error_abort);
+    *ret_data = qobject_from_qlit(&qmp_schema_qlit);
 }
 
 /*
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 6c86673a09..e5c491d936 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -13,26 +13,35 @@ See the COPYING file in the top-level directory.
 from qapi.common import *
 
 
-# Caveman's json.dumps() replacement (we're stuck at Python 2.4)
-# TODO try to use json.dumps() once we get unstuck
-def to_json(obj, level=0):
+def to_qlit(obj, level=0, suppress_first_indent=False):
+
+    def indent(level):
+        return level * 4 * ' '
+
+    if not suppress_first_indent:
+        ret = indent(level)
     if obj is None:
-        ret = 'null'
+        ret = 'QLIT_QNULL'
     elif isinstance(obj, str):
-        ret = '"' + obj.replace('"', r'\"') + '"'
+        ret = 'QLIT_QSTR(' + to_c_string(obj) + ')'
     elif isinstance(obj, list):
-        elts = [to_json(elt, level + 1)
+        elts = [to_qlit(elt, level + 1)
                 for elt in obj]
-        ret = '[' + ', '.join(elts) + ']'
+        elts.append(indent(level + 1) + "{}")
+        ret = 'QLIT_QLIST(((QLitObject[]) {\n'
+        ret += ',\n'.join(elts) + '\n'
+        ret += indent(level) + '}))'
     elif isinstance(obj, dict):
-        elts = ['"%s": %s' % (key.replace('"', r'\"'),
-                              to_json(obj[key], level + 1))
-                for key in sorted(obj.keys())]
-        ret = '{' + ', '.join(elts) + '}'
+        elts = []
+        for key, value in sorted(obj.iteritems()):
+            elts.append(indent(level + 1) + '{ %s, %s }' %
+                        (to_c_string(key), to_qlit(value, level + 1, True)))
+        elts.append(indent(level + 1) + '{}')
+        ret = 'QLIT_QDICT(((QLitDictEntry[]) {\n'
+        ret += ',\n'.join(elts) + '\n'
+        ret += indent(level) + '}))'
     else:
         assert False                # not implemented
-    if level == 1:
-        ret = '\n' + ret
     return ret
 
 
@@ -48,38 +57,41 @@ class QAPISchemaGenIntrospectVisitor(QAPISchemaMonolithicCVisitor):
             ' * QAPI/QMP schema introspection', __doc__)
         self._unmask = unmask
         self._schema = None
-        self._jsons = []
+        self._qlits = None
+        self._used_types = None
+        self._name_map = None
+
+    def visit_begin(self, schema):
+        self._schema = schema
+        self._qlits = []
         self._used_types = []
         self._name_map = {}
-        var = c_name(self._prefix, protect=False) + 'qmp_schema_json'
+        var = c_name(self._prefix, protect=False) + 'qmp_schema_qlit'
         self._genc.add(mcgen('''
 #include "qemu/osdep.h"
 #include "%(prefix)sqapi-introspect.h"
 
-const char %(var)s[] = ''',
-                             var=var, prefix=prefix))
+const QLitObject %(var)s = ''',
+                             var=var, prefix=self._prefix))
         self._genh.add(mcgen('''
-extern const char %(var)s[];
+#include "qapi/qmp/qlit.h"
+
+extern const QLitObject %(var)s;
 ''',
                              var=var))
 
-    def visit_begin(self, schema):
-        self._schema = schema
-
     def visit_end(self):
         # visit the types that are actually used
         for typ in self._used_types:
             typ.visit(self)
         # generate C
         # TODO can generate awfully long lines
-        lines = to_json(self._jsons).split('\n')
-        c_string = '\n    '.join([to_c_string(line) for line in lines])
         self._genc.add(mcgen('''
 %(c_string)s;
 ''',
-                             c_string=c_string))
+                             c_string=to_qlit(self._qlits)))
         self._schema = None
-        self._jsons = []
+        self._qlits = None
         self._used_types = []
         self._name_map = {}
 
@@ -113,12 +125,12 @@ extern const char %(var)s[];
             return '[' + self._use_type(typ.element_type) + ']'
         return self._name(typ.name)
 
-    def _gen_json(self, name, mtype, obj):
+    def _gen_qlit(self, name, mtype, obj):
         if mtype not in ('command', 'event', 'builtin', 'array'):
             name = self._name(name)
         obj['name'] = name
         obj['meta-type'] = mtype
-        self._jsons.append(obj)
+        self._qlits.append(obj)
 
     def _gen_member(self, member):
         ret = {'name': member.name, 'type': self._use_type(member.type)}
@@ -134,24 +146,24 @@ extern const char %(var)s[];
         return {'case': variant.name, 'type': self._use_type(variant.type)}
 
     def visit_builtin_type(self, name, info, json_type):
-        self._gen_json(name, 'builtin', {'json-type': json_type})
+        self._gen_qlit(name, 'builtin', {'json-type': json_type})
 
     def visit_enum_type(self, name, info, values, prefix):
-        self._gen_json(name, 'enum', {'values': values})
+        self._gen_qlit(name, 'enum', {'values': values})
 
     def visit_array_type(self, name, info, element_type):
         element = self._use_type(element_type)
-        self._gen_json('[' + element + ']', 'array', {'element-type': element})
+        self._gen_qlit('[' + element + ']', 'array', {'element-type': element})
 
     def visit_object_type_flat(self, name, info, members, variants):
         obj = {'members': [self._gen_member(m) for m in members]}
         if variants:
             obj.update(self._gen_variants(variants.tag_member.name,
                                           variants.variants))
-        self._gen_json(name, 'object', obj)
+        self._gen_qlit(name, 'object', obj)
 
     def visit_alternate_type(self, name, info, variants):
-        self._gen_json(name, 'alternate',
+        self._gen_qlit(name, 'alternate',
                        {'members': [{'type': self._use_type(m.type)}
                                     for m in variants.variants]})
 
@@ -159,13 +171,13 @@ extern const char %(var)s[];
                       gen, success_response, boxed):
         arg_type = arg_type or self._schema.the_empty_object_type
         ret_type = ret_type or self._schema.the_empty_object_type
-        self._gen_json(name, 'command',
+        self._gen_qlit(name, 'command',
                        {'arg-type': self._use_type(arg_type),
                         'ret-type': self._use_type(ret_type)})
 
     def visit_event(self, name, info, arg_type, boxed):
         arg_type = arg_type or self._schema.the_empty_object_type
-        self._gen_json(name, 'event', {'arg-type': self._use_type(arg_type)})
+        self._gen_qlit(name, 'event', {'arg-type': self._use_type(arg_type)})
 
 
 def gen_introspect(schema, output_dir, prefix, opt_unmask):
diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-input-visitor.c
index 79b1a8cb17..55db6bdef1 100644
--- a/tests/test-qobject-input-visitor.c
+++ b/tests/test-qobject-input-visitor.c
@@ -1250,24 +1250,27 @@ static void test_visitor_in_fail_alternate(TestInputVisitorData *data,
 }
 
 static void do_test_visitor_in_qmp_introspect(TestInputVisitorData *data,
-                                              const char *schema_json)
+                                              const QLitObject *qlit)
 {
     SchemaInfoList *schema = NULL;
+    QObject *obj = qobject_from_qlit(qlit);
     Visitor *v;
 
-    v = visitor_input_test_init_raw(data, schema_json);
+    v = qobject_input_visitor_new(obj);
 
     visit_type_SchemaInfoList(v, NULL, &schema, &error_abort);
     g_assert(schema);
 
     qapi_free_SchemaInfoList(schema);
+    qobject_decref(obj);
+    visit_free(v);
 }
 
 static void test_visitor_in_qmp_introspect(TestInputVisitorData *data,
                                            const void *unused)
 {
-    do_test_visitor_in_qmp_introspect(data, test_qmp_schema_json);
-    do_test_visitor_in_qmp_introspect(data, qmp_schema_json);
+    do_test_visitor_in_qmp_introspect(data, &test_qmp_schema_qlit);
+    do_test_visitor_in_qmp_introspect(data, &qmp_schema_qlit);
 }
 
 int main(int argc, char **argv)
-- 
2.13.6

[Qemu-devel] [PATCH RFC 05/14] qapi2texi: minor python code simplification

[Markus Armbruster]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/doc.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
index 0ea68bf813..79d11bbe9b 100644
--- a/scripts/qapi/doc.py
+++ b/scripts/qapi/doc.py
@@ -134,10 +134,9 @@ def texi_enum_value(value):
 def texi_member(member, suffix=''):
     """Format a table of members item for an object type member"""
     typ = member.type.doc_type()
-    return '@item @code{%s%s%s}%s%s\n' % (
-        member.name,
-        ': ' if typ else '',
-        typ if typ else '',
+    membertype = ': ' + typ if typ else ''
+    return '@item @code{%s%s}%s%s\n' % (
+        member.name, membertype,
         ' (optional)' if member.optional else '',
         suffix)
 
-- 
2.13.6

[Qemu-devel] [PATCH RFC 06/14] qapi: add 'if' to top-level expressions

[Markus Armbruster]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Accept 'if' key in top-level elements, accepted as string or list of
string type. The following patches will modify the test visitor to
check the value is correctly saved, and generate #if/#endif code (as a
single #if/endif line or a series for a list).

Example of 'if' key:
{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
  'if': 'defined(TEST_IF_STRUCT)' }

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

A following patch for qapi-code-gen.txt will provide more complete
documentation for 'if' usage.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/common.py                   | 36 ++++++++++++++++++++++++++------
 tests/Makefile.include                   |  4 ++++
 tests/qapi-schema/bad-if-empty-list.err  |  1 +
 tests/qapi-schema/bad-if-empty-list.exit |  1 +
 tests/qapi-schema/bad-if-empty-list.json |  3 +++
 tests/qapi-schema/bad-if-empty-list.out  |  0
 tests/qapi-schema/bad-if-empty.err       |  1 +
 tests/qapi-schema/bad-if-empty.exit      |  1 +
 tests/qapi-schema/bad-if-empty.json      |  3 +++
 tests/qapi-schema/bad-if-empty.out       |  0
 tests/qapi-schema/bad-if-list.err        |  1 +
 tests/qapi-schema/bad-if-list.exit       |  1 +
 tests/qapi-schema/bad-if-list.json       |  3 +++
 tests/qapi-schema/bad-if-list.out        |  0
 tests/qapi-schema/bad-if.err             |  1 +
 tests/qapi-schema/bad-if.exit            |  1 +
 tests/qapi-schema/bad-if.json            |  3 +++
 tests/qapi-schema/bad-if.out             |  0
 tests/qapi-schema/qapi-schema-test.json  | 20 ++++++++++++++++++
 tests/qapi-schema/qapi-schema-test.out   | 22 +++++++++++++++++++
 tests/test-qmp-cmds.c                    |  6 ++++++
 21 files changed, 102 insertions(+), 6 deletions(-)
 create mode 100644 tests/qapi-schema/bad-if-empty-list.err
 create mode 100644 tests/qapi-schema/bad-if-empty-list.exit
 create mode 100644 tests/qapi-schema/bad-if-empty-list.json
 create mode 100644 tests/qapi-schema/bad-if-empty-list.out
 create mode 100644 tests/qapi-schema/bad-if-empty.err
 create mode 100644 tests/qapi-schema/bad-if-empty.exit
 create mode 100644 tests/qapi-schema/bad-if-empty.json
 create mode 100644 tests/qapi-schema/bad-if-empty.out
 create mode 100644 tests/qapi-schema/bad-if-list.err
 create mode 100644 tests/qapi-schema/bad-if-list.exit
 create mode 100644 tests/qapi-schema/bad-if-list.json
 create mode 100644 tests/qapi-schema/bad-if-list.out
 create mode 100644 tests/qapi-schema/bad-if.err
 create mode 100644 tests/qapi-schema/bad-if.exit
 create mode 100644 tests/qapi-schema/bad-if.json
 create mode 100644 tests/qapi-schema/bad-if.out

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index b9a52e820d..0254b9e1ef 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -637,6 +637,27 @@ def add_name(name, info, meta, implicit=False):
     all_names[name] = meta
 
 
+def check_if(expr, info):
+
+    def check_if_str(ifcond, info):
+        if not isinstance(ifcond, str):
+            raise QAPISemError(
+                info, "'if' condition must be a string or a list of strings")
+        if ifcond == '':
+            raise QAPISemError(info, "'if' condition '' makes no sense")
+
+    ifcond = expr.get('if')
+    if ifcond is None:
+        return
+    elif isinstance(ifcond, list):
+        if ifcond == []:
+            raise QAPISemError(info, "'if' condition [] is useless")
+        for elt in ifcond:
+            check_if_str(elt, info)
+    else:
+        check_if_str(ifcond, info)
+
+
 def check_type(info, source, value, allow_array=False,
                allow_dict=False, allow_optional=False,
                allow_metas=[]):
@@ -876,6 +897,8 @@ def check_keys(expr_elem, meta, required, optional=[]):
             raise QAPISemError(info,
                                "'%s' of %s '%s' should only use true value"
                                % (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'"
@@ -904,27 +927,28 @@ def check_exprs(exprs):
 
         if 'enum' in expr:
             meta = 'enum'
-            check_keys(expr_elem, 'enum', ['data'], ['prefix'])
+            check_keys(expr_elem, 'enum', ['data'], ['if', 'prefix'])
             enum_types[expr[meta]] = expr
         elif 'union' in expr:
             meta = 'union'
             check_keys(expr_elem, 'union', ['data'],
-                       ['base', 'discriminator'])
+                       ['base', 'discriminator', 'if'])
             union_types[expr[meta]] = expr
         elif 'alternate' in expr:
             meta = 'alternate'
-            check_keys(expr_elem, 'alternate', ['data'])
+            check_keys(expr_elem, 'alternate', ['data'], ['if'])
         elif 'struct' in expr:
             meta = 'struct'
-            check_keys(expr_elem, 'struct', ['data'], ['base'])
+            check_keys(expr_elem, 'struct', ['data'], ['base', 'if'])
             struct_types[expr[meta]] = expr
         elif 'command' in expr:
             meta = 'command'
             check_keys(expr_elem, 'command', [],
-                       ['data', 'returns', 'gen', 'success-response', 'boxed'])
+                       ['data', 'returns', 'gen', 'success-response', 'boxed',
+                        'if'])
         elif 'event' in expr:
             meta = 'event'
-            check_keys(expr_elem, 'event', [], ['data', 'boxed'])
+            check_keys(expr_elem, 'event', [], ['data', 'boxed', 'if'])
         else:
             raise QAPISemError(expr_elem['info'],
                                "Expression is missing metatype")
diff --git a/tests/Makefile.include b/tests/Makefile.include
index d71adf3996..a060ce24b7 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -429,6 +429,10 @@ qapi-schema += args-unknown.json
 qapi-schema += bad-base.json
 qapi-schema += bad-data.json
 qapi-schema += bad-ident.json
+qapi-schema += bad-if.json
+qapi-schema += bad-if-empty.json
+qapi-schema += bad-if-empty-list.json
+qapi-schema += bad-if-list.json
 qapi-schema += bad-type-bool.json
 qapi-schema += bad-type-dict.json
 qapi-schema += bad-type-int.json
diff --git a/tests/qapi-schema/bad-if-empty-list.err b/tests/qapi-schema/bad-if-empty-list.err
new file mode 100644
index 0000000000..75fe6497bc
--- /dev/null
+++ b/tests/qapi-schema/bad-if-empty-list.err
@@ -0,0 +1 @@
+tests/qapi-schema/bad-if-empty-list.json:2: 'if' condition [] is useless
diff --git a/tests/qapi-schema/bad-if-empty-list.exit b/tests/qapi-schema/bad-if-empty-list.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/bad-if-empty-list.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/bad-if-empty-list.json b/tests/qapi-schema/bad-if-empty-list.json
new file mode 100644
index 0000000000..94f2eb8670
--- /dev/null
+++ b/tests/qapi-schema/bad-if-empty-list.json
@@ -0,0 +1,3 @@
+# check empty 'if' list
+{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
+  'if': [] }
diff --git a/tests/qapi-schema/bad-if-empty-list.out b/tests/qapi-schema/bad-if-empty-list.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/bad-if-empty.err b/tests/qapi-schema/bad-if-empty.err
new file mode 100644
index 0000000000..358bdc3e51
--- /dev/null
+++ b/tests/qapi-schema/bad-if-empty.err
@@ -0,0 +1 @@
+tests/qapi-schema/bad-if-empty.json:2: 'if' condition '' makes no sense
diff --git a/tests/qapi-schema/bad-if-empty.exit b/tests/qapi-schema/bad-if-empty.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/bad-if-empty.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/bad-if-empty.json b/tests/qapi-schema/bad-if-empty.json
new file mode 100644
index 0000000000..fe1dd4eca6
--- /dev/null
+++ b/tests/qapi-schema/bad-if-empty.json
@@ -0,0 +1,3 @@
+# check empty 'if'
+{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
+  'if': '' }
diff --git a/tests/qapi-schema/bad-if-empty.out b/tests/qapi-schema/bad-if-empty.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/bad-if-list.err b/tests/qapi-schema/bad-if-list.err
new file mode 100644
index 0000000000..0af6316f78
--- /dev/null
+++ b/tests/qapi-schema/bad-if-list.err
@@ -0,0 +1 @@
+tests/qapi-schema/bad-if-list.json:2: 'if' condition '' makes no sense
diff --git a/tests/qapi-schema/bad-if-list.exit b/tests/qapi-schema/bad-if-list.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/bad-if-list.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/bad-if-list.json b/tests/qapi-schema/bad-if-list.json
new file mode 100644
index 0000000000..49ced9b9ca
--- /dev/null
+++ b/tests/qapi-schema/bad-if-list.json
@@ -0,0 +1,3 @@
+# check invalid 'if' content
+{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
+  'if': ['foo', ''] }
diff --git a/tests/qapi-schema/bad-if-list.out b/tests/qapi-schema/bad-if-list.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/bad-if.err b/tests/qapi-schema/bad-if.err
new file mode 100644
index 0000000000..c2e3f5f44c
--- /dev/null
+++ b/tests/qapi-schema/bad-if.err
@@ -0,0 +1 @@
+tests/qapi-schema/bad-if.json:2: 'if' condition must be a string or a list of strings
diff --git a/tests/qapi-schema/bad-if.exit b/tests/qapi-schema/bad-if.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/bad-if.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/bad-if.json b/tests/qapi-schema/bad-if.json
new file mode 100644
index 0000000000..3edd1a0bf2
--- /dev/null
+++ b/tests/qapi-schema/bad-if.json
@@ -0,0 +1,3 @@
+# check invalid 'if' type
+{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
+  'if': { 'value': 'defined(TEST_IF_STRUCT)' } }
diff --git a/tests/qapi-schema/bad-if.out b/tests/qapi-schema/bad-if.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index c72dbd8050..b997b2d43d 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -188,3 +188,23 @@
   'data': { 'a': ['__org.qemu_x-Enum'], 'b': ['__org.qemu_x-Struct'],
             'c': '__org.qemu_x-Union2', 'd': '__org.qemu_x-Alt' },
   'returns': '__org.qemu_x-Union1' }
+
+# test 'if' condition handling
+
+{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
+  'if': 'defined(TEST_IF_STRUCT)' }
+
+{ 'enum': 'TestIfEnum', 'data': [ 'foo', 'bar' ],
+  'if': 'defined(TEST_IF_ENUM)' }
+
+{ 'union': 'TestIfUnion', 'data': { 'foo': 'TestStruct' },
+  'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' }
+
+{ 'alternate': 'TestIfAlternate', 'data': { 'foo': 'int', 'bar': 'TestStruct' },
+  'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' }
+
+{ 'command': 'TestIfCmd', 'data': { 'foo': 'TestIfStruct' },
+  'if': ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] }
+
+{ 'event': 'TestIfEvent', 'data': { 'foo': 'TestIfStruct' },
+  '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
index 012e7fc06a..b11682314c 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -230,3 +230,25 @@ object q_obj___org.qemu_x-command-arg
     member d: __org.qemu_x-Alt optional=False
 command __org.qemu_x-command q_obj___org.qemu_x-command-arg -> __org.qemu_x-Union1
    gen=True success_response=True boxed=False
+object TestIfStruct
+    member foo: int optional=False
+enum TestIfEnum ['foo', 'bar']
+object q_obj_TestStruct-wrapper
+    member data: TestStruct optional=False
+enum TestIfUnionKind ['foo']
+object TestIfUnion
+    member type: TestIfUnionKind optional=False
+    tag type
+    case foo: q_obj_TestStruct-wrapper
+alternate TestIfAlternate
+    tag type
+    case foo: int
+    case bar: TestStruct
+object q_obj_TestIfCmd-arg
+    member foo: TestIfStruct optional=False
+command TestIfCmd q_obj_TestIfCmd-arg -> None
+   gen=True success_response=True boxed=False
+object q_obj_TestIfEvent-arg
+    member foo: TestIfStruct optional=False
+event TestIfEvent q_obj_TestIfEvent-arg
+   boxed=False
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index 5b1cee6912..c0a3c46439 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -12,6 +12,12 @@
 
 static QmpCommandList qmp_commands;
 
+/* #if defined(TEST_IF_CMD) */
+void qmp_TestIfCmd(TestIfStruct *foo, Error **errp)
+{
+}
+/* #endif */
+
 void qmp_user_def_cmd(Error **errp)
 {
 }
-- 
2.13.6

[Qemu-devel] [PATCH RFC 07/14] qapi: pass 'if' condition into QAPISchemaEntity objects

[Markus Armbruster]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Built-in objects remain unconditional.  Explicitly defined objects
use the condition specified in the schema.  Implicitly defined
objects inherit their condition from their users.  For most of them,
there is exactly one user, so the condition to use is obvious.  The
exception is the wrapped type's generated for simple union variants,
which can be shared by any number of simple unions.  The tight
condition would be the disjunction of the conditions of these simple
unions.  For now, use wrapped type's condition instead.  Much
simpler and good enough for now.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/common.py | 98 +++++++++++++++++++++++++++++++++-----------------
 1 file changed, 66 insertions(+), 32 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 0254b9e1ef..789c77f11f 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1007,8 +1007,17 @@ def check_exprs(exprs):
 # Schema compiler frontend
 #
 
+def listify_cond(ifcond):
+    if not ifcond:
+        return []
+    elif not isinstance(ifcond, list):
+        return [ifcond]
+    else:
+        return ifcond
+
+
 class QAPISchemaEntity(object):
-    def __init__(self, name, info, doc):
+    def __init__(self, name, info, doc, ifcond=None):
         assert name is None or isinstance(name, str)
         self.name = name
         self.module = None
@@ -1019,6 +1028,7 @@ class QAPISchemaEntity(object):
         # such place).
         self.info = info
         self.doc = doc
+        self.ifcond = listify_cond(ifcond)
 
     def c_name(self):
         return c_name(self.name)
@@ -1151,8 +1161,8 @@ class QAPISchemaBuiltinType(QAPISchemaType):
 
 
 class QAPISchemaEnumType(QAPISchemaType):
-    def __init__(self, name, info, doc, values, prefix):
-        QAPISchemaType.__init__(self, name, info, doc)
+    def __init__(self, name, info, doc, ifcond, values, prefix):
+        QAPISchemaType.__init__(self, name, info, doc, ifcond)
         for v in values:
             assert isinstance(v, QAPISchemaMember)
             v.set_owner(name)
@@ -1187,7 +1197,7 @@ class QAPISchemaEnumType(QAPISchemaType):
 
 class QAPISchemaArrayType(QAPISchemaType):
     def __init__(self, name, info, element_type):
-        QAPISchemaType.__init__(self, name, info, None)
+        QAPISchemaType.__init__(self, name, info, None, None)
         assert isinstance(element_type, str)
         self._element_type_name = element_type
         self.element_type = None
@@ -1195,6 +1205,7 @@ class QAPISchemaArrayType(QAPISchemaType):
     def check(self, schema):
         self.element_type = schema.lookup_type(self._element_type_name)
         assert self.element_type
+        self.ifcond = self.element_type.ifcond
 
     def is_implicit(self):
         return True
@@ -1216,11 +1227,12 @@ class QAPISchemaArrayType(QAPISchemaType):
 
 
 class QAPISchemaObjectType(QAPISchemaType):
-    def __init__(self, name, info, doc, base, local_members, variants):
+    def __init__(self, name, info, doc, ifcond,
+                 base, local_members, variants):
         # struct has local_members, optional base, and no variants
         # flat union has base, variants, and no local_members
         # simple union has local_members, variants, and no base
-        QAPISchemaType.__init__(self, name, info, doc)
+        QAPISchemaType.__init__(self, name, info, doc, ifcond)
         assert base is None or isinstance(base, str)
         for m in local_members:
             assert isinstance(m, QAPISchemaObjectTypeMember)
@@ -1408,8 +1420,8 @@ class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
 
 
 class QAPISchemaAlternateType(QAPISchemaType):
-    def __init__(self, name, info, doc, variants):
-        QAPISchemaType.__init__(self, name, info, doc)
+    def __init__(self, name, info, doc, ifcond, variants):
+        QAPISchemaType.__init__(self, name, info, doc, ifcond)
         assert isinstance(variants, QAPISchemaObjectTypeVariants)
         assert variants.tag_member
         variants.set_owner(name)
@@ -1445,9 +1457,9 @@ class QAPISchemaAlternateType(QAPISchemaType):
 
 
 class QAPISchemaCommand(QAPISchemaEntity):
-    def __init__(self, name, info, doc, arg_type, ret_type,
+    def __init__(self, name, info, doc, ifcond, arg_type, ret_type,
                  gen, success_response, boxed):
-        QAPISchemaEntity.__init__(self, name, info, doc)
+        QAPISchemaEntity.__init__(self, name, info, doc, ifcond)
         assert not arg_type or isinstance(arg_type, str)
         assert not ret_type or isinstance(ret_type, str)
         self._arg_type_name = arg_type
@@ -1484,8 +1496,8 @@ class QAPISchemaCommand(QAPISchemaEntity):
 
 
 class QAPISchemaEvent(QAPISchemaEntity):
-    def __init__(self, name, info, doc, arg_type, boxed):
-        QAPISchemaEntity.__init__(self, name, info, doc)
+    def __init__(self, name, info, doc, ifcond, arg_type, boxed):
+        QAPISchemaEntity.__init__(self, name, info, doc, ifcond)
         assert not arg_type or isinstance(arg_type, str)
         self._arg_type_name = arg_type
         self.arg_type = None
@@ -1579,22 +1591,22 @@ class QAPISchema(object):
                   ('null',   'null',    'QNull' + pointer_suffix)]:
             self._def_builtin_type(*t)
         self.the_empty_object_type = QAPISchemaObjectType(
-            'q_empty', None, None, None, [], None)
+            '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'])
-        self._def_entity(QAPISchemaEnumType('QType', None, None,
+        self._def_entity(QAPISchemaEnumType('QType', None, None, None,
                                             qtype_values, 'QTYPE'))
 
     def _make_enum_members(self, values):
         return [QAPISchemaMember(v) for v in values]
 
-    def _make_implicit_enum_type(self, name, info, values):
+    def _make_implicit_enum_type(self, name, info, ifcond, values):
         # See also QAPISchemaObjectTypeMember._pretty_owner()
         name = name + 'Kind'   # Use namespace reserved by add_name()
         self._def_entity(QAPISchemaEnumType(
-            name, info, None, self._make_enum_members(values), None))
+            name, info, None, ifcond, self._make_enum_members(values), None))
         return name
 
     def _make_array_type(self, element_type, info):
@@ -1603,22 +1615,37 @@ class QAPISchema(object):
             self._def_entity(QAPISchemaArrayType(name, info, element_type))
         return name
 
-    def _make_implicit_object_type(self, name, info, doc, role, members):
+    def _make_implicit_object_type(self, name, info, doc, ifcond,
+                                   role, members):
         if not members:
             return None
         # See also QAPISchemaObjectTypeMember._pretty_owner()
         name = 'q_obj_%s-%s' % (name, role)
-        if not self.lookup_entity(name, QAPISchemaObjectType):
-            self._def_entity(QAPISchemaObjectType(name, info, doc, None,
-                                                  members, None))
+        typ = self.lookup_entity(name, QAPISchemaObjectType)
+        if typ:
+            # The implicit object type has multiple users.  This can
+            # happen only for simple unions' implicit wrapper types.
+            # Its ifcond should be the disjunction of its user's
+            # ifconds.  Not implemented.  Instead, we always pass the
+            # wrapped type's ifcond, which is trivially the same for all
+            # users.  It's also necessary for the wrapper to compile.
+            # But it's not tight: the disjunction need not imply it.  We
+            # may end up compiling useless wrapper types.
+            # TODO kill simple unions or implement the disjunction
+            assert ifcond == typ.ifcond
+        else:
+            self._def_entity(QAPISchemaObjectType(name, info, doc, ifcond,
+                                                  None, members, None))
         return name
 
     def _def_enum_type(self, expr, info, doc):
         name = expr['enum']
         data = expr['data']
         prefix = expr.get('prefix')
+        ifcond = expr.get('if')
         self._def_entity(QAPISchemaEnumType(
-            name, info, doc, self._make_enum_members(data), prefix))
+            name, info, doc, ifcond,
+            self._make_enum_members(data), prefix))
 
     def _make_member(self, name, typ, info):
         optional = False
@@ -1638,7 +1665,8 @@ class QAPISchema(object):
         name = expr['struct']
         base = expr.get('base')
         data = expr['data']
-        self._def_entity(QAPISchemaObjectType(name, info, doc, base,
+        ifcond = expr.get('if')
+        self._def_entity(QAPISchemaObjectType(name, info, doc, ifcond, base,
                                               self._make_members(data, info),
                                               None))
 
@@ -1650,18 +1678,21 @@ class QAPISchema(object):
             assert len(typ) == 1
             typ = self._make_array_type(typ[0], info)
         typ = self._make_implicit_object_type(
-            typ, info, None, 'wrapper', [self._make_member('data', typ, info)])
+            typ, info, None, self.lookup_type(typ).ifcond,
+            'wrapper', [self._make_member('data', typ, info)])
         return QAPISchemaObjectTypeVariant(case, typ)
 
     def _def_union_type(self, expr, info, doc):
         name = expr['union']
         data = expr['data']
         base = expr.get('base')
+        ifcond = expr.get('if')
         tag_name = expr.get('discriminator')
         tag_member = None
         if isinstance(base, dict):
-            base = (self._make_implicit_object_type(
-                name, info, doc, 'base', self._make_members(base, info)))
+            base = self._make_implicit_object_type(
+                name, info, doc, ifcond,
+                'base', self._make_members(base, info))
         if tag_name:
             variants = [self._make_variant(key, value)
                         for (key, value) in data.items()]
@@ -1669,12 +1700,12 @@ class QAPISchema(object):
         else:
             variants = [self._make_simple_variant(key, value, info)
                         for (key, value) in data.items()]
-            typ = self._make_implicit_enum_type(name, info,
+            typ = self._make_implicit_enum_type(name, info, ifcond,
                                                 [v.name for v in variants])
             tag_member = QAPISchemaObjectTypeMember('type', typ, False)
             members = [tag_member]
         self._def_entity(
-            QAPISchemaObjectType(name, info, doc, base, members,
+            QAPISchemaObjectType(name, info, doc, ifcond, base, members,
                                  QAPISchemaObjectTypeVariants(tag_name,
                                                               tag_member,
                                                               variants)))
@@ -1682,11 +1713,12 @@ class QAPISchema(object):
     def _def_alternate_type(self, expr, info, doc):
         name = expr['alternate']
         data = expr['data']
+        ifcond = expr.get('if')
         variants = [self._make_variant(key, value)
                     for (key, value) in data.items()]
         tag_member = QAPISchemaObjectTypeMember('type', 'QType', False)
         self._def_entity(
-            QAPISchemaAlternateType(name, info, doc,
+            QAPISchemaAlternateType(name, info, doc, ifcond,
                                     QAPISchemaObjectTypeVariants(None,
                                                                  tag_member,
                                                                  variants)))
@@ -1698,23 +1730,25 @@ class QAPISchema(object):
         gen = expr.get('gen', True)
         success_response = expr.get('success-response', True)
         boxed = expr.get('boxed', False)
+        ifcond = expr.get('if')
         if isinstance(data, OrderedDict):
             data = self._make_implicit_object_type(
-                name, info, doc, 'arg', self._make_members(data, info))
+                name, info, doc, ifcond, 'arg', self._make_members(data, info))
         if isinstance(rets, list):
             assert len(rets) == 1
             rets = self._make_array_type(rets[0], info)
-        self._def_entity(QAPISchemaCommand(name, info, doc, data, rets,
+        self._def_entity(QAPISchemaCommand(name, info, doc, ifcond, data, rets,
                                            gen, success_response, boxed))
 
     def _def_event(self, expr, info, doc):
         name = expr['event']
         data = expr.get('data')
         boxed = expr.get('boxed', False)
+        ifcond = expr.get('if')
         if isinstance(data, OrderedDict):
             data = self._make_implicit_object_type(
-                name, info, doc, 'arg', self._make_members(data, info))
-        self._def_entity(QAPISchemaEvent(name, info, doc, data, boxed))
+                name, info, doc, ifcond, 'arg', self._make_members(data, info))
+        self._def_entity(QAPISchemaEvent(name, info, doc, ifcond, data, boxed))
 
     def _def_exprs(self, exprs):
         for expr_elem in exprs:
-- 
2.13.6

[Qemu-devel] [PATCH RFC 08/14] qapi: leave the ifcond attribute undefined until check()

[Markus Armbruster]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

We commonly initialize attributes to None in .init(), then set their
real value in .check().  Accessing the attribute before .check()
yields None.  If we're lucky, the code that accesses the attribute
prematurely chokes on None.

It won't for .ifcond, because None is a legitimate value.

Leave the ifcond attribute undefined until check().

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi/common.py | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 789c77f11f..78aeec785e 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1028,13 +1028,19 @@ class QAPISchemaEntity(object):
         # such place).
         self.info = info
         self.doc = doc
-        self.ifcond = listify_cond(ifcond)
+        self._ifcond = ifcond  # self.ifcond is set after check()
 
     def c_name(self):
         return c_name(self.name)
 
     def check(self, schema):
-        pass
+        if isinstance(self._ifcond, QAPISchemaType):
+            # inherit the condition from a type
+            typ = self._ifcond
+            typ.check(schema)
+            self.ifcond = typ.ifcond
+        else:
+            self.ifcond = listify_cond(self._ifcond)
 
     def is_implicit(self):
         return not self.info
@@ -1171,6 +1177,7 @@ class QAPISchemaEnumType(QAPISchemaType):
         self.prefix = prefix
 
     def check(self, schema):
+        QAPISchemaType.check(self, schema)
         seen = {}
         for v in self.values:
             v.check_clash(self.info, seen)
@@ -1203,8 +1210,10 @@ class QAPISchemaArrayType(QAPISchemaType):
         self.element_type = None
 
     def check(self, schema):
+        QAPISchemaType.check(self, schema)
         self.element_type = schema.lookup_type(self._element_type_name)
         assert self.element_type
+        self.element_type.check(schema)
         self.ifcond = self.element_type.ifcond
 
     def is_implicit(self):
@@ -1247,6 +1256,7 @@ class QAPISchemaObjectType(QAPISchemaType):
         self.members = None
 
     def check(self, schema):
+        QAPISchemaType.check(self, schema)
         if self.members is False:               # check for cycles
             raise QAPISemError(self.info,
                                "Object %s contains itself" % self.name)
@@ -1429,6 +1439,7 @@ class QAPISchemaAlternateType(QAPISchemaType):
         self.variants = variants
 
     def check(self, schema):
+        QAPISchemaType.check(self, schema)
         self.variants.tag_member.check(schema)
         # Not calling self.variants.check_clash(), because there's nothing
         # to clash with
@@ -1471,6 +1482,7 @@ class QAPISchemaCommand(QAPISchemaEntity):
         self.boxed = boxed
 
     def check(self, schema):
+        QAPISchemaEntity.check(self, schema)
         if self._arg_type_name:
             self.arg_type = schema.lookup_type(self._arg_type_name)
             assert (isinstance(self.arg_type, QAPISchemaObjectType) or
@@ -1504,6 +1516,7 @@ class QAPISchemaEvent(QAPISchemaEntity):
         self.boxed = boxed
 
     def check(self, schema):
+        QAPISchemaEntity.check(self, schema)
         if self._arg_type_name:
             self.arg_type = schema.lookup_type(self._arg_type_name)
             assert (isinstance(self.arg_type, QAPISchemaObjectType) or
@@ -1632,7 +1645,7 @@ class QAPISchema(object):
             # But it's not tight: the disjunction need not imply it.  We
             # may end up compiling useless wrapper types.
             # TODO kill simple unions or implement the disjunction
-            assert ifcond == typ.ifcond
+            assert ifcond == typ._ifcond
         else:
             self._def_entity(QAPISchemaObjectType(name, info, doc, ifcond,
                                                   None, members, None))
@@ -1678,7 +1691,7 @@ class QAPISchema(object):
             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).ifcond,
+            typ, info, None, self.lookup_type(typ),
             'wrapper', [self._make_member('data', typ, info)])
         return QAPISchemaObjectTypeVariant(case, typ)
 
-- 
2.13.6

[Qemu-devel] [PATCH RFC 09/14] qapi: Pass ifcond to visitors

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/common.py                 | 15 +++++++++++++++
 tests/qapi-schema/qapi-schema-test.out |  9 +++++++++
 tests/qapi-schema/test-qapi.py         | 19 +++++++++++++++++++
 3 files changed, 43 insertions(+)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 78aeec785e..692a7ec7c2 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1066,6 +1066,9 @@ class QAPISchemaVisitor(object):
     def visit_include(self, fname, info):
         pass
 
+    def visit_ifcond(self, ifcond, begin):
+        pass
+
     def visit_builtin_type(self, name, info, json_type):
         pass
 
@@ -1792,12 +1795,24 @@ class QAPISchema(object):
     def visit(self, visitor):
         visitor.visit_begin(self)
         module = None
+        ifcond = None
         for entity in self._entity_list:
             if visitor.visit_needed(entity):
                 if entity.module != module:
+                    if ifcond:
+                        visitor.visit_ifcond(ifcond, False)
+                        ifcond = None
                     module = entity.module
                     visitor.visit_module(module)
+                if entity.ifcond != ifcond:
+                    if ifcond:
+                        visitor.visit_ifcond(ifcond, False)
+                    ifcond = entity.ifcond
+                    if ifcond:
+                        visitor.visit_ifcond(ifcond, True)
                 entity.visit(visitor)
+        if ifcond:
+            visitor.visit_ifcond(ifcond, False)
         visitor.visit_end()
 
 
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index b11682314c..8fe9d7a3a8 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -232,23 +232,32 @@ command __org.qemu_x-command q_obj___org.qemu_x-command-arg -> __org.qemu_x-Unio
    gen=True success_response=True boxed=False
 object TestIfStruct
     member foo: int optional=False
+    if ['defined(TEST_IF_STRUCT)']
 enum TestIfEnum ['foo', 'bar']
+    if ['defined(TEST_IF_ENUM)']
 object q_obj_TestStruct-wrapper
     member data: TestStruct optional=False
 enum TestIfUnionKind ['foo']
+    if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
 object TestIfUnion
     member type: TestIfUnionKind optional=False
     tag type
     case foo: q_obj_TestStruct-wrapper
+    if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
 alternate TestIfAlternate
     tag type
     case foo: int
     case bar: TestStruct
+    if ['defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)']
 object q_obj_TestIfCmd-arg
     member foo: TestIfStruct optional=False
+    if ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)']
 command TestIfCmd q_obj_TestIfCmd-arg -> None
    gen=True success_response=True boxed=False
+    if ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)']
 object q_obj_TestIfEvent-arg
     member foo: TestIfStruct optional=False
+    if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)']
 event TestIfEvent q_obj_TestIfEvent-arg
    boxed=False
+    if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)']
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index 67e417e298..fcdbb5b1ea 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -17,16 +17,26 @@ from qapi.common import QAPIError, QAPISchema, QAPISchemaVisitor
 
 class QAPISchemaTestVisitor(QAPISchemaVisitor):
 
+    def __init__(self):
+        self._ifcond = None
+
     def visit_module(self, name):
         print('module %s' % name)
 
     def visit_include(self, name, info):
         print('include %s' % name)
 
+    def visit_ifcond(self, ifcond, begin):
+        if begin:
+            self._ifcond = ifcond
+        else:
+            self._ifcond = None
+
     def visit_enum_type(self, name, info, values, prefix):
         print('enum %s %s' % (name, values))
         if prefix:
             print('    prefix %s' % prefix)
+        self._print_if(self._ifcond)
 
     def visit_object_type(self, name, info, base, members, variants):
         print('object %s' % name)
@@ -36,10 +46,12 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
             print('    member %s: %s optional=%s' % \
                   (m.name, m.type.name, m.optional))
         self._print_variants(variants)
+        self._print_if(self._ifcond)
 
     def visit_alternate_type(self, name, info, variants):
         print('alternate %s' % name)
         self._print_variants(variants)
+        self._print_if(self._ifcond)
 
     def visit_command(self, name, info, arg_type, ret_type,
                       gen, success_response, boxed):
@@ -47,10 +59,12 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
               (name, arg_type and arg_type.name, ret_type and ret_type.name))
         print('   gen=%s success_response=%s boxed=%s' % \
               (gen, success_response, boxed))
+        self._print_if(self._ifcond)
 
     def visit_event(self, name, info, arg_type, boxed):
         print('event %s %s' % (name, arg_type and arg_type.name))
         print('   boxed=%s' % boxed)
+        self._print_if(self._ifcond)
 
     @staticmethod
     def _print_variants(variants):
@@ -59,6 +73,11 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
             for v in variants.variants:
                 print('    case %s: %s' % (v.name, v.type.name))
 
+    @staticmethod
+    def _print_if(ifcond, indent=4):
+        if ifcond:
+            print('%sif %s' % (' ' * indent, ifcond))
+
 
 try:
     schema = QAPISchema(sys.argv[1])
-- 
2.13.6

[Qemu-devel] [PATCH RFC 10/14] qapi: mcgen() shouldn't indent # lines

[Markus Armbruster]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Skip preprocessor lines when adding indentation, since that would
likely result in invalid code.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi/common.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 692a7ec7c2..1b254d5b32 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1940,8 +1940,8 @@ def cgen(code, **kwds):
     if indent_level:
         indent = genindent(indent_level)
         # re.subn() lacks flags support before Python 2.7, use re.compile()
-        raw = re.subn(re.compile(r'^.', re.MULTILINE),
-                      indent + r'\g<0>', raw)
+        raw = re.subn(re.compile(r'^(?!(#|$))', re.MULTILINE),
+                      indent, raw)
         raw = raw[0]
     return re.sub(re.escape(eatspace) + r' *', '', raw)
 
-- 
2.13.6

[Qemu-devel] [PATCH RFC 11/14] qapi: add #if/#endif helpers

[Markus Armbruster]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Add helpers to wrap generated code with #if/#endif lines.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi/common.py | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 1b254d5b32..1a78dfaf3f 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1973,6 +1973,24 @@ def guardend(name):
                  name=guardname(name))
 
 
+def gen_if(ifcond):
+    ret = ''
+    for ifc in ifcond:
+        ret += mcgen('''
+#if %(cond)s
+''', cond=ifc)
+    return ret
+
+
+def gen_endif(ifcond):
+    ret = ''
+    for ifc in reversed(ifcond):
+        ret += mcgen('''
+#endif /* %(cond)s */
+''', cond=ifc)
+    return ret
+
+
 def gen_enum_lookup(name, values, prefix=None):
     ret = mcgen('''
 
-- 
2.13.6

[Qemu-devel] [PATCH RFC 12/14] qapi-introspect: modify to_qlit() to append ', ' on level > 0

[Markus Armbruster]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The following patch is going to break list entries with #if/#endif, so
they should have the trailing ',' as suffix.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/introspect.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index e5c491d936..c02df95e72 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -29,7 +29,7 @@ def to_qlit(obj, level=0, suppress_first_indent=False):
                 for elt in obj]
         elts.append(indent(level + 1) + "{}")
         ret = 'QLIT_QLIST(((QLitObject[]) {\n'
-        ret += ',\n'.join(elts) + '\n'
+        ret += '\n'.join(elts) + '\n'
         ret += indent(level) + '}))'
     elif isinstance(obj, dict):
         elts = []
@@ -42,6 +42,8 @@ def to_qlit(obj, level=0, suppress_first_indent=False):
         ret += indent(level) + '}))'
     else:
         assert False                # not implemented
+    if level > 0:
+        ret += ','
     return ret
 
 
-- 
2.13.6

[Qemu-devel] [PATCH RFC 13/14] qapi-introspect: Add #if conditions to introspection value

[Markus Armbruster]

Generated tests/test-qapi-introspect.c changes as follows:

    --- test-qapi-introspect.c.old	2018-02-11 17:36:15.039696522 +0100
    +++ tests/test-qapi-introspect.c	2018-02-11 17:36:34.455419852 +0100
    @@ -118,6 +118,9 @@ QLIT_QDICT(((QLitDictEntry[]) {
	     { "ret-type", QLIT_QSTR("14"), },
	     {}
	 })),
    +#if defined(TEST_IF_CMD)
    +#if defined(TEST_IF_STRUCT)
    +
     QLIT_QDICT(((QLitDictEntry[]) {
	     { "arg-type", QLIT_QSTR("15"), },
	     { "meta-type", QLIT_QSTR("command"), },
    @@ -125,12 +128,19 @@ QLIT_QDICT(((QLitDictEntry[]) {
	     { "ret-type", QLIT_QSTR("1"), },
	     {}
	 })),
    +#endif /* defined(TEST_IF_STRUCT) */
    +#endif /* defined(TEST_IF_CMD) */
    +
    +#if defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)
    +
     QLIT_QDICT(((QLitDictEntry[]) {
	     { "arg-type", QLIT_QSTR("16"), },
	     { "meta-type", QLIT_QSTR("event"), },
	     { "name", QLIT_QSTR("TestIfEvent"), },
	     {}
	 })),
    +#endif /* defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT) */
    +
     QLIT_QDICT(((QLitDictEntry[]) {
	     { "members", QLIT_QLIST(((QLitObject[]) {
		 {}

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/introspect.py | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index c02df95e72..342ae09422 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -40,6 +40,12 @@ def to_qlit(obj, level=0, suppress_first_indent=False):
         ret = 'QLIT_QDICT(((QLitDictEntry[]) {\n'
         ret += ',\n'.join(elts) + '\n'
         ret += indent(level) + '}))'
+    elif isinstance(obj, tuple):
+        # Use of tuples for conditionals is a bit of a hack
+        ifcond, begin = obj
+        if begin:
+            return gen_if(ifcond)
+        return gen_endif(ifcond)
     else:
         assert False                # not implemented
     if level > 0:
@@ -101,6 +107,9 @@ extern const QLitObject %(var)s;
         # Ignore types on first pass; visit_end() will pick up used types
         return not isinstance(entity, QAPISchemaType)
 
+    def visit_ifcond(self, ifcond, begin):
+        self._qlits.append((ifcond, begin))
+
     def _name(self, name):
         if self._unmask:
             return name
-- 
2.13.6

[Qemu-devel] [PATCH RFC 14/14] qapi: Add #if conditions to commands, events, types, visitors

[Markus Armbruster]

Example change to generated code:

    diff -rup test-qapi-events.h.old test-qapi-events.h
    --- test-qapi-events.h.old	2018-02-12 07:02:45.672737544 +0100
    +++ test-qapi-events.h	2018-02-12 07:03:01.128517669 +0100
    @@ -30,8 +30,10 @@ void qapi_event_send_event_e(UserDefZero
     void qapi_event_send_event_f(UserDefAlternate *arg, Error **errp);

     void qapi_event_send___org_qemu_x_event(__org_qemu_x_Enum __org_qemu_x_member1, const char *__org_qemu_x_member2, bool has_q_wchar_t, int64_t q_wchar_t, Error **errp);
    +#if defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)

     void qapi_event_send_testifevent(TestIfStruct *foo, Error **errp);
    +#endif /* defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT) */

     typedef enum test_QAPIEvent {
	 TEST_QAPI_EVENT_EVENT_A = 0,

TODO nice blank lines before #if and after #endif
FIXME unclean access of protected members in commands.py

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/commands.py |  7 ++++++-
 scripts/qapi/common.py   | 37 +++++++++++++++++++++++++++++++++++++
 tests/test-qmp-cmds.c    |  4 ++--
 3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 21a7e0dbe6..439a8714e2 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -276,8 +276,13 @@ void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
             self._visited_ret_types[self._genc].add(ret_type)
             self._genc.add(gen_marshal_output(ret_type))
         self._genh.add(gen_marshal_decl(name))
-        self._genc.add(gen_marshal(name, arg_type, boxed, ret_type))
+        self._genc.add(gen_marshal(name, arg_type, boxed, ret_type,))
+        # FIXME unclean access of protected members
+        if self._genc._open_ifcond:
+            self._regy += gen_if(self._genc._open_ifcond)
         self._regy += gen_register_command(name, success_response)
+        if self._genc._open_ifcond:
+            self._regy += gen_endif(self._genc._open_ifcond)
 
 
 def gen_commands(schema, output_dir, prefix):
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 1a78dfaf3f..164d3e2daa 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -2122,6 +2122,9 @@ class QAPIGenC(QAPIGen):
         self._blurb = blurb.strip('\n')
         self._copyright = '\n * '.join(re.findall(r'^Copyright .*', pydoc,
                                                   re.MULTILINE))
+        self._open_ifcond = None
+        self._preamble_needs_ifcond = False
+        self._body_needs_ifcond = False
 
     def _top(self, fname):
         return mcgen('''
@@ -2139,6 +2142,36 @@ class QAPIGenC(QAPIGen):
 ''',
                      blurb=self._blurb, copyright=self._copyright)
 
+    def ifcond(self, ifcond, begin):
+        if begin:
+            assert not self._open_ifcond
+            self._open_ifcond = ifcond
+            self._preamble_needs_ifcond = True
+            self._body_needs_ifcond = True
+        else:
+            assert self._open_ifcond == ifcond
+            if not self._preamble_needs_ifcond:
+                QAPIGen.preamble_add(self, gen_endif(ifcond))
+                # TODO emit blank line
+            if not self._body_needs_ifcond:
+                QAPIGen.add(self, gen_endif(ifcond))
+                # TODO emit blank line
+            self._open_ifcond = None
+
+    def preamble_add(self, text):
+        if self._open_ifcond and self._preamble_needs_ifcond:
+            # TODO emit blank line
+            QAPIGen.preamble_add(self, gen_if(self._open_ifcond))
+            self._preamble_needs_ifcond = False
+        QAPIGen.preamble_add(self, text)
+
+    def add(self, text):
+        if self._open_ifcond and self._body_needs_ifcond:
+            # TODO emit blank line
+            QAPIGen.add(self, gen_if(self._open_ifcond))
+            self._body_needs_ifcond = False
+        QAPIGen.add(self, text)
+
 
 class QAPIGenH(QAPIGenC):
 
@@ -2224,3 +2257,7 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
 #include "%(basename)s.h"
 ''',
                                       basename=basename))
+
+    def visit_ifcond(self, ifcond, begin):
+        self._genc.ifcond(ifcond, begin)
+        self._genh.ifcond(ifcond, begin)
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index c0a3c46439..b709a1fa3a 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -12,11 +12,11 @@
 
 static QmpCommandList qmp_commands;
 
-/* #if defined(TEST_IF_CMD) */
+#if defined(TEST_IF_CMD)
 void qmp_TestIfCmd(TestIfStruct *foo, Error **errp)
 {
 }
-/* #endif */
+#endif
 
 void qmp_user_def_cmd(Error **errp)
 {
-- 
2.13.6

Re: [Qemu-devel] [PATCH RFC 14/14] qapi: Add #if conditions to commands, events, types, visitors

[Marc-Andre Lureau]

Hi

On Mon, Feb 12, 2018 at 8:22 AM, Markus Armbruster <armbru@redhat.com> wrote:
> Example change to generated code:
>
>     diff -rup test-qapi-events.h.old test-qapi-events.h
>     --- test-qapi-events.h.old  2018-02-12 07:02:45.672737544 +0100
>     +++ test-qapi-events.h      2018-02-12 07:03:01.128517669 +0100
>     @@ -30,8 +30,10 @@ void qapi_event_send_event_e(UserDefZero
>      void qapi_event_send_event_f(UserDefAlternate *arg, Error **errp);
>
>      void qapi_event_send___org_qemu_x_event(__org_qemu_x_Enum __org_qemu_x_member1, const char *__org_qemu_x_member2, bool has_q_wchar_t, int64_t q_wchar_t, Error **errp);
>     +#if defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)
>
>      void qapi_event_send_testifevent(TestIfStruct *foo, Error **errp);
>     +#endif /* defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT) */
>
>      typedef enum test_QAPIEvent {
>          TEST_QAPI_EVENT_EVENT_A = 0,
>
> TODO nice blank lines before #if and after #endif
> FIXME unclean access of protected members in commands.py
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

I reviewed the RFC series, and your approach to visit ifcond instead
of having them as attribute argument for the visitor (something you
suggested me initially iirc).

I think the approach is interesting but that single patch shows the
complexity involved. The decorator approach still looks cleaner and
simpler to me. Furthermore, I don't fancy much having to redo and tune
the generation *again* to fix the inden, extra-spaces etc that were
fixed after several revisions (it takes hours to get there, it's
boring). Can't we go first with my approach and then look at replacing
it? Can't one add a "FIXME: replace the decorator with something less
magic" at ifcond_decorator definition for now? Is this code so
critical that it has to be the way you want in the first place? The
approach to take it first and improve it worked very well for
qapi2texi, it just took a few more days for you (and reviewers) to
improve it. I'd suggest we work that way instead of having me rewrite
and rewrite until you are happy (which is something I can't do right
without many painful iterations for you and me).

thanks

> ---
>  scripts/qapi/commands.py |  7 ++++++-
>  scripts/qapi/common.py   | 37 +++++++++++++++++++++++++++++++++++++
>  tests/test-qmp-cmds.c    |  4 ++--
>  3 files changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
> index 21a7e0dbe6..439a8714e2 100644
> --- a/scripts/qapi/commands.py
> +++ b/scripts/qapi/commands.py
> @@ -276,8 +276,13 @@ void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
>              self._visited_ret_types[self._genc].add(ret_type)
>              self._genc.add(gen_marshal_output(ret_type))
>          self._genh.add(gen_marshal_decl(name))
> -        self._genc.add(gen_marshal(name, arg_type, boxed, ret_type))
> +        self._genc.add(gen_marshal(name, arg_type, boxed, ret_type,))
> +        # FIXME unclean access of protected members
> +        if self._genc._open_ifcond:
> +            self._regy += gen_if(self._genc._open_ifcond)
>          self._regy += gen_register_command(name, success_response)
> +        if self._genc._open_ifcond:
> +            self._regy += gen_endif(self._genc._open_ifcond)
>
>
>  def gen_commands(schema, output_dir, prefix):
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 1a78dfaf3f..164d3e2daa 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -2122,6 +2122,9 @@ class QAPIGenC(QAPIGen):
>          self._blurb = blurb.strip('\n')
>          self._copyright = '\n * '.join(re.findall(r'^Copyright .*', pydoc,
>                                                    re.MULTILINE))
> +        self._open_ifcond = None
> +        self._preamble_needs_ifcond = False
> +        self._body_needs_ifcond = False
>
>      def _top(self, fname):
>          return mcgen('''
> @@ -2139,6 +2142,36 @@ class QAPIGenC(QAPIGen):
>  ''',
>                       blurb=self._blurb, copyright=self._copyright)
>
> +    def ifcond(self, ifcond, begin):
> +        if begin:
> +            assert not self._open_ifcond
> +            self._open_ifcond = ifcond
> +            self._preamble_needs_ifcond = True
> +            self._body_needs_ifcond = True
> +        else:
> +            assert self._open_ifcond == ifcond
> +            if not self._preamble_needs_ifcond:
> +                QAPIGen.preamble_add(self, gen_endif(ifcond))
> +                # TODO emit blank line
> +            if not self._body_needs_ifcond:
> +                QAPIGen.add(self, gen_endif(ifcond))
> +                # TODO emit blank line
> +            self._open_ifcond = None
> +
> +    def preamble_add(self, text):
> +        if self._open_ifcond and self._preamble_needs_ifcond:
> +            # TODO emit blank line
> +            QAPIGen.preamble_add(self, gen_if(self._open_ifcond))
> +            self._preamble_needs_ifcond = False
> +        QAPIGen.preamble_add(self, text)
> +
> +    def add(self, text):
> +        if self._open_ifcond and self._body_needs_ifcond:
> +            # TODO emit blank line
> +            QAPIGen.add(self, gen_if(self._open_ifcond))
> +            self._body_needs_ifcond = False
> +        QAPIGen.add(self, text)
> +
>
>  class QAPIGenH(QAPIGenC):
>
> @@ -2224,3 +2257,7 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
>  #include "%(basename)s.h"
>  ''',
>                                        basename=basename))
> +
> +    def visit_ifcond(self, ifcond, begin):
> +        self._genc.ifcond(ifcond, begin)
> +        self._genh.ifcond(ifcond, begin)
> diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
> index c0a3c46439..b709a1fa3a 100644
> --- a/tests/test-qmp-cmds.c
> +++ b/tests/test-qmp-cmds.c
> @@ -12,11 +12,11 @@
>
>  static QmpCommandList qmp_commands;
>
> -/* #if defined(TEST_IF_CMD) */
> +#if defined(TEST_IF_CMD)
>  void qmp_TestIfCmd(TestIfStruct *foo, Error **errp)
>  {
>  }
> -/* #endif */
> +#endif
>
>  void qmp_user_def_cmd(Error **errp)
>  {
> --
> 2.13.6
>

Re: [Qemu-devel] [PATCH RFC 14/14] qapi: Add #if conditions to commands, events, types, visitors

[Markus Armbruster]

Marc-Andre Lureau <mlureau@redhat.com> writes:

> Hi
>
> On Mon, Feb 12, 2018 at 8:22 AM, Markus Armbruster <armbru@redhat.com> wrote:
>> Example change to generated code:
>>
>>     diff -rup test-qapi-events.h.old test-qapi-events.h
>>     --- test-qapi-events.h.old  2018-02-12 07:02:45.672737544 +0100
>>     +++ test-qapi-events.h      2018-02-12 07:03:01.128517669 +0100
>>     @@ -30,8 +30,10 @@ void qapi_event_send_event_e(UserDefZero
>>      void qapi_event_send_event_f(UserDefAlternate *arg, Error **errp);
>>
>>      void qapi_event_send___org_qemu_x_event(__org_qemu_x_Enum __org_qemu_x_member1, const char *__org_qemu_x_member2, bool has_q_wchar_t, int64_t q_wchar_t, Error **errp);
>>     +#if defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)
>>
>>      void qapi_event_send_testifevent(TestIfStruct *foo, Error **errp);
>>     +#endif /* defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT) */
>>
>>      typedef enum test_QAPIEvent {
>>          TEST_QAPI_EVENT_EVENT_A = 0,
>>
>> TODO nice blank lines before #if and after #endif
>> FIXME unclean access of protected members in commands.py
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> I reviewed the RFC series, and your approach to visit ifcond instead
> of having them as attribute argument for the visitor (something you
> suggested me initially iirc).

I think I tossed out the idea, no more.

> I think the approach is interesting but that single patch shows the
> complexity involved. The decorator approach still looks cleaner and
> simpler to me.

De gustibus...

For what it's worth, I disliked the decorator magic enough to write this
series.

>                Furthermore, I don't fancy much having to redo and tune
> the generation *again* to fix the inden, extra-spaces etc that were
> fixed after several revisions (it takes hours to get there, it's
> boring). Can't we go first with my approach and then look at replacing
> it? Can't one add a "FIXME: replace the decorator with something less
> magic" at ifcond_decorator definition for now? Is this code so
> critical that it has to be the way you want in the first place? The
> approach to take it first and improve it worked very well for
> qapi2texi, it just took a few more days for you (and reviewers) to
> improve it. I'd suggest we work that way instead of having me rewrite
> and rewrite until you are happy (which is something I can't do right
> without many painful iterations for you and me).

This is up to the backup QAPI maintainer now :)

Re: [Qemu-devel] [PATCH RFC 14/14] qapi: Add #if conditions to commands, events, types, visitors

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1327 bytes --]

On 02/23/2018 12:13 PM, Markus Armbruster wrote:

[reviving this RFC]

> 
> For what it's worth, I disliked the decorator magic enough to write this
> series.
> 
>>                Furthermore, I don't fancy much having to redo and tune
>> the generation *again* to fix the inden, extra-spaces etc that were
>> fixed after several revisions (it takes hours to get there, it's
>> boring). Can't we go first with my approach and then look at replacing
>> it? Can't one add a "FIXME: replace the decorator with something less
>> magic" at ifcond_decorator definition for now? Is this code so
>> critical that it has to be the way you want in the first place? The
>> approach to take it first and improve it worked very well for
>> qapi2texi, it just took a few more days for you (and reviewers) to
>> improve it. I'd suggest we work that way instead of having me rewrite
>> and rewrite until you are happy (which is something I can't do right
>> without many painful iterations for you and me).
> 
> This is up to the backup QAPI maintainer now :)

Except my (cop-out?) decision was that it was not 2.12 material, so now
that 2.13 is opening up it's somewhat back to you...

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]