Re: [PATCH v5 5/7] migration: Deprecate old compression method

Markus Armbruster <armbru(a)redhat.com&gt; wrote: > Juan Quintela <quintela(a)redhat.com&gt; writes: > >> Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; >> Acked-by: Stefan Hajnoczi <stefanha(a)redhat.com&gt; >> Acked-by: Peter Xu <peterx(a)redhat.com&gt; >> # @deprecated: Member @disk is deprecated because block migration is. >> +# Member @compression is deprecated because it is unreliable and >> +# untested. It is recommended to use multifd migration, which >> +# offers an alternative compression implementation that is >> +# reliable and tested. > > Two spaces between sentences for consistency, please. I have reviewed all the patches again. Let's hope that I didn't miss one. >> # @announce-step: Increase in delay (in milliseconds) between >> # subsequent packets in the announcement (Since 4.0) >> # >> -# @compress-level: compression level >> +# @compress-level: compression level. >> # >> -# @compress-threads: compression thread count >> +# @compress-threads: compression thread count. >> # >> # @compress-wait-thread: Controls behavior when all compression >> # threads are currently busy. If true (default), wait for a free >> # compression thread to become available; otherwise, send the page >> -# uncompressed. (Since 3.1) >> +# uncompressed. (Since 3.1) >> # >> -# @decompress-threads: decompression thread count >> +# @decompress-threads: decompression thread count. >> # >> # @throttle-trigger-threshold: The ratio of bytes_dirty_period and >> # bytes_xfer_period to trigger throttling. It is expressed as > > Unrelated. Rebases are bad for you O:-) >> @@ -1023,7 +1036,9 @@ >> # Features: >> # >> # @deprecated: Member @block-incremental is deprecated. Use >> -# blockdev-mirror with NBD instead. >> +# blockdev-mirror with NBD instead. Members @compress-level, >> +# @compress-threads, @decompress-threads and @compress-wait-thread >> +# are deprecated because @compression is deprecated. > > Two spaces between sentences for consistency, please. Done. >> @@ -1078,7 +1097,7 @@ >> # Example: >> # >> # -> { "execute": "migrate-set-parameters" , >> -# "arguments": { "compress-level": 1 } } >> +# "arguments": { "multifd-channels": 5 } } >> # <- { "return": {} } >> ## > > Thanks for taking care of updating the example! You are welcome. grep for all occurences of compress-level and friends has its advantages. >> # @compress-wait-thread: Controls behavior when all compression >> # threads are currently busy. If true (default), wait for a free >> # compression thread to become available; otherwise, send the page >> -# uncompressed. (Since 3.1) >> +# uncompressed. (Since 3.1) >> # >> -# @decompress-threads: decompression thread count >> +# @decompress-threads: decompression thread count. >> # >> # @throttle-trigger-threshold: The ratio of bytes_dirty_period and >> # bytes_xfer_period to trigger throttling. It is expressed as > > Unrelated. I have removed the periods. But I have a question, why the descriptions that are less than one line don't have period and the other have it.

>> if (params->has_compress_level) { >> + warn_report("Old compression is deprecated. " >> + "Use multifd compression methods instead."); >> s->parameters.compress_level = params->compress_level; >> } >> >> if (params->has_compress_threads) { >> + warn_report("Old compression is deprecated. " >> + "Use multifd compression methods instead."); >> s->parameters.compress_threads = params->compress_threads; >> } >> >> if (params->has_compress_wait_thread) { >> + warn_report("Old compression is deprecated. " >> + "Use multifd compression methods instead."); >> s->parameters.compress_wait_thread = params->compress_wait_thread; >> } >> >> if (params->has_decompress_threads) { >> + warn_report("Old compression is deprecated. " Once here, I did s/Old/old/ as all your examples of description start with lowercase.

>> + "Use multifd compression methods instead."); >> s->parameters.decompress_threads = params->decompress_threads; >> } > > Other than that > Reviewed-by: Markus Armbruster <armbru(a)redhat.com&gt; Thanks for your patience.