From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34733) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WhXf0-0003oR-Gx for qemu-devel@nongnu.org; Tue, 06 May 2014 01:11:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WhXev-0001mJ-1c for qemu-devel@nongnu.org; Tue, 06 May 2014 01:11:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48217) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WhXeu-0001mB-PR for qemu-devel@nongnu.org; Tue, 06 May 2014 01:11:16 -0400 Date: Tue, 6 May 2014 13:11:24 +0800 From: Fam Zheng Message-ID: <20140506051124.GE1574@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> <20140506013011.GA1574@T430.nay.redhat.com> <53685259.2030109@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <53685259.2030109@redhat.com> 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: Eric Blake Cc: Kevin Wolf , Peter Maydell , Michael Roth , Markus Armbruster , qemu-devel@nongnu.org, Luiz Capitulino , akong@redhat.com On Mon, 05/05 21:09, Eric Blake wrote: > On 05/05/2014 07:30 PM, Fam Zheng wrote: > > >> NAME: { 'type': TYPE, 'default': DEFAULT } > >> > >> where > >> > >> NAME: { 'type': TYPE } > >> > >> can be abbreviated to > >> > >> NAME: TYPE > > > > > 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 } > > Oh, good catch. The alternative is to drop all instances of inline > sub-structs. Searching for 'data.*{.*{', I found only VersionInfo and > PciBridgeInfo; I then did a full manual search of qapi-schema.json and > only found the additional instance of PciDeviceInfo where the sub-struct > is not on the same line as the initial { of the 'data'. Just getting > rid of inlined sub-structs may be quite feasible. Sounds good. > > On a related vein, there are a number of types that aren't merely a > string name. For example: > > { 'command': 'migrate-set-capabilities', > 'data': { 'capabilities': ['MigrationCapabilityStatus'] } } > > and similar, where we have an array type rather than a raw string type > name. But at least with that, NAME: { 'type': [ 'array' ] } is still a > reasonable parse. > > The problem with MAGIC:{'name'...} is that you need to express more than > one parameter, but as a dict, you can't reuse the same MAGIC more than > once. That is: > > 'data': { MAGIC : { 'name': 'qemu', 'type': { 'major'...} }, > MAGIC : { 'name': 'package', 'type', 'str' } } } Right. Not necessarily better than dropping inline structure, just another idea: 'data' { '@arg_with_properties': { 'type': 'str' }, '*simple_optional': 'str', '@optional_full': { 'type': 'int', 'default': 1 }, 'the_most_common_form': 'bool', 'inline_structure': { 'major': ...} } Where '@' is a prefix for property dictionary similar to, while exclusive with, optional mark '*'. > > proves that you have to have two distinct MAGIC in the same 'data', so > '' for both is out. > > > > > 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. > > Also, while we aren't strict JSON, it's nice that we're still fairly > close to JSON5, and worth keeping that property. > > > > > Personally, I think empty str '' makes more sense for me. What do you think? > > > > At this point, I'm leaning towards dropping support for unnamed inlined > sub-structs. Yes. That said, I'm also leaning towards dropping, that keeps things simple. Fam