[v3] migration: Improve error reporting

[PATCH v3 05/26] migration: Add Error** argument to vmstate_save()

Posted by Cédric Le Goater 8 months, 3 weeks ago

This will prepare ground for futur changes adding an Error** argument
to qemu_savevm_state_setup().

Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 migration/savevm.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index d612c8a9020b204d5d078d5df85f0e6449c27645..c3642fac9dad3e314fe3fd5058889db1d2cecd47 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1008,11 +1008,10 @@ static void save_section_footer(QEMUFile *f, SaveStateEntry *se)
     }
 }
 
-static int vmstate_save(QEMUFile *f, SaveStateEntry *se, JSONWriter *vmdesc)
+static int vmstate_save(QEMUFile *f, SaveStateEntry *se, JSONWriter *vmdesc,
+                        Error **errp)
 {
     int ret;
-    Error *local_err = NULL;
-    MigrationState *s = migrate_get_current();
 
     if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
         return 0;
@@ -1034,10 +1033,9 @@ static int vmstate_save(QEMUFile *f, SaveStateEntry *se, JSONWriter *vmdesc)
     if (!se->vmsd) {
         vmstate_save_old_style(f, se, vmdesc);
     } else {
-        ret = vmstate_save_state_with_err(f, se->vmsd, se->opaque, vmdesc, &local_err);
+        ret = vmstate_save_state_with_err(f, se->vmsd, se->opaque, vmdesc,
+                                          errp);
         if (ret) {
-            migrate_set_error(s, local_err);
-            error_report_err(local_err);
             return ret;
         }
     }
@@ -1324,8 +1322,10 @@ void qemu_savevm_state_setup(QEMUFile *f)
     trace_savevm_state_setup();
     QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
         if (se->vmsd && se->vmsd->early_setup) {
-            ret = vmstate_save(f, se, ms->vmdesc);
+            ret = vmstate_save(f, se, ms->vmdesc, &local_err);
             if (ret) {
+                migrate_set_error(ms, local_err);
+                error_report_err(local_err);
                 qemu_file_set_error(f, ret);
                 break;
             }
@@ -1540,6 +1540,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
     JSONWriter *vmdesc = ms->vmdesc;
     int vmdesc_len;
     SaveStateEntry *se;
+    Error *local_err = NULL;
     int ret;
 
     QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
@@ -1550,8 +1551,10 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
 
         start_ts_each = qemu_clock_get_us(QEMU_CLOCK_REALTIME);
 
-        ret = vmstate_save(f, se, vmdesc);
+        ret = vmstate_save(f, se, vmdesc, &local_err);
         if (ret) {
+            migrate_set_error(ms, local_err);
+            error_report_err(local_err);
             qemu_file_set_error(f, ret);
             return ret;
         }
@@ -1566,7 +1569,6 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
          * bdrv_activate_all() on the other end won't fail. */
         ret = bdrv_inactivate_all();
         if (ret) {
-            Error *local_err = NULL;
             error_setg(&local_err, "%s: bdrv_inactivate_all() failed (%d)",
                        __func__, ret);
             migrate_set_error(ms, local_err);
@@ -1762,6 +1764,8 @@ void qemu_savevm_live_state(QEMUFile *f)
 
 int qemu_save_device_state(QEMUFile *f)
 {
+    MigrationState *ms = migrate_get_current();
+    Error *local_err = NULL;
     SaveStateEntry *se;
 
     if (!migration_in_colo_state()) {
@@ -1776,8 +1780,10 @@ int qemu_save_device_state(QEMUFile *f)
         if (se->is_ram) {
             continue;
         }
-        ret = vmstate_save(f, se, NULL);
+        ret = vmstate_save(f, se, NULL, &local_err);
         if (ret) {
+            migrate_set_error(ms, local_err);
+            error_report_err(local_err);
             return ret;
         }
     }
-- 
2.44.0

Re: [PATCH v3 05/26] migration: Add Error** argument to vmstate_save()

Posted by Prasad Pandit 8 months, 3 weeks ago

On Mon, 4 Mar 2024 at 19:38, Cédric Le Goater <clg@redhat.com> wrote:
> This will prepare ground for futur changes adding an Error** argument

* futur -> furure

>
> -static int vmstate_save(QEMUFile *f, SaveStateEntry *se, JSONWriter *vmdesc)
> +static int vmstate_save(QEMUFile *f, SaveStateEntry *se, JSONWriter *vmdesc,
> +                        Error **errp)
>  {
>      int ret;
> -    Error *local_err = NULL;
> -    MigrationState *s = migrate_get_current();
>
>      if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
>          return 0;
> @@ -1034,10 +1033,9 @@ static int vmstate_save(QEMUFile *f, SaveStateEntry *se, JSONWriter *vmdesc)
>      if (!se->vmsd) {
>          vmstate_save_old_style(f, se, vmdesc);
>      } else {
> -        ret = vmstate_save_state_with_err(f, se->vmsd, se->opaque, vmdesc, &local_err);
> +        ret = vmstate_save_state_with_err(f, se->vmsd, se->opaque, vmdesc,
> +                                          errp);
>          if (ret) {
> -            migrate_set_error(s, local_err);
> -            error_report_err(local_err);
>              return ret;
>          }
>      }
> @@ -1324,8 +1322,10 @@ void qemu_savevm_state_setup(QEMUFile *f)
>      trace_savevm_state_setup();
>      QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>          if (se->vmsd && se->vmsd->early_setup) {
> -            ret = vmstate_save(f, se, ms->vmdesc);
> +            ret = vmstate_save(f, se, ms->vmdesc, &local_err);
>              if (ret) {
> +                migrate_set_error(ms, local_err);
> +                error_report_err(local_err);
>                  qemu_file_set_error(f, ret);
>                  break;
>              }
> @@ -1540,6 +1540,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>      JSONWriter *vmdesc = ms->vmdesc;
>      int vmdesc_len;
>      SaveStateEntry *se;
> +    Error *local_err = NULL;
>      int ret;
>
>      QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> @@ -1550,8 +1551,10 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>
>          start_ts_each = qemu_clock_get_us(QEMU_CLOCK_REALTIME);
>
> -        ret = vmstate_save(f, se, vmdesc);
> +        ret = vmstate_save(f, se, vmdesc, &local_err);
>          if (ret) {
> +            migrate_set_error(ms, local_err);
> +            error_report_err(local_err);
>              qemu_file_set_error(f, ret);
>              return ret;
>          }
> @@ -1566,7 +1569,6 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>           * bdrv_activate_all() on the other end won't fail. */
>          ret = bdrv_inactivate_all();
>          if (ret) {
> -            Error *local_err = NULL;
>              error_setg(&local_err, "%s: bdrv_inactivate_all() failed (%d)",
>                         __func__, ret);
>              migrate_set_error(ms, local_err);
> @@ -1762,6 +1764,8 @@ void qemu_savevm_live_state(QEMUFile *f)
>
>  int qemu_save_device_state(QEMUFile *f)
>  {
> +    MigrationState *ms = migrate_get_current();
> +    Error *local_err = NULL;
>      SaveStateEntry *se;
>
>      if (!migration_in_colo_state()) {
> @@ -1776,8 +1780,10 @@ int qemu_save_device_state(QEMUFile *f)
>          if (se->is_ram) {
>              continue;
>          }
> -        ret = vmstate_save(f, se, NULL);
> +        ret = vmstate_save(f, se, NULL, &local_err);
>          if (ret) {
> +            migrate_set_error(ms, local_err);
> +            error_report_err(local_err);
>              return ret;
>          }
>      }
> --

Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>

Thank you.
---
  - Prasad