qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Amos Kong <akong@redhat.com>
To: Luiz Capitulino <lcapitulino@redhat.com>
Cc: mdroth@linux.vnet.ibm.com, qiaonuohan@cn.fujitsu.com,
	qemu-devel@nongnu.org, xiawenc@linux.vnet.ibm.com
Subject: Re: [Qemu-devel] [PATCH v3 2/3] qapi: change qapi to convert schema json
Date: Thu, 23 Jan 2014 11:25:01 +0800	[thread overview]
Message-ID: <20140123032501.GA28603@amosk.info> (raw)
In-Reply-To: <20140123030506.GA3580@amosk.info>

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
 

-- 
			Amos.

  reply	other threads:[~2014-01-23  3:25 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-05 12:02 [Qemu-devel] [PATCH v3 0/3] QMP full introspection Amos Kong
2014-01-05 12:02 ` [Qemu-devel] [PATCH v3 1/3] qapi: cleanup redundant variable Amos Kong
2014-01-05 12:02 ` [Qemu-devel] [PATCH v3 2/3] qapi: change qapi to convert schema json Amos Kong
2014-01-06  7:53   ` Fam Zheng
2014-01-06 10:11   ` Fam Zheng
2014-01-22 18:06   ` Luiz Capitulino
2014-01-23  3:05     ` Amos Kong
2014-01-23  3:25       ` Amos Kong [this message]
2014-01-23 13:30       ` Luiz Capitulino
2014-01-05 12:02 ` [Qemu-devel] [PATCH v3 3/3] qmp: full introspection support for QMP Amos Kong
2014-01-06  9:37   ` Fam Zheng
2014-01-09  9:49     ` Amos Kong
2014-01-22 18:18       ` Luiz Capitulino

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140123032501.GA28603@amosk.info \
    --to=akong@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qiaonuohan@cn.fujitsu.com \
    --cc=xiawenc@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).