Patchwork [v3,2/3] qapi: change qapi to convert schema json

login
register
mail settings
Submitter Amos Kong
Date Jan. 5, 2014, 12:02 p.m.
Message ID <1388923351-10556-3-git-send-email-akong@redhat.com>
Download mbox | patch
Permalink /patch/306954/
State New
Headers show

Comments

Amos Kong - Jan. 5, 2014, 12:02 p.m.
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(-)
Fam Zheng - Jan. 6, 2014, 7:53 a.m.
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):
>
Fam Zheng - Jan. 6, 2014, 10:11 a.m.
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
Luiz Capitulino - Jan. 22, 2014, 6:06 p.m.
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):
Amos Kong - Jan. 23, 2014, 3:05 a.m.
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
Amos Kong - Jan. 23, 2014, 3:25 a.m.
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
Luiz Capitulino - Jan. 23, 2014, 1:30 p.m.
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
>

Patch

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