Message ID | 1388923351-10556-3-git-send-email-akong@redhat.com |
---|---|
State | New |
Headers | show |
On 2014年01月05日 20:02, Amos Kong 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 <akong@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 > > if not do_c and not do_h: > do_c = True > do_h = True > > +if schema_dump_file: > + do_c = False > + do_h = False > + > c_file = output_dir + prefix + c_file > h_file = output_dir + prefix + h_file > > @@ -381,7 +390,40 @@ fdecl.write(mcgen(''' > ''', > guard=guardname(h_file))) > > -exprs = parse_schema(sys.stdin) > +exprs_all = parse_schema(sys.stdin) > + > +schema_table = """/* AUTOMATICALLY GENERATED, DO NOT MODIFY */ > + > +/* > + * Schema json string table converted from qapi-schema.json > + * > + * Copyright (c) 2013 Red Hat, Inc. > + * > + * Authors: > + * Amos Kong <akong@redhat.com> > + * > + * 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. > + * > + */ > + > +const char *const qmp_schema_table[] = { > +""" > + > +if schema_dump_file: > + for line in exprs_all[1]: > + line = re.sub(r'#.*\n', ' ', line.strip()) > + line = re.sub(r'\n', ' ', line.strip()) > + line = re.sub(r' +', ' ', line) > + schema_table += ' "%s",\n' % (line) > + > + schema_table += ' NULL };\n' > + f = open(schema_dump_file, "w") > + f.write(schema_table) > + f.flush() > + f.close() > + > +exprs = exprs_all[0] > exprs = filter(lambda expr: not expr.has_key('gen'), exprs) > > fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL")) > diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py > index 65f1a54..db68084 100644 > --- a/scripts/qapi-visit.py > +++ b/scripts/qapi-visit.py > @@ -480,7 +480,7 @@ fdecl.write(mcgen(''' > ''', > prefix=prefix, guard=guardname(h_file))) > > -exprs = parse_schema(sys.stdin) > +exprs = parse_schema(sys.stdin)[0] > > # to avoid header dependency hell, we always generate declarations > # for built-in types in our header files and simply guard them > diff --git a/scripts/qapi.py b/scripts/qapi.py > index 4263bbd..43cc401 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -61,10 +61,13 @@ class QAPISchema: > self.src += '\n' > self.cursor = 0 > self.exprs = [] > + self.raw_exprs = [] > self.accept() > > while self.tok != None: > - self.exprs.append(self.get_expr(False)) > + self.cur_entry= '' Missing whitespace before "=". > + self.exprs.append(self.get_expr(False, True)) > + self.raw_exprs.append(self.cur_entry) > > def accept(self): > while True: > @@ -103,9 +106,11 @@ class QAPISchema: > elif not self.tok.isspace(): > raise QAPISchemaError(self, 'Stray "%s"' % self.tok) > > - def get_members(self): > + def get_members(self, start=None): > expr = OrderedDict() > if self.tok == '}': > + if start != None: > + self.cur_entry = self.src[start:self.cursor] > self.accept() > return expr > if self.tok != "'": > @@ -118,6 +123,8 @@ class QAPISchema: > self.accept() > expr[key] = self.get_expr(True) > if self.tok == '}': > + if start != None: > + self.cur_entry = self.src[start:self.cursor] > self.accept() > return expr > if self.tok != ',': > @@ -142,12 +149,15 @@ class QAPISchema: > raise QAPISchemaError(self, 'Expected "," or "]"') > self.accept() > > - def get_expr(self, nested): > + def get_expr(self, nested, first=False): > if self.tok != '{' and not nested: > raise QAPISchemaError(self, 'Expected "{"') > if self.tok == '{': > + start = None > + if first: > + start = self.cursor - 1 > self.accept() > - expr = self.get_members() > + expr = self.get_members(start) > elif self.tok == '[': > self.accept() > expr = self.get_values() > @@ -174,7 +184,7 @@ def parse_schema(fp): > elif expr.has_key('type'): > add_struct(expr) > > - return schema.exprs > + return schema.exprs, schema.raw_exprs > > def parse_args(typeinfo): > if isinstance(typeinfo, basestring): >
On 2014年01月05日 20:02, Amos Kong 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 <akong@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 $@") > It would be nice to also add this file to .gitignore together with this patch. Fam
On Sun, 5 Jan 2014 20:02:30 +0800 Amos Kong <akong@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 <akong@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? 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. > > if not do_c and not do_h: > do_c = True > do_h = True > > +if schema_dump_file: > + do_c = False > + do_h = False > + > c_file = output_dir + prefix + c_file > h_file = output_dir + prefix + h_file > > @@ -381,7 +390,40 @@ fdecl.write(mcgen(''' > ''', > guard=guardname(h_file))) > > -exprs = parse_schema(sys.stdin) > +exprs_all = parse_schema(sys.stdin) > + > +schema_table = """/* AUTOMATICALLY GENERATED, DO NOT MODIFY */ > + > +/* > + * Schema json string table converted from qapi-schema.json > + * > + * Copyright (c) 2013 Red Hat, Inc. > + * > + * Authors: > + * Amos Kong <akong@redhat.com> > + * > + * 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. > + * > + */ > + > +const char *const qmp_schema_table[] = { > +""" > + > +if schema_dump_file: > + for line in exprs_all[1]: > + line = re.sub(r'#.*\n', ' ', line.strip()) > + line = re.sub(r'\n', ' ', line.strip()) > + line = re.sub(r' +', ' ', line) > + schema_table += ' "%s",\n' % (line) > + > + schema_table += ' NULL };\n' > + f = open(schema_dump_file, "w") > + f.write(schema_table) > + f.flush() > + f.close() > + > +exprs = exprs_all[0] > exprs = filter(lambda expr: not expr.has_key('gen'), exprs) > > fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL")) > diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py > index 65f1a54..db68084 100644 > --- a/scripts/qapi-visit.py > +++ b/scripts/qapi-visit.py > @@ -480,7 +480,7 @@ fdecl.write(mcgen(''' > ''', > prefix=prefix, guard=guardname(h_file))) > > -exprs = parse_schema(sys.stdin) > +exprs = parse_schema(sys.stdin)[0] > > # to avoid header dependency hell, we always generate declarations > # for built-in types in our header files and simply guard them > diff --git a/scripts/qapi.py b/scripts/qapi.py > index 4263bbd..43cc401 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -61,10 +61,13 @@ class QAPISchema: > self.src += '\n' > self.cursor = 0 > self.exprs = [] > + self.raw_exprs = [] > self.accept() > > while self.tok != None: > - self.exprs.append(self.get_expr(False)) > + self.cur_entry= '' > + self.exprs.append(self.get_expr(False, True)) > + self.raw_exprs.append(self.cur_entry) > > def accept(self): > while True: > @@ -103,9 +106,11 @@ class QAPISchema: > elif not self.tok.isspace(): > raise QAPISchemaError(self, 'Stray "%s"' % self.tok) > > - def get_members(self): > + def get_members(self, start=None): > expr = OrderedDict() > if self.tok == '}': > + if start != None: > + self.cur_entry = self.src[start:self.cursor] > self.accept() > return expr > if self.tok != "'": > @@ -118,6 +123,8 @@ class QAPISchema: > self.accept() > expr[key] = self.get_expr(True) > if self.tok == '}': > + if start != None: > + self.cur_entry = self.src[start:self.cursor] > self.accept() > return expr > if self.tok != ',': > @@ -142,12 +149,15 @@ class QAPISchema: > raise QAPISchemaError(self, 'Expected "," or "]"') > self.accept() > > - def get_expr(self, nested): > + def get_expr(self, nested, first=False): > if self.tok != '{' and not nested: > raise QAPISchemaError(self, 'Expected "{"') > if self.tok == '{': > + start = None > + if first: > + start = self.cursor - 1 > self.accept() > - expr = self.get_members() > + expr = self.get_members(start) > elif self.tok == '[': > self.accept() > expr = self.get_values() > @@ -174,7 +184,7 @@ def parse_schema(fp): > elif expr.has_key('type'): > add_struct(expr) > > - return schema.exprs > + return schema.exprs, schema.raw_exprs > > def parse_args(typeinfo): > if isinstance(typeinfo, basestring):
On Wed, Jan 22, 2014 at 01:06:51PM -0500, Luiz Capitulino wrote: > On Sun, 5 Jan 2014 20:02:30 +0800 > Amos Kong <akong@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 <akong@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? 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
On Thu, Jan 23, 2014 at 11:05:06AM +0800, Amos Kong 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 <akong@redhat.com> wrote: > > > 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? > > 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. The return data to QMP monitor is dynamically allocated, we can't generate some static struct/table reference to the OrderedDicts. I tried to generate "qmp_query_qmp_schema(Error **errp)" in qap-introspect.h. Python generates some redundant g_malloc0() code when visit all the OrderedDicts It's a little bit ugly, no performance effects. But it's failed, we have to use recursion functions to visit nested dictionaries. So I continue to add qmp_query_qmp_schema() in qmp.c. For reduceing the C work in running time, I also tried to parse out the metadata in Python. > > 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
On Thu, 23 Jan 2014 11:05:06 +0800 Amos Kong <akong@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 <akong@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 <akong@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 >
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 if not do_c and not do_h: do_c = True do_h = True +if schema_dump_file: + do_c = False + do_h = False + c_file = output_dir + prefix + c_file h_file = output_dir + prefix + h_file @@ -381,7 +390,40 @@ fdecl.write(mcgen(''' ''', guard=guardname(h_file))) -exprs = parse_schema(sys.stdin) +exprs_all = parse_schema(sys.stdin) + +schema_table = """/* AUTOMATICALLY GENERATED, DO NOT MODIFY */ + +/* + * Schema json string table converted from qapi-schema.json + * + * Copyright (c) 2013 Red Hat, Inc. + * + * Authors: + * Amos Kong <akong@redhat.com> + * + * 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. + * + */ + +const char *const qmp_schema_table[] = { +""" + +if schema_dump_file: + for line in exprs_all[1]: + line = re.sub(r'#.*\n', ' ', line.strip()) + line = re.sub(r'\n', ' ', line.strip()) + line = re.sub(r' +', ' ', line) + schema_table += ' "%s",\n' % (line) + + schema_table += ' NULL };\n' + f = open(schema_dump_file, "w") + f.write(schema_table) + f.flush() + f.close() + +exprs = exprs_all[0] exprs = filter(lambda expr: not expr.has_key('gen'), exprs) fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL")) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 65f1a54..db68084 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -480,7 +480,7 @@ fdecl.write(mcgen(''' ''', prefix=prefix, guard=guardname(h_file))) -exprs = parse_schema(sys.stdin) +exprs = parse_schema(sys.stdin)[0] # to avoid header dependency hell, we always generate declarations # for built-in types in our header files and simply guard them diff --git a/scripts/qapi.py b/scripts/qapi.py index 4263bbd..43cc401 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -61,10 +61,13 @@ class QAPISchema: self.src += '\n' self.cursor = 0 self.exprs = [] + self.raw_exprs = [] self.accept() while self.tok != None: - self.exprs.append(self.get_expr(False)) + self.cur_entry= '' + self.exprs.append(self.get_expr(False, True)) + self.raw_exprs.append(self.cur_entry) def accept(self): while True: @@ -103,9 +106,11 @@ class QAPISchema: elif not self.tok.isspace(): raise QAPISchemaError(self, 'Stray "%s"' % self.tok) - def get_members(self): + def get_members(self, start=None): expr = OrderedDict() if self.tok == '}': + if start != None: + self.cur_entry = self.src[start:self.cursor] self.accept() return expr if self.tok != "'": @@ -118,6 +123,8 @@ class QAPISchema: self.accept() expr[key] = self.get_expr(True) if self.tok == '}': + if start != None: + self.cur_entry = self.src[start:self.cursor] self.accept() return expr if self.tok != ',': @@ -142,12 +149,15 @@ class QAPISchema: raise QAPISchemaError(self, 'Expected "," or "]"') self.accept() - def get_expr(self, nested): + def get_expr(self, nested, first=False): if self.tok != '{' and not nested: raise QAPISchemaError(self, 'Expected "{"') if self.tok == '{': + start = None + if first: + start = self.cursor - 1 self.accept() - expr = self.get_members() + expr = self.get_members(start) elif self.tok == '[': self.accept() expr = self.get_values() @@ -174,7 +184,7 @@ def parse_schema(fp): elif expr.has_key('type'): add_struct(expr) - return schema.exprs + return schema.exprs, schema.raw_exprs def parse_args(typeinfo): if isinstance(typeinfo, basestring):
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 <akong@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(-)