New VMState table based load/save infrastructure
cover New VMState table based load/save infrastructure
This this is 3rd version of the series, I changed:
v3:
* Patches are again on top of my loadvm changes, as they use the new loadvm functionality.
* Add size paramente to VMState field, it is needed for buffers
* Redo all the STRUCT and ARRAY support. It makes easier to have combinations of things.
* Once there created:
* POINTER: field is a pointer to the real thing
* VARRAY: This is one array whose size is another field
* BUFFER: A traditional array. So far it is only for static sized buffers. No need for variable sized buffers yet.
* PCIBUS ported to new API
* PCIDevice ported to new API. Here is an example of how to create your own function in the config array. Basically PCI wants to read the array and sanitize it before doing anything else with it.
* Add run_after_load() callback.
* Port PS2 devices
* Port ACPI (uses PCIDevice)
* Put const left, right, and center.
ToDo:
- still not optional test function. Notice that this can be done with a local VMStateInfo struct. I still think that this is not needed.
- Start/end functions: We have run_after_load(), problaby will ned some more uhci_save() calls uhci_async_cancel_all() before saving its state.
- Better/Different design for containers? Just now we can have things like:
an array of structs pointed from a pointer of dynamic size.
This means having to create a new a new VMSTATE_FOO() construct. Any idea about how to improve this? I would like something that is componable:
ARRAY(POINTER(INT32(field, struct), extra), extra2) or somesuch. Haven't found a good way to abuse cpp to get this.
- I tested that it loads qemu/master images. It can save/load new ones.
- Next try will be to convert the virtio-* drivers. Basically everything else should work with current abstractions. virtion_load() is a mess, but I think that I can get it working with before/after loading functions.
- If you know of any device that does strange things with state, please, let me know, to start for there.
- i440fx_load()
Can we put pci_irq_levels[] array in any struct? This is the only driver that I have found that uses a global variable. Gerd, can you take a look? It is not possible to have the PCIDevice struct inside a PIIXDevice or similar?
Comments, suggestions?
v2:
* Add _V() constructors. We almost never need the version field.
* Add const to VMStateDescription uses (BlueWirl suggestion)
* Add const to VMStateInfo uses
* Remove VMStateInfo parameter from get/put callbacks. Not needed.
* Load of old versions is done with old foo_load function (Kraxel suggestion)
* Add struct support
* Move i8254 to new infrastructure, to test struct support
* I removed the autostart patches, updated version sent to list
i8254 note:
There is only one timer in the 1st channel, in the other two channels, the timer is not created ever, this is the reason why I sent the irq_timer not in the irq channels.
Juan Quintela (23):
move useful type definitons to osdep.h
split do_loadvm() into do_loadvm() and load_vmstate() move do_loadvm() to monitor.c
make load_vmstate() return errors
Use return value from load_state() call back Add vmstate_load() and vmstate_save() functions New VMstate save/load infrastructure
Add VMState support for pointers
Add VMState support for arrays
Port apic to new VMState design
Add VMState support for structs
Add VMState support for arrays of structs Port i8254 to new VMState design
Add VMState support for int32_t check value Add VMState support for variable sized arrays Port PCI Bus to VMState design
Add VMState support for static sized buffers (uint_8) Port PS2 devices to VMState design
Add VMState support for int32_t check value Add version_id to PCIDevice.
Port PCIDevice state to VMState
Add VMState support to run a function after load Port ACPI to VMState
hw/acpi.c | 55 +++----- hw/apic.c | 67 +++++----- hw/hw.h | 234 ++++++++++++++++++++++++++++++++ hw/i8254.c | 66 +++++---- hw/pci.c | 107 ++++++++------- hw/pci.h | 2 + hw/ps2.c | 119 +++++++--------- hw/qdev.h | 3 - monitor.c | 10 ++ osdep.h | 3 + savevm.c | 434 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- sysemu.h | 2 +- vl.c | 7 +- 13 files changed, 869 insertions(+), 240 deletions(-)
PATCH 1/23 move useful type definitons to osdep.h
--- hw/qdev.h | 3 --- osdep.h | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-)
hw/qdev.h +0 -3
@@ -163,9 +163,6 @@ extern PropertyInfo qdev_prop_macaddr;extern PropertyInfo qdev_prop_drive;extern PropertyInfo qdev_prop_pci_devfn; -#define typeof_field (type, field) typeof(((type *)0)->field) -#define type_check (t1,t2) ((t1*)0 - (t2*)0) -#define DEFINE_PROP (_name, _state, _field, _prop, _type) { \ .name = (_name), \ .info = &(_prop), \
osdep.h +3 -0
@@ -36,6 +36,9 @@ (type *) ((char *) __mptr - offsetof(type, member));})#endif +#define typeof_field (type, field) typeof(((type *)0)->field) +#define type_check (t1,t2) ((t1*)0 - (t2*)0) +#ifndef MIN #define MIN (a, b) (((a) < (b)) ? (a) : (b))#endif
Signature · 1 line
1.6.2.5
PATCH 2/23 split do_loadvm() into do_loadvm() and load_vmstate()
In reply to Juan Quintelado_loadvm() is now called from the monitor.load_vmstate() is called by do_loadvm() and when -loadvm command line is used. Command line don't have to play games with vmstop()/vmstart()
--- savevm.c | 22 +++++++++++++--------- sysemu.h | 1 + vl.c | 2 +- 3 files changed, 15 insertions(+), 10 deletions(-)
savevm.c +13 -9
@@ -1145,14 +1145,13 @@ void do_savevm(Monitor *mon, const char *name)vm_start (); } -void do_loadvm (Monitor *mon,const char *name) +void load_vmstate (Monitor *mon,const char *name) {DriveInfo *dinfo;BlockDriverState *bs, *bs1;QEMUSnapshotInfo sn;QEMUFile *f;int ret; -int saved_vm_running; bs =get_bs_snapshots ();if (!bs) {
@@ -1163,9 +1162,6 @@ void do_loadvm(Monitor *mon, const char *name)/* Flush all IO requests so they don't interfere with the new state. */ qemu_aio_flush (); - saved_vm_running = vm_running; -vm_stop (0 ); - TAILQ_FOREACH(dinfo, &drives, next) { bs1 = dinfo->bdrv;if (bdrv_has_snapshot (bs1)) {
@@ -1191,7 +1187,7 @@ void do_loadvm(Monitor *mon, const char *name) }/* fatal on snapshot block device */ if (bs == bs1) - goto the_end; +return ; } } }
@@ -1199,20 +1195,28 @@ void do_loadvm(Monitor *mon, const char *name)/* Don't even try to load empty VM states */ ret =bdrv_snapshot_find (bs, &sn, name);if ((ret >=0 ) && (sn.vm_state_size ==0 )) - goto the_end; +return ;/* restore the VM state */ f =qemu_fopen_bdrv (bs,0 );if (!f) {monitor_printf (mon,"Could not open VM state file\n" ); - goto the_end; +return ; } ret =qemu_loadvm_state (f);qemu_fclose (f);if (ret <0 ) {monitor_printf (mon,"Error %d while loading VM state\n" , ret); } - the_end: +} + +void do_loadvm (Monitor *mon,const char *name) +{ +int saved_vm_running = vm_running; + +vm_stop (0 ); + +load_vmstate (mon, name);if (saved_vm_running)vm_start (); }
sysemu.h +1 -0
@@ -52,6 +52,7 @@ void qemu_system_reset(void);void do_savevm (Monitor *mon,const char *name);void do_loadvm (Monitor *mon,const char *name); +void load_vmstate (Monitor *mon,const char *name);void do_delvm (Monitor *mon,const char *name);void do_info_snapshots (Monitor *mon);
vl.c +1 -1
@@ -6037,7 +6037,7 @@ int main(int argc, char **argv, char **envp) }if (loadvm) -do_loadvm (cur_mon, loadvm); +load_vmstate (cur_mon, loadvm);if (incoming) { autostart =0 ;
Signature · 1 line
1.6.2.5
PATCH 3/23 move do_loadvm() to monitor.c
In reply to Juan Quintela--- monitor.c | 11 +++++++++++ savevm.c | 11 ----------- sysemu.h | 1 - 3 files changed, 11 insertions(+), 12 deletions(-)
monitor.c +11 -0
@@ -1776,6 +1776,17 @@ static void do_closefd(Monitor *mon, const char *fdname) fdname); } +static void do_loadvm (Monitor *mon,const char *name) +{ +int saved_vm_running = vm_running; + +vm_stop (0 ); + +load_vmstate (mon, name); +if (saved_vm_running) +vm_start (); +} +int monitor_get_fd (Monitor *mon,const char *fdname) {mon_fd_t *monfd;
savevm.c +0 -11
@@ -1210,17 +1210,6 @@ void load_vmstate(Monitor *mon, const char *name) } } -void do_loadvm (Monitor *mon,const char *name) -{ -int saved_vm_running = vm_running; - -vm_stop (0 ); - -load_vmstate (mon, name); -if (saved_vm_running) -vm_start (); -} -void do_delvm (Monitor *mon,const char *name) {DriveInfo *dinfo;
sysemu.h +0 -1
@@ -51,7 +51,6 @@ extern qemu_irq qemu_system_powerdown;void qemu_system_reset (void );void do_savevm (Monitor *mon,const char *name); -void do_loadvm (Monitor *mon,const char *name);void load_vmstate (Monitor *mon,const char *name);void do_delvm (Monitor *mon,const char *name);void do_info_snapshots (Monitor *mon);
Signature · 1 line
1.6.2.5
PATCH 4/23 make load_vmstate() return errors
In reply to Juan Quintela--- monitor.c | 3 +-- savevm.c | 12 +++++++----- sysemu.h | 2 +- vl.c | 7 +++++-- 4 files changed, 14 insertions(+), 10 deletions(-)
monitor.c +1 -2
@@ -1782,8 +1782,7 @@ static void do_loadvm(Monitor *mon, const char *name)vm_stop (0 ); -load_vmstate (mon, name); -if (saved_vm_running) +if (load_vmstate (mon, name) >=0 && saved_vm_running)vm_start (); }
savevm.c +7 -5
@@ -1145,7 +1145,7 @@ void do_savevm(Monitor *mon, const char *name)vm_start (); } -void load_vmstate (Monitor *mon,const char *name) +int load_vmstate (Monitor *mon,const char *name) {DriveInfo *dinfo;BlockDriverState *bs, *bs1;
@@ -1156,7 +1156,7 @@ void load_vmstate(Monitor *mon, const char *name) bs =get_bs_snapshots ();if (!bs) {monitor_printf (mon,"No block device supports snapshots\n" ); -return ; +return -EINVAL ; }/* Flush all IO requests so they don't interfere with the new state. */
@@ -1187,7 +1187,7 @@ void load_vmstate(Monitor *mon, const char *name) }/* fatal on snapshot block device */ if (bs == bs1) -return ; +return 0 ; } } }
@@ -1195,19 +1195,21 @@ void load_vmstate(Monitor *mon, const char *name)/* Don't even try to load empty VM states */ ret =bdrv_snapshot_find (bs, &sn, name);if ((ret >=0 ) && (sn.vm_state_size ==0 )) -return ; +return -EINVAL ;/* restore the VM state */ f =qemu_fopen_bdrv (bs,0 );if (!f) {monitor_printf (mon,"Could not open VM state file\n" ); -return ; +return -EINVAL ; } ret =qemu_loadvm_state (f);qemu_fclose (f);if (ret <0 ) {monitor_printf (mon,"Error %d while loading VM state\n" , ret); +return ret; } +return 0 ; }void do_delvm (Monitor *mon,const char *name)
sysemu.h +1 -1
@@ -51,7 +51,7 @@ extern qemu_irq qemu_system_powerdown;void qemu_system_reset (void );void do_savevm (Monitor *mon,const char *name); -void load_vmstate (Monitor *mon,const char *name); +int load_vmstate (Monitor *mon,const char *name);void do_delvm (Monitor *mon,const char *name);void do_info_snapshots (Monitor *mon);
vl.c +5 -2
@@ -6036,8 +6036,11 @@ int main(int argc, char **argv, char **envp)exit (1 ); } -if (loadvm) -load_vmstate (cur_mon, loadvm); +if (loadvm) { +if (load_vmstate (cur_mon, loadvm) <0 ) { + autostart =0 ; + } + }if (incoming) { autostart =0 ;
Signature · 1 line
1.6.2.5
PATCH 5/23 Use return value from load_state() call back
In reply to Juan Quintela--- savevm.c | 14 ++++++++++++-- 1 files changed, 12 insertions(+), 2 deletions(-)
savevm.c +12 -2
@@ -955,7 +955,12 @@ int qemu_loadvm_state(QEMUFile *f) le->next = first_le; first_le = le; - le->se->load_state (f, le->se->opaque, le->version_id); + ret = le->se->load_state (f, le->se->opaque, le->version_id); +if (ret <0 ) { +fprintf (stderr,"qemu: warning: error while loading state for instance 0x%x of device '%s'\n" , + instance_id, idstr); + goto out; + }break ;case QEMU_VM_SECTION_PART :case QEMU_VM_SECTION_END :
@@ -968,7 +973,12 @@ int qemu_loadvm_state(QEMUFile *f) goto out; } - le->se->load_state (f, le->se->opaque, le->version_id); + ret = le->se->load_state (f, le->se->opaque, le->version_id); +if (ret <0 ) { +fprintf (stderr,"qemu: warning: error while loading state section id %d\n" , + section_id); + goto out; + }break ;default :fprintf (stderr,"Unknown savevm section type %d\n" , section_type);
Signature · 1 line
1.6.2.5
PATCH 6/23 Add vmstate_load() and vmstate_save() functions
In reply to Juan Quintela--- savevm.c | 19 +++++++++++++++---- 1 files changed, 15 insertions(+), 4 deletions(-)
savevm.c +15 -4
@@ -690,6 +690,17 @@ void unregister_savevm(const char *idstr, void *opaque) } } +static int vmstate_load (QEMUFile *f,SaveStateEntry *se,int version_id) +{ +return se->load_state (f, se->opaque, version_id); +} + +static void vmstate_save (QEMUFile *f,SaveStateEntry *se) +{ + se->save_state (f, se->opaque); +} + +#define QEMU_VM_FILE_MAGIC 0x5145564d#define QEMU_VM_FILE_VERSION_COMPAT 0x00000002#define QEMU_VM_FILE_VERSION 0x00000003
@@ -792,7 +803,7 @@ int qemu_savevm_state_complete(QEMUFile *f)qemu_put_be32 (f, se->instance_id);qemu_put_be32 (f, se->version_id); - se->save_state (f, se->opaque); +vmstate_save (f, se); }qemu_put_byte (f,QEMU_VM_EOF );
@@ -878,7 +889,7 @@ static int qemu_loadvm_state_v2(QEMUFile *f)fprintf (stderr,"qemu: warning: instance 0x%x of device '%s' not present in current VM\n" , instance_id, idstr); }else { - ret = se->load_state (f, se->opaque, version_id); + ret =vmstate_load (f, se, version_id);if (ret <0 ) {fprintf (stderr,"qemu: warning: error while loading state for instance 0x%x of device '%s'\n" , instance_id, idstr);
@@ -955,7 +966,7 @@ int qemu_loadvm_state(QEMUFile *f) le->next = first_le; first_le = le; - ret = le->se->load_state (f, le->se->opaque, le->version_id); + ret =vmstate_load (f, le->se, le->version_id);if (ret <0 ) {fprintf (stderr,"qemu: warning: error while loading state for instance 0x%x of device '%s'\n" , instance_id, idstr);
@@ -973,7 +984,7 @@ int qemu_loadvm_state(QEMUFile *f) goto out; } - ret = le->se->load_state (f, le->se->opaque, le->version_id); + ret =vmstate_load (f, le->se, le->version_id);if (ret <0 ) {fprintf (stderr,"qemu: warning: error while loading state section id %d\n" , section_id);
Signature · 1 line
1.6.2.5
PATCH 7/23 New VMstate save/load infrastructure
In reply to Juan QuintelaThis patch introduces VMState infrastructure, to convert the save/load functions of devices to a table approach. This new approach has the following advantages:
- it is type-safe
- you can't have load/save functions out of sync
- will allows us to have new interesting commands, like dump <device>, that shows all its internal state.
- Just now, the only added type is arrays, but we can add structures.
- Uses old load_state() function for loading old state.
--- hw/hw.h | 102 +++++++++++++++++++++++ savevm.c | 273 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 370 insertions(+), 5 deletions(-)
hw/hw.h +102 -0
@@ -269,4 +269,106 @@ typedef int QEMUBootSetHandler(void *opaque, const char *boot_devices);void qemu_register_boot_set (QEMUBootSetHandler *func,void *opaque);int qemu_boot_set (const char *boot_devices); +typedef struct VMStateInfo VMStateInfo ; +typedef struct VMStateDescription VMStateDescription ; + +struct VMStateInfo { +const char *name; +int (*get)(QEMUFile *f,void *pv,size_t size); +void (*put)(QEMUFile *f,const void *pv,size_t size); +}; + +enum VMStateFlags { +VMS_SINGLE =0x001 , +}; + +typedef struct { +const char *name; +size_t offset; +size_t size; +const VMStateInfo *info; +enum VMStateFlags flags; +int version_id; +}VMStateField ; + +struct VMStateDescription { +const char *name; +int version_id; +int minimum_version_id; +int minimum_version_id_old; +LoadStateHandler *load_state_old; +VMStateField *fields; +}; + +extern const VMStateInfo vmstate_info_int8; +extern const VMStateInfo vmstate_info_int16; +extern const VMStateInfo vmstate_info_int32; +extern const VMStateInfo vmstate_info_int64; + +extern const VMStateInfo vmstate_info_uint8; +extern const VMStateInfo vmstate_info_uint16; +extern const VMStateInfo vmstate_info_uint32; +extern const VMStateInfo vmstate_info_uint64; + +#define VMSTATE_SINGLE (_field, _state, _version, _info, _type) { \ + .name = (stringify(_field)), \ + .version_id = (_version), \ + .size = sizeof(_type), \ + .info = &(_info), \ + .flags = VMS_SINGLE, \ + .offset = offsetof(_state, _field) \ + + type_check(_type,typeof_field(_state, _field)) \ +} + +/* _f : field name +_s : struct state name +_v : version +*/ + +#define VMSTATE_INT8_V (_f, _s, _v) \ + VMSTATE_SINGLE(_f, _s, _v, vmstate_info_int8, int8_t) +#define VMSTATE_INT16_V (_f, _s, _v) \ + VMSTATE_SINGLE(_f, _s, _v, vmstate_info_int16, int16_t) +#define VMSTATE_INT32_V (_f, _s, _v) \ + VMSTATE_SINGLE(_f, _s, _v, vmstate_info_int32, int32_t) +#define VMSTATE_INT64_V (_f, _s, _v) \ + VMSTATE_SINGLE(_f, _s, _v, vmstate_info_int64, int64_t) + +#define VMSTATE_UINT8_V (_f, _s, _v) \ + VMSTATE_SINGLE(_f, _s, _v, vmstate_info_uint8, uint8_t) +#define VMSTATE_UINT16_V (_f, _s, _v) \ + VMSTATE_SINGLE(_f, _s, _v, vmstate_info_uint16, uint16_t) +#define VMSTATE_UINT32_V (_f, _s, _v) \ + VMSTATE_SINGLE(_f, _s, _v, vmstate_info_uint32, uint32_t) +#define VMSTATE_UINT64_V (_f, _s, _v) \ + VMSTATE_SINGLE(_f, _s, _v, vmstate_info_uint64, uint64_t) + +#define VMSTATE_INT8 (_f, _s) \ + VMSTATE_INT8_V(_f, _s, 0) +#define VMSTATE_INT16 (_f, _s) \ + VMSTATE_INT16_V(_f, _s, 0) +#define VMSTATE_INT32 (_f, _s) \ + VMSTATE_INT32_V(_f, _s, 0) +#define VMSTATE_INT64 (_f, _s) \ + VMSTATE_INT64_V(_f, _s, 0) + +#define VMSTATE_UINT8 (_f, _s) \ + VMSTATE_UINT8_V(_f, _s, 0) +#define VMSTATE_UINT16 (_f, _s) \ + VMSTATE_UINT16_V(_f, _s, 0) +#define VMSTATE_UINT32 (_f, _s) \ + VMSTATE_UINT32_V(_f, _s, 0) +#define VMSTATE_UINT64 (_f, _s) \ + VMSTATE_UINT64_V(_f, _s, 0) + +#define VMSTATE_END_OF_LIST () \ + {} + +extern int vmstate_load_state (QEMUFile *f,const VMStateDescription *vmsd, +void *opaque,int version_id); +extern void vmstate_save_state (QEMUFile *f,const VMStateDescription *vmsd, +const void *opaque); +extern int vmstate_register (int instance_id,const VMStateDescription *vmsd, +void *base); +extern void vmstate_unregister (const char *idstr,void *opaque);#endif
savevm.c +268 -5
322 lines not on this page · raw message
Signature · 1 line
1.6.2.5
PATCH 8/23 Add VMState support for pointers
In reply to Juan QuintelaThis patch adds support for saving pointers to values
--- hw/hw.h | 19 +++++++++++++++++++ savevm.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 0 deletions(-)
hw/hw.h +19 -0
@@ -280,6 +280,7 @@ struct VMStateInfo {enum VMStateFlags {VMS_SINGLE =0x001 , +VMS_POINTER =0x002 , }; typedef struct {
@@ -310,6 +311,8 @@ extern const VMStateInfo vmstate_info_uint16;extern const VMStateInfo vmstate_info_uint32;extern const VMStateInfo vmstate_info_uint64; +extern const VMStateInfo vmstate_info_timer; +#define VMSTATE_SINGLE (_field, _state, _version, _info, _type) { \ .name = (stringify(_field)), \ .version_id = (_version), \
@@ -320,6 +323,16 @@ extern const VMStateInfo vmstate_info_uint64; + type_check(_type,typeof_field(_state, _field)) \ } +#define VMSTATE_POINTER (_field, _state, _version, _info, _type) { \ + .name = (stringify(_field)), \ + .version_id = (_version), \ + .info = &(_info), \ + .size = sizeof(_type), \ + .flags = VMS_SINGLE|VMS_POINTER, \ + .offset = offsetof(_state, _field) \ + + type_check(_type,typeof_field(_state, _field)) \ +} +/* _f : field name _s : struct state name _v : version
@@ -361,6 +374,12 @@ extern const VMStateInfo vmstate_info_uint64;#define VMSTATE_UINT64 (_f, _s) \ VMSTATE_UINT64_V(_f, _s, 0) +#define VMSTATE_TIMER_V (_f, _s, _v) \ + VMSTATE_POINTER(_f, _s, _v, vmstate_info_timer, QEMUTimer *) + +#define VMSTATE_TIMER (_f, _s) \ + VMSTATE_TIMER_V(_f, _s, 0) +#define VMSTATE_END_OF_LIST () \ {}
savevm.c +28 -0
@@ -778,6 +778,27 @@ const VMStateInfo vmstate_info_uint64 = { .put = put_uint64, }; +/* timers */ + +static int get_timer (QEMUFile *f,void *pv,size_t size) +{ +QEMUTimer *v = pv; +qemu_get_timer (f, v); +return 0 ; +} + +static void put_timer (QEMUFile *f,const void *pv,size_t size) +{ +QEMUTimer *v = (void *)pv; +qemu_put_timer (f, v); +} + +const VMStateInfo vmstate_info_timer = { + .name ="timer" , + .get = get_timer, + .put = put_timer, +}; + typedef struct SaveStateEntry { char idstr[256]; int instance_id;
@@ -925,6 +946,9 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,void *addr = opaque + field->offset;int ret; +if (field->flags &VMS_POINTER ) { + addr = *(void **)addr; + } ret = field->info->get (f, addr, field->size);if (ret <0 ) {return ret;
@@ -942,6 +966,10 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,while (field->name) {const void *addr = opaque + field->offset; + +if (field->flags &VMS_POINTER ) { + addr = *(void **)addr; + } field->info->put (f, addr, field->size); field++; }
Signature · 1 line
1.6.2.5
PATCH 9/23 Add VMState support for arrays
In reply to Juan QuintelaThis patch adds support for saving arrays inside the struct
--- hw/hw.h | 28 ++++++++++++++++++++++++++++ savevm.c | 31 ++++++++++++++++++++++--------- 2 files changed, 50 insertions(+), 9 deletions(-)
hw/hw.h +28 -0
@@ -281,12 +281,14 @@ struct VMStateInfo {enum VMStateFlags {VMS_SINGLE =0x001 ,VMS_POINTER =0x002 , +VMS_ARRAY =0x004 , }; typedef struct { const char *name; size_t offset; size_t size; + int num; const VMStateInfo *info; enum VMStateFlags flags; int version_id;
@@ -313,6 +315,8 @@ extern const VMStateInfo vmstate_info_uint64;extern const VMStateInfo vmstate_info_timer; +#define type_check_array (t1,t2,n) ((t1(*)[n])0 - (t2*)0) +#define VMSTATE_SINGLE (_field, _state, _version, _info, _type) { \ .name = (stringify(_field)), \ .version_id = (_version), \
@@ -333,7 +337,19 @@ extern const VMStateInfo vmstate_info_timer; + type_check(_type,typeof_field(_state, _field)) \ } +#define VMSTATE_ARRAY (_field, _state, _num, _version, _info, _type) {\ + .name = (stringify(_field)), \ + .version_id = (_version), \ + .num = (_num), \ + .info = &(_info), \ + .size = sizeof(_type), \ + .flags = VMS_ARRAY, \ + .offset = offsetof(_state, _field) \ + + type_check_array(_type,typeof_field(_state, _field),_num) \ +} +/* _f : field name +_n : num of elements _s : struct state name _v : version */
@@ -380,6 +396,18 @@ extern const VMStateInfo vmstate_info_timer;#define VMSTATE_TIMER (_f, _s) \ VMSTATE_TIMER_V(_f, _s, 0) +#define VMSTATE_UINT32_ARRAY_V (_f, _s, _n, _v) \ + VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_uint32, uint32_t) + +#define VMSTATE_UINT32_ARRAY (_f, _s, _n) \ + VMSTATE_UINT32_ARRAY_V(_f, _s, _n, 0) + +#define VMSTATE_INT32_ARRAY_V (_f, _s, _n, _v) \ + VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_int32, int32_t) + +#define VMSTATE_INT32_ARRAY (_f, _s, _n) \ + VMSTATE_INT32_ARRAY_V(_f, _s, _n, 0) +#define VMSTATE_END_OF_LIST () \ {}
savevm.c +22 -9
@@ -943,15 +943,21 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd, }while (field->name) {if (field->version_id <= version_id) { -void *addr = opaque + field->offset; -int ret; +void *base_addr = opaque + field->offset; +int ret, i, n_elems =1 ; +if (field->flags &VMS_ARRAY ) { + n_elems = field->num; + }if (field->flags &VMS_POINTER ) { - addr = *(void **)addr; + base_addr = *(void **)base_addr; } - ret = field->info->get (f, addr, field->size); -if (ret <0 ) { -return ret; +for (i =0 ; i < n_elems; i++) { +void *addr = base_addr + field->size * i; + ret = field->info->get (f, addr, field->size); +if (ret <0 ) { +return ret; + } } } field++;
@@ -965,12 +971,19 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,VMStateField *field = vmsd->fields;while (field->name) { -const void *addr = opaque + field->offset; +const void *base_addr = opaque + field->offset; +int i, n_elems =1 ; +if (field->flags &VMS_ARRAY ) { + n_elems = field->num; + }if (field->flags &VMS_POINTER ) { - addr = *(void **)addr; + base_addr = *(void **)base_addr; + } +for (i =0 ; i < n_elems; i++) { +const void *addr = base_addr + field->size * i; + field->info->put (f, addr, field->size); } - field->info->put (f, addr, field->size); field++; } }
Signature · 1 line
1.6.2.5
PATCH 11/23 Add VMState support for structs
In reply to Juan QuintelaThis patch adds support for saving one VMStateDescription from other VMStateDescription.
--- hw/hw.h | 12 ++++++++++++ savevm.c | 15 +++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-)
hw/hw.h +12 -0
@@ -282,6 +282,7 @@ enum VMStateFlags {VMS_SINGLE =0x001 ,VMS_POINTER =0x002 ,VMS_ARRAY =0x004 , + VMS_STRUCT = 0x008, }; typedef struct {
@@ -291,6 +292,7 @@ typedef struct {int num;const VMStateInfo *info;enum VMStateFlags flags; +const VMStateDescription *vmsd;int version_id; } VMStateField;
@@ -348,6 +350,16 @@ extern const VMStateInfo vmstate_info_timer; + type_check_array(_type,typeof_field(_state, _field),_num) \ } +#define VMSTATE_STRUCT (_field, _state, _version, _vmsd, _type) { \ + .name = (stringify(_field)), \ + .version_id = (_version), \ + .vmsd = &(_vmsd), \ + .size = sizeof(_type), \ + .flags = VMS_STRUCT, \ + .offset = offsetof(_state, _field) \ + + type_check(_type,typeof_field(_state, _field)) \ +} +/* _f : field name _n : num of elements _s : struct state name
savevm.c +13 -2
@@ -954,7 +954,13 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd, }for (i =0 ; i < n_elems; i++) {void *addr = base_addr + field->size * i; - ret = field->info->get (f, addr, field->size); + +if (field->flags &VMS_STRUCT ) { + ret =vmstate_load_state (f, field->vmsd, addr, version_id); + }else { + ret = field->info->get (f, addr, field->size); + + }if (ret <0 ) {return ret; }
@@ -982,7 +988,12 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd, }for (i =0 ; i < n_elems; i++) {const void *addr = base_addr + field->size * i; - field->info->put (f, addr, field->size); + +if (field->flags &VMS_STRUCT ) { +vmstate_save_state (f, field->vmsd, addr); + }else { + field->info->put (f, addr, field->size); + } } field++; }
Signature · 1 line
1.6.2.5
PATCH 10/23 Port apic to new VMState design
In reply to Juan Quintela--- hw/apic.c | 67 ++++++++++++++++++++++++++++++------------------------------- 1 files changed, 33 insertions(+), 34 deletions(-)
hw/apic.c +33 -34
@@ -864,39 +864,8 @@ static void apic_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val) } } -static void apic_save (QEMUFile *f,void *opaque) -{ -APICState *s = opaque; -int i; - -qemu_put_be32s (f, &s->apicbase); -qemu_put_8s (f, &s->id); -qemu_put_8s (f, &s->arb_id); -qemu_put_8s (f, &s->tpr); -qemu_put_be32s (f, &s->spurious_vec); -qemu_put_8s (f, &s->log_dest); -qemu_put_8s (f, &s->dest_mode); -for (i =0 ; i <8 ; i++) { -qemu_put_be32s (f, &s->isr[i]); -qemu_put_be32s (f, &s->tmr[i]); -qemu_put_be32s (f, &s->irr[i]); - } -for (i =0 ; i <APIC_LVT_NB ; i++) { -qemu_put_be32s (f, &s->lvt[i]); - } -qemu_put_be32s (f, &s->esr); -qemu_put_be32s (f, &s->icr[0 ]); -qemu_put_be32s (f, &s->icr[1 ]); -qemu_put_be32s (f, &s->divide_conf); -qemu_put_be32 (f, s->count_shift); -qemu_put_be32s (f, &s->initial_count); -qemu_put_be64 (f, s->initial_count_load_time); -qemu_put_be64 (f, s->next_time); - -qemu_put_timer (f, s->timer); -} - -static int apic_load (QEMUFile *f,void *opaque,int version_id) +/* This function is only used for old state version 1 and 2 */ +static int apic_load_old (QEMUFile *f,void *opaque,int version_id) {APICState *s = opaque;int i;
@@ -934,6 +903,36 @@ static int apic_load(QEMUFile *f, void *opaque, int version_id)return 0 ; } +static const VMStateDescription vmstate_apic = { + .name ="apic" , + .version_id =3 , + .minimum_version_id =3 , + .minimum_version_id_old =1 , + .load_state_old = apic_load_old, + .fields = (VMStateField []) { +VMSTATE_UINT32 (apicbase, APICState), +VMSTATE_UINT8 (id, APICState), +VMSTATE_UINT8 (arb_id, APICState), +VMSTATE_UINT8 (tpr, APICState), +VMSTATE_UINT32 (spurious_vec, APICState), +VMSTATE_UINT8 (log_dest, APICState), +VMSTATE_UINT8 (dest_mode, APICState), +VMSTATE_UINT32_ARRAY (isr, APICState,8 ), +VMSTATE_UINT32_ARRAY (tmr, APICState,8 ), +VMSTATE_UINT32_ARRAY (irr, APICState,8 ), +VMSTATE_UINT32_ARRAY (lvt, APICState,APIC_LVT_NB ), +VMSTATE_UINT32 (esr, APICState), +VMSTATE_UINT32_ARRAY (icr, APICState,2 ), +VMSTATE_UINT32 (divide_conf, APICState), +VMSTATE_INT32 (count_shift, APICState), +VMSTATE_UINT32 (initial_count, APICState), +VMSTATE_INT64 (initial_count_load_time, APICState), +VMSTATE_INT64 (next_time, APICState), +VMSTATE_TIMER (timer, APICState), +VMSTATE_END_OF_LIST () + } +}; +static void apic_reset (void *opaque) {APICState *s = opaque;
@@ -996,7 +995,7 @@ int apic_init(CPUState *env) } s->timer =qemu_new_timer (vm_clock, apic_timer, s); -register_savevm ("apic" , s->idx,2 , apic_save, apic_load, s); +vmstate_register (s->idx, &vmstate_apic, s);qemu_register_reset (apic_reset, s); local_apics[s->idx] = s;
Signature · 1 line
1.6.2.5
PATCH 12/23 Add VMState support for arrays of structs
In reply to Juan QuintelaThis patch add supports for arrays of structs
--- hw/hw.h | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-)
hw/hw.h +11 -0
@@ -360,6 +360,17 @@ extern const VMStateInfo vmstate_info_timer; + type_check(_type,typeof_field(_state, _field)) \ } +#define VMSTATE_STRUCT_ARRAY (_field, _state, _num, _version, _vmsd, _type) { \ + .name = (stringify(_field)), \ + .num = (_num), \ + .version_id = (_version), \ + .vmsd = &(_vmsd), \ + .size = sizeof(_type), \ + .flags = VMS_STRUCT|VMS_ARRAY, \ + .offset = offsetof(_state, _field) \ + + type_check_array(_type,typeof_field(_state, _field),_num) \ +} +/* _f : field name _n : num of elements _s : struct state name
Signature · 1 line
1.6.2.5
PATCH 13/23 Port i8254 to new VMState design
In reply to Juan Quintela--- hw/i8254.c | 66 +++++++++++++++++++++++++++++++++-------------------------- 1 files changed, 37 insertions(+), 29 deletions(-)
hw/i8254.c +37 -29
@@ -389,35 +389,31 @@ static void pit_irq_timer(void *opaque)pit_irq_timer_update (s, s->next_transition_time); } -static void pit_save (QEMUFile *f,void *opaque) -{ -PITState *pit = opaque; -PITChannelState *s; -int i; - -for (i =0 ; i <3 ; i++) { - s = &pit->channels[i]; -qemu_put_be32 (f, s->count); -qemu_put_be16s (f, &s->latched_count); -qemu_put_8s (f, &s->count_latched); -qemu_put_8s (f, &s->status_latched); -qemu_put_8s (f, &s->status); -qemu_put_8s (f, &s->read_state); -qemu_put_8s (f, &s->write_state); -qemu_put_8s (f, &s->write_latch); -qemu_put_8s (f, &s->rw_mode); -qemu_put_8s (f, &s->mode); -qemu_put_8s (f, &s->bcd); -qemu_put_8s (f, &s->gate); -qemu_put_be64 (f, s->count_load_time); -if (s->irq_timer) { -qemu_put_be64 (f, s->next_transition_time); -qemu_put_timer (f, s->irq_timer); - } +static const VMStateDescription vmstate_pit_channel = { + .name ="pit channel" , + .version_id =2 , + .minimum_version_id =2 , + .minimum_version_id_old =2 , + .fields = (VMStateField []) { +VMSTATE_INT32 (count, PITChannelState), +VMSTATE_UINT16 (latched_count, PITChannelState), +VMSTATE_UINT8 (count_latched, PITChannelState), +VMSTATE_UINT8 (status_latched, PITChannelState), +VMSTATE_UINT8 (status, PITChannelState), +VMSTATE_UINT8 (read_state, PITChannelState), +VMSTATE_UINT8 (write_state, PITChannelState), +VMSTATE_UINT8 (write_latch, PITChannelState), +VMSTATE_UINT8 (rw_mode, PITChannelState), +VMSTATE_UINT8 (mode, PITChannelState), +VMSTATE_UINT8 (bcd, PITChannelState), +VMSTATE_UINT8 (gate, PITChannelState), +VMSTATE_INT64 (count_load_time, PITChannelState), +VMSTATE_INT64 (next_transition_time, PITChannelState), +VMSTATE_END_OF_LIST () } -} +}; -static int pit_load (QEMUFile *f,void *opaque,int version_id) +static int pit_load_old (QEMUFile *f,void *opaque,int version_id) {PITState *pit = opaque;PITChannelState *s;
@@ -449,6 +445,19 @@ static int pit_load(QEMUFile *f, void *opaque, int version_id)return 0 ; } +static const VMStateDescription vmstate_pit = { + .name ="i8254" , + .version_id =2 , + .minimum_version_id =2 , + .minimum_version_id_old =1 , + .load_state_old = pit_load_old, + .fields = (VMStateField []) { +VMSTATE_STRUCT_ARRAY (channels, PITState,3 ,2 , vmstate_pit_channel, PITChannelState), +VMSTATE_TIMER (channels[0 ].irq_timer, PITState), +VMSTATE_END_OF_LIST () + } +}; +static void pit_reset (void *opaque) {PITState *pit = opaque;
@@ -495,8 +504,7 @@ PITState *pit_init(int base, qemu_irq irq) s->irq_timer =qemu_new_timer (vm_clock, pit_irq_timer, s); s->irq = irq; -register_savevm ("i8254" , base,1 , pit_save, pit_load, pit); - +vmstate_register (base, &vmstate_pit, pit);qemu_register_reset (pit_reset, pit);register_ioport_write (base,4 ,1 , pit_ioport_write, pit);register_ioport_read (base,3 ,1 , pit_ioport_read, pit);
Signature · 1 line
1.6.2.5
PATCH 15/23 Add VMState support for variable sized arrays
In reply to Juan QuintelaThis patch add supports for variable sized arrays whose size is another field of the state.
--- hw/hw.h | 22 ++++++++++++++++++++++ savevm.c | 4 ++++ 2 files changed, 26 insertions(+), 0 deletions(-)
hw/hw.h +22 -0
@@ -283,6 +283,7 @@ enum VMStateFlags {VMS_POINTER =0x002 ,VMS_ARRAY =0x004 ,VMS_STRUCT =0x008 , + VMS_VARRAY = 0x010, /* Array with size in another field */ }; typedef struct {
@@ -290,6 +291,7 @@ typedef struct {size_t offset;size_t size;int num; +size_t num_offset;const VMStateInfo *info;enum VMStateFlags flags;const VMStateDescription *vmsd;
@@ -320,6 +322,7 @@ extern const VMStateInfo vmstate_info_uint64;extern const VMStateInfo vmstate_info_timer;#define type_check_array (t1,t2,n) ((t1(*)[n])0 - (t2*)0) +#define type_check_pointer (t1,t2) ((t1**)0 - (t2*)0)#define VMSTATE_SINGLE (_field, _state, _version, _info, _type) { \ .name = (stringify(_field)), \
@@ -352,6 +355,18 @@ extern const VMStateInfo vmstate_info_timer; + type_check_array(_type,typeof_field(_state, _field),_num) \ } +#define VMSTATE_VARRAY (_field, _state, _field_num, _version, _info, _type) {\ + .name = (stringify(_field)), \ + .version_id = (_version), \ + .num_offset = offsetof(_state, _field_num) \ + + type_check(int32_t,typeof_field(_state, _field_num)), \ + .info = &(_info), \ + .size = sizeof(_type), \ + .flags = VMS_VARRAY|VMS_POINTER, \ + .offset = offsetof(_state, _field) \ + + type_check_pointer(_type,typeof_field(_state, _field)) \ +} +#define VMSTATE_STRUCT (_field, _state, _version, _vmsd, _type) { \ .name = (stringify(_field)), \ .version_id = (_version), \
@@ -374,6 +389,7 @@ extern const VMStateInfo vmstate_info_timer; }/* _f : field name +_f_n : num of elements field_name _n : num of elements _s : struct state name _v : version
@@ -436,6 +452,12 @@ extern const VMStateInfo vmstate_info_timer;#define VMSTATE_INT32_ARRAY (_f, _s, _n) \ VMSTATE_INT32_ARRAY_V(_f, _s, _n, 0) +#define VMSTATE_INT32_VARRAY_V (_f, _s, _f_n, _v) \ + VMSTATE_VARRAY(_f, _s, _f_n, _v, vmstate_info_int32, int32_t) + +#define VMSTATE_INT32_VARRAY (_f, _s, _f_n) \ + VMSTATE_INT32_VARRAY_V(_f, _s, _f_n, 0) +#define VMSTATE_END_OF_LIST () \ {}
savevm.c +4 -0
@@ -968,6 +968,8 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,if (field->flags &VMS_ARRAY ) { n_elems = field->num; + }else if (field->flags &VMS_VARRAY ) { + n_elems = *(size_t *)(opaque+field->num_offset); }if (field->flags &VMS_POINTER ) { base_addr = *(void **)base_addr;
@@ -1002,6 +1004,8 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,if (field->flags &VMS_ARRAY ) { n_elems = field->num; + }else if (field->flags &VMS_VARRAY ) { + n_elems = *(size_t *)(opaque+field->num_offset); }if (field->flags &VMS_POINTER ) { base_addr = *(void **)base_addr;
Signature · 1 line
1.6.2.5
PATCH 14/23 Add VMState support for int32_t check value
In reply to Juan QuintelaWe read the saved value and check that it is the same that the one is stored in the structure.
--- hw/hw.h | 5 +++++ savevm.c | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 0 deletions(-)
hw/hw.h +5 -0
@@ -310,6 +310,8 @@ extern const VMStateInfo vmstate_info_int16;extern const VMStateInfo vmstate_info_int32;extern const VMStateInfo vmstate_info_int64; +extern const VMStateInfo vmstate_info_int32_equal; +extern const VMStateInfo vmstate_info_uint8;extern const VMStateInfo vmstate_info_uint16;extern const VMStateInfo vmstate_info_uint32;
@@ -413,6 +415,9 @@ extern const VMStateInfo vmstate_info_timer;#define VMSTATE_UINT64 (_f, _s) \ VMSTATE_UINT64_V(_f, _s, 0) +#define VMSTATE_INT32_EQUAL (_f, _s) \ + VMSTATE_SINGLE(_f, _s, 0, vmstate_info_int32_equal, int32_t) +#define VMSTATE_TIMER_V (_f, _s, _v) \ VMSTATE_POINTER(_f, _s, _v, vmstate_info_timer, QEMUTimer *)
savevm.c +20 -0
@@ -673,6 +673,26 @@ const VMStateInfo vmstate_info_int32 = { .put = put_int32, }; +/* 32 bit int. See that the received value is the same than the one +in the field */ + +static int get_int32_equal (QEMUFile *f,void *pv,size_t size) +{ +int32_t *v = pv; +int32_t v2; +qemu_get_sbe32s (f, &v2); + +if (*v == v2) +return 0 ; +return -EINVAL ; +} + +const VMStateInfo vmstate_info_int32_equal = { + .name ="int32 equal" , + .get = get_int32_equal, + .put = put_int32, +}; +/* 64 bit int */ static int get_int64 (QEMUFile *f,void *pv,size_t size)
Signature · 1 line
1.6.2.5
PATCH 16/23 Port PCI Bus to VMState design
In reply to Juan QuintelaThis uses VARRAY and INT32_EQUAL values
--- hw/pci.c | 41 +++++++++++------------------------------ 1 files changed, 11 insertions(+), 30 deletions(-)
hw/pci.c +11 -30
@@ -73,36 +73,17 @@ static uint16_t pci_default_sub_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET;static uint16_t pci_default_sub_device_id =PCI_SUBDEVICE_ID_QEMU ;static PCIBus *first_bus; -static void pcibus_save (QEMUFile *f,void *opaque) -{ -PCIBus *bus = (PCIBus *)opaque; -int i; - -qemu_put_be32 (f, bus->nirq); -for (i =0 ; i < bus->nirq; i++) -qemu_put_be32 (f, bus->irq_count[i]); -} - -static int pcibus_load (QEMUFile *f,void *opaque,int version_id) -{ -PCIBus *bus = (PCIBus *)opaque; -int i, nirq; - -if (version_id !=1 ) -return -EINVAL ; - - nirq =qemu_get_be32 (f); -if (bus->nirq != nirq) { -fprintf (stderr,"pcibus_load: nirq mismatch: src=%d dst=%d\n" , - nirq, bus->nirq); -return -EINVAL ; +static const VMStateDescription vmstate_pcibus = { + .name ="PCIBUS" , + .version_id =1 , + .minimum_version_id =1 , + .minimum_version_id_old =1 , + .fields = (VMStateField []) { +VMSTATE_INT32_EQUAL (nirq, PCIBus), +VMSTATE_INT32_VARRAY (irq_count, PCIBus, nirq), +VMSTATE_END_OF_LIST () } - -for (i =0 ; i < nirq; i++) - bus->irq_count[i] =qemu_get_be32 (f); - -return 0 ; -} +};static void pci_bus_reset (void *opaque) {
@@ -135,7 +116,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name, bus->irq_count =qemu_mallocz (nirq *sizeof (bus->irq_count[0 ])); bus->next = first_bus; first_bus = bus; -register_savevm ("PCIBUS" , nbus++,1 , pcibus_save, pcibus_load, bus); +vmstate_register (nbus++, &vmstate_pcibus, bus);qemu_register_reset (pci_bus_reset, bus);return bus; }
Signature · 1 line
1.6.2.5
PATCH 17/23 Add VMState support for static sized buffers (uint_8)
In reply to Juan QuintelaThis patch adds support for static sized buffer and typecheks that the buffer is right.
--- hw/hw.h | 18 ++++++++++++++++++ savevm.c | 21 +++++++++++++++++++++ 2 files changed, 39 insertions(+), 0 deletions(-)
hw/hw.h +18 -0
@@ -284,6 +284,7 @@ enum VMStateFlags {VMS_ARRAY =0x004 ,VMS_STRUCT =0x008 ,VMS_VARRAY =0x010 ,/* Array with size in another field */ + VMS_BUFFER = 0x020, /* static sized buffer */ }; typedef struct {
@@ -320,6 +321,7 @@ extern const VMStateInfo vmstate_info_uint32;extern const VMStateInfo vmstate_info_uint64;extern const VMStateInfo vmstate_info_timer; +extern const VMStateInfo vmstate_info_buffer;#define type_check_array (t1,t2,n) ((t1(*)[n])0 - (t2*)0)#define type_check_pointer (t1,t2) ((t1**)0 - (t2*)0)
@@ -388,6 +390,16 @@ extern const VMStateInfo vmstate_info_timer; + type_check_array(_type,typeof_field(_state, _field),_num) \ } +#define VMSTATE_STATIC_BUFFER (_field, _state, _version) { \ + .name = (stringify(_field)), \ + .version_id = (_version), \ + .size = sizeof(typeof_field(_state,_field)), \ + .info = &vmstate_info_buffer, \ + .flags = VMS_BUFFER, \ + .offset = offsetof(_state, _field) \ + + type_check_array(uint8_t,typeof_field(_state, _field),sizeof(typeof_field(_state,_field))) \ +} +/* _f : field name _f_n : num of elements field_name _n : num of elements
@@ -458,6 +470,12 @@ extern const VMStateInfo vmstate_info_timer;#define VMSTATE_INT32_VARRAY (_f, _s, _f_n) \ VMSTATE_INT32_VARRAY_V(_f, _s, _f_n, 0) +#define VMSTATE_BUFFER_V (_f, _s, _v) \ + VMSTATE_STATIC_BUFFER(_f, _s, _v) + +#define VMSTATE_BUFFER (_f, _s) \ + VMSTATE_STATIC_BUFFER(_f, _s, 0) +#define VMSTATE_END_OF_LIST () \ {}
savevm.c +21 -0
@@ -819,6 +819,27 @@ const VMStateInfo vmstate_info_timer = { .put = put_timer, }; +/* uint8_t buffers */ + +static int get_buffer (QEMUFile *f,void *pv,size_t size) +{ +uint8_t *v = pv; +qemu_get_buffer (f, v, size); +return 0 ; +} + +static void put_buffer (QEMUFile *f,const void *pv,size_t size) +{ +uint8_t *v = (void *)pv; +qemu_put_buffer (f, v, size); +} + +const VMStateInfo vmstate_info_buffer = { + .name ="buffer" , + .get = get_buffer, + .put = put_buffer, +}; + typedef struct SaveStateEntry { char idstr[256]; int instance_id;
Signature · 1 line
1.6.2.5
PATCH 18/23 Port PS2 devices to VMState design
In reply to Juan QuintelaThis uses STRUCT and BUFFER
--- hw/ps2.c | 119 +++++++++++++++++++++++++++----------------------------------- 1 files changed, 52 insertions(+), 67 deletions(-)
hw/ps2.c +52 -67
@@ -526,59 +526,29 @@ static void ps2_mouse_reset(void *opaque) s->mouse_buttons =0 ; } -static void ps2_common_save (QEMUFile *f,PS2State *s) -{ -qemu_put_be32 (f, s->write_cmd); -qemu_put_be32 (f, s->queue.rptr); -qemu_put_be32 (f, s->queue.wptr); -qemu_put_be32 (f, s->queue.count); -qemu_put_buffer (f, s->queue.data,sizeof (s->queue.data)); -} - -static void ps2_common_load (QEMUFile *f,PS2State *s) -{ - s->write_cmd=qemu_get_be32 (f); - s->queue.rptr=qemu_get_be32 (f); - s->queue.wptr=qemu_get_be32 (f); - s->queue.count=qemu_get_be32 (f); -qemu_get_buffer (f, s->queue.data,sizeof (s->queue.data)); -} - -static void ps2_kbd_save (QEMUFile * f,void * opaque) -{ -PS2KbdState *s = (PS2KbdState *)opaque; - -ps2_common_save (f, &s->common); -qemu_put_be32 (f, s->scan_enabled); -qemu_put_be32 (f, s->translate); -qemu_put_be32 (f, s->scancode_set); -} - -static void ps2_mouse_save (QEMUFile * f,void * opaque) -{ -PS2MouseState *s = (PS2MouseState *)opaque; - -ps2_common_save (f, &s->common); -qemu_put_8s (f, &s->mouse_status); -qemu_put_8s (f, &s->mouse_resolution); -qemu_put_8s (f, &s->mouse_sample_rate); -qemu_put_8s (f, &s->mouse_wrap); -qemu_put_8s (f, &s->mouse_type); -qemu_put_8s (f, &s->mouse_detect_state); -qemu_put_be32 (f, s->mouse_dx); -qemu_put_be32 (f, s->mouse_dy); -qemu_put_be32 (f, s->mouse_dz); -qemu_put_8s (f, &s->mouse_buttons); -} +static const VMStateDescription vmstate_ps2_common = { + .name ="PS2 Common State" , + .version_id =3 , + .minimum_version_id =2 , + .minimum_version_id_old =2 , + .fields = (VMStateField []) { +VMSTATE_INT32 (write_cmd, PS2State), +VMSTATE_INT32 (queue.rptr, PS2State), +VMSTATE_INT32 (queue.wptr, PS2State), +VMSTATE_INT32 (queue.count, PS2State), +VMSTATE_BUFFER (queue.data, PS2State), +VMSTATE_END_OF_LIST () + } +}; -static int ps2_kbd_load (QEMUFile * f,void * opaque,int version_id) +static int ps2_kbd_load_old (QEMUFile * f,void * opaque,int version_id) {PS2KbdState *s = (PS2KbdState *)opaque;if (version_id !=2 && version_id !=3 )return -EINVAL ; -ps2_common_load (f, &s->common); +vmstate_load_state (f, &vmstate_ps2_common, &s->common, version_id); s->scan_enabled=qemu_get_be32 (f); s->translate=qemu_get_be32 (f);if (version_id ==3 )
@@ -588,26 +558,41 @@ static int ps2_kbd_load(QEMUFile* f, void* opaque, int version_id)return 0 ; } -static int ps2_mouse_load (QEMUFile * f,void * opaque,int version_id) -{ -PS2MouseState *s = (PS2MouseState *)opaque; - -if (version_id !=2 ) -return -EINVAL ; +static const VMStateDescription vmstate_ps2_keyboard = { + .name ="ps2kbd" , + .version_id =3 , + .minimum_version_id =3 , + .minimum_version_id_old =2 , + .load_state_old = ps2_kbd_load_old, + .fields = (VMStateField []) { +VMSTATE_STRUCT (common, PS2KbdState,0 , vmstate_ps2_common, PS2State), +VMSTATE_INT32 (scan_enabled, PS2KbdState), +VMSTATE_INT32 (translate, PS2KbdState), +VMSTATE_INT32_V (scancode_set, PS2KbdState,3 ), +VMSTATE_END_OF_LIST () + } +}; -ps2_common_load (f, &s->common); -qemu_get_8s (f, &s->mouse_status); -qemu_get_8s (f, &s->mouse_resolution); -qemu_get_8s (f, &s->mouse_sample_rate); -qemu_get_8s (f, &s->mouse_wrap); -qemu_get_8s (f, &s->mouse_type); -qemu_get_8s (f, &s->mouse_detect_state); - s->mouse_dx=qemu_get_be32 (f); - s->mouse_dy=qemu_get_be32 (f); - s->mouse_dz=qemu_get_be32 (f); -qemu_get_8s (f, &s->mouse_buttons); -return 0 ; -} +static const VMStateDescription vmstate_ps2_mouse = { + .name ="ps2mouse" , + .version_id =2 , + .minimum_version_id =2 , + .minimum_version_id_old =2 , + .fields = (VMStateField []) { +VMSTATE_STRUCT (common, PS2MouseState,0 , vmstate_ps2_common, PS2State), +VMSTATE_UINT8 (mouse_status, PS2MouseState), +VMSTATE_UINT8 (mouse_resolution, PS2MouseState), +VMSTATE_UINT8 (mouse_sample_rate, PS2MouseState), +VMSTATE_UINT8 (mouse_wrap, PS2MouseState), +VMSTATE_UINT8 (mouse_type, PS2MouseState), +VMSTATE_UINT8 (mouse_detect_state, PS2MouseState), +VMSTATE_INT32 (mouse_dx, PS2MouseState), +VMSTATE_INT32 (mouse_dy, PS2MouseState), +VMSTATE_INT32 (mouse_dz, PS2MouseState), +VMSTATE_UINT8 (mouse_buttons, PS2MouseState), +VMSTATE_END_OF_LIST () + } +};void *ps2_kbd_init (void (*update_irq)(void *,int ),void *update_arg) {
@@ -617,7 +602,7 @@ void *ps2_kbd_init(void (*update_irq)(void *, int), void *update_arg) s->common.update_arg = update_arg; s->scancode_set =2 ;ps2_kbd_reset (s); -register_savevm ("ps2kbd" ,0 ,3 , ps2_kbd_save, ps2_kbd_load, s); +vmstate_register (0 , &vmstate_ps2_keyboard, s);qemu_add_kbd_event_handler (ps2_put_keycode, s);qemu_register_reset (ps2_kbd_reset, s);return s;
@@ -630,7 +615,7 @@ void *ps2_mouse_init(void (*update_irq)(void *, int), void *update_arg) s->common.update_irq = update_irq; s->common.update_arg = update_arg;ps2_mouse_reset (s); -register_savevm ("ps2mouse" ,0 ,2 , ps2_mouse_save, ps2_mouse_load, s); +vmstate_register (0 , &vmstate_ps2_mouse, s);qemu_add_mouse_event_handler (ps2_mouse_event, s,0 ,"QEMU PS/2 Mouse" );qemu_register_reset (ps2_mouse_reset, s);return s;
Signature · 1 line
1.6.2.5
PATCH 19/23 Add VMState support for int32_t check value
In reply to Juan QuintelaWe read the saved value and check that it is less or equal than the one stored in the structure.
--- hw/hw.h | 4 ++++ savevm.c | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 0 deletions(-)
hw/hw.h +4 -0
@@ -314,6 +314,7 @@ extern const VMStateInfo vmstate_info_int32;extern const VMStateInfo vmstate_info_int64;extern const VMStateInfo vmstate_info_int32_equal; +extern const VMStateInfo vmstate_info_int32_le;extern const VMStateInfo vmstate_info_uint8;extern const VMStateInfo vmstate_info_uint16;
@@ -446,6 +447,9 @@ extern const VMStateInfo vmstate_info_buffer;#define VMSTATE_INT32_EQUAL (_f, _s) \ VMSTATE_SINGLE(_f, _s, 0, vmstate_info_int32_equal, int32_t) +#define VMSTATE_INT32_LE (_f, _s) \ + VMSTATE_SINGLE(_f, _s, 0, vmstate_info_int32_le, int32_t) +#define VMSTATE_TIMER_V (_f, _s, _v) \ VMSTATE_POINTER(_f, _s, _v, vmstate_info_timer, QEMUTimer *)
savevm.c +20 -0
@@ -693,6 +693,26 @@ const VMStateInfo vmstate_info_int32_equal = { .put = put_int32, }; +/* 32 bit int. See that the received value is the less or the same +than the one in the field */ + +static int get_int32_le (QEMUFile *f,void *pv,size_t size) +{ +int32_t *old = pv; +int32_t new; +qemu_get_sbe32s (f, &new); + +if (*old <= new) +return 0 ; +return -EINVAL ; +} + +const VMStateInfo vmstate_info_int32_le = { + .name ="int32 equal" , + .get = get_int32_le, + .put = put_int32, +}; +/* 64 bit int */ static int get_int64 (QEMUFile *f,void *pv,size_t size)
Signature · 1 line
1.6.2.5
PATCH 22/23 Add VMState support to run a function after load
In reply to Juan Quintela--- hw/hw.h | 1 + savevm.c | 2 ++ 2 files changed, 3 insertions(+), 0 deletions(-)
hw/hw.h +1 -0
@@ -305,6 +305,7 @@ struct VMStateDescription {int minimum_version_id;int minimum_version_id_old;LoadStateHandler *load_state_old; +int (*run_after_load)(void *opaque);VMStateField *fields; };
savevm.c +2 -0
@@ -1031,6 +1031,8 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd, } field++; } +if (vmsd->run_after_load) +return vmsd->run_after_load (opaque);return 0 ; }
Signature · 1 line
1.6.2.5
PATCH 20/23 Add version_id to PCIDevice.
In reply to Juan QuintelaIt is needed for VMState
--- hw/pci.c | 3 ++- hw/pci.h | 2 ++ 2 files changed, 4 insertions(+), 1 deletions(-)
hw/pci.c +2 -1
@@ -144,7 +144,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f) {int i; -qemu_put_be32 (f,2 );/* PCI device version */ +qemu_put_be32 (f, s->version_id);/* PCI device version */ qemu_put_buffer (f, s->config,256 );for (i =0 ; i <4 ; i++)qemu_put_be32 (f, s->irq_state[i]);
@@ -319,6 +319,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, pci_dev->config_write = config_write; bus->devices[devfn] = pci_dev; pci_dev->irq =qemu_allocate_irqs (pci_set_irq, pci_dev,4 ); + pci_dev->version_id =2 ;/* Current pci device vmstate version */ return pci_dev; }
hw/pci.h +2 -0
@@ -209,6 +209,8 @@ struct PCIDevice {unsigned *msix_entry_used;/* Region including the MSI-X table */ uint32_t msix_bar_size; +/* Version id needed for VMState */ +int32_t version_id; }; PCIDevice *pci_register_device(PCIBus *bus, const char *name,
Signature · 1 line
1.6.2.5
PATCH 21/23 Port PCIDevice state to VMState
In reply to Juan QuintelaThis uses a variant of buffer, with extra checks. Also uses the new support for cheking that a read value is less or equal than a field.
--- hw/hw.h | 12 +++++++++++ hw/pci.c | 65 +++++++++++++++++++++++++++++++++++++++++--------------------- 2 files changed, 55 insertions(+), 22 deletions(-)
hw/hw.h +12 -0
@@ -401,6 +401,18 @@ extern const VMStateInfo vmstate_info_buffer; + type_check_array(uint8_t,typeof_field(_state, _field),sizeof(typeof_field(_state,_field))) \ } +extern const VMStateDescription vmstate_pci_device; + +#define VMSTATE_PCI_DEVICE (_field, _state) { \ + .name = (stringify(_field)), \ + .version_id = 2, \ + .size = sizeof(PCIDevice), \ + .vmsd = &vmstate_pci_device, \ + .flags = VMS_STRUCT, \ + .offset = offsetof(_state, _field) \ + + type_check(PCIDevice,typeof_field(_state, _field)) \ +} +/* _f : field name _f_n : num of elements field_name _n : num of elements
hw/pci.c +43 -22
82 lines not on this page · raw message
Signature · 1 line
1.6.2.5
PATCH 23/23 Port ACPI to VMState
In reply to Juan QuintelaThis uses a run_after_load() function, and VMSTATE_PCI_DEVICE() It could be made smaller changing the type of pm_io_space_update() to return an int.
--- hw/acpi.c | 55 +++++++++++++++++++++---------------------------------- 1 files changed, 21 insertions(+), 34 deletions(-)
hw/acpi.c +21 -34
@@ -440,46 +440,33 @@ static void pm_write_config(PCIDevice *d,pm_io_space_update ((PIIX4PMState *)d); } -static void pm_save (QEMUFile * f,void *opaque) +static int vmstate_acpi_after_load (void *opaque) {PIIX4PMState *s = opaque; -pci_device_save (&s->dev, f); - -qemu_put_be16s (f, &s->pmsts); -qemu_put_be16s (f, &s->pmen); -qemu_put_be16s (f, &s->pmcntrl); -qemu_put_8s (f, &s->apmc); -qemu_put_8s (f, &s->apms); -qemu_put_timer (f, s->tmr_timer); -qemu_put_be64 (f, s->tmr_overflow_time); -} - -static int pm_load (QEMUFile * f,void * opaque,int version_id) -{ -PIIX4PMState *s = opaque; -int ret; - -if (version_id >1 ) -return -EINVAL ; - - ret =pci_device_load (&s->dev, f); -if (ret <0 ) -return ret; - -qemu_get_be16s (f, &s->pmsts); -qemu_get_be16s (f, &s->pmen); -qemu_get_be16s (f, &s->pmcntrl); -qemu_get_8s (f, &s->apmc); -qemu_get_8s (f, &s->apms); -qemu_get_timer (f, s->tmr_timer); - s->tmr_overflow_time=qemu_get_be64 (f); -pm_io_space_update (s); -return 0 ; } +static const VMStateDescription vmstate_acpi = { + .name ="piix4_pm" , + .version_id =1 , + .minimum_version_id =1 , + .minimum_version_id_old =1 , + .run_after_load = vmstate_acpi_after_load, + .fields = (VMStateField []) { +VMSTATE_PCI_DEVICE (dev, PIIX4PMState), +VMSTATE_UINT16 (pmsts, PIIX4PMState), +VMSTATE_UINT16 (pmen, PIIX4PMState), +VMSTATE_UINT16 (pmcntrl, PIIX4PMState), +VMSTATE_UINT8 (apmc, PIIX4PMState), +VMSTATE_UINT8 (apms, PIIX4PMState), +VMSTATE_TIMER (tmr_timer, PIIX4PMState), +VMSTATE_INT64 (tmr_overflow_time, PIIX4PMState), +VMSTATE_END_OF_LIST () + } +}; +static void piix4_reset (void *opaque) {PIIX4PMState *s = opaque;
@@ -561,7 +548,7 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base, qemu_system_powerdown = *qemu_allocate_irqs (piix4_powerdown, s,1 ); -register_savevm ("piix4_pm" ,0 ,1 , pm_save, pm_load, s); +vmstate_register (0 , &vmstate_acpi, s); s->smbus =i2c_init_bus (NULL ,"i2c" ); s->irq = sci_irq;
Signature · 1 line
1.6.2.5
Re: [Qemu-devel] [PATCH 16/23] Port PCI Bus to VMState design
In reply to Juan QuintelaQuoted 1 line · Juan Quintela
This uses VARRAY and
INT32_EQUALvalues
This is one place where I would change the code to reduce load/save complexity.
Quick grep shows that the max value for bus->nirqs is 32. s/*irq_count/irq_count[32]/ for PCIBus, drop dynamic allocation and allways save/load 32 values. INT32_EQUAL + VARRAY isn't needed then.
cheers,
Gerd
Re: [Qemu-devel] [PATCH 21/23] Port PCIDevice state to VMState
In reply to Juan QuintelaQuoted 2 lines · Juan Quintela
This uses a variant of buffer, with extra checks. Also uses the new support for cheking that a read value is less or equal than a field.
Hmm, why you are hopping through all these loops?
I'd just keep the existing pci_device_{save,load} functions. load anyway for backward compatibility. save also for devices not yet converted to vmstate. I think alot of the complex stuff you are building up here isn't needed then, unless I missed something ...
Beside that we'll have to think about how to handle versioning of structs. Do we want vmstate_pci_device (and all others) have its own version? Probably makes sense. Handling this should go into the generic code then and not be hacked into each structure using VMSTATE_INT32_LE().
cheers,
Gerd
Quoted 4 lines · Juan Quintela
-
i440fx_load()
Can we put pci_irq_levels[] array in any struct? This is the only driver that I have found that uses a global variable. Gerd, can you take a look? It is not possible to have the PCIDevice struct inside a PIIXDevice or similar?
Stuffing a pointer to pci_irq_levels into a struct should work without problems and should be good enougth to get vmstate save/load going ;)
Sticking the array itself into a struct would be even better, that might need more cleanup work though.
cheers,
Gerd
[Qemu-devel] Re: [PATCH 21/23] Port PCIDevice state to VMState
In reply to Gerd HoffmannQuoted 10 lines · Gerd Hoffmann
Quoted 2 lines · Juan Quintela
This uses a variant of buffer, with extra checks. Also uses the new support for cheking that a read value is less or equal than a field.
Hmm, why you are hopping through all these loops?
I'd just keep the existing pci_device_{save,load} functions. load anyway for backward compatibility. save also for devices not yet converted to vmstate. I think alot of the complex stuff you are building up here isn't needed then, unless I missed something ...
In this specific case, I think that sending:
size + number of things
it is safe. I agree that the INT32_EQUAL is a bad idea. But that is how the current code is done.
Quoted 5 lines
Beside that we'll have to think about how to handle versioning of structs. Do we want
vmstate_pci_device(and all others) have its own version? Probably makes sense. Handling this should go into the generic code then and not be hacked into each structure usingVMSTATE_INT32_LE().
Proper solution here is to use subsections.
Each time that you call VMSTATE_PCI_DEVICE() it sends a subsection there. Then we get the versioning by free. This is how I am thinking it is the right way to do it.
virtio stuff: the more than I think about it, the easier way is to just get rid of the whole mess and do something that is sensible.
Later, Juan.
Quoted 2 lines
cheers,
Gerd
[Qemu-devel] Re: [PATCH 16/23] Port PCI Bus to VMState design
In reply to Gerd HoffmannQuoted 10 lines · Gerd Hoffmann
Quoted 1 line · Juan Quintela
This uses VARRAY and
INT32_EQUALvaluesThis is one place where I would change the code to reduce load/save complexity.
Quick grep shows that the max value for
bus->nirqsis
32. s/*irq_count/irq_count[32]/ for PCIBus, drop dynamic allocation and allways save/load 32 values.INT32_EQUAL+ VARRAY isn't needed then.
I agree to drop INT32_EQUAL, but maintain the VARRAY.
VARRAY is nice if there is another field in the stuct that tolds you how many are used.
If you change the 32 number, you just need to upgrade the version. Will do something like that.
The reason that I don't want to send the whole thing is that there are times when we are sending tables that are empty (devices not being used). Having an initial size makes things go away. And then, I will create the VMSTATE_VARRAY() that handles the two fields in one go.
Later, Juan.
Quoted 2 lines
cheers,
Gerd
Quoted 11 lines · Gerd Hoffmann
Quoted 4 lines · Juan Quintela
-
i440fx_load()
Can we put pci_irq_levels[] array in any struct? This is the only driver that I have found that uses a global variable. Gerd, can you take a look? It is not possible to have the PCIDevice struct inside a PIIXDevice or similar?Stuffing a pointer to
pci_irq_levelsinto a struct should work without problems and should be good enougth to get vmstate save/load going ;)Sticking the array itself into a struct would be even better, that might need more cleanup work though.
Do you mean to do any of the tow approachs?
I promise to study your patch for other places that I need :)
Later, Juan.
[Qemu-devel] Re: [PATCH 21/23] Port PCIDevice state to VMState
In reply to Juan QuintelaQuoted 2 lines · Juan Quintela
I agree that the
INT32_EQUALis a bad idea. But that is how the current code is done.
Consider changing the code then ;)
Quoted 10 lines
Quoted 5 lines
Beside that we'll have to think about how to handle versioning of structs. Do we want
vmstate_pci_device(and all others) have its own version? Probably makes sense. Handling this should go into the generic code then and not be hacked into each structure usingVMSTATE_INT32_LE().Proper solution here is to use subsections.
Each time that you callVMSTATE_PCI_DEVICE()it sends a subsection there. Then we get the versioning by free. This is how I am thinking it is the right way to do it.
Yep, something like this.
Quoted 2 lines
virtio stuff: the more than I think about it, the easier way is to just get rid of the whole mess and do something that is sensible.
You are talking about VirtIOPCIProxy I guess?
Yea, that is messy ...
cheers,
Gerd
[Qemu-devel] Re: [PATCH 16/23] Port PCI Bus to VMState design
In reply to Juan QuintelaQuoted 6 lines · Juan Quintela
Quoted 5 lines · Gerd Hoffmann
Quoted 1 line · Juan Quintela
This uses VARRAY and
INT32_EQUALvaluesThis is one place where I would change the code to reduce load/save complexity.
Quoted 8 lines
Quoted 4 lines
Quick grep shows that the max value for
bus->nirqsis
32. s/*irq_count/irq_count[32]/ for PCIBus, drop dynamic allocation and allways save/load 32 values.INT32_EQUAL+ VARRAY isn't needed then.I agree to drop
INT32_EQUAL, but maintain the VARRAY.
VARRAY is nice if there is another field in the stuct that tolds you how many are used.
If you save all of them anyway you don't need that. (but you still must save the bus->nirqs field obviously).
Quoted 6 lines
If you change the 32 number, you just need to upgrade the version. Will do something like that.
The reason that I don't want to send the whole thing is that there are times when we are sending tables that are empty (devices not being used).
I still would save it. If you find a NULL pointer save a bunch of zeros instead. Device state isn't that big, and I think it is good to have fixed-size sections.
cheers
Gerd
Quoted 11 lines · Juan Quintela
Quoted 5 lines · Gerd Hoffmann
Stuffing a pointer to
pci_irq_levelsinto a struct should work without problems and should be good enougth to get vmstate save/load going ;)Sticking the array itself into a struct would be even better, that might need more cleanup work though.
Do you mean to do any of the tow approachs?
I promise to study your patch for other places that I need :)Later, Juan.
I can have a look, probably next week though.
cheers,
Gerd
[Qemu-devel] Re: [PATCH 21/23] Port PCIDevice state to VMState
In reply to Gerd HoffmannQuoted 17 lines · Gerd Hoffmann
Quoted 2 lines · Juan Quintela
I agree that the
INT32_EQUALis a bad idea. But that is how the current code is done.Consider changing the code then ;)
Quoted 10 lines
Quoted 5 lines
Beside that we'll have to think about how to handle versioning of structs. Do we want
vmstate_pci_device(and all others) have its own version? Probably makes sense. Handling this should go into the generic code then and not be hacked into each structure usingVMSTATE_INT32_LE().Proper solution here is to use subsections.
Each time that you callVMSTATE_PCI_DEVICE()it sends a subsection there. Then we get the versioning by free. This is how I am thinking it is the right way to do it.
Quoted 7 lines
Yep, something like this.
Quoted 2 lines
virtio stuff: the more than I think about it, the easier way is to just get rid of the whole mess and do something that is sensible.
You are talking about VirtIOPCIProxy I guess?
Yea, that is messy ...
I am talking about virtio_load() actually. How do you translate this to a table?
- If at the beggining (and that load_config() reads the config from the savevm
- and another if in the middle of a for
int virtio_load(VirtIODevice *vdev, QEMUFile *f)
{
int num, i, ret;
if (vdev->binding->load_config) {
ret = vdev->binding->load_config(vdev->binding_opaque, f);
if (ret)
return ret;
}
qemu_get_8s(f, &vdev->status);
qemu_get_8s(f, &vdev->isr);
qemu_get_be16s(f, &vdev->queue_sel);
qemu_get_be32s(f, &vdev->features);
vdev->config_len = qemu_get_be32(f);
qemu_get_buffer(f, vdev->config, vdev->config_len);
num = qemu_get_be32(f);
for (i = 0; i < num; i++) {
vdev->vq[i].vring.num = qemu_get_be32(f);
vdev->vq[i].pa = qemu_get_be64(f);
qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);
if (vdev->vq[i].pa) {
virtqueue_init(&vdev->vq[i]);
}
if (vdev->binding->load_queue) {
ret = vdev->binding->load_queue(vdev->binding_opaque, i, f);
if (ret)
return ret;
}
}
virtio_notify_vector(vdev, VIRTIO_NO_VECTOR);
return 0;
}Can we have something saner like:
int virtio_load(VirtIODevice *vdev, QEMUFile *f)
{
int num, i, ret;
qemu_get_be32s(f, &vdev->config_load_size);
qemu_get_buffer(f, &vdev->config_load_value, &vdev->config_load_size);
qemu_get_8s(f, &vdev->status);
qemu_get_8s(f, &vdev->isr);
qemu_get_be16s(f, &vdev->queue_sel);
qemu_get_be32s(f, &vdev->features);
vdev->config_len = qemu_get_be32(f);
qemu_get_buffer(f, vdev->config, vdev->config_len);
num = qemu_get_be32(f);
for (i = 0; i < num; i++) {
vdev->vq[i].vring.num = qemu_get_be32(f);
vdev->vq[i].pa = qemu_get_be64(f);
qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);
qemu_get_be32s(f, &vdev->queue_size[i]);
qemu_get_buffer(f, &vdev->queue_value, &vdev->queue_size[i]);
}
/* load of state has ended, time to start configuring things */
if (vdev->binding->load_config) {
ret = vdev->binding->load_config(vdev->binding_opaque, f);
if (ret)
return ret;
}
for (i = 0; i < num; i++) {
if (vdev->vq[i].pa) {
virtqueue_init(&vdev->vq[i]);
if (vdev->binding->load_queue) {
ret = vdev->binding->load_queue(vdev->binding_opaque, i, f);
if (ret)
return ret;
}
}
}
virtio_notify_vector(vdev, VIRTIO_NO_VECTOR);
return 0;
}I.e. we just reserved the space to load the stuff (0 is ok), and after loading, We initialize things at virtio maintainers pace. Obviously the load_queue/config stuff has to be changed to work from a local variable instead of the savevm stream.
Later, Juan.
Quoted 8 lines · Gerd Hoffmann
Quoted 7 lines · Juan Quintela
Quoted 5 lines · Gerd Hoffmann
Stuffing a pointer to
pci_irq_levelsinto a struct should work without problems and should be good enougth to get vmstate save/load going ;)Sticking the array itself into a struct would be even better, that might need more cleanup work though.
Quoted 6 lines
Quoted 4 lines
Do you mean to do any of the tow approachs?
I promise to study your patch for other places that I need :)Later, Juan.
I can have a look, probably next week though.
I have just saw Markus Watchdog conversion. Perhaps can get enough "inspiration" from there to try to do it myself.
Later, Juan.
[Qemu-devel] Re: [PATCH 21/23] Port PCIDevice state to VMState
In reply to Juan QuintelaQuoted 2 lines · Juan Quintela
I am talking about
virtio_load()actually. How do you translate this to a table?
Oh yea, that is nasty.
Quoted 9 lines
int
virtio_load(VirtIODevice *vdev, QEMUFile *f){ int num, i, ret; if (vdev->binding->load_config) { ret = vdev->binding->load_config(vdev->binding_opaque, f); if (ret) return ret; }
Hmm. Move register_savevm from virtio.c to virtio_pci.c?
Then have virtio_load_pci() which basically calls pci_device_save() + virtio_common_save() and kill this binding indirection.
Note: There are non-pci virtio bindings somewhere in qemu.
cheers,
Gerd
Re: [Qemu-devel] [PATCH 07/23] New VMstate save/load infrastructure
In reply to Juan QuintelaQuoted 12 lines · Juan Quintela
This patch introduces VMState infrastructure, to convert the save/load functions of devices to a table approach. This new approach has the following advantages:
- it is type-safe
- you can't have load/save functions out of sync
- will allows us to have new interesting commands, like dump <device>, that shows all its internal state.
- Just now, the only added type is arrays, but we can add structures.
- Uses oldload_state()function for loading old state.Signed-off-by Juan Quintela <[email protected]>---
...
hw/hw.h typedef int QEMUBootSetHandler(void *opaque, const char *boot_devices); · line 321
Quoted 9 lines
hw/hw.h +9
-0+#define VMSTATE_SINGLE (_field, _state, _version, _info, _type) { \ + .name = (stringify(_field)), \ + .version_id = (_version), \ + .size = sizeof(_type), \ + .info = &(_info), \ + .flags = VMS_SINGLE, \ + .offset = offsetof(_state, _field) \ + + type_check(_type,typeof_field(_state, _field)) \ +}
As I said in a different thread, I think the versioning approach taken here is too limited. In particular, it does not easily support optional features such as msix. We can use other mechanisms for this, such as encoding options in the device name, but I think it's too limiting.
What I think we should have instead of a version is an identifier for each field. This way it is easy to add or skip each field separately, depending on command line options, and we can also verify, at load time, that all required fields are present. This will also reduce the amount of duplicated code required each time we extend migration format.
As a bonus, we'll be able to write a utility parsing the state and checking that it's sane.
Signature · 1 line
MST