From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:36557) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TipWu-0004sp-QU for qemu-devel@nongnu.org; Wed, 12 Dec 2012 11:51:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TipWk-0002nl-Cz for qemu-devel@nongnu.org; Wed, 12 Dec 2012 11:51:32 -0500 Received: from cantor2.suse.de ([195.135.220.15]:46764 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TipWk-0002nD-3Q for qemu-devel@nongnu.org; Wed, 12 Dec 2012 11:51:22 -0500 Message-ID: <50C8B606.4000102@suse.de> Date: Wed, 12 Dec 2012 17:51:18 +0100 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1353888766-6951-1-git-send-email-afaerber@suse.de> <1353888766-6951-5-git-send-email-afaerber@suse.de> <20121212142905.GA5334@otherpad.lan.raisama.net> In-Reply-To: <20121212142905.GA5334@otherpad.lan.raisama.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC 04/34] qdev: Prepare "realized" property List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: Paolo Bonzini , qemu-devel@nongnu.org, anthony@codemonkey.ws Am 12.12.2012 15:29, schrieb Eduardo Habkost: > On Mon, Nov 26, 2012 at 01:12:16AM +0100, Andreas F=E4rber wrote: >> Based on earlier patches by Paolo and me, introduce the QOM realizefn = at >> device level only, as requested by Anthony. >> >> For now this just wraps the qdev initfn. ...which it deprecates. >> >> Signed-off-by: Paolo Bonzini >> Signed-off-by: Andreas F=E4rber >> Cc: Anthony Liguori >> --- >> hw/qdev-core.h | 4 +++ >> hw/qdev.c | 100 ++++++++++++++++++++++++++++++++++++++++++-----= --------- >> 2 Dateien ge=E4ndert, 80 Zeilen hinzugef=FCgt(+), 24 Zeilen entfernt(= -) >> >> diff --git a/hw/qdev-core.h b/hw/qdev-core.h >> index f0d3a5e..580a811 100644 >> --- a/hw/qdev-core.h >> +++ b/hw/qdev-core.h >> @@ -29,6 +29,8 @@ enum { >> typedef int (*qdev_initfn)(DeviceState *dev); >> typedef int (*qdev_event)(DeviceState *dev); >> typedef void (*qdev_resetfn)(DeviceState *dev); >> +typedef void (*DeviceRealize)(DeviceState *dev, Error **err); >> +typedef void (*DeviceUnrealize)(DeviceState *dev, Error **err); >> =20 >> struct VMStateDescription; >> =20 >> @@ -47,6 +49,8 @@ typedef struct DeviceClass { >> const struct VMStateDescription *vmsd; >> =20 >> /* Private to qdev / bus. */ >> + DeviceRealize realize; >> + DeviceUnrealize unrealize; >> qdev_initfn init; >> qdev_event unplug; >> qdev_event exit; >> diff --git a/hw/qdev.c b/hw/qdev.c >> index bce6ad5..d7b6320 100644 >> --- a/hw/qdev.c >> +++ b/hw/qdev.c >> @@ -147,37 +147,30 @@ DeviceState *qdev_try_create(BusState *bus, cons= t char *type) >> Return 0 on success. */ >> int qdev_init(DeviceState *dev) >> { >> - DeviceClass *dc =3D DEVICE_GET_CLASS(dev); >> - int rc; >> + Error *local_err =3D NULL; >> =20 >> assert(!dev->realized); >> =20 >> - rc =3D dc->init(dev); >> - if (rc < 0) { >> + object_property_set_bool(OBJECT(dev), true, "realized", &local_er= r); >> + if (local_err !=3D NULL) { >> + error_free(local_err); >> object_delete(OBJECT(dev)); >> - return rc; >> + return -1; >> } >> + return 0; >> +} >> =20 >> - if (!OBJECT(dev)->parent) { >> - static int unattached_count =3D 0; >> - gchar *name =3D g_strdup_printf("device[%d]", unattached_coun= t++); >> - >> - object_property_add_child(container_get(qdev_get_machine(), >> - "/unattached"), >> - name, OBJECT(dev), NULL); >> - g_free(name); >> - } >> +static void device_realize(DeviceState *dev, Error **err) >> +{ >> + DeviceClass *dc =3D DEVICE_GET_CLASS(dev); >> =20 >> - if (qdev_get_vmsd(dev)) { >> - vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), d= ev, >> - dev->instance_id_alias, >> - dev->alias_required_for_versio= n); >> - } >> - dev->realized =3D true; >> - if (dev->hotplugged) { >> - device_reset(dev); >> + if (dc->init) { >> + int rc =3D dc->init(dev); >> + if (rc < 0) { >> + error_setg(err, "Device initialization failed."); >> + return; >> + } >> } >> - return 0; >> } > [...] >> +static void device_class_init(ObjectClass *oc, void *data) >> +{ >> + DeviceClass *dc =3D DEVICE_CLASS(oc); >> + dc->realize =3D device_realize; >> +} >> + > [...] >=20 > Stupid question: what exactly is the difference in capabilities and > semantics between DeviceClass.init() and DeviceClass.realize()? They > look exactly the same to me, from a first look. On which cases would > somebody use the former instead of the latter, and why? Wherever possible, users should use the new DeviceClass::realize. "init" or "initfn" clashes with QOM's instance_init name-wise. As shown in this series, instance_init can be used in devices today already, without having realize. The distinction between instance_init and realize is that the former initializes simple variables, sets up properties and anything the user may want to modify, then realize processes these variables and is able to fail and report the cause of failure (unlike DeviceClass::init). In the isa patch after all the boring QOM'ish preparations, isa-device's class_init overwrites DeviceClass:realize, thereby no longer invoking any DeviceClass::init callback for the ISA devices. I2C, PCI, SysBus, etc. would need to be done in further steps, but a) this is work, b) the series shouldn't get too long for review/merge and c) it all depends on in which class and thereby with which signature we want to have realize - DeviceClass::realize(DeviceState *, Error **) or ObjectClass::realize(Object *, Error **). Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg