From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49709) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W0NHX-00023Y-Hk for qemu-devel@nongnu.org; Mon, 06 Jan 2014 22:24:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W0NHP-00036U-6D for qemu-devel@nongnu.org; Mon, 06 Jan 2014 22:24:43 -0500 Received: from e28smtp09.in.ibm.com ([122.248.162.9]:37178) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W0NHO-00036E-Ix for qemu-devel@nongnu.org; Mon, 06 Jan 2014 22:24:35 -0500 Received: from /spool/local by e28smtp09.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 7 Jan 2014 08:54:31 +0530 Received: from d28relay03.in.ibm.com (d28relay03.in.ibm.com [9.184.220.60]) by d28dlp03.in.ibm.com (Postfix) with ESMTP id 6BB1B1258051 for ; Tue, 7 Jan 2014 08:55:57 +0530 (IST) Received: from d28av05.in.ibm.com (d28av05.in.ibm.com [9.184.220.67]) by d28relay03.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s073OMaJ3014996 for ; Tue, 7 Jan 2014 08:54:23 +0530 Received: from d28av05.in.ibm.com (localhost [127.0.0.1]) by d28av05.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s073OSeh015967 for ; Tue, 7 Jan 2014 08:54:28 +0530 Message-ID: <52CB736B.1060901@linux.vnet.ibm.com> Date: Tue, 07 Jan 2014 11:24:27 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1388704234-22498-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1388704234-22498-4-git-send-email-xiawenc@linux.vnet.ibm.com> <20140106181004.192b4472@redhat.com> <20140106181700.3215f8ae@redhat.com> In-Reply-To: <20140106181700.3215f8ae@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH V2 3/5] qapi script: add event support by qapi-event.py List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: mreitz@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com >>> + >>> + >>> +# 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 >>> + * >>> + * 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 >>> + * >>> + * 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() >> >