From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:50529)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <vsementsov@virtuozzo.com>) id 1aQw6t-0005bj-A4
	for qemu-devel@nongnu.org; Wed, 03 Feb 2016 07:00:36 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <vsementsov@virtuozzo.com>) id 1aQw6n-0004AU-6O
	for qemu-devel@nongnu.org; Wed, 03 Feb 2016 07:00:35 -0500
Received: from mx2.parallels.com ([199.115.105.18]:42764)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <vsementsov@virtuozzo.com>) id 1aQw6n-00049S-0G
	for qemu-devel@nongnu.org; Wed, 03 Feb 2016 07:00:29 -0500
Message-ID: <56B1EBCA.30201@virtuozzo.com>
Date: Wed, 3 Feb 2016 15:00:10 +0300
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
MIME-Version: 1.0
References: <1453963872-13549-1-git-send-email-vsementsov@virtuozzo.com>
	<1453963872-13549-3-git-send-email-vsementsov@virtuozzo.com>
	<56B129CC.4010005@redhat.com>
In-Reply-To: <56B129CC.4010005@redhat.com>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 2/3] pcdimm: add 'type' field to
	PCDIMMDeviceInfo
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: Eric Blake <eblake@redhat.com>, qemu-devel@nongnu.org
Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>, "Michael S. Tsirkin" <mst@redhat.com>, Markus Armbruster <armbru@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Igor Mammedov <imammedo@redhat.com>, "Denis V. Lunev" <den@openvz.org>

On 03.02.2016 01:12, Eric Blake wrote:
> On 01/27/2016 11:51 PM, Vladimir Sementsov-Ogievskiy wrote:
>> The field is needed to distinguish pc-dimm and nvdimm.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Stefan Hajnoczi <stefanha@redhat.com>
>> CC: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>> CC: "Michael S. Tsirkin" <mst@redhat.com>
>> CC: Igor Mammedov <imammedo@redhat.com>
>> CC: Eric Blake <eblake@redhat.com>
>> CC: Markus Armbruster <armbru@redhat.com>
>> ---
>> +++ b/qapi-schema.json
>> @@ -3924,6 +3924,8 @@
>>   #
>>   # @hotpluggable: true if device if could be added/removed while machine is running
>>   #
>> +# @type: device type: 'pc-dimm' or 'nvdimm' (since 2.6)
>> +#
>>   # Since: 2.1
>>   ##
>>   { 'struct': 'PCDIMMDeviceInfo',
>> @@ -3934,7 +3936,8 @@
>>               'node': 'int',
>>               'memdev': 'str',
>>               'hotplugged': 'bool',
>> -            'hotpluggable': 'bool'
>> +            'hotpluggable': 'bool',
>> +            'type': 'str'
> No. Since it is a finite set of values (just two possible), you should
> be using an enum here rather than open-coded 'str'. Something like:
>
> { 'enum': 'DIMMType', 'data': [ 'pc-dimm', 'nvdimm' ] }
>

Are you sure? This is only output Info, so user will never "set" this 
field. Also, qemu type system (as I understand) is based on string 
names. object_dynamic_cast and other functions uses "const char 
*typename". This enum will be out of qemu type system and we will have 
to sync it.. Is there already some practice of translating string 
typenames to enum values?


-- 
Best regards,
Vladimir