Re: [Qemu-devel] [PATCH 2/3] migrate: Share common MigrationParameters struct

2016-10-17 Thread Eric Blake
On 09/08/2016 10:14 PM, Eric Blake wrote:
> It is rather verbose, and slightly error-prone, to repeat
> the same set of parameters for input (migrate-set-parameters)
> as for output (query-migrate-parameters), where the only
> difference is whether the members are optional.  We can just
> document that the optional members will always be present
> on output, and then share a common struct between both
> commands.  The next patch can then reduce the amount of
> code needed on input.

>  '*tls-hostname': 'str'} }
> -
> -#
> -# @MigrationParameters
> -#
...
> -##
> -{ 'struct': 'MigrationParameters',
> -  'data': { 'compress-level': 'int',
> -'compress-threads': 'int',
> -'decompress-threads': 'int',
> -'cpu-throttle-initial': 'int',
> -'cpu-throttle-increment': 'int',
> -'tls-creds': 'str',
> -'tls-hostname': 'str'} }
>  ##
>  # @query-migrate-parameters

Pre-existing - there was no blank line before the docs for
query-migrate-parameters.  Commit a43edcf fixed the blank line, then the
merge conflict resolution undid things; so I've submitted a followup.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 2/3] migrate: Share common MigrationParameters struct

2016-10-05 Thread Juan Quintela
Eric Blake  wrote:
> It is rather verbose, and slightly error-prone, to repeat
> the same set of parameters for input (migrate-set-parameters)
> as for output (query-migrate-parameters), where the only
> difference is whether the members are optional.  We can just
> document that the optional members will always be present
> on output, and then share a common struct between both
> commands.  The next patch can then reduce the amount of
> code needed on input.
>
> Also, we made a mistake in qemu 2.7 of returning an empty
> string during 'query-migrate-parameters' when there is no
> TLS, rather than omitting TLS details entirely.  Technically,
> this change risks breaking any 2.7 client that is hard-coded
> to expect the parameter's existence; on the other hand, clients
> that are portable to 2.6 already must be prepared for those
> members to not be present.
>
> And this gets rid of yet one more place where the QMP output
> visitor is silently converting a NULL string into "" (which
> is a hack I ultimately want to kill off).
>
> Signed-off-by: Eric Blake 

Reviewed-by: Juan Quintela 



Re: [Qemu-devel] [PATCH 2/3] migrate: Share common MigrationParameters struct

2016-09-09 Thread Marc-André Lureau
On Fri, Sep 9, 2016 at 7:19 AM Eric Blake  wrote:

> It is rather verbose, and slightly error-prone, to repeat
> the same set of parameters for input (migrate-set-parameters)
> as for output (query-migrate-parameters), where the only
> difference is whether the members are optional.  We can just
> document that the optional members will always be present
> on output, and then share a common struct between both
> commands.  The next patch can then reduce the amount of
> code needed on input.
>
> Also, we made a mistake in qemu 2.7 of returning an empty
> string during 'query-migrate-parameters' when there is no
> TLS, rather than omitting TLS details entirely.  Technically,
> this change risks breaking any 2.7 client that is hard-coded
> to expect the parameter's existence; on the other hand, clients
> that are portable to 2.6 already must be prepared for those
> members to not be present.
>
> And this gets rid of yet one more place where the QMP output
> visitor is silently converting a NULL string into "" (which
> is a hack I ultimately want to kill off).
>
> Signed-off-by: Eric Blake 
>


Reviewed-by: Marc-André Lureau 

> ---
>  qapi-schema.json  | 116
> +++---
>  hmp.c |   9 +++-
>  migration/migration.c |   7 +++
>  3 files changed, 57 insertions(+), 75 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index c4f3674..4a51e5b 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -647,40 +647,53 @@
>  #
>  # @migrate-set-parameters
>  #
> -# Set the following migration parameters
> -#
> -# @compress-level: compression level
> -#
> -# @compress-threads: compression thread count
> -#
> -# @decompress-threads: decompression thread count
> -#
> -# @cpu-throttle-initial: Initial percentage of time guest cpus are
> 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. The default value is 10. (Since 2.7)
> -#
> -# @tls-creds: ID of the 'tls-creds' object that provides credentials 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. Setting this
> -# will enable TLS for all migrations. The default is unset,
> -# resulting in unsecured migration at the QEMU level. (Since
> 2.7)
> -#
> -# @tls-hostname: hostname of the target host for the migration. This is
> -#required when using x509 based TLS credentials and the
> -#migration URI does not already include a hostname. For
> -#example if using fd: or exec: based migration, the
> -#hostname must be provided so that the server's x509
> -#certificate identity can be validated. (Since 2.7)
> +# Set various migration parameters.  See MigrationParameters for details.
>  #
>  # Since: 2.4
>  ##
>  { 'command': 'migrate-set-parameters',
> +  'data': 'MigrationParameters' }
> +
> +#
> +# @MigrationParameters
> +#
> +# Optional members can be omitted on input ('migrate-set-parameters')
> +# but most members will always be present on output
> +# ('query-migrate-parameters'), with the exception of tls-creds and
> +# tls-hostname.
> +#
> +# @compress-level: #optional compression level
> +#
> +# @compress-threads: #optional compression thread count
> +#
> +# @decompress-threads: #optional decompression thread count
> +#
> +# @cpu-throttle-initial: #optional Initial percentage of time guest cpus
> are
> +#throttledwhen migration auto-converge is
> activated.
> +#The default value is 20. (Since 2.7)
> +#
> +# @cpu-throttle-increment: #optional throttle percentage increase each
> time
> +#  auto-converge detects that migration is not
> making
> +#  progress. The default value is 10. (Since 2.7)
> +#
> +# @tls-creds: #optional ID of the 'tls-creds' object that provides
> credentials
> +# 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. Setting this
> +# will enable TLS for all migrations. The default is unset,
> +# resulting in unsecured migration at the QEMU level. (Since
> 2.7)
> +#
> +# @tls-hostname: #optional hostname of the target host for the migration.
> This
> +#is required