diff mbox series

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

Message ID 20230622165527.2417-22-quintela@redhat.com
State New
Headers show
Series [PULL,01/30] migration/multifd: Rename threadinfo.c functions | expand

Commit Message

Juan Quintela June 22, 2023, 4:55 p.m. UTC 
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(-)

Comments

Tejus GK July 27, 2023, 8:28 a.m. UTC | #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
diff mbox series

Patch

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