Re: [PATCH v2 01/13] migration: Propagate errors in migration_completion_precopy()

[Peter Xu <peterx@redhat.com>]

On Tue, Jun 02, 2026 at 12:26:09PM +0300, Avihai Horon wrote:
> migration_completion_precopy() doesn't propagate errors to migration
> core which leads to error information loss. Fix that.
> 
> This prepares for a follow-up where migration_switchover_start() can
> fail on switchover-ack and still report a useful error. Errors from
> qemu_savevm_state_complete_precopy() are not propagated yet as it
> requires more plumbing.
> 
> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
> ---
>  migration/migration.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 074d3f2c69..7488a94206 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2814,7 +2814,7 @@ static bool migration_switchover_start(MigrationState *s, Error **errp)
>      return true;
>  }
>  
> -static int migration_completion_precopy(MigrationState *s)
> +static int migration_completion_precopy(MigrationState *s, Error **errp)
>  {
>      int ret;
>  
> @@ -2823,11 +2823,12 @@ static int migration_completion_precopy(MigrationState *s)
>      if (!migrate_mode_is_cpr()) {
>          ret = migration_stop_vm(s, RUN_STATE_FINISH_MIGRATE);
>          if (ret < 0) {
> +            error_setg_errno(errp, -ret, "Failed to stop the VM");
>              goto out_unlock;
>          }
>      }
>  
> -    if (!migration_switchover_start(s, NULL)) {
> +    if (!migration_switchover_start(s, errp)) {
>          ret = -EFAULT;
>          goto out_unlock;
>      }

IIUC this patch overlooked the follow up call:

    ret = qemu_savevm_state_complete_precopy(s);

We should make sure ret!=0 will always set Error*.  Better pass Error**
into qemu_savevm_state_complete_precopy() too.

Thanks,

> @@ -2869,7 +2870,7 @@ static void migration_completion(MigrationState *s)
>      Error *local_err = NULL;
>  
>      if (s->state == MIGRATION_STATUS_ACTIVE) {
> -        ret = migration_completion_precopy(s);
> +        ret = migration_completion_precopy(s, &local_err);
>      } else if (s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE) {
>          migration_completion_postcopy(s);
>      } else {
> @@ -2900,7 +2901,9 @@ static void migration_completion(MigrationState *s)
>      return;
>  
>  fail:
> -    if (qemu_file_get_error_obj(s->to_dst_file, &local_err)) {
> +    if (local_err) {
> +        migrate_error_propagate(s, local_err);
> +    } else if (qemu_file_get_error_obj(s->to_dst_file, &local_err)) {
>          migrate_error_propagate(s, local_err);
>      } else if (ret) {
>          error_setg_errno(&local_err, -ret, "Error in migration completion");
> -- 
> 2.40.1
> 

-- 
Peter Xu