Re: [Qemu-devel] [PATCH v5 00/16] uq/master: Introduce basic irqchip support

[Jan Kiszka]

On 2011-12-20 02:08, Anthony Liguori wrote:
> On 12/19/2011 06:37 PM, Jan Kiszka wrote:
>> On 2011-12-20 01:32, Anthony Liguori wrote:
>>> On 12/19/2011 05:49 PM, Jan Kiszka wrote:
>>>> On 2011-12-19 23:24, Anthony Liguori wrote:
>>>>> On 12/19/2011 03:17 PM, Marcelo Tosatti wrote:
>>>>>>
>>>>>> Anthony,
>>>>>>
>>>>>> Can you please review&    ACK?
>>>>>>
>>>>>> You could even apply directly but well do a kvm-autotest run through
>>>>>> uq/master. Still, your review is needed.
>>>>>
>>>>> Overall, it looks good except for the backend/frontend split.  This
>>>>> should be done in terms of qdev inheritance.
>>>>
>>>> I cannot follow your idea here yet. There is no inheritance as we
>>>> end up
>>>> with only a single class that permutes (selects a different backend) on
>>>> creation. I'm not sure how to model two classes that will still only
>>>> mean a single qdev registration.
>>>
>>> See other reply in thread.
>>>
>>> We should model this as two separate qdev devices.  We can avoid
>>> regressing migration in qemu-kvm by just having a common vmstate name.
>>>
>>> apic is a no-user device so there's no way that changing the name of it
>>> in qemu-kvm can affect users.
>>
>> Look down http://thread.gmane.org/gmane.comp.emulators.kvm.devel/82598
>> for the discussion of that model.
> 
> Let me say that I know this is the last bit of qemu-kvm that needs
> merging and that this has been an epic effort.  I wouldn't refuse to
> merge a pull request that came in with this in its current form.
> 
> If we merged this now, I would be submitting patches in the not too
> distant future to remove all of this backend stuff in favor of proper
> modeling (including using two separate devices).
> 
> There's lot of inconsistency in qdev already today so adding a little
> more isn't the end of the world.  We're going to need to eventually have
> this debate soon so it's up to you whether you want to just get this
> merged now and worry about this another day or resolve this before merge.
> 
> I don't see any compatibility issues here so I'm not really concerned
> about introducing a regression by breaking it into two devices.

I don't want to see yet another attempt merged that requires foreseeable
refactoring later on. The point of this one is to do it in a way that is
providing a sound foundation for all those other features that still
wait in qemu-kvm for refactoring.

The point is that migration support between in-kernel on/off is a
worthwhile feature we should design for. That either means skipping the
backend property on device tree migration (maybe a feature we want in
other use cases as well) or provide an alias naming scheme where you can
address APICs, IOAPICs, i8259, i8254 and all the chips that non-x86 will
bring us without knowing where they are implemented and without worrying
to migrate between those variants. If you have a good model for that in
mind, rolling back to v1, rebasing improvements from v5 over it would
not be a big deal. But everyone in this round should agree on this
first. I don't wanna port back and forth nor refactor all this again
when once it's in.

Jan

signature.asc
Description: OpenPGP digital signature