[PATCH v2 22/24] migration: Make qemu_savevm_state_non_iterable() take errp

Peter Xu posted 24 patches 1 week, 5 days ago
Maintainers: Hailiang Zhang <zhanghailiang@xfusion.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Markus Armbruster <armbru@redhat.com>
[PATCH v2 22/24] migration: Make qemu_savevm_state_non_iterable() take errp
Posted by Peter Xu 1 week, 5 days ago
Let the function report errors to upper layers.  Out of three current
users, two of them already process the errors, except one outlier,
qemu_savevm_state_complete_precopy(), where we do it manually for now with
a comment for TODO.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/savevm.h    |  2 +-
 migration/migration.c |  8 ++++----
 migration/savevm.c    | 13 +++++++------
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/migration/savevm.h b/migration/savevm.h
index f2750eca09..6a589b2990 100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -74,7 +74,7 @@ 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_non_iterable(QEMUFile *f);
+int qemu_savevm_state_non_iterable(QEMUFile *f, Error **errp);
 int qemu_savevm_state_non_iterable_early(QEMUFile *f,
                                          JSONWriter *vmdesc,
                                          Error **errp);
diff --git a/migration/migration.c b/migration/migration.c
index ddf6faab46..22cfa5f19f 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2545,9 +2545,9 @@ static int postcopy_start(MigrationState *ms, Error **errp)
      */
     qemu_savevm_send_postcopy_listen(fb);
 
-    ret = qemu_savevm_state_non_iterable(fb);
+    ret = qemu_savevm_state_non_iterable(fb, errp);
     if (ret) {
-        error_setg(errp, "Postcopy save non-iterable device states failed");
+        error_prepend(errp, "Postcopy save non-iterable states failed: ");
         goto fail_closefb;
     }
 
@@ -3675,8 +3675,8 @@ static void *bg_migration_thread(void *opaque)
         goto fail_with_bql;
     }
 
-    if (qemu_savevm_state_non_iterable(fb)) {
-        error_setg(&local_err, "Failed to save non-iterable devices");
+    if (qemu_savevm_state_non_iterable(fb, &local_err)) {
+        error_prepend(&local_err, "Failed to save non-iterable devices");
         goto fail_with_bql;
     }
 
diff --git a/migration/savevm.c b/migration/savevm.c
index c16951b532..130b9764a7 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1710,13 +1710,12 @@ void qemu_savevm_state_end_precopy(MigrationState *s, QEMUFile *f)
     qemu_savevm_state_vm_desc(s, f);
 }
 
-int qemu_savevm_state_non_iterable(QEMUFile *f)
+int qemu_savevm_state_non_iterable(QEMUFile *f, Error **errp)
 {
     MigrationState *ms = migrate_get_current();
     int64_t start_ts_each, end_ts_each;
     JSONWriter *vmdesc = ms->vmdesc;
     SaveStateEntry *se;
-    Error *local_err = NULL;
     int ret;
 
     /* Making sure cpu states are synchronized before saving non-iterable */
@@ -1730,10 +1729,8 @@ int qemu_savevm_state_non_iterable(QEMUFile *f)
 
         start_ts_each = qemu_clock_get_us(QEMU_CLOCK_REALTIME);
 
-        ret = vmstate_save(f, se, vmdesc, &local_err);
+        ret = vmstate_save(f, se, vmdesc, errp);
         if (ret) {
-            migrate_error_propagate(ms, error_copy(local_err));
-            error_report_err(local_err);
             return ret;
         }
 
@@ -1750,6 +1747,7 @@ int qemu_savevm_state_non_iterable(QEMUFile *f)
 int qemu_savevm_state_complete_precopy(MigrationState *s)
 {
     QEMUFile *f = s->to_dst_file;
+    Error *local_err = NULL;
     int ret;
 
     ret = qemu_savevm_state_complete_precopy_iterable(f, false);
@@ -1757,8 +1755,11 @@ int qemu_savevm_state_complete_precopy(MigrationState *s)
         return ret;
     }
 
-    ret = qemu_savevm_state_non_iterable(f);
+    /* TODO: pass error upper */
+    ret = qemu_savevm_state_non_iterable(f, &local_err);
     if (ret) {
+        migrate_error_propagate(s, error_copy(local_err));
+        error_report_err(local_err);
         return ret;
     }
 
-- 
2.50.1
Re: [PATCH v2 22/24] migration: Make qemu_savevm_state_non_iterable() take errp
Posted by Fabiano Rosas 1 week, 4 days ago
Peter Xu <peterx@redhat.com> writes:

> Let the function report errors to upper layers.  Out of three current
> users, two of them already process the errors, except one outlier,
> qemu_savevm_state_complete_precopy(), where we do it manually for now with
> a comment for TODO.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/savevm.h    |  2 +-
>  migration/migration.c |  8 ++++----
>  migration/savevm.c    | 13 +++++++------
>  3 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/migration/savevm.h b/migration/savevm.h
> index f2750eca09..6a589b2990 100644
> --- a/migration/savevm.h
> +++ b/migration/savevm.h
> @@ -74,7 +74,7 @@ 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_non_iterable(QEMUFile *f);
> +int qemu_savevm_state_non_iterable(QEMUFile *f, Error **errp);
>  int qemu_savevm_state_non_iterable_early(QEMUFile *f,
>                                           JSONWriter *vmdesc,
>                                           Error **errp);
> diff --git a/migration/migration.c b/migration/migration.c
> index ddf6faab46..22cfa5f19f 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2545,9 +2545,9 @@ static int postcopy_start(MigrationState *ms, Error **errp)
>       */
>      qemu_savevm_send_postcopy_listen(fb);
>  
> -    ret = qemu_savevm_state_non_iterable(fb);
> +    ret = qemu_savevm_state_non_iterable(fb, errp);
>      if (ret) {
> -        error_setg(errp, "Postcopy save non-iterable device states failed");
> +        error_prepend(errp, "Postcopy save non-iterable states failed: ");
>          goto fail_closefb;
>      }
>  
> @@ -3675,8 +3675,8 @@ static void *bg_migration_thread(void *opaque)
>          goto fail_with_bql;
>      }
>  
> -    if (qemu_savevm_state_non_iterable(fb)) {
> -        error_setg(&local_err, "Failed to save non-iterable devices");
> +    if (qemu_savevm_state_non_iterable(fb, &local_err)) {
> +        error_prepend(&local_err, "Failed to save non-iterable devices");

No space or punctuation at the end?

>          goto fail_with_bql;
>      }
>  
> diff --git a/migration/savevm.c b/migration/savevm.c
> index c16951b532..130b9764a7 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1710,13 +1710,12 @@ void qemu_savevm_state_end_precopy(MigrationState *s, QEMUFile *f)
>      qemu_savevm_state_vm_desc(s, f);
>  }
>  
> -int qemu_savevm_state_non_iterable(QEMUFile *f)
> +int qemu_savevm_state_non_iterable(QEMUFile *f, Error **errp)
>  {
>      MigrationState *ms = migrate_get_current();
>      int64_t start_ts_each, end_ts_each;
>      JSONWriter *vmdesc = ms->vmdesc;
>      SaveStateEntry *se;
> -    Error *local_err = NULL;
>      int ret;
>  
>      /* Making sure cpu states are synchronized before saving non-iterable */
> @@ -1730,10 +1729,8 @@ int qemu_savevm_state_non_iterable(QEMUFile *f)
>  
>          start_ts_each = qemu_clock_get_us(QEMU_CLOCK_REALTIME);
>  
> -        ret = vmstate_save(f, se, vmdesc, &local_err);
> +        ret = vmstate_save(f, se, vmdesc, errp);
>          if (ret) {
> -            migrate_error_propagate(ms, error_copy(local_err));
> -            error_report_err(local_err);
>              return ret;
>          }
>  
> @@ -1750,6 +1747,7 @@ int qemu_savevm_state_non_iterable(QEMUFile *f)
>  int qemu_savevm_state_complete_precopy(MigrationState *s)
>  {
>      QEMUFile *f = s->to_dst_file;
> +    Error *local_err = NULL;
>      int ret;
>  
>      ret = qemu_savevm_state_complete_precopy_iterable(f, false);
> @@ -1757,8 +1755,11 @@ int qemu_savevm_state_complete_precopy(MigrationState *s)
>          return ret;
>      }
>  
> -    ret = qemu_savevm_state_non_iterable(f);
> +    /* TODO: pass error upper */
> +    ret = qemu_savevm_state_non_iterable(f, &local_err);
>      if (ret) {
> +        migrate_error_propagate(s, error_copy(local_err));
> +        error_report_err(local_err);
>          return ret;
>      }