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

[Anthony Liguori <aliguori@us.ibm.com>]

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<jan.kiszka@siemens.com>
>>
>> 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