Re: [Qemu-devel] [PATCH 2/3] pcdimm: add 'type' field to PCDIMMDeviceInfo

[Eric Blake]

On 02/03/2016 05:00 AM, Vladimir Sementsov-Ogievskiy wrote:

>>> +# @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?

Yes, exposing a finite set of strings as an enum is ideal for the user
interface, even if we carry string values instead of enum values in
other places in the code.  QAPI already includes convenience methods for
translating between strings and enum values (EnumName_lookup[] to go
from int to string, and qapi_enum_parse() to reverse from string back to
enum).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature