Am Mo., 2. März 2026 um 22:47 Uhr schrieb Peter Xu <[email protected]>: > > On Tue, Feb 17, 2026 at 04:25:16PM +0100, Alexander Mikhalitsyn wrote: > > From: Alexander Mikhalitsyn <[email protected]> > > > > Add VMSTATE_VARRAY_OF_POINTER_TO_STRUCT_ALLOC, which > > helps to save/restore a dynamic array of pointers to > > structures.
Dear Peter, > > Sorry for the late response. No problem at all, thanks for reviewing ;) > > > > > Signed-off-by: Alexander Mikhalitsyn <[email protected]> > > --- > > include/migration/vmstate.h | 21 +++++++++ > > migration/vmstate-types.c | 88 +++++++++++++++++++++++++++++++++++++ > > 2 files changed, 109 insertions(+) > > > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > > index 89f9f49d20a..bc6495a7f67 100644 > > --- a/include/migration/vmstate.h > > +++ b/include/migration/vmstate.h > > @@ -265,6 +265,7 @@ extern const VMStateInfo vmstate_info_bitmap; > > extern const VMStateInfo vmstate_info_qtailq; > > extern const VMStateInfo vmstate_info_gtree; > > extern const VMStateInfo vmstate_info_qlist; > > +extern const VMStateInfo vmstate_info_ptrs_array_entry; > > > > #define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0) > > /* > > @@ -537,6 +538,26 @@ extern const VMStateInfo vmstate_info_qlist; > > .offset = vmstate_offset_array(_s, _f, _type*, _n), \ > > } > > > > +/* > > + * For migrating a dynamically allocated uint32-indexed array > > + * of pointers to structures (with NULL entries and with auto memory > > allocation). > > + * > > + * _type: type of structure pointed to > > + * _vmsd: VMSD for structure > > + * start: size of structure pointed to (for auto memory allocation) > > + */ > > +#define VMSTATE_VARRAY_OF_POINTER_TO_STRUCT_ALLOC(_field, _state, > > _field_num, _version, _vmsd, _type) { \ > > + .name = (stringify(_field)), \ > > + .version_id = (_version), \ > > + .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \ > > + .info = &vmstate_info_ptrs_array_entry, \ > > I wonder if we need this hard-coded new info structure for this, more > below. > > > + .vmsd = &(_vmsd), \ > > + .start = sizeof(_type), \ > > + .size = sizeof(_type *), \ > > + .flags = VMS_VARRAY_UINT32|VMS_POINTER, \ > > + .offset = vmstate_offset_pointer(_state, _field, _type *), \ > > +} > > + > > #define VMSTATE_VARRAY_OF_POINTER_UINT32(_field, _state, _field_num, > > _version, _info, _type) { \ > > .name = (stringify(_field)), \ > > .version_id = (_version), \ > > diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c > > index 89cb2114721..3335377cd07 100644 > > --- a/migration/vmstate-types.c > > +++ b/migration/vmstate-types.c > > @@ -942,3 +942,91 @@ const VMStateInfo vmstate_info_qlist = { > > .get = get_qlist, > > .put = put_qlist, > > }; > > + > > +static int put_ptrs_array_entry(QEMUFile *f, void *ppv, size_t unused_size, > > + const VMStateField *field, JSONWriter > > *vmdesc) > > +{ > > + const VMStateDescription *vmsd = field->vmsd; > > + int ret; > > + Error *local_err = NULL; > > + void *pv; > > + > > + /* > > + * (ppv) is an address of an i-th element of a dynamic array. > > + * > > + * (ppv) can not be NULL unless we have some regression/bug in > > + * vmstate_save_state_v(), because it is result of pointer arithemic > > like: > > + * first_elem + size * i. > > + */ > > + if (ppv == NULL) { > > + error_report("vmstate: put_ptrs_array_entry must be called with > > ppv != NULL"); > > + return -EINVAL; > > + } > > + > > + /* get a pointer to a structure */ > > + pv = *(void **)ppv; > > Here IIUC it's almost what VMS_ARRAY_OF_POINTER should do. Could we > declare the vmsd with VMS_ARRAY_OF_POINTER so as to save this open-coded > dereference of pointer? Yes, this is something I tried to do in the first place, but this didn't work for my use-case. Let me explain what I'm doing in detail. I have the following structure: typedef struct NvmeCtrl { ... uint32_t num_queues; // < MAX number of elements in sq NvmeSQueue **sq; ... } NvmeCtrl; Suppose we have variable n with type NvmeCtrl *. // somewhere early during initialization of QEMU device: n->sq = g_new0(NvmeSQueue *, n->num_queues); // then in runtime (depending on what *guest* wants) we might create some sq[i], e.g: NvmeSQueue *sq; sq = g_malloc0(sizeof(*sq)); n->sq[3] = sq; please, note that n->sq[0], n->sq[1], n->sq[2] can still be NULL. So, we allow holes in that array. What I found is that if I raise VMS_ARRAY_OF_POINTER flag on VMStateField, then migration code in vmstate_load_state() will expect to see so-called "fake nullptr field" (vmsd_create_fake_nullptr_field() function call). But this is a problem, because our array at the time of vmstate_load_state() looks like (N == NULL): 0 1 2 3 4 5 N N N N N N while after vmstate_load_state() I expect it to be: 0 1 2 3 4 5 N N N SQ N N so, somebody should allocate memory for sq[3] and load data from image into that memory. Probably, it makes sense to somehow implement a combination of VMS_ARRAY_OF_POINTER and VMS_ALLOC, but I tried to be as less invasive as I can and decided to introduce a new kind of VMStateField instead. > > > + > > + if (pv == NULL) { > > + /* write a mark telling that there was a NULL pointer */ > > + qemu_put_byte(f, false); > > + return 0; > > + } > > + > > + /* if pointer is not NULL, dump the structure contents with help of > > vmsd */ > > + qemu_put_byte(f, true); > > + ret = vmstate_save_state(f, vmsd, pv, vmdesc, &local_err); > > This looks really like VMS_STRUCT, except that it also tries to detect if > the pointer is NULL, then save some dumps. Precisely, I'm mimicking behavior of VMS_STRUCT | VMS_ALLOC and VMS_ARRAY_OF_POINTER here. > > Is it feasible to make both queues to always be available so as to reuse > VMS_STRUCT? Or is it intentional here to avoid dumping queues where the > pointer is NULL? By "available" you mean to always allocate memory for all n->sq[i] elements? > > Even if for the latter, I wonder if we can have better way to do it. For > example, if we can integrate this directly into vmstate_save/load_state() > to support some new flag, VMS_ARRAY_OF_POINTER_DYNAMIC, then we can declare > some more "official" way to say some pointer of an dynamic array is NULL. > Yeah, I thought about this too, I believe this is a great idea. But taking into account that this is my first contribution to QEMU and I'm learning code around I tried to be very conservative and don't change any generic code ;-) I'm happy to do this work in -v3 if you would like me to do it and guide me a bit (I have just send a v2 series, but there was no changes in migration API part, only NVMe specifics). Kind regards, Alex > Thanks, > > > + if (ret) { > > + error_report_err(local_err); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +static int get_ptrs_array_entry(QEMUFile *f, void *ppv, size_t unused_size, > > + const VMStateField *field) > > +{ > > + int ret = 0; > > + Error *local_err = NULL; > > + const VMStateDescription *vmsd = field->vmsd; > > + /* size of structure pointed to by elements of array */ > > + size_t size = field->start; > > + > > + if (ppv == NULL) { > > + error_report("vmstate: get_ptrs_array_entry must be called with > > ppv != NULL"); > > + return -EINVAL; > > + } > > + > > + /* > > + * We start from a clean array, all elements must be NULL, unless > > + * something we haven't prepared for has changed in > > vmstate_save_state_v(). > > + * Let's check for this just in case. > > + */ > > + if (*(void **)ppv != NULL) { > > + error_report("vmstate: get_ptrs_array_entry must be called with > > *ppv == NULL"); > > + return -EINVAL; > > + } > > + > > + if (qemu_get_byte(f)) { > > + void *pv; > > + > > + /* allocate memory for structure */ > > + pv = g_malloc0(size); > > + ret = vmstate_load_state(f, vmsd, pv, vmsd->version_id, > > &local_err); > > + if (ret) { > > + error_report_err(local_err); > > + g_free(pv); > > + return ret; > > + } > > + > > + *(void **)ppv = pv; > > + } > > + > > + return ret; > > +} > > + > > +const VMStateInfo vmstate_info_ptrs_array_entry = { > > + .name = "ptrs_array_entry", > > + .get = get_ptrs_array_entry, > > + .put = put_ptrs_array_entry, > > +}; > > -- > > 2.47.3 > > > > -- > Peter Xu >
