Re: [Qemu-devel] [PATCH RFC 6/6] migration: enable return path for precopy

[Juan Quintela <quintela@redhat.com>]

Peter Xu <peterx@redhat.com> wrote:
> Let this be a flag, default to on. Turn it off for <=2.9 versions.
>
> After this patch, return path will be on even for pre-copy migration as
> long as the transport support, e.g., for socket typed transport
> including "tcp|udp|unix" typed.
>
> This will naturally fix the bug mentioned below, when destination failed
> on migration but source assumed it was successful - since now even for
> precopy, source will wait for destination's MIG_RP_MSG_SHUT signal,
> which will carry the final migration status of destination. Then, when
> destination failed at any point of migration, source will know it, and
> it'll resume the VM instead of a data lost.
>
> Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1439147
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  include/hw/compat.h           |  4 ++++
>  include/migration/migration.h |  3 +++
>  migration/migration.c         | 15 ++++++++++++++-
>  3 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 55b1765..049457b 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -6,6 +6,10 @@
>          .driver   = "pci-bridge",\
>          .property = "shpc",\
>          .value    = "off",\
> +    },{\
> +        .driver   = "migration",\
> +        .property = "return-path",\
> +        .value    = "off",\
>      },
>  
>  #define HW_COMPAT_2_8 \
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 70710de..e44119c 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -169,6 +169,9 @@ typedef struct MigrationState {
>      int64_t colo_checkpoint_time;
>      QEMUTimer *colo_delay_timer;
>  
> +    /* Whether to try to enable return-path even for pre-copy */
> +    bool enable_return_path;
> +
>      /* The last error that occurred */
>      Error *error;
>  } MigrationState ;
> diff --git a/migration/migration.c b/migration/migration.c
> index 6df3483..16a856a 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2046,7 +2046,7 @@ static void *migration_thread(void *opaque)
>  static bool migrate_return_path_create(MigrationState *s)
>  {
>      /* Whether we should enable return path */
> -    bool enable_return_path = false;
> +    bool enable_return_path = s->enable_return_path;

As you can see on my suggestion for this piece of code, just add the
()s->enable_return_path &&) to the right place on the call?

Thanks, Juan.