Re: [Qemu-devel] [PATCH 03/16] qdev-properties: add PROP_TYPE_ENUM

[Anthony Liguori <anthony@codemonkey.ws>]

On 02/07/2011 08:49 AM, Anthony Liguori wrote:
> On 02/07/2011 08:14 AM, Alon Levy wrote:
>> Fair enough, so a patch that added enumeration through QMP would be 
>> acceptable?
>> I'm not even sure that makes sense, would you mind outlining how you 
>> see this
>> implemented?
>
> Before we write any code, we need to figure out what an enum is, how 
> we want to transport it via QMP, and how we will support introspection 
> on it.
>
> In terms of the protocol format, I'm inclined to suggest that we 
> transmit enums as integers as it maps better to C.  That way, we can 
> write a first class enumeration type and build our interfaces on top 
> of that.
>
> I'm working on a QMP rewrite using a schema located at 
> http://repo.or.cz/w/qemu/aliguori.git/shortlog/refs/heads/glib
>
> The schema has the notion of defining types along side with defining 
> interfaces.  Adding a syntax to define an enum would be pretty 
> straight forward.  Probably something like:
>
> { 'DriftCompensationPolicy': [{'gradual': 'int'}, {'fast': 'int'}, 
> {'none': 'int'}] }

http://repo.or.cz/w/qemu/aliguori.git/commit/50375b29f512c18d03cec8b1f9fea47ffdb6b232

Is a start at what I'm talking about here.  Adding the qdev type bits 
should be pretty straight forward.

Regards,

Anthony Liguori

>
> This would in turn generate:
>
> typedef enum DriftCompensationPolicy {
>     DRIFT_COMPENSATION_POLICY_GRADUAL = 0,
>     DRIFT_COMPENSATION_POLICY_FAST,
>     DRIFT_COMPENSATION_POLICY_NONE
> } DriftCompensationPolicy;
>
> From a QMP side, the value would be marshalled as an integer but the 
> schema would contain the type 'DriftCompensationPolicy' as the type.
>
> For -device, this would mean that enums would be treated as integer 
> properties, and we'd need some boiler plate code to register the enum 
> with symbolic names.
>
> From a qdev perspective, I think it would be easier to generate a 
> unique property type for every specific enum type.   That means the 
> qdev side ends up looking like:
>
> PropertyInfo qdev_prop_drift_compensation_policy = {
>     .name  = "DriftCompensationPolicy",
>     .type  = PROP_TYPE_INT32,
>     .size  = sizeof(DriftCompensationPolicy),
>     .parse = parse_drift_compensation_policy,
>     .print = print_drift_compensation_policy,
> };
>
> struct MyDevice {
>      DriftCompensationPolicy drift_policy;
> };
>
> DEFINE_PROP_DRIFT_COMPENSATION_POLICY("drift_policy", MyDevice, 
> drift_policy,
>                                                                                    
> DRIFT_COMPENSATION_POLICY_NONE);
>
> We could autogenerate all of this code from the QMP schema too.  It's 
> possible to do a one-off forwards compatible enum type for qdev but 
> I'd strongly prefer to get the QMP infrastructure in place first.
>
> But the advantage of this approach is pretty significant IMHO.  We get 
> to work in native C enumeration types both in QEMU and libqmp.  That's 
> a big win for type safety.
>
> BTW, if we treat QMP introspection as just returning the QMP schema 
> that we use for code generation (it's valid JSON afterall), then we 
> get enumeration introspection for free.
>
> I think this efficiently gives us what danpb's earlier series was 
> going for with his introspection patch set.
>
> Regards,
>
> Anthony Liguori
>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>>>
>
>