qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH 03/16] qdev-properties: add PROP_TYPE_ENUM
Date: Mon, 07 Feb 2011 09:23:39 -0600
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.15) Gecko/20101027 Lightning/1.0b1 Thunderbird/3.0.10

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








reply via email to

[Prev in Thread] Current Thread [Next in Thread]