* [PATCH 1/4] migration: Remove VMSTATE_ARRAY_INT32_UNSAFE
2026-07-29 22:52 [PATCH 0/4] migration: Remove extra type-checking from vmstate macros Fabiano Rosas
@ 2026-07-29 22:52 ` Fabiano Rosas
2026-07-30 8:07 ` Vladimir Sementsov-Ogievskiy
2026-07-29 22:52 ` [PATCH 2/4] migration: Introduce VMStateOffset Fabiano Rosas
` (4 subsequent siblings)
5 siblings, 1 reply; 22+ messages in thread
From: Fabiano Rosas @ 2026-07-29 22:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Michael S . Tsirkin, Alexandr Moshkov
The ARRAY_INT32_UNSAFE vmstate macro is not used anywhere. Remove it.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
include/migration/vmstate.h | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 1b7f295417e..92a1a9fe98e 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -451,15 +451,6 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.offset = vmstate_offset_sub_array(_state, _field, _type, _start), \
}
-#define VMSTATE_ARRAY_INT32_UNSAFE(_field, _state, _field_num, _info, _type) {\
- .name = (stringify(_field)), \
- .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
- .info = &(_info), \
- .size = sizeof(_type), \
- .flags = VMS_VARRAY_INT32, \
- .offset = vmstate_offset_varray(_state, _field, _type), \
-}
-
#define VMSTATE_VARRAY_INT32(_field, _state, _field_num, _version, _info, _type) {\
.name = (stringify(_field)), \
.version_id = (_version), \
--
2.53.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 2/4] migration: Introduce VMStateOffset
2026-07-29 22:52 [PATCH 0/4] migration: Remove extra type-checking from vmstate macros Fabiano Rosas
2026-07-29 22:52 ` [PATCH 1/4] migration: Remove VMSTATE_ARRAY_INT32_UNSAFE Fabiano Rosas
@ 2026-07-29 22:52 ` Fabiano Rosas
2026-07-30 8:06 ` Vladimir Sementsov-Ogievskiy
2026-07-30 14:35 ` Michael S. Tsirkin
2026-07-29 22:52 ` [PATCH 3/4] migration: Remove redundant flags Fabiano Rosas
` (3 subsequent siblings)
5 siblings, 2 replies; 22+ messages in thread
From: Fabiano Rosas @ 2026-07-29 22:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Michael S . Tsirkin, Alexandr Moshkov
When migrating a buffer or array, the vmstate code needs to know the
size of the buffer and the number of elements of the array. Today a
vmstate writer can choose from a number of macros that take as last
input the name of a struct member from where the size/num will be
read.
At load time, the code will access those value via an opaque pointer
to the migrated data and therefore it needs to also know the size of
the struct member at that offset.
Currently that information is communicated by means of the
VMS_VARRAY_* and VMS_VBUFFER_* flags, where each possible type is
represented by a flag.
So far, that's all fine, but since the vmstate code makes heavy use of
macros, handling several types individually (i.e. by name: int,
int32_t, etc) requires several versions of a same macro, one for each
type. E.g: VMSTATE_VBUFFER_ALLOC_UINT32
^
This creates a pattern where the vmstate writer has to match the macro
name to the data type and has resulted in the code having a tendency
of having one macro version for each type, for each type of vmstate.
There is also some cognitive load to deal with, e.g.
VMSTATE_VARRAY_INT32 doesn't hold an array of int32, it holds an array
of something else and the number of elements for the array is stored
in a variable of type int32.
We're now dealing with the scenario where the code has been expecting
int32_t at some places, but a uint64_t macro variant has been added
without the code being updated.
To address all these situations, introduce a new struct that will hold
the offset of the struct members, but also their size, so the various
extra macros can all be removed and the person writing the vmstate
doesn't need to care about type-checking. Still, keep a minimum check
that those fields are at least integers and fit into 64 bits.
What changes:
1) type checking changes from individual types to a single check for
all integers;
2) there are new ways to access the offsets;
num_offset -> num_offset.off
size_offset -> size_offset.off
[new] num_offset.size
[new] size_offset.size
2) reading the offsets goes from checking the VMS_VARRAY_* flags in an
if/elseif block to comparing offset.size against the hardcoded
sizes in bytes;
3) the VMS_VARRAY_* and VMS_VBUFFER_* flags become obsolete. Removed
in the next patch;
4) memory usage increases +1 byte per vmstate;
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
include/migration/vmstate.h | 66 +++++++++++++++++++++++--------------
migration/savevm.c | 4 +--
migration/vmstate.c | 51 ++++++++++++++++++----------
3 files changed, 76 insertions(+), 45 deletions(-)
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 92a1a9fe98e..2ad3cc01371 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -31,6 +31,7 @@
typedef struct VMStateInfo VMStateInfo;
typedef struct VMStateField VMStateField;
+typedef struct VMStateOffset VMStateOffset;
/*
* VMStateInfo allows customized migration of objects that don't fit in
@@ -187,6 +188,11 @@ typedef enum {
MIG_PRI_MAX,
} MigrationPriority;
+struct VMStateOffset {
+ uint32_t off;
+ uint8_t size;
+};
+
struct VMStateField {
const char *name;
size_t offset;
@@ -205,11 +211,11 @@ struct VMStateField {
* pointer point to.
*/
size_t size;
- size_t size_offset;
+ VMStateOffset size_offset;
size_t start;
int num;
- size_t num_offset;
+ VMStateOffset num_offset;
const VMStateInfo *info;
enum VMStateFlags flags;
const VMStateDescription *vmsd;
@@ -328,6 +334,16 @@ extern const VMStateInfo vmstate_info_g_byte_array;
(type_check(t1, typeof_elt_of_field(t2, f)) \
+ QEMU_BUILD_BUG_ON_ZERO(!QEMU_IS_ARRAY(((t2 *)0)->f)))
+#define type_check_int64(t) \
+ (((ptrdiff_t)0 * (ptrdiff_t)(~((t)0))) + \
+ (0 * sizeof(char[(sizeof(t) <= sizeof(uint64_t)) ? 1 : -1])))
+
+#define vmstate_field_offset(_state, _field) { \
+ .off = (offsetof(_state, _field) + \
+ type_check_int64(typeof_field(_state, _field))), \
+ .size = sizeof(typeof_field(_state, _field)), \
+}
+
#define vmstate_offset_value(_state, _field, _type) \
(offsetof(_state, _field) + \
type_check(_type, typeof_field(_state, _field)))
@@ -454,7 +470,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
#define VMSTATE_VARRAY_INT32(_field, _state, _field_num, _version, _info, _type) {\
.name = (stringify(_field)), \
.version_id = (_version), \
- .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
+ .num_offset = vmstate_field_offset(_state, _field_num), \
.info = &(_info), \
.size = sizeof(_type), \
.flags = VMS_VARRAY_INT32|VMS_POINTER, \
@@ -464,7 +480,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
#define VMSTATE_VARRAY_UINT32(_field, _state, _field_num, _version, _info, _type) {\
.name = (stringify(_field)), \
.version_id = (_version), \
- .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
+ .num_offset = vmstate_field_offset(_state, _field_num), \
.info = &(_info), \
.size = sizeof(_type), \
.flags = VMS_VARRAY_UINT32|VMS_POINTER, \
@@ -474,7 +490,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
#define VMSTATE_VARRAY_INT32_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
.name = (stringify(_field)), \
.version_id = (_version), \
- .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
+ .num_offset = vmstate_field_offset(_state, _field_num), \
.info = &(_info), \
.size = sizeof(_type), \
.flags = VMS_VARRAY_INT32 | VMS_POINTER | VMS_ALLOC, \
@@ -484,7 +500,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
#define VMSTATE_VARRAY_UINT32_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
.name = (stringify(_field)), \
.version_id = (_version), \
- .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
+ .num_offset = vmstate_field_offset(_state, _field_num), \
.info = &(_info), \
.size = sizeof(_type), \
.flags = VMS_VARRAY_UINT32|VMS_POINTER|VMS_ALLOC, \
@@ -494,7 +510,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
#define VMSTATE_VARRAY_UINT16_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
.name = (stringify(_field)), \
.version_id = (_version), \
- .num_offset = vmstate_offset_value(_state, _field_num, uint16_t),\
+ .num_offset = vmstate_field_offset(_state, _field_num), \
.info = &(_info), \
.size = sizeof(_type), \
.flags = VMS_VARRAY_UINT16 | VMS_POINTER | VMS_ALLOC, \
@@ -504,7 +520,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
#define VMSTATE_VARRAY_UINT16_UNSAFE(_field, _state, _field_num, _version, _info, _type) {\
.name = (stringify(_field)), \
.version_id = (_version), \
- .num_offset = vmstate_offset_value(_state, _field_num, uint16_t),\
+ .num_offset = vmstate_field_offset(_state, _field_num), \
.info = &(_info), \
.size = sizeof(_type), \
.flags = VMS_VARRAY_UINT16, \
@@ -583,7 +599,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
_field, _state, _field_num, _version, _vmsd, _type) { \
.name = (stringify(_field)), \
.version_id = (_version), \
- .num_offset = vmstate_offset_value(_state, _field_num, uint8_t), \
+ .num_offset = vmstate_field_offset(_state, _field_num), \
.vmsd = &(_vmsd), \
.size = sizeof(_type), \
.flags = VMS_POINTER | VMS_VARRAY_UINT8 | \
@@ -596,7 +612,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
_field, _state, _field_num, _version, _vmsd, _type) { \
.name = (stringify(_field)), \
.version_id = (_version), \
- .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \
+ .num_offset = vmstate_field_offset(_state, _field_num), \
.vmsd = &(_vmsd), \
.size = sizeof(_type), \
.flags = VMS_POINTER | VMS_VARRAY_UINT32 | \
@@ -608,7 +624,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
#define VMSTATE_VARRAY_OF_POINTER_UINT32(_field, _state, _field_num, _version, _info, _type) { \
.name = (stringify(_field)), \
.version_id = (_version), \
- .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \
+ .num_offset = vmstate_field_offset(_state, _field_num), \
.info = &(_info), \
.flags = VMS_VARRAY_UINT32 | VMS_ARRAY_OF_POINTER | VMS_POINTER, \
.offset = vmstate_offset_pointer(_state, _field, _type *), \
@@ -650,7 +666,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
#define VMSTATE_STRUCT_VARRAY_UINT8(_field, _state, _field_num, _version, _vmsd, _type) { \
.name = (stringify(_field)), \
- .num_offset = vmstate_offset_value(_state, _field_num, uint8_t), \
+ .num_offset = vmstate_field_offset(_state, _field_num), \
.version_id = (_version), \
.vmsd = &(_vmsd), \
.size = sizeof(_type), \
@@ -674,7 +690,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
#define VMSTATE_STRUCT_VARRAY_POINTER_INT32(_field, _state, _field_num, _vmsd, _type) { \
.name = (stringify(_field)), \
.version_id = 0, \
- .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
+ .num_offset = vmstate_field_offset(_state, _field_num), \
.size = sizeof(_type), \
.vmsd = &(_vmsd), \
.flags = VMS_POINTER | VMS_VARRAY_INT32 | VMS_STRUCT, \
@@ -684,7 +700,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
#define VMSTATE_STRUCT_VARRAY_POINTER_UINT32(_field, _state, _field_num, _vmsd, _type) { \
.name = (stringify(_field)), \
.version_id = 0, \
- .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
+ .num_offset = vmstate_field_offset(_state, _field_num), \
.size = sizeof(_type), \
.vmsd = &(_vmsd), \
.flags = VMS_POINTER | VMS_VARRAY_INT32 | VMS_STRUCT, \
@@ -694,7 +710,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
#define VMSTATE_STRUCT_VARRAY_POINTER_UINT16(_field, _state, _field_num, _vmsd, _type) { \
.name = (stringify(_field)), \
.version_id = 0, \
- .num_offset = vmstate_offset_value(_state, _field_num, uint16_t),\
+ .num_offset = vmstate_field_offset(_state, _field_num), \
.size = sizeof(_type), \
.vmsd = &(_vmsd), \
.flags = VMS_POINTER | VMS_VARRAY_UINT16 | VMS_STRUCT, \
@@ -703,7 +719,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
#define VMSTATE_STRUCT_VARRAY_INT32(_field, _state, _field_num, _version, _vmsd, _type) { \
.name = (stringify(_field)), \
- .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
+ .num_offset = vmstate_field_offset(_state, _field_num, int32_t), \
.version_id = (_version), \
.vmsd = &(_vmsd), \
.size = sizeof(_type), \
@@ -713,7 +729,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
#define VMSTATE_STRUCT_VARRAY_UINT32(_field, _state, _field_num, _version, _vmsd, _type) { \
.name = (stringify(_field)), \
- .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \
+ .num_offset = vmstate_field_offset(_state, _field_num), \
.version_id = (_version), \
.vmsd = &(_vmsd), \
.size = sizeof(_type), \
@@ -725,7 +741,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.name = (stringify(_field)), \
.version_id = (_version), \
.vmsd = &(_vmsd), \
- .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
+ .num_offset = vmstate_field_offset(_state, _field_num), \
.size = sizeof(_type), \
.flags = VMS_STRUCT|VMS_VARRAY_INT32|VMS_ALLOC|VMS_POINTER, \
.offset = vmstate_offset_pointer(_state, _field, _type), \
@@ -746,7 +762,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.name = (stringify(_field)), \
.version_id = (_version), \
.field_exists = (_test), \
- .size_offset = vmstate_offset_value(_state, _field_size, uint32_t),\
+ .size_offset = vmstate_field_offset(_state, _field_size), \
.size = (_multiply), \
.info = &vmstate_info_buffer, \
.flags = VMS_VBUFFER|VMS_POINTER|VMS_MULTIPLY, \
@@ -757,7 +773,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.name = (stringify(_field)), \
.version_id = (_version), \
.field_exists = (_test), \
- .size_offset = vmstate_offset_value(_state, _field_size, int32_t),\
+ .size_offset = vmstate_field_offset(_state, _field_size), \
.info = &vmstate_info_buffer, \
.flags = VMS_VBUFFER|VMS_POINTER, \
.offset = offsetof(_state, _field), \
@@ -767,7 +783,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.name = (stringify(_field)), \
.version_id = (_version), \
.field_exists = (_test), \
- .size_offset = vmstate_offset_value(_state, _field_size, uint32_t),\
+ .size_offset = vmstate_field_offset(_state, _field_size), \
.info = &vmstate_info_buffer, \
.flags = VMS_VBUFFER|VMS_POINTER, \
.offset = offsetof(_state, _field), \
@@ -777,7 +793,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.name = (stringify(_field)), \
.version_id = (_version), \
.field_exists = (_test), \
- .size_offset = vmstate_offset_value(_state, _field_size, uint64_t),\
+ .size_offset = vmstate_field_offset(_state, _field_size), \
.info = &vmstate_info_buffer, \
.flags = VMS_VBUFFER | VMS_POINTER, \
.offset = offsetof(_state, _field), \
@@ -788,7 +804,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.name = (stringify(_field)), \
.version_id = (_version), \
.field_exists = (_test), \
- .size_offset = vmstate_offset_value(_state, _field_size, uint32_t),\
+ .size_offset = vmstate_field_offset(_state, _field_size), \
.info = &vmstate_info_buffer, \
.flags = VMS_VBUFFER|VMS_POINTER|VMS_ALLOC, \
.offset = offsetof(_state, _field), \
@@ -848,7 +864,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
#define VMSTATE_UNUSED_VARRAY_UINT32(_state, _test, _version, _field_num, _size) {\
.name = "unused", \
.field_exists = (_test), \
- .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
+ .num_offset = vmstate_field_offset(_state, _field_num), \
.version_id = (_version), \
.size = (_size), \
.info = &vmstate_info_unused_buffer, \
@@ -862,7 +878,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.name = (stringify(_field)), \
.field_exists = (_test), \
.version_id = (_version), \
- .size_offset = vmstate_offset_value(_state, _field_size, int32_t),\
+ .size_offset = vmstate_field_offset(_state, _field_size), \
.info = &vmstate_info_bitmap, \
.flags = VMS_VBUFFER|VMS_POINTER, \
.offset = offsetof(_state, _field), \
diff --git a/migration/savevm.c b/migration/savevm.c
index 34dd06f9f73..a272bcfd0b4 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -873,14 +873,14 @@ static void vmstate_check(const VMStateDescription *vmsd)
* Size must be provided because dest QEMU needs that
* info to know what to allocate
*/
- assert(field->size || field->size_offset);
+ assert(field->size || field->size_offset.size != 0);
} else {
/*
* Otherwise size info isn't useful (because it's
* always the size of host pointer), detect accidental
* setup of sizes in this case.
*/
- assert(field->size == 0 && field->size_offset == 0);
+ assert(field->size == 0 && field->size_offset.size == 0);
}
/*
* VMS_ARRAY_OF_POINTER must be used only together with one
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 50ebe378452..0a0b9faa20e 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -78,32 +78,45 @@ vmsd_init_ptr_marker_field(VMStateField *fake, const VMStateField *field)
};
}
-static int vmstate_n_elems(void *opaque, const VMStateField *field)
+static uint64_t vmstate_read_from_offset(void *opaque,
+ const VMStateOffset *offset)
{
- int n_elems = 1;
+ uint8_t *ptr = (uint8_t *)opaque + offset->off;
+
+ switch (offset->size) {
+ case 1:
+ return *(uint8_t *)ptr;
+ case 2:
+ return *(uint16_t *)ptr;
+ case 4:
+ return *(uint32_t *)ptr;
+ case 8:
+ return *(uint64_t *)ptr;
+ }
+ g_assert_not_reached();
+}
+
+static uint64_t vmstate_n_elems(void *opaque, const VMStateField *field)
+{
+ uint64_t n_elems = 1;
if (field->flags & VMS_ARRAY) {
n_elems = field->num;
- } else if (field->flags & VMS_VARRAY_INT32) {
- n_elems = *(int32_t *)(opaque + field->num_offset);
- } else if (field->flags & VMS_VARRAY_UINT32) {
- n_elems = *(uint32_t *)(opaque + field->num_offset);
- } else if (field->flags & VMS_VARRAY_UINT16) {
- n_elems = *(uint16_t *)(opaque + field->num_offset);
- } else if (field->flags & VMS_VARRAY_UINT8) {
- n_elems = *(uint8_t *)(opaque + field->num_offset);
+ } else if (field->flags & (VMS_VARRAY_INT32 | VMS_VARRAY_UINT32
+ | VMS_VARRAY_UINT16 | VMS_VARRAY_UINT8)) {
+ n_elems = vmstate_read_from_offset(opaque, &field->num_offset);
}
trace_vmstate_n_elems(field->name, n_elems);
return n_elems;
}
-static int vmstate_size(void *opaque, const VMStateField *field)
+static uint64_t vmstate_size(void *opaque, const VMStateField *field)
{
- int size;
+ uint64_t size;
if (field->flags & VMS_VBUFFER) {
- size = *(int32_t *)(opaque + field->size_offset);
+ size = vmstate_read_from_offset(opaque, &field->size_offset);
if (field->flags & VMS_MULTIPLY) {
size *= field->size;
}
@@ -124,7 +137,7 @@ static void vmstate_handle_alloc(void *ptr, const VMStateField *field,
void *opaque)
{
if (field->flags & VMS_POINTER && field->flags & VMS_ALLOC) {
- gsize size = vmstate_size(opaque, field);
+ uint64_t size = vmstate_size(opaque, field);
size *= vmstate_n_elems(opaque, field);
if (size) {
*(void **)ptr = g_malloc(size);
@@ -335,8 +348,9 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
if (exists) {
void *first_elem = opaque + field->offset;
- int i, n_elems = vmstate_n_elems(opaque, field);
- int size = vmstate_size(opaque, field);
+ int i;
+ uint64_t n_elems = vmstate_n_elems(opaque, field);
+ uint64_t size = vmstate_size(opaque, field);
vmstate_handle_alloc(first_elem, field, opaque);
if (field->flags & VMS_POINTER) {
@@ -650,8 +664,9 @@ 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;
- int i, n_elems = vmstate_n_elems(opaque, field);
- int size = vmstate_size(opaque, field);
+ int i;
+ uint64_t n_elems = vmstate_n_elems(opaque, field);
+ uint64_t size = vmstate_size(opaque, field);
JSONWriter *vmdesc_loop = vmdesc;
bool is_prev_null = false;
/*
--
2.53.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 2/4] migration: Introduce VMStateOffset
2026-07-29 22:52 ` [PATCH 2/4] migration: Introduce VMStateOffset Fabiano Rosas
@ 2026-07-30 8:06 ` Vladimir Sementsov-Ogievskiy
2026-07-30 14:45 ` Fabiano Rosas
2026-07-30 14:35 ` Michael S. Tsirkin
1 sibling, 1 reply; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2026-07-30 8:06 UTC (permalink / raw)
To: Fabiano Rosas, qemu-devel; +Cc: Peter Xu, Michael S . Tsirkin, Alexandr Moshkov
On 30.07.26 01:52, Fabiano Rosas wrote:
> When migrating a buffer or array, the vmstate code needs to know the
> size of the buffer and the number of elements of the array. Today a
> vmstate writer can choose from a number of macros that take as last
> input the name of a struct member from where the size/num will be
> read.
>
> At load time, the code will access those value via an opaque pointer
> to the migrated data and therefore it needs to also know the size of
> the struct member at that offset.
>
> Currently that information is communicated by means of the
> VMS_VARRAY_* and VMS_VBUFFER_* flags, where each possible type is
> represented by a flag.
>
> So far, that's all fine, but since the vmstate code makes heavy use of
> macros, handling several types individually (i.e. by name: int,
> int32_t, etc) requires several versions of a same macro, one for each
> type. E.g: VMSTATE_VBUFFER_ALLOC_UINT32
> ^
>
> This creates a pattern where the vmstate writer has to match the macro
> name to the data type and has resulted in the code having a tendency
> of having one macro version for each type, for each type of vmstate.
>
> There is also some cognitive load to deal with, e.g.
> VMSTATE_VARRAY_INT32 doesn't hold an array of int32, it holds an array
> of something else and the number of elements for the array is stored
> in a variable of type int32.
>
> We're now dealing with the scenario where the code has been expecting
> int32_t at some places, but a uint64_t macro variant has been added
> without the code being updated.
>
> To address all these situations, introduce a new struct that will hold
> the offset of the struct members, but also their size, so the various
> extra macros can all be removed and the person writing the vmstate
> doesn't need to care about type-checking. Still, keep a minimum check
> that those fields are at least integers and fit into 64 bits.
>
> What changes:
>
> 1) type checking changes from individual types to a single check for
> all integers;
>
> 2) there are new ways to access the offsets;
>
> num_offset -> num_offset.off
> size_offset -> size_offset.off
> [new] num_offset.size
> [new] size_offset.size
>
> 2) reading the offsets goes from checking the VMS_VARRAY_* flags in an
> if/elseif block to comparing offset.size against the hardcoded
> sizes in bytes;
>
> 3) the VMS_VARRAY_* and VMS_VBUFFER_* flags become obsolete. Removed
> in the next patch;
>
> 4) memory usage increases +1 byte per vmstate;
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> include/migration/vmstate.h | 66 +++++++++++++++++++++++--------------
> migration/savevm.c | 4 +--
> migration/vmstate.c | 51 ++++++++++++++++++----------
> 3 files changed, 76 insertions(+), 45 deletions(-)
>
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index 92a1a9fe98e..2ad3cc01371 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -31,6 +31,7 @@
>
> typedef struct VMStateInfo VMStateInfo;
> typedef struct VMStateField VMStateField;
> +typedef struct VMStateOffset VMStateOffset;
>
> /*
> * VMStateInfo allows customized migration of objects that don't fit in
> @@ -187,6 +188,11 @@ typedef enum {
> MIG_PRI_MAX,
> } MigrationPriority;
>
> +struct VMStateOffset {
> + uint32_t off;
> + uint8_t size;
> +};
A bit strange to have offset field (off) inside "Offset" structure.
VMStatePosition may be?
> +
> struct VMStateField {
> const char *name;
> size_t offset;
> @@ -205,11 +211,11 @@ struct VMStateField {
> * pointer point to.
> */
> size_t size;
> - size_t size_offset;
> + VMStateOffset size_offset;
size_offset variable of type Offset, which has offset and size fields inside. And different
meaning for each "size" and "offset" in this context.. Partly preexisting, but we make
it even more complicated by this patch. Maybe:
VMStatePosition size_pos;
or even
VMStatePosition bufsize_pos;
to also distinguish "buffer size" consept of "size of field containing buffer size".
Side questions: is there any difference between VARRAY of bytes and BUFFER?
We could probably share same @num and @num_offset fields for buffers, an
drop size/size_offset at all.
>
> size_t start;
> int num;
> - size_t num_offset;
> + VMStateOffset num_offset;
> const VMStateInfo *info;
> enum VMStateFlags flags;
> const VMStateDescription *vmsd;
> @@ -328,6 +334,16 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> (type_check(t1, typeof_elt_of_field(t2, f)) \
> + QEMU_BUILD_BUG_ON_ZERO(!QEMU_IS_ARRAY(((t2 *)0)->f)))
>
> +#define type_check_int64(t) \
> + (((ptrdiff_t)0 * (ptrdiff_t)(~((t)0))) + \
why use ptrdiff_t, if want to compatibility with int64?
> + (0 * sizeof(char[(sizeof(t) <= sizeof(uint64_t)) ? 1 : -1])))
could QEMU_BUILD_BUG_ON() be used to make more readable check?
> +
> +#define vmstate_field_offset(_state, _field) { \
> + .off = (offsetof(_state, _field) + \
> + type_check_int64(typeof_field(_state, _field))), \
> + .size = sizeof(typeof_field(_state, _field)), \
> +}
> +
> #define vmstate_offset_value(_state, _field, _type) \
> (offsetof(_state, _field) + \
> type_check(_type, typeof_field(_state, _field)))
> @@ -454,7 +470,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_VARRAY_INT32(_field, _state, _field_num, _version, _info, _type) {\
> .name = (stringify(_field)), \
> .version_id = (_version), \
> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .info = &(_info), \
> .size = sizeof(_type), \
> .flags = VMS_VARRAY_INT32|VMS_POINTER, \
> @@ -464,7 +480,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_VARRAY_UINT32(_field, _state, _field_num, _version, _info, _type) {\
> .name = (stringify(_field)), \
> .version_id = (_version), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .info = &(_info), \
> .size = sizeof(_type), \
> .flags = VMS_VARRAY_UINT32|VMS_POINTER, \
> @@ -474,7 +490,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_VARRAY_INT32_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
> .name = (stringify(_field)), \
> .version_id = (_version), \
> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .info = &(_info), \
> .size = sizeof(_type), \
> .flags = VMS_VARRAY_INT32 | VMS_POINTER | VMS_ALLOC, \
> @@ -484,7 +500,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_VARRAY_UINT32_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
> .name = (stringify(_field)), \
> .version_id = (_version), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .info = &(_info), \
> .size = sizeof(_type), \
> .flags = VMS_VARRAY_UINT32|VMS_POINTER|VMS_ALLOC, \
> @@ -494,7 +510,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_VARRAY_UINT16_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
> .name = (stringify(_field)), \
> .version_id = (_version), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint16_t),\
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .info = &(_info), \
> .size = sizeof(_type), \
> .flags = VMS_VARRAY_UINT16 | VMS_POINTER | VMS_ALLOC, \
> @@ -504,7 +520,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_VARRAY_UINT16_UNSAFE(_field, _state, _field_num, _version, _info, _type) {\
> .name = (stringify(_field)), \
> .version_id = (_version), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint16_t),\
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .info = &(_info), \
> .size = sizeof(_type), \
> .flags = VMS_VARRAY_UINT16, \
> @@ -583,7 +599,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> _field, _state, _field_num, _version, _vmsd, _type) { \
> .name = (stringify(_field)), \
> .version_id = (_version), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint8_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .vmsd = &(_vmsd), \
> .size = sizeof(_type), \
> .flags = VMS_POINTER | VMS_VARRAY_UINT8 | \
> @@ -596,7 +612,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> _field, _state, _field_num, _version, _vmsd, _type) { \
> .name = (stringify(_field)), \
> .version_id = (_version), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .vmsd = &(_vmsd), \
> .size = sizeof(_type), \
> .flags = VMS_POINTER | VMS_VARRAY_UINT32 | \
> @@ -608,7 +624,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_VARRAY_OF_POINTER_UINT32(_field, _state, _field_num, _version, _info, _type) { \
> .name = (stringify(_field)), \
> .version_id = (_version), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .info = &(_info), \
> .flags = VMS_VARRAY_UINT32 | VMS_ARRAY_OF_POINTER | VMS_POINTER, \
> .offset = vmstate_offset_pointer(_state, _field, _type *), \
> @@ -650,7 +666,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>
> #define VMSTATE_STRUCT_VARRAY_UINT8(_field, _state, _field_num, _version, _vmsd, _type) { \
> .name = (stringify(_field)), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint8_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .version_id = (_version), \
> .vmsd = &(_vmsd), \
> .size = sizeof(_type), \
> @@ -674,7 +690,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_STRUCT_VARRAY_POINTER_INT32(_field, _state, _field_num, _vmsd, _type) { \
> .name = (stringify(_field)), \
> .version_id = 0, \
> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .size = sizeof(_type), \
> .vmsd = &(_vmsd), \
> .flags = VMS_POINTER | VMS_VARRAY_INT32 | VMS_STRUCT, \
> @@ -684,7 +700,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_STRUCT_VARRAY_POINTER_UINT32(_field, _state, _field_num, _vmsd, _type) { \
> .name = (stringify(_field)), \
> .version_id = 0, \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .size = sizeof(_type), \
> .vmsd = &(_vmsd), \
> .flags = VMS_POINTER | VMS_VARRAY_INT32 | VMS_STRUCT, \
> @@ -694,7 +710,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_STRUCT_VARRAY_POINTER_UINT16(_field, _state, _field_num, _vmsd, _type) { \
> .name = (stringify(_field)), \
> .version_id = 0, \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint16_t),\
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .size = sizeof(_type), \
> .vmsd = &(_vmsd), \
> .flags = VMS_POINTER | VMS_VARRAY_UINT16 | VMS_STRUCT, \
> @@ -703,7 +719,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>
> #define VMSTATE_STRUCT_VARRAY_INT32(_field, _state, _field_num, _version, _vmsd, _type) { \
> .name = (stringify(_field)), \
> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num, int32_t), \
> .version_id = (_version), \
> .vmsd = &(_vmsd), \
> .size = sizeof(_type), \
> @@ -713,7 +729,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>
> #define VMSTATE_STRUCT_VARRAY_UINT32(_field, _state, _field_num, _version, _vmsd, _type) { \
> .name = (stringify(_field)), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .version_id = (_version), \
> .vmsd = &(_vmsd), \
> .size = sizeof(_type), \
> @@ -725,7 +741,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> .name = (stringify(_field)), \
> .version_id = (_version), \
> .vmsd = &(_vmsd), \
> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .size = sizeof(_type), \
> .flags = VMS_STRUCT|VMS_VARRAY_INT32|VMS_ALLOC|VMS_POINTER, \
> .offset = vmstate_offset_pointer(_state, _field, _type), \
> @@ -746,7 +762,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> .name = (stringify(_field)), \
> .version_id = (_version), \
> .field_exists = (_test), \
> - .size_offset = vmstate_offset_value(_state, _field_size, uint32_t),\
> + .size_offset = vmstate_field_offset(_state, _field_size), \
> .size = (_multiply), \
> .info = &vmstate_info_buffer, \
> .flags = VMS_VBUFFER|VMS_POINTER|VMS_MULTIPLY, \
> @@ -757,7 +773,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> .name = (stringify(_field)), \
> .version_id = (_version), \
> .field_exists = (_test), \
> - .size_offset = vmstate_offset_value(_state, _field_size, int32_t),\
> + .size_offset = vmstate_field_offset(_state, _field_size), \
> .info = &vmstate_info_buffer, \
> .flags = VMS_VBUFFER|VMS_POINTER, \
> .offset = offsetof(_state, _field), \
> @@ -767,7 +783,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> .name = (stringify(_field)), \
> .version_id = (_version), \
> .field_exists = (_test), \
> - .size_offset = vmstate_offset_value(_state, _field_size, uint32_t),\
> + .size_offset = vmstate_field_offset(_state, _field_size), \
> .info = &vmstate_info_buffer, \
> .flags = VMS_VBUFFER|VMS_POINTER, \
> .offset = offsetof(_state, _field), \
> @@ -777,7 +793,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> .name = (stringify(_field)), \
> .version_id = (_version), \
> .field_exists = (_test), \
> - .size_offset = vmstate_offset_value(_state, _field_size, uint64_t),\
> + .size_offset = vmstate_field_offset(_state, _field_size), \
> .info = &vmstate_info_buffer, \
> .flags = VMS_VBUFFER | VMS_POINTER, \
> .offset = offsetof(_state, _field), \
> @@ -788,7 +804,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> .name = (stringify(_field)), \
> .version_id = (_version), \
> .field_exists = (_test), \
> - .size_offset = vmstate_offset_value(_state, _field_size, uint32_t),\
> + .size_offset = vmstate_field_offset(_state, _field_size), \
> .info = &vmstate_info_buffer, \
> .flags = VMS_VBUFFER|VMS_POINTER|VMS_ALLOC, \
> .offset = offsetof(_state, _field), \
> @@ -848,7 +864,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_UNUSED_VARRAY_UINT32(_state, _test, _version, _field_num, _size) {\
> .name = "unused", \
> .field_exists = (_test), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .version_id = (_version), \
> .size = (_size), \
> .info = &vmstate_info_unused_buffer, \
> @@ -862,7 +878,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> .name = (stringify(_field)), \
> .field_exists = (_test), \
> .version_id = (_version), \
> - .size_offset = vmstate_offset_value(_state, _field_size, int32_t),\
> + .size_offset = vmstate_field_offset(_state, _field_size), \
> .info = &vmstate_info_bitmap, \
> .flags = VMS_VBUFFER|VMS_POINTER, \
> .offset = offsetof(_state, _field), \
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 34dd06f9f73..a272bcfd0b4 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -873,14 +873,14 @@ static void vmstate_check(const VMStateDescription *vmsd)
> * Size must be provided because dest QEMU needs that
> * info to know what to allocate
> */
> - assert(field->size || field->size_offset);
> + assert(field->size || field->size_offset.size != 0);
> } else {
> /*
> * Otherwise size info isn't useful (because it's
> * always the size of host pointer), detect accidental
> * setup of sizes in this case.
> */
> - assert(field->size == 0 && field->size_offset == 0);
> + assert(field->size == 0 && field->size_offset.size == 0);
> }
> /*
> * VMS_ARRAY_OF_POINTER must be used only together with one
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index 50ebe378452..0a0b9faa20e 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -78,32 +78,45 @@ vmsd_init_ptr_marker_field(VMStateField *fake, const VMStateField *field)
> };
> }
>
> -static int vmstate_n_elems(void *opaque, const VMStateField *field)
> +static uint64_t vmstate_read_from_offset(void *opaque,
> + const VMStateOffset *offset)
> {
> - int n_elems = 1;
> + uint8_t *ptr = (uint8_t *)opaque + offset->off;
> +
> + switch (offset->size) {
> + case 1:
> + return *(uint8_t *)ptr;
> + case 2:
> + return *(uint16_t *)ptr;
> + case 4:
> + return *(uint32_t *)ptr;
> + case 8:
> + return *(uint64_t *)ptr;
> + }
> + g_assert_not_reached();
> +}
> +
> +static uint64_t vmstate_n_elems(void *opaque, const VMStateField *field)
> +{
> + uint64_t n_elems = 1;
>
> if (field->flags & VMS_ARRAY) {
> n_elems = field->num;
> - } else if (field->flags & VMS_VARRAY_INT32) {
> - n_elems = *(int32_t *)(opaque + field->num_offset);
> - } else if (field->flags & VMS_VARRAY_UINT32) {
> - n_elems = *(uint32_t *)(opaque + field->num_offset);
> - } else if (field->flags & VMS_VARRAY_UINT16) {
> - n_elems = *(uint16_t *)(opaque + field->num_offset);
> - } else if (field->flags & VMS_VARRAY_UINT8) {
> - n_elems = *(uint8_t *)(opaque + field->num_offset);
> + } else if (field->flags & (VMS_VARRAY_INT32 | VMS_VARRAY_UINT32
> + | VMS_VARRAY_UINT16 | VMS_VARRAY_UINT8)) {
> + n_elems = vmstate_read_from_offset(opaque, &field->num_offset);
> }
>
> trace_vmstate_n_elems(field->name, n_elems);
> return n_elems;
> }
>
> -static int vmstate_size(void *opaque, const VMStateField *field)
> +static uint64_t vmstate_size(void *opaque, const VMStateField *field)
> {
> - int size;
> + uint64_t size;
>
> if (field->flags & VMS_VBUFFER) {
> - size = *(int32_t *)(opaque + field->size_offset);
> + size = vmstate_read_from_offset(opaque, &field->size_offset);
> if (field->flags & VMS_MULTIPLY) {
> size *= field->size;
> }
> @@ -124,7 +137,7 @@ static void vmstate_handle_alloc(void *ptr, const VMStateField *field,
> void *opaque)
> {
> if (field->flags & VMS_POINTER && field->flags & VMS_ALLOC) {
> - gsize size = vmstate_size(opaque, field);
> + uint64_t size = vmstate_size(opaque, field);
> size *= vmstate_n_elems(opaque, field);
> if (size) {
> *(void **)ptr = g_malloc(size);
> @@ -335,8 +348,9 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
>
> if (exists) {
> void *first_elem = opaque + field->offset;
> - int i, n_elems = vmstate_n_elems(opaque, field);
> - int size = vmstate_size(opaque, field);
> + int i;
> + uint64_t n_elems = vmstate_n_elems(opaque, field);
> + uint64_t size = vmstate_size(opaque, field);
>
> vmstate_handle_alloc(first_elem, field, opaque);
> if (field->flags & VMS_POINTER) {
> @@ -650,8 +664,9 @@ 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;
> - int i, n_elems = vmstate_n_elems(opaque, field);
> - int size = vmstate_size(opaque, field);
> + int i;
> + uint64_t n_elems = vmstate_n_elems(opaque, field);
> + uint64_t size = vmstate_size(opaque, field);
> JSONWriter *vmdesc_loop = vmdesc;
> bool is_prev_null = false;
> /*
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 2/4] migration: Introduce VMStateOffset
2026-07-30 8:06 ` Vladimir Sementsov-Ogievskiy
@ 2026-07-30 14:45 ` Fabiano Rosas
0 siblings, 0 replies; 22+ messages in thread
From: Fabiano Rosas @ 2026-07-30 14:45 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel
Cc: Peter Xu, Michael S . Tsirkin, Alexandr Moshkov
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> On 30.07.26 01:52, Fabiano Rosas wrote:
>> When migrating a buffer or array, the vmstate code needs to know the
>> size of the buffer and the number of elements of the array. Today a
>> vmstate writer can choose from a number of macros that take as last
>> input the name of a struct member from where the size/num will be
>> read.
>>
>> At load time, the code will access those value via an opaque pointer
>> to the migrated data and therefore it needs to also know the size of
>> the struct member at that offset.
>>
>> Currently that information is communicated by means of the
>> VMS_VARRAY_* and VMS_VBUFFER_* flags, where each possible type is
>> represented by a flag.
>>
>> So far, that's all fine, but since the vmstate code makes heavy use of
>> macros, handling several types individually (i.e. by name: int,
>> int32_t, etc) requires several versions of a same macro, one for each
>> type. E.g: VMSTATE_VBUFFER_ALLOC_UINT32
>> ^
>>
>> This creates a pattern where the vmstate writer has to match the macro
>> name to the data type and has resulted in the code having a tendency
>> of having one macro version for each type, for each type of vmstate.
>>
>> There is also some cognitive load to deal with, e.g.
>> VMSTATE_VARRAY_INT32 doesn't hold an array of int32, it holds an array
>> of something else and the number of elements for the array is stored
>> in a variable of type int32.
>>
>> We're now dealing with the scenario where the code has been expecting
>> int32_t at some places, but a uint64_t macro variant has been added
>> without the code being updated.
>>
>> To address all these situations, introduce a new struct that will hold
>> the offset of the struct members, but also their size, so the various
>> extra macros can all be removed and the person writing the vmstate
>> doesn't need to care about type-checking. Still, keep a minimum check
>> that those fields are at least integers and fit into 64 bits.
>>
>> What changes:
>>
>> 1) type checking changes from individual types to a single check for
>> all integers;
>>
>> 2) there are new ways to access the offsets;
>>
>> num_offset -> num_offset.off
>> size_offset -> size_offset.off
>> [new] num_offset.size
>> [new] size_offset.size
>>
>> 2) reading the offsets goes from checking the VMS_VARRAY_* flags in an
>> if/elseif block to comparing offset.size against the hardcoded
>> sizes in bytes;
>>
>> 3) the VMS_VARRAY_* and VMS_VBUFFER_* flags become obsolete. Removed
>> in the next patch;
>>
>> 4) memory usage increases +1 byte per vmstate;
>>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>> include/migration/vmstate.h | 66 +++++++++++++++++++++++--------------
>> migration/savevm.c | 4 +--
>> migration/vmstate.c | 51 ++++++++++++++++++----------
>> 3 files changed, 76 insertions(+), 45 deletions(-)
>>
>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
>> index 92a1a9fe98e..2ad3cc01371 100644
>> --- a/include/migration/vmstate.h
>> +++ b/include/migration/vmstate.h
>> @@ -31,6 +31,7 @@
>>
>> typedef struct VMStateInfo VMStateInfo;
>> typedef struct VMStateField VMStateField;
>> +typedef struct VMStateOffset VMStateOffset;
>>
>> /*
>> * VMStateInfo allows customized migration of objects that don't fit in
>> @@ -187,6 +188,11 @@ typedef enum {
>> MIG_PRI_MAX,
>> } MigrationPriority;
>>
>> +struct VMStateOffset {
>> + uint32_t off;
>> + uint8_t size;
>> +};
>
> A bit strange to have offset field (off) inside "Offset" structure.
>
Yes! All the naming is this series is a bit off (hehe), I didn't want to
start changing too much and diverting from the core of the series.
> VMStatePosition may be?
>
I want to try to use that struct for the main offset also and for
storing the maximum size of any "len" and "size" fields. Maybe:
VMStateIndirectField // as in, not the main field being migrated
VMStateMetaField // as in, it brings extra data about the main field
If we have Field in there, then we can treat this as the main
VMStateField, but with less stuff in it. I think it makes sense because
then "offset" and "size" map naturally.
>> +
>> struct VMStateField {
>> const char *name;
>> size_t offset;
>> @@ -205,11 +211,11 @@ struct VMStateField {
>> * pointer point to.
>> */
>> size_t size;
>> - size_t size_offset;
>> + VMStateOffset size_offset;
>
> size_offset variable of type Offset, which has offset and size fields inside. And different
> meaning for each "size" and "offset" in this context.. Partly preexisting, but we make
> it even more complicated by this patch. Maybe:
>
Yes, even writing the commit message is hard, we don't have good terms
yet. Let's see what others think.
> VMStatePosition size_pos;
>
> or even
>
> VMStatePosition bufsize_pos;
>
> to also distinguish "buffer size" consept of "size of field containing buffer size".
>
> Side questions: is there any difference between VARRAY of bytes and BUFFER?
> We could probably share same @num and @num_offset fields for buffers, an
> drop size/size_offset at all.
>
Hm, maybe, that would be nice indeed. Let me check.
>>
>> size_t start;
>> int num;
>> - size_t num_offset;
>> + VMStateOffset num_offset;
>> const VMStateInfo *info;
>> enum VMStateFlags flags;
>> const VMStateDescription *vmsd;
>> @@ -328,6 +334,16 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> (type_check(t1, typeof_elt_of_field(t2, f)) \
>> + QEMU_BUILD_BUG_ON_ZERO(!QEMU_IS_ARRAY(((t2 *)0)->f)))
>>
>> +#define type_check_int64(t) \
>> + (((ptrdiff_t)0 * (ptrdiff_t)(~((t)0))) + \
>
> why use ptrdiff_t, if want to compatibility with int64?
>
Yeah, I think that cast is useless here. I changed this macro a lot
because it was having issues with the gcc-11 from the CI. I'll fix it.
>> + (0 * sizeof(char[(sizeof(t) <= sizeof(uint64_t)) ? 1 : -1])))
>
> could QEMU_BUILD_BUG_ON() be used to make more readable check?
>
Good idea. Some trickery is still needed to satisfy the compiler, but it
seems more readable and the error message now shows the type name:
#define type_check_int64(t) \
(~((t)0) * sizeof(struct { \
QEMU_BUILD_BUG_ON(sizeof(t) > sizeof(uint64_t)); \
}))
>> +
>> +#define vmstate_field_offset(_state, _field) { \
>> + .off = (offsetof(_state, _field) + \
>> + type_check_int64(typeof_field(_state, _field))), \
>> + .size = sizeof(typeof_field(_state, _field)), \
>> +}
>> +
>> #define vmstate_offset_value(_state, _field, _type) \
>> (offsetof(_state, _field) + \
>> type_check(_type, typeof_field(_state, _field)))
>> @@ -454,7 +470,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_VARRAY_INT32(_field, _state, _field_num, _version, _info, _type) {\
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .info = &(_info), \
>> .size = sizeof(_type), \
>> .flags = VMS_VARRAY_INT32|VMS_POINTER, \
>> @@ -464,7 +480,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_VARRAY_UINT32(_field, _state, _field_num, _version, _info, _type) {\
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .info = &(_info), \
>> .size = sizeof(_type), \
>> .flags = VMS_VARRAY_UINT32|VMS_POINTER, \
>> @@ -474,7 +490,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_VARRAY_INT32_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .info = &(_info), \
>> .size = sizeof(_type), \
>> .flags = VMS_VARRAY_INT32 | VMS_POINTER | VMS_ALLOC, \
>> @@ -484,7 +500,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_VARRAY_UINT32_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .info = &(_info), \
>> .size = sizeof(_type), \
>> .flags = VMS_VARRAY_UINT32|VMS_POINTER|VMS_ALLOC, \
>> @@ -494,7 +510,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_VARRAY_UINT16_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint16_t),\
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .info = &(_info), \
>> .size = sizeof(_type), \
>> .flags = VMS_VARRAY_UINT16 | VMS_POINTER | VMS_ALLOC, \
>> @@ -504,7 +520,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_VARRAY_UINT16_UNSAFE(_field, _state, _field_num, _version, _info, _type) {\
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint16_t),\
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .info = &(_info), \
>> .size = sizeof(_type), \
>> .flags = VMS_VARRAY_UINT16, \
>> @@ -583,7 +599,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> _field, _state, _field_num, _version, _vmsd, _type) { \
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint8_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .vmsd = &(_vmsd), \
>> .size = sizeof(_type), \
>> .flags = VMS_POINTER | VMS_VARRAY_UINT8 | \
>> @@ -596,7 +612,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> _field, _state, _field_num, _version, _vmsd, _type) { \
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .vmsd = &(_vmsd), \
>> .size = sizeof(_type), \
>> .flags = VMS_POINTER | VMS_VARRAY_UINT32 | \
>> @@ -608,7 +624,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_VARRAY_OF_POINTER_UINT32(_field, _state, _field_num, _version, _info, _type) { \
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .info = &(_info), \
>> .flags = VMS_VARRAY_UINT32 | VMS_ARRAY_OF_POINTER | VMS_POINTER, \
>> .offset = vmstate_offset_pointer(_state, _field, _type *), \
>> @@ -650,7 +666,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>>
>> #define VMSTATE_STRUCT_VARRAY_UINT8(_field, _state, _field_num, _version, _vmsd, _type) { \
>> .name = (stringify(_field)), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint8_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .version_id = (_version), \
>> .vmsd = &(_vmsd), \
>> .size = sizeof(_type), \
>> @@ -674,7 +690,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_STRUCT_VARRAY_POINTER_INT32(_field, _state, _field_num, _vmsd, _type) { \
>> .name = (stringify(_field)), \
>> .version_id = 0, \
>> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .size = sizeof(_type), \
>> .vmsd = &(_vmsd), \
>> .flags = VMS_POINTER | VMS_VARRAY_INT32 | VMS_STRUCT, \
>> @@ -684,7 +700,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_STRUCT_VARRAY_POINTER_UINT32(_field, _state, _field_num, _vmsd, _type) { \
>> .name = (stringify(_field)), \
>> .version_id = 0, \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .size = sizeof(_type), \
>> .vmsd = &(_vmsd), \
>> .flags = VMS_POINTER | VMS_VARRAY_INT32 | VMS_STRUCT, \
>> @@ -694,7 +710,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_STRUCT_VARRAY_POINTER_UINT16(_field, _state, _field_num, _vmsd, _type) { \
>> .name = (stringify(_field)), \
>> .version_id = 0, \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint16_t),\
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .size = sizeof(_type), \
>> .vmsd = &(_vmsd), \
>> .flags = VMS_POINTER | VMS_VARRAY_UINT16 | VMS_STRUCT, \
>> @@ -703,7 +719,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>>
>> #define VMSTATE_STRUCT_VARRAY_INT32(_field, _state, _field_num, _version, _vmsd, _type) { \
>> .name = (stringify(_field)), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num, int32_t), \
>> .version_id = (_version), \
>> .vmsd = &(_vmsd), \
>> .size = sizeof(_type), \
>> @@ -713,7 +729,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>>
>> #define VMSTATE_STRUCT_VARRAY_UINT32(_field, _state, _field_num, _version, _vmsd, _type) { \
>> .name = (stringify(_field)), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .version_id = (_version), \
>> .vmsd = &(_vmsd), \
>> .size = sizeof(_type), \
>> @@ -725,7 +741,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> .vmsd = &(_vmsd), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .size = sizeof(_type), \
>> .flags = VMS_STRUCT|VMS_VARRAY_INT32|VMS_ALLOC|VMS_POINTER, \
>> .offset = vmstate_offset_pointer(_state, _field, _type), \
>> @@ -746,7 +762,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> .field_exists = (_test), \
>> - .size_offset = vmstate_offset_value(_state, _field_size, uint32_t),\
>> + .size_offset = vmstate_field_offset(_state, _field_size), \
>> .size = (_multiply), \
>> .info = &vmstate_info_buffer, \
>> .flags = VMS_VBUFFER|VMS_POINTER|VMS_MULTIPLY, \
>> @@ -757,7 +773,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> .field_exists = (_test), \
>> - .size_offset = vmstate_offset_value(_state, _field_size, int32_t),\
>> + .size_offset = vmstate_field_offset(_state, _field_size), \
>> .info = &vmstate_info_buffer, \
>> .flags = VMS_VBUFFER|VMS_POINTER, \
>> .offset = offsetof(_state, _field), \
>> @@ -767,7 +783,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> .field_exists = (_test), \
>> - .size_offset = vmstate_offset_value(_state, _field_size, uint32_t),\
>> + .size_offset = vmstate_field_offset(_state, _field_size), \
>> .info = &vmstate_info_buffer, \
>> .flags = VMS_VBUFFER|VMS_POINTER, \
>> .offset = offsetof(_state, _field), \
>> @@ -777,7 +793,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> .field_exists = (_test), \
>> - .size_offset = vmstate_offset_value(_state, _field_size, uint64_t),\
>> + .size_offset = vmstate_field_offset(_state, _field_size), \
>> .info = &vmstate_info_buffer, \
>> .flags = VMS_VBUFFER | VMS_POINTER, \
>> .offset = offsetof(_state, _field), \
>> @@ -788,7 +804,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> .field_exists = (_test), \
>> - .size_offset = vmstate_offset_value(_state, _field_size, uint32_t),\
>> + .size_offset = vmstate_field_offset(_state, _field_size), \
>> .info = &vmstate_info_buffer, \
>> .flags = VMS_VBUFFER|VMS_POINTER|VMS_ALLOC, \
>> .offset = offsetof(_state, _field), \
>> @@ -848,7 +864,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_UNUSED_VARRAY_UINT32(_state, _test, _version, _field_num, _size) {\
>> .name = "unused", \
>> .field_exists = (_test), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .version_id = (_version), \
>> .size = (_size), \
>> .info = &vmstate_info_unused_buffer, \
>> @@ -862,7 +878,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> .name = (stringify(_field)), \
>> .field_exists = (_test), \
>> .version_id = (_version), \
>> - .size_offset = vmstate_offset_value(_state, _field_size, int32_t),\
>> + .size_offset = vmstate_field_offset(_state, _field_size), \
>> .info = &vmstate_info_bitmap, \
>> .flags = VMS_VBUFFER|VMS_POINTER, \
>> .offset = offsetof(_state, _field), \
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index 34dd06f9f73..a272bcfd0b4 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -873,14 +873,14 @@ static void vmstate_check(const VMStateDescription *vmsd)
>> * Size must be provided because dest QEMU needs that
>> * info to know what to allocate
>> */
>> - assert(field->size || field->size_offset);
>> + assert(field->size || field->size_offset.size != 0);
>> } else {
>> /*
>> * Otherwise size info isn't useful (because it's
>> * always the size of host pointer), detect accidental
>> * setup of sizes in this case.
>> */
>> - assert(field->size == 0 && field->size_offset == 0);
>> + assert(field->size == 0 && field->size_offset.size == 0);
>> }
>> /*
>> * VMS_ARRAY_OF_POINTER must be used only together with one
>> diff --git a/migration/vmstate.c b/migration/vmstate.c
>> index 50ebe378452..0a0b9faa20e 100644
>> --- a/migration/vmstate.c
>> +++ b/migration/vmstate.c
>> @@ -78,32 +78,45 @@ vmsd_init_ptr_marker_field(VMStateField *fake, const VMStateField *field)
>> };
>> }
>>
>> -static int vmstate_n_elems(void *opaque, const VMStateField *field)
>> +static uint64_t vmstate_read_from_offset(void *opaque,
>> + const VMStateOffset *offset)
>> {
>> - int n_elems = 1;
>> + uint8_t *ptr = (uint8_t *)opaque + offset->off;
>> +
>> + switch (offset->size) {
>> + case 1:
>> + return *(uint8_t *)ptr;
>> + case 2:
>> + return *(uint16_t *)ptr;
>> + case 4:
>> + return *(uint32_t *)ptr;
>> + case 8:
>> + return *(uint64_t *)ptr;
>> + }
>> + g_assert_not_reached();
>> +}
>> +
>> +static uint64_t vmstate_n_elems(void *opaque, const VMStateField *field)
>> +{
>> + uint64_t n_elems = 1;
>>
>> if (field->flags & VMS_ARRAY) {
>> n_elems = field->num;
>> - } else if (field->flags & VMS_VARRAY_INT32) {
>> - n_elems = *(int32_t *)(opaque + field->num_offset);
>> - } else if (field->flags & VMS_VARRAY_UINT32) {
>> - n_elems = *(uint32_t *)(opaque + field->num_offset);
>> - } else if (field->flags & VMS_VARRAY_UINT16) {
>> - n_elems = *(uint16_t *)(opaque + field->num_offset);
>> - } else if (field->flags & VMS_VARRAY_UINT8) {
>> - n_elems = *(uint8_t *)(opaque + field->num_offset);
>> + } else if (field->flags & (VMS_VARRAY_INT32 | VMS_VARRAY_UINT32
>> + | VMS_VARRAY_UINT16 | VMS_VARRAY_UINT8)) {
>> + n_elems = vmstate_read_from_offset(opaque, &field->num_offset);
>> }
>>
>> trace_vmstate_n_elems(field->name, n_elems);
>> return n_elems;
>> }
>>
>> -static int vmstate_size(void *opaque, const VMStateField *field)
>> +static uint64_t vmstate_size(void *opaque, const VMStateField *field)
>> {
>> - int size;
>> + uint64_t size;
>>
>> if (field->flags & VMS_VBUFFER) {
>> - size = *(int32_t *)(opaque + field->size_offset);
>> + size = vmstate_read_from_offset(opaque, &field->size_offset);
>> if (field->flags & VMS_MULTIPLY) {
>> size *= field->size;
>> }
>> @@ -124,7 +137,7 @@ static void vmstate_handle_alloc(void *ptr, const VMStateField *field,
>> void *opaque)
>> {
>> if (field->flags & VMS_POINTER && field->flags & VMS_ALLOC) {
>> - gsize size = vmstate_size(opaque, field);
>> + uint64_t size = vmstate_size(opaque, field);
>> size *= vmstate_n_elems(opaque, field);
>> if (size) {
>> *(void **)ptr = g_malloc(size);
>> @@ -335,8 +348,9 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
>>
>> if (exists) {
>> void *first_elem = opaque + field->offset;
>> - int i, n_elems = vmstate_n_elems(opaque, field);
>> - int size = vmstate_size(opaque, field);
>> + int i;
>> + uint64_t n_elems = vmstate_n_elems(opaque, field);
>> + uint64_t size = vmstate_size(opaque, field);
>>
>> vmstate_handle_alloc(first_elem, field, opaque);
>> if (field->flags & VMS_POINTER) {
>> @@ -650,8 +664,9 @@ 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;
>> - int i, n_elems = vmstate_n_elems(opaque, field);
>> - int size = vmstate_size(opaque, field);
>> + int i;
>> + uint64_t n_elems = vmstate_n_elems(opaque, field);
>> + uint64_t size = vmstate_size(opaque, field);
>> JSONWriter *vmdesc_loop = vmdesc;
>> bool is_prev_null = false;
>> /*
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] migration: Introduce VMStateOffset
2026-07-29 22:52 ` [PATCH 2/4] migration: Introduce VMStateOffset Fabiano Rosas
2026-07-30 8:06 ` Vladimir Sementsov-Ogievskiy
@ 2026-07-30 14:35 ` Michael S. Tsirkin
2026-07-30 15:15 ` Fabiano Rosas
2026-07-30 15:23 ` Peter Xu
1 sibling, 2 replies; 22+ messages in thread
From: Michael S. Tsirkin @ 2026-07-30 14:35 UTC (permalink / raw)
To: Fabiano Rosas; +Cc: qemu-devel, Peter Xu, Alexandr Moshkov
On Wed, Jul 29, 2026 at 07:52:25PM -0300, Fabiano Rosas wrote:
> When migrating a buffer or array, the vmstate code needs to know the
> size of the buffer and the number of elements of the array. Today a
> vmstate writer can choose from a number of macros that take as last
> input the name of a struct member from where the size/num will be
> read.
>
> At load time, the code will access those value via an opaque pointer
> to the migrated data and therefore it needs to also know the size of
> the struct member at that offset.
>
> Currently that information is communicated by means of the
> VMS_VARRAY_* and VMS_VBUFFER_* flags, where each possible type is
> represented by a flag.
>
> So far, that's all fine, but since the vmstate code makes heavy use of
> macros, handling several types individually (i.e. by name: int,
> int32_t, etc) requires several versions of a same macro, one for each
> type. E.g: VMSTATE_VBUFFER_ALLOC_UINT32
> ^
>
> This creates a pattern where the vmstate writer has to match the macro
> name to the data type and has resulted in the code having a tendency
> of having one macro version for each type, for each type of vmstate.
>
> There is also some cognitive load to deal with, e.g.
> VMSTATE_VARRAY_INT32 doesn't hold an array of int32, it holds an array
> of something else and the number of elements for the array is stored
> in a variable of type int32.
>
> We're now dealing with the scenario where the code has been expecting
> int32_t at some places, but a uint64_t macro variant has been added
> without the code being updated.
>
> To address all these situations, introduce a new struct that will hold
> the offset of the struct members, but also their size, so the various
> extra macros can all be removed and the person writing the vmstate
> doesn't need to care about type-checking. Still, keep a minimum check
> that those fields are at least integers and fit into 64 bits.
>
> What changes:
>
> 1) type checking changes from individual types to a single check for
> all integers;
>
> 2) there are new ways to access the offsets;
>
> num_offset -> num_offset.off
> size_offset -> size_offset.off
> [new] num_offset.size
> [new] size_offset.size
>
> 2) reading the offsets goes from checking the VMS_VARRAY_* flags in an
> if/elseif block to comparing offset.size against the hardcoded
> sizes in bytes;
>
> 3) the VMS_VARRAY_* and VMS_VBUFFER_* flags become obsolete. Removed
> in the next patch;
>
> 4) memory usage increases +1 byte per vmstate;
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
Nice work! Just some minor comments below.
> ---
> include/migration/vmstate.h | 66 +++++++++++++++++++++++--------------
> migration/savevm.c | 4 +--
> migration/vmstate.c | 51 ++++++++++++++++++----------
> 3 files changed, 76 insertions(+), 45 deletions(-)
>
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index 92a1a9fe98e..2ad3cc01371 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -31,6 +31,7 @@
>
> typedef struct VMStateInfo VMStateInfo;
> typedef struct VMStateField VMStateField;
> +typedef struct VMStateOffset VMStateOffset;
>
> /*
> * VMStateInfo allows customized migration of objects that don't fit in
> @@ -187,6 +188,11 @@ typedef enum {
> MIG_PRI_MAX,
> } MigrationPriority;
>
> +struct VMStateOffset {
> + uint32_t off;
> + uint8_t size;
> +};
> +
I think it would be great to add some comments here.
What does the struct describe? size and offset of what where?
And IIUC size 0 has a special meaning?
And neither offset nor size ever come from the migration stream, right?
> struct VMStateField {
> const char *name;
> size_t offset;
> @@ -205,11 +211,11 @@ struct VMStateField {
> * pointer point to.
> */
> size_t size;
> - size_t size_offset;
> + VMStateOffset size_offset;
>
> size_t start;
> int num;
> - size_t num_offset;
> + VMStateOffset num_offset;
> const VMStateInfo *info;
> enum VMStateFlags flags;
> const VMStateDescription *vmsd;
> @@ -328,6 +334,16 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> (type_check(t1, typeof_elt_of_field(t2, f)) \
> + QEMU_BUILD_BUG_ON_ZERO(!QEMU_IS_ARRAY(((t2 *)0)->f)))
>
> +#define type_check_int64(t) \
> + (((ptrdiff_t)0 * (ptrdiff_t)(~((t)0))) + \
> + (0 * sizeof(char[(sizeof(t) <= sizeof(uint64_t)) ? 1 : -1])))
> +
> +#define vmstate_field_offset(_state, _field) { \
> + .off = (offsetof(_state, _field) + \
> + type_check_int64(typeof_field(_state, _field))), \
> + .size = sizeof(typeof_field(_state, _field)), \
> +}
> +
> #define vmstate_offset_value(_state, _field, _type) \
> (offsetof(_state, _field) + \
> type_check(_type, typeof_field(_state, _field)))
> @@ -454,7 +470,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_VARRAY_INT32(_field, _state, _field_num, _version, _info, _type) {\
> .name = (stringify(_field)), \
> .version_id = (_version), \
> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .info = &(_info), \
> .size = sizeof(_type), \
> .flags = VMS_VARRAY_INT32|VMS_POINTER, \
> @@ -464,7 +480,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_VARRAY_UINT32(_field, _state, _field_num, _version, _info, _type) {\
> .name = (stringify(_field)), \
> .version_id = (_version), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .info = &(_info), \
> .size = sizeof(_type), \
> .flags = VMS_VARRAY_UINT32|VMS_POINTER, \
> @@ -474,7 +490,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_VARRAY_INT32_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
> .name = (stringify(_field)), \
> .version_id = (_version), \
> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .info = &(_info), \
> .size = sizeof(_type), \
> .flags = VMS_VARRAY_INT32 | VMS_POINTER | VMS_ALLOC, \
> @@ -484,7 +500,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_VARRAY_UINT32_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
> .name = (stringify(_field)), \
> .version_id = (_version), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .info = &(_info), \
> .size = sizeof(_type), \
> .flags = VMS_VARRAY_UINT32|VMS_POINTER|VMS_ALLOC, \
> @@ -494,7 +510,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_VARRAY_UINT16_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
> .name = (stringify(_field)), \
> .version_id = (_version), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint16_t),\
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .info = &(_info), \
> .size = sizeof(_type), \
> .flags = VMS_VARRAY_UINT16 | VMS_POINTER | VMS_ALLOC, \
> @@ -504,7 +520,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_VARRAY_UINT16_UNSAFE(_field, _state, _field_num, _version, _info, _type) {\
> .name = (stringify(_field)), \
> .version_id = (_version), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint16_t),\
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .info = &(_info), \
> .size = sizeof(_type), \
> .flags = VMS_VARRAY_UINT16, \
> @@ -583,7 +599,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> _field, _state, _field_num, _version, _vmsd, _type) { \
> .name = (stringify(_field)), \
> .version_id = (_version), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint8_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .vmsd = &(_vmsd), \
> .size = sizeof(_type), \
> .flags = VMS_POINTER | VMS_VARRAY_UINT8 | \
> @@ -596,7 +612,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> _field, _state, _field_num, _version, _vmsd, _type) { \
> .name = (stringify(_field)), \
> .version_id = (_version), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .vmsd = &(_vmsd), \
> .size = sizeof(_type), \
> .flags = VMS_POINTER | VMS_VARRAY_UINT32 | \
> @@ -608,7 +624,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_VARRAY_OF_POINTER_UINT32(_field, _state, _field_num, _version, _info, _type) { \
> .name = (stringify(_field)), \
> .version_id = (_version), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .info = &(_info), \
> .flags = VMS_VARRAY_UINT32 | VMS_ARRAY_OF_POINTER | VMS_POINTER, \
> .offset = vmstate_offset_pointer(_state, _field, _type *), \
> @@ -650,7 +666,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>
> #define VMSTATE_STRUCT_VARRAY_UINT8(_field, _state, _field_num, _version, _vmsd, _type) { \
> .name = (stringify(_field)), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint8_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .version_id = (_version), \
> .vmsd = &(_vmsd), \
> .size = sizeof(_type), \
> @@ -674,7 +690,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_STRUCT_VARRAY_POINTER_INT32(_field, _state, _field_num, _vmsd, _type) { \
> .name = (stringify(_field)), \
> .version_id = 0, \
> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .size = sizeof(_type), \
> .vmsd = &(_vmsd), \
> .flags = VMS_POINTER | VMS_VARRAY_INT32 | VMS_STRUCT, \
> @@ -684,7 +700,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_STRUCT_VARRAY_POINTER_UINT32(_field, _state, _field_num, _vmsd, _type) { \
> .name = (stringify(_field)), \
> .version_id = 0, \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .size = sizeof(_type), \
> .vmsd = &(_vmsd), \
> .flags = VMS_POINTER | VMS_VARRAY_INT32 | VMS_STRUCT, \
> @@ -694,7 +710,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_STRUCT_VARRAY_POINTER_UINT16(_field, _state, _field_num, _vmsd, _type) { \
> .name = (stringify(_field)), \
> .version_id = 0, \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint16_t),\
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .size = sizeof(_type), \
> .vmsd = &(_vmsd), \
> .flags = VMS_POINTER | VMS_VARRAY_UINT16 | VMS_STRUCT, \
> @@ -703,7 +719,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>
> #define VMSTATE_STRUCT_VARRAY_INT32(_field, _state, _field_num, _version, _vmsd, _type) { \
> .name = (stringify(_field)), \
> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num, int32_t), \
> .version_id = (_version), \
> .vmsd = &(_vmsd), \
> .size = sizeof(_type), \
> @@ -713,7 +729,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>
> #define VMSTATE_STRUCT_VARRAY_UINT32(_field, _state, _field_num, _version, _vmsd, _type) { \
> .name = (stringify(_field)), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .version_id = (_version), \
> .vmsd = &(_vmsd), \
> .size = sizeof(_type), \
> @@ -725,7 +741,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> .name = (stringify(_field)), \
> .version_id = (_version), \
> .vmsd = &(_vmsd), \
> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .size = sizeof(_type), \
> .flags = VMS_STRUCT|VMS_VARRAY_INT32|VMS_ALLOC|VMS_POINTER, \
> .offset = vmstate_offset_pointer(_state, _field, _type), \
> @@ -746,7 +762,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> .name = (stringify(_field)), \
> .version_id = (_version), \
> .field_exists = (_test), \
> - .size_offset = vmstate_offset_value(_state, _field_size, uint32_t),\
> + .size_offset = vmstate_field_offset(_state, _field_size), \
> .size = (_multiply), \
> .info = &vmstate_info_buffer, \
> .flags = VMS_VBUFFER|VMS_POINTER|VMS_MULTIPLY, \
> @@ -757,7 +773,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> .name = (stringify(_field)), \
> .version_id = (_version), \
> .field_exists = (_test), \
> - .size_offset = vmstate_offset_value(_state, _field_size, int32_t),\
> + .size_offset = vmstate_field_offset(_state, _field_size), \
> .info = &vmstate_info_buffer, \
> .flags = VMS_VBUFFER|VMS_POINTER, \
> .offset = offsetof(_state, _field), \
> @@ -767,7 +783,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> .name = (stringify(_field)), \
> .version_id = (_version), \
> .field_exists = (_test), \
> - .size_offset = vmstate_offset_value(_state, _field_size, uint32_t),\
> + .size_offset = vmstate_field_offset(_state, _field_size), \
> .info = &vmstate_info_buffer, \
> .flags = VMS_VBUFFER|VMS_POINTER, \
> .offset = offsetof(_state, _field), \
> @@ -777,7 +793,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> .name = (stringify(_field)), \
> .version_id = (_version), \
> .field_exists = (_test), \
> - .size_offset = vmstate_offset_value(_state, _field_size, uint64_t),\
> + .size_offset = vmstate_field_offset(_state, _field_size), \
> .info = &vmstate_info_buffer, \
> .flags = VMS_VBUFFER | VMS_POINTER, \
> .offset = offsetof(_state, _field), \
> @@ -788,7 +804,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> .name = (stringify(_field)), \
> .version_id = (_version), \
> .field_exists = (_test), \
> - .size_offset = vmstate_offset_value(_state, _field_size, uint32_t),\
> + .size_offset = vmstate_field_offset(_state, _field_size), \
> .info = &vmstate_info_buffer, \
> .flags = VMS_VBUFFER|VMS_POINTER|VMS_ALLOC, \
> .offset = offsetof(_state, _field), \
> @@ -848,7 +864,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> #define VMSTATE_UNUSED_VARRAY_UINT32(_state, _test, _version, _field_num, _size) {\
> .name = "unused", \
> .field_exists = (_test), \
> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
> + .num_offset = vmstate_field_offset(_state, _field_num), \
> .version_id = (_version), \
> .size = (_size), \
> .info = &vmstate_info_unused_buffer, \
> @@ -862,7 +878,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> .name = (stringify(_field)), \
> .field_exists = (_test), \
> .version_id = (_version), \
> - .size_offset = vmstate_offset_value(_state, _field_size, int32_t),\
> + .size_offset = vmstate_field_offset(_state, _field_size), \
> .info = &vmstate_info_bitmap, \
> .flags = VMS_VBUFFER|VMS_POINTER, \
> .offset = offsetof(_state, _field), \
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 34dd06f9f73..a272bcfd0b4 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -873,14 +873,14 @@ static void vmstate_check(const VMStateDescription *vmsd)
> * Size must be provided because dest QEMU needs that
> * info to know what to allocate
> */
> - assert(field->size || field->size_offset);
> + assert(field->size || field->size_offset.size != 0);
Why is field->size checked by coersing it to bool, but
field->size_offset.size - by comparison to 0?
field->size != 0 for consistency?
> } else {
> /*
> * Otherwise size info isn't useful (because it's
> * always the size of host pointer), detect accidental
> * setup of sizes in this case.
> */
> - assert(field->size == 0 && field->size_offset == 0);
> + assert(field->size == 0 && field->size_offset.size == 0);
> }
> /*
> * VMS_ARRAY_OF_POINTER must be used only together with one
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index 50ebe378452..0a0b9faa20e 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -78,32 +78,45 @@ vmsd_init_ptr_marker_field(VMStateField *fake, const VMStateField *field)
> };
> }
>
> -static int vmstate_n_elems(void *opaque, const VMStateField *field)
> +static uint64_t vmstate_read_from_offset(void *opaque,
> + const VMStateOffset *offset)
> {
> - int n_elems = 1;
> + uint8_t *ptr = (uint8_t *)opaque + offset->off;
> +
The below can easily produce at least UB if offset is
not size aligned.
I *think* offset and size both come from macros that guarantee
this never happens, but maybe better be safe.
> + switch (offset->size) {
> + case 1:
> + return *(uint8_t *)ptr;
> + case 2:
> + return *(uint16_t *)ptr;
> + case 4:
> + return *(uint32_t *)ptr;
> + case 8:
> + return *(uint64_t *)ptr;
> + }
> + g_assert_not_reached();
> +}
> +
> +static uint64_t vmstate_n_elems(void *opaque, const VMStateField *field)
> +{
> + uint64_t n_elems = 1;
>
> if (field->flags & VMS_ARRAY) {
> n_elems = field->num;
> - } else if (field->flags & VMS_VARRAY_INT32) {
> - n_elems = *(int32_t *)(opaque + field->num_offset);
> - } else if (field->flags & VMS_VARRAY_UINT32) {
> - n_elems = *(uint32_t *)(opaque + field->num_offset);
> - } else if (field->flags & VMS_VARRAY_UINT16) {
> - n_elems = *(uint16_t *)(opaque + field->num_offset);
> - } else if (field->flags & VMS_VARRAY_UINT8) {
> - n_elems = *(uint8_t *)(opaque + field->num_offset);
> + } else if (field->flags & (VMS_VARRAY_INT32 | VMS_VARRAY_UINT32
> + | VMS_VARRAY_UINT16 | VMS_VARRAY_UINT8)) {
> + n_elems = vmstate_read_from_offset(opaque, &field->num_offset);
> }
>
> trace_vmstate_n_elems(field->name, n_elems);
> return n_elems;
> }
>
> -static int vmstate_size(void *opaque, const VMStateField *field)
> +static uint64_t vmstate_size(void *opaque, const VMStateField *field)
> {
> - int size;
> + uint64_t size;
>
> if (field->flags & VMS_VBUFFER) {
> - size = *(int32_t *)(opaque + field->size_offset);
> + size = vmstate_read_from_offset(opaque, &field->size_offset);
> if (field->flags & VMS_MULTIPLY) {
> size *= field->size;
> }
> @@ -124,7 +137,7 @@ static void vmstate_handle_alloc(void *ptr, const VMStateField *field,
> void *opaque)
> {
> if (field->flags & VMS_POINTER && field->flags & VMS_ALLOC) {
> - gsize size = vmstate_size(opaque, field);
> + uint64_t size = vmstate_size(opaque, field);
> size *= vmstate_n_elems(opaque, field);
> if (size) {
> *(void **)ptr = g_malloc(size);
> @@ -335,8 +348,9 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
>
> if (exists) {
> void *first_elem = opaque + field->offset;
> - int i, n_elems = vmstate_n_elems(opaque, field);
> - int size = vmstate_size(opaque, field);
> + int i;
> + uint64_t n_elems = vmstate_n_elems(opaque, field);
> + uint64_t size = vmstate_size(opaque, field);
>
> vmstate_handle_alloc(first_elem, field, opaque);
> if (field->flags & VMS_POINTER) {
> @@ -650,8 +664,9 @@ 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;
> - int i, n_elems = vmstate_n_elems(opaque, field);
> - int size = vmstate_size(opaque, field);
> + int i;
> + uint64_t n_elems = vmstate_n_elems(opaque, field);
> + uint64_t size = vmstate_size(opaque, field);
> JSONWriter *vmdesc_loop = vmdesc;
> bool is_prev_null = false;
> /*
> --
> 2.53.0
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 2/4] migration: Introduce VMStateOffset
2026-07-30 14:35 ` Michael S. Tsirkin
@ 2026-07-30 15:15 ` Fabiano Rosas
2026-07-30 15:23 ` Peter Xu
1 sibling, 0 replies; 22+ messages in thread
From: Fabiano Rosas @ 2026-07-30 15:15 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel, Peter Xu, Alexandr Moshkov
"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Wed, Jul 29, 2026 at 07:52:25PM -0300, Fabiano Rosas wrote:
>> When migrating a buffer or array, the vmstate code needs to know the
>> size of the buffer and the number of elements of the array. Today a
>> vmstate writer can choose from a number of macros that take as last
>> input the name of a struct member from where the size/num will be
>> read.
>>
>> At load time, the code will access those value via an opaque pointer
>> to the migrated data and therefore it needs to also know the size of
>> the struct member at that offset.
>>
>> Currently that information is communicated by means of the
>> VMS_VARRAY_* and VMS_VBUFFER_* flags, where each possible type is
>> represented by a flag.
>>
>> So far, that's all fine, but since the vmstate code makes heavy use of
>> macros, handling several types individually (i.e. by name: int,
>> int32_t, etc) requires several versions of a same macro, one for each
>> type. E.g: VMSTATE_VBUFFER_ALLOC_UINT32
>> ^
>>
>> This creates a pattern where the vmstate writer has to match the macro
>> name to the data type and has resulted in the code having a tendency
>> of having one macro version for each type, for each type of vmstate.
>>
>> There is also some cognitive load to deal with, e.g.
>> VMSTATE_VARRAY_INT32 doesn't hold an array of int32, it holds an array
>> of something else and the number of elements for the array is stored
>> in a variable of type int32.
>>
>> We're now dealing with the scenario where the code has been expecting
>> int32_t at some places, but a uint64_t macro variant has been added
>> without the code being updated.
>>
>> To address all these situations, introduce a new struct that will hold
>> the offset of the struct members, but also their size, so the various
>> extra macros can all be removed and the person writing the vmstate
>> doesn't need to care about type-checking. Still, keep a minimum check
>> that those fields are at least integers and fit into 64 bits.
>>
>> What changes:
>>
>> 1) type checking changes from individual types to a single check for
>> all integers;
>>
>> 2) there are new ways to access the offsets;
>>
>> num_offset -> num_offset.off
>> size_offset -> size_offset.off
>> [new] num_offset.size
>> [new] size_offset.size
>>
>> 2) reading the offsets goes from checking the VMS_VARRAY_* flags in an
>> if/elseif block to comparing offset.size against the hardcoded
>> sizes in bytes;
>>
>> 3) the VMS_VARRAY_* and VMS_VBUFFER_* flags become obsolete. Removed
>> in the next patch;
>>
>> 4) memory usage increases +1 byte per vmstate;
>>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>
> Nice work! Just some minor comments below.
>
>> ---
>> include/migration/vmstate.h | 66 +++++++++++++++++++++++--------------
>> migration/savevm.c | 4 +--
>> migration/vmstate.c | 51 ++++++++++++++++++----------
>> 3 files changed, 76 insertions(+), 45 deletions(-)
>>
>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
>> index 92a1a9fe98e..2ad3cc01371 100644
>> --- a/include/migration/vmstate.h
>> +++ b/include/migration/vmstate.h
>> @@ -31,6 +31,7 @@
>>
>> typedef struct VMStateInfo VMStateInfo;
>> typedef struct VMStateField VMStateField;
>> +typedef struct VMStateOffset VMStateOffset;
>>
>> /*
>> * VMStateInfo allows customized migration of objects that don't fit in
>> @@ -187,6 +188,11 @@ typedef enum {
>> MIG_PRI_MAX,
>> } MigrationPriority;
>>
>> +struct VMStateOffset {
>> + uint32_t off;
>> + uint8_t size;
>> +};
>> +
>
>
>
> I think it would be great to add some comments here.
>
Indeed, hopefully also some better naming for everything.
> What does the struct describe? size and offset of what where?
size and offset of a field inside the struct being migrated that
represent either the size of a buffer or the numer of elements in an
array. Super simple =D
> And IIUC size 0 has a special meaning?
See below.
> And neither offset nor size ever come from the migration stream, right?
Right.
>
>> struct VMStateField {
>> const char *name;
>> size_t offset;
>> @@ -205,11 +211,11 @@ struct VMStateField {
>> * pointer point to.
>> */
>> size_t size;
>> - size_t size_offset;
>> + VMStateOffset size_offset;
>>
>> size_t start;
>> int num;
>> - size_t num_offset;
>> + VMStateOffset num_offset;
>> const VMStateInfo *info;
>> enum VMStateFlags flags;
>> const VMStateDescription *vmsd;
>> @@ -328,6 +334,16 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> (type_check(t1, typeof_elt_of_field(t2, f)) \
>> + QEMU_BUILD_BUG_ON_ZERO(!QEMU_IS_ARRAY(((t2 *)0)->f)))
>>
>> +#define type_check_int64(t) \
>> + (((ptrdiff_t)0 * (ptrdiff_t)(~((t)0))) + \
>> + (0 * sizeof(char[(sizeof(t) <= sizeof(uint64_t)) ? 1 : -1])))
>> +
>> +#define vmstate_field_offset(_state, _field) { \
>> + .off = (offsetof(_state, _field) + \
>> + type_check_int64(typeof_field(_state, _field))), \
>> + .size = sizeof(typeof_field(_state, _field)), \
>> +}
>> +
>> #define vmstate_offset_value(_state, _field, _type) \
>> (offsetof(_state, _field) + \
>> type_check(_type, typeof_field(_state, _field)))
>> @@ -454,7 +470,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_VARRAY_INT32(_field, _state, _field_num, _version, _info, _type) {\
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .info = &(_info), \
>> .size = sizeof(_type), \
>> .flags = VMS_VARRAY_INT32|VMS_POINTER, \
>> @@ -464,7 +480,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_VARRAY_UINT32(_field, _state, _field_num, _version, _info, _type) {\
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .info = &(_info), \
>> .size = sizeof(_type), \
>> .flags = VMS_VARRAY_UINT32|VMS_POINTER, \
>> @@ -474,7 +490,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_VARRAY_INT32_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .info = &(_info), \
>> .size = sizeof(_type), \
>> .flags = VMS_VARRAY_INT32 | VMS_POINTER | VMS_ALLOC, \
>> @@ -484,7 +500,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_VARRAY_UINT32_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .info = &(_info), \
>> .size = sizeof(_type), \
>> .flags = VMS_VARRAY_UINT32|VMS_POINTER|VMS_ALLOC, \
>> @@ -494,7 +510,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_VARRAY_UINT16_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint16_t),\
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .info = &(_info), \
>> .size = sizeof(_type), \
>> .flags = VMS_VARRAY_UINT16 | VMS_POINTER | VMS_ALLOC, \
>> @@ -504,7 +520,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_VARRAY_UINT16_UNSAFE(_field, _state, _field_num, _version, _info, _type) {\
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint16_t),\
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .info = &(_info), \
>> .size = sizeof(_type), \
>> .flags = VMS_VARRAY_UINT16, \
>> @@ -583,7 +599,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> _field, _state, _field_num, _version, _vmsd, _type) { \
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint8_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .vmsd = &(_vmsd), \
>> .size = sizeof(_type), \
>> .flags = VMS_POINTER | VMS_VARRAY_UINT8 | \
>> @@ -596,7 +612,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> _field, _state, _field_num, _version, _vmsd, _type) { \
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .vmsd = &(_vmsd), \
>> .size = sizeof(_type), \
>> .flags = VMS_POINTER | VMS_VARRAY_UINT32 | \
>> @@ -608,7 +624,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_VARRAY_OF_POINTER_UINT32(_field, _state, _field_num, _version, _info, _type) { \
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .info = &(_info), \
>> .flags = VMS_VARRAY_UINT32 | VMS_ARRAY_OF_POINTER | VMS_POINTER, \
>> .offset = vmstate_offset_pointer(_state, _field, _type *), \
>> @@ -650,7 +666,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>>
>> #define VMSTATE_STRUCT_VARRAY_UINT8(_field, _state, _field_num, _version, _vmsd, _type) { \
>> .name = (stringify(_field)), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint8_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .version_id = (_version), \
>> .vmsd = &(_vmsd), \
>> .size = sizeof(_type), \
>> @@ -674,7 +690,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_STRUCT_VARRAY_POINTER_INT32(_field, _state, _field_num, _vmsd, _type) { \
>> .name = (stringify(_field)), \
>> .version_id = 0, \
>> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .size = sizeof(_type), \
>> .vmsd = &(_vmsd), \
>> .flags = VMS_POINTER | VMS_VARRAY_INT32 | VMS_STRUCT, \
>> @@ -684,7 +700,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_STRUCT_VARRAY_POINTER_UINT32(_field, _state, _field_num, _vmsd, _type) { \
>> .name = (stringify(_field)), \
>> .version_id = 0, \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .size = sizeof(_type), \
>> .vmsd = &(_vmsd), \
>> .flags = VMS_POINTER | VMS_VARRAY_INT32 | VMS_STRUCT, \
>> @@ -694,7 +710,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_STRUCT_VARRAY_POINTER_UINT16(_field, _state, _field_num, _vmsd, _type) { \
>> .name = (stringify(_field)), \
>> .version_id = 0, \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint16_t),\
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .size = sizeof(_type), \
>> .vmsd = &(_vmsd), \
>> .flags = VMS_POINTER | VMS_VARRAY_UINT16 | VMS_STRUCT, \
>> @@ -703,7 +719,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>>
>> #define VMSTATE_STRUCT_VARRAY_INT32(_field, _state, _field_num, _version, _vmsd, _type) { \
>> .name = (stringify(_field)), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num, int32_t), \
>> .version_id = (_version), \
>> .vmsd = &(_vmsd), \
>> .size = sizeof(_type), \
>> @@ -713,7 +729,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>>
>> #define VMSTATE_STRUCT_VARRAY_UINT32(_field, _state, _field_num, _version, _vmsd, _type) { \
>> .name = (stringify(_field)), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .version_id = (_version), \
>> .vmsd = &(_vmsd), \
>> .size = sizeof(_type), \
>> @@ -725,7 +741,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> .vmsd = &(_vmsd), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .size = sizeof(_type), \
>> .flags = VMS_STRUCT|VMS_VARRAY_INT32|VMS_ALLOC|VMS_POINTER, \
>> .offset = vmstate_offset_pointer(_state, _field, _type), \
>> @@ -746,7 +762,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> .field_exists = (_test), \
>> - .size_offset = vmstate_offset_value(_state, _field_size, uint32_t),\
>> + .size_offset = vmstate_field_offset(_state, _field_size), \
>> .size = (_multiply), \
>> .info = &vmstate_info_buffer, \
>> .flags = VMS_VBUFFER|VMS_POINTER|VMS_MULTIPLY, \
>> @@ -757,7 +773,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> .field_exists = (_test), \
>> - .size_offset = vmstate_offset_value(_state, _field_size, int32_t),\
>> + .size_offset = vmstate_field_offset(_state, _field_size), \
>> .info = &vmstate_info_buffer, \
>> .flags = VMS_VBUFFER|VMS_POINTER, \
>> .offset = offsetof(_state, _field), \
>> @@ -767,7 +783,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> .field_exists = (_test), \
>> - .size_offset = vmstate_offset_value(_state, _field_size, uint32_t),\
>> + .size_offset = vmstate_field_offset(_state, _field_size), \
>> .info = &vmstate_info_buffer, \
>> .flags = VMS_VBUFFER|VMS_POINTER, \
>> .offset = offsetof(_state, _field), \
>> @@ -777,7 +793,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> .field_exists = (_test), \
>> - .size_offset = vmstate_offset_value(_state, _field_size, uint64_t),\
>> + .size_offset = vmstate_field_offset(_state, _field_size), \
>> .info = &vmstate_info_buffer, \
>> .flags = VMS_VBUFFER | VMS_POINTER, \
>> .offset = offsetof(_state, _field), \
>> @@ -788,7 +804,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> .name = (stringify(_field)), \
>> .version_id = (_version), \
>> .field_exists = (_test), \
>> - .size_offset = vmstate_offset_value(_state, _field_size, uint32_t),\
>> + .size_offset = vmstate_field_offset(_state, _field_size), \
>> .info = &vmstate_info_buffer, \
>> .flags = VMS_VBUFFER|VMS_POINTER|VMS_ALLOC, \
>> .offset = offsetof(_state, _field), \
>> @@ -848,7 +864,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> #define VMSTATE_UNUSED_VARRAY_UINT32(_state, _test, _version, _field_num, _size) {\
>> .name = "unused", \
>> .field_exists = (_test), \
>> - .num_offset = vmstate_offset_value(_state, _field_num, uint32_t),\
>> + .num_offset = vmstate_field_offset(_state, _field_num), \
>> .version_id = (_version), \
>> .size = (_size), \
>> .info = &vmstate_info_unused_buffer, \
>> @@ -862,7 +878,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>> .name = (stringify(_field)), \
>> .field_exists = (_test), \
>> .version_id = (_version), \
>> - .size_offset = vmstate_offset_value(_state, _field_size, int32_t),\
>> + .size_offset = vmstate_field_offset(_state, _field_size), \
>> .info = &vmstate_info_bitmap, \
>> .flags = VMS_VBUFFER|VMS_POINTER, \
>> .offset = offsetof(_state, _field), \
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index 34dd06f9f73..a272bcfd0b4 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -873,14 +873,14 @@ static void vmstate_check(const VMStateDescription *vmsd)
>> * Size must be provided because dest QEMU needs that
>> * info to know what to allocate
>> */
>> - assert(field->size || field->size_offset);
>> + assert(field->size || field->size_offset.size != 0);
>
> Why is field->size checked by coersing it to bool, but
> field->size_offset.size - by comparison to 0?
> field->size != 0 for consistency?
>
Ah, I initially had field->size_offset.off != 0 but 0 is a valid offset,
so I changed to size.
This just wants to say "there's no size_offset". So 0 is not special, it
just means that the size_offset was never set. I'll make both sides
uniform.
>
>> } else {
>> /*
>> * Otherwise size info isn't useful (because it's
>> * always the size of host pointer), detect accidental
>> * setup of sizes in this case.
>> */
>> - assert(field->size == 0 && field->size_offset == 0);
>> + assert(field->size == 0 && field->size_offset.size == 0);
>> }
>> /*
>> * VMS_ARRAY_OF_POINTER must be used only together with one
>> diff --git a/migration/vmstate.c b/migration/vmstate.c
>> index 50ebe378452..0a0b9faa20e 100644
>> --- a/migration/vmstate.c
>> +++ b/migration/vmstate.c
>> @@ -78,32 +78,45 @@ vmsd_init_ptr_marker_field(VMStateField *fake, const VMStateField *field)
>> };
>> }
>>
>> -static int vmstate_n_elems(void *opaque, const VMStateField *field)
>> +static uint64_t vmstate_read_from_offset(void *opaque,
>> + const VMStateOffset *offset)
>> {
>> - int n_elems = 1;
>> + uint8_t *ptr = (uint8_t *)opaque + offset->off;
>> +
>
> The below can easily produce at least UB if offset is
> not size aligned.
> I *think* offset and size both come from macros that guarantee
> this never happens, but maybe better be safe.
>
Right, I'll put a memcpy inside the switch then.
>> + switch (offset->size) {
>> + case 1:
>> + return *(uint8_t *)ptr;
>> + case 2:
>> + return *(uint16_t *)ptr;
>> + case 4:
>> + return *(uint32_t *)ptr;
>> + case 8:
>> + return *(uint64_t *)ptr;
>> + }
>> + g_assert_not_reached();
>> +}
>> +
>> +static uint64_t vmstate_n_elems(void *opaque, const VMStateField *field)
>> +{
>> + uint64_t n_elems = 1;
>>
>> if (field->flags & VMS_ARRAY) {
>> n_elems = field->num;
>> - } else if (field->flags & VMS_VARRAY_INT32) {
>> - n_elems = *(int32_t *)(opaque + field->num_offset);
>> - } else if (field->flags & VMS_VARRAY_UINT32) {
>> - n_elems = *(uint32_t *)(opaque + field->num_offset);
>> - } else if (field->flags & VMS_VARRAY_UINT16) {
>> - n_elems = *(uint16_t *)(opaque + field->num_offset);
>> - } else if (field->flags & VMS_VARRAY_UINT8) {
>> - n_elems = *(uint8_t *)(opaque + field->num_offset);
>> + } else if (field->flags & (VMS_VARRAY_INT32 | VMS_VARRAY_UINT32
>> + | VMS_VARRAY_UINT16 | VMS_VARRAY_UINT8)) {
>> + n_elems = vmstate_read_from_offset(opaque, &field->num_offset);
>> }
>>
>> trace_vmstate_n_elems(field->name, n_elems);
>> return n_elems;
>> }
>>
>> -static int vmstate_size(void *opaque, const VMStateField *field)
>> +static uint64_t vmstate_size(void *opaque, const VMStateField *field)
>> {
>> - int size;
>> + uint64_t size;
>>
>> if (field->flags & VMS_VBUFFER) {
>> - size = *(int32_t *)(opaque + field->size_offset);
>> + size = vmstate_read_from_offset(opaque, &field->size_offset);
>> if (field->flags & VMS_MULTIPLY) {
>> size *= field->size;
>> }
>> @@ -124,7 +137,7 @@ static void vmstate_handle_alloc(void *ptr, const VMStateField *field,
>> void *opaque)
>> {
>> if (field->flags & VMS_POINTER && field->flags & VMS_ALLOC) {
>> - gsize size = vmstate_size(opaque, field);
>> + uint64_t size = vmstate_size(opaque, field);
>> size *= vmstate_n_elems(opaque, field);
>> if (size) {
>> *(void **)ptr = g_malloc(size);
>> @@ -335,8 +348,9 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
>>
>> if (exists) {
>> void *first_elem = opaque + field->offset;
>> - int i, n_elems = vmstate_n_elems(opaque, field);
>> - int size = vmstate_size(opaque, field);
>> + int i;
>> + uint64_t n_elems = vmstate_n_elems(opaque, field);
>> + uint64_t size = vmstate_size(opaque, field);
>>
>> vmstate_handle_alloc(first_elem, field, opaque);
>> if (field->flags & VMS_POINTER) {
>> @@ -650,8 +664,9 @@ 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;
>> - int i, n_elems = vmstate_n_elems(opaque, field);
>> - int size = vmstate_size(opaque, field);
>> + int i;
>> + uint64_t n_elems = vmstate_n_elems(opaque, field);
>> + uint64_t size = vmstate_size(opaque, field);
>> JSONWriter *vmdesc_loop = vmdesc;
>> bool is_prev_null = false;
>> /*
>> --
>> 2.53.0
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 2/4] migration: Introduce VMStateOffset
2026-07-30 14:35 ` Michael S. Tsirkin
2026-07-30 15:15 ` Fabiano Rosas
@ 2026-07-30 15:23 ` Peter Xu
1 sibling, 0 replies; 22+ messages in thread
From: Peter Xu @ 2026-07-30 15:23 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Fabiano Rosas, qemu-devel, Alexandr Moshkov
On Thu, Jul 30, 2026 at 10:35:41AM -0400, Michael S. Tsirkin wrote:
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index 34dd06f9f73..a272bcfd0b4 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -873,14 +873,14 @@ static void vmstate_check(const VMStateDescription *vmsd)
> > * Size must be provided because dest QEMU needs that
> > * info to know what to allocate
> > */
> > - assert(field->size || field->size_offset);
> > + assert(field->size || field->size_offset.size != 0);
>
> Why is field->size checked by coersing it to bool, but
> field->size_offset.size - by comparison to 0?
> field->size != 0 for consistency?
Indeed it might be better to add a comment, but for another reason: IIUC
old code was wrong.. size_offset can be 0 if the size field is the 1st
element.. so this may have fixed a bug.
--
Peter Xu
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/4] migration: Remove redundant flags
2026-07-29 22:52 [PATCH 0/4] migration: Remove extra type-checking from vmstate macros Fabiano Rosas
2026-07-29 22:52 ` [PATCH 1/4] migration: Remove VMSTATE_ARRAY_INT32_UNSAFE Fabiano Rosas
2026-07-29 22:52 ` [PATCH 2/4] migration: Introduce VMStateOffset Fabiano Rosas
@ 2026-07-29 22:52 ` Fabiano Rosas
2026-07-30 8:11 ` Vladimir Sementsov-Ogievskiy
2026-07-29 22:52 ` [PATCH 4/4] migration: Remove duplicate vmstate macros Fabiano Rosas
` (2 subsequent siblings)
5 siblings, 1 reply; 22+ messages in thread
From: Fabiano Rosas @ 2026-07-29 22:52 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Michael S . Tsirkin, Alexandr Moshkov,
Manos Pitsidianakis
Remove the VMS_VARRAY and VMS_VBUFFER flags that became redundant due
to the previous commit which stores the size of the vmstate offset
variables along with the offset itself.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
include/migration/vmstate.h | 76 ++++++++++++------------------
migration/savevm.c | 6 +--
migration/vmstate.c | 3 +-
rust/bindings/migration-sys/lib.rs | 15 ++++--
rust/migration/src/vmstate.rs | 30 ++++++++++--
rust/tests/tests/vmstate_tests.rs | 30 ++++++------
6 files changed, 83 insertions(+), 77 deletions(-)
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 2ad3cc01371..c4fd036344d 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -69,8 +69,9 @@ enum VMStateFlags {
* }). Dereference the pointer before using it as basis for
* further pointer arithmetic (see e.g. VMS_ARRAY). Does not
* affect the meaning of VMStateField.num_offset or
- * VMStateField.size_offset; see VMS_VARRAY* and VMS_VBUFFER for
- * those. */
+ * VMStateField.size_offset; see VMS_VARRAY and VMS_VBUFFER for
+ * those.
+ */
VMS_POINTER = 0x002,
/* The field is an array of fixed size. VMStateField.num contains
@@ -80,21 +81,22 @@ enum VMStateFlags {
* VMS_MULTIPLY. Each array entry will be processed individually
* (VMStateField.info.get()/put() if VMS_STRUCT is not set,
* recursion into VMStateField.vmsd if VMS_STRUCT is set). May not
- * be combined with VMS_VARRAY*. */
+ * be combined with VMS_VARRAY.
+ */
VMS_ARRAY = 0x004,
/* The field is itself a struct, containing one or more
* fields. Recurse into VMStateField.vmsd. Most useful in
- * combination with VMS_ARRAY / VMS_VARRAY*, recursing into each
+ * combination with VMS_ARRAY / VMS_VARRAY, recursing into each
* array entry. */
VMS_STRUCT = 0x008,
/* The field is an array of variable size. The int32_t at opaque +
* VMStateField.num_offset contains the number of entries in the
* array. See the VMS_ARRAY description regarding array handling
- * in general. May not be combined with VMS_ARRAY or any other
- * VMS_VARRAY*. */
- VMS_VARRAY_INT32 = 0x010,
+ * in general. May not be combined with VMS_ARRAY.
+ */
+ VMS_VARRAY = 0x010,
/* Ignored */
VMS_BUFFER = 0x020,
@@ -102,18 +104,12 @@ enum VMStateFlags {
/* The field is a (fixed-size or variable-size) array of pointers
* (e.g. struct a { uint8_t *b[]; }). Dereference each array entry
* before using it. Note: Does not imply any one of VMS_ARRAY /
- * VMS_VARRAY*; these need to be set explicitly. */
+ * VMS_VARRAY; these need to be set explicitly.
+ */
VMS_ARRAY_OF_POINTER = 0x040,
- /* The field is an array of variable size. The uint16_t at opaque
- * + VMStateField.num_offset
- * contains the number of entries in the array. See the VMS_ARRAY
- * description regarding array handling in general. May not be
- * combined with VMS_ARRAY or any other VMS_VARRAY*. */
- VMS_VARRAY_UINT16 = 0x080,
-
/* The size of the individual entries (a single array entry if
- * VMS_ARRAY or any of VMS_VARRAY* are set, or the field itself if
+ * VMS_ARRAY or VMS_VARRAY are set, or the field itself if
* neither is set) is variable (i.e. not known at compile-time),
* but the same for all entries. Use the int32_t at opaque +
* VMStateField.size_offset (subject to VMS_MULTIPLY) to determine
@@ -126,20 +122,6 @@ enum VMStateFlags {
* allocated. Only valid in combination with VMS_VBUFFER. */
VMS_MULTIPLY = 0x200,
- /* The field is an array of variable size. The uint8_t at opaque +
- * VMStateField.num_offset
- * contains the number of entries in the array. See the VMS_ARRAY
- * description regarding array handling in general. May not be
- * combined with VMS_ARRAY or any other VMS_VARRAY*. */
- VMS_VARRAY_UINT8 = 0x400,
-
- /* The field is an array of variable size. The uint32_t at opaque
- * + VMStateField.num_offset
- * contains the number of entries in the array. See the VMS_ARRAY
- * description regarding array handling in general. May not be
- * combined with VMS_ARRAY or any other VMS_VARRAY*. */
- VMS_VARRAY_UINT32 = 0x800,
-
/* Fail loading the serialised VM state if this field is missing
* from the input. */
VMS_MUST_EXIST = 0x1000,
@@ -473,7 +455,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.num_offset = vmstate_field_offset(_state, _field_num), \
.info = &(_info), \
.size = sizeof(_type), \
- .flags = VMS_VARRAY_INT32|VMS_POINTER, \
+ .flags = VMS_VARRAY | VMS_POINTER, \
.offset = vmstate_offset_pointer(_state, _field, _type), \
}
@@ -483,7 +465,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.num_offset = vmstate_field_offset(_state, _field_num), \
.info = &(_info), \
.size = sizeof(_type), \
- .flags = VMS_VARRAY_UINT32|VMS_POINTER, \
+ .flags = VMS_VARRAY | VMS_POINTER, \
.offset = vmstate_offset_pointer(_state, _field, _type), \
}
@@ -493,7 +475,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.num_offset = vmstate_field_offset(_state, _field_num), \
.info = &(_info), \
.size = sizeof(_type), \
- .flags = VMS_VARRAY_INT32 | VMS_POINTER | VMS_ALLOC, \
+ .flags = VMS_VARRAY | VMS_POINTER | VMS_ALLOC, \
.offset = vmstate_offset_pointer(_state, _field, _type), \
}
@@ -503,7 +485,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.num_offset = vmstate_field_offset(_state, _field_num), \
.info = &(_info), \
.size = sizeof(_type), \
- .flags = VMS_VARRAY_UINT32|VMS_POINTER|VMS_ALLOC, \
+ .flags = VMS_VARRAY | VMS_POINTER | VMS_ALLOC, \
.offset = vmstate_offset_pointer(_state, _field, _type), \
}
@@ -513,7 +495,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.num_offset = vmstate_field_offset(_state, _field_num), \
.info = &(_info), \
.size = sizeof(_type), \
- .flags = VMS_VARRAY_UINT16 | VMS_POINTER | VMS_ALLOC, \
+ .flags = VMS_VARRAY | VMS_POINTER | VMS_ALLOC, \
.offset = vmstate_offset_pointer(_state, _field, _type), \
}
@@ -523,7 +505,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.num_offset = vmstate_field_offset(_state, _field_num), \
.info = &(_info), \
.size = sizeof(_type), \
- .flags = VMS_VARRAY_UINT16, \
+ .flags = VMS_VARRAY, \
.offset = vmstate_offset_varray(_state, _field, _type), \
}
@@ -602,7 +584,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.num_offset = vmstate_field_offset(_state, _field_num), \
.vmsd = &(_vmsd), \
.size = sizeof(_type), \
- .flags = VMS_POINTER | VMS_VARRAY_UINT8 | \
+ .flags = VMS_POINTER | VMS_VARRAY | \
VMS_ARRAY_OF_POINTER | VMS_STRUCT | \
VMS_ARRAY_OF_POINTER_AUTO_ALLOC, \
.offset = vmstate_offset_pointer(_state, _field, _type *), \
@@ -615,7 +597,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.num_offset = vmstate_field_offset(_state, _field_num), \
.vmsd = &(_vmsd), \
.size = sizeof(_type), \
- .flags = VMS_POINTER | VMS_VARRAY_UINT32 | \
+ .flags = VMS_POINTER | VMS_VARRAY | \
VMS_ARRAY_OF_POINTER | VMS_STRUCT | \
VMS_ARRAY_OF_POINTER_AUTO_ALLOC, \
.offset = vmstate_offset_pointer(_state, _field, _type *), \
@@ -626,7 +608,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.version_id = (_version), \
.num_offset = vmstate_field_offset(_state, _field_num), \
.info = &(_info), \
- .flags = VMS_VARRAY_UINT32 | VMS_ARRAY_OF_POINTER | VMS_POINTER, \
+ .flags = VMS_VARRAY | VMS_ARRAY_OF_POINTER | VMS_POINTER, \
.offset = vmstate_offset_pointer(_state, _field, _type *), \
}
@@ -670,7 +652,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.version_id = (_version), \
.vmsd = &(_vmsd), \
.size = sizeof(_type), \
- .flags = VMS_STRUCT|VMS_VARRAY_UINT8, \
+ .flags = VMS_STRUCT | VMS_VARRAY, \
.offset = vmstate_offset_varray(_state, _field, _type), \
}
@@ -693,7 +675,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.num_offset = vmstate_field_offset(_state, _field_num), \
.size = sizeof(_type), \
.vmsd = &(_vmsd), \
- .flags = VMS_POINTER | VMS_VARRAY_INT32 | VMS_STRUCT, \
+ .flags = VMS_POINTER | VMS_VARRAY | VMS_STRUCT, \
.offset = vmstate_offset_pointer(_state, _field, _type), \
}
@@ -703,7 +685,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.num_offset = vmstate_field_offset(_state, _field_num), \
.size = sizeof(_type), \
.vmsd = &(_vmsd), \
- .flags = VMS_POINTER | VMS_VARRAY_INT32 | VMS_STRUCT, \
+ .flags = VMS_POINTER | VMS_VARRAY | VMS_STRUCT, \
.offset = vmstate_offset_pointer(_state, _field, _type), \
}
@@ -713,7 +695,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.num_offset = vmstate_field_offset(_state, _field_num), \
.size = sizeof(_type), \
.vmsd = &(_vmsd), \
- .flags = VMS_POINTER | VMS_VARRAY_UINT16 | VMS_STRUCT, \
+ .flags = VMS_POINTER | VMS_VARRAY | VMS_STRUCT, \
.offset = vmstate_offset_pointer(_state, _field, _type), \
}
@@ -723,7 +705,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.version_id = (_version), \
.vmsd = &(_vmsd), \
.size = sizeof(_type), \
- .flags = VMS_STRUCT|VMS_VARRAY_INT32, \
+ .flags = VMS_STRUCT | VMS_VARRAY, \
.offset = vmstate_offset_varray(_state, _field, _type), \
}
@@ -733,7 +715,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.version_id = (_version), \
.vmsd = &(_vmsd), \
.size = sizeof(_type), \
- .flags = VMS_STRUCT|VMS_VARRAY_UINT32, \
+ .flags = VMS_STRUCT | VMS_VARRAY, \
.offset = vmstate_offset_varray(_state, _field, _type), \
}
@@ -743,7 +725,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.vmsd = &(_vmsd), \
.num_offset = vmstate_field_offset(_state, _field_num), \
.size = sizeof(_type), \
- .flags = VMS_STRUCT|VMS_VARRAY_INT32|VMS_ALLOC|VMS_POINTER, \
+ .flags = VMS_STRUCT | VMS_VARRAY | VMS_ALLOC | VMS_POINTER, \
.offset = vmstate_offset_pointer(_state, _field, _type), \
}
@@ -868,7 +850,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.version_id = (_version), \
.size = (_size), \
.info = &vmstate_info_unused_buffer, \
- .flags = VMS_VARRAY_UINT32 | VMS_BUFFER, \
+ .flags = VMS_VARRAY | VMS_BUFFER, \
}
/* _field_size should be a int32_t field in the _state struct giving the
diff --git a/migration/savevm.c b/migration/savevm.c
index a272bcfd0b4..0b4fbfc72bb 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -884,11 +884,9 @@ static void vmstate_check(const VMStateDescription *vmsd)
}
/*
* VMS_ARRAY_OF_POINTER must be used only together with one
- * of VMS_(V)ARRAY* flags.
+ * of VMS_(V)ARRAY flags.
*/
- assert(field->flags & (VMS_ARRAY | VMS_VARRAY_INT32 |
- VMS_VARRAY_UINT16 | VMS_VARRAY_UINT8 |
- VMS_VARRAY_UINT32));
+ assert(field->flags & (VMS_ARRAY | VMS_VARRAY));
}
if (field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC) {
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 0a0b9faa20e..bc8eb3d3ca6 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -102,8 +102,7 @@ static uint64_t vmstate_n_elems(void *opaque, const VMStateField *field)
if (field->flags & VMS_ARRAY) {
n_elems = field->num;
- } else if (field->flags & (VMS_VARRAY_INT32 | VMS_VARRAY_UINT32
- | VMS_VARRAY_UINT16 | VMS_VARRAY_UINT8)) {
+ } else if (field->flags & VMS_VARRAY) {
n_elems = vmstate_read_from_offset(opaque, &field->num_offset);
}
diff --git a/rust/bindings/migration-sys/lib.rs b/rust/bindings/migration-sys/lib.rs
index 9581481e421..75b8c1387c2 100644
--- a/rust/bindings/migration-sys/lib.rs
+++ b/rust/bindings/migration-sys/lib.rs
@@ -47,6 +47,7 @@ fn default() -> Self {
unsafe impl Zeroable for VMStateFlags {}
unsafe impl Zeroable for VMStateField {}
unsafe impl Zeroable for VMStateDescription {}
+unsafe impl Zeroable for VMStateOffset {}
// The following higher-level helpers could be in "migration"
// crate when Rust has const trait impl.
@@ -57,10 +58,7 @@ pub trait VMStateFlagsExt {
impl VMStateFlagsExt for VMStateFlags {
const VMS_VARRAY_FLAGS: VMStateFlags = VMStateFlags(
- VMStateFlags::VMS_VARRAY_INT32.0
- | VMStateFlags::VMS_VARRAY_UINT8.0
- | VMStateFlags::VMS_VARRAY_UINT16.0
- | VMStateFlags::VMS_VARRAY_UINT32.0,
+ VMStateFlags::VMS_VARRAY.0
);
}
@@ -115,3 +113,12 @@ pub const fn with_varray_flag(mut self, flag: VMStateFlags) -> Self {
self.with_varray_flag_unchecked(flag)
}
}
+
+impl VMStateOffset {
+ pub const fn new(off: usize, size: usize) -> Self {
+ Self {
+ off: off as u32,
+ size: size as u8,
+ }
+ }
+}
diff --git a/rust/migration/src/vmstate.rs b/rust/migration/src/vmstate.rs
index 63d78b4f275..7023ebd2c22 100644
--- a/rust/migration/src/vmstate.rs
+++ b/rust/migration/src/vmstate.rs
@@ -42,7 +42,7 @@
};
use crate::bindings::{self, VMStateFlags};
-pub use crate::bindings::{MigrationPriority, VMStateField};
+pub use crate::bindings::{MigrationPriority, VMStateField, VMStateOffset};
/// This macro is used to call a function with a generic argument bound
/// to the type of a field. The function must take a
@@ -120,6 +120,23 @@ pub const fn vmstate_varray_flag<T: VMState>(_: PhantomData<T>) -> VMStateFlags
T::VARRAY_FLAG
}
+pub const OPAQUE: &[u8; 1048576] = &[0; 1048576];
+
+pub const fn size_of_ptr_type<T>(_: *const T) -> usize {
+ ::core::mem::size_of::<T>()
+}
+
+#[macro_export]
+macro_rules! size_of_field_type {
+ ($struct_name:ty, $($field_name:ident).+) => {
+ $crate::vmstate::size_of_ptr_type(unsafe {
+ ::core::ptr::addr_of!(
+ (*$crate::vmstate::OPAQUE.as_ptr().cast::<$struct_name>()).$($field_name).+
+ )
+ })
+ };
+}
+
/// Return the `VMStateField` for a field of a struct. The field must be
/// visible in the current scope.
///
@@ -148,7 +165,10 @@ macro_rules! vmstate_of {
.as_bytes()
.as_ptr().cast::<::std::os::raw::c_char>(),
offset: ::std::mem::offset_of!($struct_name, $($field_name).+),
- $(num_offset: ::std::mem::offset_of!($struct_name, $($num).+),)?
+ $(num_offset: $crate::vmstate::VMStateOffset {
+ off: ::std::mem::offset_of!($struct_name, $($num).+) as u32,
+ size: $crate::size_of_field_type!($struct_name, $($num).+) as u8,
+ },)?
$(field_exists: $crate::vmstate_exist_fn!($struct_name, $test_fn),)?
// The calls to `call_func_with_field!` are the magic that
// computes most of the VMStateField from the type of the field.
@@ -267,9 +287,9 @@ unsafe impl $crate::vmstate::VMState for $type {
impl_vmstate_scalar!(vmstate_info_int16, i16);
impl_vmstate_scalar!(vmstate_info_int32, i32);
impl_vmstate_scalar!(vmstate_info_int64, i64);
-impl_vmstate_scalar!(vmstate_info_uint8, u8, VMS_VARRAY_UINT8);
-impl_vmstate_scalar!(vmstate_info_uint16, u16, VMS_VARRAY_UINT16);
-impl_vmstate_scalar!(vmstate_info_uint32, u32, VMS_VARRAY_UINT32);
+impl_vmstate_scalar!(vmstate_info_uint8, u8, VMS_VARRAY);
+impl_vmstate_scalar!(vmstate_info_uint16, u16, VMS_VARRAY);
+impl_vmstate_scalar!(vmstate_info_uint32, u32, VMS_VARRAY);
impl_vmstate_scalar!(vmstate_info_uint64, u64);
impl_vmstate_scalar!(vmstate_info_timer, util::timer::Timer);
diff --git a/rust/tests/tests/vmstate_tests.rs b/rust/tests/tests/vmstate_tests.rs
index c2c12cfab52..c5baed4d40b 100644
--- a/rust/tests/tests/vmstate_tests.rs
+++ b/rust/tests/tests/vmstate_tests.rs
@@ -65,7 +65,7 @@ fn test_vmstate_uint16() {
b"elem\0"
);
assert_eq!(foo_fields[0].offset, 16);
- assert_eq!(foo_fields[0].num_offset, 0);
+ assert_eq!(foo_fields[0].num_offset.size, 0);
assert_eq!(foo_fields[0].info, unsafe { &vmstate_info_int8 });
assert_eq!(foo_fields[0].version_id, 0);
assert_eq!(foo_fields[0].size, 1);
@@ -86,7 +86,7 @@ fn test_vmstate_unused() {
b"unused\0"
);
assert_eq!(foo_fields[1].offset, 0);
- assert_eq!(foo_fields[1].num_offset, 0);
+ assert_eq!(foo_fields[1].num_offset.size, 0);
assert_eq!(foo_fields[1].info, unsafe { &vmstate_info_unused_buffer });
assert_eq!(foo_fields[1].version_id, 0);
assert_eq!(foo_fields[1].size, 8);
@@ -108,12 +108,12 @@ fn test_vmstate_varray_uint16_unsafe() {
b"arr\0"
);
assert_eq!(foo_fields[2].offset, 0);
- assert_eq!(foo_fields[2].num_offset, 4);
+ assert_eq!(foo_fields[2].num_offset.off, 4);
assert_eq!(foo_fields[2].info, unsafe { &vmstate_info_uint8 });
assert_eq!(foo_fields[2].version_id, 0);
assert_eq!(foo_fields[2].size, 1);
assert_eq!(foo_fields[2].num, 0);
- assert_eq!(foo_fields[2].flags, VMStateFlags::VMS_VARRAY_UINT16);
+ assert_eq!(foo_fields[2].flags, VMStateFlags::VMS_VARRAY);
assert!(foo_fields[2].vmsd.is_null());
assert!(foo_fields[2].field_exists.is_none());
}
@@ -172,7 +172,7 @@ fn test_vmstate_bool_v() {
b"val\0"
);
assert_eq!(foo_fields[0].offset, 136);
- assert_eq!(foo_fields[0].num_offset, 0);
+ assert_eq!(foo_fields[0].num_offset.size, 0);
assert_eq!(foo_fields[0].info, unsafe { &vmstate_info_bool });
assert_eq!(foo_fields[0].version_id, 2);
assert_eq!(foo_fields[0].size, 1);
@@ -193,7 +193,7 @@ fn test_vmstate_uint64() {
b"wrap\0"
);
assert_eq!(foo_fields[1].offset, 128);
- assert_eq!(foo_fields[1].num_offset, 0);
+ assert_eq!(foo_fields[1].num_offset.size, 0);
assert_eq!(foo_fields[1].info, unsafe { &vmstate_info_uint64 });
assert_eq!(foo_fields[1].version_id, 0);
assert_eq!(foo_fields[1].size, 8);
@@ -215,14 +215,14 @@ fn test_vmstate_struct_varray_uint8() {
b"arr_a\0"
);
assert_eq!(foo_fields[2].offset, 0);
- assert_eq!(foo_fields[2].num_offset, 60);
+ assert_eq!(foo_fields[2].num_offset.off, 60);
assert!(foo_fields[2].info.is_null()); // VMSTATE_STRUCT_VARRAY_UINT8 doesn't set info field.
assert_eq!(foo_fields[2].version_id, 1);
assert_eq!(foo_fields[2].size, 20);
assert_eq!(foo_fields[2].num, 0);
assert_eq!(
foo_fields[2].flags.0,
- VMStateFlags::VMS_STRUCT.0 | VMStateFlags::VMS_VARRAY_UINT8.0
+ VMStateFlags::VMS_STRUCT.0 | VMStateFlags::VMS_VARRAY.0
);
assert_eq!(foo_fields[2].vmsd, VMSTATE_FOOA.as_ref());
assert!(foo_fields[2].field_exists.is_none());
@@ -240,7 +240,7 @@ fn test_vmstate_macro_array() {
b"arr_i64\0"
);
assert_eq!(foo_fields[4].offset, 144);
- assert_eq!(foo_fields[4].num_offset, 0);
+ assert_eq!(foo_fields[4].num_offset.size, 0);
assert_eq!(foo_fields[4].info, unsafe { &vmstate_info_int64 });
assert_eq!(foo_fields[4].version_id, 0);
assert_eq!(foo_fields[4].size, 8);
@@ -264,7 +264,7 @@ fn test_vmstate_struct_varray_uint8_wrapper() {
unsafe { CStr::from_ptr(foo_fields[5].name) }.to_bytes_with_nul(),
b"arr_a_wrap\0"
);
- assert_eq!(foo_fields[5].num_offset, 228);
+ assert_eq!(foo_fields[5].num_offset.off, 228);
assert!(unsafe { foo_fields[5].field_exists.unwrap()(foo_b_p, 0) });
// The last VMStateField in VMSTATE_FOOB.
@@ -316,7 +316,7 @@ fn test_vmstate_pointer() {
b"ptr\0"
);
assert_eq!(foo_fields[0].offset, 0);
- assert_eq!(foo_fields[0].num_offset, 0);
+ assert_eq!(foo_fields[0].num_offset.size, 0);
assert_eq!(foo_fields[0].info, unsafe { &vmstate_info_int32 });
assert_eq!(foo_fields[0].version_id, 2);
assert_eq!(foo_fields[0].size, 4);
@@ -341,7 +341,7 @@ fn test_vmstate_struct_pointer() {
b"ptr_a\0"
);
assert_eq!(foo_fields[1].offset, PTR_SIZE);
- assert_eq!(foo_fields[1].num_offset, 0);
+ assert_eq!(foo_fields[1].num_offset.size, 0);
assert_eq!(foo_fields[1].vmsd, VMSTATE_FOOA.as_ref());
assert_eq!(foo_fields[1].version_id, 0);
assert_eq!(foo_fields[1].size, size_of::<FooA>());
@@ -366,7 +366,7 @@ fn test_vmstate_macro_array_of_pointer() {
b"arr_ptr\0"
);
assert_eq!(foo_fields[2].offset, 2 * PTR_SIZE);
- assert_eq!(foo_fields[2].num_offset, 0);
+ assert_eq!(foo_fields[2].num_offset.size, 0);
assert_eq!(foo_fields[2].info, unsafe { &vmstate_info_uint8 });
assert_eq!(foo_fields[2].version_id, 0);
assert_eq!(foo_fields[2].size, PTR_SIZE);
@@ -391,7 +391,7 @@ fn test_vmstate_macro_array_of_pointer_wrapped() {
b"arr_ptr_wrap\0"
);
assert_eq!(foo_fields[3].offset, (FOO_ARRAY_MAX + 2) * PTR_SIZE);
- assert_eq!(foo_fields[3].num_offset, 0);
+ assert_eq!(foo_fields[3].num_offset.size, 0);
assert_eq!(foo_fields[3].info, unsafe { &vmstate_info_uint8 });
assert_eq!(foo_fields[3].version_id, 0);
assert_eq!(foo_fields[3].size, PTR_SIZE);
@@ -454,7 +454,7 @@ fn test_vmstate_validate() {
b"foo_d_0\0"
);
assert_eq!(foo_fields[0].offset, 0);
- assert_eq!(foo_fields[0].num_offset, 0);
+ assert_eq!(foo_fields[0].num_offset.size, 0);
assert!(foo_fields[0].info.is_null());
assert_eq!(foo_fields[0].version_id, 0);
assert_eq!(foo_fields[0].size, 0);
--
2.53.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 3/4] migration: Remove redundant flags
2026-07-29 22:52 ` [PATCH 3/4] migration: Remove redundant flags Fabiano Rosas
@ 2026-07-30 8:11 ` Vladimir Sementsov-Ogievskiy
2026-07-30 8:23 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2026-07-30 8:11 UTC (permalink / raw)
To: Fabiano Rosas, qemu-devel
Cc: Peter Xu, Michael S . Tsirkin, Alexandr Moshkov,
Manos Pitsidianakis
On 30.07.26 01:52, Fabiano Rosas wrote:
> Remove the VMS_VARRAY and VMS_VBUFFER flags that became redundant due
> to the previous commit which stores the size of the vmstate offset
> variables along with the offset itself.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
[..]
> --- a/rust/bindings/migration-sys/lib.rs
> +++ b/rust/bindings/migration-sys/lib.rs
> @@ -47,6 +47,7 @@ fn default() -> Self {
> unsafe impl Zeroable for VMStateFlags {}
> unsafe impl Zeroable for VMStateField {}
> unsafe impl Zeroable for VMStateDescription {}
> +unsafe impl Zeroable for VMStateOffset {}
>
> // The following higher-level helpers could be in "migration"
> // crate when Rust has const trait impl.
> @@ -57,10 +58,7 @@ pub trait VMStateFlagsExt {
>
> impl VMStateFlagsExt for VMStateFlags {
> const VMS_VARRAY_FLAGS: VMStateFlags = VMStateFlags(
> - VMStateFlags::VMS_VARRAY_INT32.0
> - | VMStateFlags::VMS_VARRAY_UINT8.0
> - | VMStateFlags::VMS_VARRAY_UINT16.0
> - | VMStateFlags::VMS_VARRAY_UINT32.0,
> + VMStateFlags::VMS_VARRAY.0
> );
> }
>
> @@ -115,3 +113,12 @@ pub const fn with_varray_flag(mut self, flag: VMStateFlags) -> Self {
> self.with_varray_flag_unchecked(flag)
> }
> }
> +
> +impl VMStateOffset {
This looks like part of patch 01? And may be some other changes in rust.
> + pub const fn new(off: usize, size: usize) -> Self {
> + Self {
> + off: off as u32,
> + size: size as u8,
> + }
> + }
> +}
[..]
> diff --git a/rust/tests/tests/vmstate_tests.rs b/rust/tests/tests/vmstate_tests.rs
> index c2c12cfab52..c5baed4d40b 100644
> --- a/rust/tests/tests/vmstate_tests.rs
> +++ b/rust/tests/tests/vmstate_tests.rs
> @@ -65,7 +65,7 @@ fn test_vmstate_uint16() {
> b"elem\0"
> );
> assert_eq!(foo_fields[0].offset, 16);
> - assert_eq!(foo_fields[0].num_offset, 0);
> + assert_eq!(foo_fields[0].num_offset.size, 0);
This too.
> assert_eq!(foo_fields[0].info, unsafe { &vmstate_info_int8 });
> assert_eq!(foo_fields[0].version_id, 0);
> assert_eq!(foo_fields[0].size, 1);
> @@ -86,7 +86,7 @@ fn test_vmstate_unused() {
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/4] migration: Remove duplicate vmstate macros
2026-07-29 22:52 [PATCH 0/4] migration: Remove extra type-checking from vmstate macros Fabiano Rosas
` (2 preceding siblings ...)
2026-07-29 22:52 ` [PATCH 3/4] migration: Remove redundant flags Fabiano Rosas
@ 2026-07-29 22:52 ` Fabiano Rosas
2026-07-30 8:19 ` Vladimir Sementsov-Ogievskiy
2026-07-30 8:22 ` [PATCH 0/4] migration: Remove extra type-checking from " Vladimir Sementsov-Ogievskiy
2026-07-30 14:09 ` Peter Xu
5 siblings, 1 reply; 22+ messages in thread
From: Fabiano Rosas @ 2026-07-29 22:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Michael S . Tsirkin, Alexandr Moshkov
Now that type-checking is not being done per integer size anymore,
various macros have duplicated bodies. Remove them.
The original names are left the same so the device code can be altered
per-subsystem in the next patches.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
include/migration/vmstate.h | 136 ++++++++++--------------------------
1 file changed, 36 insertions(+), 100 deletions(-)
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index c4fd036344d..24631fd6782 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -449,7 +449,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.offset = vmstate_offset_sub_array(_state, _field, _type, _start), \
}
-#define VMSTATE_VARRAY_INT32(_field, _state, _field_num, _version, _info, _type) {\
+#define VMSTATE_VARRAY(_field, _state, _field_num, _version, _info, _type) {\
.name = (stringify(_field)), \
.version_id = (_version), \
.num_offset = vmstate_field_offset(_state, _field_num), \
@@ -459,17 +459,10 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.offset = vmstate_offset_pointer(_state, _field, _type), \
}
-#define VMSTATE_VARRAY_UINT32(_field, _state, _field_num, _version, _info, _type) {\
- .name = (stringify(_field)), \
- .version_id = (_version), \
- .num_offset = vmstate_field_offset(_state, _field_num), \
- .info = &(_info), \
- .size = sizeof(_type), \
- .flags = VMS_VARRAY | VMS_POINTER, \
- .offset = vmstate_offset_pointer(_state, _field, _type), \
-}
+#define VMSTATE_VARRAY_INT32 VMSTATE_VARRAY
+#define VMSTATE_VARRAY_UINT32 VMSTATE_VARRAY
-#define VMSTATE_VARRAY_INT32_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
+#define VMSTATE_VARRAY_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
.name = (stringify(_field)), \
.version_id = (_version), \
.num_offset = vmstate_field_offset(_state, _field_num), \
@@ -479,27 +472,11 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.offset = vmstate_offset_pointer(_state, _field, _type), \
}
-#define VMSTATE_VARRAY_UINT32_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
- .name = (stringify(_field)), \
- .version_id = (_version), \
- .num_offset = vmstate_field_offset(_state, _field_num), \
- .info = &(_info), \
- .size = sizeof(_type), \
- .flags = VMS_VARRAY | VMS_POINTER | VMS_ALLOC, \
- .offset = vmstate_offset_pointer(_state, _field, _type), \
-}
-
-#define VMSTATE_VARRAY_UINT16_ALLOC(_field, _state, _field_num, _version, _info, _type) {\
- .name = (stringify(_field)), \
- .version_id = (_version), \
- .num_offset = vmstate_field_offset(_state, _field_num), \
- .info = &(_info), \
- .size = sizeof(_type), \
- .flags = VMS_VARRAY | VMS_POINTER | VMS_ALLOC, \
- .offset = vmstate_offset_pointer(_state, _field, _type), \
-}
+#define VMSTATE_VARRAY_INT32_ALLOC VMSTATE_VARRAY_ALLOC
+#define VMSTATE_VARRAY_UINT32_ALLOC VMSTATE_VARRAY_ALLOC
+#define VMSTATE_VARRAY_UINT16_ALLOC VMSTATE_VARRAY_ALLOC
-#define VMSTATE_VARRAY_UINT16_UNSAFE(_field, _state, _field_num, _version, _info, _type) {\
+#define VMSTATE_VARRAY_UNSAFE(_field, _state, _field_num, _version, _info, _type) {\
.name = (stringify(_field)), \
.version_id = (_version), \
.num_offset = vmstate_field_offset(_state, _field_num), \
@@ -509,6 +486,8 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.offset = vmstate_offset_varray(_state, _field, _type), \
}
+#define VMSTATE_VARRAY_UINT16_UNSAFE VMSTATE_VARRAY_UNSAFE
+
#define VMSTATE_VSTRUCT_TEST(_field, _state, _test, _version, _vmsd, _type, _struct_version) { \
.name = (stringify(_field)), \
.version_id = (_version), \
@@ -577,7 +556,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
* _info: VMStateInfo for _type (when VMS_STRUCT is not set)
* start: size of (_type) pointed to (for auto memory allocation)
*/
-#define VMSTATE_VARRAY_OF_POINTER_TO_STRUCT_UINT8_ALLOC(\
+#define VMSTATE_VARRAY_OF_POINTER_TO_STRUCT_ALLOC( \
_field, _state, _field_num, _version, _vmsd, _type) { \
.name = (stringify(_field)), \
.version_id = (_version), \
@@ -590,20 +569,12 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.offset = vmstate_offset_pointer(_state, _field, _type *), \
}
-#define VMSTATE_VARRAY_OF_POINTER_TO_STRUCT_UINT32_ALLOC(\
- _field, _state, _field_num, _version, _vmsd, _type) { \
- .name = (stringify(_field)), \
- .version_id = (_version), \
- .num_offset = vmstate_field_offset(_state, _field_num), \
- .vmsd = &(_vmsd), \
- .size = sizeof(_type), \
- .flags = VMS_POINTER | VMS_VARRAY | \
- VMS_ARRAY_OF_POINTER | VMS_STRUCT | \
- VMS_ARRAY_OF_POINTER_AUTO_ALLOC, \
- .offset = vmstate_offset_pointer(_state, _field, _type *), \
-}
+#define VMSTATE_VARRAY_OF_POINTER_TO_STRUCT_UINT8_ALLOC \
+ VMSTATE_VARRAY_OF_POINTER_TO_STRUCT_ALLOC
+#define VMSTATE_VARRAY_OF_POINTER_TO_STRUCT_UINT32_ALLOC \
+ VMSTATE_VARRAY_OF_POINTER_TO_STRUCT_ALLOC
-#define VMSTATE_VARRAY_OF_POINTER_UINT32(_field, _state, _field_num, _version, _info, _type) { \
+#define VMSTATE_VARRAY_OF_POINTER(_field, _state, _field_num, _version, _info, _type) { \
.name = (stringify(_field)), \
.version_id = (_version), \
.num_offset = vmstate_field_offset(_state, _field_num), \
@@ -612,6 +583,8 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.offset = vmstate_offset_pointer(_state, _field, _type *), \
}
+#define VMSTATE_VARRAY_OF_POINTER_UINT32 VMSTATE_VARRAY_OF_POINTER
+
#define VMSTATE_STRUCT_SUB_ARRAY(_field, _state, _start, _num, _version, _vmsd, _type) { \
.name = (stringify(_field)), \
.version_id = (_version), \
@@ -646,7 +619,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
_n1, _n2), \
}
-#define VMSTATE_STRUCT_VARRAY_UINT8(_field, _state, _field_num, _version, _vmsd, _type) { \
+#define VMSTATE_STRUCT_VARRAY(_field, _state, _field_num, _version, _vmsd, _type) { \
.name = (stringify(_field)), \
.num_offset = vmstate_field_offset(_state, _field_num), \
.version_id = (_version), \
@@ -655,6 +628,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.flags = VMS_STRUCT | VMS_VARRAY, \
.offset = vmstate_offset_varray(_state, _field, _type), \
}
+#define VMSTATE_STRUCT_VARRAY_UINT8 VMSTATE_STRUCT_VARRAY
/* a variable length array (i.e. _type *_field) but we know the
* length
@@ -669,7 +643,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.offset = offsetof(_state, _field), \
}
-#define VMSTATE_STRUCT_VARRAY_POINTER_INT32(_field, _state, _field_num, _vmsd, _type) { \
+#define VMSTATE_STRUCT_VARRAY_POINTER(_field, _state, _field_num, _vmsd, _type) { \
.name = (stringify(_field)), \
.version_id = 0, \
.num_offset = vmstate_field_offset(_state, _field_num), \
@@ -678,38 +652,11 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.flags = VMS_POINTER | VMS_VARRAY | VMS_STRUCT, \
.offset = vmstate_offset_pointer(_state, _field, _type), \
}
+#define VMSTATE_STRUCT_VARRAY_POINTER_INT32 VMSTATE_STRUCT_VARRAY_POINTER
+#define VMSTATE_STRUCT_VARRAY_POINTER_UINT32 VMSTATE_STRUCT_VARRAY_POINTER
+#define VMSTATE_STRUCT_VARRAY_POINTER_UINT16 VMSTATE_STRUCT_VARRAY_POINTER
-#define VMSTATE_STRUCT_VARRAY_POINTER_UINT32(_field, _state, _field_num, _vmsd, _type) { \
- .name = (stringify(_field)), \
- .version_id = 0, \
- .num_offset = vmstate_field_offset(_state, _field_num), \
- .size = sizeof(_type), \
- .vmsd = &(_vmsd), \
- .flags = VMS_POINTER | VMS_VARRAY | VMS_STRUCT, \
- .offset = vmstate_offset_pointer(_state, _field, _type), \
-}
-
-#define VMSTATE_STRUCT_VARRAY_POINTER_UINT16(_field, _state, _field_num, _vmsd, _type) { \
- .name = (stringify(_field)), \
- .version_id = 0, \
- .num_offset = vmstate_field_offset(_state, _field_num), \
- .size = sizeof(_type), \
- .vmsd = &(_vmsd), \
- .flags = VMS_POINTER | VMS_VARRAY | VMS_STRUCT, \
- .offset = vmstate_offset_pointer(_state, _field, _type), \
-}
-
-#define VMSTATE_STRUCT_VARRAY_INT32(_field, _state, _field_num, _version, _vmsd, _type) { \
- .name = (stringify(_field)), \
- .num_offset = vmstate_field_offset(_state, _field_num, int32_t), \
- .version_id = (_version), \
- .vmsd = &(_vmsd), \
- .size = sizeof(_type), \
- .flags = VMS_STRUCT | VMS_VARRAY, \
- .offset = vmstate_offset_varray(_state, _field, _type), \
-}
-
-#define VMSTATE_STRUCT_VARRAY_UINT32(_field, _state, _field_num, _version, _vmsd, _type) { \
+#define VMSTATE_STRUCT_VARRAY(_field, _state, _field_num, _version, _vmsd, _type) { \
.name = (stringify(_field)), \
.num_offset = vmstate_field_offset(_state, _field_num), \
.version_id = (_version), \
@@ -718,6 +665,8 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.flags = VMS_STRUCT | VMS_VARRAY, \
.offset = vmstate_offset_varray(_state, _field, _type), \
}
+#define VMSTATE_STRUCT_VARRAY_INT32 VMSTATE_STRUCT_VARRAY
+#define VMSTATE_STRUCT_VARRAY_UINT32 VMSTATE_STRUCT_VARRAY
#define VMSTATE_STRUCT_VARRAY_ALLOC(_field, _state, _field_num, _version, _vmsd, _type) {\
.name = (stringify(_field)), \
@@ -761,28 +710,11 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.offset = offsetof(_state, _field), \
}
-#define VMSTATE_VBUFFER_UINT32(_field, _state, _version, _test, _field_size) { \
- .name = (stringify(_field)), \
- .version_id = (_version), \
- .field_exists = (_test), \
- .size_offset = vmstate_field_offset(_state, _field_size), \
- .info = &vmstate_info_buffer, \
- .flags = VMS_VBUFFER|VMS_POINTER, \
- .offset = offsetof(_state, _field), \
-}
+#define VMSTATE_VBUFFER_UINT32 VMSTATE_VBUFFER
+#define VMSTATE_VBUFFER_UINT64 VMSTATE_VBUFFER
-#define VMSTATE_VBUFFER_UINT64(_field, _state, _version, _test, _field_size) { \
- .name = (stringify(_field)), \
- .version_id = (_version), \
- .field_exists = (_test), \
- .size_offset = vmstate_field_offset(_state, _field_size), \
- .info = &vmstate_info_buffer, \
- .flags = VMS_VBUFFER | VMS_POINTER, \
- .offset = offsetof(_state, _field), \
-}
-
-#define VMSTATE_VBUFFER_ALLOC_UINT32(_field, _state, _version, \
- _test, _field_size) { \
+#define VMSTATE_VBUFFER_ALLOC(_field, _state, _version, \
+ _test, _field_size) { \
.name = (stringify(_field)), \
.version_id = (_version), \
.field_exists = (_test), \
@@ -792,6 +724,8 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.offset = offsetof(_state, _field), \
}
+#define VMSTATE_VBUFFER_ALLOC_UINT32 VMSTATE_VBUFFER_ALLOC
+
#define VMSTATE_BUFFER_UNSAFE_INFO_TEST(_field, _state, _test, _version, _info, _size) { \
.name = (stringify(_field)), \
.version_id = (_version), \
@@ -843,7 +777,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
}
/* Discard size * field_num bytes, where field_num is a uint32 member */
-#define VMSTATE_UNUSED_VARRAY_UINT32(_state, _test, _version, _field_num, _size) {\
+#define VMSTATE_UNUSED_VARRAY(_state, _test, _version, _field_num, _size) {\
.name = "unused", \
.field_exists = (_test), \
.num_offset = vmstate_field_offset(_state, _field_num), \
@@ -853,6 +787,8 @@ extern const VMStateInfo vmstate_info_g_byte_array;
.flags = VMS_VARRAY | VMS_BUFFER, \
}
+#define VMSTATE_UNUSED_VARRAY_UINT32 VMSTATE_UNUSED_VARRAY
+
/* _field_size should be a int32_t field in the _state struct giving the
* size of the bitmap _field in bits.
*/
--
2.53.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 4/4] migration: Remove duplicate vmstate macros
2026-07-29 22:52 ` [PATCH 4/4] migration: Remove duplicate vmstate macros Fabiano Rosas
@ 2026-07-30 8:19 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2026-07-30 8:19 UTC (permalink / raw)
To: Fabiano Rosas, qemu-devel; +Cc: Peter Xu, Michael S . Tsirkin, Alexandr Moshkov
On 30.07.26 01:52, Fabiano Rosas wrote:
> -#define VMSTATE_STRUCT_VARRAY_INT32(_field, _state, _field_num, _version, _vmsd, _type) { \
> - .name = (stringify(_field)), \
> - .num_offset = vmstate_field_offset(_state, _field_num, int32_t), \
Hmm, this int32_t should be dropped in earlier commit.. It doesn't break
compilation because VMSTATE_STRUCT_VARRAY_INT32. Suggest to drop it
completely in patch 01, together with VMSTATE_ARRAY_INT32_UNSAFE.
(interesting, do we have more unused macros?)
> - .version_id = (_version), \
> - .vmsd = &(_vmsd), \
> - .size = sizeof(_type), \
> - .flags = VMS_STRUCT | VMS_VARRAY, \
> - .offset = vmstate_offset_varray(_state, _field, _type), \
> -}
> -
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/4] migration: Remove extra type-checking from vmstate macros
2026-07-29 22:52 [PATCH 0/4] migration: Remove extra type-checking from vmstate macros Fabiano Rosas
` (3 preceding siblings ...)
2026-07-29 22:52 ` [PATCH 4/4] migration: Remove duplicate vmstate macros Fabiano Rosas
@ 2026-07-30 8:22 ` Vladimir Sementsov-Ogievskiy
2026-07-30 14:09 ` Peter Xu
5 siblings, 0 replies; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2026-07-30 8:22 UTC (permalink / raw)
To: Fabiano Rosas, qemu-devel; +Cc: Peter Xu, Michael S . Tsirkin, Alexandr Moshkov
On 30.07.26 01:52, Fabiano Rosas wrote:
> Hi, this is basically what I ranted about in:
> https://lore.kernel.org/r/87jyqeomqz.fsf@suse.de
>
> I'm replacing the per-integer-size type checks with a single "int that
> fits in 32bit" check. This allows several lines of duplicated code to
> be removed.
>
> I haven't changed the macro names in the device code yet. If this
> series gets positive feedback then I'll send per-subsystem patches
> doing that.
>
> CI run: https://gitlab.com/farosas/qemu/-/pipelines/2716814081
> Also tested:
> - migration-test --full --thorough
> - x86_64 compat run forwards and backwards for previous 3 QEMU releases
> - s390x compat run forwards and backwards for previous 2 QEMU releases
> - ppc64 compat run forwards and backwards for previous QEMU release
> - migration-test smoke ASAN/UBSAN run
>
Thanks for doing it, all these _TYPE_ were pain!
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 0/4] migration: Remove extra type-checking from vmstate macros
2026-07-29 22:52 [PATCH 0/4] migration: Remove extra type-checking from vmstate macros Fabiano Rosas
` (4 preceding siblings ...)
2026-07-30 8:22 ` [PATCH 0/4] migration: Remove extra type-checking from " Vladimir Sementsov-Ogievskiy
@ 2026-07-30 14:09 ` Peter Xu
2026-07-30 14:22 ` Peter Xu
2026-07-30 15:37 ` Fabiano Rosas
5 siblings, 2 replies; 22+ messages in thread
From: Peter Xu @ 2026-07-30 14:09 UTC (permalink / raw)
To: Fabiano Rosas
Cc: qemu-devel, Michael S . Tsirkin, Alexandr Moshkov,
Vladimir Sementsov-Ogievskiy
On Wed, Jul 29, 2026 at 07:52:23PM -0300, Fabiano Rosas wrote:
> Hi, this is basically what I ranted about in:
> https://lore.kernel.org/r/87jyqeomqz.fsf@suse.de
>
> I'm replacing the per-integer-size type checks with a single "int that
> fits in 32bit" check. This allows several lines of duplicated code to
> be removed.
>
> I haven't changed the macro names in the device code yet. If this
> series gets positive feedback then I'll send per-subsystem patches
> doing that.
>
> CI run: https://gitlab.com/farosas/qemu/-/pipelines/2716814081
> Also tested:
> - migration-test --full --thorough
> - x86_64 compat run forwards and backwards for previous 3 QEMU releases
> - s390x compat run forwards and backwards for previous 2 QEMU releases
> - ppc64 compat run forwards and backwards for previous QEMU release
> - migration-test smoke ASAN/UBSAN run
>
> Fabiano Rosas (4):
> migration: Remove VMSTATE_ARRAY_INT32_UNSAFE
> migration: Introduce VMStateOffset
> migration: Remove redundant flags
> migration: Remove duplicate vmstate macros
Nice work!
I think I was only looking at VBUFFER side and I thought it was fine
sticking with 32bit even signed or not, not a huge deal. But cleaning up
VARRAY whole thing together looks definitely an improvement. I definitely
like your version here.
I assume with your series I can drop both of my patches here, right?
[PATCH v2 1/5] migration: Fix possible overflow in vmstate_handle_alloc()
https://lore.kernel.org/r/20260728210417.1925078-2-peterx@redhat.com
(I'll still respin with the rest)
[PATCH] vhost/migration: Fix incorrect size used in inflight->addr in VMSD
https://lore.kernel.org/r/20260728153942.1891677-1-peterx@redhat.com
The only missing piece would be an multiply overflow check in
vmstate_handle_alloc(), if you could add that check too while rewritting
that in patch 1 then I think it'll cover all.
Vladimir's ask in the separate email makes sense: I wonder if we can also
do one step further and merge VBUFFER into VARRAY.
The other trivial thing is, while looking, I found one trivial macro
VMSTATE_PARTIAL_VBUFFER not used; can drop it altogether.
--
Peter Xu
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 0/4] migration: Remove extra type-checking from vmstate macros
2026-07-30 14:09 ` Peter Xu
@ 2026-07-30 14:22 ` Peter Xu
2026-07-30 14:45 ` Michael S. Tsirkin
2026-07-30 15:50 ` Fabiano Rosas
2026-07-30 15:37 ` Fabiano Rosas
1 sibling, 2 replies; 22+ messages in thread
From: Peter Xu @ 2026-07-30 14:22 UTC (permalink / raw)
To: Fabiano Rosas
Cc: qemu-devel, Michael S . Tsirkin, Alexandr Moshkov,
Vladimir Sementsov-Ogievskiy
On Thu, Jul 30, 2026 at 10:09:30AM -0400, Peter Xu wrote:
> On Wed, Jul 29, 2026 at 07:52:23PM -0300, Fabiano Rosas wrote:
> > Hi, this is basically what I ranted about in:
> > https://lore.kernel.org/r/87jyqeomqz.fsf@suse.de
> >
> > I'm replacing the per-integer-size type checks with a single "int that
> > fits in 32bit" check. This allows several lines of duplicated code to
> > be removed.
> >
> > I haven't changed the macro names in the device code yet. If this
> > series gets positive feedback then I'll send per-subsystem patches
> > doing that.
> >
> > CI run: https://gitlab.com/farosas/qemu/-/pipelines/2716814081
> > Also tested:
> > - migration-test --full --thorough
> > - x86_64 compat run forwards and backwards for previous 3 QEMU releases
> > - s390x compat run forwards and backwards for previous 2 QEMU releases
> > - ppc64 compat run forwards and backwards for previous QEMU release
> > - migration-test smoke ASAN/UBSAN run
> >
> > Fabiano Rosas (4):
> > migration: Remove VMSTATE_ARRAY_INT32_UNSAFE
> > migration: Introduce VMStateOffset
> > migration: Remove redundant flags
> > migration: Remove duplicate vmstate macros
>
> Nice work!
>
> I think I was only looking at VBUFFER side and I thought it was fine
> sticking with 32bit even signed or not, not a huge deal. But cleaning up
> VARRAY whole thing together looks definitely an improvement. I definitely
> like your version here.
>
> I assume with your series I can drop both of my patches here, right?
>
> [PATCH v2 1/5] migration: Fix possible overflow in vmstate_handle_alloc()
> https://lore.kernel.org/r/20260728210417.1925078-2-peterx@redhat.com
> (I'll still respin with the rest)
>
> [PATCH] vhost/migration: Fix incorrect size used in inflight->addr in VMSD
> https://lore.kernel.org/r/20260728153942.1891677-1-peterx@redhat.com
>
> The only missing piece would be an multiply overflow check in
> vmstate_handle_alloc(), if you could add that check too while rewritting
> that in patch 1 then I think it'll cover all.
>
> Vladimir's ask in the separate email makes sense: I wonder if we can also
> do one step further and merge VBUFFER into VARRAY.
>
> The other trivial thing is, while looking, I found one trivial macro
> VMSTATE_PARTIAL_VBUFFER not used; can drop it altogether.
Now officially declare support for u64 on all these offsets, we also need
to double check on our alignment with security issues.
Similar reports will not be a bug anymore but results will be the same I
assume: it's anything the attacker can feed a u64 directly (instead of an
int32_t negative overflow), result is still failing a malloc() with
enormously large numbers, legally this time.
Do you still plan to work on finding per-user upper limit or whatever of
that kind? I'd say time spent working on series like this worths more than
that, but I still want to check with you while looking at this solution.
I suppose with this, one option is we can close all tickets reporting
security issues while allocating with all u64 fields.
Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/4] migration: Remove extra type-checking from vmstate macros
2026-07-30 14:22 ` Peter Xu
@ 2026-07-30 14:45 ` Michael S. Tsirkin
2026-07-30 15:50 ` Fabiano Rosas
1 sibling, 0 replies; 22+ messages in thread
From: Michael S. Tsirkin @ 2026-07-30 14:45 UTC (permalink / raw)
To: Peter Xu
Cc: Fabiano Rosas, qemu-devel, Alexandr Moshkov,
Vladimir Sementsov-Ogievskiy
On Thu, Jul 30, 2026 at 10:22:28AM -0400, Peter Xu wrote:
> On Thu, Jul 30, 2026 at 10:09:30AM -0400, Peter Xu wrote:
> > On Wed, Jul 29, 2026 at 07:52:23PM -0300, Fabiano Rosas wrote:
> > > Hi, this is basically what I ranted about in:
> > > https://lore.kernel.org/r/87jyqeomqz.fsf@suse.de
> > >
> > > I'm replacing the per-integer-size type checks with a single "int that
> > > fits in 32bit" check. This allows several lines of duplicated code to
> > > be removed.
> > >
> > > I haven't changed the macro names in the device code yet. If this
> > > series gets positive feedback then I'll send per-subsystem patches
> > > doing that.
> > >
> > > CI run: https://gitlab.com/farosas/qemu/-/pipelines/2716814081
> > > Also tested:
> > > - migration-test --full --thorough
> > > - x86_64 compat run forwards and backwards for previous 3 QEMU releases
> > > - s390x compat run forwards and backwards for previous 2 QEMU releases
> > > - ppc64 compat run forwards and backwards for previous QEMU release
> > > - migration-test smoke ASAN/UBSAN run
> > >
> > > Fabiano Rosas (4):
> > > migration: Remove VMSTATE_ARRAY_INT32_UNSAFE
> > > migration: Introduce VMStateOffset
> > > migration: Remove redundant flags
> > > migration: Remove duplicate vmstate macros
> >
> > Nice work!
> >
> > I think I was only looking at VBUFFER side and I thought it was fine
> > sticking with 32bit even signed or not, not a huge deal. But cleaning up
> > VARRAY whole thing together looks definitely an improvement. I definitely
> > like your version here.
> >
> > I assume with your series I can drop both of my patches here, right?
> >
> > [PATCH v2 1/5] migration: Fix possible overflow in vmstate_handle_alloc()
> > https://lore.kernel.org/r/20260728210417.1925078-2-peterx@redhat.com
> > (I'll still respin with the rest)
> >
> > [PATCH] vhost/migration: Fix incorrect size used in inflight->addr in VMSD
> > https://lore.kernel.org/r/20260728153942.1891677-1-peterx@redhat.com
> >
> > The only missing piece would be an multiply overflow check in
> > vmstate_handle_alloc(), if you could add that check too while rewritting
> > that in patch 1 then I think it'll cover all.
> >
> > Vladimir's ask in the separate email makes sense: I wonder if we can also
> > do one step further and merge VBUFFER into VARRAY.
> >
> > The other trivial thing is, while looking, I found one trivial macro
> > VMSTATE_PARTIAL_VBUFFER not used; can drop it altogether.
>
> Now officially declare support for u64 on all these offsets, we also need
> to double check on our alignment with security issues.
> Similar reports will not be a bug anymore but results will be the same I
> assume: it's anything the attacker can feed a u64 directly (instead of an
> int32_t negative overflow), result is still failing a malloc() with
> enormously large numbers, legally this time.
>
> Do you still plan to work on finding per-user upper limit or whatever of
> that kind? I'd say time spent working on series like this worths more than
> that, but I still want to check with you while looking at this solution.
>
> I suppose with this, one option is we can close all tickets reporting
> security issues while allocating with all u64 fields.
All issues where migration stream is malformed aren't security issues
according to the current policy.
Whether to close these or use them as a starting point in research,
is up to you guys.
>
> Thanks,
>
> --
> Peter Xu
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/4] migration: Remove extra type-checking from vmstate macros
2026-07-30 14:22 ` Peter Xu
2026-07-30 14:45 ` Michael S. Tsirkin
@ 2026-07-30 15:50 ` Fabiano Rosas
2026-07-30 16:31 ` Peter Xu
1 sibling, 1 reply; 22+ messages in thread
From: Fabiano Rosas @ 2026-07-30 15:50 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, Michael S . Tsirkin, Alexandr Moshkov,
Vladimir Sementsov-Ogievskiy
Peter Xu <peterx@redhat.com> writes:
> On Thu, Jul 30, 2026 at 10:09:30AM -0400, Peter Xu wrote:
>> On Wed, Jul 29, 2026 at 07:52:23PM -0300, Fabiano Rosas wrote:
>> > Hi, this is basically what I ranted about in:
>> > https://lore.kernel.org/r/87jyqeomqz.fsf@suse.de
>> >
>> > I'm replacing the per-integer-size type checks with a single "int that
>> > fits in 32bit" check. This allows several lines of duplicated code to
>> > be removed.
>> >
>> > I haven't changed the macro names in the device code yet. If this
>> > series gets positive feedback then I'll send per-subsystem patches
>> > doing that.
>> >
>> > CI run: https://gitlab.com/farosas/qemu/-/pipelines/2716814081
>> > Also tested:
>> > - migration-test --full --thorough
>> > - x86_64 compat run forwards and backwards for previous 3 QEMU releases
>> > - s390x compat run forwards and backwards for previous 2 QEMU releases
>> > - ppc64 compat run forwards and backwards for previous QEMU release
>> > - migration-test smoke ASAN/UBSAN run
>> >
>> > Fabiano Rosas (4):
>> > migration: Remove VMSTATE_ARRAY_INT32_UNSAFE
>> > migration: Introduce VMStateOffset
>> > migration: Remove redundant flags
>> > migration: Remove duplicate vmstate macros
>>
>> Nice work!
>>
>> I think I was only looking at VBUFFER side and I thought it was fine
>> sticking with 32bit even signed or not, not a huge deal. But cleaning up
>> VARRAY whole thing together looks definitely an improvement. I definitely
>> like your version here.
>>
>> I assume with your series I can drop both of my patches here, right?
>>
>> [PATCH v2 1/5] migration: Fix possible overflow in vmstate_handle_alloc()
>> https://lore.kernel.org/r/20260728210417.1925078-2-peterx@redhat.com
>> (I'll still respin with the rest)
>>
>> [PATCH] vhost/migration: Fix incorrect size used in inflight->addr in VMSD
>> https://lore.kernel.org/r/20260728153942.1891677-1-peterx@redhat.com
>>
>> The only missing piece would be an multiply overflow check in
>> vmstate_handle_alloc(), if you could add that check too while rewritting
>> that in patch 1 then I think it'll cover all.
>>
>> Vladimir's ask in the separate email makes sense: I wonder if we can also
>> do one step further and merge VBUFFER into VARRAY.
>>
>> The other trivial thing is, while looking, I found one trivial macro
>> VMSTATE_PARTIAL_VBUFFER not used; can drop it altogether.
>
> Now officially declare support for u64 on all these offsets, we also need
> to double check on our alignment with security issues.
>
> Similar reports will not be a bug anymore but results will be the same I
> assume: it's anything the attacker can feed a u64 directly (instead of an
> int32_t negative overflow), result is still failing a malloc() with
> enormously large numbers, legally this time.
>
Sprinkle some try_alloc maybe? Shouldn't be just a few from the
migration code itself.
> Do you still plan to work on finding per-user upper limit or whatever of
> that kind? I'd say time spent working on series like this worths more than
> that, but I still want to check with you while looking at this solution.
>
I thought of some things for the future, but I don't have anything
concrete, only experiments:
1) For all vmstate macros that take an unbounded size, length, etc, add
a new variant:
VMSTATE_FOO_BAR_V()
+VMSTATE_FOO_BAR_V_BOUNDED()
And allow the vmstate writer to provide a "Bounds" object (format to be
defined) and the vmstate core would just use that if it's there and
that's it. I still think that's a functionality the migration code
should have in its API.
2) Allowing a transfer limit in bytes to be set in QEMUFile, which can
be adjusted at will, maybe using some context-manager-like construct:
WITH_RX_CAP(f, 1024) {
qemu_get_byte();
qemu_get_byte();
vmstate_load_vmsd()
qemu_get_buf();
...
}
I'm still not sure how useful these would be.
> I suppose with this, one option is we can close all tickets reporting
> security issues while allocating with all u64 fields.
>
> Thanks,
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 0/4] migration: Remove extra type-checking from vmstate macros
2026-07-30 15:50 ` Fabiano Rosas
@ 2026-07-30 16:31 ` Peter Xu
0 siblings, 0 replies; 22+ messages in thread
From: Peter Xu @ 2026-07-30 16:31 UTC (permalink / raw)
To: Fabiano Rosas
Cc: qemu-devel, Michael S . Tsirkin, Alexandr Moshkov,
Vladimir Sementsov-Ogievskiy
On Thu, Jul 30, 2026 at 12:50:38PM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
>
> > On Thu, Jul 30, 2026 at 10:09:30AM -0400, Peter Xu wrote:
> >> On Wed, Jul 29, 2026 at 07:52:23PM -0300, Fabiano Rosas wrote:
> >> > Hi, this is basically what I ranted about in:
> >> > https://lore.kernel.org/r/87jyqeomqz.fsf@suse.de
> >> >
> >> > I'm replacing the per-integer-size type checks with a single "int that
> >> > fits in 32bit" check. This allows several lines of duplicated code to
> >> > be removed.
> >> >
> >> > I haven't changed the macro names in the device code yet. If this
> >> > series gets positive feedback then I'll send per-subsystem patches
> >> > doing that.
> >> >
> >> > CI run: https://gitlab.com/farosas/qemu/-/pipelines/2716814081
> >> > Also tested:
> >> > - migration-test --full --thorough
> >> > - x86_64 compat run forwards and backwards for previous 3 QEMU releases
> >> > - s390x compat run forwards and backwards for previous 2 QEMU releases
> >> > - ppc64 compat run forwards and backwards for previous QEMU release
> >> > - migration-test smoke ASAN/UBSAN run
> >> >
> >> > Fabiano Rosas (4):
> >> > migration: Remove VMSTATE_ARRAY_INT32_UNSAFE
> >> > migration: Introduce VMStateOffset
> >> > migration: Remove redundant flags
> >> > migration: Remove duplicate vmstate macros
> >>
> >> Nice work!
> >>
> >> I think I was only looking at VBUFFER side and I thought it was fine
> >> sticking with 32bit even signed or not, not a huge deal. But cleaning up
> >> VARRAY whole thing together looks definitely an improvement. I definitely
> >> like your version here.
> >>
> >> I assume with your series I can drop both of my patches here, right?
> >>
> >> [PATCH v2 1/5] migration: Fix possible overflow in vmstate_handle_alloc()
> >> https://lore.kernel.org/r/20260728210417.1925078-2-peterx@redhat.com
> >> (I'll still respin with the rest)
> >>
> >> [PATCH] vhost/migration: Fix incorrect size used in inflight->addr in VMSD
> >> https://lore.kernel.org/r/20260728153942.1891677-1-peterx@redhat.com
> >>
> >> The only missing piece would be an multiply overflow check in
> >> vmstate_handle_alloc(), if you could add that check too while rewritting
> >> that in patch 1 then I think it'll cover all.
> >>
> >> Vladimir's ask in the separate email makes sense: I wonder if we can also
> >> do one step further and merge VBUFFER into VARRAY.
> >>
> >> The other trivial thing is, while looking, I found one trivial macro
> >> VMSTATE_PARTIAL_VBUFFER not used; can drop it altogether.
> >
> > Now officially declare support for u64 on all these offsets, we also need
> > to double check on our alignment with security issues.
> >
> > Similar reports will not be a bug anymore but results will be the same I
> > assume: it's anything the attacker can feed a u64 directly (instead of an
> > int32_t negative overflow), result is still failing a malloc() with
> > enormously large numbers, legally this time.
> >
>
> Sprinkle some try_alloc maybe? Shouldn't be just a few from the
> migration code itself.
It's a matter of how helpful is it to fail slightly more gracefully,
v.s. malloc()'s error message.
>
> > Do you still plan to work on finding per-user upper limit or whatever of
> > that kind? I'd say time spent working on series like this worths more than
> > that, but I still want to check with you while looking at this solution.
> >
>
> I thought of some things for the future, but I don't have anything
> concrete, only experiments:
>
> 1) For all vmstate macros that take an unbounded size, length, etc, add
> a new variant:
>
> VMSTATE_FOO_BAR_V()
> +VMSTATE_FOO_BAR_V_BOUNDED()
>
> And allow the vmstate writer to provide a "Bounds" object (format to be
> defined) and the vmstate core would just use that if it's there and
> that's it. I still think that's a functionality the migration code
> should have in its API.
Something relevant I just sent:
https://lore.kernel.org/r/amt4Yv8miJ_rFYSS@x1.local
Based on that, IMHO what we need in a more useful way is, again taking
gtree example, a wrapper to g_tree_new_full() but allow specifying a size..
then gracefully allow failure to happen.
Maybe it's not easy to wrap it to make it graceful and generic, the whole
point is for the existing unlimited resources problem with migration we
should limit it from the user before migration even starts.
>
> 2) Allowing a transfer limit in bytes to be set in QEMUFile, which can
> be adjusted at will, maybe using some context-manager-like construct:
>
> WITH_RX_CAP(f, 1024) {
> qemu_get_byte();
> qemu_get_byte();
> vmstate_load_vmsd()
> qemu_get_buf();
> ...
> }
>
> I'm still not sure how useful these would be.
>
> > I suppose with this, one option is we can close all tickets reporting
> > security issues while allocating with all u64 fields.
> >
> > Thanks,
>
--
Peter Xu
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/4] migration: Remove extra type-checking from vmstate macros
2026-07-30 14:09 ` Peter Xu
2026-07-30 14:22 ` Peter Xu
@ 2026-07-30 15:37 ` Fabiano Rosas
2026-07-30 16:16 ` Peter Xu
1 sibling, 1 reply; 22+ messages in thread
From: Fabiano Rosas @ 2026-07-30 15:37 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, Michael S . Tsirkin, Alexandr Moshkov,
Vladimir Sementsov-Ogievskiy
Peter Xu <peterx@redhat.com> writes:
> On Wed, Jul 29, 2026 at 07:52:23PM -0300, Fabiano Rosas wrote:
>> Hi, this is basically what I ranted about in:
>> https://lore.kernel.org/r/87jyqeomqz.fsf@suse.de
>>
>> I'm replacing the per-integer-size type checks with a single "int that
>> fits in 32bit" check. This allows several lines of duplicated code to
>> be removed.
>>
>> I haven't changed the macro names in the device code yet. If this
>> series gets positive feedback then I'll send per-subsystem patches
>> doing that.
>>
>> CI run: https://gitlab.com/farosas/qemu/-/pipelines/2716814081
>> Also tested:
>> - migration-test --full --thorough
>> - x86_64 compat run forwards and backwards for previous 3 QEMU releases
>> - s390x compat run forwards and backwards for previous 2 QEMU releases
>> - ppc64 compat run forwards and backwards for previous QEMU release
>> - migration-test smoke ASAN/UBSAN run
>>
>> Fabiano Rosas (4):
>> migration: Remove VMSTATE_ARRAY_INT32_UNSAFE
>> migration: Introduce VMStateOffset
>> migration: Remove redundant flags
>> migration: Remove duplicate vmstate macros
>
> Nice work!
>
> I think I was only looking at VBUFFER side and I thought it was fine
> sticking with 32bit even signed or not, not a huge deal. But cleaning up
> VARRAY whole thing together looks definitely an improvement. I definitely
> like your version here.
>
> I assume with your series I can drop both of my patches here, right?
>
> [PATCH v2 1/5] migration: Fix possible overflow in vmstate_handle_alloc()
> https://lore.kernel.org/r/20260728210417.1925078-2-peterx@redhat.com
> (I'll still respin with the rest)
>
> [PATCH] vhost/migration: Fix incorrect size used in inflight->addr in VMSD
> https://lore.kernel.org/r/20260728153942.1891677-1-peterx@redhat.com
>
> The only missing piece would be an multiply overflow check in
> vmstate_handle_alloc(), if you could add that check too while rewritting
> that in patch 1 then I think it'll cover all.
>
> Vladimir's ask in the separate email makes sense: I wonder if we can also
> do one step further and merge VBUFFER into VARRAY.
>
> The other trivial thing is, while looking, I found one trivial macro
> VMSTATE_PARTIAL_VBUFFER not used; can drop it altogether.
I can look at it all. Just a heads-up, I'll be off for a couple of weeks
starting tomorrow.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/4] migration: Remove extra type-checking from vmstate macros
2026-07-30 15:37 ` Fabiano Rosas
@ 2026-07-30 16:16 ` Peter Xu
0 siblings, 0 replies; 22+ messages in thread
From: Peter Xu @ 2026-07-30 16:16 UTC (permalink / raw)
To: Fabiano Rosas
Cc: qemu-devel, Michael S . Tsirkin, Alexandr Moshkov,
Vladimir Sementsov-Ogievskiy
On Thu, Jul 30, 2026 at 12:37:13PM -0300, Fabiano Rosas wrote:
> I can look at it all. Just a heads-up, I'll be off for a couple of weeks
> starting tomorrow.
Thanks. I think it's fine we resolve this after your back, as long as
Michael is ok with it from vhost side. It's not real CVE, so I hope it's
OK.
--
Peter Xu
^ permalink raw reply [flat|nested] 22+ messages in thread