Re: [Qemu-devel] [PATCH v3 2/3] qapi: change qapi to convert schema json

[Amos Kong]

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 <address@hidden> 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.