Re: [Qemu-devel] [PATCH 21/26] kvmclock: use realize for kvmclock

[Hu Tao]

On Mon, Jun 24, 2013 at 11:01:54AM -0300, Eduardo Habkost wrote:
> On Sat, Jun 22, 2013 at 04:50:33PM +0800, Hu Tao wrote:
> > Cc: address@hidden
> > Cc: "Andreas Färber" <address@hidden>
> > Cc: Paolo Bonzini <address@hidden>
> > Cc: Anthony Liguori <address@hidden>
> > Cc: Igor Mammedov <address@hidden>
> > Cc: Eduardo Habkost <address@hidden>
> > Signed-off-by: Hu Tao <address@hidden>
> > ---
> >  hw/i386/kvm/clock.c | 8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
> > index 87d4d0f..74aa240 100644
> > --- a/hw/i386/kvm/clock.c
> > +++ b/hw/i386/kvm/clock.c
> > @@ -87,12 +87,11 @@ static void kvmclock_vm_state_change(void *opaque, int 
> > running,
> >      }
> >  }
> >  
> > -static int kvmclock_init(SysBusDevice *dev)
> > +static void kvmclock_realize(DeviceState *dev, Error **errp)
> >  {
> > -    KVMClockState *s = FROM_SYSBUS(KVMClockState, dev);
> > +    KVMClockState *s = DO_UPCAST(KVMClockState, busdev.qdev, dev);
> >  
> >      qemu_add_vm_change_state_handler(kvmclock_vm_state_change, s);
> > -    return 0;
> >  }
> >  
> >  static const VMStateDescription kvmclock_vmsd = {
> > @@ -111,9 +110,8 @@ static const VMStateDescription kvmclock_vmsd = {
> >  static void kvmclock_class_init(ObjectClass *klass, void *data)
> >  {
> >      DeviceClass *dc = DEVICE_CLASS(klass);
> > -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> >  
> > -    k->init = kvmclock_init;
> > +    dc->realize = kvmclock_realize;
> 
> Is TYPE_SYS_BUS_DEVICE guaranteed to never override ->realize() itself?
> 
> From DeviceClass documentation:
> 
>  * If a type derived directly from TYPE_DEVICE implements @realize, it does
>  * not need to implement @init and therefore does not need to store and call
>  * #DeviceClass' default @realize callback.
>  * For other types consult the documentation and implementation of the
>  * respective parent types.
> 
> The problem is that there's no documentation about ->realize() on
> SysBusDeviceClass. Can we please explicitly document SysBusDeviceClass
> expectations about ->realize() first, before making those changes?

IIUC, subclass's overriding ->realize should call parent's ->realize at
some point. Peter Crosthwaite has a patchset to access a object's parent
class at http://lists.nongnu.org/archive/html/qemu-devel/2013-06/msg02982.html

Regarding SysBusDevice::init and SysBusDevice::realize, I think it's the
same as in the case of DeviceClass. If you agree I'll document it as in
DeviceClass.