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
Attachment:
signature.asc
Description: OpenPGP digital signature
