| Message ID | 1357320880-10782-1-git-send-email-afaerber@suse.de |
|---|---|
| State | New |
| Headers | show |
On Fri, 4 Jan 2013 18:34:40 +0100 Andreas Färber <afaerber@suse.de> wrote: > Commit 667d22d1ae59da46b4c1fbd094ca61145f19b8c3 (qdev: move bus removal > to object_unparent) made the assumption that at unparenting time > parent_bus is not NULL. This assumption is unjustified since > object_unparent() may well be called directly after object_initialize(), > without any qdev_set_parent_bus(). > > This did not cause any issues yet because qdev_[try_]create() does call > qdev_set_parent_bus(), falling back to SysBus if unsupplied. > > While at it, ensure that this new function uses the device_ prefix and > make the name more neutral in light of this semantic change. > > Reported-by: Eduardo Habkost <ehabkost@redhat.com> > Signed-off-by: Andreas Färber <afaerber@suse.de> > Cc: Paolo Bonzini <pbonzini@redhat.com> > --- > Planning to insert this before the final CPU-as-a-device patch on qom-cpu, > to avoid a regression for, e.g., -cpu Haswell,enforce if unsupported by host. > > This supersedes my cosmetic patch in the "QOM realize, device-only" series: > > v1 -> v2: > * Make bus removal conditional on parent bus > * Rename function further > > hw/qdev.c | 8 +++++--- > 1 Datei geändert, 5 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-) > > diff --git a/hw/qdev.c b/hw/qdev.c > index f2c2484..e2a5c57 100644 > --- a/hw/qdev.c > +++ b/hw/qdev.c > @@ -698,16 +698,18 @@ static void device_class_base_init(ObjectClass *class, void *data) > klass->props = NULL; > } > > -static void qdev_remove_from_bus(Object *obj) > +static void device_unparent(Object *obj) > { > DeviceState *dev = DEVICE(obj); > > - bus_remove_child(dev->parent_bus, dev); > + if (dev->parent_bus != NULL) { > + bus_remove_child(dev->parent_bus, dev); > + } > } > > static void device_class_init(ObjectClass *class, void *data) > { > - class->unparent = qdev_remove_from_bus; > + class->unparent = device_unparent; > } > > void device_reset(DeviceState *dev) > -- > 1.7.10.4 > > Works for me, Tested-By: Igor Mammedov <imammedo@redhat.com>
Am 07.01.2013 14:47, schrieb Igor Mammedov: > On Fri, 4 Jan 2013 18:34:40 +0100 > Andreas Färber <afaerber@suse.de> wrote: > >> Commit 667d22d1ae59da46b4c1fbd094ca61145f19b8c3 (qdev: move bus removal >> to object_unparent) made the assumption that at unparenting time >> parent_bus is not NULL. This assumption is unjustified since >> object_unparent() may well be called directly after object_initialize(), >> without any qdev_set_parent_bus(). >> >> This did not cause any issues yet because qdev_[try_]create() does call >> qdev_set_parent_bus(), falling back to SysBus if unsupplied. >> >> While at it, ensure that this new function uses the device_ prefix and >> make the name more neutral in light of this semantic change. >> >> Reported-by: Eduardo Habkost <ehabkost@redhat.com> >> Signed-off-by: Andreas Färber <afaerber@suse.de> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> --- >> Planning to insert this before the final CPU-as-a-device patch on qom-cpu, >> to avoid a regression for, e.g., -cpu Haswell,enforce if unsupported by host. >> >> This supersedes my cosmetic patch in the "QOM realize, device-only" series: >> >> v1 -> v2: >> * Make bus removal conditional on parent bus >> * Rename function further >> >> hw/qdev.c | 8 +++++--- >> 1 Datei geändert, 5 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-) >> >> diff --git a/hw/qdev.c b/hw/qdev.c >> index f2c2484..e2a5c57 100644 >> --- a/hw/qdev.c >> +++ b/hw/qdev.c >> @@ -698,16 +698,18 @@ static void device_class_base_init(ObjectClass *class, void *data) >> klass->props = NULL; >> } >> >> -static void qdev_remove_from_bus(Object *obj) >> +static void device_unparent(Object *obj) >> { >> DeviceState *dev = DEVICE(obj); >> >> - bus_remove_child(dev->parent_bus, dev); >> + if (dev->parent_bus != NULL) { >> + bus_remove_child(dev->parent_bus, dev); >> + } >> } >> >> static void device_class_init(ObjectClass *class, void *data) >> { >> - class->unparent = qdev_remove_from_bus; >> + class->unparent = device_unparent; >> } >> >> void device_reset(DeviceState *dev) >> -- >> 1.7.10.4 >> >> > Works for me, > > Tested-By: Igor Mammedov <imammedo@redhat.com> Thanks, inserted into qom-cpu queue: https://github.com/afaerber/qemu-cpu/commits/qom-cpu Andreas
diff --git a/hw/qdev.c b/hw/qdev.c index f2c2484..e2a5c57 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -698,16 +698,18 @@ static void device_class_base_init(ObjectClass *class, void *data) klass->props = NULL; } -static void qdev_remove_from_bus(Object *obj) +static void device_unparent(Object *obj) { DeviceState *dev = DEVICE(obj); - bus_remove_child(dev->parent_bus, dev); + if (dev->parent_bus != NULL) { + bus_remove_child(dev->parent_bus, dev); + } } static void device_class_init(ObjectClass *class, void *data) { - class->unparent = qdev_remove_from_bus; + class->unparent = device_unparent; } void device_reset(DeviceState *dev)
Commit 667d22d1ae59da46b4c1fbd094ca61145f19b8c3 (qdev: move bus removal to object_unparent) made the assumption that at unparenting time parent_bus is not NULL. This assumption is unjustified since object_unparent() may well be called directly after object_initialize(), without any qdev_set_parent_bus(). This did not cause any issues yet because qdev_[try_]create() does call qdev_set_parent_bus(), falling back to SysBus if unsupplied. While at it, ensure that this new function uses the device_ prefix and make the name more neutral in light of this semantic change. Reported-by: Eduardo Habkost <ehabkost@redhat.com> Signed-off-by: Andreas Färber <afaerber@suse.de> Cc: Paolo Bonzini <pbonzini@redhat.com> --- Planning to insert this before the final CPU-as-a-device patch on qom-cpu, to avoid a regression for, e.g., -cpu Haswell,enforce if unsupported by host. This supersedes my cosmetic patch in the "QOM realize, device-only" series: v1 -> v2: * Make bus removal conditional on parent bus * Rename function further hw/qdev.c | 8 +++++--- 1 Datei geändert, 5 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)