[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/2] qapi: Allow setting default values for o
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/2] qapi: Allow setting default values for optional parameters |
Date: |
Tue, 6 May 2014 09:30:11 +0800 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Mon, 05/05 13:06, Markus Armbruster wrote:
> Fam Zheng <address@hidden> writes:
> > An example command is:
> >
> > { 'command': 'my-command',
> > - 'data': { 'arg1': 'str', '*arg2': 'str' },
> > + 'data': { 'arg1': 'str', '*arg2': 'str', '*arg3': 'int' },
> > + 'defaults': { 'arg2': 'default value for arg2', 'arg3': 12345 },
> > 'returns': 'str' }
> >
> >
>
> I'm only reviewing schema design here.
Thanks! That's very constructive.
>
> So far, a command parameter has three propagated: name, type, and
> whether it's optional. Undocumented hack: a type '**' means "who
> knows", and suppresses marshalling function generation for the command.
>
> The three properties are encoded in a single member of 'data': name
> becomes the member name, and type becomes the value, except optional is
> shoehorned into the name as well[*].
>
> Your patch adds have a fourth property, namely the default value. It is
> only valid for optional parameters. You put it into a separate member
> 'defaults', next to 'data. This spreads parameter properties over two
> separate objects. I don't like that. What if we come up with a fifth
> one? Then it'll get even worse.
>
> Can we keep a parameter's properties together? Perhaps like this:
>
> NAME: { 'type': TYPE, 'default': DEFAULT }
>
> where
>
> NAME: { 'type': TYPE }
>
> can be abbreviated to
>
> NAME: TYPE
>
> and
>
> NAME: { 'type': TYPE, 'default': null }
Good idea. We have a problem to solve though.
In data definition, we allow inline sub-structure:
{ 'type': 'VersionInfo',
'data': {'qemu': {'major': 'int', 'minor': 'int', 'micro': 'int'},
'package': 'str'} }
If we allow properties as a dict, we need to disambiguate properly from the
above case. Proposing:
MAGIC: { 'name': NAME, 'type: TYPE, 'default': DEFAULT }
Where MAGIC should NOT be something that is a valid NAME from current schema.
Some ideas:
- Special string, that has invalid chars as a identifier, like '*', '%', '&',
etc, or simply an empty str ''.
Of course we need to enforce the checking and distinguishing in
scripts/qapi-types.py.
- Non-string: current NAME must be a string, any type non-string could be
distinguished from NAME, like number, bool, null, []. But its meaning could
be confusing to reviewer.
- Special symbol: we can define a dedicate symbol for this, like the literal
TYPE, in the schema. But this way we deviate more from JSON.
Personally, I think empty str '' makes more sense for me. What do you think?
Anyway we only add things, so we will keep the '*name' suger.
Thanks,
Fam
- Re: [Qemu-devel] [PATCH v2 2/2] qapi: Allow setting default values for optional parameters, Markus Armbruster, 2014/05/05
- Re: [Qemu-devel] [PATCH v2 2/2] qapi: Allow setting default values for optional parameters, Markus Armbruster, 2014/05/05
- Re: [Qemu-devel] [PATCH v2 2/2] qapi: Allow setting default values for optional parameters, Eric Blake, 2014/05/05
- Re: [Qemu-devel] [PATCH v2 2/2] qapi: Allow setting default values for optional parameters, Markus Armbruster, 2014/05/05
- Re: [Qemu-devel] [PATCH v2 2/2] qapi: Allow setting default values for optional parameters, Eric Blake, 2014/05/05
- Re: [Qemu-devel] [PATCH v2 2/2] qapi: Allow setting default values for optional parameters, Markus Armbruster, 2014/05/06
- Re: [Qemu-devel] [PATCH v2 2/2] qapi: Allow setting default values for optional parameters, Eric Blake, 2014/05/06
- Re: [Qemu-devel] [PATCH v2 2/2] qapi: Allow setting default values for optional parameters, Markus Armbruster, 2014/05/06
Re: [Qemu-devel] [PATCH v2 2/2] qapi: Allow setting default values for optional parameters,
Fam Zheng <=
Re: [Qemu-devel] [PATCH v2 2/2] qapi: Allow setting default values for optional parameters, Markus Armbruster, 2014/05/06