[v9] migration: propagate vTPM errors using Error objects

[PATCH v9 22/27] migration: Capture error in postcopy_ram_listen_thread()

Posted by Arun Menon 4 months, 1 week ago

This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
postcopy_ram_listen_thread() calls qemu_loadvm_state_main()
to load the vm, and in case of a failure, it should set the error
in the migration object.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
 migration/savevm.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index c89fa59161fd1c4f20d0c3f8f14e86f014223dd6..4df50fb5c674e56c0173c357c1e23367373e2114 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2095,6 +2095,7 @@ static void *postcopy_ram_listen_thread(void *opaque)
     QEMUFile *f = mis->from_src_file;
     int load_res;
     MigrationState *migr = migrate_get_current();
+    Error *local_err = NULL;
 
     object_ref(OBJECT(migr));
 
@@ -2111,7 +2112,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, NULL);
+    load_res = qemu_loadvm_state_main(f, mis, &local_err);
 
     /*
      * This is tricky, but, mis->from_src_file can change after it
@@ -2137,7 +2138,12 @@ static void *postcopy_ram_listen_thread(void *opaque)
                          __func__, load_res);
             load_res = 0; /* prevent further exit() */
         } else {
-            error_report("%s: loadvm failed: %d", __func__, load_res);
+            if (local_err != NULL) {
+                error_prepend(&local_err,
+                              "loadvm failed during postcopy: %d: ", load_res);
+                migrate_set_error(migr, local_err);
+                error_report_err(local_err);
+            }
             migrate_set_state(&mis->state, MIGRATION_STATUS_POSTCOPY_ACTIVE,
                                            MIGRATION_STATUS_FAILED);
         }

-- 
2.50.1

Re: [PATCH v9 22/27] migration: Capture error in postcopy_ram_listen_thread()

Posted by Fabiano Rosas 4 months, 1 week ago

Arun Menon <armenon@redhat.com> writes:

> This is an incremental step in converting vmstate loading
> code to report error via Error objects instead of directly
> printing it to console/monitor.
> postcopy_ram_listen_thread() calls qemu_loadvm_state_main()
> to load the vm, and in case of a failure, it should set the error
> in the migration object.
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Arun Menon <armenon@redhat.com>
> ---
>  migration/savevm.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index c89fa59161fd1c4f20d0c3f8f14e86f014223dd6..4df50fb5c674e56c0173c357c1e23367373e2114 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2095,6 +2095,7 @@ static void *postcopy_ram_listen_thread(void *opaque)
>      QEMUFile *f = mis->from_src_file;
>      int load_res;
>      MigrationState *migr = migrate_get_current();
> +    Error *local_err = NULL;
>  
>      object_ref(OBJECT(migr));
>  
> @@ -2111,7 +2112,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, NULL);
> +    load_res = qemu_loadvm_state_main(f, mis, &local_err);
>  
>      /*
>       * This is tricky, but, mis->from_src_file can change after it
> @@ -2137,7 +2138,12 @@ static void *postcopy_ram_listen_thread(void *opaque)
>                           __func__, load_res);
>              load_res = 0; /* prevent further exit() */
>          } else {
> -            error_report("%s: loadvm failed: %d", __func__, load_res);
> +            if (local_err != NULL) {

This should always be true. I presume the conditional goes away at the
end of the series. Could have been mentioned in the commit message.

> +                error_prepend(&local_err,
> +                              "loadvm failed during postcopy: %d: ", load_res);
> +                migrate_set_error(migr, local_err);
> +                error_report_err(local_err);
> +            }
>              migrate_set_state(&mis->state, MIGRATION_STATUS_POSTCOPY_ACTIVE,
>                                             MIGRATION_STATUS_FAILED);
>          }