* Richard Henderson (richard.hender...@linaro.org) wrote: > On 10/10/18 1:37 PM, Aaron Lindsay wrote: > > In some cases it may be helpful to modify state before saving it for > > migration, and then modify the state back after it has been saved. The > > existing pre_save function provides half of this functionality. This > > patch adds a post_save function to provide the second half. > > > > Signed-off-by: Aaron Lindsay <aclin...@gmail.com> > > --- > > docs/devel/migration.rst | 9 +++++++-- > > include/migration/vmstate.h | 1 + > > migration/vmstate.c | 10 +++++++++- > > 3 files changed, 17 insertions(+), 3 deletions(-) > > Hmm, maybe. I believe the common practice is for pre_save to copy state into > a > separate member on the side, so that conversion back isn't necessary. > > Ccing in the migration maintainers for a second opinion.
It is common to copy stuff into a separate member; however we do occasionally think that post_save would be a useful addition; so I think we should take it (if nothing else it actually makes stuff symmetric!). Please make it return 'int' in the same way that pre_save/pre_load does, so that it can fail and stop the migration. Dave > > > r~ > > > > > diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst > > index 687570754d..2a2533c9b3 100644 > > --- a/docs/devel/migration.rst > > +++ b/docs/devel/migration.rst > > @@ -419,8 +419,13 @@ The functions to do that are inside a vmstate > > definition, and are called: > > > > This function is called before we save the state of one device. > > > > -Example: You can look at hpet.c, that uses the three function to > > -massage the state that is transferred. > > +- ``void (*post_save)(void *opaque);`` > > + > > + This function is called after we save the state of one device > > + (even upon failure, unless the call to pre_save returned and error). > > + > > +Example: You can look at hpet.c, that uses the first three functions > > +to massage the state that is transferred. > > > > The ``VMSTATE_WITH_TMP`` macro may be useful when the migration > > data doesn't match the stored device data well; it allows an > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > > index 2b501d0466..f6053b94e4 100644 > > --- a/include/migration/vmstate.h > > +++ b/include/migration/vmstate.h > > @@ -185,6 +185,7 @@ struct VMStateDescription { > > int (*pre_load)(void *opaque); > > int (*post_load)(void *opaque, int version_id); > > int (*pre_save)(void *opaque); > > + void (*post_save)(void *opaque); > > bool (*needed)(void *opaque); > > VMStateField *fields; > > const VMStateDescription **subsections; > > diff --git a/migration/vmstate.c b/migration/vmstate.c > > index 0bc240a317..9afc9298f3 100644 > > --- a/migration/vmstate.c > > +++ b/migration/vmstate.c > > @@ -387,6 +387,9 @@ int vmstate_save_state_v(QEMUFile *f, const > > VMStateDescription *vmsd, > > if (ret) { > > error_report("Save of field %s/%s failed", > > vmsd->name, field->name); > > + if (vmsd->post_save) { > > + vmsd->post_save(opaque); > > + } > > return ret; > > } > > > > @@ -412,7 +415,12 @@ int vmstate_save_state_v(QEMUFile *f, const > > VMStateDescription *vmsd, > > json_end_array(vmdesc); > > } > > > > - return vmstate_subsection_save(f, vmsd, opaque, vmdesc); > > + ret = vmstate_subsection_save(f, vmsd, opaque, vmdesc); > > + > > + if (vmsd->post_save) { > > + vmsd->post_save(opaque); > > + } > > + return ret; > > } > > > > static const VMStateDescription * > > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK