From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42825) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WhV60-0001MM-Tk for qemu-devel@nongnu.org; Mon, 05 May 2014 22:27:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WhV5s-0003V7-No for qemu-devel@nongnu.org; Mon, 05 May 2014 22:27:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:23789) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WhV5s-0003V2-Dn for qemu-devel@nongnu.org; Mon, 05 May 2014 22:26:56 -0400 Date: Tue, 6 May 2014 09:30:11 +0800 From: Fam Zheng Message-ID: <20140506013011.GA1574@T430.nay.redhat.com> References: <1398764656-27534-1-git-send-email-famz@redhat.com> <1398764656-27534-3-git-send-email-famz@redhat.com> <8761lktrg3.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8761lktrg3.fsf@blackfin.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH v2 2/2] qapi: Allow setting default values for optional parameters List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , eblake@redhat.com Cc: Kevin Wolf , Peter Maydell , Michael Roth , qemu-devel@nongnu.org, Luiz Capitulino , akong@redhat.com On Mon, 05/05 13:06, Markus Armbruster wrote: > Fam Zheng 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