* [RFC PATCH v1 01/17] vmstate: fixup the use of AUTO_ALLOC flag
2026-03-24 19:43 [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Fabiano Rosas
@ 2026-03-24 19:43 ` Fabiano Rosas
2026-03-25 16:18 ` Peter Xu
2026-03-24 19:43 ` [RFC PATCH v1 02/17] vmstate: Remove vmstate_use_marker_field Fabiano Rosas
` (16 subsequent siblings)
17 siblings, 1 reply; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-24 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Alexander Mikhalitsyn, Juraj Marcin
Please add this fixup this somewhere
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/savevm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 34223de818..db0721996c 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -869,7 +869,7 @@ static void vmstate_check(const VMStateDescription *vmsd)
if (field) {
while (field->name) {
if (field->flags & VMS_ARRAY_OF_POINTER) {
- if (VMS_ARRAY_OF_POINTER_AUTO_ALLOC) {
+ if (field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC) {
/*
* Size must be provided because dest QEMU needs that
* info to know what to allocate
--
2.51.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [RFC PATCH v1 01/17] vmstate: fixup the use of AUTO_ALLOC flag
2026-03-24 19:43 ` [RFC PATCH v1 01/17] vmstate: fixup the use of AUTO_ALLOC flag Fabiano Rosas
@ 2026-03-25 16:18 ` Peter Xu
0 siblings, 0 replies; 31+ messages in thread
From: Peter Xu @ 2026-03-25 16:18 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Alexander Mikhalitsyn, Juraj Marcin
On Tue, Mar 24, 2026 at 04:43:16PM -0300, Fabiano Rosas wrote:
> Please add this fixup this somewhere
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> migration/savevm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 34223de818..db0721996c 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -869,7 +869,7 @@ static void vmstate_check(const VMStateDescription *vmsd)
> if (field) {
> while (field->name) {
> if (field->flags & VMS_ARRAY_OF_POINTER) {
> - if (VMS_ARRAY_OF_POINTER_AUTO_ALLOC) {
> + if (field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC) {
Oops.. I'll squash this one, thanks a lot.
> /*
> * Size must be provided because dest QEMU needs that
> * info to know what to allocate
> --
> 2.51.0
>
--
Peter Xu
^ permalink raw reply [flat|nested] 31+ messages in thread
* [RFC PATCH v1 02/17] vmstate: Remove vmstate_use_marker_field
2026-03-24 19:43 [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Fabiano Rosas
2026-03-24 19:43 ` [RFC PATCH v1 01/17] vmstate: fixup the use of AUTO_ALLOC flag Fabiano Rosas
@ 2026-03-24 19:43 ` Fabiano Rosas
2026-03-25 16:37 ` Peter Xu
2026-03-24 19:43 ` [RFC PATCH v1 03/17] vmstate: Stop checking size for nullptr compression Fabiano Rosas
` (15 subsequent siblings)
17 siblings, 1 reply; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-24 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Alexander Mikhalitsyn, Juraj Marcin
Bring back is_null, but set use_marker_field based on it. This makes
it clear that the compression logic is only because of the null
marker. When the marker is not null, it doesn't make sense to
compress.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/vmstate.c | 36 ++++++++++--------------------------
1 file changed, 10 insertions(+), 26 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 7d7d9c7e18..7edfa3d990 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -240,22 +240,6 @@ static bool vmstate_post_load(const VMStateDescription *vmsd,
return true;
}
-
-/*
- * If we will use a ptr marker in the stream for a field? Two use cases:
- *
- * (1) When used with VMS_ARRAY_OF_POINTER_ALLOW_NULL, it must always be
- * present to imply the population status of the pointer.
- *
- * (2) When used with normal VMSD array fields, only emit a null ptr marker
- * if the pointer is NULL.
- */
-static inline bool
-vmstate_use_marker_field(void *ptr, int size, bool dynamic_array)
-{
- return (!ptr && size) || dynamic_array;
-}
-
bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque, int version_id, Error **errp)
{
@@ -318,8 +302,8 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
curr_elem = *(void **)curr_elem_p;
}
- use_marker_field = vmstate_use_marker_field(curr_elem, size,
- use_dynamic_array);
+ use_marker_field = use_dynamic_array || (!curr_elem && size);
+
if (use_marker_field) {
/* Read the marker instead of VMSD first */
if (!vmstate_ptr_marker_load(f, &load_field, errp)) {
@@ -634,7 +618,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
int i, n_elems = vmstate_n_elems(opaque, field);
int size = vmstate_size(opaque, field);
JSONWriter *vmdesc_loop = vmdesc;
- bool use_marker_field_prev = false;
+ bool is_null_prev = false;
/*
* When this is enabled, it means we will always push a ptr
* marker first for each element saying if it's populated.
@@ -651,7 +635,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
for (i = 0; i < n_elems; i++) {
void *curr_elem = first_elem + size * i;
const VMStateField *inner_field;
- bool use_marker_field;
+ bool use_marker_field, is_null;
int max_elems = n_elems - i;
if (field->flags & VMS_ARRAY_OF_POINTER) {
@@ -659,8 +643,8 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
curr_elem = *(void **)curr_elem;
}
- use_marker_field = vmstate_use_marker_field(curr_elem, size,
- use_dynamic_array);
+ is_null = (!curr_elem && size);
+ use_marker_field = use_dynamic_array || is_null;
if (use_marker_field) {
inner_field = vmsd_create_ptr_marker_field(field);
@@ -681,16 +665,16 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
*/
if (vmdesc && vmsd_can_compress(field) &&
(field->flags & VMS_ARRAY_OF_POINTER) &&
- use_marker_field != use_marker_field_prev) {
+ is_null != is_null_prev) {
- use_marker_field_prev = use_marker_field;
+ is_null_prev = is_null;
vmdesc_loop = vmdesc;
for (int j = i + 1; j < n_elems; j++) {
void *elem = *(void **)(first_elem + size * j);
- bool elem_use_marker_field = !elem && size;
+ bool elem_is_null = !elem && size;
- if (use_marker_field != elem_use_marker_field) {
+ if (is_null != elem_is_null) {
max_elems = j - i;
break;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [RFC PATCH v1 02/17] vmstate: Remove vmstate_use_marker_field
2026-03-24 19:43 ` [RFC PATCH v1 02/17] vmstate: Remove vmstate_use_marker_field Fabiano Rosas
@ 2026-03-25 16:37 ` Peter Xu
2026-03-25 17:51 ` Fabiano Rosas
0 siblings, 1 reply; 31+ messages in thread
From: Peter Xu @ 2026-03-25 16:37 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Alexander Mikhalitsyn, Juraj Marcin
On Tue, Mar 24, 2026 at 04:43:17PM -0300, Fabiano Rosas wrote:
> Bring back is_null, but set use_marker_field based on it. This makes
> it clear that the compression logic is only because of the null
> marker. When the marker is not null, it doesn't make sense to
> compress.
Yeah I see where this came from, I agree the is_null case is at least
harder to follow now when without that variable being around.
I'll squash this into my commit for now, maybe I'll still try to keep the
comments somewhere.
Then I'll also hope we can remove the whole JSON blob very soon. Do you
plan to look into the "vmsd walker rewrite analyze-migration script" plan a
bit? Let me know if you want me to try; it'll not happen immediately, but
after seeing this I'm more eager to try removing those..
When those are ready, we should be able to remove is_null again.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> migration/vmstate.c | 36 ++++++++++--------------------------
> 1 file changed, 10 insertions(+), 26 deletions(-)
>
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index 7d7d9c7e18..7edfa3d990 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -240,22 +240,6 @@ static bool vmstate_post_load(const VMStateDescription *vmsd,
> return true;
> }
>
> -
> -/*
> - * If we will use a ptr marker in the stream for a field? Two use cases:
> - *
> - * (1) When used with VMS_ARRAY_OF_POINTER_ALLOW_NULL, it must always be
> - * present to imply the population status of the pointer.
> - *
> - * (2) When used with normal VMSD array fields, only emit a null ptr marker
> - * if the pointer is NULL.
> - */
> -static inline bool
> -vmstate_use_marker_field(void *ptr, int size, bool dynamic_array)
> -{
> - return (!ptr && size) || dynamic_array;
> -}
> -
> bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
> void *opaque, int version_id, Error **errp)
> {
> @@ -318,8 +302,8 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
> curr_elem = *(void **)curr_elem_p;
> }
>
> - use_marker_field = vmstate_use_marker_field(curr_elem, size,
> - use_dynamic_array);
> + use_marker_field = use_dynamic_array || (!curr_elem && size);
> +
> if (use_marker_field) {
> /* Read the marker instead of VMSD first */
> if (!vmstate_ptr_marker_load(f, &load_field, errp)) {
> @@ -634,7 +618,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> int i, n_elems = vmstate_n_elems(opaque, field);
> int size = vmstate_size(opaque, field);
> JSONWriter *vmdesc_loop = vmdesc;
> - bool use_marker_field_prev = false;
> + bool is_null_prev = false;
> /*
> * When this is enabled, it means we will always push a ptr
> * marker first for each element saying if it's populated.
> @@ -651,7 +635,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> for (i = 0; i < n_elems; i++) {
> void *curr_elem = first_elem + size * i;
> const VMStateField *inner_field;
> - bool use_marker_field;
> + bool use_marker_field, is_null;
> int max_elems = n_elems - i;
>
> if (field->flags & VMS_ARRAY_OF_POINTER) {
> @@ -659,8 +643,8 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> curr_elem = *(void **)curr_elem;
> }
>
> - use_marker_field = vmstate_use_marker_field(curr_elem, size,
> - use_dynamic_array);
> + is_null = (!curr_elem && size);
> + use_marker_field = use_dynamic_array || is_null;
>
> if (use_marker_field) {
> inner_field = vmsd_create_ptr_marker_field(field);
> @@ -681,16 +665,16 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> */
> if (vmdesc && vmsd_can_compress(field) &&
> (field->flags & VMS_ARRAY_OF_POINTER) &&
> - use_marker_field != use_marker_field_prev) {
> + is_null != is_null_prev) {
>
> - use_marker_field_prev = use_marker_field;
> + is_null_prev = is_null;
> vmdesc_loop = vmdesc;
>
> for (int j = i + 1; j < n_elems; j++) {
> void *elem = *(void **)(first_elem + size * j);
> - bool elem_use_marker_field = !elem && size;
> + bool elem_is_null = !elem && size;
>
> - if (use_marker_field != elem_use_marker_field) {
> + if (is_null != elem_is_null) {
> max_elems = j - i;
> break;
> }
> --
> 2.51.0
>
--
Peter Xu
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [RFC PATCH v1 02/17] vmstate: Remove vmstate_use_marker_field
2026-03-25 16:37 ` Peter Xu
@ 2026-03-25 17:51 ` Fabiano Rosas
0 siblings, 0 replies; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-25 17:51 UTC (permalink / raw)
To: Peter Xu; +Cc: qemu-devel, Alexander Mikhalitsyn, Juraj Marcin
Peter Xu <peterx@redhat.com> writes:
> On Tue, Mar 24, 2026 at 04:43:17PM -0300, Fabiano Rosas wrote:
>> Bring back is_null, but set use_marker_field based on it. This makes
>> it clear that the compression logic is only because of the null
>> marker. When the marker is not null, it doesn't make sense to
>> compress.
>
> Yeah I see where this came from, I agree the is_null case is at least
> harder to follow now when without that variable being around.
>
> I'll squash this into my commit for now, maybe I'll still try to keep the
> comments somewhere.
>
> Then I'll also hope we can remove the whole JSON blob very soon. Do you
> plan to look into the "vmsd walker rewrite analyze-migration script" plan a
> bit? Let me know if you want me to try; it'll not happen immediately, but
> after seeing this I'm more eager to try removing those..
>
I don't think I'll have time to work on it properly. Actually, I have
the feeling this is something that will take some effort. We may need to
make little steps of progress towards it. If you have the opportunity to
work on something, go ahead.
The first step might be to stop emitting the JSON where
appropriate.
> When those are ready, we should be able to remove is_null again.
>
I'm also considering whether we really need the compression logic at
all. If we remove the blob from the stream and make it a separate
command then we could simply just output the entire array of zeroes and
let external tools deal with it. I bet jq has a way to display that data
nicely.
>>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>> migration/vmstate.c | 36 ++++++++++--------------------------
>> 1 file changed, 10 insertions(+), 26 deletions(-)
>>
>> diff --git a/migration/vmstate.c b/migration/vmstate.c
>> index 7d7d9c7e18..7edfa3d990 100644
>> --- a/migration/vmstate.c
>> +++ b/migration/vmstate.c
>> @@ -240,22 +240,6 @@ static bool vmstate_post_load(const VMStateDescription *vmsd,
>> return true;
>> }
>>
>> -
>> -/*
>> - * If we will use a ptr marker in the stream for a field? Two use cases:
>> - *
>> - * (1) When used with VMS_ARRAY_OF_POINTER_ALLOW_NULL, it must always be
>> - * present to imply the population status of the pointer.
>> - *
>> - * (2) When used with normal VMSD array fields, only emit a null ptr marker
>> - * if the pointer is NULL.
>> - */
>> -static inline bool
>> -vmstate_use_marker_field(void *ptr, int size, bool dynamic_array)
>> -{
>> - return (!ptr && size) || dynamic_array;
>> -}
>> -
>> bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
>> void *opaque, int version_id, Error **errp)
>> {
>> @@ -318,8 +302,8 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
>> curr_elem = *(void **)curr_elem_p;
>> }
>>
>> - use_marker_field = vmstate_use_marker_field(curr_elem, size,
>> - use_dynamic_array);
>> + use_marker_field = use_dynamic_array || (!curr_elem && size);
>> +
>> if (use_marker_field) {
>> /* Read the marker instead of VMSD first */
>> if (!vmstate_ptr_marker_load(f, &load_field, errp)) {
>> @@ -634,7 +618,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
>> int i, n_elems = vmstate_n_elems(opaque, field);
>> int size = vmstate_size(opaque, field);
>> JSONWriter *vmdesc_loop = vmdesc;
>> - bool use_marker_field_prev = false;
>> + bool is_null_prev = false;
>> /*
>> * When this is enabled, it means we will always push a ptr
>> * marker first for each element saying if it's populated.
>> @@ -651,7 +635,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
>> for (i = 0; i < n_elems; i++) {
>> void *curr_elem = first_elem + size * i;
>> const VMStateField *inner_field;
>> - bool use_marker_field;
>> + bool use_marker_field, is_null;
>> int max_elems = n_elems - i;
>>
>> if (field->flags & VMS_ARRAY_OF_POINTER) {
>> @@ -659,8 +643,8 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
>> curr_elem = *(void **)curr_elem;
>> }
>>
>> - use_marker_field = vmstate_use_marker_field(curr_elem, size,
>> - use_dynamic_array);
>> + is_null = (!curr_elem && size);
>> + use_marker_field = use_dynamic_array || is_null;
>>
>> if (use_marker_field) {
>> inner_field = vmsd_create_ptr_marker_field(field);
>> @@ -681,16 +665,16 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
>> */
>> if (vmdesc && vmsd_can_compress(field) &&
>> (field->flags & VMS_ARRAY_OF_POINTER) &&
>> - use_marker_field != use_marker_field_prev) {
>> + is_null != is_null_prev) {
>>
>> - use_marker_field_prev = use_marker_field;
>> + is_null_prev = is_null;
>> vmdesc_loop = vmdesc;
>>
>> for (int j = i + 1; j < n_elems; j++) {
>> void *elem = *(void **)(first_elem + size * j);
>> - bool elem_use_marker_field = !elem && size;
>> + bool elem_is_null = !elem && size;
>>
>> - if (use_marker_field != elem_use_marker_field) {
>> + if (is_null != elem_is_null) {
>> max_elems = j - i;
>> break;
>> }
>> --
>> 2.51.0
>>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [RFC PATCH v1 03/17] vmstate: Stop checking size for nullptr compression
2026-03-24 19:43 [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Fabiano Rosas
2026-03-24 19:43 ` [RFC PATCH v1 01/17] vmstate: fixup the use of AUTO_ALLOC flag Fabiano Rosas
2026-03-24 19:43 ` [RFC PATCH v1 02/17] vmstate: Remove vmstate_use_marker_field Fabiano Rosas
@ 2026-03-24 19:43 ` Fabiano Rosas
2026-03-24 19:43 ` [RFC PATCH v1 04/17] vmstate: Set error inside of vmstate_save_field_with_vmdesc Fabiano Rosas
` (14 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-24 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Alexander Mikhalitsyn, Juraj Marcin
The NULL pointer marker code applies only to VMS_ARRAY_OF_POINTER,
where the size is never NULL. Move the setting of is_null under
VMS_ARRAY_OF_POINTER, so we can stop checking the size.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
AFAICS, the size actually should never be NULL, but there are a few
vmsds doing a hack with VMS_SINGLE. I can't be bothered to learn about
it right now.
---
migration/vmstate.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 7edfa3d990..d7b1bc6b86 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -635,15 +635,15 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
for (i = 0; i < n_elems; i++) {
void *curr_elem = first_elem + size * i;
const VMStateField *inner_field;
- bool use_marker_field, is_null;
+ bool use_marker_field, is_null = false;
int max_elems = n_elems - i;
if (field->flags & VMS_ARRAY_OF_POINTER) {
assert(curr_elem);
curr_elem = *(void **)curr_elem;
+ is_null = !curr_elem;
}
- is_null = (!curr_elem && size);
use_marker_field = use_dynamic_array || is_null;
if (use_marker_field) {
@@ -672,7 +672,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
for (int j = i + 1; j < n_elems; j++) {
void *elem = *(void **)(first_elem + size * j);
- bool elem_is_null = !elem && size;
+ bool elem_is_null = !elem;
if (is_null != elem_is_null) {
max_elems = j - i;
--
2.51.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [RFC PATCH v1 04/17] vmstate: Set error inside of vmstate_save_field_with_vmdesc
2026-03-24 19:43 [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Fabiano Rosas
` (2 preceding siblings ...)
2026-03-24 19:43 ` [RFC PATCH v1 03/17] vmstate: Stop checking size for nullptr compression Fabiano Rosas
@ 2026-03-24 19:43 ` Fabiano Rosas
2026-03-24 19:43 ` [RFC PATCH v1 05/17] vmstate: Remove vmdesc_loop Fabiano Rosas
` (13 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-24 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Alexander Mikhalitsyn, Juraj Marcin
This reduces duplication and keeps the top level less visually
polluted.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/vmstate.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index d7b1bc6b86..dec9cf920b 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -586,6 +586,11 @@ vmstate_save_field_with_vmdesc(QEMUFile *f, void *pv, size_t size,
vmsd_desc_field_end(vmsd, vmdesc, field, written_bytes);
+ if (!ok) {
+ error_prepend(errp, "Save of field %s/%s failed: ",
+ vmsd->name, field->name);
+ }
+
return ok;
}
@@ -691,8 +696,6 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
}
if (!ok) {
- error_prepend(errp, "Save of field %s/%s failed: ",
- vmsd->name, field->name);
goto out;
}
@@ -709,10 +712,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
field->size, vmsd,
field, vmdesc_loop,
i, max_elems, errp);
-
if (!ok) {
- error_prepend(errp, "Save of field %s/%s failed: ",
- vmsd->name, field->name);
goto out;
}
}
--
2.51.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [RFC PATCH v1 05/17] vmstate: Remove vmdesc_loop
2026-03-24 19:43 [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Fabiano Rosas
` (3 preceding siblings ...)
2026-03-24 19:43 ` [RFC PATCH v1 04/17] vmstate: Set error inside of vmstate_save_field_with_vmdesc Fabiano Rosas
@ 2026-03-24 19:43 ` Fabiano Rosas
2026-03-25 17:07 ` Peter Xu
2026-03-24 19:43 ` [RFC PATCH v1 06/17] vmstate: Put array of pointers code together Fabiano Rosas
` (12 subsequent siblings)
17 siblings, 1 reply; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-24 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Alexander Mikhalitsyn, Juraj Marcin
The vmdesc_loop variable is being used as a hacky way of writing the
JSON description only for the first element of a compressed
array. There are also a few implicit uses, such as writing the JSON
for the first non-compressed element after a compressed portion.
Future work will have trouble with this. Use a simple boolean to be
explicit now.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/vmstate.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index dec9cf920b..7a12245d36 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -622,8 +622,9 @@ static bool vmstate_save_vmsd_v(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);
- JSONWriter *vmdesc_loop = vmdesc;
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.
@@ -673,7 +674,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
is_null != is_null_prev) {
is_null_prev = is_null;
- vmdesc_loop = vmdesc;
+ use_vmdesc = true;
for (int j = i + 1; j < n_elems; j++) {
void *elem = *(void **)(first_elem + size * j);
@@ -686,8 +687,13 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
}
}
+ if (use_dynamic_array) {
+ use_vmdesc = true;
+ }
+
ok = vmstate_save_field_with_vmdesc(f, curr_elem, size, vmsd,
- inner_field, vmdesc_loop,
+ inner_field,
+ use_vmdesc ? vmdesc : NULL,
i, max_elems, errp);
/* If we used a fake temp field.. free it now */
@@ -708,19 +714,16 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
* NOTE: do not use vmstate_size() here because we want
* to dump the real VMSD object now.
*/
- ok = vmstate_save_field_with_vmdesc(f, curr_elem,
- field->size, vmsd,
- field, vmdesc_loop,
- i, max_elems, errp);
+ ok = vmstate_save_field_with_vmdesc(
+ f, curr_elem, field->size, vmsd, field,
+ use_vmdesc ? vmdesc : NULL, i, max_elems, errp);
+
if (!ok) {
goto out;
}
}
- /* Compressed arrays only care about the first element */
- if (vmdesc_loop && vmsd_can_compress(field)) {
- vmdesc_loop = NULL;
- }
+ use_vmdesc = false;
}
} else {
if (field->flags & VMS_MUST_EXIST) {
--
2.51.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [RFC PATCH v1 05/17] vmstate: Remove vmdesc_loop
2026-03-24 19:43 ` [RFC PATCH v1 05/17] vmstate: Remove vmdesc_loop Fabiano Rosas
@ 2026-03-25 17:07 ` Peter Xu
2026-03-25 18:11 ` Fabiano Rosas
0 siblings, 1 reply; 31+ messages in thread
From: Peter Xu @ 2026-03-25 17:07 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Alexander Mikhalitsyn, Juraj Marcin
On Tue, Mar 24, 2026 at 04:43:20PM -0300, Fabiano Rosas wrote:
> The vmdesc_loop variable is being used as a hacky way of writing the
> JSON description only for the first element of a compressed
> array. There are also a few implicit uses, such as writing the JSON
> for the first non-compressed element after a compressed portion.
>
> Future work will have trouble with this. Use a simple boolean to be
> explicit now.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> migration/vmstate.c | 25 ++++++++++++++-----------
> 1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index dec9cf920b..7a12245d36 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -622,8 +622,9 @@ static bool vmstate_save_vmsd_v(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);
> - JSONWriter *vmdesc_loop = vmdesc;
> 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.
> @@ -673,7 +674,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> is_null != is_null_prev) {
>
> is_null_prev = is_null;
> - vmdesc_loop = vmdesc;
> + use_vmdesc = true;
>
> for (int j = i + 1; j < n_elems; j++) {
> void *elem = *(void **)(first_elem + size * j);
> @@ -686,8 +687,13 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> }
> }
>
> + if (use_dynamic_array) {
> + use_vmdesc = true;
> + }
[1]
> +
> ok = vmstate_save_field_with_vmdesc(f, curr_elem, size, vmsd,
> - inner_field, vmdesc_loop,
> + inner_field,
> + use_vmdesc ? vmdesc : NULL,
> i, max_elems, errp);
>
> /* If we used a fake temp field.. free it now */
> @@ -708,19 +714,16 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> * NOTE: do not use vmstate_size() here because we want
> * to dump the real VMSD object now.
> */
> - ok = vmstate_save_field_with_vmdesc(f, curr_elem,
> - field->size, vmsd,
> - field, vmdesc_loop,
> - i, max_elems, errp);
> + ok = vmstate_save_field_with_vmdesc(
> + f, curr_elem, field->size, vmsd, field,
> + use_vmdesc ? vmdesc : NULL, i, max_elems, errp);
> +
> if (!ok) {
> goto out;
> }
> }
>
> - /* Compressed arrays only care about the first element */
> - if (vmdesc_loop && vmsd_can_compress(field)) {
> - vmdesc_loop = NULL;
> - }
> + use_vmdesc = false;
Hmm...
vmsd_can_compress() can return arbitrary things depending on the field
itself, now we always do the dedup almost for everything.
The patch did add above [1] to make AUTO_ALLOC be ok, by enforce setting
TRUE for it for every loop, but what about other cases where
vmsd_can_compress() may return false (where the field does not support
compression)?
> }
> } else {
> if (field->flags & VMS_MUST_EXIST) {
> --
> 2.51.0
>
--
Peter Xu
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [RFC PATCH v1 05/17] vmstate: Remove vmdesc_loop
2026-03-25 17:07 ` Peter Xu
@ 2026-03-25 18:11 ` Fabiano Rosas
2026-03-25 21:43 ` Peter Xu
0 siblings, 1 reply; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-25 18:11 UTC (permalink / raw)
To: Peter Xu; +Cc: qemu-devel, Alexander Mikhalitsyn, Juraj Marcin
Peter Xu <peterx@redhat.com> writes:
> On Tue, Mar 24, 2026 at 04:43:20PM -0300, Fabiano Rosas wrote:
>> The vmdesc_loop variable is being used as a hacky way of writing the
>> JSON description only for the first element of a compressed
>> array. There are also a few implicit uses, such as writing the JSON
>> for the first non-compressed element after a compressed portion.
>>
>> Future work will have trouble with this. Use a simple boolean to be
>> explicit now.
>>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>> migration/vmstate.c | 25 ++++++++++++++-----------
>> 1 file changed, 14 insertions(+), 11 deletions(-)
>>
>> diff --git a/migration/vmstate.c b/migration/vmstate.c
>> index dec9cf920b..7a12245d36 100644
>> --- a/migration/vmstate.c
>> +++ b/migration/vmstate.c
>> @@ -622,8 +622,9 @@ static bool vmstate_save_vmsd_v(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);
>> - JSONWriter *vmdesc_loop = vmdesc;
>> 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.
>> @@ -673,7 +674,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
>> is_null != is_null_prev) {
>>
>> is_null_prev = is_null;
>> - vmdesc_loop = vmdesc;
>> + use_vmdesc = true;
>>
>> for (int j = i + 1; j < n_elems; j++) {
>> void *elem = *(void **)(first_elem + size * j);
>> @@ -686,8 +687,13 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
>> }
>> }
>>
>> + if (use_dynamic_array) {
>> + use_vmdesc = true;
>> + }
>
> [1]
>
>> +
>> ok = vmstate_save_field_with_vmdesc(f, curr_elem, size, vmsd,
>> - inner_field, vmdesc_loop,
>> + inner_field,
>> + use_vmdesc ? vmdesc : NULL,
>> i, max_elems, errp);
>>
>> /* If we used a fake temp field.. free it now */
>> @@ -708,19 +714,16 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
>> * NOTE: do not use vmstate_size() here because we want
>> * to dump the real VMSD object now.
>> */
>> - ok = vmstate_save_field_with_vmdesc(f, curr_elem,
>> - field->size, vmsd,
>> - field, vmdesc_loop,
>> - i, max_elems, errp);
>> + ok = vmstate_save_field_with_vmdesc(
>> + f, curr_elem, field->size, vmsd, field,
>> + use_vmdesc ? vmdesc : NULL, i, max_elems, errp);
>> +
>> if (!ok) {
>> goto out;
>> }
>> }
>>
>> - /* Compressed arrays only care about the first element */
>> - if (vmdesc_loop && vmsd_can_compress(field)) {
>> - vmdesc_loop = NULL;
>> - }
>> + use_vmdesc = false;
>
> Hmm...
>
> vmsd_can_compress() can return arbitrary things depending on the field
> itself, now we always do the dedup almost for everything.
>
> The patch did add above [1] to make AUTO_ALLOC be ok, by enforce setting
> TRUE for it for every loop, but what about other cases where
> vmsd_can_compress() may return false (where the field does not support
> compression)?
>
Hmm, I think you're right. But what I'm doing by setting
use_vmdesc=false here is just clearing it for the next iteration, where
it should be set according to necessity. So I'd leave this down here,
but up there change to:
if (vmdesc && vmsd_can_compress(field)) {
if (is_null != is_null_prev) {
use_vmdesc = true;
...
}
- }
-
- if (use_dynamic_array) {
+ } else {
use_vmdesc = true;
}
In words: all fields should appear in the JSON, unless its part of the
NULL portion of a compressed array. For compressed arrays only the first
element appears in the JSON. If a stream of NULLs is followed by a
non-NULL member, then that member should appear in the JSON after the
compressed entry.
These two catch all these corner-cases I think:
PYTHON=$(pwd)/pyvenv/bin/python3.11
QTEST_QEMU_BINARY=./qemu-system-s390x ./tests/qtest/migration-test -p
/s390x/migration/analyze-script
PYTHON=$(pwd)/pyvenv/bin/python3.11
QTEST_QEMU_BINARY=./qemu-system-ppc64 ./tests/qtest/migration-test-p
/ppc64/migration/analyze-script
>> }
>> } else {
>> if (field->flags & VMS_MUST_EXIST) {
>> --
>> 2.51.0
>>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [RFC PATCH v1 05/17] vmstate: Remove vmdesc_loop
2026-03-25 18:11 ` Fabiano Rosas
@ 2026-03-25 21:43 ` Peter Xu
0 siblings, 0 replies; 31+ messages in thread
From: Peter Xu @ 2026-03-25 21:43 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Alexander Mikhalitsyn, Juraj Marcin
On Wed, Mar 25, 2026 at 03:11:25PM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
>
> > On Tue, Mar 24, 2026 at 04:43:20PM -0300, Fabiano Rosas wrote:
> >> The vmdesc_loop variable is being used as a hacky way of writing the
> >> JSON description only for the first element of a compressed
> >> array. There are also a few implicit uses, such as writing the JSON
> >> for the first non-compressed element after a compressed portion.
> >>
> >> Future work will have trouble with this. Use a simple boolean to be
> >> explicit now.
> >>
> >> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> >> ---
> >> migration/vmstate.c | 25 ++++++++++++++-----------
> >> 1 file changed, 14 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/migration/vmstate.c b/migration/vmstate.c
> >> index dec9cf920b..7a12245d36 100644
> >> --- a/migration/vmstate.c
> >> +++ b/migration/vmstate.c
> >> @@ -622,8 +622,9 @@ static bool vmstate_save_vmsd_v(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);
> >> - JSONWriter *vmdesc_loop = vmdesc;
> >> 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.
> >> @@ -673,7 +674,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> >> is_null != is_null_prev) {
> >>
> >> is_null_prev = is_null;
> >> - vmdesc_loop = vmdesc;
> >> + use_vmdesc = true;
> >>
> >> for (int j = i + 1; j < n_elems; j++) {
> >> void *elem = *(void **)(first_elem + size * j);
> >> @@ -686,8 +687,13 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> >> }
> >> }
> >>
> >> + if (use_dynamic_array) {
> >> + use_vmdesc = true;
> >> + }
> >
> > [1]
> >
> >> +
> >> ok = vmstate_save_field_with_vmdesc(f, curr_elem, size, vmsd,
> >> - inner_field, vmdesc_loop,
> >> + inner_field,
> >> + use_vmdesc ? vmdesc : NULL,
> >> i, max_elems, errp);
> >>
> >> /* If we used a fake temp field.. free it now */
> >> @@ -708,19 +714,16 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> >> * NOTE: do not use vmstate_size() here because we want
> >> * to dump the real VMSD object now.
> >> */
> >> - ok = vmstate_save_field_with_vmdesc(f, curr_elem,
> >> - field->size, vmsd,
> >> - field, vmdesc_loop,
> >> - i, max_elems, errp);
> >> + ok = vmstate_save_field_with_vmdesc(
> >> + f, curr_elem, field->size, vmsd, field,
> >> + use_vmdesc ? vmdesc : NULL, i, max_elems, errp);
> >> +
> >> if (!ok) {
> >> goto out;
> >> }
> >> }
> >>
> >> - /* Compressed arrays only care about the first element */
> >> - if (vmdesc_loop && vmsd_can_compress(field)) {
> >> - vmdesc_loop = NULL;
> >> - }
> >> + use_vmdesc = false;
> >
> > Hmm...
> >
> > vmsd_can_compress() can return arbitrary things depending on the field
> > itself, now we always do the dedup almost for everything.
> >
> > The patch did add above [1] to make AUTO_ALLOC be ok, by enforce setting
> > TRUE for it for every loop, but what about other cases where
> > vmsd_can_compress() may return false (where the field does not support
> > compression)?
> >
>
> Hmm, I think you're right. But what I'm doing by setting
> use_vmdesc=false here is just clearing it for the next iteration, where
> it should be set according to necessity. So I'd leave this down here,
> but up there change to:
>
> if (vmdesc && vmsd_can_compress(field)) {
> if (is_null != is_null_prev) {
> use_vmdesc = true;
> ...
> }
> - }
> -
> - if (use_dynamic_array) {
> + } else {
> use_vmdesc = true;
> }
I think this may still cause some confusion and inefficiency.
For example, for uncompressed fields, we'll update this field twice for
each of the elements (setting true here, clearing to false at the end).
The old approach looks still better in that it only flips the pointer
whenever needed. Said that, it doesn't always need to be a pointer, we can
still use a bool.
>
> In words: all fields should appear in the JSON, unless its part of the
> NULL portion of a compressed array. For compressed arrays only the first
> element appears in the JSON. If a stream of NULLs is followed by a
> non-NULL member, then that member should appear in the JSON after the
> compressed entry.
>
> These two catch all these corner-cases I think:
>
> PYTHON=$(pwd)/pyvenv/bin/python3.11
> QTEST_QEMU_BINARY=./qemu-system-s390x ./tests/qtest/migration-test -p
> /s390x/migration/analyze-script
>
> PYTHON=$(pwd)/pyvenv/bin/python3.11
> QTEST_QEMU_BINARY=./qemu-system-ppc64 ./tests/qtest/migration-test-p
> /ppc64/migration/analyze-script
>
> >> }
> >> } else {
> >> if (field->flags & VMS_MUST_EXIST) {
> >> --
> >> 2.51.0
> >>
>
--
Peter Xu
^ permalink raw reply [flat|nested] 31+ messages in thread
* [RFC PATCH v1 06/17] vmstate: Put array of pointers code together
2026-03-24 19:43 [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Fabiano Rosas
` (4 preceding siblings ...)
2026-03-24 19:43 ` [RFC PATCH v1 05/17] vmstate: Remove vmdesc_loop Fabiano Rosas
@ 2026-03-24 19:43 ` Fabiano Rosas
2026-03-24 19:43 ` [RFC PATCH v1 07/17] vmstate: Create and save ptr marker in same function Fabiano Rosas
` (11 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-24 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Alexander Mikhalitsyn, Juraj Marcin
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
^ permalink raw reply related [flat|nested] 31+ messages in thread* [RFC PATCH v1 07/17] vmstate: Create and save ptr marker in same function
2026-03-24 19:43 [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Fabiano Rosas
` (5 preceding siblings ...)
2026-03-24 19:43 ` [RFC PATCH v1 06/17] vmstate: Put array of pointers code together Fabiano Rosas
@ 2026-03-24 19:43 ` Fabiano Rosas
2026-03-24 19:43 ` [RFC PATCH v1 08/17] vmstate: Don't recompute size and n_elems in vmstate_size Fabiano Rosas
` (10 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-24 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Alexander Mikhalitsyn, Juraj Marcin
Move the creation of the pointer marker along with the vmstate_save
call that writes it to the stream.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/vmstate.c | 78 ++++++++++++++++++++++-----------------------
1 file changed, 39 insertions(+), 39 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index a190c3f63f..0975d10793 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -54,36 +54,6 @@ vmstate_field_exists(const VMStateDescription *vmsd, const VMStateField *field,
return result;
}
-/*
- * Create a ptr marker field when there's a NULL pointer detected in the
- * array of a VMS_ARRAY_OF_POINTER VMSD field. It's needed because we
- * can't dereference the NULL pointer.
- */
-static const VMStateField *
-vmsd_create_ptr_marker_field(const VMStateField *field)
-{
- VMStateField *fake = g_new0(VMStateField, 1);
-
- /* It can only happen on an array of pointers! */
- assert(field->flags & VMS_ARRAY_OF_POINTER);
-
- /* Some of fake's properties should match the original's */
- fake->name = field->name;
- fake->version_id = field->version_id;
-
- /* Do not need "field_exists" check as it always exists (which is null) */
- fake->field_exists = NULL;
-
- /* See vmstate_info_nullptr - use 1 byte to represent nullptr */
- fake->size = 1;
- fake->info = &vmstate_info_ptr_marker;
- fake->flags = VMS_SINGLE;
-
- /* All the rest fields shouldn't matter.. */
-
- return (const VMStateField *)fake;
-}
-
static int vmstate_n_elems(void *opaque, const VMStateField *field)
{
int n_elems = 1;
@@ -592,6 +562,42 @@ vmstate_save_field_with_vmdesc(QEMUFile *f, void *pv, size_t size,
return ok;
}
+/*
+ * Create a ptr marker field to precede an element of a
+ * VMS_ARRAY_OF_POINTER VMSD field. This indicates whether the
+ * following element is a NULL pointer or a valid pointer. This is
+ * used to know whether a pointer needs to be followed when loading
+ * the data.
+ */
+static bool vmstate_ptr_marker_save(QEMUFile *f, void *opaque,
+ const VMStateDescription *vmsd,
+ const VMStateField *field,
+ JSONWriter *vmdesc,
+ int i, int max, Error **errp)
+{
+ g_autofree VMStateField *fake = g_new0(VMStateField, 1);
+
+ /* It can only happen on an array of pointers! */
+ assert(field->flags & VMS_ARRAY_OF_POINTER);
+
+ /* Some of fake's properties should match the original's */
+ fake->name = field->name;
+ fake->version_id = field->version_id;
+
+ /* Do not need "field_exists" check as it always exists */
+ fake->field_exists = NULL;
+
+ /* use 1 byte for the marker */
+ fake->size = 1;
+ fake->info = &vmstate_info_ptr_marker;
+ fake->flags = VMS_SINGLE;
+
+ /* All the rest fields shouldn't matter.. */
+
+ return vmstate_save_field_with_vmdesc(f, opaque, field->size, vmsd, fake,
+ vmdesc, i, max, errp);
+}
+
static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque, JSONWriter *vmdesc,
int version_id, Error **errp)
@@ -635,7 +641,6 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
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);
@@ -688,14 +693,9 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
}
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);
-
+ ok = vmstate_ptr_marker_save(f, curr_elem, vmsd, field,
+ use_vmdesc ? vmdesc : NULL,
+ i, max_elems, errp);
if (!ok) {
goto out;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [RFC PATCH v1 08/17] vmstate: Don't recompute size and n_elems in vmstate_size
2026-03-24 19:43 [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Fabiano Rosas
` (6 preceding siblings ...)
2026-03-24 19:43 ` [RFC PATCH v1 07/17] vmstate: Create and save ptr marker in same function Fabiano Rosas
@ 2026-03-24 19:43 ` Fabiano Rosas
2026-03-24 19:43 ` [RFC PATCH v1 09/17] vmstate: Increase scope of vmstate_handle_alloc Fabiano Rosas
` (9 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-24 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Alexander Mikhalitsyn, Juraj Marcin
The caller already has the size and number of elements available. Pass
them in. Subsequent commits will add other callers.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/vmstate.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 0975d10793..8437b5ef3e 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -101,11 +101,10 @@ static int vmstate_size(void *opaque, const VMStateField *field)
}
static void vmstate_handle_alloc(void *ptr, const VMStateField *field,
- void *opaque)
+ int size, int n_elems)
{
if (field->flags & VMS_POINTER && field->flags & VMS_ALLOC) {
- gsize size = vmstate_size(opaque, field);
- size *= vmstate_n_elems(opaque, field);
+ size *= n_elems;
if (size) {
*(void **)ptr = g_malloc(size);
}
@@ -248,7 +247,7 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
int i, n_elems = vmstate_n_elems(opaque, field);
int size = vmstate_size(opaque, field);
- vmstate_handle_alloc(first_elem, field, opaque);
+ vmstate_handle_alloc(first_elem, field, size, n_elems);
if (field->flags & VMS_POINTER) {
first_elem = *(void **)first_elem;
assert(first_elem || !n_elems || !size);
--
2.51.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [RFC PATCH v1 09/17] vmstate: Increase scope of vmstate_handle_alloc
2026-03-24 19:43 [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Fabiano Rosas
` (7 preceding siblings ...)
2026-03-24 19:43 ` [RFC PATCH v1 08/17] vmstate: Don't recompute size and n_elems in vmstate_size Fabiano Rosas
@ 2026-03-24 19:43 ` Fabiano Rosas
2026-03-24 19:43 ` [RFC PATCH v1 10/17] vmstate: Remove curr_elem_p Fabiano Rosas
` (8 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-24 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Alexander Mikhalitsyn, Juraj Marcin
Move more code into vmstate_handle_alloc. If it's going to handle,
make it handle it all.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/vmstate.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 8437b5ef3e..0cdfde2232 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -100,15 +100,23 @@ static int vmstate_size(void *opaque, const VMStateField *field)
return size;
}
-static void vmstate_handle_alloc(void *ptr, const VMStateField *field,
+static void *vmstate_handle_alloc(void *ptr, const VMStateField *field,
int size, int n_elems)
{
- if (field->flags & VMS_POINTER && field->flags & VMS_ALLOC) {
- size *= n_elems;
- if (size) {
- *(void **)ptr = g_malloc(size);
+ void *new = ptr;
+
+ if (field->flags & VMS_POINTER) {
+ if (field->flags & VMS_ALLOC) {
+ size *= n_elems;
+ if (size) {
+ new = g_malloc(size);
+ *(void **)ptr = new;
+ }
}
+ assert(new || !n_elems || !size);
+ return *(void **)ptr;
}
+ return new;
}
static bool vmstate_ptr_marker_load(QEMUFile *f, bool *load_field,
@@ -247,11 +255,7 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
int i, n_elems = vmstate_n_elems(opaque, field);
int size = vmstate_size(opaque, field);
- vmstate_handle_alloc(first_elem, field, size, n_elems);
- if (field->flags & VMS_POINTER) {
- first_elem = *(void **)first_elem;
- assert(first_elem || !n_elems || !size);
- }
+ first_elem = vmstate_handle_alloc(first_elem, field, size, n_elems);
for (i = 0; i < n_elems; i++) {
/* If we will process the load of field? */
--
2.51.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [RFC PATCH v1 10/17] vmstate: Remove curr_elem_p
2026-03-24 19:43 [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Fabiano Rosas
` (8 preceding siblings ...)
2026-03-24 19:43 ` [RFC PATCH v1 09/17] vmstate: Increase scope of vmstate_handle_alloc Fabiano Rosas
@ 2026-03-24 19:43 ` Fabiano Rosas
2026-03-24 19:43 ` [RFC PATCH v1 11/17] vmstate: Introduce vmstate_first Fabiano Rosas
` (7 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-24 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Alexander Mikhalitsyn, Juraj Marcin
The usage of curr_elem_p for holding the array of pointers element is
a bit confusing. The side effect of updating what this variable points
to is not very clear. Remove it in favor of a local array_elem
variable with the same purpose, but make it of type void** instead,
since this is what it really is. Move it under the
VMS_ARRAY_OF_POINTER check.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/vmstate.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 0cdfde2232..85e63305f6 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -261,16 +261,15 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
/* If we will process the load of field? */
bool load_field = true;
bool ok = true;
- void *curr_elem_p = first_elem + size * i;
- void *curr_elem = curr_elem_p;
+ void *curr_elem;
if (field->flags & VMS_ARRAY_OF_POINTER) {
+ void **array_elem = (void **)first_elem + i;
bool use_dynamic_array =
field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC;
bool use_marker_field;
- curr_elem = *(void **)curr_elem_p;
-
+ curr_elem = *array_elem;
use_marker_field = use_dynamic_array || !curr_elem;
if (use_marker_field) {
@@ -293,9 +292,11 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
*/
curr_elem = g_malloc0(field->size);
/* Remember to update the root pointer! */
- *(void **)curr_elem_p = curr_elem;
+ *(void **)array_elem = curr_elem;
}
}
+ } else {
+ curr_elem = first_elem + size * i;
}
if (load_field) {
--
2.51.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [RFC PATCH v1 11/17] vmstate: Introduce vmstate_first
2026-03-24 19:43 [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Fabiano Rosas
` (9 preceding siblings ...)
2026-03-24 19:43 ` [RFC PATCH v1 10/17] vmstate: Remove curr_elem_p Fabiano Rosas
@ 2026-03-24 19:43 ` Fabiano Rosas
2026-03-24 19:43 ` [RFC PATCH v1 12/17] vmstate: Introduce vmstate_next Fabiano Rosas
` (6 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-24 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Alexander Mikhalitsyn, Juraj Marcin
The vmstate_save|load_vmsd[_v] functions do a bit of juggling of
pointers to allocate/dereference arrays. Introduce a getter for the
first field. Take field->flags into consideration and also allocate
memory when necessary.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
maybe we can soon have an iterator
---
migration/vmstate.c | 67 +++++++++++++++++++++++++--------------------
1 file changed, 38 insertions(+), 29 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 85e63305f6..ab7c6fa4ab 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -100,23 +100,31 @@ static int vmstate_size(void *opaque, const VMStateField *field)
return size;
}
-static void *vmstate_handle_alloc(void *ptr, const VMStateField *field,
- int size, int n_elems)
+static void *vmstate_handle_alloc(void **ptr, int size, int n_elems)
{
- void *new = ptr;
+ size *= n_elems;
+ if (size) {
+ *ptr = g_malloc(size);
+ }
+
+ return *ptr;
+}
+
+static void *vmstate_first(void *opaque, const VMStateField *field,
+ int size, int n, bool alloc)
+{
+ void **first = opaque + field->offset;
+
+ if (alloc) {
+ return vmstate_handle_alloc(first, size, n);
+ }
if (field->flags & VMS_POINTER) {
- if (field->flags & VMS_ALLOC) {
- size *= n_elems;
- if (size) {
- new = g_malloc(size);
- *(void **)ptr = new;
- }
- }
- assert(new || !n_elems || !size);
- return *(void **)ptr;
+ assert(first || !n || !size);
+ return *first;
}
- return new;
+
+ return first;
}
static bool vmstate_ptr_marker_load(QEMUFile *f, bool *load_field,
@@ -251,11 +259,12 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
trace_vmstate_load_state_field(vmsd->name, field->name, exists);
if (exists) {
- void *first_elem = opaque + field->offset;
+ void *head;
int i, n_elems = vmstate_n_elems(opaque, field);
int size = vmstate_size(opaque, field);
- first_elem = vmstate_handle_alloc(first_elem, field, size, n_elems);
+ head = vmstate_first(opaque, field, size, n_elems,
+ field->flags & VMS_ALLOC);
for (i = 0; i < n_elems; i++) {
/* If we will process the load of field? */
@@ -264,7 +273,7 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
void *curr_elem;
if (field->flags & VMS_ARRAY_OF_POINTER) {
- void **array_elem = (void **)first_elem + i;
+ void **array_elem = (void **)head + i;
bool use_dynamic_array =
field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC;
bool use_marker_field;
@@ -290,13 +299,13 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
* need the object size, not entry size of the
* array.
*/
- curr_elem = g_malloc0(field->size);
- /* Remember to update the root pointer! */
- *(void **)array_elem = curr_elem;
+ assert(!curr_elem);
+ curr_elem = vmstate_handle_alloc(array_elem,
+ field->size, 1);
}
}
} else {
- curr_elem = first_elem + size * i;
+ curr_elem = head + size * i;
}
if (load_field) {
@@ -627,28 +636,26 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
while (field->name) {
if (vmstate_field_exists(vmsd, field, opaque, version_id)) {
- void *first_elem = opaque + field->offset;
+ void *head;
int i, n_elems = vmstate_n_elems(opaque, field);
int size = vmstate_size(opaque, field);
bool is_null_prev = false;
bool use_vmdesc = true;
trace_vmstate_save_state_loop(vmsd->name, field->name, n_elems);
- if (field->flags & VMS_POINTER) {
- first_elem = *(void **)first_elem;
- assert(first_elem || !n_elems || !size);
- }
+ head = vmstate_first(opaque, field, size, n_elems, false);
for (i = 0; i < n_elems; i++) {
bool save_field = true;
- void *curr_elem = first_elem + size * i;
+ void *curr_elem;
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(curr_elem);
- curr_elem = *(void **)curr_elem;
+ assert(array_elem);
+ curr_elem = *array_elem;
is_null = !curr_elem;
@@ -681,7 +688,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 = *(void **)(first_elem + size * j);
+ void *elem = *(void **)(head + size * j);
bool elem_is_null = !elem;
if (is_null != elem_is_null) {
@@ -706,6 +713,8 @@ 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
^ permalink raw reply related [flat|nested] 31+ messages in thread* [RFC PATCH v1 12/17] vmstate: Introduce vmstate_next
2026-03-24 19:43 [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Fabiano Rosas
` (10 preceding siblings ...)
2026-03-24 19:43 ` [RFC PATCH v1 11/17] vmstate: Introduce vmstate_first Fabiano Rosas
@ 2026-03-24 19:43 ` Fabiano Rosas
2026-03-26 14:18 ` Peter Xu
2026-03-24 19:43 ` [RFC PATCH v1 13/17] vmstate: Drop VMS_ARRAY_OF_POINTER_AUTO_ALLOC Fabiano Rosas
` (5 subsequent siblings)
17 siblings, 1 reply; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-24 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Alexander Mikhalitsyn, Juraj Marcin
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.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [RFC PATCH v1 12/17] vmstate: Introduce vmstate_next
2026-03-24 19:43 ` [RFC PATCH v1 12/17] vmstate: Introduce vmstate_next Fabiano Rosas
@ 2026-03-26 14:18 ` Peter Xu
0 siblings, 0 replies; 31+ messages in thread
From: Peter Xu @ 2026-03-26 14:18 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Alexander Mikhalitsyn, Juraj Marcin
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.
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* [RFC PATCH v1 13/17] vmstate: Drop VMS_ARRAY_OF_POINTER_AUTO_ALLOC
2026-03-24 19:43 [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Fabiano Rosas
` (11 preceding siblings ...)
2026-03-24 19:43 ` [RFC PATCH v1 12/17] vmstate: Introduce vmstate_next Fabiano Rosas
@ 2026-03-24 19:43 ` Fabiano Rosas
2026-03-25 19:29 ` Peter Xu
2026-03-25 21:57 ` Peter Xu
2026-03-24 19:43 ` [RFC PATCH v1 14/17] vmstate: Move VMS_MUST_EXIST check Fabiano Rosas
` (4 subsequent siblings)
17 siblings, 2 replies; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-24 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Alexander Mikhalitsyn, Juraj Marcin
The semantics of the VMS_ARRAY_OF_POINTER_AUTO_ALLOC of allocating
memory dynamically for an array of pointers could be thought to be the
same as the currently non-existent combination of VMS_ARRAY_OF_POINTER
| VMS_ALLOC. The code is now able to handle such invalid combination,
so drop the new flag and enable the VMS_ARRAY_OF_POINTER | VMS_ALLOC
combo.
Note: I don't quite understand why we cannot allocate the first
element of the array with VMS_ARRAY_OF_POINTER_AUTO_ALLOC, so I
avoided that case when using the other flags as well.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
include/migration/vmstate.h | 21 +++++----------------
migration/savevm.c | 28 ++++++++++++----------------
migration/vmstate.c | 12 +++++++-----
3 files changed, 24 insertions(+), 37 deletions(-)
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 70bebc60ed..bf12df66e2 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -146,8 +146,8 @@ enum VMStateFlags {
/* When loading serialised VM state, allocate memory for the
* (entire) field. Only valid in combination with
* VMS_POINTER. Note: Not all combinations with other flags are
- * currently supported, e.g. VMS_ALLOC|VMS_ARRAY_OF_POINTER won't
- * cause the individual entries to be allocated. */
+ * currently supported.
+ */
VMS_ALLOC = 0x2000,
/* Multiply the number of entries given by the integer at opaque +
@@ -161,19 +161,8 @@ enum VMStateFlags {
* structure we are referencing to use. */
VMS_VSTRUCT = 0x8000,
- /*
- * This is a sub-flag for VMS_ARRAY_OF_POINTER, so VMS_ARRAY_OF_POINTER
- * must be set altogether. When set, it means array elements can
- * contain either valid or NULL pointers, vmstate core will sync it
- * between the two QEMU instances via the stream protocol. When it's a
- * valid pointer, the vmstate core will be responsible to do the proper
- * memory allocations. It also means user of this flag must prepare
- * the array to be all NULLs otherwise memory may leak.
- */
- VMS_ARRAY_OF_POINTER_AUTO_ALLOC = 0x10000,
-
/* Marker for end of list */
- VMS_END = 0x20000,
+ VMS_END = 0x10000,
};
typedef enum {
@@ -610,7 +599,7 @@ extern const VMStateInfo vmstate_info_qlist;
.size = sizeof(_type), \
.flags = VMS_POINTER | VMS_VARRAY_UINT8 | \
VMS_ARRAY_OF_POINTER | VMS_STRUCT | \
- VMS_ARRAY_OF_POINTER_AUTO_ALLOC, \
+ VMS_ALLOC, \
.offset = vmstate_offset_pointer(_state, _field, _type *), \
}
@@ -623,7 +612,7 @@ extern const VMStateInfo vmstate_info_qlist;
.size = sizeof(_type), \
.flags = VMS_POINTER | VMS_VARRAY_UINT32 | \
VMS_ARRAY_OF_POINTER | VMS_STRUCT | \
- VMS_ARRAY_OF_POINTER_AUTO_ALLOC, \
+ VMS_ALLOC, \
.offset = vmstate_offset_pointer(_state, _field, _type *), \
}
diff --git a/migration/savevm.c b/migration/savevm.c
index db0721996c..5170928d87 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -869,7 +869,8 @@ static void vmstate_check(const VMStateDescription *vmsd)
if (field) {
while (field->name) {
if (field->flags & VMS_ARRAY_OF_POINTER) {
- if (field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC) {
+ if (field->flags & VMS_ARRAY_OF_POINTER &&
+ field->flags & VMS_ALLOC) {
/*
* Size must be provided because dest QEMU needs that
* info to know what to allocate
@@ -882,22 +883,17 @@ static void vmstate_check(const VMStateDescription *vmsd)
* setup of sizes in this case.
*/
assert(field->size == 0 && field->size_offset == 0);
+
+ /*
+ * Without VMS_ALLOC, VMS_ARRAY_OF_POINTER must be
+ * used only together with one of VMS_(V)ARRAY*
+ * flags.
+ */
+ assert(field->flags & (VMS_ARRAY | VMS_VARRAY_INT32 |
+ VMS_VARRAY_UINT16 |
+ VMS_VARRAY_UINT8 |
+ VMS_VARRAY_UINT32));
}
- /*
- * VMS_ARRAY_OF_POINTER must be used only together with one
- * of VMS_(V)ARRAY* flags.
- */
- assert(field->flags & (VMS_ARRAY | VMS_VARRAY_INT32 |
- VMS_VARRAY_UINT16 | VMS_VARRAY_UINT8 |
- VMS_VARRAY_UINT32));
- }
-
- /*
- * When VMS_ARRAY_OF_POINTER_ALLOW_NULL is used, we must have
- * VMS_ARRAY_OF_POINTER set too.
- */
- if (field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC) {
- assert(field->flags & VMS_ARRAY_OF_POINTER);
}
if (field->flags & (VMS_STRUCT | VMS_VSTRUCT)) {
diff --git a/migration/vmstate.c b/migration/vmstate.c
index c1ad0ef9a5..8f0f9383e2 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -115,7 +115,7 @@ static void *vmstate_first(void *opaque, const VMStateField *field,
{
void **first = opaque + field->offset;
- if (alloc) {
+ if (alloc && field->flags & VMS_ALLOC) {
return vmstate_handle_alloc(first, size, n);
}
@@ -142,7 +142,7 @@ static void *vmstate_next(void **first, const VMStateField *field,
next = *array_elem;
if (alloc) {
- if (!next || field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC) {
+ if (!next || field->flags & VMS_ALLOC) {
/*
* NOTE: do not use vmstate_size() here, because we
* need the object size, not entry size of the
@@ -291,7 +291,7 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
int size = vmstate_size(opaque, field);
head = vmstate_first(opaque, field, size, n_elems,
- field->flags & VMS_ALLOC);
+ !(field->flags & VMS_ARRAY_OF_POINTER));
for (i = 0; i < n_elems; i++) {
/* If we will process the load of field? */
@@ -408,7 +408,8 @@ static bool vmsd_can_compress(const VMStateField *field)
return false;
}
- if (field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC) {
+ if (field->flags & VMS_ARRAY_OF_POINTER &&
+ field->flags & VMS_ALLOC) {
/*
* This may involve two VMSD fields to be dumped, one for the
* marker to show if the pointer is NULL, followed by the real
@@ -659,7 +660,8 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
* marker first for each element saying if it's populated.
*/
use_dynamic_array =
- field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC;
+ field->flags & VMS_ARRAY_OF_POINTER &&
+ field->flags & VMS_ALLOC;
use_marker_field = use_dynamic_array || is_null;
--
2.51.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [RFC PATCH v1 13/17] vmstate: Drop VMS_ARRAY_OF_POINTER_AUTO_ALLOC
2026-03-24 19:43 ` [RFC PATCH v1 13/17] vmstate: Drop VMS_ARRAY_OF_POINTER_AUTO_ALLOC Fabiano Rosas
@ 2026-03-25 19:29 ` Peter Xu
2026-03-25 21:49 ` Peter Xu
2026-03-25 21:57 ` Peter Xu
1 sibling, 1 reply; 31+ messages in thread
From: Peter Xu @ 2026-03-25 19:29 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Alexander Mikhalitsyn, Juraj Marcin
On Tue, Mar 24, 2026 at 04:43:28PM -0300, Fabiano Rosas wrote:
> The semantics of the VMS_ARRAY_OF_POINTER_AUTO_ALLOC of allocating
> memory dynamically for an array of pointers could be thought to be the
> same as the currently non-existent combination of VMS_ARRAY_OF_POINTER
> | VMS_ALLOC. The code is now able to handle such invalid combination,
> so drop the new flag and enable the VMS_ARRAY_OF_POINTER | VMS_ALLOC
> combo.
I was intentionally leaving VMS_ALLOC only for the top layer vmsd field,
but then.. I think you're right, there's no way the driver code can offload
the allocation of top layer vmsd field, while without offloading together
the pointer allocations inside the array altogether when it's
VMS_ARRAY_OF_POINTERS.
Let me see how I can incorporate this change, unless extremely awkward to
rearrange, otherwise I'll make it not introduce this flag when repost.
--
Peter Xu
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC PATCH v1 13/17] vmstate: Drop VMS_ARRAY_OF_POINTER_AUTO_ALLOC
2026-03-25 19:29 ` Peter Xu
@ 2026-03-25 21:49 ` Peter Xu
0 siblings, 0 replies; 31+ messages in thread
From: Peter Xu @ 2026-03-25 21:49 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Alexander Mikhalitsyn, Juraj Marcin
On Wed, Mar 25, 2026 at 03:29:01PM -0400, Peter Xu wrote:
> On Tue, Mar 24, 2026 at 04:43:28PM -0300, Fabiano Rosas wrote:
> > The semantics of the VMS_ARRAY_OF_POINTER_AUTO_ALLOC of allocating
> > memory dynamically for an array of pointers could be thought to be the
> > same as the currently non-existent combination of VMS_ARRAY_OF_POINTER
> > | VMS_ALLOC. The code is now able to handle such invalid combination,
> > so drop the new flag and enable the VMS_ARRAY_OF_POINTER | VMS_ALLOC
> > combo.
>
> I was intentionally leaving VMS_ALLOC only for the top layer vmsd field,
> but then.. I think you're right, there's no way the driver code can offload
> the allocation of top layer vmsd field, while without offloading together
> the pointer allocations inside the array altogether when it's
> VMS_ARRAY_OF_POINTERS.
>
> Let me see how I can incorporate this change, unless extremely awkward to
> rearrange, otherwise I'll make it not introduce this flag when repost.
Hmm, I forgot one thing, which is when the array doesn't need allocation
but the field does. In that case VMS_ALLOC mustn't be set, but we still
need another flag to say "we need allocation for the elements in the array".
So maybe we still need a flag.
--
Peter Xu
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC PATCH v1 13/17] vmstate: Drop VMS_ARRAY_OF_POINTER_AUTO_ALLOC
2026-03-24 19:43 ` [RFC PATCH v1 13/17] vmstate: Drop VMS_ARRAY_OF_POINTER_AUTO_ALLOC Fabiano Rosas
2026-03-25 19:29 ` Peter Xu
@ 2026-03-25 21:57 ` Peter Xu
1 sibling, 0 replies; 31+ messages in thread
From: Peter Xu @ 2026-03-25 21:57 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Alexander Mikhalitsyn, Juraj Marcin
On Tue, Mar 24, 2026 at 04:43:28PM -0300, Fabiano Rosas wrote:
> Note: I don't quite understand why we cannot allocate the first
> element of the array with VMS_ARRAY_OF_POINTER_AUTO_ALLOC, so I
> avoided that case when using the other flags as well.
I just notice what this question is about after reading slightly more of
the code: note that we should allocate all elements if AUTO_ALLOC is
specified, what is introduced in patch 11 (vmstate_first) is essentially
processing the array itself, so if VMS_ALLOC is set we alloc the array, if
further _AUTO_ALLOC is specified we alloc all elements in the array.
--
Peter Xu
^ permalink raw reply [flat|nested] 31+ messages in thread
* [RFC PATCH v1 14/17] vmstate: Move VMS_MUST_EXIST check
2026-03-24 19:43 [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Fabiano Rosas
` (12 preceding siblings ...)
2026-03-24 19:43 ` [RFC PATCH v1 13/17] vmstate: Drop VMS_ARRAY_OF_POINTER_AUTO_ALLOC Fabiano Rosas
@ 2026-03-24 19:43 ` Fabiano Rosas
2026-03-25 19:38 ` Peter Xu
2026-03-24 19:43 ` [RFC PATCH v1 15/17] vmstate: Invert exists check Fabiano Rosas
` (3 subsequent siblings)
17 siblings, 1 reply; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-24 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Alexander Mikhalitsyn, Juraj Marcin
Move the VMS_MUST_EXIST check into the vmstate_field_exists() function
and make it return bool + errp. This deduplicates a bit of code
between save and load.
XXX: why do we assert on save?
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/vmstate.c | 48 ++++++++++++++++++++++++++++-----------------
1 file changed, 30 insertions(+), 18 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 8f0f9383e2..5bc860129e 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -30,10 +30,10 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque, JSONWriter *vmdesc,
int version_id, Error **errp);
-/* 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)
+static bool vmstate_field_exists(const VMStateDescription *vmsd,
+ const VMStateField *field,
+ void *opaque, int version_id,
+ bool *exists, Error **errp)
{
bool result;
@@ -51,7 +51,16 @@ vmstate_field_exists(const VMStateDescription *vmsd, const VMStateField *field,
result = field->version_id <= version_id;
}
- return result;
+ *exists = result;
+
+ if (!result && field->flags & VMS_MUST_EXIST) {
+ error_setg(errp, "Expected field to exist, but it doesn't: "
+ "%s/%s version_id: %d",
+ vmsd->name, field->name, vmsd->version_id);
+ return false;
+ }
+
+ return true;
}
static int vmstate_n_elems(void *opaque, const VMStateField *field)
@@ -257,6 +266,7 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
{
ERRP_GUARD();
const VMStateField *field = vmsd->fields;
+ bool ok = true;
trace_vmstate_load_state(vmsd->name, version_id);
@@ -281,7 +291,13 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
}
while (field->name) {
- bool exists = vmstate_field_exists(vmsd, field, opaque, version_id);
+ bool exists;
+
+ ok = vmstate_field_exists(vmsd, field, opaque, version_id, &exists,
+ errp);
+ if (!ok) {
+ return false;
+ }
trace_vmstate_load_state_field(vmsd->name, field->name, exists);
@@ -296,7 +312,6 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
for (i = 0; i < n_elems; i++) {
/* If we will process the load of field? */
bool load_field = true;
- bool ok = true;
void *curr_elem;
if (field->flags & VMS_ARRAY_OF_POINTER) {
@@ -326,10 +341,6 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
return false;
}
}
- } else if (field->flags & VMS_MUST_EXIST) {
- error_setg(errp, "Input validation failed: %s/%s version_id: %d",
- vmsd->name, field->name, vmsd->version_id);
- return false;
}
field++;
}
@@ -635,7 +646,14 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
}
while (field->name) {
- if (vmstate_field_exists(vmsd, field, opaque, version_id)) {
+ bool exists;
+
+ ok = vmstate_field_exists(vmsd, field, opaque, version_id, &exists,
+ errp);
+ if (!ok) {
+ g_assert_not_reached();
+ }
+ if (exists) {
void *head;
int i, n_elems = vmstate_n_elems(opaque, field);
int size = vmstate_size(opaque, field);
@@ -725,12 +743,6 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
use_vmdesc = false;
}
- } else {
- if (field->flags & VMS_MUST_EXIST) {
- error_report("Output state validation failed: %s/%s",
- vmsd->name, field->name);
- assert(!(field->flags & VMS_MUST_EXIST));
- }
}
field++;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [RFC PATCH v1 15/17] vmstate: Invert exists check
2026-03-24 19:43 [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Fabiano Rosas
` (13 preceding siblings ...)
2026-03-24 19:43 ` [RFC PATCH v1 14/17] vmstate: Move VMS_MUST_EXIST check Fabiano Rosas
@ 2026-03-24 19:43 ` Fabiano Rosas
2026-03-24 19:43 ` [RFC PATCH v1 16/17] vmstate: Declare variables at the top Fabiano Rosas
` (2 subsequent siblings)
17 siblings, 0 replies; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-24 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Alexander Mikhalitsyn, Juraj Marcin
Aiming to reduce one level of indentation on these routines, invert
the exists check so it can "return early" (continue actually). There
is too much code and doing it all at once would be hard to review.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/vmstate.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 5bc860129e..1862198c70 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -301,7 +301,10 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
trace_vmstate_load_state_field(vmsd->name, field->name, exists);
- if (exists) {
+ if (!exists) {
+ field++;
+ continue;
+ } else {
void *head;
int i, n_elems = vmstate_n_elems(opaque, field);
int size = vmstate_size(opaque, field);
@@ -653,7 +656,11 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
if (!ok) {
g_assert_not_reached();
}
- if (exists) {
+
+ if (!exists) {
+ field++;
+ continue;
+ } else {
void *head;
int i, n_elems = vmstate_n_elems(opaque, field);
int size = vmstate_size(opaque, field);
--
2.51.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [RFC PATCH v1 16/17] vmstate: Declare variables at the top
2026-03-24 19:43 [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Fabiano Rosas
` (14 preceding siblings ...)
2026-03-24 19:43 ` [RFC PATCH v1 15/17] vmstate: Invert exists check Fabiano Rosas
@ 2026-03-24 19:43 ` Fabiano Rosas
2026-03-24 19:43 ` [RFC PATCH v1 17/17] vmstate: Reduce indentation levels Fabiano Rosas
2026-03-25 19:43 ` [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Peter Xu
17 siblings, 0 replies; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-24 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Alexander Mikhalitsyn, Juraj Marcin
Move the variables declaration at the top so we can reduce the number
of lines in the function by grouping some declarations.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/vmstate.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 1862198c70..054e642ec9 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -292,6 +292,8 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
while (field->name) {
bool exists;
+ void *head;
+ int i, n_elems, size;
ok = vmstate_field_exists(vmsd, field, opaque, version_id, &exists,
errp);
@@ -305,9 +307,8 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
field++;
continue;
} else {
- void *head;
- int i, n_elems = vmstate_n_elems(opaque, field);
- int size = vmstate_size(opaque, field);
+ n_elems = vmstate_n_elems(opaque, field);
+ size = vmstate_size(opaque, field);
head = vmstate_first(opaque, field, size, n_elems,
!(field->flags & VMS_ARRAY_OF_POINTER));
@@ -649,7 +650,9 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
}
while (field->name) {
- bool exists;
+ bool exists, is_null_prev = false, use_vmdesc = true;
+ void *head;
+ int i, n_elems, size;
ok = vmstate_field_exists(vmsd, field, opaque, version_id, &exists,
errp);
@@ -661,11 +664,8 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
field++;
continue;
} else {
- void *head;
- int i, n_elems = vmstate_n_elems(opaque, field);
- int size = vmstate_size(opaque, field);
- bool is_null_prev = false;
- bool use_vmdesc = true;
+ n_elems = vmstate_n_elems(opaque, field);
+ size = vmstate_size(opaque, field);
trace_vmstate_save_state_loop(vmsd->name, field->name, n_elems);
head = vmstate_first(opaque, field, size, n_elems, false);
--
2.51.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [RFC PATCH v1 17/17] vmstate: Reduce indentation levels
2026-03-24 19:43 [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Fabiano Rosas
` (15 preceding siblings ...)
2026-03-24 19:43 ` [RFC PATCH v1 16/17] vmstate: Declare variables at the top Fabiano Rosas
@ 2026-03-24 19:43 ` Fabiano Rosas
2026-03-25 19:43 ` [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Peter Xu
17 siblings, 0 replies; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-24 19:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Alexander Mikhalitsyn, Juraj Marcin
Simply move the code out of the else branch.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/vmstate.c | 193 ++++++++++++++++++++++----------------------
1 file changed, 96 insertions(+), 97 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 054e642ec9..22f8758960 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -306,46 +306,46 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
if (!exists) {
field++;
continue;
- } else {
- n_elems = vmstate_n_elems(opaque, field);
- size = vmstate_size(opaque, field);
+ }
- head = vmstate_first(opaque, field, size, n_elems,
- !(field->flags & VMS_ARRAY_OF_POINTER));
+ n_elems = vmstate_n_elems(opaque, field);
+ size = vmstate_size(opaque, field);
- for (i = 0; i < n_elems; i++) {
- /* If we will process the load of field? */
- bool load_field = true;
- void *curr_elem;
+ head = vmstate_first(opaque, field, size, n_elems,
+ !(field->flags & VMS_ARRAY_OF_POINTER));
- if (field->flags & VMS_ARRAY_OF_POINTER) {
- /* Peek a possible pointer marker instead of VMSD first */
- ok = vmstate_ptr_marker_load(f, &load_field);
- }
+ for (i = 0; i < n_elems; i++) {
+ /* If we will process the load of field? */
+ bool load_field = true;
+ void *curr_elem;
- curr_elem = vmstate_next(head, field, size, i,
- load_field && ok);
+ if (field->flags & VMS_ARRAY_OF_POINTER) {
+ /* Peek a possible pointer marker instead of VMSD first */
+ ok = vmstate_ptr_marker_load(f, &load_field);
+ }
- if (load_field) {
- ok = vmstate_load_field(f, curr_elem, size, field, errp);
- }
+ curr_elem = vmstate_next(head, field, size, i, load_field && ok);
- if (ok) {
- int ret = qemu_file_get_error(f);
- if (ret < 0) {
- error_setg(errp,
- "Failed to load %s state: stream error: %d",
- vmsd->name, ret);
- trace_vmstate_load_field_error(field->name, ret);
- return false;
- }
- } else {
- qemu_file_set_error(f, -EINVAL);
- trace_vmstate_load_field_error(field->name, -EINVAL);
+ if (load_field) {
+ ok = vmstate_load_field(f, curr_elem, size, field, errp);
+ }
+
+ if (ok) {
+ int ret = qemu_file_get_error(f);
+ if (ret < 0) {
+ error_setg(errp,
+ "Failed to load %s state: stream error: %d",
+ vmsd->name, ret);
+ trace_vmstate_load_field_error(field->name, ret);
return false;
}
+ } else {
+ qemu_file_set_error(f, -EINVAL);
+ trace_vmstate_load_field_error(field->name, -EINVAL);
+ return false;
}
}
+
field++;
}
assert(field->flags == VMS_END);
@@ -663,93 +663,92 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
if (!exists) {
field++;
continue;
- } else {
- n_elems = vmstate_n_elems(opaque, field);
- size = vmstate_size(opaque, field);
+ }
- trace_vmstate_save_state_loop(vmsd->name, field->name, n_elems);
- head = vmstate_first(opaque, field, size, n_elems, false);
+ n_elems = vmstate_n_elems(opaque, field);
+ size = vmstate_size(opaque, field);
- for (i = 0; i < n_elems; i++) {
- bool save_field = true;
- void *curr_elem = vmstate_next(head, field, size, i, false);
- int max_elems = n_elems - i;
+ trace_vmstate_save_state_loop(vmsd->name, field->name, n_elems);
+ head = vmstate_first(opaque, field, size, n_elems, false);
- if (field->flags & VMS_ARRAY_OF_POINTER) {
- bool use_marker_field, is_null, use_dynamic_array;
+ for (i = 0; i < n_elems; i++) {
+ bool save_field = true;
+ void *curr_elem = vmstate_next(head, field, size, i, false);
+ int max_elems = n_elems - i;
- is_null = !curr_elem;
+ if (field->flags & VMS_ARRAY_OF_POINTER) {
+ bool use_marker_field, is_null, use_dynamic_array;
+ is_null = !curr_elem;
+
+ /*
+ * 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 &&
+ field->flags & VMS_ALLOC;
+
+ use_marker_field = use_dynamic_array || is_null;
+
+ if (vmdesc && vmsd_can_compress(field)) {
/*
- * When this is enabled, it means we will always push a ptr
- * marker first for each element saying if it's populated.
+ * 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.
*/
- use_dynamic_array =
- field->flags & VMS_ARRAY_OF_POINTER &&
- field->flags & VMS_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 = vmstate_next(head, field, size, j,
- false);
- bool elem_is_null = !elem;
-
- if (is_null != elem_is_null) {
- max_elems = j - i;
- break;
- }
- }
- }
- }
-
- if (use_dynamic_array) {
+ if (is_null != is_null_prev) {
+ is_null_prev = is_null;
use_vmdesc = true;
- }
- if (use_marker_field) {
- ok = vmstate_ptr_marker_save(f, curr_elem, vmsd, field,
- use_vmdesc ? vmdesc : NULL,
- i, max_elems, errp);
- if (!ok) {
- goto out;
+ for (int j = i + 1; j < n_elems; j++) {
+ void *elem = vmstate_next(head, field, size, j,
+ false);
+ bool elem_is_null = !elem;
+
+ if (is_null != elem_is_null) {
+ max_elems = j - i;
+ break;
+ }
}
-
- save_field = !!curr_elem;
}
}
- if (save_field) {
- ok = vmstate_save_field_with_vmdesc(
- f, curr_elem, size, vmsd, field,
- use_vmdesc ? vmdesc : NULL, i, max_elems, errp);
+ if (use_dynamic_array) {
+ use_vmdesc = true;
+ }
+ if (use_marker_field) {
+ ok = vmstate_ptr_marker_save(f, curr_elem, vmsd, field,
+ use_vmdesc ? vmdesc : NULL,
+ i, max_elems, errp);
if (!ok) {
goto out;
}
+
+ save_field = !!curr_elem;
}
+ }
- use_vmdesc = false;
+ if (save_field) {
+ ok = vmstate_save_field_with_vmdesc(f, curr_elem, size, vmsd,
+ field,
+ use_vmdesc ? vmdesc : NULL,
+ i, max_elems, errp);
+ if (!ok) {
+ goto out;
+ }
}
+ use_vmdesc = false;
}
field++;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx
2026-03-24 19:43 [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Fabiano Rosas
` (16 preceding siblings ...)
2026-03-24 19:43 ` [RFC PATCH v1 17/17] vmstate: Reduce indentation levels Fabiano Rosas
@ 2026-03-25 19:43 ` Peter Xu
2026-03-25 20:10 ` Fabiano Rosas
17 siblings, 1 reply; 31+ messages in thread
From: Peter Xu @ 2026-03-25 19:43 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Alexander Mikhalitsyn, Juraj Marcin
On Tue, Mar 24, 2026 at 04:43:15PM -0300, Fabiano Rosas wrote:
> Too much stuff to mention inline on the series, here's some patches on
> top. Let me know what you think! Feel free to drop it all or
> incorporate or whatever.
>
> Based-on: [PATCH RFC 00/10] vmstate: Implement VMS_ARRAY_OF_POINTER_AUTO_ALLOC
> 20260317232332.15209-1-peterx@redhat.com
> https://lore.kernel.org/r/20260317232332.15209-1-peterx@redhat.com
>
> which is in turn based on https://gitlab.com/farosas/qemu/-/commits/migration-staging
>
> I pushed it all to https://gitlab.com/farosas/qemu/-/commits/migration-rfc-auto-alloc
>
> CI run: https://gitlab.com/farosas/qemu/-/pipelines/2406278684
>
> Fabiano Rosas (17):
> vmstate: fixup the use of AUTO_ALLOC flag
> vmstate: Remove vmstate_use_marker_field
> vmstate: Stop checking size for nullptr compression
> vmstate: Set error inside of vmstate_save_field_with_vmdesc
> vmstate: Remove vmdesc_loop
> vmstate: Put array of pointers code together
> vmstate: Create and save ptr marker in same function
> vmstate: Don't recompute size and n_elems in vmstate_size
> vmstate: Increase scope of vmstate_handle_alloc
> vmstate: Remove curr_elem_p
> vmstate: Introduce vmstate_first
> vmstate: Introduce vmstate_next
> vmstate: Drop VMS_ARRAY_OF_POINTER_AUTO_ALLOC
> vmstate: Move VMS_MUST_EXIST check
> vmstate: Invert exists check
> vmstate: Declare variables at the top
> vmstate: Reduce indentation levels
Considering that this prior series may block the nvme series, I'll be
slightly conservative on what to collect, but I'll collect as much as I can
that makes me still feel confident when I repost; we'll see.
Thanks for this work, Fabiano.
--
Peter Xu
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx
2026-03-25 19:43 ` [RFC PATCH v1 00/17] migration: vmstate_save|load changes for peterx Peter Xu
@ 2026-03-25 20:10 ` Fabiano Rosas
0 siblings, 0 replies; 31+ messages in thread
From: Fabiano Rosas @ 2026-03-25 20:10 UTC (permalink / raw)
To: Peter Xu; +Cc: qemu-devel, Alexander Mikhalitsyn, Juraj Marcin
Peter Xu <peterx@redhat.com> writes:
> On Tue, Mar 24, 2026 at 04:43:15PM -0300, Fabiano Rosas wrote:
>> Too much stuff to mention inline on the series, here's some patches on
>> top. Let me know what you think! Feel free to drop it all or
>> incorporate or whatever.
>>
>> Based-on: [PATCH RFC 00/10] vmstate: Implement VMS_ARRAY_OF_POINTER_AUTO_ALLOC
>> 20260317232332.15209-1-peterx@redhat.com
>> https://lore.kernel.org/r/20260317232332.15209-1-peterx@redhat.com
>>
>> which is in turn based on https://gitlab.com/farosas/qemu/-/commits/migration-staging
>>
>> I pushed it all to https://gitlab.com/farosas/qemu/-/commits/migration-rfc-auto-alloc
>>
>> CI run: https://gitlab.com/farosas/qemu/-/pipelines/2406278684
>>
>> Fabiano Rosas (17):
>> vmstate: fixup the use of AUTO_ALLOC flag
>> vmstate: Remove vmstate_use_marker_field
>> vmstate: Stop checking size for nullptr compression
>> vmstate: Set error inside of vmstate_save_field_with_vmdesc
>> vmstate: Remove vmdesc_loop
>> vmstate: Put array of pointers code together
>> vmstate: Create and save ptr marker in same function
>> vmstate: Don't recompute size and n_elems in vmstate_size
>> vmstate: Increase scope of vmstate_handle_alloc
>> vmstate: Remove curr_elem_p
>> vmstate: Introduce vmstate_first
>> vmstate: Introduce vmstate_next
>> vmstate: Drop VMS_ARRAY_OF_POINTER_AUTO_ALLOC
>> vmstate: Move VMS_MUST_EXIST check
>> vmstate: Invert exists check
>> vmstate: Declare variables at the top
>> vmstate: Reduce indentation levels
>
> Considering that this prior series may block the nvme series, I'll be
> slightly conservative on what to collect, but I'll collect as much as I can
> that makes me still feel confident when I repost; we'll see.
>
Sure, if there's anything you still find valuable but won't take for the
repost let me know and I can send a separate series on top if that's the
case.
^ permalink raw reply [flat|nested] 31+ messages in thread