From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [Qemu-devel] KVM call minutes for Feb 8 Date: Wed, 09 Feb 2011 08:44:58 -0600 Message-ID: <4D52A86A.1030407@codemonkey.ws> References: <20110208155557.GM6198@x200.localdomain> <4D51B1C9.3080507@codemonkey.ws> <4D526D0D.9020507@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Chris Wright , qemu-devel@nongnu.org, kvm@vger.kernel.org To: Markus Armbruster Return-path: Received: from mail-wy0-f174.google.com ([74.125.82.174]:37747 "EHLO mail-wy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755891Ab1BIOpE (ORCPT ); Wed, 9 Feb 2011 09:45:04 -0500 Received: by wyb28 with SMTP id 28so244856wyb.19 for ; Wed, 09 Feb 2011 06:45:03 -0800 (PST) In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: 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