Re: [Qemu-devel] [RFC PATCH V2 3/5] qapi script: add event support by qapi-event.py

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2927 bytes --]

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

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]