On Mon, 05/05 13:06, Markus Armbruster wrote: > Fam Zheng <f...@redhat.com> 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