Skip to content

Commit

Permalink
migration: Unify and trace vmstate field_exists() checks
Browse files Browse the repository at this point in the history
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

Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
Message-ID: <20230906204722.514474-1-peterx@redhat.com>
  • Loading branch information
xzpeter authored and Juan Quintela committed Oct 4, 2023
1 parent 385f510 commit 579cedf
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 8 deletions.
1 change: 1 addition & 0 deletions migration/trace-events
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ vmstate_save_state_loop(const char *name, const char *field, int n_elems) "%s/%s
vmstate_save_state_top(const char *idstr) "%s"
vmstate_subsection_save_loop(const char *name, const char *sub) "%s/%s"
vmstate_subsection_save_top(const char *idstr) "%s"
vmstate_field_exists(const char *vmsd, const char *name, int field_version, int version, int result) "%s:%s field_version %d version %d result %d"

# vmstate-types.c
get_qtailq(const char *name, int version_id) "%s v%d"
Expand Down
34 changes: 26 additions & 8 deletions migration/vmstate.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,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;
}

return result;
}

static int vmstate_n_elems(void *opaque, const VMStateField *field)
{
int n_elems = 1;
Expand Down Expand Up @@ -105,10 +129,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
}
while (field->name) {
trace_vmstate_load_state_field(vmsd->name, field->name);
if ((field->field_exists &&
field->field_exists(opaque, version_id)) ||
(!field->field_exists &&
field->version_id <= version_id)) {
if (vmstate_field_exists(vmsd, field, opaque, version_id)) {
void *first_elem = opaque + field->offset;
int i, n_elems = vmstate_n_elems(opaque, field);
int size = vmstate_size(opaque, field);
Expand Down Expand Up @@ -349,10 +370,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
}

while (field->name) {
if ((field->field_exists &&
field->field_exists(opaque, version_id)) ||
(!field->field_exists &&
field->version_id <= version_id)) {
if (vmstate_field_exists(vmsd, field, opaque, version_id)) {
void *first_elem = opaque + field->offset;
int i, n_elems = vmstate_n_elems(opaque, field);
int size = vmstate_size(opaque, field);
Expand Down

0 comments on commit 579cedf

Please sign in to comment.