Re: [PATCH v20 09/15] qapi: add local migration parameter

[Markus Armbruster <armbru@redhat.com>]

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> We are going to implement local-migration feature: some devices will be
> able to transfer open file descriptors through migration stream (which
> must UNIX domain socket for that purpose).  This allows to transfer the
> whole backend state without reconnecting and restarting the backend
> service. For example, virtio-net will migrate its attached TAP netdev,
> together with its connected file descriptors.
>
> In this commit we introduce a migration parameter, which enables
> the feature for devices that support it (none at the moment).
>
> We can't simply auto-detect local migration by checking whether the
> migration channel is a UNIX domain socket: the UNIX socket may be only
> the first part of the transfer channel to a remote target.  Hence an
> explicit parameter is needed. More over, "local" migration parameter

Moreover

> may be useful in cpr-exec mode, when FDs are passed through exec
> instead of UNIX socket.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  include/migration/misc.h |  2 ++
>  migration/options.c      | 18 +++++++++++++++++-
>  qapi/migration.json      | 22 ++++++++++++++++++----
>  3 files changed, 37 insertions(+), 5 deletions(-)
>
> diff --git a/include/migration/misc.h b/include/migration/misc.h
> index 2b2fbb59a40..019717f4cda 100644
> --- a/include/migration/misc.h
> +++ b/include/migration/misc.h
> @@ -161,4 +161,6 @@ bool multifd_join_device_state_save_threads(void);
>  
>  void migration_request_switchover_ack_legacy(const char *requester);
>  
> +bool migrate_local(void);
> +
>  #endif
> diff --git a/migration/options.c b/migration/options.c
> index dfce19405d4..5c439f6d724 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -14,6 +14,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu/error-report.h"
>  #include "qemu/units.h"
> +#include "qapi/util.h"
>  #include "exec/target_page.h"
>  #include "qapi/clone-visitor.h"
>  #include "qapi/error.h"
> @@ -25,6 +26,7 @@
>  #include "migration/colo.h"
>  #include "migration/cpr.h"
>  #include "migration/misc.h"
> +#include "migration/options.h"
>  #include "migration.h"
>  #include "migration-stats.h"
>  #include "qemu-file.h"
> @@ -350,6 +352,12 @@ bool migrate_mapped_ram(void)
>      return s->capabilities[MIGRATION_CAPABILITY_MAPPED_RAM];
>  }
>  
> +bool migrate_local(void)
> +{
> +    MigrationState *s = migrate_get_current();
> +    return s->parameters.local;
> +}
> +
>  bool migrate_ignore_shared(void)
>  {
>      MigrationState *s = migrate_get_current();
> @@ -1085,7 +1093,7 @@ static void migrate_mark_all_params_present(MigrationParameters *p)
>          &p->has_announce_step, &p->has_block_bitmap_mapping,
>          &p->has_x_vcpu_dirty_limit_period, &p->has_vcpu_dirty_limit,
>          &p->has_mode, &p->has_zero_page_detection, &p->has_direct_io,
> -        &p->has_x_rdma_chunk_size, &p->has_cpr_exec_command,
> +        &p->has_x_rdma_chunk_size, &p->has_cpr_exec_command, &p->has_local,
>      };
>  
>      len = ARRAY_SIZE(has_fields);
> @@ -1433,6 +1441,10 @@ static void migrate_params_test_apply(MigrationParameters *params,
>          qapi_free_strList(dest->cpr_exec_command);
>          dest->cpr_exec_command = QAPI_CLONE(strList, params->cpr_exec_command);
>      }
> +
> +    if (params->has_local) {
> +        dest->local = params->local;
> +    }
>  }
>  
>  static void migrate_params_apply(MigrationParameters *params)
> @@ -1565,6 +1577,10 @@ static void migrate_params_apply(MigrationParameters *params)
>          s->parameters.cpr_exec_command =
>              QAPI_CLONE(strList, params->cpr_exec_command);
>      }
> +
> +    if (params->has_local) {
> +        s->parameters.local = params->local;
> +    }
>  }
>  
>  void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
> diff --git a/qapi/migration.json b/qapi/migration.json
> index b1eaf7b0545..bb744f134ce 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -830,7 +830,8 @@
>             'zero-page-detection',
>             'direct-io',
>             { 'name': 'x-rdma-chunk-size', 'features': [ 'unstable' ] },
> -           'cpr-exec-command'] }
> +           'cpr-exec-command',
> +           'local'] }
>  
>  ##
>  # @migrate-set-parameters:
> @@ -1012,10 +1013,21 @@
>  #     Must be set to the same value on both source and destination
>  #     before migration starts.  (Since 11.1)
>  #
> +# @local: Permit the use of optimizations for local migration.
> +#     This must only be set when either both the source and
> +#     destination QEMU processes are directly connected with a UNIX
> +#     domain socket as the migration channel (to enable use of file
> +#     descriptor passing) or cpr-exec migration mode is enabled
> +#     (this way file descriptors are inherited by new process).
> +#     Individual device backends may need additional configuration
> +#     flags set  to enable local migration optimizations.

Extra space between "set" and "to".

> +#     This will be documented against the device
> +#     backends where it applies.  (Since 11.2)

Reflow while there:

   # @local: Permit the use of optimizations for local migration.  This
   #     must only be set when either both the source and destination
   #     QEMU processes are directly connected with a UNIX domain socket
   #     as the migration channel (to enable use of file descriptor
   #     passing) or cpr-exec migration mode is enabled (this way file
   #     descriptors are inherited by new process).  Individual device
   #     backends may need additional configuration flags set to enable
   #     local migration optimizations.  This will be documented against
   #     the device backends where it applies.  (Since 11.2)

> +#
>  # Features:
>  #
> -# @unstable: Members @x-checkpoint-delay, @x-rdma-chunk-size, and
> -#     @x-vcpu-dirty-limit-period are experimental.
> +# @unstable: Members @local, @x-checkpoint-delay, @x-rdma-chunk-size,
> +#     and @x-vcpu-dirty-limit-period are experimental.
>  #
>  # Since: 2.4
>  ##
> @@ -1053,7 +1065,9 @@
>              '*direct-io': 'bool',
>              '*x-rdma-chunk-size': { 'type': 'uint64',
>                                      'features': [ 'unstable' ] },
> -            '*cpr-exec-command': [ 'str' ]} }
> +            '*cpr-exec-command': [ 'str' ],
> +            '*local': { 'type': 'bool',
> +                        'features': [ 'unstable' ] } } }
>  
>  ##
>  # @query-migrate-parameters:

With these cosmetic tweaks
Acked-by: Markus Armbruster <armbru@redhat.com>