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) {
-- 
2.40.1

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