In vmstate_save|load_vmsd[_v], move the code handling
VMS_ARRAY_OF_POINTER all to the same block. These functions deal with
single elements, scalar arrays and arrays of pointers, of which the
latter is the most complex code. Group that use-case so it's easier to
reason about.
This allows for a significant design cleanup, which is to stop using
the inner_field variable in the case where there is NO pointer
marker. Now the inner_field is internal to the VMS_ARRAY_OF_POINTER
code.
By adding a save_field boolean, we can reuse the "actual field" write
for the !use_marker case. This brings the benefit of being able to
drop the field->size specialization. Like so:
===
before:
size = vmstate_size();
if (use_marker) {
inner_field = fake;
} else {
inner_field = field;
}
vmstate_save_field_with_vmdesc(inner_field, size)
if (use_marker && !curr) {
vmstate_save_field_with_vmdesc(field, field->size)
}
after:
size = vmstate_size();
if (use_marker) {
inner_field = fake;
vmstate_save_field_with_vmdesc(inner_field, 1)
save_field = !curr;
}
if (save_field) {
vmstate_save_field_with_vmdesc(field, size)
}
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/vmstate.c | 193 +++++++++++++++++++++-----------------------
1 file changed, 92 insertions(+), 101 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 7a12245d36..a190c3f63f 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -277,12 +277,6 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
void *first_elem = opaque + field->offset;
int i, n_elems = vmstate_n_elems(opaque, field);
int size = vmstate_size(opaque, field);
- /*
- * When this is enabled, it means we will always push a ptr
- * marker first for each element saying if it's populated.
- */
- bool use_dynamic_array =
- field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC;
vmstate_handle_alloc(first_elem, field, opaque);
if (field->flags & VMS_POINTER) {
@@ -294,36 +288,40 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
/* If we will process the load of field? */
bool load_field = true;
bool ok = true;
- bool use_marker_field;
void *curr_elem_p = first_elem + size * i;
void *curr_elem = curr_elem_p;
if (field->flags & VMS_ARRAY_OF_POINTER) {
+ bool use_dynamic_array =
+ field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC;
+ bool use_marker_field;
+
curr_elem = *(void **)curr_elem_p;
- }
- use_marker_field = use_dynamic_array || (!curr_elem && size);
+ use_marker_field = use_dynamic_array || !curr_elem;
- if (use_marker_field) {
- /* Read the marker instead of VMSD first */
- if (!vmstate_ptr_marker_load(f, &load_field, errp)) {
- trace_vmstate_load_field_error(field->name, -EINVAL);
- return false;
- }
+ if (use_marker_field) {
+ /* Read the marker instead of VMSD first */
+ if (!vmstate_ptr_marker_load(f, &load_field, errp)) {
+ trace_vmstate_load_field_error(field->name,
+ -EINVAL);
+ return false;
+ }
- if (load_field) {
- /*
- * When reaching here, it means we received a
- * non-NULL ptr marker, so we need to populate the
- * field before loading it.
- *
- * NOTE: do not use vmstate_size() here, because we
- * need the object size, not entry size of the
- * array.
- */
- curr_elem = g_malloc0(field->size);
- /* Remember to update the root pointer! */
- *(void **)curr_elem_p = curr_elem;
+ if (load_field) {
+ /*
+ * When reaching here, it means we received a
+ * non-NULL ptr marker, so we need to populate the
+ * field before loading it.
+ *
+ * NOTE: do not use vmstate_size() here, because we
+ * need the object size, not entry size of the
+ * array.
+ */
+ curr_elem = g_malloc0(field->size);
+ /* Remember to update the root pointer! */
+ *(void **)curr_elem_p = curr_elem;
+ }
}
}
@@ -625,13 +623,6 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
bool is_null_prev = false;
bool use_vmdesc = true;
- /*
- * When this is enabled, it means we will always push a ptr
- * marker first for each element saying if it's populated.
- */
- bool use_dynamic_array =
- field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC;
-
trace_vmstate_save_state_loop(vmsd->name, field->name, n_elems);
if (field->flags & VMS_POINTER) {
first_elem = *(void **)first_elem;
@@ -639,83 +630,83 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
}
for (i = 0; i < n_elems; i++) {
+ bool save_field = true;
void *curr_elem = first_elem + size * i;
- const VMStateField *inner_field;
- bool use_marker_field, is_null = false;
int max_elems = n_elems - i;
if (field->flags & VMS_ARRAY_OF_POINTER) {
+ const VMStateField *inner_field;
+ bool use_marker_field, is_null, use_dynamic_array;
+
assert(curr_elem);
curr_elem = *(void **)curr_elem;
+
is_null = !curr_elem;
- }
- use_marker_field = use_dynamic_array || is_null;
-
- if (use_marker_field) {
- inner_field = vmsd_create_ptr_marker_field(field);
- } else {
- inner_field = field;
- }
-
- /*
- * This logic only matters when dumping VM Desc, and only
- * when the VMSD field can be compressed.
- *
- * Due to the fake nullptr handling above, if there's mixed
- * null/non-null data, it doesn't make sense to emit a
- * compressed array representation spanning the entire array
- * because the field types will be different (e.g. struct
- * vs. nullptr). Search ahead for the next null/non-null element
- * and start a new compressed array if found.
- */
- if (vmdesc && vmsd_can_compress(field) &&
- (field->flags & VMS_ARRAY_OF_POINTER) &&
- is_null != is_null_prev) {
-
- is_null_prev = is_null;
- use_vmdesc = true;
-
- for (int j = i + 1; j < n_elems; j++) {
- void *elem = *(void **)(first_elem + size * j);
- bool elem_is_null = !elem;
-
- if (is_null != elem_is_null) {
- max_elems = j - i;
- break;
- }
- }
- }
-
- if (use_dynamic_array) {
- use_vmdesc = true;
- }
-
- ok = vmstate_save_field_with_vmdesc(f, curr_elem, size, vmsd,
- inner_field,
- use_vmdesc ? vmdesc : NULL,
- i, max_elems, errp);
-
- /* If we used a fake temp field.. free it now */
- if (use_marker_field) {
- g_clear_pointer((gpointer *)&inner_field, g_free);
- }
-
- if (!ok) {
- goto out;
- }
-
- /*
- * If we're using dynamic array and the element is
- * populated, dump the real object right after the marker.
- */
- if (use_dynamic_array && curr_elem) {
/*
- * NOTE: do not use vmstate_size() here because we want
- * to dump the real VMSD object now.
+ * When this is enabled, it means we will always push a ptr
+ * marker first for each element saying if it's populated.
*/
+ use_dynamic_array =
+ field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC;
+
+ use_marker_field = use_dynamic_array || is_null;
+
+ if (vmdesc && vmsd_can_compress(field)) {
+ /*
+ * This logic only matters when dumping VM
+ * Desc, and only when the VMSD field can be
+ * compressed.
+ *
+ * Due to the fake nullptr handling above, if
+ * there's mixed null/non-null data, it
+ * doesn't make sense to emit a compressed
+ * array representation spanning the entire
+ * array because the field types will be
+ * different (e.g. struct vs. nullptr). Search
+ * ahead for the next null/non-null element
+ * and start a new compressed array if found.
+ */
+ if (is_null != is_null_prev) {
+ is_null_prev = is_null;
+ use_vmdesc = true;
+
+ for (int j = i + 1; j < n_elems; j++) {
+ void *elem = *(void **)(first_elem + size * j);
+ bool elem_is_null = !elem;
+
+ if (is_null != elem_is_null) {
+ max_elems = j - i;
+ break;
+ }
+ }
+ }
+ }
+
+ if (use_dynamic_array) {
+ use_vmdesc = true;
+ }
+
+ if (use_marker_field) {
+ inner_field = vmsd_create_ptr_marker_field(field);
+
+ ok = vmstate_save_field_with_vmdesc(
+ f, curr_elem, 1, vmsd, inner_field,
+ use_vmdesc ? vmdesc : NULL, i, max_elems, errp);
+
+ g_clear_pointer((gpointer *)&inner_field, g_free);
+
+ if (!ok) {
+ goto out;
+ }
+
+ save_field = !!curr_elem;
+ }
+ }
+
+ if (save_field) {
ok = vmstate_save_field_with_vmdesc(
- f, curr_elem, field->size, vmsd, field,
+ f, curr_elem, size, vmsd, field,
use_vmdesc ? vmdesc : NULL, i, max_elems, errp);
if (!ok) {
--
2.51.0© 2016 - 2026 Red Hat, Inc.