Re: [PATCH v4 2/8] migration: cpr setup notifier

[Fabiano Rosas <farosas@suse.de>]

Ben Chaney <bchaney@akamai.com> writes:

> From: Steve Sistare <steven.sistare@oracle.com>
>
> Call MIG_EVENT_PRECOPY_SETUP earlier, before CPR starts.  An early notifier
> is needed for resetting vhost devices, as explained in the next patch.
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> Signed-off-by: Ben Chaney <bchaney@akamai.com>
> ---
>  migration/migration.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index f36e59d9e8..191a34f667 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2306,7 +2306,14 @@ void qmp_migrate(const char *uri, bool has_channels,
>          stopped = true;
>      }
>  
> +    /* Notify before starting migration thread, and before starting cpr */
> +    if (!resume_requested &&
> +        migration_call_notifiers(s, MIG_EVENT_PRECOPY_SETUP, &local_err)) {
> +        goto out;
> +    }

Probably better at the end of migrate_init() or along with the state
change to SETUP.

Also note that this will emit the event even before connections have
been established. I didn't spot any issues with that.

> +
>      if (!cpr_state_save(cpr_channel, &local_err)) {
> +        migration_call_notifiers(s, MIG_EVENT_PRECOPY_FAILED, NULL);

Due to changes on master, this now needs to go into
migration_connect_error_propagate() so that all paths that saw
MIG_EVENT_PRECOPY_SETUP also see MIG_EVENT_PRECOPY_FAILED.

  migrate_prepare() -> MIG_EVENT_PRECOPY_SETUP
    migration_connect_error_propagate() -> MIG_EVENT_PRECOPY_FAILED
    migration_iteration_finish() -> MIG_EVENT_PRECOPY_FAILED
  migration_cleanup() -> MIG_EVENT_PRECOPY_DONE

And due to changes queued [0], migration_cleanup() no longer emits the
FAILED event, so migration_connect_error_propagate() needs to send the
event for both FAILED and CANCELLING states, you can use the
migration_has_failed() helper.
https://lore.kernel.org/r/20260126213614.3815900-1-peterx@redhat.com

The special case at migrate_cancel() will also need
MIG_EVENT_PRECOPY_FAILED before resuming the VM.

>          goto out;
>      }
>  
> @@ -4097,11 +4104,6 @@ void migration_connect(MigrationState *s, Error *error_in)
>      } else {
>          /* This is a fresh new migration */
>          rate_limit = migrate_max_bandwidth();
> -
> -        /* Notify before starting migration thread */
> -        if (migration_call_notifiers(s, MIG_EVENT_PRECOPY_SETUP, &local_err)) {
> -            goto fail;
> -        }
>      }
>  
>      migration_rate_set(rate_limit);