Re: [PATCH v2 04/24] migration: Remove MigrateSetParameters

[Markus Armbruster <armbru@redhat.com>]

Fabiano Rosas <farosas@suse.de> writes:

> Now that the TLS options have been made the same between
> migrate-set-parameters and query-migrate-parameters, a single type can
> be used. Remove MigrateSetParameters.
>
> The TLS options documentation from MigrationParameters were replaced
> with the ones from MigrateSetParameters which was more complete.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>

[...]

> diff --git a/qapi/migration.json b/qapi/migration.json
> index 97bb022c45..3788c39857 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -914,206 +914,10 @@
>             'zero-page-detection',
>             'direct-io'] }
>  
> -##
> -# @MigrateSetParameters:

[...]

>  ##
>  # @migrate-set-parameters:
>  #
> -# Set various migration parameters.
> +# Set migration parameters.  All fields are optional.

Use "arguments" instead of "fields".

>  #
>  # Since: 2.4
>  #
> @@ -1124,13 +928,11 @@
>  #     <- { "return": {} }
>  ##
>  { 'command': 'migrate-set-parameters', 'boxed': true,
> -  'data': 'MigrateSetParameters' }
> +  'data': 'MigrationParameters' }
>  
>  ##
>  # @MigrationParameters:
>  #
> -# The optional members aren't actually optional.
> -#
>  # @announce-initial: Initial delay (in milliseconds) before sending
>  #     the first announce (Since 4.0)
>  #
> @@ -1148,12 +950,12 @@
>  #     percentage.  The default value is 50.  (Since 5.0)
>  #
>  # @cpu-throttle-initial: Initial percentage of time guest cpus are
> -#     throttled when migration auto-converge is activated.
> -#     (Since 2.7)
> +#     throttled when migration auto-converge is activated.  The
> +#     default value is 20.  (Since 2.7)
>  #
>  # @cpu-throttle-increment: throttle percentage increase each time
>  #     auto-converge detects that migration is not making progress.
> -#     (Since 2.7)
> +#     The default value is 10.  (Since 2.7)
>  #
>  # @cpu-throttle-tailslow: Make CPU throttling slower at tail stage.
>  #     At the tail stage of throttling, the Guest is very sensitive to
> @@ -1172,21 +974,25 @@
>  #     for establishing a TLS connection over the migration data
>  #     channel.  On the outgoing side of the migration, the credentials
>  #     must be for a 'client' endpoint, while for the incoming side the
> -#     credentials must be for a 'server' endpoint.  An empty string
> -#     means that QEMU will use plain text mode for migration, rather
> -#     than TLS.  (Since 2.7)
> -#
> -#     Note: 2.8 omits empty @tls-creds instead.
> +#     credentials must be for a 'server' endpoint.  Setting this to a
> +#     non-empty string enables TLS for all migrations.  An empty
> +#     string means that QEMU will use plain text mode for migration,
> +#     rather than TLS.  This is the default.  (Since 2.7)
>  #
>  # @tls-hostname: migration target's hostname for validating the
>  #     server's x509 certificate identity.  If empty, QEMU will use the
> -#     hostname from the migration URI, if any.  (Since 2.7)
> +#     hostname from the migration URI, if any.  A non-empty value is
> +#     required when using x509 based TLS credentials and the migration
> +#     URI does not include a hostname, such as fd: or exec: based
> +#     migration.  (Since 2.7)
>  #
> -#     Note: 2.8 omits empty @tls-hostname instead.
> +#     Note: empty value works only since 2.9.
>  #
>  # @tls-authz: ID of the 'authz' object subclass that provides access
>  #     control checking of the TLS x509 certificate distinguished name.
> -#     (Since 4.0)
> +#     This object is only resolved at time of use, so can be deleted
> +#     and recreated on the fly while the migration server is active.
> +#     If missing, it will default to denying access (Since 4.0)
>  #
>  # @max-bandwidth: maximum speed for migration, in bytes per second.
>  #     (Since 2.8)
> @@ -1205,8 +1011,8 @@
>  # @downtime-limit: set maximum tolerated downtime for migration.
>  #     maximum downtime in milliseconds (Since 2.8)
>  #
> -# @x-checkpoint-delay: the delay time between two COLO checkpoints.
> -#     (Since 2.8)
> +# @x-checkpoint-delay: the delay time between two COLO checkpoints in
> +#     periodic mode.  (Since 2.8)
>  #
>  # @multifd-channels: Number of channels used to migrate data in
>  #     parallel.  This is the same number that the number of sockets

Please add

   ##
   # @query-migrate-parameters:
   #
  -# Return information about the current migration parameters
  +# Return information about the current migration parameters.
  +# Optional members of the return value are always present.
   #
   # Returns: @MigrationParameters
   #
   # Since: 2.4
   #

and double-check "always" is actually true.

With that, QAPI schema
Acked-by: Markus Armbruster <armbru@redhat.com>