Re: [PATCH v8 23/27] migration: Propagate last encountered error in vmstate_save_state_v() function

[Akihiko Odaki]

On 2025/07/31 22:21, Arun Menon wrote:
Currently post_save hook is called without checking its return. If post_save
fails, we need to set an error and propagate it to the caller.
Since post_save hook is called regardless of whether there is a preceeding 
error,
it is possible that we have 2 distict errors, one from the preceeding function
call, and the other from the post_save call.

Return the latest error to the caller.

This needs to be explained better. This patch makes two changes on the 
behavior when there are two errors:

1) Proceeding errors were propagated before, but they are now
   dismissed.
2) post_error() errors were dismissed before, but they are now
   propagated.

This message doesn't mention 1) at all. It does say 2) is necessary, but 
does not explain why.


Signed-off-by: Arun Menon <arme...@redhat.com>
---
  migration/vmstate.c | 18 ++++++++++++++----
  1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/migration/vmstate.c b/migration/vmstate.c
index 
b725202bfcf69c3c81338f1f5479aa2ddc5db86f..25a819da069b982d4043f287b4562ea402d9eb0e
 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -418,6 +418,7 @@ int vmstate_save_state_v(QEMUFile *f, const 
VMStateDescription *vmsd,
                           void *opaque, JSONWriter *vmdesc, int version_id, 
Error **errp)
  {
      int ret = 0;
+    int ps_ret = 0;
      const VMStateField *field = vmsd->fields;
  
      trace_vmstate_save_state_top(vmsd->name);
@@ -533,7 +534,14 @@ int vmstate_save_state_v(QEMUFile *f, const 
VMStateDescription *vmsd,
                      error_setg(errp, "Save of field %s/%s failed",
                                  vmsd->name, field->name);
                      if (vmsd->post_save) {
-                        vmsd->post_save(opaque);
+                        ps_ret = vmsd->post_save(opaque);
+                        if (ps_ret) {
+                            ret = ps_ret;
+                            error_free_or_abort(errp);
+                            error_setg(errp,
+                                       "post-save for %s failed, ret: '%d'",
+                                       vmsd->name, ret);
+                        }
                      }
                      return ret;
                  }
@@ -561,10 +569,12 @@ int vmstate_save_state_v(QEMUFile *f, const 
VMStateDescription *vmsd,
      ret = vmstate_subsection_save(f, vmsd, opaque, vmdesc, errp);
  
      if (vmsd->post_save) {
-        int ps_ret = vmsd->post_save(opaque);
-        if (!ret && ps_ret) {
+        ps_ret = vmsd->post_save(opaque);
+        if (ps_ret) {
              ret = ps_ret;
-            error_setg(errp, "post-save failed: %s", vmsd->name);
+            error_free_or_abort(errp);
+            error_setg(errp, "post-save for %s failed, ret: '%d'",
+                       vmsd->name, ret);
          }
      }
      return ret;