Similarly to vmstate_first(), introduce a vmstate_next(), which does
the necessary dereferencing of pointers to get to the leaf element. On
the load side, allow the caller to pass a flag indicating whether
allocation is expected and call the allocation function if so.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/vmstate.c | 84 +++++++++++++++++++++------------------------
1 file changed, 39 insertions(+), 45 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index ab7c6fa4ab..c1ad0ef9a5 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -127,24 +127,51 @@ static void *vmstate_first(void *opaque, const VMStateField *field,
return first;
}
-static bool vmstate_ptr_marker_load(QEMUFile *f, bool *load_field,
- Error **errp)
+static void *vmstate_next(void **first, const VMStateField *field,
+ int size, int i, bool alloc)
{
- int byte = qemu_get_byte(f);
+ void **array_elem;
+ void *next;
+
+ if (!(field->flags & VMS_ARRAY_OF_POINTER)) {
+ next = (void *)first + size * i;
+ return next;
+ }
+
+ array_elem = first + i;
+ next = *array_elem;
+
+ if (alloc) {
+ if (!next || field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC) {
+ /*
+ * NOTE: do not use vmstate_size() here, because we
+ * need the object size, not entry size of the
+ * array.
+ */
+ next = vmstate_handle_alloc(array_elem, field->size, 1);
+ }
+ }
+ return next;
+}
+
+static bool vmstate_ptr_marker_load(QEMUFile *f, bool *load_field)
+{
+ int byte = qemu_peek_byte(f, 0);
if (byte == VMS_MARKER_PTR_NULL) {
/* When it's a null ptr marker, do not continue the load */
*load_field = false;
+ qemu_file_skip(f, 1);
return true;
}
if (byte == VMS_MARKER_PTR_VALID) {
/* We need to load this field right after the marker */
*load_field = true;
+ qemu_file_skip(f, 1);
return true;
}
- error_setg(errp, "Unexpected ptr marker: %d", byte);
return false;
}
@@ -273,41 +300,13 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
void *curr_elem;
if (field->flags & VMS_ARRAY_OF_POINTER) {
- void **array_elem = (void **)head + i;
- bool use_dynamic_array =
- field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC;
- bool use_marker_field;
-
- curr_elem = *array_elem;
- 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 (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.
- */
- assert(!curr_elem);
- curr_elem = vmstate_handle_alloc(array_elem,
- field->size, 1);
- }
- }
- } else {
- curr_elem = head + size * i;
+ /* Peek a possible pointer marker instead of VMSD first */
+ ok = vmstate_ptr_marker_load(f, &load_field);
}
+ curr_elem = vmstate_next(head, field, size, i,
+ load_field && ok);
+
if (load_field) {
ok = vmstate_load_field(f, curr_elem, size, field, errp);
}
@@ -647,15 +646,11 @@ 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;
+ void *curr_elem = vmstate_next(head, field, size, i, false);
int max_elems = n_elems - i;
if (field->flags & VMS_ARRAY_OF_POINTER) {
bool use_marker_field, is_null, use_dynamic_array;
- void **array_elem = (void **)head + i;
-
- assert(array_elem);
- curr_elem = *array_elem;
is_null = !curr_elem;
@@ -688,7 +683,8 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
use_vmdesc = true;
for (int j = i + 1; j < n_elems; j++) {
- void *elem = *(void **)(head + size * j);
+ void *elem = vmstate_next(head, field, size, j,
+ false);
bool elem_is_null = !elem;
if (is_null != elem_is_null) {
@@ -713,8 +709,6 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
save_field = !!curr_elem;
}
- } else {
- curr_elem = head + size * i;
}
if (save_field) {
--
2.51.0On Tue, Mar 24, 2026 at 04:43:27PM -0300, Fabiano Rosas wrote:
> Similarly to vmstate_first(), introduce a vmstate_next(), which does
> the necessary dereferencing of pointers to get to the leaf element. On
> the load side, allow the caller to pass a flag indicating whether
> allocation is expected and call the allocation function if so.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> migration/vmstate.c | 84 +++++++++++++++++++++------------------------
> 1 file changed, 39 insertions(+), 45 deletions(-)
>
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index ab7c6fa4ab..c1ad0ef9a5 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -127,24 +127,51 @@ static void *vmstate_first(void *opaque, const VMStateField *field,
> return first;
> }
>
> -static bool vmstate_ptr_marker_load(QEMUFile *f, bool *load_field,
> - Error **errp)
> +static void *vmstate_next(void **first, const VMStateField *field,
> + int size, int i, bool alloc)
> {
> - int byte = qemu_get_byte(f);
> + void **array_elem;
> + void *next;
> +
> + if (!(field->flags & VMS_ARRAY_OF_POINTER)) {
> + next = (void *)first + size * i;
> + return next;
> + }
> +
> + array_elem = first + i;
> + next = *array_elem;
> +
> + if (alloc) {
> + if (!next || field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC) {
> + /*
> + * NOTE: do not use vmstate_size() here, because we
> + * need the object size, not entry size of the
> + * array.
> + */
> + next = vmstate_handle_alloc(array_elem, field->size, 1);
> + }
> + }
> + return next;
> +}
> +
> +static bool vmstate_ptr_marker_load(QEMUFile *f, bool *load_field)
> +{
> + int byte = qemu_peek_byte(f, 0);
Here, peeking the marker for ...
>
> if (byte == VMS_MARKER_PTR_NULL) {
> /* When it's a null ptr marker, do not continue the load */
> *load_field = false;
> + qemu_file_skip(f, 1);
> return true;
> }
>
> if (byte == VMS_MARKER_PTR_VALID) {
> /* We need to load this field right after the marker */
> *load_field = true;
> + qemu_file_skip(f, 1);
> return true;
> }
>
> - error_setg(errp, "Unexpected ptr marker: %d", byte);
> return false;
> }
>
> @@ -273,41 +300,13 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
> void *curr_elem;
>
> if (field->flags & VMS_ARRAY_OF_POINTER) {
... every VMS_ARRAY_OF_POINTER case makes me feel uneasy.
Note that we only have two use cases that both of them are certain on the
upcoming byte to come:
(1) for the null-only pointer, which is before this series introduced, both
src/dst qemu knows 100% which ptr is NULL, so dest QEMU expects that 0x30
for each null pointer.
(2) for the new _AUTO_ALLOC introduced here, we also know exactly either
0x30 or 0x31 will come, and one of them must come before the real field
dump.
I think we may shoot us in the foot if we see 0x30 but in reality it's just
the 1st byte of some array field's binary stream.
> - void **array_elem = (void **)head + i;
> - bool use_dynamic_array =
> - field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC;
> - bool use_marker_field;
> -
> - curr_elem = *array_elem;
> - 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 (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.
> - */
> - assert(!curr_elem);
> - curr_elem = vmstate_handle_alloc(array_elem,
> - field->size, 1);
> - }
> - }
> - } else {
> - curr_elem = head + size * i;
> + /* Peek a possible pointer marker instead of VMSD first */
> + ok = vmstate_ptr_marker_load(f, &load_field);
> }
>
> + curr_elem = vmstate_next(head, field, size, i,
> + load_field && ok);
> +
> if (load_field) {
> ok = vmstate_load_field(f, curr_elem, size, field, errp);
> }
> @@ -647,15 +646,11 @@ 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;
> + void *curr_elem = vmstate_next(head, field, size, i, false);
> int max_elems = n_elems - i;
>
> if (field->flags & VMS_ARRAY_OF_POINTER) {
> bool use_marker_field, is_null, use_dynamic_array;
> - void **array_elem = (void **)head + i;
> -
> - assert(array_elem);
> - curr_elem = *array_elem;
>
> is_null = !curr_elem;
>
> @@ -688,7 +683,8 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> use_vmdesc = true;
>
> for (int j = i + 1; j < n_elems; j++) {
> - void *elem = *(void **)(head + size * j);
> + void *elem = vmstate_next(head, field, size, j,
> + false);
> bool elem_is_null = !elem;
>
> if (is_null != elem_is_null) {
> @@ -713,8 +709,6 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
>
> save_field = !!curr_elem;
> }
> - } else {
> - curr_elem = head + size * i;
> }
>
> if (save_field) {
> --
> 2.51.0
>
--
Peter Xu
Peter Xu <peterx@redhat.com> writes:
> On Tue, Mar 24, 2026 at 04:43:27PM -0300, Fabiano Rosas wrote:
>> Similarly to vmstate_first(), introduce a vmstate_next(), which does
>> the necessary dereferencing of pointers to get to the leaf element. On
>> the load side, allow the caller to pass a flag indicating whether
>> allocation is expected and call the allocation function if so.
>>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>> migration/vmstate.c | 84 +++++++++++++++++++++------------------------
>> 1 file changed, 39 insertions(+), 45 deletions(-)
>>
>> diff --git a/migration/vmstate.c b/migration/vmstate.c
>> index ab7c6fa4ab..c1ad0ef9a5 100644
>> --- a/migration/vmstate.c
>> +++ b/migration/vmstate.c
>> @@ -127,24 +127,51 @@ static void *vmstate_first(void *opaque, const VMStateField *field,
>> return first;
>> }
>>
>> -static bool vmstate_ptr_marker_load(QEMUFile *f, bool *load_field,
>> - Error **errp)
>> +static void *vmstate_next(void **first, const VMStateField *field,
>> + int size, int i, bool alloc)
>> {
>> - int byte = qemu_get_byte(f);
>> + void **array_elem;
>> + void *next;
>> +
>> + if (!(field->flags & VMS_ARRAY_OF_POINTER)) {
>> + next = (void *)first + size * i;
>> + return next;
>> + }
>> +
>> + array_elem = first + i;
>> + next = *array_elem;
>> +
>> + if (alloc) {
>> + if (!next || field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC) {
>> + /*
>> + * NOTE: do not use vmstate_size() here, because we
>> + * need the object size, not entry size of the
>> + * array.
>> + */
>> + next = vmstate_handle_alloc(array_elem, field->size, 1);
>> + }
>> + }
>> + return next;
>> +}
>> +
>> +static bool vmstate_ptr_marker_load(QEMUFile *f, bool *load_field)
>> +{
>> + int byte = qemu_peek_byte(f, 0);
>
> Here, peeking the marker for ...
>
>>
>> if (byte == VMS_MARKER_PTR_NULL) {
>> /* When it's a null ptr marker, do not continue the load */
>> *load_field = false;
>> + qemu_file_skip(f, 1);
>> return true;
>> }
>>
>> if (byte == VMS_MARKER_PTR_VALID) {
>> /* We need to load this field right after the marker */
>> *load_field = true;
>> + qemu_file_skip(f, 1);
>> return true;
>> }
>>
>> - error_setg(errp, "Unexpected ptr marker: %d", byte);
>> return false;
>> }
>>
>> @@ -273,41 +300,13 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
>> void *curr_elem;
>>
>> if (field->flags & VMS_ARRAY_OF_POINTER) {
>
> ... every VMS_ARRAY_OF_POINTER case makes me feel uneasy.
>
There's 32k bytes in the buffer, this is likely to be just a
dereference. But fair enough.
> Note that we only have two use cases that both of them are certain on the
> upcoming byte to come:
>
> (1) for the null-only pointer, which is before this series introduced, both
> src/dst qemu knows 100% which ptr is NULL, so dest QEMU expects that 0x30
> for each null pointer.
>
> (2) for the new _AUTO_ALLOC introduced here, we also know exactly either
> 0x30 or 0x31 will come, and one of them must come before the real field
> dump.
>
> I think we may shoot us in the foot if we see 0x30 but in reality it's just
> the 1st byte of some array field's binary stream.
>
Yeah, that's a good point...
If you can put up with passing QEMUFile into vmstate_next(), we could
get the byte only when we're expecting it. Something like:
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 25fd9e52bc..16d9c1753f 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -137,7 +137,7 @@ static void *vmstate_first(void *opaque, const VMStateField *field,
}
static void *vmstate_next(void **first, const VMStateField *field,
- int size, int i, bool alloc)
+ int size, int i, QEMUFile *f)
{
void **array_elem;
void *next;
@@ -150,8 +150,11 @@ static void *vmstate_next(void **first, const VMStateField *field,
array_elem = first + i;
next = *array_elem;
- if (alloc) {
- if (!next || field->flags & VMS_ALLOC) {
+ if (f && (!next || field->flags & VMS_ALLOC)) {
+ bool alloc, ok;
+
+ ok = vmstate_ptr_marker_load(f, &alloc);
+ if (ok & alloc) {
/*
* NOTE: do not use vmstate_size() here, because we
* need the object size, not entry size of the
@@ -315,18 +318,11 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
!(field->flags & VMS_ARRAY_OF_POINTER));
for (i = 0; i < n_elems; i++) {
- /* If we will process the load of field? */
- bool load_field = true;
void *curr_elem;
- if (field->flags & VMS_ARRAY_OF_POINTER) {
- /* Peek a possible pointer marker instead of VMSD first */
- ok = vmstate_ptr_marker_load(f, &load_field);
- }
-
- curr_elem = vmstate_next(head, field, size, i, load_field && ok);
+ curr_elem = vmstate_next(head, field, size, i, f);
- if (load_field) {
+ if (curr_elem) {
ok = vmstate_load_field(f, curr_elem, size, field, errp);
}
@@ -673,7 +669,7 @@ 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 = vmstate_next(head, field, size, i, false);
+ void *curr_elem = vmstate_next(head, field, size, i, NULL);
int max_elems = n_elems - i;
if (field->flags & VMS_ARRAY_OF_POINTER) {
@@ -711,8 +707,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
use_vmdesc = true;
for (int j = i + 1; j < n_elems; j++) {
- void *elem = vmstate_next(head, field, size, j,
- false);
+ void *elem = vmstate_next(head, field, size, j, NULL);
bool elem_is_null = !elem;
if (is_null != elem_is_null) {
--
2.51.0
© 2016 - 2026 Red Hat, Inc.