On 2011-12-20 02:19, Jan Kiszka wrote: > 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. Forgot to state the why: This allows seamless migration from older, non-accelerated setups and to switch between both models in case on faces some issues. That not only applies to the APIC but to all those various in-kernel device models we have and will add in the future. > 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 Jan
Attachment:
signature.asc
Description: OpenPGP digital signature
