migration: small cleanup series

[PATCH v2 18/24] migration: Cleanup error propagates in qemu_savevm_state_setup()

Posted by Peter Xu 1 week, 5 days ago

We did two unnecessary error propagations in qemu_savevm_state_setup(), on
either propagate it to MigrationState*, or set qemufile with error.

Error propagation is not needed because:

  - Two live migration callers ([bg_]migration_thread) will propagate error
    if this function returned with an error.

  - Save snapshot (qemu_savevm_state) doesn't need to persist error; it got
    returned directly from save_snapshot().

QEMUFile set error is not needed because the callers always check for
errors explicitly.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/savevm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 830d8e5988..0683a103c8 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1385,8 +1385,6 @@ int qemu_savevm_state_setup(QEMUFile *f, Error **errp)
         if (se->vmsd && se->vmsd->early_setup) {
             ret = vmstate_save(f, se, vmdesc, errp);
             if (ret) {
-                migrate_error_propagate(ms, error_copy(*errp));
-                qemu_file_set_error(f, ret);
                 break;
             }
             continue;
@@ -1405,7 +1403,6 @@ int qemu_savevm_state_setup(QEMUFile *f, Error **errp)
         ret = se->ops->save_setup(f, se->opaque, errp);
         save_section_footer(f, se);
         if (ret < 0) {
-            qemu_file_set_error(f, ret);
             break;
         }
     }
-- 
2.50.1

Re: [PATCH v2 18/24] migration: Cleanup error propagates in qemu_savevm_state_setup()

Posted by Fabiano Rosas 1 week, 4 days ago

Peter Xu <peterx@redhat.com> writes:

> We did two unnecessary error propagations in qemu_savevm_state_setup(), on
> either propagate it to MigrationState*, or set qemufile with error.
>
> Error propagation is not needed because:
>
>   - Two live migration callers ([bg_]migration_thread) will propagate error
>     if this function returned with an error.
>
>   - Save snapshot (qemu_savevm_state) doesn't need to persist error; it got
>     returned directly from save_snapshot().
>
> QEMUFile set error is not needed because the callers always check for
> errors explicitly.
>

Nice

Reviewed-by: Fabiano Rosas <farosas@suse.de>

> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/savevm.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 830d8e5988..0683a103c8 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1385,8 +1385,6 @@ int qemu_savevm_state_setup(QEMUFile *f, Error **errp)
>          if (se->vmsd && se->vmsd->early_setup) {
>              ret = vmstate_save(f, se, vmdesc, errp);
>              if (ret) {
> -                migrate_error_propagate(ms, error_copy(*errp));
> -                qemu_file_set_error(f, ret);
>                  break;
>              }
>              continue;
> @@ -1405,7 +1403,6 @@ int qemu_savevm_state_setup(QEMUFile *f, Error **errp)
>          ret = se->ops->save_setup(f, se->opaque, errp);
>          save_section_footer(f, se);
>          if (ret < 0) {
> -            qemu_file_set_error(f, ret);
>              break;
>          }
>      }