On 02/09/2011 06:28 AM, Markus Armbruster wrote:
Except that construction of a device requires initialization from an
array of variants (which is then type checked). The way we store the
variants is lossy because we convert back and forth to a string.
Yes, there's overlap, but no, a qdev property isn't yet another variant
type scheme. Exhibit A of the defense: qdev uses QemuOpts for variant
types.
Let me elaborate. qdev_device_add() uses QemuOpts as map from name to
variant type value, uses the name to look up the property, then uses
property methods to stuff the variant value it got from QemuOpts into
the (non-variant) struct member described by the property.
I figure QemuOpts was adopted for this purpose because it was already in
use with command line and human monitor. With QMP added to the mix,
there's friction: QMP uses QDict, not QemuOpts.
I'm going to finish QMP before tackling qdev, but at a high level,
here's how I think we fix this.
Right now, qdev only really supports construction properties. In
GObject parlance, this would be properties with G_PARAM_CONSTRUCT_ONLY.
Instead of the current approach of having the construction properties
automagically set as part of the object prior to initfn() being invoked,
we should have an init function that takes the full set of construction
only properties in the native type.
With a schema description of the device's constructor, we can generate
code that invokes the native constructor based on a QemuOpts, or based
on a QDict.
So instead of:
static int serial_isa_initfn(ISADevice *dev);
static ISADeviceInfo serial_isa_info = {
.init = serial_isa_initfn,
.qdev.props = (Property[]) {
DEFINE_PROP_UINT32("index", ISASerialState, index, -1),
DEFINE_PROP_HEX32("iobase", ISASerialState, iobase, -1),
DEFINE_PROP_UINT32("irq", ISASerialState, isairq, -1),
DEFINE_PROP_CHR("chardev", ISASerialState, state.chr),
DEFINE_PROP_END_OF_LIST(),
},
};
We'd have:
void isa_serial_init(ISASerialState *obj, uint32_t index, uint32_t
iobase, uint32_t irq, CharDriverState *chardev, Error **errp);
// isa_serial.json
[ 'ISASerialState', {'index': 'uint32_t', 'iobase': 'uint32_t', 'irq':
'uint32_t', 'chardev': 'CharDriverState *'} ]
From this definition, we can generate code that handles the -device
argument doing conversion from string to the appropriate types while
also doing QObject/GVariant conversion to support the qmp_device_add()
interface.
Also, having a well typed constructor means that we can do safer
composition because instead of doing:
DeviceState *dev;
dev = qdev_create(NULL, "isa-serial");
qdev_prop_set_uint32(dev, "iobase", 0x274);
qdev_prop_set_uint32(dev, "irq", 0x07);
qdev_init_nofail(dev);
We can just do:
ISASerialState dev;
isa_serial_init(&dev, 0, 0x274, 0x07, NULL, NULL);
I think one of the reasons qdev has not been thoroughly embraced in much
of QEMU is that the interface is obnoxious to work with in C. By
addressing that, I think it'll be much more successful.
Regards,
Anthony Liguori
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
