From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:43479)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1YnNu8-0007pH-2O
	for qemu-devel@nongnu.org; Wed, 29 Apr 2015 05:03:44 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1YnNu3-0003eb-Ae
	for qemu-devel@nongnu.org; Wed, 29 Apr 2015 05:03:39 -0400
Received: from mx1.redhat.com ([209.132.183.28]:56173)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1YnNu2-0003dr-VL
	for qemu-devel@nongnu.org; Wed, 29 Apr 2015 05:03:35 -0400
From: Markus Armbruster <armbru@redhat.com>
References: <1427995743-7865-1-git-send-email-armbru@redhat.com>
	<1427995743-7865-20-git-send-email-armbru@redhat.com>
	<20150423131303.GJ5289@noname.redhat.com>
Date: Wed, 29 Apr 2015 11:02:49 +0200
In-Reply-To: <20150423131303.GJ5289@noname.redhat.com> (Kevin Wolf's message
	of "Thu, 23 Apr 2015 15:13:03 +0200")
Message-ID: <87vbgfb93a.fsf@blackfin.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH RFC 19/19] qapi: New QMP command
	query-schema for QMP schema introspection
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Kevin Wolf <kwolf@redhat.com>
Cc: akong@redhat.com, berto@igalia.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com

Kevin Wolf <kwolf@redhat.com> writes:

> One question in addition to what Eric already asked:
>
> Am 02.04.2015 um 19:29 hat Markus Armbruster geschrieben:
>> +{ 'enum': 'SchemaMetaType',
>> +  'data': [ 'builtin', 'enum', 'array', 'object', 'alternate',
>> +            'command', 'event' ] }
>> +
>> +{ 'type': 'SchemaInfoBase',
>> +  'data': { 'name': 'str', 'meta-type': 'SchemaMetaType' } }
>> +
>> +{ 'enum': 'JSONPrimitiveType',
>> +  'data': [ 'str', 'int', 'number', 'bool', 'null' ] }
>> +
>> +{ 'type': 'SchemaInfoBuiltin',
>> +  'data': { 'json-type': 'JSONPrimitiveType' } }
>> +
>> +{ 'type': 'SchemaInfoEnum',
>> +  'data': { 'values': ['str'] } }
>> +
>> +{ 'type': 'SchemaInfoArray',
>> +  'data': { 'element-type': 'str' } }
>> +
>> +{ 'alternate': 'Value',
>> + 'data': { 'int': 'int', 'number': 'number', 'str': 'str', 'bool':
>> bool' } }
>> +
>> +{ 'type': 'SchemaInfoObjectMember',
>> +  'data': { 'name': 'str', 'type': 'str', '*default': 'Value' } }
>> +# @default's type must match @type
>> +# can only default simple types, not objects or arrays
>> +
>> +{ 'type': 'SchemaInfoObjectVariant',
>> +  'data': { 'case': 'str',
>> +            'members': [ 'SchemaInfoObjectMember' ] } }
>> +
>> +{ 'type': 'SchemaInfoObject',
>> +  'data': { 'members': [ 'SchemaInfoObjectMember' ],
>> +            '*discriminator': 'str',
>> +            '*variants': [ 'SchemaInfoObjectVariant' ] } }
>> +
>> +{ 'type': 'SchemaInfoAlternate',
>> +  'data': { 'members': [ 'SchemaInfoObjectMember' ] } }
>
> I'm not sure if I understand correctly how this one works. My guess
> would be this:
>
> - The elements in members describe the different variants. That is, you
>   choose of the members, whereas in SchemaInfoObject all of the members
>   exist.

Correct.

>          Perhaps they should be using different names then?

I simply reused SchemaInfoObjectMember.  Similar to how C's struct and
union share member declaration syntax.

> - The type (= discriminator) of a variant is indirectly specified by the
>   'type' of the SchemaInfoObjectMember: This is a QAPI type, but the
>   JSON type can be resolved by checking 'meta-type' of the QAPI type
>   (and SchemaInfoBuiltin if necessary)

Correct.

The introspection schema doesn't reflect the "the variant's JSON types
are all different" rule.

> - The 'name' in SchemaInfoObjectMember could be the key name of the
>   variant in the alternate definition. However, this name isn't used
>   anywhere in the QMP protocol, so maybe it is better kept internal.
>   'name' isn't optional, though.

This is actually a more convincing argument for having a separate
SchemaInfoAlternateMember.

> - The 'default' of SchemaInfoObjectMember must be omitted.

Likewise.

> Do I understand the intention reasonably right? If so, maybe it would be
> better to use a separate type for alternate variants.

I'll certainly consider that for the next iteration.

Thanks!