Peter Xu <pet...@redhat.com> writes: > For both save/load we actually share the logic on deciding whether a field > should exist. Merge the checks into a helper and use it for both save and > load. When doing so, add documentations and reformat the code to make it > much easier to read. > > The real benefit here (besides code cleanups) is we add a trace-point for > this; this is a known spot where we can easily break migration > compatibilities between binaries, and this trace point will be critical for > us to identify such issues. > > For example, this will be handy when debugging things like: > > https://gitlab.com/qemu-project/qemu/-/issues/932 > > Signed-off-by: Peter Xu <pet...@redhat.com> > --- > migration/vmstate.c | 34 ++++++++++++++++++++++++++-------- > migration/trace-events | 1 + > 2 files changed, 27 insertions(+), 8 deletions(-) > > diff --git a/migration/vmstate.c b/migration/vmstate.c > index 31842c3afb..73e74ddea0 100644 > --- a/migration/vmstate.c > +++ b/migration/vmstate.c > @@ -25,6 +25,30 @@ static int vmstate_subsection_save(QEMUFile *f, const > VMStateDescription *vmsd, > static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription > *vmsd, > void *opaque); > > +/* Whether this field should exist for either save or load the VM? */ > +static bool > +vmstate_field_exists(const VMStateDescription *vmsd, const VMStateField > *field, > + void *opaque, int version_id) > +{ > + bool result; > + > + if (field->field_exists) { > + /* If there's the function checker, that's the solo truth */ > + result = field->field_exists(opaque, version_id); > + trace_vmstate_field_exists(vmsd->name, field->name, > field->version_id, > + version_id, result); > + } else { > + /* > + * Otherwise, we only save/load if field version is same or older. > + * For example, when loading from an old binary with old version, > + * we ignore new fields with newer version_ids. > + */ > + result = field->version_id <= version_id;
This one doesn't get a trace? Aside from that: Reviewed-by: Fabiano Rosas <faro...@suse.de>
