On Thu, 23 Jan 2014 11:05:06 +0800
Amos Kong <ak...@redhat.com> wrote:
> On Wed, Jan 22, 2014 at 01:06:51PM -0500, Luiz Capitulino wrote:
> > On Sun, 5 Jan 2014 20:02:30 +0800
> > Amos Kong <ak...@redhat.com> wrote:
> >
> > > QMP schema is defined in a json file, it will be parsed by
> > > qapi scripts and generate C files.
> > >
> > > We want to return the schema information to management,
> > > this patch converts the json file to a string table in a
> > > C head file, then we can use the json content in QEMU code.
> > >
> > > eg: (qmp-schema.h)
> > > const char *const qmp_schema_table[] = {
> > > "{ 'type': 'NameInfo', 'data': {'*name': 'str'} }",
> > > "{ 'command': 'query-name', 'returns': 'NameInfo' }",
> > > ...
> > > }
> > >
> > > Signed-off-by: Amos Kong <ak...@redhat.com>
> > > ---
> > > Makefile | 5 ++++-
> > > scripts/qapi-commands.py | 2 +-
> > > scripts/qapi-types.py | 48
> > > +++++++++++++++++++++++++++++++++++++++++++++---
> > > scripts/qapi-visit.py | 2 +-
> > > scripts/qapi.py | 20 +++++++++++++++-----
> > > 5 files changed, 66 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index bdff4e4..2c29755 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
> > > qmp-schema.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 $@")
> > > +qmp-schema.h:\
> > > +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
> > > + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py
> > > $(gen-out-type) -o "." -s "$@" < $<, " 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/scripts/qapi-commands.py b/scripts/qapi-commands.py
> > > index b12b696..5f4fb94 100644
> > > --- a/scripts/qapi-commands.py
> > > +++ b/scripts/qapi-commands.py
> > > @@ -440,7 +440,7 @@ except os.error, e:
> > > if e.errno != errno.EEXIST:
> > > raise
> > >
> > > -exprs = parse_schema(sys.stdin)
> > > +exprs = parse_schema(sys.stdin)[0]
> > > commands = filter(lambda expr: expr.has_key('command'), exprs)
> > > commands = filter(lambda expr: not expr.has_key('gen'), commands)
> > >
> > > diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> > > index 4a1652b..0f86b95 100644
> > > --- a/scripts/qapi-types.py
> > > +++ b/scripts/qapi-types.py
> > > @@ -15,6 +15,7 @@ import sys
> > > import os
> > > import getopt
> > > import errno
> > > +import re
> > >
> > > def generate_fwd_struct(name, members, builtin_type=False):
> > > if builtin_type:
> > > @@ -282,9 +283,10 @@ void qapi_free_%(type)s(%(c_type)s obj)
> > >
> > >
> > > try:
> > > - opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:",
> > > + opts, args = getopt.gnu_getopt(sys.argv[1:], "chbs:p:o:",
> > > ["source", "header", "builtins",
> > > - "prefix=", "output-dir="])
> > > + "schema-dump-file=", "prefix=",
> > > + "output-dir="])
> > > except getopt.GetoptError, err:
> > > print str(err)
> > > sys.exit(1)
> > > @@ -293,6 +295,7 @@ output_dir = ""
> > > prefix = ""
> > > c_file = 'qapi-types.c'
> > > h_file = 'qapi-types.h'
> > > +schema_dump_file = ""
> > >
> > > do_c = False
> > > do_h = False
> > > @@ -309,11 +312,17 @@ for o, a in opts:
> > > do_h = True
> > > elif o in ("-b", "--builtins"):
> > > do_builtins = True
> > > + elif o in ("-s", "--schema-dump-file"):
> > > + schema_dump_file = a
> >
> > Instead of adding this to qapi-types.py, wouldn't it be clearer to add
> > a qapi-dump.py file instead?
>
> I used scripts/qapi-introspect.py to generate qapi-introspect.h.
> Q: qapi-introspect.py or qapi-dump.py? which one is better?
qapi-introspect.py is good.
>
> It also helps to extend schema and generate a nested dictionaries with
> metadata, it's very simple to use python to extend.
>
> /* OrderedDict([('command', 'query-name'), ('returns', 'NameInfo')]) */
> "{'_obj_member': 'False', '_obj_type': 'command', '_obj_name':
> 'query-name', '_obj_data': {'returns': {'_obj_type': 'type', '_obj_member':
> 'False', '_obj_name': 'NameInfo', '_obj_data': {'data': {'_obj_type':
> 'anonymous-struct', '_obj_member': 'True', '_obj_name': '', '_obj_data':
> {'*name': 'str'}, '_obj_recursion': 'false'}}, '_obj_recursion': 'true'}},
> '_obj_recursion': 'false'}",
>
> Then in qmp.c, we only need to visit the dictionaries and fill the data
> to allocated structs, which will be returned to QMP monitor.
>
> > Also, I think it would be interesting to split this patch into two. The
> > first
> > patch changes qapi.py (and related files), this will allow you to better
> > describe your changes to that file. The second patch adds qapi-dump.py.
> >
> > In general, this looks good to me but I haven't looked into the
> > changes done in qapi.py in detail.
>
> In v3, we just change qapi.py:parse_schema() to additionally return raw json
> string.
> In my latest patches, we don't need to change qapi.py, we can directly use
> OrderedDicts.
>
> I'm working in qmp.c part, maybe we can try to simple DataObject definitions
> in V4.
>
> BTW, no need to review v3, please wait my refreshed V4 :-)
>
> Thanks, Amos
>
