Re: [Qemu-devel] [RFC PATCH] qapi: Allow setting default values for optional parameters

[Amos Kong]

On Fri, Apr 18, 2014 at 03:15:09PM -0600, Eric Blake wrote:
> On 04/16/2014 12:04 AM, Fam Zheng wrote:
> > In command definition, 'default' is now parsed as a dict of default
> > values. Only optional parameters will have effect in generated code.
> 
> Can you make the python code explicitly error out for a default supplied
> for a parameter not marked with * in data?
> 
> > 
> > 'str' and 'int' are supported, both need single quote in the schema. In
> > generated code, 'str' will be converted to g_strdup'ed pointer, 'int'
> > will be identically set.
> > 
> > E.g.
> > 
> >     { 'command': 'block-commit',
> >       'data': { 'device': 'str', '*base': 'str', 'top': 'str',
> >                 '*speed': 'int' },
> >       'default': {'base': 'earthquake', 'speed': '100' } }
> 
> 'data' is plural, 'default' is singular; should it be named 'defaults'?
> 
> > 
> > will generate
> > 
> >     int qmp_marshal_input_block_commit(Monitor *mon, const QDict *qdict, 
> > QObject **ret)
> >     {
> >         ...
> >         bool has_base = true;
> >         char * base = g_strdup("earthquake");
> 
> Any reason the generator can't be fixed to avoid the space after '*'?

I knew the reason :-) I will post a patch later.

> >         ...
> >         bool has_speed = true;
> >         int64_t speed = 100;
> 
> Looks like it would work.  But why not allow a JSON integer for an
> integer default:
> 
> 'data': { '*base': 'str', '*speed', 'int' },
> 'default': { 'base': 'earthquake', 'speed': 100 },

Currently we set the default values insider qmp_COMMAND(), then we
need to remove it, or make sure the defule value is same as in
qapi-schema.json

Do we want to just support simple type? or more complex like:

'data': { '*value': 'ENUM_NAME', .... }
'default': { 'value': 'ENUM_NAME_E1', ... }

Or union, type, etc
 
> I'm not sure if that adds or reduces complexity; by using JSON types as
> the default, you have to add code in the generator to ensure correct
> type usage.  But it also opens the possibility of providing a default
> for a struct, rather than limiting to just strings and integers.
> 
> > 
> > Signed-off-by: Fam Zheng <address@hidden>
> > ---
> >  scripts/qapi-commands.py | 24 +++++++++++++++++-------
> >  scripts/qapi.py          |  8 ++++++++
> >  2 files changed, 25 insertions(+), 7 deletions(-)
> 
> Missing documentation and tests.  Docs to docs/qapi-code-gen.txt.


-- 
                        Amos.

pgpThp1MpMqbn.pgp
Description: PGP signature