[PATCH v3 05/18] migration: vmstate_save/load_state(): refactor tracing errors

Vladimir Sementsov-Ogievskiy posted 18 patches 3 weeks, 1 day ago
Maintainers: Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Pierrick Bouvier <pierrick.bouvier@linaro.org>, John Snow <jsnow@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, "Michael S. Tsirkin" <mst@redhat.com>, Dmitry Fleytman <dmitry.fleytman@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Nicholas Piggin <npiggin@gmail.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Gautam Menghani <gautam@linux.ibm.com>, Glenn Miles <milesg@linux.ibm.com>, Stefan Weil <sw@weilnetz.de>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Farhan Ali <alifm@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Ilya Leoshkevich <iii@linux.ibm.com>, David Hildenbrand <david@kernel.org>, Thomas Huth <thuth@redhat.com>, Mark Kanda <mark.kanda@oracle.com>, Ben Chaney <bchaney@akamai.com>, Peter Maydell <peter.maydell@linaro.org>, Chinmay Rath <rathc@linux.ibm.com>
[PATCH v3 05/18] migration: vmstate_save/load_state(): refactor tracing errors
Posted by Vladimir Sementsov-Ogievskiy 3 weeks, 1 day ago
To simplify further changes (convertion to bool+errp APIs),
let's rework some error paths:

- get rid of int ret in traces, as we are moving to bool+errp APIs
- split traces to _fail / _success (seems better than add boolean
  result to the message).
- prefer short error paths (return immediately on error)
- around trace_vmstate_load_field_error(), do not call
  qemu_file_set_error(), if the erroc comes from qemu_file_get_error()

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 migration/trace-events |  6 ++++--
 migration/vmstate.c    | 25 +++++++++++++++----------
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/migration/trace-events b/migration/trace-events
index 90629f828f8..0b3f759ccc4 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -57,13 +57,15 @@ postcopy_page_req_sync(void *host_addr) "sync page req %p"
 # vmstate.c
 vmstate_load_field_error(const char *field, int ret) "field \"%s\" load failed, ret = %d"
 vmstate_load_state(const char *name, int version_id) "%s v%d"
-vmstate_load_state_end(const char *name, const char *reason, int val) "%s %s/%d"
+vmstate_load_state_fail(const char *name, const char *reason) "%s %s"
+vmstate_load_state_success(const char *name) "%s"
 vmstate_load_state_field(const char *name, const char *field, bool exists) "%s:%s exists=%d"
 vmstate_n_elems(const char *name, int n_elems) "%s: %d"
 vmstate_subsection_load(const char *parent) "%s"
 vmstate_subsection_load_bad(const char *parent,  const char *sub, const char *sub2) "%s: %s/%s"
 vmstate_subsection_load_good(const char *parent) "%s"
-vmstate_save_state_pre_save_res(const char *name, int res) "%s/%d"
+vmstate_save_state_pre_save_fail(const char *name) "%s"
+vmstate_save_state_pre_save_success(const char *name) "%s"
 vmstate_save_state_loop(const char *name, const char *field, int n_elems) "%s/%s[%d]"
 vmstate_save_state_top(const char *idstr) "%s"
 vmstate_subsection_save_loop(const char *name, const char *sub) "%s/%s"
diff --git a/migration/vmstate.c b/migration/vmstate.c
index dd7cd279937..87e1f049592 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -144,7 +144,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
         error_setg(errp, "%s: incoming version_id %d is too new "
                    "for local version_id %d",
                    vmsd->name, version_id, vmsd->version_id);
-        trace_vmstate_load_state_end(vmsd->name, "too new", -EINVAL);
+        trace_vmstate_load_state_fail(vmsd->name, "too new");
         return -EINVAL;
     }
 
@@ -152,7 +152,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
         error_setg(errp, "%s: incoming version_id %d is too old "
                    "for local minimum version_id %d",
                    vmsd->name, version_id, vmsd->minimum_version_id);
-        trace_vmstate_load_state_end(vmsd->name, "too old", -EINVAL);
+        trace_vmstate_load_state_fail(vmsd->name, "too old");
         return -EINVAL;
     }
 
@@ -240,10 +240,10 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                         error_setg(errp,
                                    "Failed to load %s state: stream error: %d",
                                    vmsd->name, ret);
+                        trace_vmstate_load_field_error(field->name, ret);
+                        return ret;
                     }
-                }
-
-                if (ret < 0) {
+                } else {
                     qemu_file_set_error(f, ret);
                     trace_vmstate_load_field_error(field->name, ret);
                     return ret;
@@ -269,7 +269,8 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
             error_prepend(errp, "post load hook failed for: %s, version_id: "
                           "%d, minimum_version: %d: ", vmsd->name,
                           vmsd->version_id, vmsd->minimum_version_id);
-            ret = -EINVAL;
+            trace_vmstate_load_state_fail(vmsd->name, "post-load");
+            return -EINVAL;
         }
     } else if (vmsd->post_load) {
         ret = vmsd->post_load(opaque, version_id);
@@ -279,12 +280,14 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                        "minimum_version: %d, ret: %d",
                        vmsd->name, vmsd->version_id, vmsd->minimum_version_id,
                        ret);
+            trace_vmstate_load_state_fail(vmsd->name, "post-load");
+            return ret;
         }
     }
 
-    trace_vmstate_load_state_end(vmsd->name, "end", ret);
+    trace_vmstate_load_state_success(vmsd->name);
 
-    return ret;
+    return 0;
 }
 
 static int vmfield_name_num(const VMStateField *start,
@@ -444,20 +447,22 @@ static int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
 
     if (vmsd->pre_save_errp) {
         ret = vmsd->pre_save_errp(opaque, errp) ? 0 : -EINVAL;
-        trace_vmstate_save_state_pre_save_res(vmsd->name, ret);
         if (ret < 0) {
             error_prepend(errp, "pre-save for %s failed: ", vmsd->name);
+            trace_vmstate_save_state_pre_save_fail(vmsd->name);
             return ret;
         }
     } else if (vmsd->pre_save) {
         ret = vmsd->pre_save(opaque);
-        trace_vmstate_save_state_pre_save_res(vmsd->name, ret);
         if (ret) {
             error_setg(errp, "pre-save failed: %s", vmsd->name);
+            trace_vmstate_save_state_pre_save_fail(vmsd->name);
             return ret;
         }
     }
 
+    trace_vmstate_save_state_pre_save_success(vmsd->name);
+
     if (vmdesc) {
         json_writer_str(vmdesc, "vmsd_name", vmsd->name);
         json_writer_int64(vmdesc, "version", version_id);
-- 
2.52.0
Re: [PATCH v3 05/18] migration: vmstate_save/load_state(): refactor tracing errors
Posted by Peter Xu 3 weeks ago
On Thu, Mar 05, 2026 at 12:22:49AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> To simplify further changes (convertion to bool+errp APIs),
> let's rework some error paths:
> 
> - get rid of int ret in traces, as we are moving to bool+errp APIs
> - split traces to _fail / _success (seems better than add boolean
>   result to the message).
> - prefer short error paths (return immediately on error)
> - around trace_vmstate_load_field_error(), do not call
>   qemu_file_set_error(), if the erroc comes from qemu_file_get_error()
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu