Re: [Qemu-devel] [RFC 02/10] fdc: add default drive type option

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> On 07/03/2015 09:18 AM, Markus Armbruster wrote:
[...]
>>> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
>>> index 0cfff1c..0872b41 100644
>>> --- a/include/hw/qdev-properties.h
>>> +++ b/include/hw/qdev-properties.h
>>> @@ -20,6 +20,7 @@ extern PropertyInfo qdev_prop_ptr;
>>>  extern PropertyInfo qdev_prop_macaddr;
>>>  extern PropertyInfo qdev_prop_losttickpolicy;
>>>  extern PropertyInfo qdev_prop_bios_chs_trans;
>>> +extern PropertyInfo qdev_prop_fdc_drive_type;
>>>  extern PropertyInfo qdev_prop_drive;
>>>  extern PropertyInfo qdev_prop_netdev;
>>>  extern PropertyInfo qdev_prop_vlan;
>>> diff --git a/qapi/block.json b/qapi/block.json
>>> index aad645c..0c747a1 100644
>>> --- a/qapi/block.json
>>> +++ b/qapi/block.json
>>> @@ -40,6 +40,21 @@
>>>    'data': ['auto', 'none', 'lba', 'large', 'rechs']}
>>>  
>>>  ##
>>> +# FDRIVE_DRV:
>> 
>> Stick in the usual @, please, just for consistency.
>> 
>> FDRIVE_DRV is an unusual name for a QAPI type.  I guess you chose it so
>> only the type name changes, but the enumeration constants stay the same.
>> You then hide away the type name change with a typedef in fdc.h.
>> 
>> Clever, but I think in the longer run, I'd rather take a more
>> conventional type name.
>> 
>
> If the rest of the series can be polished, I'll add a more traditional
> enum. This was more or less a quick PoC hack to avoid a lot of churn.

Makes sense, actually :)

[...]