[Qemu-devel] [PATCH Resend V3 0/9] qapi script: support enum as discriminator and better enum name

[Qemu-devel] [PATCH Resend V3 0/9] qapi script: support enum as discriminator and better enum name

[Wenchao Xia]

I forgot to cc the maillist before, so resend it to qemu-devel@nongnu.org.
 
This series address two issues:

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 benefit is that every thing is defined explicitly in schema file,
the discriminator enum type can be used in other API define in schema,
and a compile time check will be put to verify the correctness according
to enum define. Currently BlockdevOptions used discriminator which can
be converted, in the future other union can also use enum discriminator.

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), more strict check(patch 2)
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 switch the code path, when pre-defined enum type is used as discriminator,
don't generate a hidden enum type, use the enum type instead, add
docs/qapi-code-gen.txt.(Patch 6)
1.5 test case shows how it looks like.(Patch 7)
1.6 convert BlockdevOptions. (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 9)


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. RFC series at:
http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg00363.html

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

v3:
  General:
  move enum name generation patch to last in the series, add convert patch
8/9.
  Address Luiz and Kevin's comments:
  Better introduction.
  6/9: renamed this patch, add docs/qapi-code-gen.txt part.

Wenchao Xia (9):
  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: support pre-defined enum type as discriminator in union
  7 tests: add cases for inherited struct and union with discriminator
  8 qapi: convert BlockdevOptions to use enum discriminator
  9 qapi script: do not add "_" for every capitalized char in enum

 docs/qapi-code-gen.txt                  |    8 ++-
 include/qapi/qmp/qerror.h               |    2 +-
 qapi-schema.json                        |   14 +++-
 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 +++++++++++++++++++++++++++++++
 12 files changed, 473 insertions(+), 39 deletions(-)

[Qemu-devel] [PATCH V3 1/9] 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 V3 2/9] 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 V3 3/9] 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 65f1a54..c0efb5f 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -255,6 +255,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 V3 4/9] 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 V3 5/9] 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 c0efb5f..1b55924 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -214,18 +214,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))
 
@@ -272,7 +277,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']
@@ -330,13 +338,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 V3 6/9] qapi script: support pre-defined enum type as discriminator in union

[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, if discriminator is an enum field.

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

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 0728f36..12d7d97 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -123,11 +123,15 @@ And it looks like this on the wire:
 
 Flat union types avoid the nesting on the wire. They are used whenever a
 specific field of the base type is declared as the discriminator ('type' is
-then no longer generated). The discriminator must always be a string field.
+then no longer generated). The discriminator can be a string field or a
+predfined enum field. If it is a string field, a hidden enum type will be
+generated as "[UNION_NAME]Kind". If it is an enum field, compile time check
+will be done to verify the correctness. It is recommended to use enum field.
 The above example can then be modified as follows:
 
+ { 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] }
  { 'type': 'BlockdevCommonOptions',
-   'data': { 'driver': 'str', 'readonly': 'bool' } }
+   'data': { 'driver': 'BlockdevDriver', 'readonly': 'bool' } }
  { 'union': 'BlockdevOptions',
    'base': 'BlockdevCommonOptions',
    'discriminator': 'driver',
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 1b55924..3240921 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -277,10 +277,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']
@@ -320,11 +324,12 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
     else:
         desc_type = discriminator
     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=desc_type)
+                 discriminator_type_name=discriminator_type_name,
+                 type=desc_type)
 
     for key in members:
         if not discriminator:
@@ -538,7 +543,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 V3 7/9] 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

[Qemu-devel] [PATCH V3 8/9] qapi: convert BlockdevOptions to use enum discriminator

[Wenchao Xia]

After this patch, hidden enum type BlockdevOptionsKind will not
be generated, and other API can use enum BlockdevDriver.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 qapi-schema.json |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 83fa485..ce9197d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4024,6 +4024,18 @@
             '*no-flush': 'bool' } }
 
 ##
+# @BlockdevDriver
+#
+# Drivers that are supported in block device operations.
+#
+# Since: 2.0
+##
+{ 'enum': 'BlockdevDriver',
+  'data': [ 'file', 'http', 'https', 'ftp', 'ftps', 'tftp', 'vvfat', 'bochs',
+            'cloop', 'cow', 'dmg', 'parallels', 'qcow', 'qcow2', 'qed', 'raw',
+            'vdi', 'vhdx', 'vmdk', 'vpc' ] }
+
+##
 # @BlockdevOptionsBase
 #
 # Options that are available for all block devices, independent of the block
@@ -4046,7 +4058,7 @@
 # Since: 1.7
 ##
 { 'type': 'BlockdevOptionsBase',
-  'data': { 'driver': 'str',
+  'data': { 'driver': 'BlockdevDriver',
             '*id': 'str',
             '*discard': 'BlockdevDiscardOptions',
             '*cache': 'BlockdevCacheOptions',
-- 
1.7.1

[Qemu-devel] [PATCH V3 9/9] 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 47af9a8..03bb7fd 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

Re: [Qemu-devel] [PATCH V3 6/9] qapi script: support pre-defined enum type as discriminator in union

[Eric Blake]

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

On 11/29/2013 01:41 AM, Wenchao Xia wrote:
> 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, if discriminator is an enum field.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---

>  Flat union types avoid the nesting on the wire. They are used whenever a
>  specific field of the base type is declared as the discriminator ('type' is
> -then no longer generated). The discriminator must always be a string field.
> +then no longer generated). The discriminator can be a string field or a
> +predfined enum field. If it is a string field, a hidden enum type will be

s/predfined/predefined/

> +generated as "[UNION_NAME]Kind". If it is an enum field, compile time check

s/compile/a compile/

> +will be done to verify the correctness. It is recommended to use enum field.

s/enum/an enum/

Thanks for the added documentation on this round.

-- 
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 V3 8/9] qapi: convert BlockdevOptions to use enum discriminator

[Eric Blake]

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

On 11/29/2013 01:41 AM, Wenchao Xia wrote:
> After this patch, hidden enum type BlockdevOptionsKind will not
> be generated, and other API can use enum BlockdevDriver.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  qapi-schema.json |   14 +++++++++++++-
>  1 files changed, 13 insertions(+), 1 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 83fa485..ce9197d 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4024,6 +4024,18 @@
>              '*no-flush': 'bool' } }
>  
>  ##
> +# @BlockdevDriver
> +#
> +# Drivers that are supported in block device operations.
> +#
> +# Since: 2.0

It will be interesting documenting any drivers that are added after 2.0,
but we can worry about that when it happens.  Looks good!

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

> +##
> +{ 'enum': 'BlockdevDriver',
> +  'data': [ 'file', 'http', 'https', 'ftp', 'ftps', 'tftp', 'vvfat', 'bochs',
> +            'cloop', 'cow', 'dmg', 'parallels', 'qcow', 'qcow2', 'qed', 'raw',
> +            'vdi', 'vhdx', 'vmdk', 'vpc' ] }
> +
> +##

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

[Eric Blake]

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

On 11/29/2013 01:41 AM, 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>
> ---

> +    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

s/appear/appears/   s/check/checks/

> +        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():

Indentation looks odd.

> +                new_name += '_'
>          new_name += c
>      return new_name.lstrip('_').upper()

Those are minor, so feel free to add:

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 --]