Re: [Qemu-devel] [RFC 04/34] qdev: Prepare "realized" property

[Andreas Färber]

Am 12.12.2012 15:29, schrieb Eduardo Habkost:
> On Mon, Nov 26, 2012 at 01:12:16AM +0100, Andreas Färber 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 <address@hidden>
>> Signed-off-by: Andreas Färber <address@hidden>
>> Cc: Anthony Liguori <address@hidden>
>> ---
>>  hw/qdev-core.h |    4 +++
>>  hw/qdev.c      |  100 
>> ++++++++++++++++++++++++++++++++++++++++++--------------
>>  2 Dateien geändert, 80 Zeilen hinzugefügt(+), 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);
>>  
>>  struct VMStateDescription;
>>  
>> @@ -47,6 +49,8 @@ typedef struct DeviceClass {
>>      const struct VMStateDescription *vmsd;
>>  
>>      /* 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, const char 
>> *type)
>>     Return 0 on success.  */
>>  int qdev_init(DeviceState *dev)
>>  {
>> -    DeviceClass *dc = DEVICE_GET_CLASS(dev);
>> -    int rc;
>> +    Error *local_err = NULL;
>>  
>>      assert(!dev->realized);
>>  
>> -    rc = dc->init(dev);
>> -    if (rc < 0) {
>> +    object_property_set_bool(OBJECT(dev), true, "realized", &local_err);
>> +    if (local_err != NULL) {
>> +        error_free(local_err);
>>          object_delete(OBJECT(dev));
>> -        return rc;
>> +        return -1;
>>      }
>> +    return 0;
>> +}
>>  
>> -    if (!OBJECT(dev)->parent) {
>> -        static int unattached_count = 0;
>> -        gchar *name = g_strdup_printf("device[%d]", unattached_count++);
>> -
>> -        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 = DEVICE_GET_CLASS(dev);
>>  
>> -    if (qdev_get_vmsd(dev)) {
>> -        vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
>> -                                       dev->instance_id_alias,
>> -                                       dev->alias_required_for_version);
>> -    }
>> -    dev->realized = true;
>> -    if (dev->hotplugged) {
>> -        device_reset(dev);
>> +    if (dc->init) {
>> +        int rc = 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 = DEVICE_CLASS(oc);
>> +    dc->realize = device_realize;
>> +}
>> +
> [...]
> 
> 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

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg