| Message ID | 1443689387-34473-4-git-send-email-jfrei@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
Am 01.10.2015 um 10:49 schrieb Jens Freimann: > From: David Hildenbrand <dahi@linux.vnet.ibm.com> > > Existing code missed to set a parent for the quiesce and hotplug event. > While this didn't matter in practise, new introspection APIs basically now > do an object_unref(object_new(T)), which loops forever. > > When trying to remove the event facility bus, the code tries to > unparent all childs on the bus, so they are properly deleted and therefore removed. > As object_unparent() on these child devices doesn't work, as there is no parent, > we loop forever. > > Let's fix this by adding the event facility as a parent. Also switch from > object_initialize to object_new, so the only valid reference is in fact the > parent property. This makes it more obvious when the device (state) is actually > gone (and how the reference counting works). Markus, I applied this one to s390-next and it should make "qdev: Protect device-list-properties against broken devices" unnecessary for the sclp devices. Depending on who is upstream first, I will add a revert of your changes in event-facility.c and sclp.c or you can then drop the hunks. Christian > > Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com> > Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com> > --- > hw/s390x/event-facility.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c > index ef2a051..907b485 100644 > --- a/hw/s390x/event-facility.c > +++ b/hw/s390x/event-facility.c > @@ -27,8 +27,6 @@ typedef struct SCLPEventsBus { > struct SCLPEventFacility { > SysBusDevice parent_obj; > SCLPEventsBus sbus; > - SCLPEvent quiesce_event; > - SCLPEvent cpu_hotplug_event; > /* guest' receive mask */ > unsigned int receive_mask; > }; > @@ -347,19 +345,21 @@ static void init_event_facility(Object *obj) > { > SCLPEventFacility *event_facility = EVENT_FACILITY(obj); > DeviceState *sdev = DEVICE(obj); > + Object *new; > > /* Spawn a new bus for SCLP events */ > qbus_create_inplace(&event_facility->sbus, sizeof(event_facility->sbus), > TYPE_SCLP_EVENTS_BUS, sdev, NULL); > > - object_initialize(&event_facility->quiesce_event, sizeof(SCLPEvent), > - TYPE_SCLP_QUIESCE); > - qdev_set_parent_bus(DEVICE(&event_facility->quiesce_event), > - &event_facility->sbus.qbus); > - object_initialize(&event_facility->cpu_hotplug_event, sizeof(SCLPEvent), > - TYPE_SCLP_CPU_HOTPLUG); > - qdev_set_parent_bus(DEVICE(&event_facility->cpu_hotplug_event), > - &event_facility->sbus.qbus); > + new = object_new(TYPE_SCLP_QUIESCE); > + object_property_add_child(obj, TYPE_SCLP_QUIESCE, new, NULL); > + object_unref(new); > + qdev_set_parent_bus(DEVICE(new), &event_facility->sbus.qbus); > + > + new = object_new(TYPE_SCLP_CPU_HOTPLUG); > + object_property_add_child(obj, TYPE_SCLP_CPU_HOTPLUG, new, NULL); > + object_unref(new); > + qdev_set_parent_bus(DEVICE(new), &event_facility->sbus.qbus); > /* the facility will automatically realize the devices via the bus */ > } >
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c index ef2a051..907b485 100644 --- a/hw/s390x/event-facility.c +++ b/hw/s390x/event-facility.c @@ -27,8 +27,6 @@ typedef struct SCLPEventsBus { struct SCLPEventFacility { SysBusDevice parent_obj; SCLPEventsBus sbus; - SCLPEvent quiesce_event; - SCLPEvent cpu_hotplug_event; /* guest' receive mask */ unsigned int receive_mask; }; @@ -347,19 +345,21 @@ static void init_event_facility(Object *obj) { SCLPEventFacility *event_facility = EVENT_FACILITY(obj); DeviceState *sdev = DEVICE(obj); + Object *new; /* Spawn a new bus for SCLP events */ qbus_create_inplace(&event_facility->sbus, sizeof(event_facility->sbus), TYPE_SCLP_EVENTS_BUS, sdev, NULL); - object_initialize(&event_facility->quiesce_event, sizeof(SCLPEvent), - TYPE_SCLP_QUIESCE); - qdev_set_parent_bus(DEVICE(&event_facility->quiesce_event), - &event_facility->sbus.qbus); - object_initialize(&event_facility->cpu_hotplug_event, sizeof(SCLPEvent), - TYPE_SCLP_CPU_HOTPLUG); - qdev_set_parent_bus(DEVICE(&event_facility->cpu_hotplug_event), - &event_facility->sbus.qbus); + new = object_new(TYPE_SCLP_QUIESCE); + object_property_add_child(obj, TYPE_SCLP_QUIESCE, new, NULL); + object_unref(new); + qdev_set_parent_bus(DEVICE(new), &event_facility->sbus.qbus); + + new = object_new(TYPE_SCLP_CPU_HOTPLUG); + object_property_add_child(obj, TYPE_SCLP_CPU_HOTPLUG, new, NULL); + object_unref(new); + qdev_set_parent_bus(DEVICE(new), &event_facility->sbus.qbus); /* the facility will automatically realize the devices via the bus */ }