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

[Amos Kong <akong@redhat.com>]

On Fri, Jan 24, 2014 at 05:12:12PM +0800, Fam Zheng wrote:
> 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 <akong@redhat.com>
> > ---
> >  .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?

Not, it just used to help the extending.

>  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".

It's more meaningful.
 
> > +
> > +'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 <akong@redhat.com>
> > +#
> > +# 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?

Not, I will remove it.
 
> Thanks,
> Fam
 
Thanks for your review.
Amos