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
