On 11/08/2011 05:04 PM, Anthony Liguori wrote: > > There's no code generation in QOM :-) > > This just comes down to how we do save/restore. We white list things > we care about. We should move to a model where we save/restore > everything (preferably via code generation), and then black > list/transform state before it goes over the wire. > > Mike Roth's migration Visitor series is a first step in this > direction. The reason I bring this up in this context though is that > using that mind set makes the answer about what to do here obvious. > If it's a member of a device's state, it should be save/restored.
Ok. > > MemoryRegion is a member of the device's state, so it should be > save/restored with the device. Not all MemoryRegion fields are state. In some instantiations, none of them are. > >>> That means we should have a VMSTATE_MEMORY_REGION(). >>> >>> VMSTATE_MEMORY_REGION should save off the state of the memory region, >>> and restore it appropriately. VMSTATE_MEMORY_REGION's implementation >>> does not need to live in memory.c. It can certainly live in savevm.c >>> or somewhere else more appropriate. >> >> What state is that? Some devices have fixed size, offset, parent, and >> enable/disable state (is there a word for that?), so there is no state >> that needs to be transferred. For other devices this is all dynamic. > > Any mutable state should be save/restored. Immutable state doesn't > need to be saved as it's created as part of the device model. The memory API doesn't know which fields are mutable and which are not. > > 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. You can have a real region modeled as a set of nested regions, or as one big region (with a more complicated switch () statement in the callback). This shouldn't be reflected in the save/restore ABI. > > 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". > That way, we start with a strong statement of correctness as opposed > to starting from a position of weak correctness. We also start from a position of fragility wrt. implementation details. >> flash_mapped always reflects a bit in a real register. We shouldn't >> duplicate 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. -- error compiling committee.c: too many arguments to function
