Re: [Qemu-devel] [PATCH 2/2] Introduce macro for defining qdev properties

[Anthony Liguori]

Paul Brook wrote:
> I think I agree. Using GCC extensions for error checking (e.g. DO_UPCAST) or 
> performance (__builtin_clz) is fine, but I'm a reluctant to rely on it for 
> correct operation.
>   
Practically speaking, we've never supported anything but GCC and I doubt 
we ever will.  In this case, it's an important part of something I'm 
trying to fix about the current property system.

It seems very brittle to me.  You have to specify a type in both the 
state structure and in the property definition.  Those two things are 
very, very far apart in the code.  Right now, the rules about type 
compatible are ill defined which makes it more likely to break beyond 
simple mistakes.  For instance, uint32 is used for uint32_t, int32_t, 
and int.  That seems odd.

I also don't like the fact that we mix field type information with 
display information.  I haven't thought about the best solution to this 
but I think it's either introducing new struct types or adding an 
optional decorator parameter.

The system I'm aiming for looks like this:

typedef struct {
  SysBusDevice parent;

  /* public */
  uint32_t queue_depth;
  uint32_t tx_mitigation_delay;
  CharDriverState *chr;

  /* private */
  ...
} MyDeviceState;

static Property my_device_properties[] = {
 QDEV_PROP(MyDeviceState, queue_depth),
 QDEV_PROP(MyDeviceState, tx_mitigation_delay),
 QDEV_PROP(MyDeviceState, chr),
 {}
};

Where there's a connection between properties and device state fields 
and there's no duplicate type information.  That means that for the most 
part, the rules of type compatible can be ignored by most users.

I'd like to see most uses of QDEV_PROP_NAME eliminated by renaming 
variables and accepting '-' in place of '_'.  We'll always need a way to 
accept default values.

I'm not sure how to do this without GCC extensions.  We could 
potentially add macro decorators and use a sparse-like tool to extract 
property lists automatically from device state.

--
Regards,

Anthony Liguori