From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47060) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UtIjl-0000kt-A7 for qemu-devel@nongnu.org; Sun, 30 Jun 2013 10:36:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UtIjk-0004th-5R for qemu-devel@nongnu.org; Sun, 30 Jun 2013 10:36:21 -0400 Received: from cantor2.suse.de ([195.135.220.15]:51159 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UtIjj-0004tX-Rx for qemu-devel@nongnu.org; Sun, 30 Jun 2013 10:36:20 -0400 Message-ID: <51D0425D.60603@suse.de> Date: Sun, 30 Jun 2013 16:36:13 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <20130624140154.GC13956@otherpad.lan.raisama.net> <20130625022008.GJ18772@G08FNSTD100614.fnst.cn.fujitsu.com> <20130625174555.GA11420@otherpad.lan.raisama.net> In-Reply-To: <20130625174555.GA11420@otherpad.lan.raisama.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 21/26] kvmclock: use realize for kvmclock List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost , Hu Tao , Anthony Liguori Cc: Paolo Bonzini , Peter Crosthwaite , qemu-devel , Igor Mammedov , Anthony Liguori Am 25.06.2013 19:45, schrieb Eduardo Habkost: > On Tue, Jun 25, 2013 at 10:20:08AM +0800, Hu Tao wrote: > [...] >>> Is TYPE_SYS_BUS_DEVICE guaranteed to never override ->realize() itsel= f? >>> >>> From DeviceClass documentation: >>> >>> * If a type derived directly from TYPE_DEVICE implements @realize, i= t does >>> * not need to implement @init and therefore does not need to store a= nd call >>> * #DeviceClass' default @realize callback. >>> * For other types consult the documentation and implementation of th= e >>> * respective parent types. >>> >>> The problem is that there's no documentation about ->realize() on >>> SysBusDeviceClass. Can we please explicitly document SysBusDeviceClas= s >>> expectations about ->realize() first, before making those changes? If someone wants to add a paragraph to sysbus.h:SysBusDeviceClass documentation I would happily ack or pick that up. :) >> IIUC, subclass's overriding ->realize should call parent's ->realize a= t >> some point. Peter Crosthwaite has a patchset to access a object's pare= nt >> class at http://lists.nongnu.org/archive/html/qemu-devel/2013-06/msg02= 982.html >> >> Regarding SysBusDevice::init and SysBusDevice::realize, I think it's t= he >> same as in the case of DeviceClass. If you agree I'll document it as i= n >> DeviceClass. >=20 > I believe it is reasonable to document that SysBusDevice subclasses > don't need to call the parent ->realize() method, like on DeviceClass. > This is exactly what this patch set does, after all, and I haven't seen > anybody complaining about it yet. So the thing is that SysBusDevice's DeviceClass::init implementation, called by DeviceState's DeviceClass::realize implementation, just calls SysBusDeviceClass::init if non-NULL. When we introduce our own device's realizefn to replace our SysBusDeviceClass::init implementation, there is no point calling that effectively no-op DeviceClass::realize implementation. And if we tried to, ... * ... how would we decide whether to call the parent's implementation first or last? It's not yes or no, it's no or when. Changing between either is more than just moving one line, it affects error handling and with knowledge about parent implementation never failing we could so far save us some work. * ... with the current QOM method scheme we'd go insane introducing a FooClass per device to save SysBusDevice's DeviceClass::realize in FooClass::parent_realize. Still waiting for Anthony on whether and how we want to change our scheme. Long story short: If someone wants to mess with base classes they get to check derived classes for compatibility with their change. Ideally qtest would help automate that to some degree. I would be all in favor if someone wanted to add a dummy test case per non-default, non-KVM device converted so that we can see that we didn't screw up -device instantiation too badly. 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