* Zeyu Jin (jinz...@huawei.com) wrote:
> On 2020/11/30 16:35, Markus Armbruster wrote:
> > Zeyu Jin <jinz...@huawei.com> writes:
> >
> >> On 2020/11/27 17:48, Markus Armbruster wrote:
> >>> Kevin, Max, suggest to skip right to Qcow2CompressionType.
> >>>
> >>> Zeyu Jin <jinz...@huawei.com> writes:
> >>>
> >>>> A multi-thread compress method parameter is added to hold the method we
> >>>> are going to use. By default the 'zlib' method is used to maintain the
> >>>> compatibility as before.
> >>>>
> >>>> Signed-off-by: Zeyu Jin <jinz...@huawei.com>
> >>>> Signed-off-by: Ying Fang <fangyi...@huawei.com>
> >>> [...]
> >>>> diff --git a/qapi/migration.json b/qapi/migration.json
> >>>> index 3c75820527..2ed6a55b92 100644
> >>>> --- a/qapi/migration.json
> >>>> +++ b/qapi/migration.json
> >>>> @@ -525,6 +525,19 @@
> >>>> 'data': [ 'none', 'zlib',
> >>>> { 'name': 'zstd', 'if': 'defined(CONFIG_ZSTD)' } ] }
> >>>>
> >>>> +##
> >>>> +# @CompressMethod:
> >>>> +#
> >>>> +# An enumeration of multi-thread compression methods.
> >>>> +#
> >>>> +# @zlib: use zlib compression method.
> >>>> +#
> >>>> +# Since: 6.0
> >>>> +#
> >>>> +##
> >>>> +{ 'enum': 'CompressMethod',
> >>>> + 'data': [ 'zlib' ] }
> >>>> +
> >>>> ##
> >>>> # @BitmapMigrationBitmapAlias:
> >>>> #
> >>>> @@ -599,6 +612,9 @@
> >>>> # compression, so set the decompress-threads to
> >>>> the number about 1/4
> >>>> # of compress-threads is adequate.
> >>>> #
> >>>> +# @compress-method: Set compression method to use in multi-thread
> >>>> compression.
> >>>> +# Defaults to zlib. (Since 6.0)
> >>>
> >>> We already have @multifd-compression. Why do we need to control the two
> >>> separately? Can you give a use case for different settings?
> >>>
> >>
> >> Generally, mulit-thread compression deals with the situation
> >> where network bandwith is limited but cpu resource is adequate. Multifd
> >> instead aims to situation where single fd cannot take full advantage of
> >> network bandwith. So compression based on multifd cannot fully cover the
> >> cases for multi-thread compression.
> >>
> >> For example, for migration with a bandwith limitation of 10M
> >> bytes/second, single fd is enough for data delivery. This is the case
> >> for multi-thread compression.
> >
> > Let me rephrase my question.
> >
> > According to query-migrate-parameters, we default to
> >
> > "compress-level": 1
> > "compress-threads": 8
> > "compress-wait-thread": true
> > "decompress-threads": 2
> > "multifd-channels": 2
> > "multifd-compression": "none"
> > "multifd-zlib-level": 1
> > "multifd-zstd-level": 1
> >
> > Your patch adds
> >
> > "compress-method": "zlib"
> >
> > I have several basic questions I can't answer from the documentation:
> >
> > 1. We appear to have two distinct sets of compression parameters:
> >
> > * Traditional: compress-level, compress-threads,
> > compress-wait-thread, decompress-threads.
> >
> > These parameters all apply to the same compression. Correct?
> >
> > What data is being compressed by it?
> >
> > * Multi-fd: multifd-channels, multifd-compression,
> > multifd-zlib-level, multifd-std-level
> >
> > These parameters all apply to the same compression. Correct?
> >
> > What data is being compressed by it?
> >
> > * Why do we want *two*? I understand why multi-fd is optional, but
> > why do we need the capability to compress differently there? Use
> > case?
> >
> > All of these questions predate your patch. David, Juan?
> >
>
> I see. The problem is that the parameter sets seem to be redundant and
> maybe there is an overlap between these two compression capabilities.
>
> As you said, the questions predate my patch, so maybe we can have a
> discussion here. What do you think, David, Juan?
Yes it's true, they're redundant - it's the same settings duplicated
for the two systems, traditinoal and multifd.
Can I ask - have you compared the behaviour of multifd-zstd with plain
zstd? I ask, because it's a shame to have two separate systems; and if
multifd-zstd worked well, then it would be good someday to deprecate the
non-multifd version of compression completely, and simplify a lot of
code that way.
Dave
> > 2. Does compress-method belong to "traditional"?
> >
>
> Yes.
>
> >>> If we do want two parameters: the names @compress-method and
> >>> @multifd-compression are inconsistent. According to your comment,
> >>> @compress-method applies only to multi-thread compression. That leads
> >>> me to suggest renaming it to @multi-thread-compression.
> >>>
> >>
> >> For the names, my original idea is to make 'CompressMethod' consistent
> >> with other multi-thread compression parameters, like 'compress-threads'
> >> and 'compress-level'. There is truly some inconsistency here, between
> >> multifd-compression params and old multi-thread compression params.
> >
> > I see.
> >
> >> For now, I agree with you that 'multi-thread-compression' is better. It
> >> is more specific and clear. I will rename the params in next version.
> >> Thanks for the suggestion.
> >
> > Please wait until we've sorted out the documentation mess.
> >
> >>> After PATCH 4, CompressMethod is almost the same as MultiFDCompression:
> >>>
> >>> { 'enum': 'CompressMethod',
> >>> 'data': [ 'zlib' ] }
> >>> 'data': [ 'zlib', { 'name': 'zstd', 'if': 'defined(CONFIG_ZSTD)' } ]
> >>> }
> >>>
> >>> { 'enum': 'MultiFDCompression',
> >>> 'data': [ 'none', 'zlib',
> >>> { 'name': 'zstd', 'if': 'defined(CONFIG_ZSTD)' } ] }
> >>>
> >>> The difference is member 'none'. Why does compression 'none' make sense
> >>> for multi-fd, but not for multi-thread?
> >>>
> >>
> >> When you set 'none'in multi-fd compression, you would not use the
> >> compression capability in multi-fd.
> >>
> >> In comparison, once you turn on multi-thread compression capability, you
> >> have already admitted to use compression. In this case, member 'none' is
> >> meaningless.
> >
> > Let me rephrase my question:
> >
> > How do you select zlib, zstd and no compression for "traditional"?
> >
>
> By parameter "compress-method". And there is no need to support "no
> compression" in "traditional" compression.
>
> > If zlib, how do you set the compression level (0 = none, 1 = fastest
> > compression, 9 = best compression)?
> >
> > If zstd, how do you set the compression level (0 = none, 1 = fastest
> > compression, 20 = best compression)?
> >
>
> In "traditional", zlib and zstd use the same parameter "compress_level".
> For each compression method, we will use different parameter check to
> make sure the level number is correct.
>
> > How do you select zlib, zstd and no compression for "multi-fd"?
> >
>
> By parameter "multifd-compression".
>
> > If zlib, how do you set the compression level (0 = none, 1 = fastest
> > compression, 9 = best compression)?
> >
> > If zstd, how do you set the compression level (0 = none, 1 = fastest
> > compression, 20 = best compression)?
> >
>
> In "multi-fd", the compress level for each method is separated, which
> means, you use "multifd-zlib-level" for zlib, "multifd-zstd-level" for
> zstd.
>
> The way to set level is different.
>
> >>> If the difference is wanted: the names are inconsistent. Less of an
> >>> issue than member names, because type names are not externally visible.
> >>> Let's make them consistent anyway.
> >>>
> >>> Hmm, there's also Qcow2CompressionType. That's another conversation;
> >>> I'll start a new thread for it.
> >>>
> >>> [...]
> >>>
> >>> .
> >>>
> >
> > .
> >
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
