[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] migration: Move yank outside qemu_start_incoming_migrati
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [PATCH 1/2] migration: Move yank outside qemu_start_incoming_migration() |
Date: |
Wed, 30 Jun 2021 16:33:51 +0100 |
User-agent: |
Mutt/2.0.7 (2021-05-04) |
* Peter Xu (peterx@redhat.com) wrote:
> Starting from commit b5eea99ec2f5c, qmp_migrate_recover() calls unregister
> before calling qemu_start_incoming_migration(). I believe it wanted to
> mitigate
> the next call to yank_register_instance(), but I think that's wrong.
>
> Firstly, if during recover, we should keep the yank instance there, not
> "quickly removing and adding it back".
>
> Meanwhile, calling qmp_migrate_recover() twice with b5eea99ec2f5c will
> directly
> crash the dest qemu (right now it can't; but it'll start to work right after
> the next patch) because the 1st call of qmp_migrate_recover() will unregister
> permanently when the channel failed to establish, then the 2nd call of
> qmp_migrate_recover() crashes at yank_unregister_instance().
>
> This patch fixes it by moving yank ops out of qemu_start_incoming_migration()
> into qmp_migrate_incoming. For qmp_migrate_recover(), drop the unregister of
> yank instance too since we keep it there during the recovery phase.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/migration.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 4228635d18..1bb03d1eca 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -456,10 +456,6 @@ static void qemu_start_incoming_migration(const char
> *uri, Error **errp)
> {
> const char *p = NULL;
>
> - if (!yank_register_instance(MIGRATION_YANK_INSTANCE, errp)) {
> - return;
> - }
> -
> qapi_event_send_migration(MIGRATION_STATUS_SETUP);
> if (strstart(uri, "tcp:", &p) ||
> strstart(uri, "unix:", NULL) ||
> @@ -474,7 +470,6 @@ static void qemu_start_incoming_migration(const char
> *uri, Error **errp)
> } else if (strstart(uri, "fd:", &p)) {
> fd_start_incoming_migration(p, errp);
> } else {
> - yank_unregister_instance(MIGRATION_YANK_INSTANCE);
> error_setg(errp, "unknown migration protocol: %s", uri);
> }
> }
> @@ -2083,9 +2078,14 @@ void qmp_migrate_incoming(const char *uri, Error
> **errp)
> return;
> }
>
> + if (!yank_register_instance(MIGRATION_YANK_INSTANCE, errp)) {
> + return;
> + }
> +
> qemu_start_incoming_migration(uri, &local_err);
>
> if (local_err) {
> + yank_unregister_instance(MIGRATION_YANK_INSTANCE);
> error_propagate(errp, local_err);
> return;
> }
> @@ -2114,7 +2114,6 @@ void qmp_migrate_recover(const char *uri, Error **errp)
> * only re-setup the migration stream and poke existing migration
> * to continue using that newly established channel.
> */
> - yank_unregister_instance(MIGRATION_YANK_INSTANCE);
> qemu_start_incoming_migration(uri, errp);
> }
>
> --
> 2.31.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK