RFC New VMState table based load/save infrastructure
Get series with b4
b4 am [email protected]Versions newest first
- v1 RFC Latest5 patches 16 repliesIn review
Patches in v1 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/5 | loadvm already call vm_start()vl.c | +3 | 0 replies | |
| 2/5 | Don't call vm_start() if there was an error loadingsavevm.c | +1 | 0 replies | |
| 3/5 | Don't ignore load_state() error return values.savevm.c | +14 | 0 replies | |
| 4/5 | New VMstate save/load infrastructurehw/ savevm.c | +427 | 5 replies | |
| 5/5 | Port apic to new VMState designhw/ | +27 | 8 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.
Cover letter [Qemu-devel] [PATCH RFC 0/5] New VMState table based load/save infrastructure
This series implement a new save/load approach for device state. I want to start a discussion on how is better to design it. This is the reason why only one device has been ported to this new approach.
I increased the version_id of apic state, just to sent the 3 arrays (isr, tmr, irr) as arrays. I didn't want to introduce structure support before there is any discussion about this, not that it is imposible to do it in a compatible way.
Problems of current approach:
- have to maintain both load and save functions on sync
- it is not type safe
- you can't use easily the device save/load information to do more things. For instance, print the state of a device in the monitor. To do this, you basically have to copy foo_apic() function.
- You can't examine the saved images in any meaningful way
New approach:
- you only have to declare the state once, in a table, save/load functions are generic for all devices.
- it is type safe
- it is qdev-like design (i.e. you don't have to learn yet a completely different appearch for a similar problem)
- it is trivial to add new operations to VMStateInfo, and then you can use them anywhere (for instance printing in the monitor, etc).
Problems of new approach:
- it is less flexible, you can't progamatically save in any order that you want. It is not clear to me that we _want_ that flexibility. Look for instance and hw/msix.c and think if you can be sure that save/load are on sync, and as soon as we get new versions, that things could be tested to be wright.
What do I want to discuss:
a- do we want to be able to load state form old versions?
I am not sure that we are going to get this right for complex devices, and I don't completely see how we can have any kind of testing here. There was already a discussion about this on the list.
b- Is this the right approach? What more do we want/need?
For instance, implementing struct save support, and calling other "sub-descriptions" is not difficult, we just have to decide if we want it.
c- In the current approach, we have loops to send arrays, I think that one
got already done better on new approarch. But we don't have support for ifs (see hw/ide.c
if (s->identify_set) {
qemu_get_buffer(f, (uint8_t *)s->identify_data, 512);
} Do we want support for things like that?
d- how aggresive should the new design be? i.e. be able to be compatible with
old design is good, or can we start with a clean sheet and just remove the gotchas of the previous design?
Comments?
Later, Juan.
P.D. the 1st two patches just improve loadvm error messages. I included then
here because they make easier to play with the state/versions. Thinking about proper solution that will be sent in a different patch.
Juan Quintela (5):
loadvm already call vm_start()
Don't call vm_start() if there was an error loading Don't ignore load_state() error return values. New VMstate save/load infrastructure
Port apic to new VMState design
hw/apic.c | 96 +++++------------- hw/hw.h | 124 +++++++++++++++++++++++ savevm.c | 322 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- vl.c | 4 +- 4 files changed, 470 insertions(+), 76 deletions(-)