Re: [Qemu-devel] [PATCH 01/11] qdev: export and use qbus_init

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

Am 05.12.2012 21:44, schrieb Paolo Bonzini:
> BusState subclasses need to do their own allocation because
> qbus_create_inplace calls object_initialize (which wipes out the
> "free" callback).  This patch separates the initialization of the object
> (object_initialize) from its insertion in the qdev tree (qbus_realize); to
> do so, it moves the remaining bits of qbus_create_inplace to qbus_realize
> and export it as qbus_init.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

The code movements look okay, but I wonder if we can resolve the "init"
ambivalence somehow? As in instance_init, which is being called
internally by object_new() and object_initialize().
You're right that it's not realize either. Maybe qbus_setup()?

Andreas

> ---
>  hw/qdev-core.h |  1 +
>  hw/qdev.c      | 18 +++++++-----------
>  2 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/qdev-core.h b/hw/qdev-core.h
> index fff7f0f..18f5f73 100644
> --- a/hw/qdev-core.h
> +++ b/hw/qdev-core.h
> @@ -180,6 +180,7 @@ DeviceState *qdev_find_recursive(BusState *bus, const char *id);
>  typedef int (qbus_walkerfn)(BusState *bus, void *opaque);
>  typedef int (qdev_walkerfn)(DeviceState *dev, void *opaque);
>  
> +void qbus_init(BusState *bus, DeviceState *parent, const char *name);
>  void qbus_create_inplace(BusState *bus, const char *typename,
>                           DeviceState *parent, const char *name);
>  BusState *qbus_create(const char *typename, DeviceState *parent, const char *name);
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 788b4da..e758131 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -403,14 +403,16 @@ DeviceState *qdev_find_recursive(BusState *bus, const char *id)
>      return NULL;
>  }
>  
> -static void qbus_realize(BusState *bus)
> +void qbus_init(BusState *bus, DeviceState *parent, const char *name)
>  {
>      const char *typename = object_get_typename(OBJECT(bus));
>      char *buf;
>      int i,len;
>  
> -    if (bus->name) {
> -        /* use supplied name */
> +    bus->parent = parent;
> +
> +    if (name) {
> +        bus->name = g_strdup(name);
>      } else if (bus->parent && bus->parent->id) {
>          /* parent device has id -> use it for bus name */
>          len = strlen(bus->parent->id) + 16;
> @@ -443,10 +445,7 @@ void qbus_create_inplace(BusState *bus, const char *typename,
>                           DeviceState *parent, const char *name)
>  {
>      object_initialize(bus, typename);
> -
> -    bus->parent = parent;
> -    bus->name = name ? g_strdup(name) : NULL;
> -    qbus_realize(bus);
> +    qbus_init(bus, parent, name);
>  }
>  
>  BusState *qbus_create(const char *typename, DeviceState *parent, const char *name)
> @@ -454,10 +453,7 @@ BusState *qbus_create(const char *typename, DeviceState *parent, const char *nam
>      BusState *bus;
>  
>      bus = BUS(object_new(typename));
> -
> -    bus->parent = parent;
> -    bus->name = name ? g_strdup(name) : NULL;
> -    qbus_realize(bus);
> +    qbus_init(bus, parent, name);
>  
>      return bus;
>  }
> 


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