From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57590) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W6Aup-00086Y-Tx for qemu-devel@nongnu.org; Wed, 22 Jan 2014 22:25:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W6Aul-0007Ar-7Q for qemu-devel@nongnu.org; Wed, 22 Jan 2014 22:25:15 -0500 Received: from mx1.redhat.com ([209.132.183.28]:6305) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W6Auk-0007Ah-VH for qemu-devel@nongnu.org; Wed, 22 Jan 2014 22:25:11 -0500 Date: Thu, 23 Jan 2014 11:25:01 +0800 From: Amos Kong Message-ID: <20140123032501.GA28603@amosk.info> References: <1388923351-10556-1-git-send-email-akong@redhat.com> <1388923351-10556-3-git-send-email-akong@redhat.com> <20140122130651.7480739a@redhat.com> <20140123030506.GA3580@amosk.info> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140123030506.GA3580@amosk.info> Subject: Re: [Qemu-devel] [PATCH v3 2/3] qapi: change qapi to convert schema json List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: mdroth@linux.vnet.ibm.com, qiaonuohan@cn.fujitsu.com, qemu-devel@nongnu.org, xiawenc@linux.vnet.ibm.com 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 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 -- Amos.