Re: [Qemu-devel] [PATCH 05/10] qdev: push state up to Object

["Andreas Färber" <afaerber@suse.de>]

Am 23.05.2012 17:44, schrieb Paolo Bonzini:
> qdev properties use the state member (an embryo of the "realized"
> property) in order to disable setting them after a device has been
> initialized.  So, in order to push qdev properties up to Object
> we need to push this bit there too.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
[...]
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 5d6dc1f..552169b 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
[...]
> @@ -671,7 +671,6 @@ static void device_initfn(Object *obj)
>      }
>  
>      dev->instance_id_alias = -1;
> -    dev->state = DEV_STATE_CREATED;
>  
>      class = object_get_class(OBJECT(dev));
>      do {
[...]
> diff --git a/include/qemu/object.h b/include/qemu/object.h
> index cb08cfa..cb11288 100644
> --- a/include/qemu/object.h
> +++ b/include/qemu/object.h
> @@ -244,6 +244,11 @@ struct ObjectClass
>      const char *(*get_id)(Object *);
>  };
>  
> +typedef enum ObjectState {
> +    OBJECT_STATE_CREATED = 1,
> +    OBJECT_STATE_REALIZED,
> +} ObjectState;
> +
>  /**
>   * Object:
>   *
[...]
> diff --git a/qom/object.c b/qom/object.c
> index b19ef94..ed89b16 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1249,9 +1249,15 @@ static char *qdev_get_type(Object *obj, Error **errp)
>      return g_strdup(object_get_typename(obj));
>  }
>  
> +bool object_is_realized(Object *obj)
> +{
> +    return obj->state == OBJECT_STATE_REALIZED;
> +}
> +
>  static void object_instance_init(Object *obj)
>  {
>      object_property_add_str(obj, "type", qdev_get_type, NULL, NULL);
> +    obj->state = OBJECT_STATE_CREATED;

I wonder if we could improve the ordering here? device_initfn happens to
be the only instance_init function for qdev devices in hw/, but that's
different for Object.

What happens here is that TYPE_FOO is going to be initialized. TYPE_FOO
has .parent TYPE_BAR, TYPE_BAR in turn TYPE_OBJECT. So the QOM init
functions first run our object_instance_init(), which sets the state to
CREATED, then runs bar_initfn() and foo_initfn(). If those call any
helpers that check the object state, they would get a wrong impression.

Seeing that we only check for REALIZED or not, do we actually need this
state at all? We could just have OBJECT_STATE_UNREALIZED = 0 for
implicit initialization.

On the other hand the naming is a bit unfortunate, we actually seem to
mean INITIALIZED rather than CREATED, in QOM terms. Having an indicator
whether QOM actually created the object would solve part of the
qdev/qom/glib_allocated issue I ran into with QBus, i.e. a flag
OBJECT_STATE_ALLOCATED to be combined with INITIALIZED/REALIZED could
indicate whether we can/should object_delete() it in qbus_free(),
INITIALIZED would optionally indicate whether all initializers have been
run (move it out of object_instance_init for clearer semantics) and
REALIZED for the second-stage init we are longing to get. ;-)

Andreas

>  }
>  
>  static void object_class_init(ObjectClass *klass, void *class_data)


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