On 11/08/2011 05:32 PM, Anthony Liguori wrote: > >>> >>> If the question is, how do we restore the immutable state, that should >>> be happening as part of device creation, no? >>> >>>> The way I see it, we create a link between some device state (a >>>> register) and a memory API field (like the offset). This way, when >>>> one >>>> changes, so does the other. In complicated devices we'll have to >>>> write >>>> a callback. >>> >>> In devices where we dynamically change the offset (it's mutable), we >>> should save the offset and restore it. Since offset is sometimes >>> mutable and sometimes immutable, we should always save/restore it. In >>> the cases where it's really immutable, since the value isn't changing, >>> there's no harm in doing save/restore. >> >> There is, you're taking an implementation detail and making it into an >> ABI. Change the implementation and migration breaks. > > Yes, that's a feature, not a bug. If we send too little state today > in version X, then discover this while working on version X + 1, we > have no recourse. We have to black list version X. > > Discovering this is hard because we have to find a symptom of broken > migration. This is often subtle like, "if you migrate while a floppy > request is in flight, the request is lost resulting in a timeout in > the guest kernel". > > If we send too much state (internal implementation that is derived > from something else) in version X, then discover this while working on > version X + 1, we can filter the incoming state in X + 1 to just > ignore the extra state and derive the correct internal state from the > other stable registers. > > Discovering cases like this is easy because migration fails > directly--not indirectly through a functional regression. That means > this is something we can very easily catch in regression testing. > > I actually think this is the way to do it too. Save/restore > everything by default and then as we develop and discover migration > breaks, add filtering in the new versions to ignore and not send > internal state. I don't think there's a tremendous amount of value is > proactively filtering internal state. A lot of internal state never > changes over a long period of time.
I might agree if a significant fraction of the memory API's state needed to be saved. But that's not the case -- indeed I expect it to be zero. Take this patch for example, the only field that is mutable is the enabled/disabled state, which mirrors some bit in a register. PIIX's PAM, PCI's BARs are the same. I doubt there is *any* case where the memory API is the sole source of this information. The way we do this now is to call device_update_mappings() whenever a register that contains mapping information changes, whether it is in a device_write() callback or in device_post_load(). All that you'd save with automatic memory API state migration is the latter call. > >>> Yes, we could save just the device register, and use a callback to >>> regenerate the offset. But that adds complexity and leads to more >>> save/restore bugs. >>> >>> We shouldn't be reluctant to save/restore derived state. Whether we >>> send it over the wire is a different story. We should start by saving >>> as much state as we need to, and then sit down and start removing >>> state and adding callbacks as we need to. >> >> "saving state without sending it over the wire" is another way of saying >> "not saving state". > > Or filtering it on the receiving end. That's the fundamental difference. I might agree if I thought there is anything worthwhile in the memory API's state. > >>> Why? The only thing that removing it does is create additional >>> complexity for save/restore. You may argue that sending minimal state >>> improves migration compatibility but I think the current state of >>> save/restore is an existence proof that this line of reasoning is >>> incorrect. >> >> It doesn't create additional complexity for save restore, and I don't >> think that the current state of save/restore proves anything except that >> it needs a lot more work. > > It's very hard to do the style of save/restore that we do correctly. If we had a Register class, that would take care of device registers automatically. As to in flight transactions or hidden state, I don't think there are any shortcuts. -- error compiling committee.c: too many arguments to function
