Re: [Qemu-devel] [PATCH v5 06/16] apic: Introduce backend/frontend infrastructure for KVM reuse

[Anthony Liguori <anthony@codemonkey.ws>]

On 12/19/2011 06:32 PM, Jan Kiszka wrote:
> On 2011-12-20 01:28, Anthony Liguori wrote:
>> On 12/19/2011 05:32 PM, Jan Kiszka wrote:
>>>> struct APICCommonInfo {
>>>>       DeviceInfo qdev;
>>>>       void (*init)(APICState *s);
>>>>       void (*set_base)(APICState *s, uint64_t val);
>>>>       void (*set_tpr)(APICState *s, uint8_t val);
>>>>       void (*external_nmi)(APICState *s);
>>>> };
>>>>
>>>> Take a look at SCSIDevice for an example of this in practice.  This is
>>>> nicer because as we move save/load into devices methods, it becomes
>>>> natural to define the state and save/load function in the base class.
>>>> Provided it only uses base class state, it lets save/load be compatible
>>>> between both in-kernel and in-qemu device model.
>>>
>>> The difference is (unless I completely miss your point) that a common
>>> SCSI base class is used by different derived classes.
>>
>> The 'frontend' is the common code and the 'backend' are the bits that
>> are different, no?
>>
>> We ultimately want there to be two devices that share all of the
>> 'frontend' code by providing different 'backend' implementations.
>>
>> So make the 'frontend' a base class that provides a set of abstract
>> virtual methods (the set you have as the 'backend' interface).  Each
>> device instance then inherits from the base class and provides its own
>> implementation of the virtual methods.
>>
>>> Here we have a
>>> common frontend class but different base classes, so to say. And we have
>>> a mechanism to chose where to inherit from on instantiation. Precisely
>>> this allows to keep the compatibility between in-kernel and user space
>>> model in this series.
>>
>> Okay, so I really think this is the problem.  The in-kernel APIC is a
>> separate device, no a property of the userspace APIC device.
>>
>> It should be modeled as two separate devices.
>
> That was v1 of my patches. Avi didn't like it, I tried it like this, and
> in the end I had to agree. So, no, I don't think we want such a model.

Yes, we do :-)

The in-kernel APIC is a different implementation of the APIC device.  It's not 
an "accelerator" for the userspace APIC.

All that you're doing here is reinventing qdev.  You're defining your own type 
system (APICBackend), creating a new regression system for it, and then defining 
your own factory function for creating it (through a qdev property).

I'm struggling to understand the reason to avoid using the infrastructure we 
already have to do all of this.

Regards,

Anthony Liguori

>
> Jan
>