[RFC,V2,3/5] qapi script: add event support by qapi-event.py

qapi-event.py will parse the schema and generate qapi-event.c, then
the API in qapi-event.c can be used to handle event in qemu code.
All API have prefix "qapi_event", all types have prefix "QAPIEvent".
Examples can be found in following patches.

The script mainly include three parts: generate API for each event
define, generate an enum type for all defined event, generate behavior
control functions.

Since in some case the real emit behavior may change, for example,
qemu-img would not send a event, a callback layer is added to
control the behavior. As a result, the stubs at compile time
can be saved, the binding of block layer code and monitor code
will become looser.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 Makefile              |    9 +-
 Makefile.objs         |    2 +-
 scripts/qapi-event.py |  432 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 439 insertions(+), 4 deletions(-)
 create mode 100644 scripts/qapi-event.py

On Fri,  3 Jan 2014 07:10:32 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

> qapi-event.py will parse the schema and generate qapi-event.c, then
> the API in qapi-event.c can be used to handle event in qemu code.
> All API have prefix "qapi_event", all types have prefix "QAPIEvent".
> Examples can be found in following patches.
> 
> The script mainly include three parts: generate API for each event
> define, generate an enum type for all defined event, generate behavior
> control functions.
> 
> Since in some case the real emit behavior may change, for example,
> qemu-img would not send a event, a callback layer is added to
> control the behavior. As a result, the stubs at compile time
> can be saved, the binding of block layer code and monitor code
> will become looser.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  Makefile              |    9 +-
>  Makefile.objs         |    2 +-
>  scripts/qapi-event.py |  432 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 439 insertions(+), 4 deletions(-)
>  create mode 100644 scripts/qapi-event.py
> 
> diff --git a/Makefile b/Makefile
> index bdff4e4..fa59765 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -45,8 +45,8 @@ endif
>  endif
>  
>  GENERATED_HEADERS = config-host.h qemu-options.def
> -GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
> -GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c
> +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
> +GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
>  
>  GENERATED_HEADERS += trace/generated-events.h
>  GENERATED_SOURCES += trace/generated-events.c
> @@ -185,7 +185,7 @@ Makefile: $(version-obj-y) $(version-lobj-y)
>  # Build libraries
>  
>  libqemustub.a: $(stub-obj-y)
> -libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o
> +libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o qapi-event.o
>  
>  ######################################################################
>  
> @@ -226,6 +226,9 @@ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
>  qapi-visit.c qapi-visit.h :\
>  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
>  	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o "." -b < $<, "  GEN   $@")
> +qapi-event.c qapi-event.h :\
> +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py)
> +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py $(gen-out-type) -o "." -b < $<, "  GEN   $@")
>  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   $@")
> diff --git a/Makefile.objs b/Makefile.objs
> index 2b6c1fe..33f5950 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -12,7 +12,7 @@ block-obj-y += main-loop.o iohandler.o qemu-timer.o
>  block-obj-$(CONFIG_POSIX) += aio-posix.o
>  block-obj-$(CONFIG_WIN32) += aio-win32.o
>  block-obj-y += block/
> -block-obj-y += qapi-types.o qapi-visit.o
> +block-obj-y += qapi-types.o qapi-visit.o qapi-event.o
>  block-obj-y += qemu-io-cmds.o
>  
>  block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
> new file mode 100644
> index 0000000..7526366
> --- /dev/null
> +++ b/scripts/qapi-event.py
> @@ -0,0 +1,432 @@
> +#
> +# QAPI event generator
> +#
> +# Copyright IBM, Corp. 2014
> +#
> +# Authors:
> +#  Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> +#
> +# This work is licensed under the terms of the GNU GPLv2+ or later.
> +# 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
> +
> +def _generate_event_api_name(event_name, params):

Why the underline? And, what you generate is a function declaration...

> +    api_name = "void qapi_event_send_%s(" % c_fun(event_name).lower();
> +    l = len(api_name)
> +
> +    if params:
> +        for argname, argentry, optional, structured in parse_args(params):
> +            if structured:
> +                sys.stderr.write("Nested structure define in event is not "
> +                                 "supported now, event '%s', argname '%s'\n" %
> +                                 (event_name, argname))
> +                sys.exit(1)
> +                continue
> +
> +            if optional:
> +                api_name += "bool has_%s,\n" % c_var(argname)
> +                api_name += "".ljust(l)
> +
> +            if argentry == "str":
> +                api_name += "const "
> +            api_name += "%s %s,\n" % (c_type(argentry), c_var(argname))
> +            api_name += "".ljust(l)
> +
> +    api_name += "Error **errp)"
> +    return api_name;
> +
> +
> +# Following are the core functions that transate user input into a qdict going

s/transate/translate

Although the comment doesn't make much sense to me.

> +# to be emitted in the wire.
> +
> +def generate_event_declaration(api_name):
> +    return mcgen('''
> +
> +%(api_name)s;
> +''',
> +                 api_name = api_name)
> +
> +def generate_event_implement(api_name, event_name, params):

I wonder if it would be clearer to to generate the declaration here.

> +    # step 1: declare and variables
> +    ret = mcgen("""
> +
> +%(api_name)s
> +{
> +    QDict *qmp;
> +    Error *local_err = NULL;
> +    QAPIEventFuncEmit emit;
> +""",
> +                api_name = api_name)
> +
> +    if params:
> +        ret += mcgen("""
> +    QmpOutputVisitor *qov;
> +    Visitor *v;
> +    QObject *obj;
> +
> +""")
> +
> +    # step 2: check emit function, create a dict
> +    ret += mcgen("""
> +    emit = qapi_event_get_func_emit();
> +    if (!emit) {
> +        return;
> +    }
> +
> +    qmp = qmp_event_build_dict("%(event_name)s");
> +
> +""",
> +                 event_name = event_name)
> +
> +    # step 3: visit the params if params != None
> +    if params:
> +        ret += mcgen("""
> +    qov = qmp_output_visitor_new();
> +    g_assert(qov);
> +
> +    v = qmp_output_get_visitor(qov);
> +    g_assert(v);
> +
> +    /* Fake visit, as if all member are under a structure */
> +    visit_start_struct(v, NULL, "", "%(event_name)s", 0, &local_err);
> +    if (error_is_set(&local_err)) {
> +        goto clean;
> +    }
> +
> +""",
> +                event_name = event_name)
> +
> +        for argname, argentry, optional, structured in parse_args(params):
> +            if structured:
> +                sys.stderr.write("Nested structure define in event is not "
> +                                 "supported now, event '%s', argname '%s'\n" %
> +                                 (event_name, argname))
> +                sys.exit(1)
> +
> +            if optional:
> +                ret += mcgen("""
> +    if (has_%(var)s) {
> +""",
> +                             var = c_var(argname))
> +                push_indent()
> +
> +            if argentry == "str":
> +                var_type = "(char **)"
> +            else:
> +                var_type = ""
> +
> +            ret += mcgen("""
> +    visit_type_%(type)s(v, %(var_type)s&%(var)s, "%(name)s", &local_err);
> +    if (error_is_set(&local_err)) {
> +        goto clean;
> +    }
> +""",
> +                         var_type = var_type,
> +                         var = c_var(argname),
> +                         type = type_name(argentry),
> +                         name = argname)
> +
> +            if optional:
> +                pop_indent()
> +                ret += mcgen("""
> +    }
> +""")
> +
> +        ret += mcgen("""
> +
> +    visit_end_struct(v, &local_err);
> +    if (error_is_set(&local_err)) {
> +        goto clean;
> +    }
> +
> +    obj = qmp_output_get_qobject(qov);
> +    g_assert(obj != NULL);
> +
> +    qdict_put_obj(qmp, "data", obj);
> +""")
> +
> +    # step 4: call qmp event api
> +    ret += mcgen("""
> +    emit(%(event_enum_value)s, qmp, &local_err);
> +
> +""",
> +                 event_enum_value = event_enum_value)
> +
> +    # step 5: clean up
> +    if params:
> +        ret += mcgen("""
> + clean:
> +    qmp_output_visitor_cleanup(qov);
> +""")
> +    ret += mcgen("""
> +    error_propagate(errp, local_err);
> +    QDECREF(qmp);
> +}
> +""")
> +
> +    return ret
> +
> +
> +# Following are the functions that generate an enum type for all defined
> +# events, similar with qapi-types.py. Here we already have enum name and
> +# values which is generated before and recorded in event_enum_*. It also
> +# walk around the issue that "import qapi-types" can't work.
> +
> +def generate_event_enum_decl(event_enum_name, event_enum_values):
> +    lookup_decl = mcgen('''
> +
> +extern const char *%(event_enum_name)s_lookup[];
> +''',
> +                        event_enum_name = event_enum_name)
> +
> +    enum_decl = mcgen('''
> +typedef enum %(event_enum_name)s
> +{
> +''',
> +                      event_enum_name = event_enum_name)
> +
> +    # append automatically generated _MAX value
> +    enum_max_value = generate_enum_full_value_string(event_enum_name, "MAX")
> +    enum_values = event_enum_values + [ enum_max_value ]
> +
> +    i = 0
> +    for value in enum_values:
> +        enum_decl += mcgen('''
> +    %(value)s = %(i)d,
> +''',
> +                     value = value,
> +                     i = i)
> +        i += 1
> +
> +    enum_decl += mcgen('''
> +} %(event_enum_name)s;
> +''',
> +                       event_enum_name = event_enum_name)
> +
> +    return lookup_decl + enum_decl
> +
> +def generate_event_enum_lookup(event_enum_name, event_enum_strings):
> +    ret = mcgen('''
> +
> +const char *%(event_enum_name)s_lookup[] = {
> +''',
> +                event_enum_name = event_enum_name)
> +
> +    i = 0
> +    for string in event_enum_strings:
> +        ret += mcgen('''
> +    "%(string)s",
> +''',
> +                     string = string)
> +
> +    ret += mcgen('''
> +    NULL,
> +};
> +''')
> +    return ret
> +
> +
> +# Following are the functions that generate event behavior control functions.
> +# Those functions are put here in the qapi-event.c, since it need to include
> +# qapi-event.h for the event enum type declaration, put them in other file
> +# requiring other file include qapi-event.h, causing a cross including. For
> +# example: if we have qmp-event.c and qmp-event.h, then qmp-event.c
> +# ->qmp-event.h->qapi-event.h, qapi-event.c->qmp-event.h. Another problem
> +# follow: test-qapi-event.c will meet event enum double declaration since it
> +# include both test-qapi-event.h and qmp-event.h. One solution is putting event
> +# enum declaration in a separate header file, but then qmp-event.h need to
> +# include test-qapi-event.h or qapi-event.h on compile time condition. So the
> +# easist way is, just generate them here.
> +
> +def generate_event_behavior_control_decl(event_enum_name):
> +    ret = mcgen('''
> +
> +typedef void (*QAPIEventFuncEmit)(%(event_enum_name)s ev,
> +                                  QDict *dict,
> +                                  Error **errp);
> +
> +void qapi_event_set_func_emit(QAPIEventFuncEmit emit);
> +
> +QAPIEventFuncEmit qapi_event_get_func_emit(void);
> +''',
> +                event_enum_name = event_enum_name)
> +    return ret;
> +
> +def generate_event_behavior_control_implement():
> +    ret = mcgen('''
> +
> +typedef struct QAPIEventFunctions {
> +    QAPIEventFuncEmit emit;
> +} QAPIEventFunctions;
> +
> +QAPIEventFunctions qapi_event_functions;
> +
> +void qapi_event_set_func_emit(QAPIEventFuncEmit emit)
> +{
> +    qapi_event_functions.emit = emit;
> +}
> +
> +QAPIEventFuncEmit qapi_event_get_func_emit(void)
> +{
> +    return qapi_event_functions.emit;
> +}
> +''')
> +    return ret
> +
> +
> +# Start the real job
> +
> +try:
> +    opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:",
> +                                   ["source", "header", "builtins", "prefix=",
> +                                    "output-dir="])
> +except getopt.GetoptError, err:
> +    print str(err)
> +    sys.exit(1)
> +
> +output_dir = ""
> +prefix = ""
> +c_file = 'qapi-event.c'
> +h_file = 'qapi-event.h'
> +
> +do_c = False
> +do_h = False
> +do_builtins = False
> +
> +for o, a in opts:
> +    if o in ("-p", "--prefix"):
> +        prefix = a
> +    elif o in ("-o", "--output-dir"):
> +        output_dir = a + "/"
> +    elif o in ("-c", "--source"):
> +        do_c = True
> +    elif o in ("-h", "--header"):
> +        do_h = True
> +    elif o in ("-b", "--builtins"):
> +        do_builtins = True
> +
> +if not do_c and not do_h:
> +    do_c = True
> +    do_h = True
> +
> +c_file = output_dir + prefix + c_file
> +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()
> +
> +fdef = maybe_open(do_c, c_file, 'w')
> +fdecl = maybe_open(do_h, h_file, 'w')
> +
> +fdef.write(mcgen('''
> +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
> +
> +/*
> + * schema-defined QAPI event functions
> + *
> + * Copyright IBM, Corp. 2014
> + *
> + * Authors:
> + *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPLv2+ or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#include "qemu-common.h"
> +#include "%(header)s"
> +#include "%(prefix)sqapi-visit.h"
> +#include "qapi/qmp-output-visitor.h"
> +#include "qapi/qmp-event.h"
> +
> +''',
> +                 prefix=prefix, header=basename(h_file)))
> +
> +fdecl.write(mcgen('''
> +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
> +
> +/*
> + * schema-defined QAPI event function
> + *
> + * Copyright IBM, Corp. 2014
> + *
> + * Authors:
> + *  Wenchao Xia  <xiawenc@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPLv2+ or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#ifndef %(guard)s
> +#define %(guard)s
> +
> +#include "qapi/error.h"
> +#include "qapi/qmp/qdict.h"
> +#include "%(prefix)sqapi-types.h"
> +
> +''',
> +                  prefix=prefix, guard=guardname(h_file)))
> +
> +exprs = parse_schema(sys.stdin)
> +
> +event_enum_name = "QAPIEvent"
> +event_enum_values = []
> +event_enum_strings = []
> +
> +for expr in exprs:
> +    if expr.has_key('event'):
> +        event_name = expr['event']
> +        params = expr.get('data')
> +        if params and len(params) == 0:
> +            params = None
> +
> +        api_name = _generate_event_api_name(event_name, params)
> +        ret = generate_event_declaration(api_name)
> +        fdecl.write(ret)
> +
> +        # We need an enum value per event
> +        event_enum_value = generate_enum_full_value_string(event_enum_name,
> +                                                           event_name)
> +        ret = generate_event_implement(api_name, event_name, params)
> +        fdef.write(ret)
> +
> +        # Record it, and generate enum later
> +        event_enum_values.append(event_enum_value)
> +        event_enum_strings.append(event_name)
> +
> +ret = generate_event_enum_decl(event_enum_name, event_enum_values)
> +fdecl.write(ret)
> +ret = generate_event_enum_lookup(event_enum_name, event_enum_strings)
> +fdef.write(ret)
> +ret = generate_event_behavior_control_decl(event_enum_name)
> +fdecl.write(ret)
> +ret = generate_event_behavior_control_implement()
> +fdef.write(ret)
> +
> +fdecl.write('''
> +#endif
> +''')
> +
> +fdecl.flush()
> +fdecl.close()
> +
> +fdef.flush()
> +fdef.close()

[Pressed enter too soon, forgot two things]

On Mon, 6 Jan 2014 18:10:04 -0500
Luiz Capitulino <lcapitulino@redhat.com> wrote:

> On Fri,  3 Jan 2014 07:10:32 +0800
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
> 
> > qapi-event.py will parse the schema and generate qapi-event.c, then
> > the API in qapi-event.c can be used to handle event in qemu code.
> > All API have prefix "qapi_event", all types have prefix "QAPIEvent".
> > Examples can be found in following patches.
> > 
> > The script mainly include three parts: generate API for each event
> > define, generate an enum type for all defined event, generate behavior
> > control functions.
> > 
> > Since in some case the real emit behavior may change, for example,
> > qemu-img would not send a event, a callback layer is added to
> > control the behavior. As a result, the stubs at compile time
> > can be saved, the binding of block layer code and monitor code
> > will become looser.
> > 
> > Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> > ---
> >  Makefile              |    9 +-
> >  Makefile.objs         |    2 +-
> >  scripts/qapi-event.py |  432 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 439 insertions(+), 4 deletions(-)
> >  create mode 100644 scripts/qapi-event.py
> > 
> > diff --git a/Makefile b/Makefile
> > index bdff4e4..fa59765 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -45,8 +45,8 @@ endif
> >  endif
> >  
> >  GENERATED_HEADERS = config-host.h qemu-options.def
> > -GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
> > -GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c
> > +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
> > +GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
> >  
> >  GENERATED_HEADERS += trace/generated-events.h
> >  GENERATED_SOURCES += trace/generated-events.c
> > @@ -185,7 +185,7 @@ Makefile: $(version-obj-y) $(version-lobj-y)
> >  # Build libraries
> >  
> >  libqemustub.a: $(stub-obj-y)
> > -libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o
> > +libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o qapi-event.o
> >  
> >  ######################################################################
> >  
> > @@ -226,6 +226,9 @@ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
> >  qapi-visit.c qapi-visit.h :\
> >  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
> >  	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o "." -b < $<, "  GEN   $@")
> > +qapi-event.c qapi-event.h :\
> > +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py)
> > +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py $(gen-out-type) -o "." -b < $<, "  GEN   $@")
> >  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   $@")
> > diff --git a/Makefile.objs b/Makefile.objs
> > index 2b6c1fe..33f5950 100644
> > --- a/Makefile.objs
> > +++ b/Makefile.objs
> > @@ -12,7 +12,7 @@ block-obj-y += main-loop.o iohandler.o qemu-timer.o
> >  block-obj-$(CONFIG_POSIX) += aio-posix.o
> >  block-obj-$(CONFIG_WIN32) += aio-win32.o
> >  block-obj-y += block/
> > -block-obj-y += qapi-types.o qapi-visit.o
> > +block-obj-y += qapi-types.o qapi-visit.o qapi-event.o
> >  block-obj-y += qemu-io-cmds.o
> >  
> >  block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
> > diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
> > new file mode 100644
> > index 0000000..7526366
> > --- /dev/null
> > +++ b/scripts/qapi-event.py
> > @@ -0,0 +1,432 @@
> > +#
> > +# QAPI event generator
> > +#
> > +# Copyright IBM, Corp. 2014
> > +#
> > +# Authors:
> > +#  Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> > +#
> > +# This work is licensed under the terms of the GNU GPLv2+ or later.
> > +# 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
> > +
> > +def _generate_event_api_name(event_name, params):
> 
> Why the underline? And, what you generate is a function declaration...
> 
> > +    api_name = "void qapi_event_send_%s(" % c_fun(event_name).lower();
> > +    l = len(api_name)
> > +
> > +    if params:
> > +        for argname, argentry, optional, structured in parse_args(params):
> > +            if structured:
> > +                sys.stderr.write("Nested structure define in event is not "
> > +                                 "supported now, event '%s', argname '%s'\n" %
> > +                                 (event_name, argname))
> > +                sys.exit(1)
> > +                continue
> > +
> > +            if optional:
> > +                api_name += "bool has_%s,\n" % c_var(argname)
> > +                api_name += "".ljust(l)
> > +
> > +            if argentry == "str":
> > +                api_name += "const "
> > +            api_name += "%s %s,\n" % (c_type(argentry), c_var(argname))
> > +            api_name += "".ljust(l)
> > +
> > +    api_name += "Error **errp)"
> > +    return api_name;
> > +
> > +
> > +# Following are the core functions that transate user input into a qdict going
> 
> s/transate/translate
> 
> Although the comment doesn't make much sense to me.
> 
> > +# to be emitted in the wire.
> > +
> > +def generate_event_declaration(api_name):
> > +    return mcgen('''
> > +
> > +%(api_name)s;
> > +''',
> > +                 api_name = api_name)
> > +
> > +def generate_event_implement(api_name, event_name, params):
> 
> I wonder if it would be clearer to to generate the declaration here.
> 
> > +    # step 1: declare and variables
> > +    ret = mcgen("""
> > +
> > +%(api_name)s
> > +{
> > +    QDict *qmp;
> > +    Error *local_err = NULL;
> > +    QAPIEventFuncEmit emit;
> > +""",
> > +                api_name = api_name)
> > +
> > +    if params:
> > +        ret += mcgen("""
> > +    QmpOutputVisitor *qov;
> > +    Visitor *v;
> > +    QObject *obj;
> > +
> > +""")
> > +
> > +    # step 2: check emit function, create a dict
> > +    ret += mcgen("""
> > +    emit = qapi_event_get_func_emit();
> > +    if (!emit) {
> > +        return;
> > +    }
> > +
> > +    qmp = qmp_event_build_dict("%(event_name)s");
> > +
> > +""",
> > +                 event_name = event_name)
> > +
> > +    # step 3: visit the params if params != None
> > +    if params:
> > +        ret += mcgen("""
> > +    qov = qmp_output_visitor_new();
> > +    g_assert(qov);
> > +
> > +    v = qmp_output_get_visitor(qov);
> > +    g_assert(v);
> > +
> > +    /* Fake visit, as if all member are under a structure */
> > +    visit_start_struct(v, NULL, "", "%(event_name)s", 0, &local_err);
> > +    if (error_is_set(&local_err)) {
> > +        goto clean;
> > +    }
> > +
> > +""",
> > +                event_name = event_name)
> > +
> > +        for argname, argentry, optional, structured in parse_args(params):
> > +            if structured:
> > +                sys.stderr.write("Nested structure define in event is not "
> > +                                 "supported now, event '%s', argname '%s'\n" %
> > +                                 (event_name, argname))
> > +                sys.exit(1)
> > +
> > +            if optional:
> > +                ret += mcgen("""
> > +    if (has_%(var)s) {
> > +""",
> > +                             var = c_var(argname))
> > +                push_indent()
> > +
> > +            if argentry == "str":
> > +                var_type = "(char **)"
> > +            else:
> > +                var_type = ""
> > +
> > +            ret += mcgen("""
> > +    visit_type_%(type)s(v, %(var_type)s&%(var)s, "%(name)s", &local_err);
> > +    if (error_is_set(&local_err)) {
> > +        goto clean;
> > +    }
> > +""",
> > +                         var_type = var_type,
> > +                         var = c_var(argname),
> > +                         type = type_name(argentry),
> > +                         name = argname)
> > +
> > +            if optional:
> > +                pop_indent()
> > +                ret += mcgen("""
> > +    }
> > +""")
> > +
> > +        ret += mcgen("""
> > +
> > +    visit_end_struct(v, &local_err);
> > +    if (error_is_set(&local_err)) {
> > +        goto clean;
> > +    }
> > +
> > +    obj = qmp_output_get_qobject(qov);
> > +    g_assert(obj != NULL);
> > +
> > +    qdict_put_obj(qmp, "data", obj);
> > +""")
> > +
> > +    # step 4: call qmp event api
> > +    ret += mcgen("""
> > +    emit(%(event_enum_value)s, qmp, &local_err);
> > +
> > +""",
> > +                 event_enum_value = event_enum_value)
> > +
> > +    # step 5: clean up
> > +    if params:
> > +        ret += mcgen("""
> > + clean:
> > +    qmp_output_visitor_cleanup(qov);
> > +""")
> > +    ret += mcgen("""
> > +    error_propagate(errp, local_err);
> > +    QDECREF(qmp);
> > +}
> > +""")
> > +
> > +    return ret
> > +
> > +
> > +# Following are the functions that generate an enum type for all defined
> > +# events, similar with qapi-types.py. Here we already have enum name and
> > +# values which is generated before and recorded in event_enum_*. It also
> > +# walk around the issue that "import qapi-types" can't work.
> > +
> > +def generate_event_enum_decl(event_enum_name, event_enum_values):
> > +    lookup_decl = mcgen('''
> > +
> > +extern const char *%(event_enum_name)s_lookup[];
> > +''',
> > +                        event_enum_name = event_enum_name)
> > +
> > +    enum_decl = mcgen('''
> > +typedef enum %(event_enum_name)s
> > +{
> > +''',
> > +                      event_enum_name = event_enum_name)
> > +
> > +    # append automatically generated _MAX value
> > +    enum_max_value = generate_enum_full_value_string(event_enum_name, "MAX")
> > +    enum_values = event_enum_values + [ enum_max_value ]
> > +
> > +    i = 0
> > +    for value in enum_values:
> > +        enum_decl += mcgen('''
> > +    %(value)s = %(i)d,
> > +''',
> > +                     value = value,
> > +                     i = i)
> > +        i += 1
> > +
> > +    enum_decl += mcgen('''
> > +} %(event_enum_name)s;
> > +''',
> > +                       event_enum_name = event_enum_name)
> > +
> > +    return lookup_decl + enum_decl
> > +
> > +def generate_event_enum_lookup(event_enum_name, event_enum_strings):
> > +    ret = mcgen('''
> > +
> > +const char *%(event_enum_name)s_lookup[] = {
> > +''',
> > +                event_enum_name = event_enum_name)
> > +
> > +    i = 0
> > +    for string in event_enum_strings:
> > +        ret += mcgen('''
> > +    "%(string)s",
> > +''',
> > +                     string = string)
> > +
> > +    ret += mcgen('''
> > +    NULL,
> > +};
> > +''')
> > +    return ret
> > +
> > +
> > +# Following are the functions that generate event behavior control functions.
> > +# Those functions are put here in the qapi-event.c, since it need to include
> > +# qapi-event.h for the event enum type declaration, put them in other file
> > +# requiring other file include qapi-event.h, causing a cross including. For
> > +# example: if we have qmp-event.c and qmp-event.h, then qmp-event.c
> > +# ->qmp-event.h->qapi-event.h, qapi-event.c->qmp-event.h. Another problem
> > +# follow: test-qapi-event.c will meet event enum double declaration since it
> > +# include both test-qapi-event.h and qmp-event.h. One solution is putting event
> > +# enum declaration in a separate header file, but then qmp-event.h need to
> > +# include test-qapi-event.h or qapi-event.h on compile time condition. So the
> > +# easist way is, just generate them here.
> > +
> > +def generate_event_behavior_control_decl(event_enum_name):
> > +    ret = mcgen('''
> > +
> > +typedef void (*QAPIEventFuncEmit)(%(event_enum_name)s ev,
> > +                                  QDict *dict,
> > +                                  Error **errp);

Why does the emit function need 'ev'? Doesn't 'dict' contain all the
info it needs? Also, it's better to rename it to 'event' or 'qmp_event'.

> > +
> > +void qapi_event_set_func_emit(QAPIEventFuncEmit emit);
> > +
> > +QAPIEventFuncEmit qapi_event_get_func_emit(void);
> > +''',
> > +                event_enum_name = event_enum_name)
> > +    return ret;
> > +
> > +def generate_event_behavior_control_implement():
> > +    ret = mcgen('''
> > +
> > +typedef struct QAPIEventFunctions {
> > +    QAPIEventFuncEmit emit;
> > +} QAPIEventFunctions;
> > +
> > +QAPIEventFunctions qapi_event_functions;
> > +
> > +void qapi_event_set_func_emit(QAPIEventFuncEmit emit)
> > +{
> > +    qapi_event_functions.emit = emit;
> > +}

If this function and the typedefs don't change, I think they shouldn't
be generated.

> > +
> > +QAPIEventFuncEmit qapi_event_get_func_emit(void)
> > +{
> > +    return qapi_event_functions.emit;
> > +}
> > +''')
> > +    return ret
> > +
> > +
> > +# Start the real job
> > +
> > +try:
> > +    opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:",
> > +                                   ["source", "header", "builtins", "prefix=",
> > +                                    "output-dir="])
> > +except getopt.GetoptError, err:
> > +    print str(err)
> > +    sys.exit(1)
> > +
> > +output_dir = ""
> > +prefix = ""
> > +c_file = 'qapi-event.c'
> > +h_file = 'qapi-event.h'
> > +
> > +do_c = False
> > +do_h = False
> > +do_builtins = False
> > +
> > +for o, a in opts:
> > +    if o in ("-p", "--prefix"):
> > +        prefix = a
> > +    elif o in ("-o", "--output-dir"):
> > +        output_dir = a + "/"
> > +    elif o in ("-c", "--source"):
> > +        do_c = True
> > +    elif o in ("-h", "--header"):
> > +        do_h = True
> > +    elif o in ("-b", "--builtins"):
> > +        do_builtins = True
> > +
> > +if not do_c and not do_h:
> > +    do_c = True
> > +    do_h = True
> > +
> > +c_file = output_dir + prefix + c_file
> > +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()
> > +
> > +fdef = maybe_open(do_c, c_file, 'w')
> > +fdecl = maybe_open(do_h, h_file, 'w')
> > +
> > +fdef.write(mcgen('''
> > +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
> > +
> > +/*
> > + * schema-defined QAPI event functions
> > + *
> > + * Copyright IBM, Corp. 2014
> > + *
> > + * Authors:
> > + *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPLv2+ or later.
> > + * See the COPYING.LIB file in the top-level directory.
> > + *
> > + */
> > +
> > +#include "qemu-common.h"
> > +#include "%(header)s"
> > +#include "%(prefix)sqapi-visit.h"
> > +#include "qapi/qmp-output-visitor.h"
> > +#include "qapi/qmp-event.h"
> > +
> > +''',
> > +                 prefix=prefix, header=basename(h_file)))
> > +
> > +fdecl.write(mcgen('''
> > +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
> > +
> > +/*
> > + * schema-defined QAPI event function
> > + *
> > + * Copyright IBM, Corp. 2014
> > + *
> > + * Authors:
> > + *  Wenchao Xia  <xiawenc@linux.vnet.ibm.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPLv2+ or later.
> > + * See the COPYING.LIB file in the top-level directory.
> > + *
> > + */
> > +
> > +#ifndef %(guard)s
> > +#define %(guard)s
> > +
> > +#include "qapi/error.h"
> > +#include "qapi/qmp/qdict.h"
> > +#include "%(prefix)sqapi-types.h"
> > +
> > +''',
> > +                  prefix=prefix, guard=guardname(h_file)))
> > +
> > +exprs = parse_schema(sys.stdin)
> > +
> > +event_enum_name = "QAPIEvent"
> > +event_enum_values = []
> > +event_enum_strings = []
> > +
> > +for expr in exprs:
> > +    if expr.has_key('event'):
> > +        event_name = expr['event']
> > +        params = expr.get('data')
> > +        if params and len(params) == 0:
> > +            params = None
> > +
> > +        api_name = _generate_event_api_name(event_name, params)
> > +        ret = generate_event_declaration(api_name)
> > +        fdecl.write(ret)
> > +
> > +        # We need an enum value per event
> > +        event_enum_value = generate_enum_full_value_string(event_enum_name,
> > +                                                           event_name)
> > +        ret = generate_event_implement(api_name, event_name, params)
> > +        fdef.write(ret)
> > +
> > +        # Record it, and generate enum later
> > +        event_enum_values.append(event_enum_value)
> > +        event_enum_strings.append(event_name)
> > +
> > +ret = generate_event_enum_decl(event_enum_name, event_enum_values)
> > +fdecl.write(ret)
> > +ret = generate_event_enum_lookup(event_enum_name, event_enum_strings)
> > +fdef.write(ret)
> > +ret = generate_event_behavior_control_decl(event_enum_name)
> > +fdecl.write(ret)
> > +ret = generate_event_behavior_control_implement()
> > +fdef.write(ret)
> > +
> > +fdecl.write('''
> > +#endif
> > +''')
> > +
> > +fdecl.flush()
> > +fdecl.close()
> > +
> > +fdef.flush()
> > +fdef.close()
>

>> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
>> new file mode 100644
>> index 0000000..7526366
>> --- /dev/null
>> +++ b/scripts/qapi-event.py
>> @@ -0,0 +1,432 @@
>> +#
>> +# QAPI event generator
>> +#
>> +# Copyright IBM, Corp. 2014
>> +#
>> +# Authors:
>> +#  Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> +#
>> +# This work is licensed under the terms of the GNU GPLv2+ or later.
>> +# 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
>> +
>> +def _generate_event_api_name(event_name, params):
>
> Why the underline? And, what you generate is a function declaration...
>

   I want to tip it is not a public API. It is generating API name, 
missing ";" for declaration, implement may want it without ";". :)

>> +    api_name = "void qapi_event_send_%s(" % c_fun(event_name).lower();
>> +    l = len(api_name)
>> +
>> +    if params:
>> +        for argname, argentry, optional, structured in parse_args(params):
>> +            if structured:
>> +                sys.stderr.write("Nested structure define in event is not "
>> +                                 "supported now, event '%s', argname '%s'\n" %
>> +                                 (event_name, argname))
>> +                sys.exit(1)
>> +                continue
>> +
>> +            if optional:
>> +                api_name += "bool has_%s,\n" % c_var(argname)
>> +                api_name += "".ljust(l)
>> +
>> +            if argentry == "str":
>> +                api_name += "const "
>> +            api_name += "%s %s,\n" % (c_type(argentry), c_var(argname))
>> +            api_name += "".ljust(l)
>> +
>> +    api_name += "Error **errp)"
>> +    return api_name;
>> +
>> +
>> +# Following are the core functions that transate user input into a qdict going
>
> s/transate/translate
>
> Although the comment doesn't make much sense to me.
>

   OK, let me improve.

>> +# to be emitted in the wire.
>> +
>> +def generate_event_declaration(api_name):
>> +    return mcgen('''
>> +
>> +%(api_name)s;
>> +''',
>> +                 api_name = api_name)
>> +
>> +def generate_event_implement(api_name, event_name, params):
>
> I wonder if it would be clearer to to generate the declaration here.
>
   I think so, but a little slower at compile time. I use api_name to
avoid call _generate_event_api_name() twice. I am not sure which
way is better.

>>> +
>>> +
>>> +# Following are the functions that generate event behavior control functions.
>>> +# Those functions are put here in the qapi-event.c, since it need to include
>>> +# qapi-event.h for the event enum type declaration, put them in other file
>>> +# requiring other file include qapi-event.h, causing a cross including. For
>>> +# example: if we have qmp-event.c and qmp-event.h, then qmp-event.c
>>> +# ->qmp-event.h->qapi-event.h, qapi-event.c->qmp-event.h. Another problem
>>> +# follow: test-qapi-event.c will meet event enum double declaration since it
>>> +# include both test-qapi-event.h and qmp-event.h. One solution is putting event
>>> +# enum declaration in a separate header file, but then qmp-event.h need to
>>> +# include test-qapi-event.h or qapi-event.h on compile time condition. So the
>>> +# easist way is, just generate them here.
>>> +
>>> +def generate_event_behavior_control_decl(event_enum_name):
>>> +    ret = mcgen('''
>>> +
>>> +typedef void (*QAPIEventFuncEmit)(%(event_enum_name)s ev,
>>> +                                  QDict *dict,
>>> +                                  Error **errp);
>
> Why does the emit function need 'ev'? Doesn't 'dict' contain all the
> info it needs? Also, it's better to rename it to 'event' or 'qmp_event'.
>

   ev is for rate limit. I didn't move rate limit logic from callback to
internal, mainly because it is a generated function, which seems
too complex. And the genrated function was not moved into a separte
file mainly because the enum and including issue, see the comments
in script above.
   The problem is enum is changing, so the new file include it need
to recompile, for example, test schema and qapi-schema.json may
force recompile twice, I am not sure if it is good to do
some tricks in build system.

>>> +
>>> +void qapi_event_set_func_emit(QAPIEventFuncEmit emit);
>>> +
>>> +QAPIEventFuncEmit qapi_event_get_func_emit(void);
>>> +''',
>>> +                event_enum_name = event_enum_name)
>>> +    return ret;
>>> +
>>> +def generate_event_behavior_control_implement():
>>> +    ret = mcgen('''
>>> +
>>> +typedef struct QAPIEventFunctions {
>>> +    QAPIEventFuncEmit emit;
>>> +} QAPIEventFunctions;
>>> +
>>> +QAPIEventFunctions qapi_event_functions;
>>> +
>>> +void qapi_event_set_func_emit(QAPIEventFuncEmit emit)
>>> +{
>>> +    qapi_event_functions.emit = emit;
>>> +}
>
> If this function and the typedefs don't change, I think they shouldn't
> be generated.
>
>>> +
>>> +QAPIEventFuncEmit qapi_event_get_func_emit(void)
>>> +{
>>> +    return qapi_event_functions.emit;
>>> +}
>>> +''')
>>> +    return ret
>>> +
>>> +
>>> +# Start the real job
>>> +
>>> +try:
>>> +    opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:",
>>> +                                   ["source", "header", "builtins", "prefix=",
>>> +                                    "output-dir="])
>>> +except getopt.GetoptError, err:
>>> +    print str(err)
>>> +    sys.exit(1)
>>> +
>>> +output_dir = ""
>>> +prefix = ""
>>> +c_file = 'qapi-event.c'
>>> +h_file = 'qapi-event.h'
>>> +
>>> +do_c = False
>>> +do_h = False
>>> +do_builtins = False
>>> +
>>> +for o, a in opts:
>>> +    if o in ("-p", "--prefix"):
>>> +        prefix = a
>>> +    elif o in ("-o", "--output-dir"):
>>> +        output_dir = a + "/"
>>> +    elif o in ("-c", "--source"):
>>> +        do_c = True
>>> +    elif o in ("-h", "--header"):
>>> +        do_h = True
>>> +    elif o in ("-b", "--builtins"):
>>> +        do_builtins = True
>>> +
>>> +if not do_c and not do_h:
>>> +    do_c = True
>>> +    do_h = True
>>> +
>>> +c_file = output_dir + prefix + c_file
>>> +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()
>>> +
>>> +fdef = maybe_open(do_c, c_file, 'w')
>>> +fdecl = maybe_open(do_h, h_file, 'w')
>>> +
>>> +fdef.write(mcgen('''
>>> +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
>>> +
>>> +/*
>>> + * schema-defined QAPI event functions
>>> + *
>>> + * Copyright IBM, Corp. 2014
>>> + *
>>> + * Authors:
>>> + *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
>>> + *
>>> + * This work is licensed under the terms of the GNU GPLv2+ or later.
>>> + * See the COPYING.LIB file in the top-level directory.
>>> + *
>>> + */
>>> +
>>> +#include "qemu-common.h"
>>> +#include "%(header)s"
>>> +#include "%(prefix)sqapi-visit.h"
>>> +#include "qapi/qmp-output-visitor.h"
>>> +#include "qapi/qmp-event.h"
>>> +
>>> +''',
>>> +                 prefix=prefix, header=basename(h_file)))
>>> +
>>> +fdecl.write(mcgen('''
>>> +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
>>> +
>>> +/*
>>> + * schema-defined QAPI event function
>>> + *
>>> + * Copyright IBM, Corp. 2014
>>> + *
>>> + * Authors:
>>> + *  Wenchao Xia  <xiawenc@linux.vnet.ibm.com>
>>> + *
>>> + * This work is licensed under the terms of the GNU GPLv2+ or later.
>>> + * See the COPYING.LIB file in the top-level directory.
>>> + *
>>> + */
>>> +
>>> +#ifndef %(guard)s
>>> +#define %(guard)s
>>> +
>>> +#include "qapi/error.h"
>>> +#include "qapi/qmp/qdict.h"
>>> +#include "%(prefix)sqapi-types.h"
>>> +
>>> +''',
>>> +                  prefix=prefix, guard=guardname(h_file)))
>>> +
>>> +exprs = parse_schema(sys.stdin)
>>> +
>>> +event_enum_name = "QAPIEvent"
>>> +event_enum_values = []
>>> +event_enum_strings = []
>>> +
>>> +for expr in exprs:
>>> +    if expr.has_key('event'):
>>> +        event_name = expr['event']
>>> +        params = expr.get('data')
>>> +        if params and len(params) == 0:
>>> +            params = None
>>> +
>>> +        api_name = _generate_event_api_name(event_name, params)
>>> +        ret = generate_event_declaration(api_name)
>>> +        fdecl.write(ret)
>>> +
>>> +        # We need an enum value per event
>>> +        event_enum_value = generate_enum_full_value_string(event_enum_name,
>>> +                                                           event_name)
>>> +        ret = generate_event_implement(api_name, event_name, params)
>>> +        fdef.write(ret)
>>> +
>>> +        # Record it, and generate enum later
>>> +        event_enum_values.append(event_enum_value)
>>> +        event_enum_strings.append(event_name)
>>> +
>>> +ret = generate_event_enum_decl(event_enum_name, event_enum_values)
>>> +fdecl.write(ret)
>>> +ret = generate_event_enum_lookup(event_enum_name, event_enum_strings)
>>> +fdef.write(ret)
>>> +ret = generate_event_behavior_control_decl(event_enum_name)
>>> +fdecl.write(ret)
>>> +ret = generate_event_behavior_control_implement()
>>> +fdef.write(ret)
>>> +
>>> +fdecl.write('''
>>> +#endif
>>> +''')
>>> +
>>> +fdecl.flush()
>>> +fdecl.close()
>>> +
>>> +fdef.flush()
>>> +fdef.close()
>>
>

于 2014/1/7 11:24, Wenchao Xia 写道:
>>>> +
>>>> +
>>>> +# Following are the functions that generate event behavior control
>>>> functions.
>>>> +# Those functions are put here in the qapi-event.c, since it need
>>>> to include
>>>> +# qapi-event.h for the event enum type declaration, put them in
>>>> other file
>>>> +# requiring other file include qapi-event.h, causing a cross
>>>> including. For
>>>> +# example: if we have qmp-event.c and qmp-event.h, then qmp-event.c
>>>> +# ->qmp-event.h->qapi-event.h, qapi-event.c->qmp-event.h. Another
>>>> problem
>>>> +# follow: test-qapi-event.c will meet event enum double declaration
>>>> since it
>>>> +# include both test-qapi-event.h and qmp-event.h. One solution is
>>>> putting event
>>>> +# enum declaration in a separate header file, but then qmp-event.h
>>>> need to
>>>> +# include test-qapi-event.h or qapi-event.h on compile time
>>>> condition. So the
>>>> +# easist way is, just generate them here.
>>>> +
>>>> +def generate_event_behavior_control_decl(event_enum_name):
>>>> +    ret = mcgen('''
>>>> +
>>>> +typedef void (*QAPIEventFuncEmit)(%(event_enum_name)s ev,
>>>> +                                  QDict *dict,
>>>> +                                  Error **errp);
>>
>> Why does the emit function need 'ev'? Doesn't 'dict' contain all the
>> info it needs? Also, it's better to rename it to 'event' or 'qmp_event'.
>>
>
>    ev is for rate limit. I didn't move rate limit logic from callback to
> internal, mainly because it is a generated function, which seems
> too complex. And the genrated function was not moved into a separte
> file mainly because the enum and including issue, see the comments
> in script above.
>    The problem is enum is changing, so the new file include it need
> to recompile, for example, test schema and qapi-schema.json may
> force recompile twice, I am not sure if it is good to do
> some tricks in build system.
>
   I am going to respin this series after sync with upstream.
"ev" will be removed, and will try use a runtime call to solve
issue above.

On 01/02/2014 04:10 PM, Wenchao Xia wrote:
> qapi-event.py will parse the schema and generate qapi-event.c, then
> the API in qapi-event.c can be used to handle event in qemu code.
> All API have prefix "qapi_event", all types have prefix "QAPIEvent".
> Examples can be found in following patches.
> 
> The script mainly include three parts: generate API for each event
> define, generate an enum type for all defined event, generate behavior

s/event/events/

> control functions.
> 
> Since in some case the real emit behavior may change, for example,
> qemu-img would not send a event, a callback layer is added to
> control the behavior. As a result, the stubs at compile time
> can be saved, the binding of block layer code and monitor code
> will become looser.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  Makefile              |    9 +-
>  Makefile.objs         |    2 +-
>  scripts/qapi-event.py |  432 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 439 insertions(+), 4 deletions(-)
>  create mode 100644 scripts/qapi-event.py

In addition to Luiz' comments, you are missing documentation.  At a
minimum, docs/qapi-code-gen.txt should be updated.


> +try:
> +    opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:",
> +                                   ["source", "header", "builtins", "prefix=",
> +                                    "output-dir="])
> +except getopt.GetoptError, err:
> +    print str(err)
> +    sys.exit(1)
> +
> +output_dir = ""
> +prefix = ""
> +c_file = 'qapi-event.c'
> +h_file = 'qapi-event.h'
> +
> +do_c = False
> +do_h = False
> +do_builtins = False
> +
> +for o, a in opts:
> +    if o in ("-p", "--prefix"):
> +        prefix = a
> +    elif o in ("-o", "--output-dir"):
> +        output_dir = a + "/"
> +    elif o in ("-c", "--source"):
> +        do_c = True
> +    elif o in ("-h", "--header"):
> +        do_h = True
> +    elif o in ("-b", "--builtins"):
> +        do_builtins = True

You may need to rebase this on top of other patches that refactor the
qapi generators to track the input file, for improved error messages.

> +fdef.write(mcgen('''
> +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
> +
> +/*
> + * schema-defined QAPI event functions
> + *
> + * Copyright IBM, Corp. 2014
> + *
> + * Authors:
> + *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPLv2+ or later.
> + * See the COPYING.LIB file in the top-level directory.

Fix this.  If these are really GPL files, then they should refer to
COPYING.  If you meant LGPL for the generated files, say so (but that's
possibly at odds with other parts of your patch that put the generator
under GPL).

于 2014/3/7 2:49, Eric Blake 写道:
> On 01/02/2014 04:10 PM, Wenchao Xia wrote:
>> qapi-event.py will parse the schema and generate qapi-event.c, then
>> the API in qapi-event.c can be used to handle event in qemu code.
>> All API have prefix "qapi_event", all types have prefix "QAPIEvent".
>> Examples can be found in following patches.
>>
>> The script mainly include three parts: generate API for each event
>> define, generate an enum type for all defined event, generate behavior
> s/event/events/
>
>> control functions.
>>
>> Since in some case the real emit behavior may change, for example,
>> qemu-img would not send a event, a callback layer is added to
>> control the behavior. As a result, the stubs at compile time
>> can be saved, the binding of block layer code and monitor code
>> will become looser.
>>
>> Signed-off-by: Wenchao Xia<xiawenc@linux.vnet.ibm.com>
>> ---
>>   Makefile              |    9 +-
>>   Makefile.objs         |    2 +-
>>   scripts/qapi-event.py |  432 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 439 insertions(+), 4 deletions(-)
>>   create mode 100644 scripts/qapi-event.py
> In addition to Luiz' comments, you are missing documentation.  At a
> minimum, docs/qapi-code-gen.txt should be updated.
>
>
>> +try:
>> +    opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:",
>> +                                   ["source", "header", "builtins", "prefix=",
>> +                                    "output-dir="])
>> +except getopt.GetoptError, err:
>> +    print str(err)
>> +    sys.exit(1)
>> +
>> +output_dir = ""
>> +prefix = ""
>> +c_file = 'qapi-event.c'
>> +h_file = 'qapi-event.h'
>> +
>> +do_c = False
>> +do_h = False
>> +do_builtins = False
>> +
>> +for o, a in opts:
>> +    if o in ("-p", "--prefix"):
>> +        prefix = a
>> +    elif o in ("-o", "--output-dir"):
>> +        output_dir = a + "/"
>> +    elif o in ("-c", "--source"):
>> +        do_c = True
>> +    elif o in ("-h", "--header"):
>> +        do_h = True
>> +    elif o in ("-b", "--builtins"):
>> +        do_builtins = True
> You may need to rebase this on top of other patches that refactor the
> qapi generators to track the input file, for improved error messages.
   It seems qapi-visit.py and qapi-types.py remains the same as above in
upstream, which kind of change are your referring to?

>> +fdef.write(mcgen('''
>> +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
>> +
>> +/*
>> + * schema-defined QAPI event functions
>> + *
>> + * Copyright IBM, Corp. 2014
>> + *
>> + * Authors:
>> + *  Wenchao Xia<xiawenc@linux.vnet.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPLv2+ or later.
>> + * See the COPYING.LIB file in the top-level directory.
> Fix this.  If these are really GPL files, then they should refer to
> COPYING.  If you meant LGPL for the generated files, say so (but that's
> possibly at odds with other parts of your patch that put the generator
> under GPL).
>

On 03/18/2014 08:38 PM, Wenchao Xia wrote:
> 于 2014/3/7 2:49, Eric Blake 写道:
>> On 01/02/2014 04:10 PM, Wenchao Xia wrote:
>>> qapi-event.py will parse the schema and generate qapi-event.c, then
>>> the API in qapi-event.c can be used to handle event in qemu code.
>>> All API have prefix "qapi_event", all types have prefix "QAPIEvent".
>>> Examples can be found in following patches.
>>>

>>> +for o, a in opts:
>>> +    if o in ("-p", "--prefix"):
>>> +        prefix = a
>>> +    elif o in ("-o", "--output-dir"):
>>> +        output_dir = a + "/"
>>> +    elif o in ("-c", "--source"):
>>> +        do_c = True
>>> +    elif o in ("-h", "--header"):
>>> +        do_h = True
>>> +    elif o in ("-b", "--builtins"):
>>> +        do_builtins = True
>> You may need to rebase this on top of other patches that refactor the
>> qapi generators to track the input file, for improved error messages.
>   It seems qapi-visit.py and qapi-types.py remains the same as above in
> upstream, which kind of change are your referring to?

Lluís' patch to use an explicit input file via a new -i option:
https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05220.html

于 2014/3/21 6:29, Eric Blake 写道:
> On 03/18/2014 08:38 PM, Wenchao Xia wrote:
>> 于 2014/3/7 2:49, Eric Blake 写道:
>>> On 01/02/2014 04:10 PM, Wenchao Xia wrote:
>>>> qapi-event.py will parse the schema and generate qapi-event.c, then
>>>> the API in qapi-event.c can be used to handle event in qemu code.
>>>> All API have prefix "qapi_event", all types have prefix "QAPIEvent".
>>>> Examples can be found in following patches.
>>>>
>>>> +for o, a in opts:
>>>> +    if o in ("-p", "--prefix"):
>>>> +        prefix = a
>>>> +    elif o in ("-o", "--output-dir"):
>>>> +        output_dir = a + "/"
>>>> +    elif o in ("-c", "--source"):
>>>> +        do_c = True
>>>> +    elif o in ("-h", "--header"):
>>>> +        do_h = True
>>>> +    elif o in ("-b", "--builtins"):
>>>> +        do_builtins = True
>>> You may need to rebase this on top of other patches that refactor the
>>> qapi generators to track the input file, for improved error messages.
>>    It seems qapi-visit.py and qapi-types.py remains the same as above in
>> upstream, which kind of change are your referring to?
> Lluís' patch to use an explicit input file via a new -i option:
> https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05220.html
>
I see, will adjust it.

Wenchao Xia <wenchaoqemu@gmail.com> writes:

> 于 2014/3/21 6:29, Eric Blake 写道:
>> On 03/18/2014 08:38 PM, Wenchao Xia wrote:
>>> 于 2014/3/7 2:49, Eric Blake 写道:
>>>> On 01/02/2014 04:10 PM, Wenchao Xia wrote:
>>>>> qapi-event.py will parse the schema and generate qapi-event.c, then
>>>>> the API in qapi-event.c can be used to handle event in qemu code.
>>>>> All API have prefix "qapi_event", all types have prefix "QAPIEvent".
>>>>> Examples can be found in following patches.
>>>>>
>>>>> +for o, a in opts:
>>>>> +    if o in ("-p", "--prefix"):
>>>>> +        prefix = a
>>>>> +    elif o in ("-o", "--output-dir"):
>>>>> +        output_dir = a + "/"
>>>>> +    elif o in ("-c", "--source"):
>>>>> +        do_c = True
>>>>> +    elif o in ("-h", "--header"):
>>>>> +        do_h = True
>>>>> +    elif o in ("-b", "--builtins"):
>>>>> +        do_builtins = True
>>>> You may need to rebase this on top of other patches that refactor the
>>>> qapi generators to track the input file, for improved error messages.
>>>    It seems qapi-visit.py and qapi-types.py remains the same as above in
>>> upstream, which kind of change are your referring to?
>> Lluís' patch to use an explicit input file via a new -i option:
>> https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05220.html
>>
> I see, will adjust it.

Part of "[PATCH v4 0/3] qapi: Allow modularization of QAPI schema
files".  There's also Benoît's "[PATCH 0/2] Create and include directive
for QAPI JSON files"[*].  Benoît, care to explain how the two are
related?

Wenchao, please note "you *may* need to rebase".  First patch in wins :)

[*] https://lists.nongnu.org/archive/html/qemu-devel/2014-03/msg04412.html

The Wednesday 26 Mar 2014 à 13:42:38 (+0100), Markus Armbruster wrote :
> Wenchao Xia <wenchaoqemu@gmail.com> writes:
> 
> > 于 2014/3/21 6:29, Eric Blake 写道:
> >> On 03/18/2014 08:38 PM, Wenchao Xia wrote:
> >>> 于 2014/3/7 2:49, Eric Blake 写道:
> >>>> On 01/02/2014 04:10 PM, Wenchao Xia wrote:
> >>>>> qapi-event.py will parse the schema and generate qapi-event.c, then
> >>>>> the API in qapi-event.c can be used to handle event in qemu code.
> >>>>> All API have prefix "qapi_event", all types have prefix "QAPIEvent".
> >>>>> Examples can be found in following patches.
> >>>>>
> >>>>> +for o, a in opts:
> >>>>> +    if o in ("-p", "--prefix"):
> >>>>> +        prefix = a
> >>>>> +    elif o in ("-o", "--output-dir"):
> >>>>> +        output_dir = a + "/"
> >>>>> +    elif o in ("-c", "--source"):
> >>>>> +        do_c = True
> >>>>> +    elif o in ("-h", "--header"):
> >>>>> +        do_h = True
> >>>>> +    elif o in ("-b", "--builtins"):
> >>>>> +        do_builtins = True
> >>>> You may need to rebase this on top of other patches that refactor the
> >>>> qapi generators to track the input file, for improved error messages.
> >>>    It seems qapi-visit.py and qapi-types.py remains the same as above in
> >>> upstream, which kind of change are your referring to?
> >> Lluís' patch to use an explicit input file via a new -i option:
> >> https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05220.html
> >>
> > I see, will adjust it.
> 
> Part of "[PATCH v4 0/3] qapi: Allow modularization of QAPI schema
> files".  There's also Benoît's "[PATCH 0/2] Create and include directive
> for QAPI JSON files"[*].  Benoît, care to explain how the two are
> related?

I don't see the relation excepted that my patch changes the various qapi-generators
to accept their input files as argv[1].

The purpose of my patch is to implement a directive to modularise the pseudo JSON
files we use. Lluis told me that he would not have the time to work on his series
until the end of the month so I picked up the ball.

Best regards

Benoît

> 
> Wenchao, please note "you *may* need to rebase".  First patch in wins :)
> 
> [*] https://lists.nongnu.org/archive/html/qemu-devel/2014-03/msg04412.html

>>>>>> You may need to rebase this on top of other patches that refactor the
>>>>>> qapi generators to track the input file, for improved error messages.
>>>>>     It seems qapi-visit.py and qapi-types.py remains the same as above in
>>>>> upstream, which kind of change are your referring to?
>>>> Lluís' patch to use an explicit input file via a new -i option:
>>>> https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05220.html
>>>>
>>> I see, will adjust it.
>>
>> Part of "[PATCH v4 0/3] qapi: Allow modularization of QAPI schema
>> files".  There's also Benoît's "[PATCH 0/2] Create and include directive
>> for QAPI JSON files"[*].  Benoît, care to explain how the two are
>> related?
>
> I don't see the relation excepted that my patch changes the various qapi-generators
> to accept their input files as argv[1].
>
> The purpose of my patch is to implement a directive to modularise the pseudo JSON
> files we use. Lluis told me that he would not have the time to work on his series
> until the end of the month so I picked up the ball.
>
> Best regards
>
> Benoît
>
   Thanks for the explain, I'll rebase on top once your series merged
into Luiz's tree.


>>
>> Wenchao, please note "you *may* need to rebase".  First patch in wins :)
>>
>> [*] https://lists.nongnu.org/archive/html/qemu-devel/2014-03/msg04412.html