We used to have one vmstate called "nullptr" which is only used to generate
one-byte hint to say one pointer is NULL.
Let's extend its use so that it will generate another byte to say the
pointer is non-NULL.
With that, the name of the info struct (or functions) do not apply anymore.
Update correspondingly.
No functional change intended yet.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/migration/vmstate.h | 9 +++++++--
migration/vmstate-types.c | 34 ++++++++++++++++------------------
migration/vmstate.c | 29 ++++++++++++++---------------
3 files changed, 37 insertions(+), 35 deletions(-)
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 092e8f7e9a..2e51b5ea04 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -282,9 +282,14 @@ extern const VMStateInfo vmstate_info_uint32;
extern const VMStateInfo vmstate_info_uint64;
extern const VMStateInfo vmstate_info_fd;
-/** Put this in the stream when migrating a null pointer.*/
+/*
+ * Put this in the stream when migrating a pointer to reflect either a NULL
+ * or valid pointer.
+ */
#define VMS_MARKER_PTR_NULL (0x30U) /* '0' */
-extern const VMStateInfo vmstate_info_nullptr;
+#define VMS_MARKER_PTR_VALID (0x31U) /* '1' */
+
+extern const VMStateInfo vmstate_info_ptr_marker;
extern const VMStateInfo vmstate_info_cpudouble;
diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
index 7622cf8f01..b31689fc3c 100644
--- a/migration/vmstate-types.c
+++ b/migration/vmstate-types.c
@@ -359,36 +359,34 @@ const VMStateInfo vmstate_info_fd = {
.save = save_fd,
};
-static bool load_nullptr(QEMUFile *f, void *pv, size_t size,
- const VMStateField *field, Error **errp)
+static bool load_ptr_marker(QEMUFile *f, void *pv, size_t size,
+ const VMStateField *field, Error **errp)
{
- if (qemu_get_byte(f) == VMS_MARKER_PTR_NULL) {
+ int byte = qemu_get_byte(f);
+
+ if (byte == VMS_MARKER_PTR_NULL || byte == VMS_MARKER_PTR_VALID) {
+ /* TODO: process PTR_VALID case */
return true;
}
- error_setg(errp, "vmstate: load_nullptr expected VMS_NULLPTR_MARKER");
+ error_setg(errp, "%s: unexpected ptr marker: %d", __func__, byte);
return false;
}
-static bool save_nullptr(QEMUFile *f, void *pv, size_t size,
- const VMStateField *field, JSONWriter *vmdesc,
- Error **errp)
+static bool save_ptr_marker(QEMUFile *f, void *pv, size_t size,
+ const VMStateField *field, JSONWriter *vmdesc,
+ Error **errp)
{
- if (pv == NULL) {
- qemu_put_byte(f, VMS_MARKER_PTR_NULL);
- return true;
- }
-
- error_setg(errp, "vmstate: save_nullptr must be called with pv == NULL");
- return false;
+ qemu_put_byte(f, pv ? VMS_MARKER_PTR_VALID : VMS_MARKER_PTR_NULL);
+ return true;
}
-const VMStateInfo vmstate_info_nullptr = {
- .name = "nullptr",
- .load = load_nullptr,
- .save = save_nullptr,
+const VMStateInfo vmstate_info_ptr_marker = {
+ .name = "ptr-marker",
+ .load = load_ptr_marker,
+ .save = save_ptr_marker,
};
/* 64 bit unsigned int. See that the received value is the same than the one
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 5a6b352764..a3a5f25946 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -55,12 +55,12 @@ vmstate_field_exists(const VMStateDescription *vmsd, const VMStateField *field,
}
/*
- * Create a fake nullptr field when there's a NULL pointer detected in the
+ * Create a ptr marker field when there's a NULL pointer detected in the
* array of a VMS_ARRAY_OF_POINTER VMSD field. It's needed because we
* can't dereference the NULL pointer.
*/
static const VMStateField *
-vmsd_create_fake_nullptr_field(const VMStateField *field)
+vmsd_create_ptr_marker_field(const VMStateField *field)
{
VMStateField *fake = g_new0(VMStateField, 1);
@@ -76,7 +76,7 @@ vmsd_create_fake_nullptr_field(const VMStateField *field)
/* See vmstate_info_nullptr - use 1 byte to represent nullptr */
fake->size = 1;
- fake->info = &vmstate_info_nullptr;
+ fake->info = &vmstate_info_ptr_marker;
fake->flags = VMS_SINGLE;
/* All the rest fields shouldn't matter.. */
@@ -278,7 +278,7 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
* an array of pointers), use null placeholder and do
* not follow.
*/
- inner_field = vmsd_create_fake_nullptr_field(field);
+ inner_field = vmsd_create_ptr_marker_field(field);
} else {
inner_field = field;
}
@@ -567,7 +567,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
int i, n_elems = vmstate_n_elems(opaque, field);
int size = vmstate_size(opaque, field);
JSONWriter *vmdesc_loop = vmdesc;
- bool is_prev_null = false;
+ bool use_marker_field_prev = false;
trace_vmstate_save_state_loop(vmsd->name, field->name, n_elems);
if (field->flags & VMS_POINTER) {
@@ -578,7 +578,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
for (i = 0; i < n_elems; i++) {
void *curr_elem = first_elem + size * i;
const VMStateField *inner_field;
- bool is_null;
+ bool use_marker_field;
int max_elems = n_elems - i;
if (field->flags & VMS_ARRAY_OF_POINTER) {
@@ -586,17 +586,16 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
curr_elem = *(void **)curr_elem;
}
- if (!curr_elem && size) {
+ use_marker_field = !curr_elem && size;
+ if (use_marker_field) {
/*
* 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;
+ inner_field = vmsd_create_ptr_marker_field(field);
} else {
inner_field = field;
- is_null = false;
}
/*
@@ -612,16 +611,16 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
*/
if (vmdesc && vmsd_can_compress(field) &&
(field->flags & VMS_ARRAY_OF_POINTER) &&
- is_null != is_prev_null) {
+ use_marker_field != use_marker_field_prev) {
- is_prev_null = is_null;
+ use_marker_field_prev = use_marker_field;
vmdesc_loop = vmdesc;
for (int j = i + 1; j < n_elems; j++) {
void *elem = *(void **)(first_elem + size * j);
- bool elem_is_null = !elem && size;
+ bool elem_use_marker_field = !elem && size;
- if (is_null != elem_is_null) {
+ if (use_marker_field != elem_use_marker_field) {
max_elems = j - i;
break;
}
@@ -633,7 +632,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
i, max_elems, errp);
/* If we used a fake temp field.. free it now */
- if (is_null) {
+ if (use_marker_field) {
g_clear_pointer((gpointer *)&inner_field, g_free);
}
--
2.50.1Peter Xu <peterx@redhat.com> writes:
> We used to have one vmstate called "nullptr" which is only used to generate
> one-byte hint to say one pointer is NULL.
>
> Let's extend its use so that it will generate another byte to say the
> pointer is non-NULL.
>
> With that, the name of the info struct (or functions) do not apply anymore.
> Update correspondingly.
>
> No functional change intended yet.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> include/migration/vmstate.h | 9 +++++++--
> migration/vmstate-types.c | 34 ++++++++++++++++------------------
> migration/vmstate.c | 29 ++++++++++++++---------------
> 3 files changed, 37 insertions(+), 35 deletions(-)
>
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index 092e8f7e9a..2e51b5ea04 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -282,9 +282,14 @@ extern const VMStateInfo vmstate_info_uint32;
> extern const VMStateInfo vmstate_info_uint64;
> extern const VMStateInfo vmstate_info_fd;
>
> -/** Put this in the stream when migrating a null pointer.*/
> +/*
> + * Put this in the stream when migrating a pointer to reflect either a NULL
> + * or valid pointer.
> + */
> #define VMS_MARKER_PTR_NULL (0x30U) /* '0' */
> -extern const VMStateInfo vmstate_info_nullptr;
> +#define VMS_MARKER_PTR_VALID (0x31U) /* '1' */
> +
> +extern const VMStateInfo vmstate_info_ptr_marker;
>
> extern const VMStateInfo vmstate_info_cpudouble;
>
> diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
> index 7622cf8f01..b31689fc3c 100644
> --- a/migration/vmstate-types.c
> +++ b/migration/vmstate-types.c
> @@ -359,36 +359,34 @@ const VMStateInfo vmstate_info_fd = {
> .save = save_fd,
> };
>
> -static bool load_nullptr(QEMUFile *f, void *pv, size_t size,
> - const VMStateField *field, Error **errp)
> +static bool load_ptr_marker(QEMUFile *f, void *pv, size_t size,
> + const VMStateField *field, Error **errp)
>
> {
> - if (qemu_get_byte(f) == VMS_MARKER_PTR_NULL) {
> + int byte = qemu_get_byte(f);
> +
> + if (byte == VMS_MARKER_PTR_NULL || byte == VMS_MARKER_PTR_VALID) {
> + /* TODO: process PTR_VALID case */
> return true;
> }
>
> - error_setg(errp, "vmstate: load_nullptr expected VMS_NULLPTR_MARKER");
> + error_setg(errp, "%s: unexpected ptr marker: %d", __func__, byte);
> return false;
> }
>
> -static bool save_nullptr(QEMUFile *f, void *pv, size_t size,
> - const VMStateField *field, JSONWriter *vmdesc,
> - Error **errp)
> +static bool save_ptr_marker(QEMUFile *f, void *pv, size_t size,
> + const VMStateField *field, JSONWriter *vmdesc,
> + Error **errp)
>
> {
> - if (pv == NULL) {
> - qemu_put_byte(f, VMS_MARKER_PTR_NULL);
> - return true;
> - }
> -
> - error_setg(errp, "vmstate: save_nullptr must be called with pv == NULL");
> - return false;
> + qemu_put_byte(f, pv ? VMS_MARKER_PTR_VALID : VMS_MARKER_PTR_NULL);
> + return true;
> }
>
> -const VMStateInfo vmstate_info_nullptr = {
> - .name = "nullptr",
> - .load = load_nullptr,
> - .save = save_nullptr,
> +const VMStateInfo vmstate_info_ptr_marker = {
> + .name = "ptr-marker",
> + .load = load_ptr_marker,
> + .save = save_ptr_marker,
> };
>
> /* 64 bit unsigned int. See that the received value is the same than the one
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index 5a6b352764..a3a5f25946 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -55,12 +55,12 @@ vmstate_field_exists(const VMStateDescription *vmsd, const VMStateField *field,
> }
>
> /*
> - * Create a fake nullptr field when there's a NULL pointer detected in the
> + * Create a ptr marker field when there's a NULL pointer detected in the
> * array of a VMS_ARRAY_OF_POINTER VMSD field. It's needed because we
> * can't dereference the NULL pointer.
> */
> static const VMStateField *
> -vmsd_create_fake_nullptr_field(const VMStateField *field)
> +vmsd_create_ptr_marker_field(const VMStateField *field)
> {
> VMStateField *fake = g_new0(VMStateField, 1);
>
> @@ -76,7 +76,7 @@ vmsd_create_fake_nullptr_field(const VMStateField *field)
>
> /* See vmstate_info_nullptr - use 1 byte to represent nullptr */
This comment needs updating.
> fake->size = 1;
> - fake->info = &vmstate_info_nullptr;
> + fake->info = &vmstate_info_ptr_marker;
> fake->flags = VMS_SINGLE;
>
> /* All the rest fields shouldn't matter.. */
> @@ -278,7 +278,7 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
> * an array of pointers), use null placeholder and do
> * not follow.
> */
> - inner_field = vmsd_create_fake_nullptr_field(field);
> + inner_field = vmsd_create_ptr_marker_field(field);
> } else {
> inner_field = field;
> }
> @@ -567,7 +567,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> int i, n_elems = vmstate_n_elems(opaque, field);
> int size = vmstate_size(opaque, field);
> JSONWriter *vmdesc_loop = vmdesc;
> - bool is_prev_null = false;
> + bool use_marker_field_prev = false;
The logic below won't handle valid pointer as well, will it? We could
leave the is_null/is_prev_null terminology because it's way more
descriptive. Right? We're adding the marker here because we're
compressing and know the field is null.
>
> trace_vmstate_save_state_loop(vmsd->name, field->name, n_elems);
> if (field->flags & VMS_POINTER) {
> @@ -578,7 +578,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> for (i = 0; i < n_elems; i++) {
> void *curr_elem = first_elem + size * i;
> const VMStateField *inner_field;
> - bool is_null;
> + bool use_marker_field;
> int max_elems = n_elems - i;
>
> if (field->flags & VMS_ARRAY_OF_POINTER) {
> @@ -586,17 +586,16 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> curr_elem = *(void **)curr_elem;
> }
>
> - if (!curr_elem && size) {
> + use_marker_field = !curr_elem && size;
> + if (use_marker_field) {
> /*
> * 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;
> + inner_field = vmsd_create_ptr_marker_field(field);
> } else {
> inner_field = field;
> - is_null = false;
> }
>
> /*
> @@ -612,16 +611,16 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> */
> if (vmdesc && vmsd_can_compress(field) &&
> (field->flags & VMS_ARRAY_OF_POINTER) &&
> - is_null != is_prev_null) {
> + use_marker_field != use_marker_field_prev) {
>
> - is_prev_null = is_null;
> + use_marker_field_prev = use_marker_field;
> vmdesc_loop = vmdesc;
>
> for (int j = i + 1; j < n_elems; j++) {
> void *elem = *(void **)(first_elem + size * j);
> - bool elem_is_null = !elem && size;
> + bool elem_use_marker_field = !elem && size;
See?
>
> - if (is_null != elem_is_null) {
> + if (use_marker_field != elem_use_marker_field) {
> max_elems = j - i;
> break;
> }
> @@ -633,7 +632,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> i, max_elems, errp);
>
> /* If we used a fake temp field.. free it now */
> - if (is_null) {
> + if (use_marker_field) {
> g_clear_pointer((gpointer *)&inner_field, g_free);
> }
On Thu, Mar 19, 2026 at 05:46:36PM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
>
> > We used to have one vmstate called "nullptr" which is only used to generate
> > one-byte hint to say one pointer is NULL.
> >
> > Let's extend its use so that it will generate another byte to say the
> > pointer is non-NULL.
> >
> > With that, the name of the info struct (or functions) do not apply anymore.
> > Update correspondingly.
> >
> > No functional change intended yet.
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> > include/migration/vmstate.h | 9 +++++++--
> > migration/vmstate-types.c | 34 ++++++++++++++++------------------
> > migration/vmstate.c | 29 ++++++++++++++---------------
> > 3 files changed, 37 insertions(+), 35 deletions(-)
> >
> > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> > index 092e8f7e9a..2e51b5ea04 100644
> > --- a/include/migration/vmstate.h
> > +++ b/include/migration/vmstate.h
> > @@ -282,9 +282,14 @@ extern const VMStateInfo vmstate_info_uint32;
> > extern const VMStateInfo vmstate_info_uint64;
> > extern const VMStateInfo vmstate_info_fd;
> >
> > -/** Put this in the stream when migrating a null pointer.*/
> > +/*
> > + * Put this in the stream when migrating a pointer to reflect either a NULL
> > + * or valid pointer.
> > + */
> > #define VMS_MARKER_PTR_NULL (0x30U) /* '0' */
> > -extern const VMStateInfo vmstate_info_nullptr;
> > +#define VMS_MARKER_PTR_VALID (0x31U) /* '1' */
> > +
> > +extern const VMStateInfo vmstate_info_ptr_marker;
> >
> > extern const VMStateInfo vmstate_info_cpudouble;
> >
> > diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
> > index 7622cf8f01..b31689fc3c 100644
> > --- a/migration/vmstate-types.c
> > +++ b/migration/vmstate-types.c
> > @@ -359,36 +359,34 @@ const VMStateInfo vmstate_info_fd = {
> > .save = save_fd,
> > };
> >
> > -static bool load_nullptr(QEMUFile *f, void *pv, size_t size,
> > - const VMStateField *field, Error **errp)
> > +static bool load_ptr_marker(QEMUFile *f, void *pv, size_t size,
> > + const VMStateField *field, Error **errp)
> >
> > {
> > - if (qemu_get_byte(f) == VMS_MARKER_PTR_NULL) {
> > + int byte = qemu_get_byte(f);
> > +
> > + if (byte == VMS_MARKER_PTR_NULL || byte == VMS_MARKER_PTR_VALID) {
> > + /* TODO: process PTR_VALID case */
> > return true;
> > }
> >
> > - error_setg(errp, "vmstate: load_nullptr expected VMS_NULLPTR_MARKER");
> > + error_setg(errp, "%s: unexpected ptr marker: %d", __func__, byte);
> > return false;
> > }
> >
> > -static bool save_nullptr(QEMUFile *f, void *pv, size_t size,
> > - const VMStateField *field, JSONWriter *vmdesc,
> > - Error **errp)
> > +static bool save_ptr_marker(QEMUFile *f, void *pv, size_t size,
> > + const VMStateField *field, JSONWriter *vmdesc,
> > + Error **errp)
> >
> > {
> > - if (pv == NULL) {
> > - qemu_put_byte(f, VMS_MARKER_PTR_NULL);
> > - return true;
> > - }
> > -
> > - error_setg(errp, "vmstate: save_nullptr must be called with pv == NULL");
> > - return false;
> > + qemu_put_byte(f, pv ? VMS_MARKER_PTR_VALID : VMS_MARKER_PTR_NULL);
> > + return true;
> > }
> >
> > -const VMStateInfo vmstate_info_nullptr = {
> > - .name = "nullptr",
> > - .load = load_nullptr,
> > - .save = save_nullptr,
> > +const VMStateInfo vmstate_info_ptr_marker = {
> > + .name = "ptr-marker",
> > + .load = load_ptr_marker,
> > + .save = save_ptr_marker,
> > };
> >
> > /* 64 bit unsigned int. See that the received value is the same than the one
> > diff --git a/migration/vmstate.c b/migration/vmstate.c
> > index 5a6b352764..a3a5f25946 100644
> > --- a/migration/vmstate.c
> > +++ b/migration/vmstate.c
> > @@ -55,12 +55,12 @@ vmstate_field_exists(const VMStateDescription *vmsd, const VMStateField *field,
> > }
> >
> > /*
> > - * Create a fake nullptr field when there's a NULL pointer detected in the
> > + * Create a ptr marker field when there's a NULL pointer detected in the
> > * array of a VMS_ARRAY_OF_POINTER VMSD field. It's needed because we
> > * can't dereference the NULL pointer.
> > */
> > static const VMStateField *
> > -vmsd_create_fake_nullptr_field(const VMStateField *field)
> > +vmsd_create_ptr_marker_field(const VMStateField *field)
> > {
> > VMStateField *fake = g_new0(VMStateField, 1);
> >
> > @@ -76,7 +76,7 @@ vmsd_create_fake_nullptr_field(const VMStateField *field)
> >
> > /* See vmstate_info_nullptr - use 1 byte to represent nullptr */
>
> This comment needs updating.
Fixed.
>
> > fake->size = 1;
> > - fake->info = &vmstate_info_nullptr;
> > + fake->info = &vmstate_info_ptr_marker;
> > fake->flags = VMS_SINGLE;
> >
> > /* All the rest fields shouldn't matter.. */
> > @@ -278,7 +278,7 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
> > * an array of pointers), use null placeholder and do
> > * not follow.
> > */
> > - inner_field = vmsd_create_fake_nullptr_field(field);
> > + inner_field = vmsd_create_ptr_marker_field(field);
> > } else {
> > inner_field = field;
> > }
> > @@ -567,7 +567,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> > int i, n_elems = vmstate_n_elems(opaque, field);
> > int size = vmstate_size(opaque, field);
> > JSONWriter *vmdesc_loop = vmdesc;
> > - bool is_prev_null = false;
> > + bool use_marker_field_prev = false;
>
> The logic below won't handle valid pointer as well, will it? We could
> leave the is_null/is_prev_null terminology because it's way more
> descriptive. Right? We're adding the marker here because we're
> compressing and know the field is null.
I recovered the old bool for better readability.
One other note is I'll squash below analyze-migration.py change into this
patch when repost, let me know if there's early comments:
===8<===
From 82ac38a85ecd990a1aaa7b45b172e71f0d5b84da Mon Sep 17 00:00:00 2001
From: Peter Xu <peterx@redhat.com>
Date: Thu, 26 Mar 2026 14:57:56 -0400
Subject: [PATCH] scripts/analyze-migration.py: Support new ptr-marker field
Signed-off-by: Peter Xu <peterx@redhat.com>
---
scripts/analyze-migration.py | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py
index e81deab8f9..1771ff781b 100755
--- a/scripts/analyze-migration.py
+++ b/scripts/analyze-migration.py
@@ -469,26 +469,26 @@ def __init__(self, desc, file):
super(VMSDFieldIntLE, self).__init__(desc, file)
self.dtype = '<i%d' % self.size
-class VMSDFieldNull(VMSDFieldGeneric):
+class VMSDFieldPtrMarker(VMSDFieldGeneric):
NULL_PTR_MARKER = b'0'
+ VALID_PTR_MARKER = b'1'
def __init__(self, desc, file):
- super(VMSDFieldNull, self).__init__(desc, file)
+ super(VMSDFieldPtrMarker, self).__init__(desc, file)
def __repr__(self):
- # A NULL pointer is encoded in the stream as a '0' to
- # disambiguate from a mere 0x0 value and avoid consumers
- # trying to follow the NULL pointer. Displaying '0', 0x30 or
- # 0x0 when analyzing the JSON debug stream could become
+ # A NULL / non-NULL pointer may be encoded in the stream as a
+ # '0'/'1' to represent the status of the pointer. Displaying '0',
+ # 0x30 or 0x0 when analyzing the JSON debug stream could become
# confusing, so use an explicit term instead.
- return "nullptr"
+ return "null-ptr" if self.data == self.NULL_PTR_MARKER else "valid-ptr"
def __str__(self):
return self.__repr__()
def read(self):
- super(VMSDFieldNull, self).read()
- assert(self.data == self.NULL_PTR_MARKER)
+ super(VMSDFieldPtrMarker, self).read()
+ assert(self.data in [self.NULL_PTR_MARKER, self.VALID_PTR_MARKER])
return self.data
class VMSDFieldBool(VMSDFieldGeneric):
@@ -642,7 +642,9 @@ def getDict(self):
"bitmap" : VMSDFieldGeneric,
"struct" : VMSDFieldStruct,
"capability": VMSDFieldCap,
- "nullptr": VMSDFieldNull,
+ # Keep the old nullptr for old binaries
+ "nullptr": VMSDFieldPtrMarker,
+ "ptr-marker": VMSDFieldPtrMarker,
"unknown" : VMSDFieldGeneric,
}
--
2.50.1
--
Peter Xu
Peter Xu <peterx@redhat.com> writes:
> On Thu, Mar 19, 2026 at 05:46:36PM -0300, Fabiano Rosas wrote:
>> Peter Xu <peterx@redhat.com> writes:
>>
>> > We used to have one vmstate called "nullptr" which is only used to generate
>> > one-byte hint to say one pointer is NULL.
>> >
>> > Let's extend its use so that it will generate another byte to say the
>> > pointer is non-NULL.
>> >
>> > With that, the name of the info struct (or functions) do not apply anymore.
>> > Update correspondingly.
>> >
>> > No functional change intended yet.
>> >
>> > Signed-off-by: Peter Xu <peterx@redhat.com>
>> > ---
>> > include/migration/vmstate.h | 9 +++++++--
>> > migration/vmstate-types.c | 34 ++++++++++++++++------------------
>> > migration/vmstate.c | 29 ++++++++++++++---------------
>> > 3 files changed, 37 insertions(+), 35 deletions(-)
>> >
>> > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
>> > index 092e8f7e9a..2e51b5ea04 100644
>> > --- a/include/migration/vmstate.h
>> > +++ b/include/migration/vmstate.h
>> > @@ -282,9 +282,14 @@ extern const VMStateInfo vmstate_info_uint32;
>> > extern const VMStateInfo vmstate_info_uint64;
>> > extern const VMStateInfo vmstate_info_fd;
>> >
>> > -/** Put this in the stream when migrating a null pointer.*/
>> > +/*
>> > + * Put this in the stream when migrating a pointer to reflect either a NULL
>> > + * or valid pointer.
>> > + */
>> > #define VMS_MARKER_PTR_NULL (0x30U) /* '0' */
>> > -extern const VMStateInfo vmstate_info_nullptr;
>> > +#define VMS_MARKER_PTR_VALID (0x31U) /* '1' */
>> > +
>> > +extern const VMStateInfo vmstate_info_ptr_marker;
>> >
>> > extern const VMStateInfo vmstate_info_cpudouble;
>> >
>> > diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
>> > index 7622cf8f01..b31689fc3c 100644
>> > --- a/migration/vmstate-types.c
>> > +++ b/migration/vmstate-types.c
>> > @@ -359,36 +359,34 @@ const VMStateInfo vmstate_info_fd = {
>> > .save = save_fd,
>> > };
>> >
>> > -static bool load_nullptr(QEMUFile *f, void *pv, size_t size,
>> > - const VMStateField *field, Error **errp)
>> > +static bool load_ptr_marker(QEMUFile *f, void *pv, size_t size,
>> > + const VMStateField *field, Error **errp)
>> >
>> > {
>> > - if (qemu_get_byte(f) == VMS_MARKER_PTR_NULL) {
>> > + int byte = qemu_get_byte(f);
>> > +
>> > + if (byte == VMS_MARKER_PTR_NULL || byte == VMS_MARKER_PTR_VALID) {
>> > + /* TODO: process PTR_VALID case */
>> > return true;
>> > }
>> >
>> > - error_setg(errp, "vmstate: load_nullptr expected VMS_NULLPTR_MARKER");
>> > + error_setg(errp, "%s: unexpected ptr marker: %d", __func__, byte);
>> > return false;
>> > }
>> >
>> > -static bool save_nullptr(QEMUFile *f, void *pv, size_t size,
>> > - const VMStateField *field, JSONWriter *vmdesc,
>> > - Error **errp)
>> > +static bool save_ptr_marker(QEMUFile *f, void *pv, size_t size,
>> > + const VMStateField *field, JSONWriter *vmdesc,
>> > + Error **errp)
>> >
>> > {
>> > - if (pv == NULL) {
>> > - qemu_put_byte(f, VMS_MARKER_PTR_NULL);
>> > - return true;
>> > - }
>> > -
>> > - error_setg(errp, "vmstate: save_nullptr must be called with pv == NULL");
>> > - return false;
>> > + qemu_put_byte(f, pv ? VMS_MARKER_PTR_VALID : VMS_MARKER_PTR_NULL);
>> > + return true;
>> > }
>> >
>> > -const VMStateInfo vmstate_info_nullptr = {
>> > - .name = "nullptr",
>> > - .load = load_nullptr,
>> > - .save = save_nullptr,
>> > +const VMStateInfo vmstate_info_ptr_marker = {
>> > + .name = "ptr-marker",
>> > + .load = load_ptr_marker,
>> > + .save = save_ptr_marker,
>> > };
>> >
>> > /* 64 bit unsigned int. See that the received value is the same than the one
>> > diff --git a/migration/vmstate.c b/migration/vmstate.c
>> > index 5a6b352764..a3a5f25946 100644
>> > --- a/migration/vmstate.c
>> > +++ b/migration/vmstate.c
>> > @@ -55,12 +55,12 @@ vmstate_field_exists(const VMStateDescription *vmsd, const VMStateField *field,
>> > }
>> >
>> > /*
>> > - * Create a fake nullptr field when there's a NULL pointer detected in the
>> > + * Create a ptr marker field when there's a NULL pointer detected in the
>> > * array of a VMS_ARRAY_OF_POINTER VMSD field. It's needed because we
>> > * can't dereference the NULL pointer.
>> > */
>> > static const VMStateField *
>> > -vmsd_create_fake_nullptr_field(const VMStateField *field)
>> > +vmsd_create_ptr_marker_field(const VMStateField *field)
>> > {
>> > VMStateField *fake = g_new0(VMStateField, 1);
>> >
>> > @@ -76,7 +76,7 @@ vmsd_create_fake_nullptr_field(const VMStateField *field)
>> >
>> > /* See vmstate_info_nullptr - use 1 byte to represent nullptr */
>>
>> This comment needs updating.
>
> Fixed.
>
>>
>> > fake->size = 1;
>> > - fake->info = &vmstate_info_nullptr;
>> > + fake->info = &vmstate_info_ptr_marker;
>> > fake->flags = VMS_SINGLE;
>> >
>> > /* All the rest fields shouldn't matter.. */
>> > @@ -278,7 +278,7 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
>> > * an array of pointers), use null placeholder and do
>> > * not follow.
>> > */
>> > - inner_field = vmsd_create_fake_nullptr_field(field);
>> > + inner_field = vmsd_create_ptr_marker_field(field);
>> > } else {
>> > inner_field = field;
>> > }
>> > @@ -567,7 +567,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
>> > int i, n_elems = vmstate_n_elems(opaque, field);
>> > int size = vmstate_size(opaque, field);
>> > JSONWriter *vmdesc_loop = vmdesc;
>> > - bool is_prev_null = false;
>> > + bool use_marker_field_prev = false;
>>
>> The logic below won't handle valid pointer as well, will it? We could
>> leave the is_null/is_prev_null terminology because it's way more
>> descriptive. Right? We're adding the marker here because we're
>> compressing and know the field is null.
>
> I recovered the old bool for better readability.
>
> One other note is I'll squash below analyze-migration.py change into this
> patch when repost, let me know if there's early comments:
>
> ===8<===
>
> From 82ac38a85ecd990a1aaa7b45b172e71f0d5b84da Mon Sep 17 00:00:00 2001
> From: Peter Xu <peterx@redhat.com>
> Date: Thu, 26 Mar 2026 14:57:56 -0400
> Subject: [PATCH] scripts/analyze-migration.py: Support new ptr-marker field
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> scripts/analyze-migration.py | 22 ++++++++++++----------
> 1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py
> index e81deab8f9..1771ff781b 100755
> --- a/scripts/analyze-migration.py
> +++ b/scripts/analyze-migration.py
> @@ -469,26 +469,26 @@ def __init__(self, desc, file):
> super(VMSDFieldIntLE, self).__init__(desc, file)
> self.dtype = '<i%d' % self.size
>
> -class VMSDFieldNull(VMSDFieldGeneric):
> +class VMSDFieldPtrMarker(VMSDFieldGeneric):
> NULL_PTR_MARKER = b'0'
> + VALID_PTR_MARKER = b'1'
>
> def __init__(self, desc, file):
> - super(VMSDFieldNull, self).__init__(desc, file)
> + super(VMSDFieldPtrMarker, self).__init__(desc, file)
>
> def __repr__(self):
> - # A NULL pointer is encoded in the stream as a '0' to
> - # disambiguate from a mere 0x0 value and avoid consumers
> - # trying to follow the NULL pointer. Displaying '0', 0x30 or
> - # 0x0 when analyzing the JSON debug stream could become
> + # A NULL / non-NULL pointer may be encoded in the stream as a
> + # '0'/'1' to represent the status of the pointer. Displaying '0',
> + # 0x30 or 0x0 when analyzing the JSON debug stream could become
> # confusing, so use an explicit term instead.
> - return "nullptr"
> + return "null-ptr" if self.data == self.NULL_PTR_MARKER else "valid-ptr"
>
> def __str__(self):
> return self.__repr__()
>
> def read(self):
> - super(VMSDFieldNull, self).read()
> - assert(self.data == self.NULL_PTR_MARKER)
> + super(VMSDFieldPtrMarker, self).read()
> + assert(self.data in [self.NULL_PTR_MARKER, self.VALID_PTR_MARKER])
> return self.data
>
> class VMSDFieldBool(VMSDFieldGeneric):
> @@ -642,7 +642,9 @@ def getDict(self):
> "bitmap" : VMSDFieldGeneric,
> "struct" : VMSDFieldStruct,
> "capability": VMSDFieldCap,
> - "nullptr": VMSDFieldNull,
> + # Keep the old nullptr for old binaries
> + "nullptr": VMSDFieldPtrMarker,
> + "ptr-marker": VMSDFieldPtrMarker,
> "unknown" : VMSDFieldGeneric,
> }
>
LGTM
Am Mi., 18. März 2026 um 00:23 Uhr schrieb Peter Xu <peterx@redhat.com>:
>
> We used to have one vmstate called "nullptr" which is only used to generate
> one-byte hint to say one pointer is NULL.
>
> Let's extend its use so that it will generate another byte to say the
> pointer is non-NULL.
>
> With that, the name of the info struct (or functions) do not apply anymore.
> Update correspondingly.
>
> No functional change intended yet.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
> ---
> include/migration/vmstate.h | 9 +++++++--
> migration/vmstate-types.c | 34 ++++++++++++++++------------------
> migration/vmstate.c | 29 ++++++++++++++---------------
> 3 files changed, 37 insertions(+), 35 deletions(-)
>
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index 092e8f7e9a..2e51b5ea04 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -282,9 +282,14 @@ extern const VMStateInfo vmstate_info_uint32;
> extern const VMStateInfo vmstate_info_uint64;
> extern const VMStateInfo vmstate_info_fd;
>
> -/** Put this in the stream when migrating a null pointer.*/
> +/*
> + * Put this in the stream when migrating a pointer to reflect either a NULL
> + * or valid pointer.
> + */
> #define VMS_MARKER_PTR_NULL (0x30U) /* '0' */
> -extern const VMStateInfo vmstate_info_nullptr;
> +#define VMS_MARKER_PTR_VALID (0x31U) /* '1' */
> +
> +extern const VMStateInfo vmstate_info_ptr_marker;
>
> extern const VMStateInfo vmstate_info_cpudouble;
>
> diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
> index 7622cf8f01..b31689fc3c 100644
> --- a/migration/vmstate-types.c
> +++ b/migration/vmstate-types.c
> @@ -359,36 +359,34 @@ const VMStateInfo vmstate_info_fd = {
> .save = save_fd,
> };
>
> -static bool load_nullptr(QEMUFile *f, void *pv, size_t size,
> - const VMStateField *field, Error **errp)
> +static bool load_ptr_marker(QEMUFile *f, void *pv, size_t size,
> + const VMStateField *field, Error **errp)
>
> {
> - if (qemu_get_byte(f) == VMS_MARKER_PTR_NULL) {
> + int byte = qemu_get_byte(f);
> +
> + if (byte == VMS_MARKER_PTR_NULL || byte == VMS_MARKER_PTR_VALID) {
> + /* TODO: process PTR_VALID case */
> return true;
> }
>
> - error_setg(errp, "vmstate: load_nullptr expected VMS_NULLPTR_MARKER");
> + error_setg(errp, "%s: unexpected ptr marker: %d", __func__, byte);
> return false;
> }
>
> -static bool save_nullptr(QEMUFile *f, void *pv, size_t size,
> - const VMStateField *field, JSONWriter *vmdesc,
> - Error **errp)
> +static bool save_ptr_marker(QEMUFile *f, void *pv, size_t size,
> + const VMStateField *field, JSONWriter *vmdesc,
> + Error **errp)
>
> {
> - if (pv == NULL) {
> - qemu_put_byte(f, VMS_MARKER_PTR_NULL);
> - return true;
> - }
> -
> - error_setg(errp, "vmstate: save_nullptr must be called with pv == NULL");
> - return false;
> + qemu_put_byte(f, pv ? VMS_MARKER_PTR_VALID : VMS_MARKER_PTR_NULL);
> + return true;
> }
>
> -const VMStateInfo vmstate_info_nullptr = {
> - .name = "nullptr",
> - .load = load_nullptr,
> - .save = save_nullptr,
> +const VMStateInfo vmstate_info_ptr_marker = {
> + .name = "ptr-marker",
> + .load = load_ptr_marker,
> + .save = save_ptr_marker,
> };
>
> /* 64 bit unsigned int. See that the received value is the same than the one
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index 5a6b352764..a3a5f25946 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -55,12 +55,12 @@ vmstate_field_exists(const VMStateDescription *vmsd, const VMStateField *field,
> }
>
> /*
> - * Create a fake nullptr field when there's a NULL pointer detected in the
> + * Create a ptr marker field when there's a NULL pointer detected in the
> * array of a VMS_ARRAY_OF_POINTER VMSD field. It's needed because we
> * can't dereference the NULL pointer.
> */
> static const VMStateField *
> -vmsd_create_fake_nullptr_field(const VMStateField *field)
> +vmsd_create_ptr_marker_field(const VMStateField *field)
> {
> VMStateField *fake = g_new0(VMStateField, 1);
>
> @@ -76,7 +76,7 @@ vmsd_create_fake_nullptr_field(const VMStateField *field)
>
> /* See vmstate_info_nullptr - use 1 byte to represent nullptr */
> fake->size = 1;
> - fake->info = &vmstate_info_nullptr;
> + fake->info = &vmstate_info_ptr_marker;
> fake->flags = VMS_SINGLE;
>
> /* All the rest fields shouldn't matter.. */
> @@ -278,7 +278,7 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
> * an array of pointers), use null placeholder and do
> * not follow.
> */
> - inner_field = vmsd_create_fake_nullptr_field(field);
> + inner_field = vmsd_create_ptr_marker_field(field);
> } else {
> inner_field = field;
> }
> @@ -567,7 +567,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> int i, n_elems = vmstate_n_elems(opaque, field);
> int size = vmstate_size(opaque, field);
> JSONWriter *vmdesc_loop = vmdesc;
> - bool is_prev_null = false;
> + bool use_marker_field_prev = false;
>
> trace_vmstate_save_state_loop(vmsd->name, field->name, n_elems);
> if (field->flags & VMS_POINTER) {
> @@ -578,7 +578,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> for (i = 0; i < n_elems; i++) {
> void *curr_elem = first_elem + size * i;
> const VMStateField *inner_field;
> - bool is_null;
> + bool use_marker_field;
> int max_elems = n_elems - i;
>
> if (field->flags & VMS_ARRAY_OF_POINTER) {
> @@ -586,17 +586,16 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> curr_elem = *(void **)curr_elem;
> }
>
> - if (!curr_elem && size) {
> + use_marker_field = !curr_elem && size;
> + if (use_marker_field) {
> /*
> * 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;
> + inner_field = vmsd_create_ptr_marker_field(field);
> } else {
> inner_field = field;
> - is_null = false;
> }
>
> /*
> @@ -612,16 +611,16 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> */
> if (vmdesc && vmsd_can_compress(field) &&
> (field->flags & VMS_ARRAY_OF_POINTER) &&
> - is_null != is_prev_null) {
> + use_marker_field != use_marker_field_prev) {
>
> - is_prev_null = is_null;
> + use_marker_field_prev = use_marker_field;
> vmdesc_loop = vmdesc;
>
> for (int j = i + 1; j < n_elems; j++) {
> void *elem = *(void **)(first_elem + size * j);
> - bool elem_is_null = !elem && size;
> + bool elem_use_marker_field = !elem && size;
>
> - if (is_null != elem_is_null) {
> + if (use_marker_field != elem_use_marker_field) {
> max_elems = j - i;
> break;
> }
> @@ -633,7 +632,7 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
> i, max_elems, errp);
>
> /* If we used a fake temp field.. free it now */
> - if (is_null) {
> + if (use_marker_field) {
> g_clear_pointer((gpointer *)&inner_field, g_free);
> }
>
> --
> 2.50.1
>
© 2016 - 2026 Red Hat, Inc.