Re: [PATCH v2 7/7] migration: Postpone the outgoing preempt channel for preempt_pre_7_2

[Fabiano Rosas <farosas@suse.de>]

Peter Xu <peterx@redhat.com> writes:

> This is the corresponding outgoing fix for the same problem mentioned in
> the previous patch.
>
> The idea is simple: either migration to an old QEMU or when TLS is enabled,
> we rely on correct ordering of channels to make it work: first main, then
> multifd, finally preempt.
>
> This patch re-orders multifd and postcopy preempt channel on outgoing side,
> but only for the old machine types where preempt_pre_7_2 set, to make it
> follow the right orders to create the channels.
>
> Keep the resume path alone because the resume path never involve multifd
> channels, even if multifd is enabled.  For that, fun fact: when multifd is
> enabled with postcopy preempt, network interruption will break all multifd
> channels, but they're never re-connected, until they got recycled at the
> end of postcopy migration.  OTOH, multifd_recv_all_channels_created()
> reports true even after postcopy network down, as multifd_recv_state->count
> is never decremented.
>
> NOTE: this will move the preempt channel connection no matter multifd is
> enabled or not.  It should fix multifd's case, but should have zero impact
> on when multifd is not enabled.
>
> Fixes: e27418861288 ("migration: enable multifd and postcopy together")
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/migration.c | 33 ++++++++++++++++++++++++---------
>  1 file changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index b140fb6950..53cb6d3821 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3729,6 +3729,21 @@ static void *migration_thread(void *opaque)
>          goto out;
>      }
>  
> +    /*
> +     * Only for old 7.1/7.2 machine types (preempt mode supported since
> +     * 7.1).  Connect the preempt channel only after multifd channels,
> +     * because some QEMU versions (v10.1 to v11.1) will assume the order of
> +     * channels to be: (1) main, then (2) multifd channels, then (3)
> +     * preempt channel.  IOW, (3) must be after (2).
> +     *
> +     * multifd_send_setup() makes sure all multifd channels be connected
> +     * first, then initiate preempt channel here guarantees the ordering to
> +     * those QEMU binaries.
> +     */
> +    if (migrate_postcopy_preempt() && s->preempt_pre_7_2) {
> +        postcopy_preempt_setup(s);
> +    }
> +
>      bql_lock();
>      qemu_savevm_state_header(s->to_dst_file);
>      bql_unlock();
> @@ -4009,16 +4024,16 @@ void migration_start_outgoing(MigrationState *s)
>          open_return_path_on_source(s);
>      }
>  
> -    /*
> -     * This needs to be done before resuming a postcopy.  Note: for newer
> -     * QEMUs we will delay the channel creation until postcopy_start(), to
> -     * avoid disorder of channel creations.
> -     */
> -    if (migrate_postcopy_preempt() && s->preempt_pre_7_2) {
> -        postcopy_preempt_setup(s);
> -    }
> -
>      if (resume) {
> +        /*
> +         * This needs to be done before resuming a postcopy.  Note: for newer
> +         * QEMUs we will delay the channel creation until postcopy_start(), to
> +         * avoid disorder of channel creations.
> +         */
> +        if (migrate_postcopy_preempt() && s->preempt_pre_7_2) {
> +            postcopy_preempt_setup(s);
> +        }
> +
>          /* Wakeup the main migration thread to do the recovery */
>          migrate_set_state(&s->state, MIGRATION_STATUS_POSTCOPY_RECOVER_SETUP,
>                            MIGRATION_STATUS_POSTCOPY_RECOVER);

Reviewed-by: Fabiano Rosas <farosas@suse.de>