[RFC PATCH v1 14/17] vmstate: Move VMS_MUST_EXIST check

Fabiano Rosas posted 17 patches 1 week, 2 days ago
[RFC PATCH v1 14/17] vmstate: Move VMS_MUST_EXIST check
Posted by Fabiano Rosas 1 week, 2 days ago
Move the VMS_MUST_EXIST check into the vmstate_field_exists() function
and make it return bool + errp. This deduplicates a bit of code
between save and load.

XXX: why do we assert on save?

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 migration/vmstate.c | 48 ++++++++++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/migration/vmstate.c b/migration/vmstate.c
index 8f0f9383e2..5bc860129e 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -30,10 +30,10 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
                                 void *opaque, JSONWriter *vmdesc,
                                 int version_id, Error **errp);
 
-/* Whether this field should exist for either save or load the VM? */
-static bool
-vmstate_field_exists(const VMStateDescription *vmsd, const VMStateField *field,
-                     void *opaque, int version_id)
+static bool vmstate_field_exists(const VMStateDescription *vmsd,
+                                 const VMStateField *field,
+                                 void *opaque, int version_id,
+                                 bool *exists, Error **errp)
 {
     bool result;
 
@@ -51,7 +51,16 @@ vmstate_field_exists(const VMStateDescription *vmsd, const VMStateField *field,
         result = field->version_id <= version_id;
     }
 
-    return result;
+    *exists = result;
+
+    if (!result && field->flags & VMS_MUST_EXIST) {
+        error_setg(errp, "Expected field to exist, but it doesn't: "
+                   "%s/%s version_id: %d",
+                   vmsd->name, field->name, vmsd->version_id);
+        return false;
+    }
+
+    return true;
 }
 
 static int vmstate_n_elems(void *opaque, const VMStateField *field)
@@ -257,6 +266,7 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
 {
     ERRP_GUARD();
     const VMStateField *field = vmsd->fields;
+    bool ok = true;
 
     trace_vmstate_load_state(vmsd->name, version_id);
 
@@ -281,7 +291,13 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
     }
 
     while (field->name) {
-        bool exists = vmstate_field_exists(vmsd, field, opaque, version_id);
+        bool exists;
+
+        ok = vmstate_field_exists(vmsd, field, opaque, version_id, &exists,
+                                  errp);
+        if (!ok) {
+            return false;
+        }
 
         trace_vmstate_load_state_field(vmsd->name, field->name, exists);
 
@@ -296,7 +312,6 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
             for (i = 0; i < n_elems; i++) {
                 /* If we will process the load of field? */
                 bool load_field = true;
-                bool ok = true;
                 void *curr_elem;
 
                 if (field->flags & VMS_ARRAY_OF_POINTER) {
@@ -326,10 +341,6 @@ bool vmstate_load_vmsd(QEMUFile *f, const VMStateDescription *vmsd,
                     return false;
                 }
             }
-        } else if (field->flags & VMS_MUST_EXIST) {
-            error_setg(errp, "Input validation failed: %s/%s version_id: %d",
-                       vmsd->name, field->name, vmsd->version_id);
-            return false;
         }
         field++;
     }
@@ -635,7 +646,14 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
     }
 
     while (field->name) {
-        if (vmstate_field_exists(vmsd, field, opaque, version_id)) {
+        bool exists;
+
+        ok = vmstate_field_exists(vmsd, field, opaque, version_id, &exists,
+                                  errp);
+        if (!ok) {
+            g_assert_not_reached();
+        }
+        if (exists) {
             void *head;
             int i, n_elems = vmstate_n_elems(opaque, field);
             int size = vmstate_size(opaque, field);
@@ -725,12 +743,6 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const VMStateDescription *vmsd,
 
                 use_vmdesc = false;
             }
-        } else {
-            if (field->flags & VMS_MUST_EXIST) {
-                error_report("Output state validation failed: %s/%s",
-                        vmsd->name, field->name);
-                assert(!(field->flags & VMS_MUST_EXIST));
-            }
         }
         field++;
     }
-- 
2.51.0
Re: [RFC PATCH v1 14/17] vmstate: Move VMS_MUST_EXIST check
Posted by Peter Xu 1 week, 1 day ago
On Tue, Mar 24, 2026 at 04:43:29PM -0300, Fabiano Rosas wrote:
> Move the VMS_MUST_EXIST check into the vmstate_field_exists() function
> and make it return bool + errp. This deduplicates a bit of code
> between save and load.
> 
> XXX: why do we assert on save?

I don't see a good reason to do that.

OTOH, I have a todo to remove VMS_MUST_EXIST, it can always be done by
pre_save() or post_load() hooks, afaict..

-- 
Peter Xu