Re: [Qemu-devel] KVM call minutes for Feb 8

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

 



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


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux