[PATCH v2 0/2] migration/vmstate: Add VMState support to safely migrate GByteArray

[PATCH v2 0/2] migration/vmstate: Add VMState support to safely migrate GByteArray

[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

[PATCH v2 1/2] migration/vmstate: Add VMState support for GByteArray

[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

[PATCH v2 2/2] ui/vdagent: Use VMSTATE_GBYTEARRAY to safely migrate outbuf

[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

Re: [PATCH v2 1/2] migration/vmstate: Add VMState support for GByteArray

[Marc-André Lureau]

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>

Re: [PATCH v2 1/2] migration/vmstate: Add VMState support for GByteArray

[Peter Xu]

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

Re: [PATCH v2 1/2] migration/vmstate: Add VMState support for GByteArray

[Arun Menon]

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