| Message ID | 1358771422-14282-2-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
Am 21.01.2013 13:30, 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> I think I left some comments on v1: Would it be possible to leave a bus_initialize() function (without q ;)) in place that calls object_initialize() plus the-artist-formerly-known-as-qbus_realize(), shared between object_new() and object_initialize()? ->free was always set afterwards. The issue I am trying to contain here is a surge of *_init functions beyond class_init, instance_init, DeviceClass::init. Sticking to the QOM naming of having *bus_initialize() and *bus_new() would address that. Maybe if we reorder the two patches, dropping the use of g_malloc0() first? Currently care needs to be taken with the in-place bus initialization functions to not apply PCI_BUS() etc. on the uninitialized variable. Having a pci_bus_initialize(void *, ...) -> bus_initialize(void *, ...) -> object_initialize(void *, ...) call chain would solve that. Andreas
Il 21/01/2013 14:01, Andreas Färber ha scritto: >> > 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> > I think I left some comments on v1: Would it be possible to leave a > bus_initialize() function (without q ;)) in place that calls > object_initialize() plus the-artist-formerly-known-as-qbus_realize(), > shared between object_new() and object_initialize()? ->free was always > set afterwards. The issue I am trying to contain here is a surge of > *_init functions beyond class_init, instance_init, DeviceClass::init. > Sticking to the QOM naming of having *bus_initialize() and *bus_new() > would address that. Yes, you did and I thought about it but I had no good ideas really. We have a lot of differently named functions, but in the end "everything before realize should be named init" sounds like the only sensible rule... Paolo
diff --git a/hw/qdev-core.h b/hw/qdev-core.h index 731aadd..c9f7fa1 100644 --- a/hw/qdev-core.h +++ b/hw/qdev-core.h @@ -229,6 +229,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 9761016..b473bd7 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -390,14 +390,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; @@ -430,10 +432,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) @@ -441,10 +440,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; }
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(-)