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.0On 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
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
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
© 2016 - 2026 Red Hat, Inc.