[Qemu-devel] [PATCH V2 0/8] qapi script: support enum as discriminator and better enum name

[Qemu-devel] [PATCH V2 0/8] qapi script: support enum as discriminator and better enum name

[Wenchao Xia]

This series is respined from RFC series at:
http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg00363.html

Patch 1-6 add support for enum as discriminator.
Patch 7 improve enum name generation, now AIOContext->AIO_CONTEXT, X86CPU->
X86_CPU.
Patch 8 are the test cases.

Changes from RFC:
  Mainly address Eric's comments: fix typo, add patch 2 to allow partly mapping
enum value in union, add related test case, remove direct inherit support "_base"
and related test case.

v2:
  General:
  3: use Raise exception instead of sys.error.write in qapi.py.
  Address Eric's comments:
  2,3: more check for enum value at compile time.
  8: correspond test case change.

Wenchao Xia (8):
  1 qapi script: remember enum values
  2 qapi script: add check for duplicated key
  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: do not add "_" for every capitalized char in enum
  8 tests: add cases for inherited struct and union with discriminator

 include/qapi/qmp/qerror.h               |    2 +-
 scripts/qapi-types.py                   |   34 ++++----
 scripts/qapi-visit.py                   |   55 +++++++++--
 scripts/qapi.py                         |   84 ++++++++++++++++-
 target-i386/cpu.c                       |    2 +-
 tests/qapi-schema/comments.out          |    2 +-
 tests/qapi-schema/qapi-schema-test.json |   27 ++++++
 tests/qapi-schema/qapi-schema-test.out  |   15 +++-
 tests/test-qmp-input-visitor.c          |  120 +++++++++++++++++++++++++
 tests/test-qmp-output-visitor.c         |  149 +++++++++++++++++++++++++++++++
 10 files changed, 454 insertions(+), 36 deletions(-)

[Qemu-devel] [PATCH V2 1/8] qapi script: remember enum values

[Wenchao Xia]

Later other scripts will need to check the enum values.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.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 V2 2/8] qapi script: add check for duplicated key

[Wenchao Xia]

It is bad that same key was specified twice, especially when a union have
two branches with same condition. This patch can prevent it.

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

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 82f586e..aa91edc 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -116,6 +116,8 @@ class QAPISchema:
             if self.tok != ':':
                 raise QAPISchemaError(self, 'Expected ":"')
             self.accept()
+            if key in expr:
+                raise QAPISchemaError(self, 'Duplicated key "%s"' % key)
             expr[key] = self.get_expr(True)
             if self.tok == '}':
                 self.accept()
-- 
1.7.1

[Qemu-devel] [PATCH V2 3/8] qapi script: check correctness of discriminator values in union

[Wenchao Xia]

It will check whether the values specified are written correctly,
and whether all enum values are covered, when discriminator is a
pre-defined enum type

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

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index c39e628..61e03a1 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -249,6 +249,23 @@ 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 = discriminator_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' is not found in "
+                                 "enum '%s'\n" %
+                                 (key, enum_define["enum_name"]))
+                sys.exit(1)
+        for key in enum_define["enum_values"]:
+            if not key in members:
+                sys.stderr.write("Enum value '%s' is not covered by a branch "
+                                 "of union '%s'\n" %
+                                 (key, 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 aa91edc..3f50d52 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -385,3 +385,34 @@ 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 discriminator enum define, if discriminator is specified in
+# @expr and it is a pre-defined enum type
+def discriminator_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:
+        raise StandardError("Base '%s' is not a valid type\n"
+                            % base)
+
+    discriminator_type = base_fields.get(discriminator)
+
+    if not discriminator_type:
+        raise StandardError("Discriminator '%s' not found in schema\n"
+                            % discriminator)
+
+    return find_enum(discriminator_type)
-- 
1.7.1

[Qemu-devel] [PATCH V2 4/8] 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>
Reviewed-by: Eric Blake <eblake@redhat.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 3f50d52..0e4e9d7 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -416,3 +416,13 @@ def discriminator_find_enum_define(expr):
                             % discriminator)
 
     return find_enum(discriminator_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 V2 5/8] 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 |   21 +++++++++++++++------
 scripts/qapi.py       |   15 +++++++++++----
 3 files changed, 29 insertions(+), 13 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 61e03a1..1769470 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -208,18 +208,23 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
 ''',
     name=name)
 
+    # For anon union, always use the default enum type automatically generated
+    # as "'%sKind' % (name)"
+    discriminator_type_name = '%sKind' % (name)
+
     for key in members:
         assert (members[key] in builtin_types
             or find_struct(members[key])
             or find_union(members[key])), "Invalid anonymous union member"
 
+        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:
             visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err);
             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))
 
@@ -266,7 +271,10 @@ def generate_visit_union(expr):
                                  (key, name))
                 sys.exit(1)
 
+    # There will always be a discriminator in the C switch code, by default it
+    # is an enum type generated silently as "'%sKind' % (name)"
     ret = generate_visit_enum('%sKind' % name, members.keys())
+    discriminator_type_name = '%sKind' % (name)
 
     if base:
         base_fields = find_struct(base)['data']
@@ -319,13 +327,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 0e4e9d7..28ede6f 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -417,12 +417,19 @@ def discriminator_find_enum_define(expr):
 
     return find_enum(discriminator_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 V2 6/8] 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 |   23 ++++++++++++++++-------
 scripts/qapi.py       |    4 +++-
 3 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 914f055..5031c4b 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 = discriminator_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 = discriminator_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 1769470..e06dfa7 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -271,10 +271,14 @@ def generate_visit_union(expr):
                                  (key, name))
                 sys.exit(1)
 
-    # There will always be a discriminator in the C switch code, by default it
-    # is an enum type generated silently as "'%sKind' % (name)"
-    ret = generate_visit_enum('%sKind' % name, members.keys())
-    discriminator_type_name = '%sKind' % (name)
+        # Use the predefined enum type as discriminator
+        ret = ""
+        discriminator_type_name = enum_define['enum_name']
+    else:
+        # There will always be a discriminator in the C switch code, by default it
+        # is an enum type generated silently as "'%sKind' % (name)"
+        ret = generate_visit_enum('%sKind' % name, members.keys())
+        discriminator_type_name = '%sKind' % (name)
 
     if base:
         base_fields = find_struct(base)['data']
@@ -309,11 +313,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:
@@ -527,7 +532,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 = discriminator_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 28ede6f..50be200 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -174,7 +174,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 = discriminator_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 V2 7/8] 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 50be200..bfe080a 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -419,19 +419,31 @@ def discriminator_find_enum_define(expr):
 
     return find_enum(discriminator_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 V2 8/8] 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 |   27 ++++++
 tests/qapi-schema/qapi-schema-test.out  |   11 +++
 tests/test-qmp-input-visitor.c          |  120 +++++++++++++++++++++++++
 tests/test-qmp-output-visitor.c         |  149 +++++++++++++++++++++++++++++++
 4 files changed, 307 insertions(+), 0 deletions(-)

diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index fe5af75..1e23d21 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -21,8 +21,18 @@
             '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' } }
 
 # for testing unions
+{ 'type': 'UserDefBase0',
+  'data': { 'base-string0': 'str', 'base-enum0': 'EnumOne' } }
+
 { 'type': 'UserDefA',
   'data': { 'boolean': 'bool' } }
 
@@ -32,6 +42,23 @@
 { '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' : 'UserDefInherit',
+            'value3' : 'UserDefB' } }
+
 # 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..ce174a2 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -3,9 +3,15 @@
  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', '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', 'UserDefInherit'), ('value3', 'UserDefB')]))]),
  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 +19,16 @@
  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', '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..0c00115 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -286,6 +286,31 @@ 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_union(TestInputVisitorData *data,
                                   const void *unused)
 {
@@ -302,6 +327,91 @@ 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, \
+        'integer': 2, \
+        'enum1': 'value2', \
+        'string': 'test', \
+        'base-enum0': 'value2', \
+        '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->integer, ==, 2);
+    g_assert_cmpint(tmp->value2->base->enum1, ==, ENUM_ONE_VALUE2);
+    g_assert_cmpstr(tmp->value2->base->string, ==, "test");
+
+    qapi_free_UserDefEnumDiscriminatorUnion(tmp);
+}
+
 static void test_native_list_integer_helper(TestInputVisitorData *data,
                                             const void *unused,
                                             UserDefNativeListUnionKind kind)
@@ -633,8 +743,18 @@ 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/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..aae161a 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -402,6 +402,38 @@ 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_union(TestOutputVisitorData *data,
                                    const void *unused)
 {
@@ -434,6 +466,113 @@ 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;
+    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(UserDefInherit));
+    tmp->value2->integer = 2;
+    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);
+
+    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_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_UserDefEnumDiscriminatorUnion(tmp);
+    QDECREF(qdict);
+
+}
+
 static void init_native_list(UserDefNativeListUnion *cvalue)
 {
     int i;
@@ -780,8 +919,18 @@ 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/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 V2 0/8] qapi script: support enum as discriminator and better enum name

[Wenchao Xia]

  Just found timestamp of this series seems older than v1, so ping to
correct it.

Re: [Qemu-devel] [PATCH V2 0/8] qapi script: support enum as discriminator and better enum name

[Luiz Capitulino]

On Wed, 13 Nov 2013 06:25:00 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

> This series is respined from RFC series at:
> http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg00363.html
> 
> Patch 1-6 add support for enum as discriminator.
> Patch 7 improve enum name generation, now AIOContext->AIO_CONTEXT, X86CPU->
> X86_CPU.
> Patch 8 are the test cases.

Can you please clarify what is the problem this series is trying to
solve, how it does it and provide before/after type of examples?

That's what I'd expect from an intro email, but this one has only a
reference to an RFC series that has no better info, and some crypt
changelog with magic numbers :(

Besides, this doesn't apply anymore...

> 
> Changes from RFC:
>   Mainly address Eric's comments: fix typo, add patch 2 to allow partly mapping
> enum value in union, add related test case, remove direct inherit support "_base"
> and related test case.
> 
> v2:
>   General:
>   3: use Raise exception instead of sys.error.write in qapi.py.
>   Address Eric's comments:
>   2,3: more check for enum value at compile time.
>   8: correspond test case change.
> 
> Wenchao Xia (8):
>   1 qapi script: remember enum values
>   2 qapi script: add check for duplicated key
>   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: do not add "_" for every capitalized char in enum
>   8 tests: add cases for inherited struct and union with discriminator
> 
>  include/qapi/qmp/qerror.h               |    2 +-
>  scripts/qapi-types.py                   |   34 ++++----
>  scripts/qapi-visit.py                   |   55 +++++++++--
>  scripts/qapi.py                         |   84 ++++++++++++++++-
>  target-i386/cpu.c                       |    2 +-
>  tests/qapi-schema/comments.out          |    2 +-
>  tests/qapi-schema/qapi-schema-test.json |   27 ++++++
>  tests/qapi-schema/qapi-schema-test.out  |   15 +++-
>  tests/test-qmp-input-visitor.c          |  120 +++++++++++++++++++++++++
>  tests/test-qmp-output-visitor.c         |  149 +++++++++++++++++++++++++++++++
>  10 files changed, 454 insertions(+), 36 deletions(-)
> 

Re: [Qemu-devel] [PATCH V2 0/8] qapi script: support enum as discriminator and better enum name

[Wenchao Xia]

于 2013/11/26 0:47, Luiz Capitulino 写道:
> On Wed, 13 Nov 2013 06:25:00 +0800
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>
>> This series is respined from RFC series at:
>> http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg00363.html
>>
>> Patch 1-6 add support for enum as discriminator.
>> Patch 7 improve enum name generation, now AIOContext->AIO_CONTEXT, X86CPU->
>> X86_CPU.
>> Patch 8 are the test cases.
>
> Can you please clarify what is the problem this series is trying to
> solve, how it does it and provide before/after type of examples?
>
> That's what I'd expect from an intro email, but this one has only a
> reference to an RFC series that has no better info, and some crypt
> changelog with magic numbers :(
>
> Besides, this doesn't apply anymore...
>
   Let me have an introduction:

1. support using enum as discriminator in union.
For example, if we have following define in qapi schema:
{ 'enum': 'EnumOne',
   'data': [ 'value1', 'value2', 'value3' ] }

{ 'type': 'UserDefBase0',
   'data': { 'base-string0': 'str', 'base-enum0': 'EnumOne' } }

Before this series, discriminator in union must be a string, and a
hidden enum type as discriminator is generated. After this series,
qapi schema can directly use predefined enum type:
{ 'union': 'UserDefEnumDiscriminatorUnion',
   'base': 'UserDefBase0',
   'discriminator' : 'base-enum0',
   'data': { 'value1' : 'UserDefA',
             'value2' : 'UserDefInherit',
             'value3' : 'UserDefB' } }

The implement is done by:
1.1 remember the enum defines by qapi scripts.(patch 1)
1.2 use the remembered enum define to check correctness at compile
time.(patch 3)
1.3 use the same enum name generation rule to avoid C code mismatch,
esp for "case [ENUM_VALUE]" in qapi-visit.c.(patch 4,5)
1.4 when pre-defined enum type is used as discriminator, don't generate
a hidden enum type, switch the code path to support enum as
discriminator.
1.5 test case shows how it looks like.(Patch 8)

2. Better enum name generation
Before this patch, AIOContext->A_I_O_Context, after this patch,
AIOContet->AIO_Context. Since previous patch has foldered enum
name generation codes into one function, it is done easily by modifying
it.(Patch 7)

>>
>> Changes from RFC:
>>    Mainly address Eric's comments: fix typo, add patch 2 to allow partly mapping
>> enum value in union, add related test case, remove direct inherit support "_base"
>> and related test case.
>>
>> v2:
>>    General:
>>    3: use Raise exception instead of sys.error.write in qapi.py.
>>    Address Eric's comments:
>>    2,3: more check for enum value at compile time.
>>    8: correspond test case change.
>>
>> Wenchao Xia (8):
>>    1 qapi script: remember enum values
>>    2 qapi script: add check for duplicated key
>>    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: do not add "_" for every capitalized char in enum
>>    8 tests: add cases for inherited struct and union with discriminator
>>
>>   include/qapi/qmp/qerror.h               |    2 +-
>>   scripts/qapi-types.py                   |   34 ++++----
>>   scripts/qapi-visit.py                   |   55 +++++++++--
>>   scripts/qapi.py                         |   84 ++++++++++++++++-
>>   target-i386/cpu.c                       |    2 +-
>>   tests/qapi-schema/comments.out          |    2 +-
>>   tests/qapi-schema/qapi-schema-test.json |   27 ++++++
>>   tests/qapi-schema/qapi-schema-test.out  |   15 +++-
>>   tests/test-qmp-input-visitor.c          |  120 +++++++++++++++++++++++++
>>   tests/test-qmp-output-visitor.c         |  149 +++++++++++++++++++++++++++++++
>>   10 files changed, 454 insertions(+), 36 deletions(-)
>>
>

Re: [Qemu-devel] [PATCH V2 0/8] qapi script: support enum as discriminator and better enum name

[Luiz Capitulino]

On Thu, 28 Nov 2013 14:19:48 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

> 于 2013/11/26 0:47, Luiz Capitulino 写道:
> > On Wed, 13 Nov 2013 06:25:00 +0800
> > Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
> >
> >> This series is respined from RFC series at:
> >> http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg00363.html
> >>
> >> Patch 1-6 add support for enum as discriminator.
> >> Patch 7 improve enum name generation, now AIOContext->AIO_CONTEXT, X86CPU->
> >> X86_CPU.
> >> Patch 8 are the test cases.
> >
> > Can you please clarify what is the problem this series is trying to
> > solve, how it does it and provide before/after type of examples?
> >
> > That's what I'd expect from an intro email, but this one has only a
> > reference to an RFC series that has no better info, and some crypt
> > changelog with magic numbers :(
> >
> > Besides, this doesn't apply anymore...
> >
>    Let me have an introduction:
> 
> 1. support using enum as discriminator in union.
> For example, if we have following define in qapi schema:
> { 'enum': 'EnumOne',
>    'data': [ 'value1', 'value2', 'value3' ] }
> 
> { 'type': 'UserDefBase0',
>    'data': { 'base-string0': 'str', 'base-enum0': 'EnumOne' } }
> 
> Before this series, discriminator in union must be a string, and a
> hidden enum type as discriminator is generated. After this series,
> qapi schema can directly use predefined enum type:
> { 'union': 'UserDefEnumDiscriminatorUnion',
>    'base': 'UserDefBase0',
>    'discriminator' : 'base-enum0',
>    'data': { 'value1' : 'UserDefA',
>              'value2' : 'UserDefInherit',
>              'value3' : 'UserDefB' } }

OK, now tell me who will benefit from this. You don't have to answer
right now, you can resend your series with this information. Note that
I'm not asking to be convinced, I'm asking to have enough info so that
I can understand what this is about when I get to review this (and this
is just what an intro email is about).

> 
> The implement is done by:
> 1.1 remember the enum defines by qapi scripts.(patch 1)
> 1.2 use the remembered enum define to check correctness at compile
> time.(patch 3)
> 1.3 use the same enum name generation rule to avoid C code mismatch,
> esp for "case [ENUM_VALUE]" in qapi-visit.c.(patch 4,5)
> 1.4 when pre-defined enum type is used as discriminator, don't generate
> a hidden enum type, switch the code path to support enum as
> discriminator.
> 1.5 test case shows how it looks like.(Patch 8)
> 
> 2. Better enum name generation
> Before this patch, AIOContext->A_I_O_Context, after this patch,
> AIOContet->AIO_Context. Since previous patch has foldered enum
> name generation codes into one function, it is done easily by modifying
> it.(Patch 7)
> 
> >>
> >> Changes from RFC:
> >>    Mainly address Eric's comments: fix typo, add patch 2 to allow partly mapping
> >> enum value in union, add related test case, remove direct inherit support "_base"
> >> and related test case.
> >>
> >> v2:
> >>    General:
> >>    3: use Raise exception instead of sys.error.write in qapi.py.
> >>    Address Eric's comments:
> >>    2,3: more check for enum value at compile time.
> >>    8: correspond test case change.
> >>
> >> Wenchao Xia (8):
> >>    1 qapi script: remember enum values
> >>    2 qapi script: add check for duplicated key
> >>    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: do not add "_" for every capitalized char in enum
> >>    8 tests: add cases for inherited struct and union with discriminator
> >>
> >>   include/qapi/qmp/qerror.h               |    2 +-
> >>   scripts/qapi-types.py                   |   34 ++++----
> >>   scripts/qapi-visit.py                   |   55 +++++++++--
> >>   scripts/qapi.py                         |   84 ++++++++++++++++-
> >>   target-i386/cpu.c                       |    2 +-
> >>   tests/qapi-schema/comments.out          |    2 +-
> >>   tests/qapi-schema/qapi-schema-test.json |   27 ++++++
> >>   tests/qapi-schema/qapi-schema-test.out  |   15 +++-
> >>   tests/test-qmp-input-visitor.c          |  120 +++++++++++++++++++++++++
> >>   tests/test-qmp-output-visitor.c         |  149 +++++++++++++++++++++++++++++++
> >>   10 files changed, 454 insertions(+), 36 deletions(-)
> >>
> >
> 

Re: [Qemu-devel] [PATCH V2 0/8] qapi script: support enum as discriminator and better enum name

[Kevin Wolf]

Am 12.11.2013 um 23:25 hat Wenchao Xia geschrieben:
> This series is respined from RFC series at:
> http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg00363.html
> 
> Patch 1-6 add support for enum as discriminator.
> Patch 7 improve enum name generation, now AIOContext->AIO_CONTEXT, X86CPU->
> X86_CPU.
> Patch 8 are the test cases.
> 
> Changes from RFC:
>   Mainly address Eric's comments: fix typo, add patch 2 to allow partly mapping
> enum value in union, add related test case, remove direct inherit support "_base"
> and related test case.
> 
> v2:
>   General:
>   3: use Raise exception instead of sys.error.write in qapi.py.
>   Address Eric's comments:
>   2,3: more check for enum value at compile time.
>   8: correspond test case change.
> 
> Wenchao Xia (8):
>   1 qapi script: remember enum values
>   2 qapi script: add check for duplicated key
>   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: do not add "_" for every capitalized char in enum
>   8 tests: add cases for inherited struct and union with discriminator
> 
>  include/qapi/qmp/qerror.h               |    2 +-
>  scripts/qapi-types.py                   |   34 ++++----
>  scripts/qapi-visit.py                   |   55 +++++++++--
>  scripts/qapi.py                         |   84 ++++++++++++++++-
>  target-i386/cpu.c                       |    2 +-
>  tests/qapi-schema/comments.out          |    2 +-
>  tests/qapi-schema/qapi-schema-test.json |   27 ++++++
>  tests/qapi-schema/qapi-schema-test.out  |   15 +++-
>  tests/test-qmp-input-visitor.c          |  120 +++++++++++++++++++++++++
>  tests/test-qmp-output-visitor.c         |  149 +++++++++++++++++++++++++++++++
>  10 files changed, 454 insertions(+), 36 deletions(-)

I think there should be an update for docs/qapi-code-gen.txt somewhere
in the series. If you put it in the same patch as the actual generator
change, you can simplify the commit message by pointing to the hunk
modifying the documentation.

Kevin

Re: [Qemu-devel] [PATCH V2 0/8] qapi script: support enum as discriminator and better enum name

[Wenchao Xia]

于 2013/11/28 22:24, Luiz Capitulino 写道:
> On Thu, 28 Nov 2013 14:19:48 +0800
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>
>> 于 2013/11/26 0:47, Luiz Capitulino 写道:
>>> On Wed, 13 Nov 2013 06:25:00 +0800
>>> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>>
>>>> This series is respined from RFC series at:
>>>> http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg00363.html
>>>>
>>>> Patch 1-6 add support for enum as discriminator.
>>>> Patch 7 improve enum name generation, now AIOContext->AIO_CONTEXT, X86CPU->
>>>> X86_CPU.
>>>> Patch 8 are the test cases.
>>>
>>> Can you please clarify what is the problem this series is trying to
>>> solve, how it does it and provide before/after type of examples?
>>>
>>> That's what I'd expect from an intro email, but this one has only a
>>> reference to an RFC series that has no better info, and some crypt
>>> changelog with magic numbers :(
>>>
>>> Besides, this doesn't apply anymore...
>>>
>>     Let me have an introduction:
>>
>> 1. support using enum as discriminator in union.
>> For example, if we have following define in qapi schema:
>> { 'enum': 'EnumOne',
>>     'data': [ 'value1', 'value2', 'value3' ] }
>>
>> { 'type': 'UserDefBase0',
>>     'data': { 'base-string0': 'str', 'base-enum0': 'EnumOne' } }
>>
>> Before this series, discriminator in union must be a string, and a
>> hidden enum type as discriminator is generated. After this series,
>> qapi schema can directly use predefined enum type:
>> { 'union': 'UserDefEnumDiscriminatorUnion',
>>     'base': 'UserDefBase0',
>>     'discriminator' : 'base-enum0',
>>     'data': { 'value1' : 'UserDefA',
>>               'value2' : 'UserDefInherit',
>>               'value3' : 'UserDefB' } }
>
> OK, now tell me who will benefit from this. You don't have to answer
> right now, you can resend your series with this information. Note that
> I'm not asking to be convinced, I'm asking to have enough info so that
> I can understand what this is about when I get to review this (and this
> is just what an intro email is about).
>

   OK, I will send v3 with better doc.

>>
>> The implement is done by:
>> 1.1 remember the enum defines by qapi scripts.(patch 1)
>> 1.2 use the remembered enum define to check correctness at compile
>> time.(patch 3)
>> 1.3 use the same enum name generation rule to avoid C code mismatch,
>> esp for "case [ENUM_VALUE]" in qapi-visit.c.(patch 4,5)
>> 1.4 when pre-defined enum type is used as discriminator, don't generate
>> a hidden enum type, switch the code path to support enum as
>> discriminator.
>> 1.5 test case shows how it looks like.(Patch 8)
>>
>> 2. Better enum name generation
>> Before this patch, AIOContext->A_I_O_Context, after this patch,
>> AIOContet->AIO_Context. Since previous patch has foldered enum
>> name generation codes into one function, it is done easily by modifying
>> it.(Patch 7)
>>
>>>>
>>>> Changes from RFC:
>>>>     Mainly address Eric's comments: fix typo, add patch 2 to allow partly mapping
>>>> enum value in union, add related test case, remove direct inherit support "_base"
>>>> and related test case.
>>>>
>>>> v2:
>>>>     General:
>>>>     3: use Raise exception instead of sys.error.write in qapi.py.
>>>>     Address Eric's comments:
>>>>     2,3: more check for enum value at compile time.
>>>>     8: correspond test case change.
>>>>
>>>> Wenchao Xia (8):
>>>>     1 qapi script: remember enum values
>>>>     2 qapi script: add check for duplicated key
>>>>     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: do not add "_" for every capitalized char in enum
>>>>     8 tests: add cases for inherited struct and union with discriminator
>>>>
>>>>    include/qapi/qmp/qerror.h               |    2 +-
>>>>    scripts/qapi-types.py                   |   34 ++++----
>>>>    scripts/qapi-visit.py                   |   55 +++++++++--
>>>>    scripts/qapi.py                         |   84 ++++++++++++++++-
>>>>    target-i386/cpu.c                       |    2 +-
>>>>    tests/qapi-schema/comments.out          |    2 +-
>>>>    tests/qapi-schema/qapi-schema-test.json |   27 ++++++
>>>>    tests/qapi-schema/qapi-schema-test.out  |   15 +++-
>>>>    tests/test-qmp-input-visitor.c          |  120 +++++++++++++++++++++++++
>>>>    tests/test-qmp-output-visitor.c         |  149 +++++++++++++++++++++++++++++++
>>>>    10 files changed, 454 insertions(+), 36 deletions(-)
>>>>
>>>
>>
>
>

Re: [Qemu-devel] [PATCH V2 2/8] qapi script: add check for duplicated key

[Eric Blake]

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

On 11/12/2013 03:25 PM, Wenchao Xia wrote:
> It is bad that same key was specified twice, especially when a union have
> two branches with same condition. This patch can prevent it.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  scripts/qapi.py |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)

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

> 
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 82f586e..aa91edc 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -116,6 +116,8 @@ class QAPISchema:
>              if self.tok != ':':
>                  raise QAPISchemaError(self, 'Expected ":"')
>              self.accept()
> +            if key in expr:
> +                raise QAPISchemaError(self, 'Duplicated key "%s"' % key)
>              expr[key] = self.get_expr(True)
>              if self.tok == '}':
>                  self.accept()
> 

-- 
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 V2 3/8] qapi script: check correctness of discriminator values in union

[Eric Blake]

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

On 11/12/2013 03:25 PM, Wenchao Xia wrote:
> It will check whether the values specified are written correctly,
> and whether all enum values are covered, when discriminator is a
> pre-defined enum type
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  scripts/qapi-visit.py |   17 +++++++++++++++++
>  scripts/qapi.py       |   31 +++++++++++++++++++++++++++++++
>  2 files changed, 48 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 V2 5/8] qapi script: use same function to generate enum string

[Eric Blake]

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

On 11/12/2013 03:25 PM, Wenchao Xia wrote:
> 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.

I'm finding this commit message awkward to read and rather wordy.  May I
suggest the shorter:

Prior to this patch, qapi-visit.py used custom code to generate enum
names used for handling a qapi union.  Fix it to instead reuse common
code, with identical generated results, and allowing future updates to
generation to only need to touch one place.

-- 
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 V2 7/8] qapi script: do not add "_" for every capitalized char in enum

[Eric Blake]

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

On 11/12/2013 03:25 PM, Wenchao Xia wrote:
> 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(-)
> 

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 V2 5/8] qapi script: use same function to generate enum string

[Wenchao Xia]

于 2013/12/3 4:17, Eric Blake 写道:
> On 11/12/2013 03:25 PM, Wenchao Xia wrote:
>> 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.
>
> I'm finding this commit message awkward to read and rather wordy.  May I
> suggest the shorter:
>
> Prior to this patch, qapi-visit.py used custom code to generate enum
> names used for handling a qapi union.  Fix it to instead reuse common
> code, with identical generated results, and allowing future updates to
> generation to only need to touch one place.
>
   That sounds better, thanks for reviewing. I have sent v3 before at:
http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg03980.html
There is no change except better intro/doc, will use the new commit
message in V4.