Re: [Qemu-devel] [PATCH 1/4] i8254: Factor out base class for KVM reuse

[Anthony Liguori]

On 01/31/2012 03:49 PM, Jan Kiszka wrote:
> On 2012-01-31 22:40, Anthony Liguori wrote:
> > On 01/31/2012 12:46 PM, Jan Kiszka wrote:
> > > Applying the concept used for the *PICs once again: establish a base
> > > class for the i8254 that can be used both by the current user space
> > > emulation and the upcoming KVM in-kernel version. We share most of the
> > > public interface of the i8254, specifically to the pcspk, vmstate, reset
> > > and certain init parts.
> > >
> > > Signed-off-by: Jan Kiszka<address@hidden>
> >
> > Now that we have QOM bits, there's no need to factor out a common base
> > class.
> >
> > Just make the methods that you want to override virtual with the default
> > implementation and then make a KVMPIT that inherits from the PIT and
> > then overrides whatever virtual functions it needs to.
>
> That doesn't sound like the proper design for this purpose.

It's hard to say really.  There's a lot more in the common class that I expected 
(like initialization of the ISA regions.  I would expect the base class to look 
a lot more like an interface such that the KVM PIT implementation was 
trivialized more than it is.

But I can argue it both ways so if you feel strongly here, I won't object.

Regards,

Anthony Liguori

 Rather, we
> have an abstract base class that both implementations are derived from.
> If I'm not using QOM properly to achieve this, please tell me.
>
> Thanks,
> Jan