Re: [Qemu-devel] [PATCH 1/2] migration: Implement a multiple compress threads feature to accelerate live migration

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4786 bytes --]

On 10/31/2014 06:40 AM, Liliang wrote:
> From: Li Liang <liang.z.li@intel.com>
> 

[metacomment] This message does not contain an 'In-Reply-To' header, and
thus does not appear threaded in spite of the name indicating that it is
part of a series.  You generally want to make sure that all patches in a
series are in reply to the 0/n cover letter.

> Instead of sending the guest memory directly, this solution compress the
> ram page before sending, after receiving, the data will be decompressed.
> This feature can help to reduce the data transferred about 60%, this
> is very useful when the network bandwidth is limited, and the migration
> time can also be reduced about 80%. The feature is off by default,
> fllowing the document docs/multiple-compression-threads.txt for

s/fllowing/following/

Umm, that file doesn't exist in the tree yet.  But I finally located
your 2/2 patch that proposes to add it; I suggest you rebase the series
to propose the docs first (for more rationale why, look at
http://wiki.qemu.org/Contribute/SubmitAPatch)

> information to use it.
> 
> Signed-off-by: Li Liang <liang.z.li@intel.com>
> ---

> +++ b/qapi-schema.json
> @@ -497,7 +497,7 @@
>  # Since: 1.2
>  ##
>  { 'enum': 'MigrationCapability',
> -  'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks'] }
> +  'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks', 'compress'] }

Missing documentation of the 'compress' flag, including the fact that it
is '(since 2.3)'.

>  
>  ##
>  # @MigrationCapabilityStatus
> @@ -1382,6 +1382,88 @@
>  { 'command': 'query-migrate-cache-size', 'returns': 'int' }
>  
>  ##
> +# @migrate-set-compress-level
> +#
> +# Set compress level
> +#
> +# @value: compress level int
> +#
> +# The compress level will be an integer between 0 and 9.
> +# The compress level can be modified before and during ongoing migration
> +#
> +# Returns: nothing on success
> +#
> +# Since: 1.2

Wrong.  This was not part of qemu 1.2.  The earliest it can be included
is 2.3.

Also, I'm not a fan of adding yet more one-off migration tunable
commands.  I have had threads in the mailing list archives before about
a proposal to make migration tunables more generic (right now,
MigrationCapability is a set of boolean-only fields, but I had a
proposal that would make it a union type where we could then add integer
capabilities, and then your compression level would set the actual
integer capability rather than just being a boolean).  Alas, I'm not in
a good position to search the archives for my previous proposals at the
moment, or I would point you to a URL of the archived message...

> +##
> +{ 'command': 'migrate-set-compress-level', 'data': {'value': 'int'} }
> +
> +##
> +# @query-migrate-compress-level
> +#
> +# query compress level
> +#
> +# Returns: compress level int
> +#
> +# Since: 1.2

Again, should be 2.3, and may be overkill if we instead enhance the
existing migration capabilities to express multiple types (my same
proposal mentioned above).

> +##
> +{ 'command': 'query-migrate-compress-level', 'returns': 'int' }
> +
> +##
> +# @migrate-set-compress-threads
> +#
> +# Set compress threads
> +#
> +# @value: compress threads int
> +#
> +# The compress thread count is an integer between 1 and 255.
> +# The compress level can be modified only before migration
> +#
> +# Returns: nothing on success
> +#
> +# Since: 1.2

Same story.

> +##
> +{ 'command': 'migrate-set-compress-threads', 'data': {'value': 'int'} }
> +
> +##
> +# @query-migrate-compress-threads
> +#
> +# query compress threads
> +#
> +# Returns: compress threads int
> +#
> +# Since: 1.2

and again

> +##
> +{ 'command': 'query-migrate-compress-threads', 'returns': 'int' }
> +
> +##
> +##
> +# @migrate-set-decompress-threads
> +#
> +# Set decompress threads
> +#
> +# @value: decompress threads int
> +#
> +# The decompress thread count is an integer between 1 and 64.
> +# The decompress level can be modified only before migration
> +#
> +# Returns: nothing on success
> +#
> +# Since: 1.2

and again. Why is this limited to 64 threads, when compression is
allowed 255 threads?

> +##
> +{ 'command': 'migrate-set-decompress-threads', 'data': {'value': 'int'} }
> +
> +##
> +# @query-migrate-decompress-threads
> +#
> +# query decompress threads
> +#
> +# Returns: decompress threads int
> +#
> +# Since: 1.2

and again

> +##
> +{ 'command': 'query-migrate-decompress-threads', 'returns': 'int' }
> +
> +##
>  # @ObjectPropertyInfo:
>  #
>  # @name: the name of the property

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]