The loader side of ptr marker is pretty straightforward, instead of playing
the inner_field trick, just do the load manually assuming the marker layout
is a stable ABI (which it is true already).
This will remove some logic while loading VMSD, and hopefully it makes it
slightly easier to read. Unfortunately, we still need to keep the sender
side because of the JSON blob we're maintaining..
This paves way for future processing of non-NULL markers as well.
When at it, not check "size" anymore for existing NULL markers, and move it
under the same VMS_ARRAY_OF_POINTER section because that's the only place
that NULL marker can happen (which guarantess size==host ptr size, which is
non-zero).
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/vmstate-types.c | 12 ++++------
migration/vmstate.c | 46 ++++++++++++++++++++++++---------------
2 files changed, 32 insertions(+), 26 deletions(-)
diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
index b31689fc3c..ae465c5c2c 100644
--- a/migration/vmstate-types.c
+++ b/migration/vmstate-types.c
@@ -363,14 +363,10 @@ static bool load_ptr_marker(QEMUFile *f, void *pv, size_t size,
const VMStateField *field, Error **errp)
{
- 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, "%s: unexpected ptr marker: %d", __func__, byte);
+ /*
+ * Load is done in vmstate core, see vmstate_ptr_marker_load().
+ */
+ g_assert_not_reached();
return false;
}
diff --git a/migration/vmstate.c b/migration/vmstate.c
index b333aa1744..47812eb882 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -142,6 +142,21 @@ static void vmstate_handle_alloc(void *ptr, const VMStateField *field,
}
}
+static bool vmstate_ptr_marker_load(QEMUFile *f, bool *load_field,
+ Error **errp)
+{
+ int byte = qemu_get_byte(f);
+
+ if (byte == VMS_MARKER_PTR_NULL) {
+ /* When it's a null ptr marker, do not continue the load */
+ *load_field = false;
+ return true;
+ }
+
+ error_setg(errp, "Unexpected ptr marker: %d", byte);
+ return false;
+}
+
static bool vmstate_pre_load(const VMStateDescription *vmsd, void *opaque,
Error **errp)
{
@@ -264,30 +279,25 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
}
for (i = 0; i < n_elems; i++) {
- bool ok;
+ /* If we will process the load of field? */
+ bool load_field = true;
+ bool ok = true;
void *curr_elem = first_elem + size * i;
- const VMStateField *inner_field;
if (field->flags & VMS_ARRAY_OF_POINTER) {
curr_elem = *(void **)curr_elem;
+ if (!curr_elem) {
+ /* Read the marker instead of VMSD itself */
+ if (!vmstate_ptr_marker_load(f, &load_field, errp)) {
+ trace_vmstate_load_field_error(field->name,
+ -EINVAL);
+ return false;
+ }
+ }
}
- 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_ptr_marker_field(field);
- } else {
- inner_field = field;
- }
-
- ok = vmstate_load_field(f, curr_elem, size, inner_field, errp);
-
- /* If we used a fake temp field.. free it now */
- if (inner_field != field) {
- g_clear_pointer((gpointer *)&inner_field, g_free);
+ if (load_field) {
+ ok = vmstate_load_field(f, curr_elem, size, field, errp);
}
if (ok) {
--
2.50.1Am Do., 26. März 2026 um 22:05 Uhr schrieb Peter Xu <peterx@redhat.com>:
>
> The loader side of ptr marker is pretty straightforward, instead of playing
> the inner_field trick, just do the load manually assuming the marker layout
> is a stable ABI (which it is true already).
>
> This will remove some logic while loading VMSD, and hopefully it makes it
> slightly easier to read. Unfortunately, we still need to keep the sender
> side because of the JSON blob we're maintaining..
>
> This paves way for future processing of non-NULL markers as well.
>
> When at it, not check "size" anymore for existing NULL markers, and move it
> under the same VMS_ARRAY_OF_POINTER section because that's the only place
> that NULL marker can happen (which guarantess size==host ptr size, which is
> non-zero).
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
> ---
> migration/vmstate-types.c | 12 ++++------
> migration/vmstate.c | 46 ++++++++++++++++++++++++---------------
> 2 files changed, 32 insertions(+), 26 deletions(-)
>
> diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
> index b31689fc3c..ae465c5c2c 100644
> --- a/migration/vmstate-types.c
> +++ b/migration/vmstate-types.c
> @@ -363,14 +363,10 @@ static bool load_ptr_marker(QEMUFile *f, void *pv, size_t size,
> const VMStateField *field, Error **errp)
>
> {
> - 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, "%s: unexpected ptr marker: %d", __func__, byte);
> + /*
> + * Load is done in vmstate core, see vmstate_ptr_marker_load().
> + */
> + g_assert_not_reached();
> return false;
> }
>
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index b333aa1744..47812eb882 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -142,6 +142,21 @@ static void vmstate_handle_alloc(void *ptr, const VMStateField *field,
> }
> }
>
> +static bool vmstate_ptr_marker_load(QEMUFile *f, bool *load_field,
> + Error **errp)
> +{
> + int byte = qemu_get_byte(f);
> +
> + if (byte == VMS_MARKER_PTR_NULL) {
> + /* When it's a null ptr marker, do not continue the load */
> + *load_field = false;
> + return true;
> + }
> +
> + error_setg(errp, "Unexpected ptr marker: %d", byte);
> + return false;
> +}
> +
> static bool vmstate_pre_load(const VMStateDescription *vmsd, void *opaque,
> Error **errp)
> {
> @@ -264,30 +279,25 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
> }
>
> for (i = 0; i < n_elems; i++) {
> - bool ok;
> + /* If we will process the load of field? */
> + bool load_field = true;
> + bool ok = true;
> void *curr_elem = first_elem + size * i;
> - const VMStateField *inner_field;
>
> if (field->flags & VMS_ARRAY_OF_POINTER) {
> curr_elem = *(void **)curr_elem;
> + if (!curr_elem) {
> + /* Read the marker instead of VMSD itself */
> + if (!vmstate_ptr_marker_load(f, &load_field, errp)) {
> + trace_vmstate_load_field_error(field->name,
> + -EINVAL);
> + return false;
> + }
> + }
> }
>
> - 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_ptr_marker_field(field);
> - } else {
> - inner_field = field;
> - }
> -
> - ok = vmstate_load_field(f, curr_elem, size, inner_field, errp);
> -
> - /* If we used a fake temp field.. free it now */
> - if (inner_field != field) {
> - g_clear_pointer((gpointer *)&inner_field, g_free);
> + if (load_field) {
> + ok = vmstate_load_field(f, curr_elem, size, field, errp);
> }
>
> if (ok) {
> --
> 2.50.1
>
Peter Xu <peterx@redhat.com> writes:
> The loader side of ptr marker is pretty straightforward, instead of playing
> the inner_field trick, just do the load manually assuming the marker layout
> is a stable ABI (which it is true already).
>
> This will remove some logic while loading VMSD, and hopefully it makes it
> slightly easier to read. Unfortunately, we still need to keep the sender
> side because of the JSON blob we're maintaining..
>
> This paves way for future processing of non-NULL markers as well.
>
> When at it, not check "size" anymore for existing NULL markers, and move it
> under the same VMS_ARRAY_OF_POINTER section because that's the only place
> that NULL marker can happen (which guarantess size==host ptr size, which is
> non-zero).
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> migration/vmstate-types.c | 12 ++++------
> migration/vmstate.c | 46 ++++++++++++++++++++++++---------------
> 2 files changed, 32 insertions(+), 26 deletions(-)
>
> diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
> index b31689fc3c..ae465c5c2c 100644
> --- a/migration/vmstate-types.c
> +++ b/migration/vmstate-types.c
> @@ -363,14 +363,10 @@ static bool load_ptr_marker(QEMUFile *f, void *pv, size_t size,
> const VMStateField *field, Error **errp)
>
> {
> - 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, "%s: unexpected ptr marker: %d", __func__, byte);
> + /*
> + * Load is done in vmstate core, see vmstate_ptr_marker_load().
> + */
> + g_assert_not_reached();
> return false;
> }
>
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index b333aa1744..47812eb882 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -142,6 +142,21 @@ static void vmstate_handle_alloc(void *ptr, const VMStateField *field,
> }
> }
>
> +static bool vmstate_ptr_marker_load(QEMUFile *f, bool *load_field,
> + Error **errp)
> +{
> + int byte = qemu_get_byte(f);
> +
> + if (byte == VMS_MARKER_PTR_NULL) {
> + /* When it's a null ptr marker, do not continue the load */
> + *load_field = false;
> + return true;
> + }
> +
> + error_setg(errp, "Unexpected ptr marker: %d", byte);
> + return false;
> +}
> +
> static bool vmstate_pre_load(const VMStateDescription *vmsd, void *opaque,
> Error **errp)
> {
> @@ -264,30 +279,25 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
> }
>
> for (i = 0; i < n_elems; i++) {
> - bool ok;
> + /* If we will process the load of field? */
> + bool load_field = true;
> + bool ok = true;
> void *curr_elem = first_elem + size * i;
> - const VMStateField *inner_field;
>
> if (field->flags & VMS_ARRAY_OF_POINTER) {
> curr_elem = *(void **)curr_elem;
> + if (!curr_elem) {
> + /* Read the marker instead of VMSD itself */
> + if (!vmstate_ptr_marker_load(f, &load_field, errp)) {
> + trace_vmstate_load_field_error(field->name,
> + -EINVAL);
> + return false;
> + }
> + }
> }
>
> - 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_ptr_marker_field(field);
> - } else {
> - inner_field = field;
> - }
> -
> - ok = vmstate_load_field(f, curr_elem, size, inner_field, errp);
> -
> - /* If we used a fake temp field.. free it now */
> - if (inner_field != field) {
> - g_clear_pointer((gpointer *)&inner_field, g_free);
> + if (load_field) {
> + ok = vmstate_load_field(f, curr_elem, size, field, errp);
> }
>
> if (ok) {
Reviewed-by: Fabiano Rosas <farosas@suse.de>
© 2016 - 2026 Red Hat, Inc.