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

[Prasad Pandit <ppandit@redhat.com>]

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