From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56185) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T60DV-0007F3-Vi for qemu-devel@nongnu.org; Mon, 27 Aug 2012 10:23:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T60DP-0005mO-Tc for qemu-devel@nongnu.org; Mon, 27 Aug 2012 10:23:01 -0400 Received: from e7.ny.us.ibm.com ([32.97.182.137]:38560) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T60DP-0005mK-PO for qemu-devel@nongnu.org; Mon, 27 Aug 2012 10:22:55 -0400 Received: from /spool/local by e7.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 27 Aug 2012 10:22:54 -0400 Received: from d01relay04.pok.ibm.com (d01relay04.pok.ibm.com [9.56.227.236]) by d01dlp02.pok.ibm.com (Postfix) with ESMTP id EB0FA6E8047 for ; Mon, 27 Aug 2012 10:22:50 -0400 (EDT) Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by d01relay04.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q7REMnuH036978 for ; Mon, 27 Aug 2012 10:22:50 -0400 Received: from d03av02.boulder.ibm.com (loopback [127.0.0.1]) by d03av02.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q7REMh46002159 for ; Mon, 27 Aug 2012 08:22:43 -0600 From: Anthony Liguori In-Reply-To: <503B7E11.6050009@suse.de> References: <1345996298-4892-1-git-send-email-aliguori@us.ibm.com> <1345996298-4892-4-git-send-email-aliguori@us.ibm.com> <503B7E11.6050009@suse.de> Date: Mon, 27 Aug 2012 09:22:13 -0500 Message-ID: <873938l8iy.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 3/9] qbus: remove glib_allocated/qom_allocated and use release hook to free memory List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?utf-8?Q?F=C3=A4rber?= Cc: Paolo Bonzini , Liu Ping Fan , qemu-devel@nongnu.org Andreas F=C3=A4rber writes: > Am 26.08.2012 17:51, schrieb Anthony Liguori: >> Signed-off-by: Anthony Liguori > > That's a really nice solution for cleaning this up, thanks! > > Acked-by: Andreas F=C3=A4rber > > However one conceptional detail... > >> --- >> hw/pci.c | 7 ++++++- >> hw/qdev.c | 15 --------------- >> hw/qdev.h | 7 ------- >> hw/sysbus.c | 7 ++++++- >> 4 files changed, 12 insertions(+), 24 deletions(-) > [...] >> diff --git a/hw/qdev.c b/hw/qdev.c >> index b5a52ac..6b61daa 100644 >> --- a/hw/qdev.c >> +++ b/hw/qdev.c > [...] >> @@ -468,18 +466,6 @@ BusState *qbus_create(const char *typename, DeviceS= tate *parent, const char *nam >> return bus; >> } >>=20=20 >> -void qbus_free(BusState *bus) >> -{ >> - if (bus->qom_allocated) { >> - object_delete(OBJECT(bus)); >> - } else { >> - object_finalize(OBJECT(bus)); >> - if (bus->glib_allocated) { >> - g_free(bus); >> - } >> - } >> -} >> - >> static char *bus_get_fw_dev_path(BusState *bus, DeviceState *dev) >> { >> BusClass *bc =3D BUS_GET_CLASS(bus); >> @@ -698,7 +684,6 @@ static void device_finalize(Object *obj) >> if (dev->state =3D=3D DEV_STATE_INITIALIZED) { >> while (dev->num_child_bus) { >> bus =3D QLIST_FIRST(&dev->child_bus); >> - qbus_free(bus); >> } >> if (qdev_get_vmsd(dev)) { >> vmstate_unregister(dev, qdev_get_vmsd(dev), dev); > > I wonder how this is gonna work: The device used to be in charge of > tearing down its bus children ... now it neither deletes nor finalizes > nor unrefs? Is the while loop even still needed? > > Wouldn't the busses still have the device as parent, referencing it, > blocking device_finalize? This has never been right.. Just because a controller goes away, it doesn't mean that the devices ought to go away too. There are different types of "remove" so let's consider each. 1) Guest visible eject: if a controller is ejected, then the guest will obviously see everything behind it get removed too. This is an emulation detail, not a QOM thing. 2) Final deletion: this only happens when all references go away. If you eject a controller but there are still children that reference it, the controller won't go away. You actually need to delete each individual disk (or whatever is behind it) in order to break the reference counting. The eject notifier could walk the full bus and attempt to break the connections but honestly, I'd much prefer that we deprecate the current device_del interface and just do everything through QOM properties. That would mean manually deleting all of the devices behind the bus if that's really what you wanted to do. Regards, Anthony Liguori > > Regards, > Andreas > > --=20 > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3= =BCrnberg