[Qemu-devel] [PATCH RFC 00/10] qapi script: support enum as discriminator and other improves

[Qemu-devel] [PATCH RFC 00/10] qapi script: support enum as discriminator and other improves

[Wenchao Xia]

Patch 1 and 9 fix two memleak issue.
Patch 2-6 add support for enum type as discriminator
Patch 7 add "_base" support which can reduce number of defined structure
Patch 8 fix enum name generation issue, now AIOContext->AIO_CONTEXT, X86CPU->
X86_CPU.
Patch 10 are a butch of test cases.

Wenchao Xia (10):
  1 qapi: fix memleak by add implict struct functions in dealloc visitor
  2 qapi script: remember enum values
  3 qapi script: check correctness of discriminator values in union
  4 qapi script: code move for generate_enum_name()
  5 qapi script: use same function to generate enum string
  6 qapi script: not generate hidden enum type for pre-defined enum discriminator
  7 qapi script: support direct inheritance for struct
  8 qapi script: do not add "_" for every capitalized char in enum
  9 tests: fix memleak in error path test for input visitor
  10 tests: add cases for inherited struct and union with discriminator

 docs/qapi-code-gen.txt                  |   21 +++
 include/qapi/qmp/qerror.h               |    2 +-
 qapi/qapi-dealloc-visitor.c             |   20 +++
 scripts/qapi-types.py                   |   34 +++---
 scripts/qapi-visit.py                   |   50 +++++--
 scripts/qapi.py                         |   84 ++++++++++-
 target-i386/cpu.c                       |    2 +-
 tests/qapi-schema/comments.out          |    2 +-
 tests/qapi-schema/qapi-schema-test.json |   36 +++++
 tests/qapi-schema/qapi-schema-test.out  |   19 +++-
 tests/test-qmp-input-visitor.c          |  189 ++++++++++++++++++++++++
 tests/test-qmp-output-visitor.c         |  238 +++++++++++++++++++++++++++++++
 12 files changed, 660 insertions(+), 37 deletions(-)

[Qemu-devel] [PATCH RFC 01/10] qapi: fix memleak by add implict struct functions in dealloc visitor

[Wenchao Xia]

Otherwise they are leaked in a qap_free_STRUCTURE() call.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Cc: qemu-stable@nongnu.org
---
 qapi/qapi-dealloc-visitor.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index 1334de3..dc53545 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -76,6 +76,24 @@ static void qapi_dealloc_end_struct(Visitor *v, Error **errp)
     }
 }
 
+static void qapi_dealloc_start_implicit_struct(Visitor *v,
+                                               void **obj,
+                                               size_t size,
+                                               Error **errp)
+{
+    QapiDeallocVisitor *qov = to_qov(v);
+    qapi_dealloc_push(qov, obj);
+}
+
+static void qapi_dealloc_end_implicit_struct(Visitor *v, Error **errp)
+{
+    QapiDeallocVisitor *qov = to_qov(v);
+    void **obj = qapi_dealloc_pop(qov);
+    if (obj) {
+        g_free(*obj);
+    }
+}
+
 static void qapi_dealloc_start_list(Visitor *v, const char *name, Error **errp)
 {
     QapiDeallocVisitor *qov = to_qov(v);
@@ -162,6 +180,8 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
 
     v->visitor.start_struct = qapi_dealloc_start_struct;
     v->visitor.end_struct = qapi_dealloc_end_struct;
+    v->visitor.start_implicit_struct = qapi_dealloc_start_implicit_struct;
+    v->visitor.end_implicit_struct = qapi_dealloc_end_implicit_struct;
     v->visitor.start_list = qapi_dealloc_start_list;
     v->visitor.next_list = qapi_dealloc_next_list;
     v->visitor.end_list = qapi_dealloc_end_list;
-- 
1.7.1

[Qemu-devel] [PATCH RFC 02/10] qapi script: remember enum values

[Wenchao Xia]

Later other script will need to check the enum values.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 scripts/qapi.py                        |   18 ++++++++++++++----
 tests/qapi-schema/comments.out         |    2 +-
 tests/qapi-schema/qapi-schema-test.out |    4 +++-
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 750e9fb..82f586e 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -169,7 +169,7 @@ def parse_schema(fp):
 
     for expr in schema.exprs:
         if expr.has_key('enum'):
-            add_enum(expr['enum'])
+            add_enum(expr['enum'], expr['data'])
         elif expr.has_key('union'):
             add_union(expr)
             add_enum('%sKind' % expr['union'])
@@ -289,13 +289,23 @@ def find_union(name):
             return union
     return None
 
-def add_enum(name):
+def add_enum(name, enum_values = None):
     global enum_types
-    enum_types.append(name)
+    enum_types.append({"enum_name": name, "enum_values": enum_values})
+
+def find_enum(name):
+    global enum_types
+    for enum in enum_types:
+        if enum['enum_name'] == name:
+            return enum
+    return None
 
 def is_enum(name):
     global enum_types
-    return (name in enum_types)
+    for enum in enum_types:
+        if enum['enum_name'] == name:
+            return True
+    return False
 
 def c_type(name):
     if name == 'str':
diff --git a/tests/qapi-schema/comments.out b/tests/qapi-schema/comments.out
index e3bd904..4ce3dcf 100644
--- a/tests/qapi-schema/comments.out
+++ b/tests/qapi-schema/comments.out
@@ -1,3 +1,3 @@
 [OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
-['Status']
+[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}]
 []
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 3851880..ad74cdb 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -11,7 +11,9 @@
  OrderedDict([('command', 'user_def_cmd1'), ('data', OrderedDict([('ud1a', 'UserDefOne')]))]),
  OrderedDict([('command', 'user_def_cmd2'), ('data', OrderedDict([('ud1a', 'UserDefOne'), ('ud1b', 'UserDefOne')])), ('returns', 'UserDefTwo')]),
  OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])]
-['EnumOne', 'UserDefUnionKind', 'UserDefNativeListUnionKind']
+[{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
+ {'enum_name': 'UserDefUnionKind', 'enum_values': None},
+ {'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None}]
 [OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
  OrderedDict([('type', 'UserDefOne'), ('data', OrderedDict([('integer', 'int'), ('string', 'str'), ('*enum1', 'EnumOne')]))]),
  OrderedDict([('type', 'UserDefTwo'), ('data', OrderedDict([('string', 'str'), ('dict', OrderedDict([('string', 'str'), ('dict', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')])), ('*dict2', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')]))]))]))]),
-- 
1.7.1

[Qemu-devel] [PATCH RFC 03/10] qapi script: check correctness of discriminator values in union

[Wenchao Xia]

It will check whether the values specfied are wrotten correctly when
discriminator is a pre-defined enum type, which help check whether the
schema is in good form.

It is allowed that, not every value in enum is used, so do not check
that case.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 scripts/qapi-visit.py |   11 +++++++++++
 scripts/qapi.py       |   33 +++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index c39e628..803d3eb 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -249,6 +249,17 @@ def generate_visit_union(expr):
         assert not base
         return generate_visit_anon_union(name, members)
 
+    # If discriminator is specified and it is a pre-defined enum in schema,
+    # check its correctness
+    enum_define = descriminator_find_enum_define(expr)
+    if enum_define:
+        for key in members:
+            if not key in enum_define["enum_values"]:
+                sys.stderr.write("Discriminator value '%s' not found in "
+                                 "enum '%s'\n" %
+                                 (key, enum_define["enum_name"]))
+                sys.exit(1)
+
     ret = generate_visit_enum('%sKind' % name, members.keys())
 
     if base:
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 82f586e..235df4f 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -383,3 +383,36 @@ def guardend(name):
 
 ''',
                  name=guardname(name))
+
+# This function can be used to check whether "base" is valid
+def find_base_fields(base):
+    base_struct_define = find_struct(base)
+    if not base_struct_define:
+        return None
+    return base_struct_define.get('data')
+
+# Return the descriminator enum define, if discriminator is specified in
+# @expr and it is a pre-defined enum type
+def descriminator_find_enum_define(expr):
+    discriminator = expr.get('discriminator')
+    base = expr.get('base')
+
+    # Only support discriminator when base present
+    if not (discriminator and base):
+        return None
+
+    base_fields = find_base_fields(base)
+
+    if not base_fields:
+        sys.stderr.write("Base '%s' is not a valid type\n"
+                         % base)
+        sys.exit(1)
+
+    descriminator_type = base_fields.get(discriminator)
+
+    if not descriminator_type:
+        sys.stderr.write("Discriminator '%s' not found in schema\n"
+                         % discriminator)
+        sys.exit(1)
+
+    return find_enum(descriminator_type)
-- 
1.7.1

[Qemu-devel] [PATCH RFC 04/10] qapi script: code move for generate_enum_name()

[Wenchao Xia]

Later both qapi-types.py and qapi-visit.py need a common function
for enum name generation.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 scripts/qapi-types.py |   10 ----------
 scripts/qapi.py       |   10 ++++++++++
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 4a1652b..88bf76a 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -127,16 +127,6 @@ const char *%(name)s_lookup[] = {
 ''')
     return ret
 
-def generate_enum_name(name):
-    if name.isupper():
-        return c_fun(name, False)
-    new_name = ''
-    for c in c_fun(name, False):
-        if c.isupper():
-            new_name += '_'
-        new_name += c
-    return new_name.lstrip('_').upper()
-
 def generate_enum(name, values):
     lookup_decl = mcgen('''
 extern const char *%(name)s_lookup[];
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 235df4f..9e2dd70 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -416,3 +416,13 @@ def descriminator_find_enum_define(expr):
         sys.exit(1)
 
     return find_enum(descriminator_type)
+
+def generate_enum_name(name):
+    if name.isupper():
+        return c_fun(name, False)
+    new_name = ''
+    for c in c_fun(name, False):
+        if c.isupper():
+            new_name += '_'
+        new_name += c
+    return new_name.lstrip('_').upper()
-- 
1.7.1

[Qemu-devel] [PATCH RFC 05/10] qapi script: use same function to generate enum string

[Wenchao Xia]

One function one rule, so the enum string generating have same
behavior for different caller. If multiple caller exist for one
enum define in schema, it is for sure the generated string is
identical.

Note before the patch qapi-visit.py used custom function to
generate the string in union visit, although the patch changes it,
the final string generated is not changed. The custom function used
before will met problem when capitalized discriminator value is
introduced.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 scripts/qapi-types.py |    6 +++---
 scripts/qapi-visit.py |    8 +++++---
 scripts/qapi.py       |   15 +++++++++++----
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 88bf76a..914f055 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -144,11 +144,11 @@ typedef enum %(name)s
 
     i = 0
     for value in enum_values:
+        enum_full_value_string = generate_enum_full_value_string(name, value)
         enum_decl += mcgen('''
-    %(abbrev)s_%(value)s = %(i)d,
+    %(enum_full_value_string)s = %(i)d,
 ''',
-                     abbrev=de_camel_case(name).upper(),
-                     value=generate_enum_name(value),
+                     enum_full_value_string=enum_full_value_string,
                      i=i)
         i += 1
 
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 803d3eb..9fc7c4c 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -261,6 +261,7 @@ def generate_visit_union(expr):
                 sys.exit(1)
 
     ret = generate_visit_enum('%sKind' % name, members.keys())
+    discriminator_type_name = '%sKind' % (name)
 
     if base:
         base_fields = find_struct(base)['data']
@@ -313,13 +314,14 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
                     visit_end_implicit_struct(m, &err);
                 }'''
 
+        enum_full_value_string = \
+                  generate_enum_full_value_string(discriminator_type_name, key)
         ret += mcgen('''
-            case %(abbrev)s_KIND_%(enum)s:
+            case %(enum_full_value_string)s:
                 ''' + fmt + '''
                 break;
 ''',
-                abbrev = de_camel_case(name).upper(),
-                enum = c_fun(de_camel_case(key),False).upper(),
+                enum_full_value_string = enum_full_value_string,
                 c_type=type_name(members[key]),
                 c_name=c_fun(key))
 
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 9e2dd70..43a3720 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -417,12 +417,19 @@ def descriminator_find_enum_define(expr):
 
     return find_enum(descriminator_type)
 
-def generate_enum_name(name):
-    if name.isupper():
-        return c_fun(name, False)
+def _generate_enum_value_string(value):
+    if value.isupper():
+        return c_fun(value, False)
     new_name = ''
-    for c in c_fun(name, False):
+    for c in c_fun(value, False):
         if c.isupper():
             new_name += '_'
         new_name += c
     return new_name.lstrip('_').upper()
+
+def generate_enum_full_value_string(enum_name, enum_value):
+    # generate abbrev string
+    abbrev_string = de_camel_case(enum_name).upper()
+    # generate value string
+    value_string = _generate_enum_value_string(enum_value)
+    return "%s_%s" % (abbrev_string, value_string)
-- 
1.7.1

[Qemu-devel] [PATCH RFC 06/10] qapi script: not generate hidden enum type for pre-defined enum discriminator

[Wenchao Xia]

By default, any union will automatically generate a enum type as
"[UnionName]Kind" in C code, and it is duplicated when the discriminator
is specified as a pre-defined enum type in schema. After this patch,
the pre-defined enum type  will be really used as the switch case
condition in generated C code.

In short, enum type as discriminator is fully supported now.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 scripts/qapi-types.py |   18 ++++++++++++++----
 scripts/qapi-visit.py |   19 ++++++++++++++-----
 scripts/qapi.py       |    4 +++-
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 914f055..a096c36 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -201,14 +201,21 @@ def generate_union(expr):
     base = expr.get('base')
     discriminator = expr.get('discriminator')
 
+    enum_define = descriminator_find_enum_define(expr)
+    if enum_define:
+        discriminator_type_name = enum_define['enum_name']
+    else:
+        discriminator_type_name = '%sKind' % (name)
+
     ret = mcgen('''
 struct %(name)s
 {
-    %(name)sKind kind;
+    %(discriminator_type_name)s kind;
     union {
         void *data;
 ''',
-                name=name)
+                name=name,
+                discriminator_type_name=discriminator_type_name)
 
     for key in typeinfo:
         ret += mcgen('''
@@ -389,8 +396,11 @@ for expr in exprs:
         fdef.write(generate_enum_lookup(expr['enum'], expr['data']))
     elif expr.has_key('union'):
         ret += generate_fwd_struct(expr['union'], expr['data']) + "\n"
-        ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
-        fdef.write(generate_enum_lookup('%sKind' % expr['union'], expr['data'].keys()))
+        enum_define = descriminator_find_enum_define(expr)
+        if not enum_define:
+            ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
+            fdef.write(generate_enum_lookup('%sKind' % expr['union'],
+                                            expr['data'].keys()))
         if expr.get('discriminator') == {}:
             fdef.write(generate_anon_union_qtypes(expr))
     else:
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 9fc7c4c..2b13ad0 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -260,8 +260,12 @@ def generate_visit_union(expr):
                                  (key, enum_define["enum_name"]))
                 sys.exit(1)
 
-    ret = generate_visit_enum('%sKind' % name, members.keys())
-    discriminator_type_name = '%sKind' % (name)
+        ret = ""
+        discriminator_type_name = enum_define['enum_name']
+    else:
+        ret = generate_visit_enum('%sKind' % name, members.keys())
+        discriminator_type_name = '%sKind' % (name)
+
 
     if base:
         base_fields = find_struct(base)['data']
@@ -296,11 +300,12 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
 
     pop_indent()
     ret += mcgen('''
-        visit_type_%(name)sKind(m, &(*obj)->kind, "%(type)s", &err);
+        visit_type_%(discriminator_type_name)s(m, &(*obj)->kind, "%(type)s", &err);
         if (!err) {
             switch ((*obj)->kind) {
 ''',
-                 name=name, type="type" if not discriminator else discriminator)
+                 discriminator_type_name=discriminator_type_name,
+                 type="type" if not discriminator else discriminator)
 
     for key in members:
         if not discriminator:
@@ -514,7 +519,11 @@ for expr in exprs:
         ret += generate_visit_list(expr['union'], expr['data'])
         fdef.write(ret)
 
-        ret = generate_decl_enum('%sKind' % expr['union'], expr['data'].keys())
+        enum_define = descriminator_find_enum_define(expr)
+        ret = ""
+        if not enum_define:
+            ret = generate_decl_enum('%sKind' % expr['union'],
+                                     expr['data'].keys())
         ret += generate_declaration(expr['union'], expr['data'])
         fdecl.write(ret)
     elif expr.has_key('enum'):
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 43a3720..8e50015 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -172,7 +172,9 @@ def parse_schema(fp):
             add_enum(expr['enum'], expr['data'])
         elif expr.has_key('union'):
             add_union(expr)
-            add_enum('%sKind' % expr['union'])
+            enum_define = descriminator_find_enum_define(expr)
+            if not enum_define:
+                add_enum('%sKind' % expr['union'])
         elif expr.has_key('type'):
             add_struct(expr)
         exprs.append(expr)
-- 
1.7.1

[Qemu-devel] [PATCH RFC 07/10] qapi script: support direct inheritance for struct

[Wenchao Xia]

Now it is possible to inherit another struct inside data directly,
which saves trouble to define trivial structure.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 docs/qapi-code-gen.txt |   21 +++++++++++++++++++++
 scripts/qapi-visit.py  |   14 ++++++++++----
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 0728f36..3e42ff4 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -70,6 +70,27 @@ both fields like this:
  { "file": "/some/place/my-image",
    "backing": "/some/place/my-backing-file" }
 
+It is possible to directly inherit other struct by keyword '_base':
+
+ { 'type': 'NetworkConnectionInfo', 'data': { 'host': 'str', 'service': 'str' } }
+ { 'type': 'VncConnectionInfo',
+   'data': {
+      'server': {
+          '_base': 'NetworkConnectionInfo',
+          '*auth': 'str' },
+      'client': 'NetworkConnectionInfo'
+      } }
+
+Result on the wire could be:
+
+{
+  "server": { "host": "192.168.1.1",
+              "service": "8080",
+              "auth': "none" },
+  "client": { "host": "192.168.1.2",
+              "service": "1223" }
+}
+
 === Enumeration types ===
 
 An enumeration type is a dictionary containing a single key whose value is a
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 2b13ad0..f0f0942 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -17,7 +17,7 @@ import os
 import getopt
 import errno
 
-def generate_visit_struct_fields(name, field_prefix, fn_prefix, members, base = None):
+def generate_visit_struct_fields(name, field_prefix, fn_prefix, members, base = None, base_name = 'base'):
     substructs = []
     ret = ''
     full_name = name if not fn_prefix else "%s_%s" % (name, fn_prefix)
@@ -30,8 +30,14 @@ def generate_visit_struct_fields(name, field_prefix, fn_prefix, members, base =
                 nested_fn_prefix = "%s_%s" % (fn_prefix, argname)
 
             nested_field_prefix = "%s%s." % (field_prefix, argname)
+
+            _base = argentry.get('_base')
+            if _base:
+                del argentry['_base']
+
             ret += generate_visit_struct_fields(name, nested_field_prefix,
-                                                nested_fn_prefix, argentry)
+                                                nested_fn_prefix, argentry,
+                                                _base, '_base')
 
     ret += mcgen('''
 
@@ -44,7 +50,7 @@ static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s ** obj, Error *
 
     if base:
         ret += mcgen('''
-visit_start_implicit_struct(m, obj ? (void**) &(*obj)->%(c_name)s : NULL, sizeof(%(type)s), &err);
+visit_start_implicit_struct(m, obj ? (void**) &(*obj)->%(c_prefix)s%(c_name)s : NULL, sizeof(%(type)s), &err);
 if (!err) {
     visit_type_%(type)s_fields(m, obj ? &(*obj)->%(c_prefix)s%(c_name)s : NULL, &err);
     error_propagate(errp, err);
@@ -53,7 +59,7 @@ if (!err) {
 }
 ''',
                      c_prefix=c_var(field_prefix),
-                     type=type_name(base), c_name=c_var('base'))
+                     type=type_name(base), c_name=c_var(base_name))
 
     for argname, argentry, optional, structured in parse_args(members):
         if optional:
-- 
1.7.1

[Qemu-devel] [PATCH RFC 08/10] qapi script: do not add "_" for every capitalized char in enum

[Wenchao Xia]

Now "enum AIOContext" will generate AIO_CONTEXT instead of A_I_O_CONTEXT,
"X86CPU" will generate X86_CPU instead of X86_C_P_U.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 include/qapi/qmp/qerror.h |    2 +-
 scripts/qapi.py           |   26 +++++++++++++++++++-------
 target-i386/cpu.c         |    2 +-
 3 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index c30c2f6..6fa07b4 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -160,7 +160,7 @@ void assert_no_error(Error *err);
     ERROR_CLASS_GENERIC_ERROR, "Invalid JSON syntax"
 
 #define QERR_KVM_MISSING_CAP \
-    ERROR_CLASS_K_V_M_MISSING_CAP, "Using KVM without %s, %s unavailable"
+    ERROR_CLASS_KVM_MISSING_CAP, "Using KVM without %s, %s unavailable"
 
 #define QERR_MIGRATION_ACTIVE \
     ERROR_CLASS_GENERIC_ERROR, "There's a migration process in progress"
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 8e50015..9f5b63e 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -419,19 +419,31 @@ def descriminator_find_enum_define(expr):
 
     return find_enum(descriminator_type)
 
-def _generate_enum_value_string(value):
+# ENUMName -> ENUM_NAME, EnumName1 -> ENUM_NAME1
+# ENUM_NAME -> ENUM_NAME, ENUM_NAME1 -> ENUM_NAME1, ENUM_Name2 -> ENUM_NAME2
+# ENUM24_Name -> ENUM24_NAME
+def _generate_enum_string(value):
+    c_fun_str = c_fun(value, False)
     if value.isupper():
-        return c_fun(value, False)
+        return c_fun_str
+
     new_name = ''
-    for c in c_fun(value, False):
-        if c.isupper():
-            new_name += '_'
+    l = len(c_fun_str)
+    for i in range(l):
+        c = c_fun_str[i]
+        # When c is upper and no "_" appear before, do more check
+        if c.isupper() and (i > 0) and c_fun_str[i - 1] != "_":
+            # Case 1: Next string is lower
+            # Case 2: previous string is digit
+            if (i < (l - 1) and c_fun_str[i + 1].islower()) or \
+               c_fun_str[i - 1].isdigit():
+                new_name += '_'
         new_name += c
     return new_name.lstrip('_').upper()
 
 def generate_enum_full_value_string(enum_name, enum_value):
     # generate abbrev string
-    abbrev_string = de_camel_case(enum_name).upper()
+    abbrev_string = _generate_enum_string(enum_name)
     # generate value string
-    value_string = _generate_enum_value_string(enum_value)
+    value_string = _generate_enum_string(enum_value)
     return "%s_%s" % (abbrev_string, value_string)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 864c80e..6ba1945 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -315,7 +315,7 @@ typedef struct X86RegisterInfo32 {
 } X86RegisterInfo32;
 
 #define REGISTER(reg) \
-    [R_##reg] = { .name = #reg, .qapi_enum = X86_C_P_U_REGISTER32_##reg }
+    [R_##reg] = { .name = #reg, .qapi_enum = X86_CPU_REGISTER32_##reg }
 X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = {
     REGISTER(EAX),
     REGISTER(ECX),
-- 
1.7.1

[Qemu-devel] [PATCH RFC 09/10] tests: fix memleak in error path test for input visitor

[Wenchao Xia]

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Cc: qemu-stable@nongnu.org
---
 tests/test-qmp-input-visitor.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 0beb8fb..1e1c6fa 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -604,6 +604,7 @@ static void test_visitor_in_errors(TestInputVisitorData *data,
     g_assert(error_is_set(&errp));
     g_assert(p->string == NULL);
 
+    error_free(errp);
     g_free(p->string);
     g_free(p);
 }
-- 
1.7.1

[Qemu-devel] [PATCH RFC 10/10] tests: add cases for inherited struct and union with discriminator

[Wenchao Xia]

Test for inherit and complex union.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 tests/qapi-schema/qapi-schema-test.json |   36 +++++
 tests/qapi-schema/qapi-schema-test.out  |   15 ++
 tests/test-qmp-input-visitor.c          |  188 ++++++++++++++++++++++++
 tests/test-qmp-output-visitor.c         |  238 +++++++++++++++++++++++++++++++
 4 files changed, 477 insertions(+), 0 deletions(-)

diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index fe5af75..b15789d 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -21,8 +21,29 @@
             'dict1': { 'string1': 'str',
                        'dict2': { 'userdef1': 'UserDefOne', 'string2': 'str' },
                        '*dict3': { 'userdef2': 'UserDefOne', 'string3': 'str' } } } }
+# for testing base
+{ 'type': 'UserDefBase',
+  'data': { 'string': 'str', 'enum1': 'EnumOne' } }
+
+{ 'type': 'UserDefInherit',
+  'base': 'UserDefBase',
+  'data': { 'boolean': 'bool', 'integer': 'int' } }
+
+{ 'type': 'UserDefDirectInherit',
+  'data': { 'boolean': 'bool',
+            'dict': { '_base': 'UserDefBase',
+                      'integer': 'int' } } }
+
+{ 'type': 'UserDefMixedInherit',
+  'base': 'UserDefBase',
+  'data': { 'boolean': 'bool',
+            'dict': { '_base': 'UserDefBase',
+                      'integer': 'int' } } }
 
 # for testing unions
+{ 'type': 'UserDefBase0',
+  'data': { 'base-string0': 'str', 'base-enum0': 'EnumOne' } }
+
 { 'type': 'UserDefA',
   'data': { 'boolean': 'bool' } }
 
@@ -32,6 +53,21 @@
 { 'union': 'UserDefUnion',
   'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } }
 
+{ 'union': 'UserDefBaseUnion',
+  'base': 'UserDefBase0',
+  'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } }
+
+{ 'union': 'UserDefDiscriminatorUnion',
+  'base': 'UserDefBase0',
+  'discriminator' : 'base-string0',
+  'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } }
+
+# A complex type
+{ 'union': 'UserDefEnumDiscriminatorUnion',
+  'base': 'UserDefBase0',
+  'discriminator' : 'base-enum0',
+  'data': { 'value1' : 'UserDefA', 'value2' : 'UserDefMixedInherit' } }
+
 # for testing native lists
 { 'union': 'UserDefNativeListUnion',
   'data': { 'integer': ['int'],
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index ad74cdb..55de233 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -3,9 +3,17 @@
  OrderedDict([('type', 'UserDefOne'), ('data', OrderedDict([('integer', 'int'), ('string', 'str'), ('*enum1', 'EnumOne')]))]),
  OrderedDict([('type', 'UserDefTwo'), ('data', OrderedDict([('string', 'str'), ('dict', OrderedDict([('string', 'str'), ('dict', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')])), ('*dict2', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')]))]))]))]),
  OrderedDict([('type', 'UserDefNested'), ('data', OrderedDict([('string0', 'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', OrderedDict([('userdef1', 'UserDefOne'), ('string2', 'str')])), ('*dict3', OrderedDict([('userdef2', 'UserDefOne'), ('string3', 'str')]))]))]))]),
+ OrderedDict([('type', 'UserDefBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
+ OrderedDict([('type', 'UserDefInherit'), ('base', 'UserDefBase'), ('data', OrderedDict([('boolean', 'bool'), ('integer', 'int')]))]),
+ OrderedDict([('type', 'UserDefDirectInherit'), ('data', OrderedDict([('boolean', 'bool'), ('dict', OrderedDict([('_base', 'UserDefBase'), ('integer', 'int')]))]))]),
+ OrderedDict([('type', 'UserDefMixedInherit'), ('base', 'UserDefBase'), ('data', OrderedDict([('boolean', 'bool'), ('dict', OrderedDict([('_base', 'UserDefBase'), ('integer', 'int')]))]))]),
+ OrderedDict([('type', 'UserDefBase0'), ('data', OrderedDict([('base-string0', 'str'), ('base-enum0', 'EnumOne')]))]),
  OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
  OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
  OrderedDict([('union', 'UserDefUnion'), ('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]),
+ OrderedDict([('union', 'UserDefBaseUnion'), ('base', 'UserDefBase0'), ('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]),
+ OrderedDict([('union', 'UserDefDiscriminatorUnion'), ('base', 'UserDefBase0'), ('discriminator', 'base-string0'), ('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]),
+ OrderedDict([('union', 'UserDefEnumDiscriminatorUnion'), ('base', 'UserDefBase0'), ('discriminator', 'base-enum0'), ('data', OrderedDict([('value1', 'UserDefA'), ('value2', 'UserDefMixedInherit')]))]),
  OrderedDict([('union', 'UserDefNativeListUnion'), ('data', OrderedDict([('integer', ['int']), ('s8', ['int8']), ('s16', ['int16']), ('s32', ['int32']), ('s64', ['int64']), ('u8', ['uint8']), ('u16', ['uint16']), ('u32', ['uint32']), ('u64', ['uint64']), ('number', ['number']), ('boolean', ['bool']), ('string', ['str'])]))]),
  OrderedDict([('command', 'user_def_cmd'), ('data', OrderedDict())]),
  OrderedDict([('command', 'user_def_cmd1'), ('data', OrderedDict([('ud1a', 'UserDefOne')]))]),
@@ -13,11 +21,18 @@
  OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])]
 [{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
  {'enum_name': 'UserDefUnionKind', 'enum_values': None},
+ {'enum_name': 'UserDefBaseUnionKind', 'enum_values': None},
+ {'enum_name': 'UserDefDiscriminatorUnionKind', 'enum_values': None},
  {'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None}]
 [OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
  OrderedDict([('type', 'UserDefOne'), ('data', OrderedDict([('integer', 'int'), ('string', 'str'), ('*enum1', 'EnumOne')]))]),
  OrderedDict([('type', 'UserDefTwo'), ('data', OrderedDict([('string', 'str'), ('dict', OrderedDict([('string', 'str'), ('dict', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')])), ('*dict2', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')]))]))]))]),
  OrderedDict([('type', 'UserDefNested'), ('data', OrderedDict([('string0', 'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', OrderedDict([('userdef1', 'UserDefOne'), ('string2', 'str')])), ('*dict3', OrderedDict([('userdef2', 'UserDefOne'), ('string3', 'str')]))]))]))]),
+ OrderedDict([('type', 'UserDefBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
+ OrderedDict([('type', 'UserDefInherit'), ('base', 'UserDefBase'), ('data', OrderedDict([('boolean', 'bool'), ('integer', 'int')]))]),
+ OrderedDict([('type', 'UserDefDirectInherit'), ('data', OrderedDict([('boolean', 'bool'), ('dict', OrderedDict([('_base', 'UserDefBase'), ('integer', 'int')]))]))]),
+ OrderedDict([('type', 'UserDefMixedInherit'), ('base', 'UserDefBase'), ('data', OrderedDict([('boolean', 'bool'), ('dict', OrderedDict([('_base', 'UserDefBase'), ('integer', 'int')]))]))]),
+ OrderedDict([('type', 'UserDefBase0'), ('data', OrderedDict([('base-string0', 'str'), ('base-enum0', 'EnumOne')]))]),
  OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
  OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
  OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])]
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 1e1c6fa..15d56ff 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -286,6 +286,89 @@ static void test_visitor_in_list(TestInputVisitorData *data,
     qapi_free_UserDefOneList(head);
 }
 
+static void test_visitor_in_inherit(TestInputVisitorData *data,
+                                    const void *unused)
+{
+    Visitor *v;
+    Error *err = NULL;
+    UserDefInherit *tmp;
+    const char *input =
+    "{ \
+        'integer': 2, \
+        'boolean': false, \
+        'enum1': 'value2', \
+        'string': 'test' \
+    }";
+
+    v = visitor_input_test_init_raw(data, input);
+
+    visit_type_UserDefInherit(v, &tmp, NULL, &err);
+    g_assert(err == NULL);
+    g_assert_cmpint(tmp->integer, ==, 2);
+    g_assert_cmpint(tmp->boolean, ==, false);
+    g_assert_cmpint(tmp->base->enum1, ==, ENUM_ONE_VALUE2);
+    g_assert_cmpstr(tmp->base->string, ==, "test");
+    qapi_free_UserDefInherit(tmp);
+}
+
+static void test_visitor_in_direct_inherit(TestInputVisitorData *data,
+                                           const void *unused)
+{
+    Visitor *v;
+    Error *err = NULL;
+    UserDefDirectInherit *tmp;
+    const char *input =
+    "{ \
+        'boolean': false, \
+        'dict': { \
+            'integer': 2, \
+            'enum1': 'value2', \
+            'string': 'test' \
+        } \
+    }";
+
+    v = visitor_input_test_init_raw(data, input);
+
+    visit_type_UserDefDirectInherit(v, &tmp, NULL, &err);
+    g_assert(err == NULL);
+    g_assert_cmpint(tmp->boolean, ==, false);
+    g_assert_cmpint(tmp->dict._base->enum1, ==, ENUM_ONE_VALUE2);
+    g_assert_cmpstr(tmp->dict._base->string, ==, "test");
+    g_assert_cmpint(tmp->dict.integer, ==, 2);
+    qapi_free_UserDefDirectInherit(tmp);
+}
+
+static void test_visitor_in_mixed_inherit(TestInputVisitorData *data,
+                                          const void *unused)
+{
+    Visitor *v;
+    Error *err = NULL;
+    UserDefMixedInherit *tmp;
+    const char *input =
+    "{ \
+        'boolean': false, \
+        'enum1': 'value2', \
+        'string': 'test', \
+        'dict': { \
+            'integer': 2, \
+            'enum1': 'value2', \
+            'string': 'test' \
+        } \
+    }";
+
+    v = visitor_input_test_init_raw(data, input);
+
+    visit_type_UserDefMixedInherit(v, &tmp, NULL, &err);
+    g_assert(err == NULL);
+    g_assert_cmpint(tmp->boolean, ==, false);
+    g_assert_cmpint(tmp->base->enum1, ==, ENUM_ONE_VALUE2);
+    g_assert_cmpstr(tmp->base->string, ==, "test");
+    g_assert_cmpint(tmp->dict._base->enum1, ==, ENUM_ONE_VALUE2);
+    g_assert_cmpstr(tmp->dict._base->string, ==, "test");
+    g_assert_cmpint(tmp->dict.integer, ==, 2);
+    qapi_free_UserDefMixedInherit(tmp);
+}
+
 static void test_visitor_in_union(TestInputVisitorData *data,
                                   const void *unused)
 {
@@ -302,6 +385,97 @@ static void test_visitor_in_union(TestInputVisitorData *data,
     qapi_free_UserDefUnion(tmp);
 }
 
+static void test_visitor_in_base_union(TestInputVisitorData *data,
+                                       const void *unused)
+{
+    Visitor *v;
+    Error *err = NULL;
+    UserDefBaseUnion *tmp;
+    const char *input =
+    "{ \
+        'base-enum0': 'value2', \
+        'base-string0': 'test', \
+        'type': 'b', \
+        'data': { \
+            'integer': 2 \
+        } \
+    }";
+
+    v = visitor_input_test_init_raw(data, input);
+
+    visit_type_UserDefBaseUnion(v, &tmp, NULL, &err);
+    g_assert(err == NULL);
+    g_assert_cmpint(tmp->base_enum0, ==, ENUM_ONE_VALUE2);
+    g_assert_cmpstr(tmp->base_string0, ==, "test");
+    g_assert_cmpint(tmp->kind, ==, USER_DEF_BASE_UNION_KIND_B);
+    g_assert_cmpint(tmp->b->integer, ==, 2);
+
+    qapi_free_UserDefBaseUnion(tmp);
+}
+
+static void test_visitor_in_discriminator_union(TestInputVisitorData *data,
+                                                const void *unused)
+{
+    Visitor *v;
+    Error *err = NULL;
+    UserDefDiscriminatorUnion *tmp;
+    const char *input =
+    "{ \
+        'integer': 2, \
+        'base-enum0': 'value2', \
+        'base-string0': 'b' \
+    }";
+
+    v = visitor_input_test_init_raw(data, input);
+
+    visit_type_UserDefDiscriminatorUnion(v, &tmp, NULL, &err);
+    g_assert(err == NULL);
+    g_assert_cmpint(tmp->base_enum0, ==, ENUM_ONE_VALUE2);
+    g_assert_cmpint(tmp->kind, ==, USER_DEF_DISCRIMINATOR_UNION_KIND_B);
+    g_assert_cmpint(tmp->b->integer, ==, 2);
+
+    qapi_free_UserDefDiscriminatorUnion(tmp);
+}
+
+static
+void test_visitor_in_enum_discriminator_union(TestInputVisitorData *data,
+                                              const void *unused)
+{
+    Visitor *v;
+    Error *err = NULL;
+    UserDefEnumDiscriminatorUnion *tmp;
+    const char *input =
+    "{ \
+        'boolean': false, \
+        'enum1': 'value2', \
+        'string': 'test', \
+        'base-enum0': 'value2', \
+        'dict': { \
+            'integer': 2, \
+            'enum1': 'value2', \
+            'string': 'test' \
+        }, \
+        'base-string0': 'test' \
+    }";
+
+    v = visitor_input_test_init_raw(data, input);
+
+    visit_type_UserDefEnumDiscriminatorUnion(v, &tmp, NULL, &err);
+    g_assert(err == NULL);
+
+    g_assert_cmpstr(tmp->base_string0, ==, "test");
+    g_assert_cmpint(tmp->kind, ==, ENUM_ONE_VALUE2);
+
+    g_assert_cmpint(tmp->value2->boolean, ==, false);
+    g_assert_cmpint(tmp->value2->base->enum1, ==, ENUM_ONE_VALUE2);
+    g_assert_cmpstr(tmp->value2->base->string, ==, "test");
+    g_assert_cmpint(tmp->value2->dict._base->enum1, ==, ENUM_ONE_VALUE2);
+    g_assert_cmpstr(tmp->value2->dict._base->string, ==, "test");
+    g_assert_cmpint(tmp->value2->dict.integer, ==, 2);
+
+    qapi_free_UserDefEnumDiscriminatorUnion(tmp);
+}
+
 static void test_native_list_integer_helper(TestInputVisitorData *data,
                                             const void *unused,
                                             UserDefNativeListUnionKind kind)
@@ -633,8 +807,22 @@ int main(int argc, char **argv)
                             &in_visitor_data, test_visitor_in_struct_nested);
     input_visitor_test_add("/visitor/input/list",
                             &in_visitor_data, test_visitor_in_list);
+    input_visitor_test_add("/visitor/input/inherit",
+                            &in_visitor_data, test_visitor_in_inherit);
+    input_visitor_test_add("/visitor/input/direct-inherit",
+                            &in_visitor_data, test_visitor_in_direct_inherit);
+    input_visitor_test_add("/visitor/input/mixed-inherit",
+                            &in_visitor_data, test_visitor_in_mixed_inherit);
     input_visitor_test_add("/visitor/input/union",
                             &in_visitor_data, test_visitor_in_union);
+    input_visitor_test_add("/visitor/input/base-union",
+                            &in_visitor_data, test_visitor_in_base_union);
+    input_visitor_test_add("/visitor/input/discriminator-union",
+                            &in_visitor_data,
+                            test_visitor_in_discriminator_union);
+    input_visitor_test_add("/visitor/input/enum-discriminator-union",
+                            &in_visitor_data,
+                            test_visitor_in_enum_discriminator_union);
     input_visitor_test_add("/visitor/input/errors",
                             &in_visitor_data, test_visitor_in_errors);
     input_visitor_test_add("/visitor/input/native_list/int",
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index e073d83..3c762a8 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -402,6 +402,112 @@ static void test_visitor_out_list_qapi_free(TestOutputVisitorData *data,
     qapi_free_UserDefNestedList(head);
 }
 
+static void test_visitor_out_inherit(TestOutputVisitorData *data,
+                                     const void *unused)
+{
+    QObject *arg;
+    QDict *qdict;
+    const char *test_str = "test";
+    Error *err = NULL;
+
+    UserDefInherit *tmp =
+            g_malloc0(sizeof(UserDefInherit));
+    tmp->integer = 2;
+    tmp->boolean = false;
+    tmp->base = g_malloc0(sizeof(UserDefBase));
+    tmp->base->enum1 = ENUM_ONE_VALUE2;
+    tmp->base->string = g_strdup(test_str);
+
+    visit_type_UserDefInherit(data->ov, &tmp, NULL, &err);
+    g_assert(err == NULL);
+    arg = qmp_output_get_qobject(data->qov);
+
+    g_assert(qobject_type(arg) == QTYPE_QDICT);
+    qdict = qobject_to_qdict(arg);
+
+    g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 2);
+    g_assert_cmpint(qdict_get_bool(qdict, "boolean"), ==, false);
+    g_assert_cmpstr(qdict_get_str(qdict, "enum1"), ==, "value2");
+    g_assert_cmpstr(qdict_get_str(qdict, "string"), ==, test_str);
+
+    qapi_free_UserDefInherit(tmp);
+    QDECREF(qdict);
+}
+
+static void test_visitor_out_direct_inherit(TestOutputVisitorData *data,
+                                            const void *unused)
+{
+    QObject *arg;
+    QDict *qdict, *qdict_sub;
+    const char *test_str = "test";
+    Error *err = NULL;
+
+    UserDefDirectInherit *tmp =
+            g_malloc0(sizeof(UserDefDirectInherit));
+    tmp->boolean = false;
+    tmp->dict._base = g_malloc0(sizeof(UserDefBase));
+    tmp->dict._base->enum1 = ENUM_ONE_VALUE2;
+    tmp->dict._base->string = g_strdup(test_str);
+    tmp->dict.integer = 2;
+
+    visit_type_UserDefDirectInherit(data->ov, &tmp, NULL, &err);
+    g_assert(err == NULL);
+    arg = qmp_output_get_qobject(data->qov);
+
+    g_assert(qobject_type(arg) == QTYPE_QDICT);
+    qdict = qobject_to_qdict(arg);
+
+    g_assert_cmpint(qdict_get_bool(qdict, "boolean"), ==, false);
+    qdict_sub = qobject_to_qdict(qdict_get(qdict, "dict"));
+    g_assert(qdict_sub);
+    g_assert_cmpint(qdict_get_int(qdict_sub, "integer"), ==, 2);
+    g_assert_cmpstr(qdict_get_str(qdict_sub, "enum1"), ==, "value2");
+    g_assert_cmpstr(qdict_get_str(qdict_sub, "string"), ==, test_str);
+
+    qapi_free_UserDefDirectInherit(tmp);
+    QDECREF(qdict);
+}
+
+static void test_visitor_out_mixed_inherit(TestOutputVisitorData *data,
+                                           const void *unused)
+{
+    QObject *arg;
+    QDict *qdict, *qdict_sub;
+    const char *test_str = "test";
+    Error *err = NULL;
+
+    UserDefMixedInherit *tmp =
+            g_malloc0(sizeof(UserDefMixedInherit));
+    tmp->boolean = false;
+    tmp->base = g_malloc0(sizeof(UserDefBase));
+    tmp->base->enum1 = ENUM_ONE_VALUE2;
+    tmp->base->string = g_strdup(test_str);
+    tmp->dict._base = g_malloc0(sizeof(UserDefBase));
+    tmp->dict._base->enum1 = ENUM_ONE_VALUE2;
+    tmp->dict._base->string = g_strdup(test_str);
+    tmp->dict.integer = 2;
+
+
+    visit_type_UserDefMixedInherit(data->ov, &tmp, NULL, &err);
+    g_assert(err == NULL);
+    arg = qmp_output_get_qobject(data->qov);
+
+    g_assert(qobject_type(arg) == QTYPE_QDICT);
+    qdict = qobject_to_qdict(arg);
+
+    g_assert_cmpint(qdict_get_bool(qdict, "boolean"), ==, false);
+    g_assert_cmpstr(qdict_get_str(qdict, "enum1"), ==, "value2");
+    g_assert_cmpstr(qdict_get_str(qdict, "string"), ==, test_str);
+    qdict_sub = qobject_to_qdict(qdict_get(qdict, "dict"));
+    g_assert(qdict_sub);
+    g_assert_cmpint(qdict_get_int(qdict_sub, "integer"), ==, 2);
+    g_assert_cmpstr(qdict_get_str(qdict_sub, "enum1"), ==, "value2");
+    g_assert_cmpstr(qdict_get_str(qdict_sub, "string"), ==, test_str);
+
+    qapi_free_UserDefMixedInherit(tmp);
+    QDECREF(qdict);
+}
+
 static void test_visitor_out_union(TestOutputVisitorData *data,
                                    const void *unused)
 {
@@ -434,6 +540,122 @@ static void test_visitor_out_union(TestOutputVisitorData *data,
     QDECREF(qdict);
 }
 
+static void test_visitor_out_base_union(TestOutputVisitorData *data,
+                                        const void *unused)
+{
+    QObject *arg;
+    QDict *qdict, *qdict_sub;
+    const char *test_str = "test";
+    Error *err = NULL;
+
+    UserDefBaseUnion *tmp =
+            g_malloc0(sizeof(UserDefBaseUnion));
+    tmp->base_enum0 = ENUM_ONE_VALUE2;
+    tmp->base_string0 = g_strdup(test_str);
+    tmp->kind = USER_DEF_BASE_UNION_KIND_B;
+    tmp->b = g_malloc0(sizeof(UserDefB));
+    tmp->b->integer = 2;
+
+    visit_type_UserDefBaseUnion(data->ov, &tmp, NULL, &err);
+    g_assert(err == NULL);
+    arg = qmp_output_get_qobject(data->qov);
+
+    g_assert(qobject_type(arg) == QTYPE_QDICT);
+    qdict = qobject_to_qdict(arg);
+
+    g_assert_cmpstr(qdict_get_str(qdict, "base-enum0"), ==, "value2");
+    g_assert_cmpstr(qdict_get_str(qdict, "base-string0"), ==, test_str);
+    g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "b");
+
+    qdict_sub = qobject_to_qdict(qdict_get(qdict, "data"));
+    g_assert(qdict_sub);
+
+    g_assert_cmpint(qdict_get_int(qdict_sub, "integer"), ==, 2);
+
+    qapi_free_UserDefBaseUnion(tmp);
+    QDECREF(qdict);
+}
+
+static void test_visitor_out_discriminator_union(TestOutputVisitorData *data,
+                                                 const void *unused)
+{
+    QObject *arg;
+    QDict *qdict;
+
+    Error *err = NULL;
+
+    UserDefDiscriminatorUnion *tmp =
+            g_malloc0(sizeof(UserDefDiscriminatorUnion));
+    tmp->base_enum0 = ENUM_ONE_VALUE2;
+    tmp->kind = USER_DEF_DISCRIMINATOR_UNION_KIND_B;
+    tmp->b = g_malloc0(sizeof(UserDefB));
+    tmp->b->integer = 2;
+
+    visit_type_UserDefDiscriminatorUnion(data->ov, &tmp, NULL, &err);
+    g_assert(err == NULL);
+    arg = qmp_output_get_qobject(data->qov);
+
+    g_assert(qobject_type(arg) == QTYPE_QDICT);
+    qdict = qobject_to_qdict(arg);
+
+    g_assert_cmpstr(qdict_get_str(qdict, "base-enum0"), ==, "value2");
+    g_assert_cmpstr(qdict_get_str(qdict, "base-string0"), ==, "b");
+    g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 2);
+
+    qapi_free_UserDefDiscriminatorUnion(tmp);
+    QDECREF(qdict);
+}
+
+/* A complex type */
+static
+void test_visitor_out_enum_discriminator_union(TestOutputVisitorData *data,
+                                               const void *unused)
+{
+
+    QObject *arg;
+    QDict *qdict, *qdict_sub;
+    const char *test_str = "test";
+    Error *err = NULL;
+
+    UserDefEnumDiscriminatorUnion *tmp =
+            g_malloc0(sizeof(UserDefEnumDiscriminatorUnion));
+    tmp->base_string0 = g_strdup(test_str);
+    tmp->kind = ENUM_ONE_VALUE2;
+    tmp->value2 = g_malloc0(sizeof(UserDefMixedInherit));
+    tmp->value2->boolean = false;
+    tmp->value2->base = g_malloc0(sizeof(UserDefBase));
+    tmp->value2->base->enum1 = ENUM_ONE_VALUE2;
+    tmp->value2->base->string = g_strdup(test_str);
+    tmp->value2->dict._base = g_malloc0(sizeof(UserDefBase));
+    tmp->value2->dict._base->enum1 = ENUM_ONE_VALUE2;
+    tmp->value2->dict._base->string = g_strdup(test_str);
+    tmp->value2->dict.integer = 2;
+
+
+    visit_type_UserDefEnumDiscriminatorUnion(data->ov, &tmp, NULL, &err);
+    g_assert(err == NULL);
+    arg = qmp_output_get_qobject(data->qov);
+
+    g_assert(qobject_type(arg) == QTYPE_QDICT);
+    qdict = qobject_to_qdict(arg);
+
+    g_assert_cmpstr(qdict_get_str(qdict, "base-enum0"), ==, "value2");
+    g_assert_cmpstr(qdict_get_str(qdict, "base-string0"), ==, test_str);
+    /* check sub type specfic */
+    g_assert_cmpint(qdict_get_bool(qdict, "boolean"), ==, false);
+    g_assert_cmpstr(qdict_get_str(qdict, "enum1"), ==, "value2");
+    g_assert_cmpstr(qdict_get_str(qdict, "string"), ==, test_str);
+    qdict_sub = qobject_to_qdict(qdict_get(qdict, "dict"));
+    g_assert(qdict_sub);
+    g_assert_cmpint(qdict_get_int(qdict_sub, "integer"), ==, 2);
+    g_assert_cmpstr(qdict_get_str(qdict_sub, "enum1"), ==, "value2");
+    g_assert_cmpstr(qdict_get_str(qdict_sub, "string"), ==, test_str);
+
+    qapi_free_UserDefEnumDiscriminatorUnion(tmp);
+    QDECREF(qdict);
+
+}
+
 static void init_native_list(UserDefNativeListUnion *cvalue)
 {
     int i;
@@ -780,8 +1002,24 @@ int main(int argc, char **argv)
                             &out_visitor_data, test_visitor_out_list);
     output_visitor_test_add("/visitor/output/list-qapi-free",
                             &out_visitor_data, test_visitor_out_list_qapi_free);
+    output_visitor_test_add("/visitor/output/inherit",
+                            &out_visitor_data, test_visitor_out_inherit);
+    output_visitor_test_add("/visitor/output/direct-inherit",
+                            &out_visitor_data,
+                            test_visitor_out_direct_inherit);
+    output_visitor_test_add("/visitor/output/mixed-inherit",
+                            &out_visitor_data,
+                            test_visitor_out_mixed_inherit);
     output_visitor_test_add("/visitor/output/union",
                             &out_visitor_data, test_visitor_out_union);
+    output_visitor_test_add("/visitor/output/base-union",
+                            &out_visitor_data, test_visitor_out_base_union);
+    output_visitor_test_add("/visitor/output/discriminator-union",
+                            &out_visitor_data,
+                            test_visitor_out_discriminator_union);
+    output_visitor_test_add("/visitor/output/enum-discriminator-union",
+                            &out_visitor_data,
+                            test_visitor_out_enum_discriminator_union);
     output_visitor_test_add("/visitor/output/native_list/int",
                             &out_visitor_data, test_visitor_out_native_list_int);
     output_visitor_test_add("/visitor/output/native_list/int8",
-- 
1.7.1

Re: [Qemu-devel] [PATCH RFC 01/10] qapi: fix memleak by add implict struct functions in dealloc visitor

[Eric Blake]

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

On 11/04/2013 05:37 PM, Wenchao Xia wrote:

s/add/adding/ in subject

> Otherwise they are leaked in a qap_free_STRUCTURE() call.

s/qap/qapi/

> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> Cc: qemu-stable@nongnu.org
> ---
>  qapi/qapi-dealloc-visitor.c |   20 ++++++++++++++++++++
>  1 files changed, 20 insertions(+), 0 deletions(-)

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

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH RFC 09/10] tests: fix memleak in error path test for input visitor

[Eric Blake]

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

On 11/04/2013 05:37 PM, Wenchao Xia wrote:
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> Cc: qemu-stable@nongnu.org
> ---
>  tests/test-qmp-input-visitor.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)

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

You should repost your mem-leak patches for qemu-stable as an
independent thread marked "for-1.7", without RFC, to ensure that they
are applied in a timely manner without waiting for the rest of this series.

> 
> diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
> index 0beb8fb..1e1c6fa 100644
> --- a/tests/test-qmp-input-visitor.c
> +++ b/tests/test-qmp-input-visitor.c
> @@ -604,6 +604,7 @@ static void test_visitor_in_errors(TestInputVisitorData *data,
>      g_assert(error_is_set(&errp));
>      g_assert(p->string == NULL);
>  
> +    error_free(errp);
>      g_free(p->string);
>      g_free(p);
>  }
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH RFC 03/10] qapi script: check correctness of discriminator values in union

[Eric Blake]

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

On 11/04/2013 05:37 PM, Wenchao Xia wrote:
> It will check whether the values specfied are wrotten correctly when

s/specfied/specified/
s/wrotten/written/

> discriminator is a pre-defined enum type, which help check whether the
> schema is in good form.
> 
> It is allowed that, not every value in enum is used, so do not check

s/that,/that/

> that case.

Why do you allow partial coverage?  That feels like an accident waiting
to happen.  Does the user get a sane error message if they request an
enum value that wasn't mapped to a union branch?  I think it would be
wiser to mandate that if the discriminator is an enum, then the union
must cover all values of the enum.

> +
> +# Return the descriminator enum define, if discriminator is specified in

s/descriminator/discriminator/

> +# @expr and it is a pre-defined enum type
> +def descriminator_find_enum_define(expr):

s/descriminator/discriminator/ - and fix all callers

> +    discriminator = expr.get('discriminator')
> +    base = expr.get('base')
> +
> +    # Only support discriminator when base present
> +    if not (discriminator and base):
> +        return None
> +
> +    base_fields = find_base_fields(base)
> +
> +    if not base_fields:
> +        sys.stderr.write("Base '%s' is not a valid type\n"
> +                         % base)
> +        sys.exit(1)
> +
> +    descriminator_type = base_fields.get(discriminator)

s/descriminator/discriminator/

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH RFC 02/10] qapi script: remember enum values

[Eric Blake]

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

On 11/04/2013 05:37 PM, Wenchao Xia wrote:
> Later other script will need to check the enum values.

s/script/scripts/

> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  scripts/qapi.py                        |   18 ++++++++++++++----
>  tests/qapi-schema/comments.out         |    2 +-
>  tests/qapi-schema/qapi-schema-test.out |    4 +++-
>  3 files changed, 18 insertions(+), 6 deletions(-)
> 

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

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH RFC 07/10] qapi script: support direct inheritance for struct

[Eric Blake]

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

On 11/04/2013 05:37 PM, Wenchao Xia wrote:
> Now it is possible to inherit another struct inside data directly,
> which saves trouble to define trivial structure.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  docs/qapi-code-gen.txt |   21 +++++++++++++++++++++
>  scripts/qapi-visit.py  |   14 ++++++++++----
>  2 files changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
> index 0728f36..3e42ff4 100644
> --- a/docs/qapi-code-gen.txt
> +++ b/docs/qapi-code-gen.txt
> @@ -70,6 +70,27 @@ both fields like this:
>   { "file": "/some/place/my-image",
>     "backing": "/some/place/my-backing-file" }
>  
> +It is possible to directly inherit other struct by keyword '_base':
> +
> + { 'type': 'NetworkConnectionInfo', 'data': { 'host': 'str', 'service': 'str' } }
> + { 'type': 'VncConnectionInfo',
> +   'data': {
> +      'server': {
> +          '_base': 'NetworkConnectionInfo',

Interesting idea for shorthand.  However, I would suggest that you pick
a different character than '_', since '_' is valid in names.  That is,
we already have special handling of leading '*' to mark a field as
optional, so I suggest something like '^' to mark a base class.  By
using a non-name character, it becomes more obvious that the leading
character has a special meaning to the qapi generator.

I'm also not convinced yet that we want this shorthand; in particular,
I'm worried whether it will make the introspection patches harder to write.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH RFC 09/10] tests: fix memleak in error path test for input visitor

[Wenchao Xia]

于 2013/11/5 21:20, Eric Blake 写道:
> On 11/04/2013 05:37 PM, Wenchao Xia wrote:
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> Cc: qemu-stable@nongnu.org
>> ---
>>   tests/test-qmp-input-visitor.c |    1 +
>>   1 files changed, 1 insertions(+), 0 deletions(-)
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
> You should repost your mem-leak patches for qemu-stable as an
> independent thread marked "for-1.7", without RFC, to ensure that they
> are applied in a timely manner without waiting for the rest of this series.
>

   OK, will resend.

>>
>> diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
>> index 0beb8fb..1e1c6fa 100644
>> --- a/tests/test-qmp-input-visitor.c
>> +++ b/tests/test-qmp-input-visitor.c
>> @@ -604,6 +604,7 @@ static void test_visitor_in_errors(TestInputVisitorData *data,
>>       g_assert(error_is_set(&errp));
>>       g_assert(p->string == NULL);
>>
>> +    error_free(errp);
>>       g_free(p->string);
>>       g_free(p);
>>   }
>>
>

Re: [Qemu-devel] [PATCH RFC 03/10] qapi script: check correctness of discriminator values in union

[Wenchao Xia]

于 2013/11/5 21:25, Eric Blake 写道:
> On 11/04/2013 05:37 PM, Wenchao Xia wrote:
>> It will check whether the values specfied are wrotten correctly when
>
> s/specfied/specified/
> s/wrotten/written/
>
>> discriminator is a pre-defined enum type, which help check whether the
>> schema is in good form.
>>
>> It is allowed that, not every value in enum is used, so do not check
>
> s/that,/that/
>
>> that case.
>
> Why do you allow partial coverage?  That feels like an accident waiting
> to happen.  Does the user get a sane error message if they request an
> enum value that wasn't mapped to a union branch?  I think it would be
> wiser to mandate that if the discriminator is an enum, then the union
> must cover all values of the enum.
>
   abort() will be called in qapi-visit.c, it is OK for output visitor,
but bad for input visitor since user input may trigger it. I think
change abort() to error_set() could solve the problem, then we allow
map part of enum value.

>> +
>> +# Return the descriminator enum define, if discriminator is specified in
>
> s/descriminator/discriminator/
>
>> +# @expr and it is a pre-defined enum type
>> +def descriminator_find_enum_define(expr):
>
> s/descriminator/discriminator/ - and fix all callers
>
>> +    discriminator = expr.get('discriminator')
>> +    base = expr.get('base')
>> +
>> +    # Only support discriminator when base present
>> +    if not (discriminator and base):
>> +        return None
>> +
>> +    base_fields = find_base_fields(base)
>> +
>> +    if not base_fields:
>> +        sys.stderr.write("Base '%s' is not a valid type\n"
>> +                         % base)
>> +        sys.exit(1)
>> +
>> +    descriminator_type = base_fields.get(discriminator)
>
> s/descriminator/discriminator/
>

Re: [Qemu-devel] [PATCH RFC 07/10] qapi script: support direct inheritance for struct

[Wenchao Xia]

于 2013/11/5 21:41, Eric Blake 写道:
> On 11/04/2013 05:37 PM, Wenchao Xia wrote:
>> Now it is possible to inherit another struct inside data directly,
>> which saves trouble to define trivial structure.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>>   docs/qapi-code-gen.txt |   21 +++++++++++++++++++++
>>   scripts/qapi-visit.py  |   14 ++++++++++----
>>   2 files changed, 31 insertions(+), 4 deletions(-)
>>
>> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
>> index 0728f36..3e42ff4 100644
>> --- a/docs/qapi-code-gen.txt
>> +++ b/docs/qapi-code-gen.txt
>> @@ -70,6 +70,27 @@ both fields like this:
>>    { "file": "/some/place/my-image",
>>      "backing": "/some/place/my-backing-file" }
>>
>> +It is possible to directly inherit other struct by keyword '_base':
>> +
>> + { 'type': 'NetworkConnectionInfo', 'data': { 'host': 'str', 'service': 'str' } }
>> + { 'type': 'VncConnectionInfo',
>> +   'data': {
>> +      'server': {
>> +          '_base': 'NetworkConnectionInfo',
>
> Interesting idea for shorthand.  However, I would suggest that you pick
> a different character than '_', since '_' is valid in names.  That is,
> we already have special handling of leading '*' to mark a field as
> optional, so I suggest something like '^' to mark a base class.  By
> using a non-name character, it becomes more obvious that the leading
> character has a special meaning to the qapi generator.
>
> I'm also not convinced yet that we want this shorthand; in particular,
> I'm worried whether it will make the introspection patches harder to write.
>
   I am not sure about this approach either, so put RFC tag on it. The
purpose is allow not to define structures that would be only used once.

   What instrospection patch do you mean? Python instrospection
mechnism? You mean there is a python utility which recognize
only keyword "base" now?

Re: [Qemu-devel] [PATCH RFC 07/10] qapi script: support direct inheritance for struct

[Eric Blake]

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

On 11/05/2013 08:20 PM, Wenchao Xia wrote:
>>> +      'server': {
>>> +          '_base': 'NetworkConnectionInfo',
>>
>> Interesting idea for shorthand.  However, I would suggest that you pick
>> a different character than '_', since '_' is valid in names.  That is,
>> we already have special handling of leading '*' to mark a field as
>> optional, so I suggest something like '^' to mark a base class.  By
>> using a non-name character, it becomes more obvious that the leading
>> character has a special meaning to the qapi generator.
>>
>> I'm also not convinced yet that we want this shorthand; in particular,
>> I'm worried whether it will make the introspection patches harder to
>> write.
>>
>   I am not sure about this approach either, so put RFC tag on it. The
> purpose is allow not to define structures that would be only used once.
> 
>   What instrospection patch do you mean? Python instrospection
> mechnism? You mean there is a python utility which recognize
> only keyword "base" now?

No, I'm talking about Amos' patches to expose the qapi to the user via a
QMP command.  Last version proposed was here:
https://lists.gnu.org/archive/html/qemu-devel/2013-07/msg02494.html
although with the addition of discriminated union types in the meantime,
my understanding is Amos is planning on posting another version soon for
the 1.8 timeframe.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH RFC 07/10] qapi script: support direct inheritance for struct

[Wenchao Xia]

于 2013/11/6 21:33, Eric Blake 写道:
> On 11/05/2013 08:20 PM, Wenchao Xia wrote:
>>>> +      'server': {
>>>> +          '_base': 'NetworkConnectionInfo',
>>>
>>> Interesting idea for shorthand.  However, I would suggest that you pick
>>> a different character than '_', since '_' is valid in names.  That is,
>>> we already have special handling of leading '*' to mark a field as
>>> optional, so I suggest something like '^' to mark a base class.  By
>>> using a non-name character, it becomes more obvious that the leading
>>> character has a special meaning to the qapi generator.
>>>
>>> I'm also not convinced yet that we want this shorthand; in particular,
>>> I'm worried whether it will make the introspection patches harder to
>>> write.
>>>
>>    I am not sure about this approach either, so put RFC tag on it. The
>> purpose is allow not to define structures that would be only used once.
>>
>>    What instrospection patch do you mean? Python instrospection
>> mechnism? You mean there is a python utility which recognize
>> only keyword "base" now?
>
> No, I'm talking about Amos' patches to expose the qapi to the user via a
> QMP command.  Last version proposed was here:
> https://lists.gnu.org/archive/html/qemu-devel/2013-07/msg02494.html
> although with the addition of discriminated union types in the meantime,
> my understanding is Amos is planning on posting another version soon for
> the 1.8 timeframe.
>
   I'll drop this patch now.