On 2025/08/06 3:25, Arun Menon wrote:
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that qemu_loadvm_state_main() must report an error
in errp, in case of failure.
loadvm_process_command also sets the errp object explicitly.

Reviewed-by: Daniel P. Berrangé <berra...@redhat.com>
Signed-off-by: Arun Menon <arme...@redhat.com>
---
  migration/colo.c   |  5 +++--
  migration/savevm.c | 33 ++++++++++++++++++++-------------
  migration/savevm.h |  3 ++-
  3 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/migration/colo.c b/migration/colo.c
index 
0ba22ee76a13e313793f653f43a728e3c433bbc1..a96e4dba15516b71d1b315c736c3b4879ff04e58
 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -659,6 +659,7 @@ void migrate_start_colo_process(MigrationState *s)
  static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
                        QEMUFile *fb, QIOChannelBuffer *bioc, Error **errp)
  {
+    ERRP_GUARD();
      uint64_t total_size;
      uint64_t value;
      Error *local_err = NULL;
@@ -686,11 +687,11 @@ static void 
colo_incoming_process_checkpoint(MigrationIncomingState *mis,
bql_lock();
      cpu_synchronize_all_states();
-    ret = qemu_loadvm_state_main(mis->from_src_file, mis);
+    ret = qemu_loadvm_state_main(mis->from_src_file, mis, errp);
      bql_unlock();
if (ret < 0) {
-        error_setg(errp, "Load VM's live state (ram) error");
+        error_prepend(errp, "Load VM's live state (ram) error: ");
          return;
      }
diff --git a/migration/savevm.c b/migration/savevm.c
index 
f3b91c8ae0eee6078406081f0bd7f686fed28601..ad3dd9b172afc541f104d2187a79bafa8e380466
 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2105,7 +2105,7 @@ static void *postcopy_ram_listen_thread(void *opaque)
      qemu_file_set_blocking(f, true);
/* TODO: sanity check that only postcopiable data will be loaded here */
-    load_res = qemu_loadvm_state_main(f, mis);
+    load_res = qemu_loadvm_state_main(f, mis, NULL);
/*
       * This is tricky, but, mis->from_src_file can change after it
@@ -2407,6 +2407,7 @@ static int 
loadvm_postcopy_handle_resume(MigrationIncomingState *mis)
   */
  static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis, Error 
**errp)
  {
+    ERRP_GUARD();
      int ret;
      size_t length;
      QIOChannelBuffer *bioc;
@@ -2456,9 +2457,9 @@ static int 
loadvm_handle_cmd_packaged(MigrationIncomingState *mis, Error **errp)
          qemu_coroutine_yield();
      } while (1);
- ret = qemu_loadvm_state_main(packf, mis);
+    ret = qemu_loadvm_state_main(packf, mis, errp);
      if (ret < 0) {
-        error_setg(errp, "VM state load failed: %d", ret);
+        error_prepend(errp, "Loading VM state failed: %d: ", ret);
      }
      trace_loadvm_handle_cmd_packaged_main(ret);
      qemu_fclose(packf);
@@ -3074,8 +3075,10 @@ static bool 
postcopy_pause_incoming(MigrationIncomingState *mis)
      return true;
  }
-int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
+int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis,
+                           Error **errp)
  {
+    ERRP_GUARD();
      uint8_t section_type;
      int ret = 0;
@@ -3083,8 +3086,10 @@ retry:
      while (true) {
          section_type = qemu_get_byte(f);
- ret = qemu_file_get_error_obj_any(f, mis->postcopy_qemufile_dst, NULL);
+        ret = qemu_file_get_error_obj_any(f, mis->postcopy_qemufile_dst, errp);
          if (ret) {
+            error_prepend(errp, "Failed to load device state section ID: %d: ",
+                          ret);
              break;
          }
@@ -3105,7 +3110,7 @@ retry:
              }
              break;
          case QEMU_VM_COMMAND:
-            ret = loadvm_process_command(f, NULL);
+            ret = loadvm_process_command(f, errp);
              trace_qemu_loadvm_state_section_command(ret);
              if ((ret < 0) || (ret == LOADVM_QUIT)) {
                  goto out;
@@ -3115,7 +3120,7 @@ retry:
              /* This is the end of migration */
              goto out;
          default:
-            error_report("Unknown savevm section type %d", section_type);
+            error_setg(errp, "Unknown savevm section type %d", section_type);
              ret = -EINVAL;
              goto out;
          }
@@ -3123,6 +3128,9 @@ retry:
out:
      if (ret < 0) {
+        if (*errp == NULL) {
+            error_setg(errp, "Loading VM state failed: %d", ret);
+        }
          qemu_file_set_error(f, ret);
/* Cancel bitmaps incoming regardless of recovery */
@@ -3143,6 +3151,8 @@ out:
              migrate_postcopy_ram() && postcopy_pause_incoming(mis)) {
              /* Reset f to point to the newly created channel */
              f = mis->from_src_file;
+            error_free(*errp);
+            *errp = NULL;

error_free_or_abort() is a convenient alternative that can save "*errp = NULL" and also catch a bug that called functions return a negative value without setting an error.

              goto retry;
          }
      }
@@ -3176,10 +3186,7 @@ int qemu_loadvm_state(QEMUFile *f, Error **errp)
cpu_synchronize_all_pre_loadvm(); - ret = qemu_loadvm_state_main(f, mis);
-    if (ret < 0) {
-        error_setg(errp, "Load VM state failed: %d", ret);
-    }
+    ret = qemu_loadvm_state_main(f, mis, errp);
      qemu_event_set(&mis->main_thread_load_event);
trace_qemu_loadvm_state_post_main(ret);
@@ -3260,9 +3267,9 @@ int qemu_load_device_state(QEMUFile *f, Error **errp)
      int ret;
/* Load QEMU_VM_SECTION_FULL section */
-    ret = qemu_loadvm_state_main(f, mis);
+    ret = qemu_loadvm_state_main(f, mis, errp);
      if (ret < 0) {
-        error_setg(errp, "Failed to load device state: %d", ret);
+        error_prepend(errp, "Failed to load device state: %d: ", ret);
          return ret;
      }
diff --git a/migration/savevm.h b/migration/savevm.h
index 
b12681839f0b1afa3255e45215d99c13a224b19f..c337e3e3d111a7f28a57b90f61e8f70b71803d4e
 100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -66,7 +66,8 @@ int qemu_save_device_state(QEMUFile *f);
int qemu_loadvm_state(QEMUFile *f, Error **errp);
  void qemu_loadvm_state_cleanup(MigrationIncomingState *mis);
-int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
+int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis,
+                           Error **errp);
  int qemu_load_device_state(QEMUFile *f, Error **errp);
  int qemu_loadvm_approve_switchover(void);
  int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,



Reply via email to