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

[Anthony Liguori <anthony@codemonkey.ws>]

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'}] }

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
>>
>>
>>      
>