Re: [PATCH] Improve error propagation via return path

[Peter Xu <peterx@redhat.com>]

On Tue, Oct 21, 2025 at 07:52:53AM +0000, Dhruv Choudhary wrote:
> Use the return-path thread to send error details from the
> destination to the source on a migration failure. Management
> applications can then query the source QEMU for errors, as
> the single source of truth, making failures easy to trace.
> 
> Signed-off-by: Dhruv Choudhary <dhruv.choudhary@nutanix.com>

+Vladimir, Dan

IIUC we may still need to know whether the src QEMU supports this message
or not.

OTOH, we have introduced exit-on-error since 9.1:

# @exit-on-error: Exit on incoming migration failure.  Default true.
#     When set to false, the failure triggers a :qapi:event:`MIGRATION`
#     event, and error details could be retrieved with `query-migrate`.
#     (since 9.1)

This patch is going the other way.  That feature suggests the mgmt query
the error from dest directly.

We should stick with one plan rather than doing both.

Thanks,

> ---
>  migration/migration.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index a63b46bbef..123cffb286 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -87,6 +87,7 @@ enum mig_rp_message_type {
>      MIG_RP_MSG_RECV_BITMAP,  /* send recved_bitmap back to source */
>      MIG_RP_MSG_RESUME_ACK,   /* tell source that we are ready to resume */
>      MIG_RP_MSG_SWITCHOVER_ACK, /* Tell source it's OK to do switchover */
> +    MIG_RP_MSG_ERROR,        /* propogate error to source */
>  
>      MIG_RP_MSG_MAX
>  };
> @@ -608,6 +609,17 @@ int migrate_send_rp_req_pages(MigrationIncomingState *mis,
>      return migrate_send_rp_message_req_pages(mis, rb, start);
>  }
>  
> +static void migrate_send_rp_error(MigrationIncomingState *mis, Error *errp)
> +{
> +    const char *rpmsg = error_get_pretty(errp);
> +    if (!mis->to_src_file) {
> +        mis->to_src_file = qemu_file_get_return_path(mis->from_src_file);
> +    }
> +    migrate_send_rp_message(mis, MIG_RP_MSG_ERROR,
> +                            (uint16_t)(strlen(rpmsg) + 1),
> +                            (char *)rpmsg);
> +}
> +
>  static bool migration_colo_enabled;
>  bool migration_incoming_colo_enabled(void)
>  {
> @@ -905,8 +917,12 @@ process_incoming_migration_co(void *opaque)
>      }
>  
>      if (ret < 0) {
> -        error_prepend(&local_err, "load of migration failed: %s: ",
> -                      strerror(-ret));
> +        error_prepend(&local_err, "destination error : load of migration failed:
> +                       %s: ", strerror(-ret));
> +        /* Check if return path is enabled and then send error to source */
> +        if (migrate_postcopy_ram() || migrate_return_path()) {
> +            migrate_send_rp_error(mis, local_err);
> +        }
>          goto fail;
>      }
>  
> @@ -2437,6 +2453,7 @@ static struct rp_cmd_args {
>      [MIG_RP_MSG_RECV_BITMAP]    = { .len = -1, .name = "RECV_BITMAP" },
>      [MIG_RP_MSG_RESUME_ACK]     = { .len =  4, .name = "RESUME_ACK" },
>      [MIG_RP_MSG_SWITCHOVER_ACK] = { .len =  0, .name = "SWITCHOVER_ACK" },
> +    [MIG_RP_MSG_ERROR]          = { .len = -1, .name = "ERROR"},
>      [MIG_RP_MSG_MAX]            = { .len = -1, .name = "MAX" },
>  };
>  
> @@ -2667,6 +2684,10 @@ static void *source_return_path_thread(void *opaque)
>              trace_source_return_path_thread_switchover_acked();
>              break;
>  
> +        case MIG_RP_MSG_ERROR:
> +            error_setg(&err, "%s", (char *)buf);
> +            goto out;
> +
>          default:
>              break;
>          }
> -- 
> 2.39.3
> 

-- 
Peter Xu