Re: [Qemu-devel] [PATCH v4 2/5] qapi: add qapi-introspect.py code generator

[Fam Zheng]

On Thu, 01/23 22:46, Amos Kong wrote:
> This is a code generator for qapi introspection. It will parse
> qapi-schema.json, extend schema definitions and generate a schema
> table with metadata, it references to the new structs which we used
> to describe dynamic data structs.  The metadata will help C code to
> allocate right structs and provide useful information to management
> to checking suported feature and QMP commandline detail. The schema
> table will be saved to qapi-introspect.h.
> 
> The $(prefix) is used to as a namespace to keep the generated code

s/used to as/used as/

> from one schema/code-generation separated from others so code and
> be generated from multiple schemas with clobbering previously

s/with/without/

> created code.
> 
> Signed-off-by: Amos Kong <address@hidden>
> ---
>  .gitignore                      |   1 +
>  Makefile                        |   5 +-
>  docs/qmp-full-introspection.txt |  17 ++++
>  scripts/qapi-introspect.py      | 172 
> ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 194 insertions(+), 1 deletion(-)
>  create mode 100644 scripts/qapi-introspect.py
> 
> diff --git a/.gitignore b/.gitignore
> index 1c9d63d..de3cb80 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -22,6 +22,7 @@ linux-headers/asm
>  qapi-generated
>  qapi-types.[ch]
>  qapi-visit.[ch]
> +qapi-introspect.h
>  qmp-commands.h
>  qmp-marshal.c
>  qemu-doc.html
> diff --git a/Makefile b/Makefile
> index bdff4e4..1dac5e7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -45,7 +45,7 @@ endif
>  endif
>  
>  GENERATED_HEADERS = config-host.h qemu-options.def
> -GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
> +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h 
> qapi-introspect.h
>  GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c
>  
>  GENERATED_HEADERS += trace/generated-events.h
> @@ -229,6 +229,9 @@ $(SRC_PATH)/qapi-schema.json 
> $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
>  qmp-commands.h qmp-marshal.c :\
>  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
>       $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py 
> $(gen-out-type) -m -o "." < $<, "  GEN   $@")
> +qapi-introspect.h:\
> +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-introspect.py 
> $(qapi-py)
> +     $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-introspect.py 
> $(gen-out-type) -o "." < $<, "  GEN   $@")
>  
>  QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h 
> qga-qapi-visit.h qga-qmp-commands.h)
>  $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
> diff --git a/docs/qmp-full-introspection.txt b/docs/qmp-full-introspection.txt
> index d2cf7b3..8ecbc0c 100644
> --- a/docs/qmp-full-introspection.txt
> +++ b/docs/qmp-full-introspection.txt
> @@ -42,3 +42,20 @@ types.
>  
>  'anonymous-struct' will be used to describe arbitrary structs
>  (dictionary, list or string).
> +
> +== Avoid dead loop in recursive extending ==
> +
> +We have four types (ImageInfo, BlockStats, PciDeviceInfo, ObjectData)
> +that uses themself in their own define data directly or indirectly,

s/themself/themselves/
s/define data/definition/

> +we will not repeatedly extend them to avoid dead loop.
> +
> +We use a 'parents List' to record the visit path, type name of each
> +extended node will be saved to the List.
> +
> +Append type name to the list before extending, and remove type name
> +from the list after extending.
> +
> +If the type name is already extended in parents List, we won't extend
> +it repeatedly for avoiding dead loop.

This "parents" list detail is not reflected in the generated information,
right?  I think it's good enough to describe that "type will not be extented
more than once in a schema, when there's direct or indirect recursive
type composition".

> +
> +'recursive' indicates if the type is extended or not.
> diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py
> new file mode 100644
> index 0000000..03179fa
> --- /dev/null
> +++ b/scripts/qapi-introspect.py
> @@ -0,0 +1,172 @@
> +#
> +# QAPI introspection info generator
> +#
> +# Copyright (C) 2014 Red Hat, Inc.
> +#
> +# Authors:
> +#  Amos Kong <address@hidden>
> +#
> +# This work is licensed under the terms of the GNU GPLv2.
> +# See the COPYING.LIB file in the top-level directory.
> +
> +from ordereddict import OrderedDict
> +from qapi import *
> +import sys
> +import os
> +import getopt
> +import errno
> +
> +
> +try:
> +    opts, args = getopt.gnu_getopt(sys.argv[1:], "hp:o:",
> +                                   ["header", "prefix=", "output-dir="])
> +except getopt.GetoptError, err:
> +    print str(err)
> +    sys.exit(1)
> +
> +output_dir = ""
> +prefix = ""
> +h_file = 'qapi-introspect.h'
> +
> +do_h = False
> +
> +for o, a in opts:
> +    if o in ("-p", "--prefix"):
> +        prefix = a

Is this option used in your series?

Thanks,
Fam

> +    elif o in ("-o", "--output-dir"):
> +        output_dir = a + "/"
> +    elif o in ("-h", "--header"):
> +        do_h = True
> +
> +h_file = output_dir + prefix + h_file
> +
> +try:
> +    os.makedirs(output_dir)
> +except os.error, e:
> +    if e.errno != errno.EEXIST:
> +        raise
> +
> +def maybe_open(really, name, opt):
> +    if really:
> +        return open(name, opt)
> +    else:
> +        import StringIO
> +        return StringIO.StringIO()
> +
> +fdecl = maybe_open(do_h, h_file, 'w')
> +
> +fdecl.write(mcgen('''
> +/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
> +
> +/*
> + * Head file to store parsed information of QAPI schema
> + *
> + * Copyright (C) 2014 Red Hat, Inc.
> + *
> + * Authors:
> + *  Amos Kong <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or 
> later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#ifndef %(guard)s
> +#define %(guard)s
> +
> +''',
> +                  guard=guardname(h_file)))
> +
> +def extend_schema(expr, parents=[], member=True):
> +    ret = {}
> +    recu = 'False'
> +    name = ""
> +
> +    if type(expr) is OrderedDict:
> +        if not member:
> +            e = expr.popitem(last=False)
> +            typ = e[0]
> +            name = e[1]
> +        else:
> +            typ = "anonymous-struct"
> +
> +        if typ == 'enum':
> +            for key in expr.keys():
> +                ret[key] = expr[key]
> +        else:
> +            ret = {}
> +            for key in expr.keys():
> +                ret[key], parents = extend_schema(expr[key], parents)
> +
> +    elif type(expr) is list:
> +        typ = 'anonymous-struct'
> +        ret = []
> +        for i in expr:
> +            tmp, parents = extend_schema(i, parents)
> +            ret.append(tmp)
> +    elif type(expr) is str:
> +        name = expr
> +        if schema_dict.has_key(expr) and expr not in parents:
> +            parents.append(expr)
> +            typ = schema_dict[expr][1]
> +            recu = 'True'
> +            ret, parents = extend_schema(schema_dict[expr][0].copy(),
> +                                         parents, False)
> +            parents.remove(expr)
> +            ret['_obj_recursive'] = 'True'
> +            return ret, parents
> +        else:
> +            return expr, parents
> +
> +    return {'_obj_member': "%s" % member, '_obj_type': typ,
> +            '_obj_name': name, '_obj_recursive': recu,
> +            '_obj_data': ret}, parents
> +
> +
> +exprs = parse_schema(sys.stdin)
> +schema_dict = {}
> +
> +for expr in exprs:
> +    if expr.has_key('type') or expr.has_key('enum') or expr.has_key('union'):
> +        e = expr.copy()
> +
> +        first = e.popitem(last=False)
> +        schema_dict[first[1]] = [expr.copy(), first[0]]
> +
> +fdecl.write('''const char *const qmp_schema_table[] = {
> +''')
> +
> +def convert(odict):
> +    d = {}
> +    for k, v in odict.items():
> +        if type(v) is OrderedDict:
> +            d[k] = convert(v)
> +        elif type(v) is list:
> +            l = []
> +            for j in v:
> +                if type(j) is OrderedDict:
> +                    l.append(convert(j))
> +                else:
> +                    l.append(j)
> +            d[k] = l
> +        else:
> +            d[k] = v
> +    return d
> +
> +count = 0
> +for expr in exprs:
> +    fdecl.write('''    /* %s */
> +''' % expr)
> +
> +    expr, parents = extend_schema(expr, [], False)
> +    fdecl.write('''    "%s",
> +
> +''' % convert(expr))
> +
> +fdecl.write('''    NULL };
> +
> +#endif
> +''')
> +
> +fdecl.flush()
> +fdecl.close()
> -- 
> 1.8.4.2
> 
>