From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40371) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZhyiA-0003SP-51 for qemu-devel@nongnu.org; Fri, 02 Oct 2015 07:41:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zhyi3-0002Jc-PG for qemu-devel@nongnu.org; Fri, 02 Oct 2015 07:41:13 -0400 Received: from e06smtp13.uk.ibm.com ([195.75.94.109]:57314) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zhyi3-0002JY-GP for qemu-devel@nongnu.org; Fri, 02 Oct 2015 07:41:07 -0400 Received: from localhost by e06smtp13.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 2 Oct 2015 12:41:06 +0100 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id 666F1219005C for ; Fri, 2 Oct 2015 12:40:34 +0100 (BST) Received: from d06av03.portsmouth.uk.ibm.com (d06av03.portsmouth.uk.ibm.com [9.149.37.213]) by b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t92Bf4hn32964676 for ; Fri, 2 Oct 2015 11:41:04 GMT Received: from d06av03.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av03.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t92Bf3QF024278 for ; Fri, 2 Oct 2015 05:41:04 -0600 References: <1443689387-34473-1-git-send-email-jfrei@linux.vnet.ibm.com> <1443689387-34473-4-git-send-email-jfrei@linux.vnet.ibm.com> From: Christian Borntraeger Message-ID: <560E6D4E.70904@de.ibm.com> Date: Fri, 2 Oct 2015 13:41:02 +0200 MIME-Version: 1.0 In-Reply-To: <1443689387-34473-4-git-send-email-jfrei@linux.vnet.ibm.com> Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/5] s390x: set missing parent for hotplug and quiesce events List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jens Freimann , Alexander Graf , Cornelia Huck Cc: David Hildenbrand , qemu-devel@nongnu.org, Markus Armbruster Am 01.10.2015 um 10:49 schrieb Jens Freimann: > From: David Hildenbrand > > 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 > Signed-off-by: Jens Freimann > --- > 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 */ > } >