Re: [Qemu-devel] [BUGFIX][PATCH for 2.2 v5 1/1] -machine vmport=auto: Fix handling of VMWare ioport emulation for xen

[Don Slutz <dslutz@verizon.com>]

On 11/21/14 10:10, Eduardo Habkost wrote:
> On Fri, Nov 21, 2014 at 09:56:33AM -0500, Don Slutz wrote:
>> On 11/21/14 08:53, Eduardo Habkost wrote:
>>> On Fri, Nov 21, 2014 at 08:30:58AM -0500, Don Slutz wrote:
>>>> On 11/20/14 21:01, Eduardo Habkost wrote:
>>>>> On Thu, Nov 20, 2014 at 05:32:56PM -0500, Don Slutz wrote:
>>>>> [...]
>> [...]
>>>>>> +    pcms->vmport = ON_OFF_AUTO_AUTO;
>>>>>> +    object_property_add(obj, PC_MACHINE_VMPORT, "str",
>>>>> I believe "OnOffAuto" is a valid type name, if it is defined in the QAPI
>>>>> schema.
>>>> I can only find:
>>>>
>>>> qapi-types.h:typedef enum OnOffAuto
>>>> qapi-types.h:} OnOffAuto;
>>> I don't understand what you mean, where did you expect to find it? You added it
>>> to the schema, so it is now a valid type name.
>>>
>>>> Which I use to define pcms->vmport.  The best I can translate this is
>>>> that "str" is what you are looking to replace.
>>>>
>>>> So I plan no change here.
>>> You are not defining a string property, but an OnOffAuto enum property,
>>> so why use "str"?
>>>
>>> See, for example, the type of an existing enum property:
>>> ide-hd.bios-chs-trans:
>>>
>>>    $ qemu-system-x86_64 -device ide-hd,?
>>>    [...]
>>>    ide-hd.bios-chs-trans=BiosAtaTranslation (Logical CHS translation algorithm, auto/none/lba/large/rechs)
>>>    [...]
>>>    $
>>>
>>>
>> Ok, I now understand what you are getting at.  From object.h:
>>
>>   * @type: the type name of the property.  This namespace is pretty loosely
>>   *   defined.  Sub namespaces are constructed by using a prefix and then
>>   *   to angle brackets.  For instance, the type 'virtio-net-pci' in the
>>   *   'link' namespace would be 'link<virtio-net-pci>'.
>>
>> It turns out that BiosAtaTranslation is a .name not a .type:
>>
> "-device ...,?" above is showing the property type. See
> qdev_device_help().
>
>> hyper-0-21-52:~/qemu>git grep BiosAtaTranslation
>> hw/core/qdev-properties.c:QEMU_BUILD_BUG_ON(sizeof(BiosAtaTranslation) !=
>> sizeof(int));
>> hw/core/qdev-properties.c:    .name = "BiosAtaTranslation",
>> hw/core/qdev-properties.c:    .enum_table = BiosAtaTranslation_lookup,
>> qapi/block.json:# BiosAtaTranslation:
>> qapi/block.json:{ 'enum': 'BiosAtaTranslation',
> PropertyInfo.name is what is used as the 'type' parameter to
> object_property_add() when qdev adds a static property to the object.
> See qdev_property_add_static().
>
>>
>> so to me there are a few options:
>>
>> 1) "enum"
>> 2) "OnOffAuto"
>> 3) "enum<OnOffAuto>"
>> 4) "enum OnOffAuto"
>> 5) "enum-OnOffAuto"
>>
>> I think #2 was what you were thinking of, but I am not sure.  Please advise.
> The documentation is unclear about the set of valid type names. But as
> existing enum properties simply use the enum type name directly, I
> suggest "OnOffAuto" to follow existing practice.
>

Fine.  I went with this.  v6 posted.

    -Don Slutz