From patchwork Thu May 24 16:48:27 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Andreas_F=C3=A4rber?= X-Patchwork-Id: 161163 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id E6196B6F9D for ; Fri, 25 May 2012 02:51:49 +1000 (EST) Received: from localhost ([::1]:59236 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SXbGN-0006jd-AV for incoming@patchwork.ozlabs.org; Thu, 24 May 2012 12:51:47 -0400 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 Received: from relay2.suse.de (unknown [195.135.220.254]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id EDE1B93E50; Thu, 24 May 2012 18:48:31 +0200 (CEST) Message-ID: <4FBE665B.8070900@suse.de> Date: Thu, 24 May 2012 18:48:27 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= Organization: SUSE LINUX Products GmbH User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120421 Thunderbird/12.0 MIME-Version: 1.0 To: Paolo Bonzini , Anthony Liguori 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> X-Enigmail-Version: 1.5pre X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4-2.6 X-Received-From: 195.135.220.15 Cc: peter.maydell@linaro.org, qemu-devel@nongnu.org Subject: Re: [Qemu-devel] [PATCH 18/21] qdev: convert busses to QEMU Object Model X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Am 02.05.2012 13:31, schrieb Paolo Bonzini: > From: Anthony Liguori > > This is far less interesting than it sounds. We simply add an Object to each > BusState and then register the types appropriately. Most of the interesting > refactoring will follow in the next patches. > > Since we're changing fundamental type names (BusInfo -> BusClass), it all needs > to convert at once. Fortunately, not a lot of code is affected. > > 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: 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 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 = g_malloc0(sizeof(*bus)); - bus->qbus.qdev_allocated = 1; + bus->qbus.glib_allocated = 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 = BUS(object_new(typename)); - bus->qdev_allocated = 1; + bus->qom_allocated = 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-place + * 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 = 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 = 1; + main_system_bus->glib_allocated = true; } BusState *sysbus_get_default(void)