Re: [Qemu-devel] [PATCH] migration: add "return-path" capability

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Peter Xu (peterx@redhat.com) wrote:
> When this capability is enabled, QEMU will use the return path even for
> precopy migration. This is helpful at least in one case when destination
> failed to load the image while source quited without confirmation. With
> return path, source will wait for the last response from destination,
> and if destination fails, it'll fail the migration on source, then the
> guest can be run again on the source (rather than assuming to be good,
> then the guest will be lost after source quits).
> 
> It needs to be enabled explicitly on source, otherwise disabled.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

OK, so this looks fine, lets see what else we have.
The migration thread already calls savevm_send_open_return_path
if it's got a from_dst_file.

We already call 'await_return_path_close_on_source' in
migration_completion if we have a from_dst_file  so that should
already work.

and on the incoming side, migration_incoming_state_destroy calls
migrate_send_rp_shut  if it has a to_src_file.  So that should close it
as well.

So that looks like a full set.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  migration/migration.c | 16 +++++++++++++---
>  migration/migration.h |  1 +
>  qapi-schema.json      |  5 ++++-
>  3 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index f588329..66fd81e 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1304,6 +1304,15 @@ bool migrate_use_block(void)
>      return s->enabled_capabilities[MIGRATION_CAPABILITY_BLOCK];
>  }
>  
> +bool migrate_use_return_path(void)
> +{
> +    MigrationState *s;
> +
> +    s = migrate_get_current();
> +
> +    return s->enabled_capabilities[MIGRATION_CAPABILITY_RETURN_PATH];
> +}
> +
>  bool migrate_use_block_incremental(void)
>  {
>      MigrationState *s;
> @@ -1968,10 +1977,11 @@ void migrate_fd_connect(MigrationState *s)
>      notifier_list_notify(&migration_state_notifiers, s);
>  
>      /*
> -     * Open the return path; currently for postcopy but other things might
> -     * also want it.
> +     * Open the return path. For postcopy, it is used exclusively. For
> +     * precopy, only if user specified "return-path" capability would
> +     * QEMU uses the return path.
>       */
> -    if (migrate_postcopy_ram()) {
> +    if (migrate_postcopy_ram() || migrate_use_return_path()) {
>          if (open_return_path_on_source(s)) {
>              error_report("Unable to open return-path for postcopy");
>              migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
> diff --git a/migration/migration.h b/migration/migration.h
> index d9a268a..e41fb68 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -144,6 +144,7 @@ bool migrate_colo_enabled(void);
>  
>  bool migrate_use_block(void);
>  bool migrate_use_block_incremental(void);
> +bool migrate_use_return_path(void);
>  
>  bool migrate_use_compression(void);
>  int migrate_compress_level(void);
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 4b50b65..9e34cf7 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -900,12 +900,15 @@
>  #          offers more flexibility.
>  #          (Since 2.10)
>  #
> +# @return-path: If enabled, migration will use the return path even
> +#               for precopy. (since 2.10)
> +#
>  # Since: 1.2
>  ##
>  { 'enum': 'MigrationCapability',
>    'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
>             'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram',
> -           'block' ] }
> +           'block', 'return-path' ] }
>  
>  ##
>  # @MigrationCapabilityStatus:
> -- 
> 2.7.4
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK