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() must report an error
in errp, in case of failure.

When postcopy live migration runs, the device states are loaded by
both the qemu coroutine process_incoming_migration_co() and the
postcopy_ram_listen_thread(). Therefore, it is important that the
coroutine also reports the error in case of failure, with
error_report_err(). Otherwise, the source qemu will not display
any errors before going into the postcopy pause state.

Signed-off-by: Arun Menon <arme...@redhat.com>
---
  migration/migration.c |  7 ++++---
  migration/savevm.c    | 31 +++++++++++++++++++------------
  migration/savevm.h    |  2 +-
  3 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 
10c216d25dec01f206eacad2edd24d21f00e614c..bb7d5e1dee52692cbea1d2c8fdca541e6a75bedb
 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -881,7 +881,7 @@ process_incoming_migration_co(void *opaque)
                        MIGRATION_STATUS_ACTIVE);
mis->loadvm_co = qemu_coroutine_self();
-    ret = qemu_loadvm_state(mis->from_src_file);
+    ret = qemu_loadvm_state(mis->from_src_file, &local_err);
      mis->loadvm_co = NULL;
trace_vmstate_downtime_checkpoint("dst-precopy-loadvm-completed");
@@ -908,7 +908,8 @@ process_incoming_migration_co(void *opaque)
      }
if (ret < 0) {
-        error_setg(&local_err, "load of migration failed: %s", strerror(-ret));
+        error_prepend(&local_err, "load of migration failed: %s: ",
+                      strerror(-ret));
          goto fail;
      }
@@ -924,7 +925,7 @@ fail:
      migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
                        MIGRATION_STATUS_FAILED);
      migrate_set_error(s, local_err);
-    error_free(local_err);
+    error_report_err(local_err);

There is "error_report_err(s->error)" after this so this can result in duplicate error printing.

migration_incoming_state_destroy(); diff --git a/migration/savevm.c b/migration/savevm.c
index 
1bd27efe437d4d911728d776e995490d0a45dcf5..ca166ebd397ad80836ed2f9cb20a92f704fd4ed5
 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -3149,28 +3149,24 @@ out:
      return ret;
  }
-int qemu_loadvm_state(QEMUFile *f)
+int qemu_loadvm_state(QEMUFile *f, Error **errp)
  {
      MigrationState *s = migrate_get_current();
      MigrationIncomingState *mis = migration_incoming_get_current();
-    Error *local_err = NULL;
      int ret;
- if (qemu_savevm_state_blocked(&local_err)) {
-        error_report_err(local_err);
+    if (qemu_savevm_state_blocked(errp)) {
          return -EINVAL;
      }
qemu_loadvm_thread_pool_create(mis); - ret = qemu_loadvm_state_header(f, &local_err);
+    ret = qemu_loadvm_state_header(f, errp);
      if (ret) {
-        error_report_err(local_err);
          return ret;
      }
- if (qemu_loadvm_state_setup(f, &local_err) != 0) {
-        error_report_err(local_err);
+    if (qemu_loadvm_state_setup(f, errp) != 0) {
          return -EINVAL;
      }
@@ -3181,6 +3177,9 @@ int qemu_loadvm_state(QEMUFile *f)
      cpu_synchronize_all_pre_loadvm();
ret = qemu_loadvm_state_main(f, mis);
+    if (ret < 0) {
+        error_setg(errp, "Load VM state failed: %d", ret);
+    }
      qemu_event_set(&mis->main_thread_load_event);
trace_qemu_loadvm_state_post_main(ret);
@@ -3198,8 +3197,14 @@ int qemu_loadvm_state(QEMUFile *f)
          if (migrate_has_error(migrate_get_current()) ||
              !qemu_loadvm_thread_pool_wait(s, mis)) {
              ret = -EINVAL;
+            error_setg(errp,
+                       "Error while loading VM state: "
+                       "Migration stream has error");
          } else {
              ret = qemu_file_get_error(f);
+            if (ret < 0) {
+                error_setg(errp, "Error while loading vmstate : %d", ret);
+            }
          }
      }
      /*
@@ -3464,6 +3469,7 @@ void qmp_xen_save_devices_state(const char *filename, 
bool has_live, bool live,
void qmp_xen_load_devices_state(const char *filename, Error **errp)
  {
+    ERRP_GUARD();
      QEMUFile *f;
      QIOChannelFile *ioc;
      int ret;
@@ -3485,10 +3491,10 @@ void qmp_xen_load_devices_state(const char *filename, 
Error **errp)
      f = qemu_file_new_input(QIO_CHANNEL(ioc));
      object_unref(OBJECT(ioc));
- ret = qemu_loadvm_state(f);
+    ret = qemu_loadvm_state(f, errp);
      qemu_fclose(f);
      if (ret < 0) {
-        error_setg(errp, "loading Xen device state failed");
+        error_prepend(errp, "loading Xen device state failed: ");
      }
      migration_incoming_state_destroy();
  }
@@ -3496,6 +3502,7 @@ void qmp_xen_load_devices_state(const char *filename, 
Error **errp)
  bool load_snapshot(const char *name, const char *vmstate,
                     bool has_devices, strList *devices, Error **errp)
  {
+    ERRP_GUARD();
      BlockDriverState *bs_vm_state;
      QEMUSnapshotInfo sn;
      QEMUFile *f;
@@ -3559,13 +3566,13 @@ bool load_snapshot(const char *name, const char 
*vmstate,
          ret = -EINVAL;
          goto err_drain;
      }
-    ret = qemu_loadvm_state(f);
+    ret = qemu_loadvm_state(f, errp);
      migration_incoming_state_destroy();
bdrv_drain_all_end(); if (ret < 0) {
-        error_setg(errp, "Error %d while loading VM state", ret);
+        error_prepend(errp, "Error %d while loading VM state: ", ret);
          return false;
      }
diff --git a/migration/savevm.h b/migration/savevm.h
index 
2d5e9c716686f06720325e82fe90c75335ced1de..b80770b7461a60e2ad6ba5e24a7baeae73d90955
 100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -64,7 +64,7 @@ void qemu_savevm_send_colo_enable(QEMUFile *f);
  void qemu_savevm_live_state(QEMUFile *f);
  int qemu_save_device_state(QEMUFile *f);
-int qemu_loadvm_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_load_device_state(QEMUFile *f);



Reply via email to