Patchwork [01/11] qdev: export and use qbus_init

login
register
mail settings
Submitter Paolo Bonzini
Date Dec. 5, 2012, 8:44 p.m.
Message ID <1354740282-20679-2-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/203939/
State New
Headers show

Comments

Paolo Bonzini - Dec. 5, 2012, 8:44 p.m.
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>
---
 hw/qdev-core.h |  1 +
 hw/qdev.c      | 18 +++++++-----------
 2 files changed, 8 insertions(+), 11 deletions(-)
Andreas Färber - Dec. 18, 2012, 9:18 p.m.
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;
>  }
>

Patch

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;
 }