Re: [Qemu-devel] [PATCH 3/5] s390x: set missing parent for hotplug and quiesce events

[Christian Borntraeger <borntraeger@de.ibm.com>]

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 */
>  }
>