From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47899) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SXbG9-00065v-49 for qemu-devel@nongnu.org; Thu, 24 May 2012 12:51:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SXbDG-0005dh-Ry for qemu-devel@nongnu.org; Thu, 24 May 2012 12:48:40 -0400 Received: from cantor2.suse.de ([195.135.220.15]:33093 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SXbDG-0005cf-IO for qemu-devel@nongnu.org; Thu, 24 May 2012 12:48:34 -0400 Message-ID: <4FBE665B.8070900@suse.de> Date: Thu, 24 May 2012 18:48:27 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1335958273-769-1-git-send-email-pbonzini@redhat.com> <1335958273-769-19-git-send-email-pbonzini@redhat.com> In-Reply-To: <1335958273-769-19-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 18/21] qdev: convert busses to QEMU Object Model List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , Anthony Liguori Cc: peter.maydell@linaro.org, qemu-devel@nongnu.org Am 02.05.2012 13:31, schrieb Paolo Bonzini: > From: Anthony Liguori >=20 > This is far less interesting than it sounds. We simply add an Object t= o each > BusState and then register the types appropriately. Most of the intere= sting > refactoring will follow in the next patches. >=20 > Since we're changing fundamental type names (BusInfo -> BusClass), it a= ll needs > to convert at once. Fortunately, not a lot of code is affected. >=20 > Signed-off-by: Anthony Liguori > Signed-off-by: Paolo Bonzini Thanks, I applied this to qom-next with two changes: http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next First, I enforced all new TypeInfos to be static const rather than just static. If someone has time to fix up all the static ones in the code base to not set bad examples that would be appreciated - they needed to be writable originally while transitioning from qdev to QOM. Second, I applied the following patch on top, backporting object_delete() / object_finalize() from "qbus: initialize in standard way", since we introduce object_new() usage here: diff --git a/hw/pci.c b/hw/pci.c index c5d3100..b2afcf6 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -295,7 +295,7 @@ PCIBus *pci_bus_new(DeviceState *parent, const char *name, PCIBus *bus; bus =3D g_malloc0(sizeof(*bus)); - bus->qbus.qdev_allocated =3D 1; + bus->qbus.glib_allocated =3D true; pci_bus_new_inplace(bus, parent, name, address_space_mem, address_space_io, devfn_min); return bus; diff --git a/hw/qdev.c b/hw/qdev.c index 63baa0a..63012b5 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -423,7 +423,7 @@ BusState *qbus_create(const char *typename, DeviceState *parent, const char *nam BusState *bus; bus =3D BUS(object_new(typename)); - bus->qdev_allocated =3D 1; + bus->qom_allocated =3D true; do_qbus_create_inplace(bus, typename, parent, name); return bus; } @@ -443,8 +443,13 @@ void qbus_free(BusState *bus) qemu_unregister_reset(qbus_reset_all_fn, bus); } g_free((void*)bus->name); - if (bus->qdev_allocated) { - g_free(bus); + if (bus->qom_allocated) { + object_delete(OBJECT(bus)); + } else { + object_finalize(OBJECT(bus)); + if (bus->glib_allocated) { + g_free(bus); + } } } diff --git a/hw/qdev.h b/hw/qdev.h index 0da4b74..f3dd004 100644 --- a/hw/qdev.h +++ b/hw/qdev.h @@ -100,12 +100,19 @@ struct BusClass { int (*reset)(BusState *bus); }; +/** + * BusState: + * @qom_allocated: Indicates whether the object was allocated by QOM. + * @glib_allocated: Indicates whether the object was initialized in-plac= e + * yet is expected to be freed with g_free(). + */ struct BusState { Object obj; DeviceState *parent; const char *name; int allow_hotplug; - int qdev_allocated; + bool qom_allocated; + bool glib_allocated; QTAILQ_HEAD(ChildrenHead, DeviceState) children; QLIST_ENTRY(BusState) sibling; }; diff --git a/hw/sysbus.c b/hw/sysbus.c index 369ee44..2347f51 100644 --- a/hw/sysbus.c +++ b/hw/sysbus.c @@ -274,7 +274,7 @@ static void main_system_bus_create(void) main_system_bus =3D g_malloc0(system_bus_info.instance_size); qbus_create_inplace(main_system_bus, TYPE_SYSTEM_BUS, NULL, "main-system-bus"); - main_system_bus->qdev_allocated =3D 1; + main_system_bus->glib_allocated =3D true; } BusState *sysbus_get_default(void) I admit that qom_allocated vs. glib_allocated is slightly ugly but it is more correct. If memory serves me correctly there was talk of hotplugging PCI host controllers on pseries for passthrough or something, so properly cleaning up seems worth the additional ~four bytes= . Fix-ups welcome - maybe use an enum or, better, fix the PCIBus allocation scheme so that we can drop glib_allocated (SysBus is asserted not to be freed). Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg