Re: [PULL 21/30] migration: Refactor repeated call of yank_unregister_instance

[Tejus GK]

On 22/06/23 10:25 pm, Juan Quintela wrote:
> From: Tejus GK <tejus.gk@nutanix.com>
>
> In the function qmp_migrate(), yank_unregister_instance() gets called
> twice which isn't required. Hence, refactoring it so that it gets called
> during the local_error cleanup.
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Acked-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Tejus GK <tejus.gk@nutanix.com>
> Message-ID: <20230621130940.178659-3-tejus.gk@nutanix.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>   migration/migration.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index e6bff2e848..7a4ba2e846 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1676,15 +1676,11 @@ void qmp_migrate(const char *uri, bool has_blk, bool 
> blk,
>       } else if (strstart(uri, "fd:", &p)) {
>           fd_start_outgoing_migration(s, p, &local_err);
>       } else {
> -        if (!(has_resume && resume)) {
> -            yank_unregister_instance(MIGRATION_YANK_INSTANCE);
> -        }
>           error_setg(&local_err, QERR_INVALID_PARAMETER_VALUE, "uri",
>                      "a valid migration protocol");
>           migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
>                             MIGRATION_STATUS_FAILED);
>           block_cleanup_parameters();
> -        return;
>       }
>   
>       if (local_err) {

Hi Juan,

I saw that this patch wasn't queued in yesterday's migration PULL, is 
there any reason why?. Without this refactor, it makes the error
description change (which got merged yesterday), in this function, quite
redundant.

Regards,
Tejus