* [PATCH v2 0/2] migration/vmstate: Add VMState support to safely migrate GByteArray @ 2026-04-09 17:51 Arun Menon 2026-04-09 17:51 ` [PATCH v2 1/2] migration/vmstate: Add VMState support for GByteArray Arun Menon 2026-04-09 17:51 ` [PATCH v2 2/2] ui/vdagent: Use VMSTATE_GBYTEARRAY to safely migrate outbuf Arun Menon 0 siblings, 2 replies; 6+ messages in thread From: Arun Menon @ 2026-04-09 17:51 UTC (permalink / raw) To: qemu-devel; +Cc: Fabiano Rosas, Marc-André Lureau, Peter Xu, Arun Menon In GLib, GByteArray is an object managed by the library. Currently, migrating a GByteArray requires treating it as a raw C struct and using VMSTATE_VBUFFER_ALLOC_UINT32. For example, see vmstate_vdba in ui/vdagent.c QEMU cannot pretend that GByteArray is a C struct and simply use VMS_ALLOC to g_malloc() the buffer. This is because, VMS_ALLOC blindly overwrites the data pointer with a newly allocated buffer, thereby leaking the previous memory. Besides, GLib tracks the array's capacity in a hidden alloc field. Bypassing GLib APIs leave this capacity out of sync with the newly allocated buffer, potentially leading to heap buffer overflows during subsequent g_byte_array_append() calls. This series adds a new VMState called VMSTATE_GBYTEARRAY, which will directly use GLib library API calls to create, or resize the object. This is then used in ui/vdaagent.c replacing the use of VMSTATE_VBUFFER_ALLOC_UINT32. Changes in v2: - Marc-André pointed out the problem of not updating the device state itself. To do that we should essentially pass a pointer to the GByteArray pointer. Remove VMS_POINTER from VMSTATE_GBYTEARRAY. This changes 'pv' to be the address of the pointer field (GByteArray **) instead of the pointer's value. - Update get_g_byte_array and put_g_byte_array to handle this additional level of indirection, allowing safe allocation and updating device state. - Link to v1: https://lore.kernel.org/all/20260406115247.4879-1-armenon@redhat.com/ Arun Menon (2): migration/vmstate: Add VMState support for GByteArray ui/vdagent: Use VMSTATE_GBYTEARRAY to safely migrate outbuf include/migration/vmstate.h | 10 ++++++++++ migration/vmstate-types.c | 37 +++++++++++++++++++++++++++++++++++++ ui/vdagent.c | 13 +------------ 3 files changed, 48 insertions(+), 12 deletions(-) -- 2.53.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] migration/vmstate: Add VMState support for GByteArray 2026-04-09 17:51 [PATCH v2 0/2] migration/vmstate: Add VMState support to safely migrate GByteArray Arun Menon @ 2026-04-09 17:51 ` Arun Menon 2026-04-09 18:52 ` Marc-André Lureau 2026-04-14 22:28 ` Peter Xu 2026-04-09 17:51 ` [PATCH v2 2/2] ui/vdagent: Use VMSTATE_GBYTEARRAY to safely migrate outbuf Arun Menon 1 sibling, 2 replies; 6+ messages in thread From: Arun Menon @ 2026-04-09 17:51 UTC (permalink / raw) To: qemu-devel; +Cc: Fabiano Rosas, Marc-André Lureau, Peter Xu, Arun Menon From: Arun Menon <armenon@redhat.com> In GLib, GByteArray is an object managed by the library. Currently, migrating a GByteArray requires treating it as a raw C struct and using VMSTATE_VBUFFER_ALLOC_UINT32. For example, see vmstate_vdba in ui/vdagent.c QEMU cannot pretend that GByteArray is a C struct and simply use VMS_ALLOC to g_malloc() the buffer. This is because, VMS_ALLOC blindly overwrites the data pointer with a newly allocated buffer, thereby leaking the previous memory. Besides, GLib tracks the array's capacity in a hidden alloc field. Bypassing GLib APIs leave this capacity out of sync with the newly allocated buffer, potentially leading to heap buffer overflows during subsequent g_byte_array_append() calls. This commit introduces VMSTATE_GBYTEARRAY which uses specific library API calls (g_byte_array_set_size()) to safely resize and populate the buffer. Signed-off-by: Arun Menon <armenon@redhat.com> Fix-Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Arun Menon <armenon@redhat.com> --- include/migration/vmstate.h | 10 ++++++++++ migration/vmstate-types.c | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 62c2abd0c4..f503a40ec0 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_g_byte_array; #define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0) /* @@ -892,6 +893,15 @@ extern const VMStateInfo vmstate_info_qlist; .start = offsetof(_type, _next), \ } +#define VMSTATE_GBYTEARRAY(_field, _state, _version) { \ + .name = (stringify(_field)), \ + .version_id = (_version), \ + .size = sizeof(GByteArray), \ + .info = &vmstate_info_g_byte_array, \ + .flags = VMS_SINGLE, \ + .offset = vmstate_offset_pointer(_state, _field, GByteArray), \ +} + /* _f : field name _f_n : num of elements field_name _n : num of elements diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c index 89cb211472..743c2092e9 100644 --- a/migration/vmstate-types.c +++ b/migration/vmstate-types.c @@ -942,3 +942,40 @@ const VMStateInfo vmstate_info_qlist = { .get = get_qlist, .put = put_qlist, }; + +static int get_g_byte_array(QEMUFile *f, void *pv, size_t size, + const VMStateField *field) +{ + GByteArray **byte_array = (GByteArray **)pv; + uint32_t len = qemu_get_be32(f); + + if (*byte_array == NULL) { + *byte_array = g_byte_array_new(); + } + + g_byte_array_set_size(*byte_array, len); + if (len > 0) { + qemu_get_buffer(f, (*byte_array)->data, len); + } + return 0; +} + +static int put_g_byte_array(QEMUFile *f, void *pv, size_t size, + const VMStateField *field, JSONWriter *vmdesc) +{ + GByteArray *byte_array = *(GByteArray **)pv; + uint32_t len = byte_array ? byte_array->len : 0; + + qemu_put_be32(f, len); + if (len > 0) { + qemu_put_buffer(f, byte_array->data, len); + } + + return 0; +} + +const VMStateInfo vmstate_info_g_byte_array = { + .name = "GByteArray", + .get = get_g_byte_array, + .put = put_g_byte_array, +}; -- 2.53.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] migration/vmstate: Add VMState support for GByteArray 2026-04-09 17:51 ` [PATCH v2 1/2] migration/vmstate: Add VMState support for GByteArray Arun Menon @ 2026-04-09 18:52 ` Marc-André Lureau 2026-04-14 22:28 ` Peter Xu 1 sibling, 0 replies; 6+ messages in thread From: Marc-André Lureau @ 2026-04-09 18:52 UTC (permalink / raw) To: Arun Menon; +Cc: qemu-devel, Fabiano Rosas, Peter Xu On Thu, Apr 9, 2026 at 9:52 PM Arun Menon <armenon@redhat.com> wrote: > > From: Arun Menon <armenon@redhat.com> > > In GLib, GByteArray is an object managed by the library. Currently, > migrating a GByteArray requires treating it as a raw C struct and using > VMSTATE_VBUFFER_ALLOC_UINT32. For example, see vmstate_vdba in > ui/vdagent.c > > QEMU cannot pretend that GByteArray is a C struct and simply use > VMS_ALLOC to g_malloc() the buffer. This is because, VMS_ALLOC blindly > overwrites the data pointer with a newly allocated buffer, thereby > leaking the previous memory. Besides, GLib tracks the array's capacity > in a hidden alloc field. Bypassing GLib APIs leave this capacity out of > sync with the newly allocated buffer, potentially leading to heap buffer > overflows during subsequent g_byte_array_append() calls. > > This commit introduces VMSTATE_GBYTEARRAY which uses specific library > API calls (g_byte_array_set_size()) to safely resize and populate the > buffer. > > Signed-off-by: Arun Menon <armenon@redhat.com> > Fix-Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com> > Signed-off-by: Arun Menon <armenon@redhat.com> you have 2 signed-off trailer tags > --- > include/migration/vmstate.h | 10 ++++++++++ > migration/vmstate-types.c | 37 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 47 insertions(+) > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > index 62c2abd0c4..f503a40ec0 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_g_byte_array; > > #define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0) > /* > @@ -892,6 +893,15 @@ extern const VMStateInfo vmstate_info_qlist; > .start = offsetof(_type, _next), \ > } > > +#define VMSTATE_GBYTEARRAY(_field, _state, _version) { \ > + .name = (stringify(_field)), \ > + .version_id = (_version), \ > + .size = sizeof(GByteArray), \ > + .info = &vmstate_info_g_byte_array, \ > + .flags = VMS_SINGLE, \ > + .offset = vmstate_offset_pointer(_state, _field, GByteArray), \ > +} > + > /* _f : field name > _f_n : num of elements field_name > _n : num of elements > diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c > index 89cb211472..743c2092e9 100644 > --- a/migration/vmstate-types.c > +++ b/migration/vmstate-types.c > @@ -942,3 +942,40 @@ const VMStateInfo vmstate_info_qlist = { > .get = get_qlist, > .put = put_qlist, > }; > + > +static int get_g_byte_array(QEMUFile *f, void *pv, size_t size, > + const VMStateField *field) > +{ > + GByteArray **byte_array = (GByteArray **)pv; > + uint32_t len = qemu_get_be32(f); > + > + if (*byte_array == NULL) { > + *byte_array = g_byte_array_new(); > + } > + > + g_byte_array_set_size(*byte_array, len); > + if (len > 0) { > + qemu_get_buffer(f, (*byte_array)->data, len); > + } > + return 0; > +} > + > +static int put_g_byte_array(QEMUFile *f, void *pv, size_t size, > + const VMStateField *field, JSONWriter *vmdesc) > +{ > + GByteArray *byte_array = *(GByteArray **)pv; > + uint32_t len = byte_array ? byte_array->len : 0; > + > + qemu_put_be32(f, len); > + if (len > 0) { > + qemu_put_buffer(f, byte_array->data, len); > + } > + > + return 0; > +} > + > +const VMStateInfo vmstate_info_g_byte_array = { > + .name = "GByteArray", > + .get = get_g_byte_array, > + .put = put_g_byte_array, > +}; > -- > 2.53.0 > the "if (len>0)" may be superflous, but ok Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] migration/vmstate: Add VMState support for GByteArray 2026-04-09 17:51 ` [PATCH v2 1/2] migration/vmstate: Add VMState support for GByteArray Arun Menon 2026-04-09 18:52 ` Marc-André Lureau @ 2026-04-14 22:28 ` Peter Xu 2026-04-15 13:53 ` Arun Menon 1 sibling, 1 reply; 6+ messages in thread From: Peter Xu @ 2026-04-14 22:28 UTC (permalink / raw) To: Arun Menon; +Cc: qemu-devel, Fabiano Rosas, Marc-André Lureau On Thu, Apr 09, 2026 at 11:21:24PM +0530, Arun Menon wrote: > From: Arun Menon <armenon@redhat.com> > > In GLib, GByteArray is an object managed by the library. Currently, > migrating a GByteArray requires treating it as a raw C struct and using > VMSTATE_VBUFFER_ALLOC_UINT32. For example, see vmstate_vdba in > ui/vdagent.c > > QEMU cannot pretend that GByteArray is a C struct and simply use > VMS_ALLOC to g_malloc() the buffer. This is because, VMS_ALLOC blindly > overwrites the data pointer with a newly allocated buffer, thereby > leaking the previous memory. Besides, GLib tracks the array's capacity > in a hidden alloc field. Bypassing GLib APIs leave this capacity out of > sync with the newly allocated buffer, potentially leading to heap buffer > overflows during subsequent g_byte_array_append() calls. > > This commit introduces VMSTATE_GBYTEARRAY which uses specific library > API calls (g_byte_array_set_size()) to safely resize and populate the > buffer. > > Signed-off-by: Arun Menon <armenon@redhat.com> > Fix-Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com> > Signed-off-by: Arun Menon <armenon@redhat.com> > --- > include/migration/vmstate.h | 10 ++++++++++ > migration/vmstate-types.c | 37 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 47 insertions(+) > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > index 62c2abd0c4..f503a40ec0 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_g_byte_array; > > #define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0) > /* > @@ -892,6 +893,15 @@ extern const VMStateInfo vmstate_info_qlist; > .start = offsetof(_type, _next), \ > } > > +#define VMSTATE_GBYTEARRAY(_field, _state, _version) { \ > + .name = (stringify(_field)), \ > + .version_id = (_version), \ > + .size = sizeof(GByteArray), \ > + .info = &vmstate_info_g_byte_array, \ > + .flags = VMS_SINGLE, \ > + .offset = vmstate_offset_pointer(_state, _field, GByteArray), \ > +} > + > /* _f : field name > _f_n : num of elements field_name > _n : num of elements > diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c > index 89cb211472..743c2092e9 100644 > --- a/migration/vmstate-types.c > +++ b/migration/vmstate-types.c > @@ -942,3 +942,40 @@ const VMStateInfo vmstate_info_qlist = { > .get = get_qlist, > .put = put_qlist, > }; > + > +static int get_g_byte_array(QEMUFile *f, void *pv, size_t size, > + const VMStateField *field) > +{ > + GByteArray **byte_array = (GByteArray **)pv; > + uint32_t len = qemu_get_be32(f); > + > + if (*byte_array == NULL) { > + *byte_array = g_byte_array_new(); > + } Not an immediate problem when GByteArray* will always be pre-allocated (even if with len=0) in the vdagent use case, but just to mention.. This is kind of over-taking the VMS_ALLOC semantics, and it will be error prone too as vmstate core has special handling of NULL pointers, you can see e.g. vmstate_save_state_v() has this piece of code: if (!curr_elem && size) { /* * If null pointer found (which should only happen in * an array of pointers), use null placeholder and do * not follow. */ inner_field = vmsd_create_fake_nullptr_field(field); is_null = true; } else { inner_field = field; is_null = false; } IIUC it'll start to generate nullptr markers if someone by accident removes the initialization of VDAgentChardev.outbuf someday, expecting it to work reading this code. Maybe it's easier we stick with non-NULL for most of vmstates, assert that the GByteArray* pointer to be valid while get() it. Same to put(). > + > + g_byte_array_set_size(*byte_array, len); > + if (len > 0) { > + qemu_get_buffer(f, (*byte_array)->data, len); > + } > + return 0; > +} > + > +static int put_g_byte_array(QEMUFile *f, void *pv, size_t size, > + const VMStateField *field, JSONWriter *vmdesc) > +{ > + GByteArray *byte_array = *(GByteArray **)pv; > + uint32_t len = byte_array ? byte_array->len : 0; > + > + qemu_put_be32(f, len); > + if (len > 0) { > + qemu_put_buffer(f, byte_array->data, len); > + } > + > + return 0; > +} > + > +const VMStateInfo vmstate_info_g_byte_array = { > + .name = "GByteArray", > + .get = get_g_byte_array, > + .put = put_g_byte_array, > +}; The other thing to mention is, if it's only about one g_byte_array_set_size() call after getting LEN and before getting the byte array, we could also use tricks like pre_load(), e.g., instead of the current VMSD defined as this: static const VMStateDescription vmstate_vdba = { .name = "vdagent/bytearray", .version_id = 0, .minimum_version_id = 0, .fields = (const VMStateField[]) { VMSTATE_UINT32(len, GByteArray), VMSTATE_VBUFFER_ALLOC_UINT32(data, GByteArray, 0, 0, len), VMSTATE_END_OF_LIST() } }; We could change VMSTATE_VBUFFER_ALLOC_UINT32() to be a VMSTATE_STRUCT_POINTER, then within that the internal vmsd can provide a pre_load() hook doing g_byte_array_set_size(): since all fields will be loaded in order, when reaching there it's guaranteed LEN is loaded. It'll avoid introducing get()/put() completely, IIUC. But no strong feelings on either approach. Thanks, -- Peter Xu ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] migration/vmstate: Add VMState support for GByteArray 2026-04-14 22:28 ` Peter Xu @ 2026-04-15 13:53 ` Arun Menon 0 siblings, 0 replies; 6+ messages in thread From: Arun Menon @ 2026-04-15 13:53 UTC (permalink / raw) To: Peter Xu; +Cc: qemu-devel, Fabiano Rosas, Marc-André Lureau Hi Peter, On Tue, Apr 14, 2026 at 06:28:52PM -0400, Peter Xu wrote: > On Thu, Apr 09, 2026 at 11:21:24PM +0530, Arun Menon wrote: > > From: Arun Menon <armenon@redhat.com> > > > > In GLib, GByteArray is an object managed by the library. Currently, > > migrating a GByteArray requires treating it as a raw C struct and using > > VMSTATE_VBUFFER_ALLOC_UINT32. For example, see vmstate_vdba in > > ui/vdagent.c > > > > QEMU cannot pretend that GByteArray is a C struct and simply use > > VMS_ALLOC to g_malloc() the buffer. This is because, VMS_ALLOC blindly > > overwrites the data pointer with a newly allocated buffer, thereby > > leaking the previous memory. Besides, GLib tracks the array's capacity > > in a hidden alloc field. Bypassing GLib APIs leave this capacity out of > > sync with the newly allocated buffer, potentially leading to heap buffer > > overflows during subsequent g_byte_array_append() calls. > > > > This commit introduces VMSTATE_GBYTEARRAY which uses specific library > > API calls (g_byte_array_set_size()) to safely resize and populate the > > buffer. > > > > Signed-off-by: Arun Menon <armenon@redhat.com> > > Fix-Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > Signed-off-by: Arun Menon <armenon@redhat.com> > > --- > > include/migration/vmstate.h | 10 ++++++++++ > > migration/vmstate-types.c | 37 +++++++++++++++++++++++++++++++++++++ > > 2 files changed, 47 insertions(+) > > > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > > index 62c2abd0c4..f503a40ec0 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_g_byte_array; > > > > #define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0) > > /* > > @@ -892,6 +893,15 @@ extern const VMStateInfo vmstate_info_qlist; > > .start = offsetof(_type, _next), \ > > } > > > > +#define VMSTATE_GBYTEARRAY(_field, _state, _version) { \ > > + .name = (stringify(_field)), \ > > + .version_id = (_version), \ > > + .size = sizeof(GByteArray), \ > > + .info = &vmstate_info_g_byte_array, \ > > + .flags = VMS_SINGLE, \ > > + .offset = vmstate_offset_pointer(_state, _field, GByteArray), \ > > +} > > + > > /* _f : field name > > _f_n : num of elements field_name > > _n : num of elements > > diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c > > index 89cb211472..743c2092e9 100644 > > --- a/migration/vmstate-types.c > > +++ b/migration/vmstate-types.c > > @@ -942,3 +942,40 @@ const VMStateInfo vmstate_info_qlist = { > > .get = get_qlist, > > .put = put_qlist, > > }; > > + > > +static int get_g_byte_array(QEMUFile *f, void *pv, size_t size, > > + const VMStateField *field) > > +{ > > + GByteArray **byte_array = (GByteArray **)pv; > > + uint32_t len = qemu_get_be32(f); > > + > > + if (*byte_array == NULL) { > > + *byte_array = g_byte_array_new(); > > + } > > Not an immediate problem when GByteArray* will always be pre-allocated > (even if with len=0) in the vdagent use case, but just to mention.. > > This is kind of over-taking the VMS_ALLOC semantics, and it will be error > prone too as vmstate core has special handling of NULL pointers, you can > see e.g. vmstate_save_state_v() has this piece of code: > > if (!curr_elem && size) { > /* > * If null pointer found (which should only happen in > * an array of pointers), use null placeholder and do > * not follow. > */ > inner_field = vmsd_create_fake_nullptr_field(field); > is_null = true; > } else { > inner_field = field; > is_null = false; > } > > IIUC it'll start to generate nullptr markers if someone by accident removes > the initialization of VDAgentChardev.outbuf someday, expecting it to work > reading this code. > > Maybe it's easier we stick with non-NULL for most of vmstates, assert that > the GByteArray* pointer to be valid while get() it. Same to put(). > > > + > > + g_byte_array_set_size(*byte_array, len); > > + if (len > 0) { > > + qemu_get_buffer(f, (*byte_array)->data, len); > > + } > > + return 0; > > +} > > + > > +static int put_g_byte_array(QEMUFile *f, void *pv, size_t size, > > + const VMStateField *field, JSONWriter *vmdesc) > > +{ > > + GByteArray *byte_array = *(GByteArray **)pv; > > + uint32_t len = byte_array ? byte_array->len : 0; > > + > > + qemu_put_be32(f, len); > > + if (len > 0) { > > + qemu_put_buffer(f, byte_array->data, len); > > + } > > + > > + return 0; > > +} > > + > > +const VMStateInfo vmstate_info_g_byte_array = { > > + .name = "GByteArray", > > + .get = get_g_byte_array, > > + .put = put_g_byte_array, > > +}; > > The other thing to mention is, if it's only about one > g_byte_array_set_size() call after getting LEN and before getting the byte > array, we could also use tricks like pre_load(), e.g., instead of the > current VMSD defined as this: > > static const VMStateDescription vmstate_vdba = { > .name = "vdagent/bytearray", > .version_id = 0, > .minimum_version_id = 0, > .fields = (const VMStateField[]) { > VMSTATE_UINT32(len, GByteArray), > VMSTATE_VBUFFER_ALLOC_UINT32(data, GByteArray, 0, 0, len), > VMSTATE_END_OF_LIST() > } > }; > > We could change VMSTATE_VBUFFER_ALLOC_UINT32() to be a > VMSTATE_STRUCT_POINTER, then within that the internal vmsd can provide a > pre_load() hook doing g_byte_array_set_size(): since all fields will be > loaded in order, when reaching there it's guaranteed LEN is loaded. > > It'll avoid introducing get()/put() completely, IIUC. But no strong > feelings on either approach. Thank you for the detailed explanation! I completely see the design flaw now regarding the nullptr marker. I am more inclined to make the one liner change of adding asserts, because I am not sure where VMStateDescription of GByteArray should live. Since it is an external GLib type, it does not have a dedicated QEMU wrapper file in util/ directory. Placing it inside migration/vmstate-types.c feels a bit out of place as that file is, AFAICT, reserved for low-level VMStateInfo primitives. I want to use the same GByteArray VMStateInfo to migrate TPM CRBState struct in another change. > > Thanks, > > -- > Peter Xu > Regards, Arun Menon ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] ui/vdagent: Use VMSTATE_GBYTEARRAY to safely migrate outbuf 2026-04-09 17:51 [PATCH v2 0/2] migration/vmstate: Add VMState support to safely migrate GByteArray Arun Menon 2026-04-09 17:51 ` [PATCH v2 1/2] migration/vmstate: Add VMState support for GByteArray Arun Menon @ 2026-04-09 17:51 ` Arun Menon 1 sibling, 0 replies; 6+ messages in thread From: Arun Menon @ 2026-04-09 17:51 UTC (permalink / raw) To: qemu-devel; +Cc: Fabiano Rosas, Marc-André Lureau, Peter Xu, Arun Menon From: Arun Menon <armenon@redhat.com> Migrating a GLib GByteArray is now possible directly using the newly introduced VMSTATE_GBYTEARRAY. It uses the standard GLib API calls to create the array, or resize it. This is safer than implementing a C struct and manually updating the data and len fields. This commit uses the VMSTATE_GBYTEARRAY in vdagent to store the outbuf variable. Signed-off-by: Arun Menon <armenon@redhat.com> Fix-Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- ui/vdagent.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/ui/vdagent.c b/ui/vdagent.c index bb0c4aa14c..9206e47000 100644 --- a/ui/vdagent.c +++ b/ui/vdagent.c @@ -967,17 +967,6 @@ static const VMStateDescription vmstate_chunk = { } }; -static const VMStateDescription vmstate_vdba = { - .name = "vdagent/bytearray", - .version_id = 0, - .minimum_version_id = 0, - .fields = (const VMStateField[]) { - VMSTATE_UINT32(len, GByteArray), - VMSTATE_VBUFFER_ALLOC_UINT32(data, GByteArray, 0, 0, len), - VMSTATE_END_OF_LIST() - } -}; - struct CBInfoArray { uint32_t n; QemuClipboardInfo cbinfo[QEMU_CLIPBOARD_SELECTION__COUNT]; @@ -1067,7 +1056,7 @@ static const VMStateDescription vmstate_vdagent = { VMSTATE_UINT32(xsize, VDAgentChardev), VMSTATE_UINT32(xoff, VDAgentChardev), VMSTATE_VBUFFER_ALLOC_UINT32(xbuf, VDAgentChardev, 0, 0, xsize), - VMSTATE_STRUCT_POINTER(outbuf, VDAgentChardev, vmstate_vdba, GByteArray), + VMSTATE_GBYTEARRAY(outbuf, VDAgentChardev, 0), VMSTATE_UINT32(mouse_x, VDAgentChardev), VMSTATE_UINT32(mouse_y, VDAgentChardev), VMSTATE_UINT32(mouse_btn, VDAgentChardev), -- 2.53.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-04-15 13:54 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-04-09 17:51 [PATCH v2 0/2] migration/vmstate: Add VMState support to safely migrate GByteArray Arun Menon 2026-04-09 17:51 ` [PATCH v2 1/2] migration/vmstate: Add VMState support for GByteArray Arun Menon 2026-04-09 18:52 ` Marc-André Lureau 2026-04-14 22:28 ` Peter Xu 2026-04-15 13:53 ` Arun Menon 2026-04-09 17:51 ` [PATCH v2 2/2] ui/vdagent: Use VMSTATE_GBYTEARRAY to safely migrate outbuf Arun Menon
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.