Re: [RFC 6/6] migration: Deprecated old compression method

[Juan Quintela]

Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Mon, Jun 12, 2023 at 09:33:44PM +0200, Juan Quintela wrote:
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  docs/about/deprecated.rst |  8 ++++
>>  qapi/migration.json       | 92 ++++++++++++++++++++++++---------------
>>  migration/options.c       | 13 ++++++
>>  3 files changed, 79 insertions(+), 34 deletions(-)
>> 
>> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>> index 173c5ba5cb..fe7f2bbde8 100644
>> --- a/docs/about/deprecated.rst
>> +++ b/docs/about/deprecated.rst
>> @@ -460,3 +460,11 @@ block migration (since 8.1)
>>  Block migration is too inflexible.  It needs to migrate all block
>>  devices or none.  Use driver_mirror+NBD instead.
>>  
>> +old compression method (since 8.1)
>> +''''''''''''''''''''''''''''''''''
>> +
>> +Compression method fails too much.  Too many races.  We are going to
>> +remove it if nobody fixes it.  For starters, migration-test
>> +compression tests are disabled becase they hand randomly.  If you need
>> +compression, use multifd compression methods.
>> +
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index a8497de48d..40a8b5d124 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -244,6 +244,7 @@
>>  #
>>  # @compression: migration compression statistics, only returned if
>>  #     compression feature is on and status is 'active' or 'completed'
>> +#     It is obsolete and deprecated.  Use multifd compression methods.
>>  #     (Since 3.1)
>
> This doesn't give users an indication /why/ we're saying this. Instead
> I'd suggest
>
>   This feature is unreliable and not tested. It is recommended to
>   use multifd migration instead, which offers an alternative reliable
>   and tested compression implementation.

Much better.  Done, thanks.


>>  # @deprecated: @disk migration is deprecated.  Use driver_mirror+NBD
>> -#     instead.
>> +#     instead. @compression is obsolete use multifd compression
>> +#     methods instead.
>
> For @deprecated, are we supposed to list multiple things at once, or
> use a separate @deprecated tag for each one ?

# @unstable: Members @x-colo and @x-ignore-shared are experimental.

This is the only example that I found that is similar.
Only one example.  Markus?

>
> Again I'd suggest rewording
>
>     @compression is unreliable and untested. It is recommended to
>     use multifd migration, which offers an alternative compression
>     implementation that is reliable and tested.

Done.



>> @@ -443,6 +443,11 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, 
>> Error **errp)
>>                      "Use driver_mirror+NBD instead.");
>>      }
>>  
>> +    if (new_caps[MIGRATION_CAPABILITY_BLOCK]) {
>
> Surely MIGRATION_CAPABILITY_COMPRESS not BLOCK ?

Good catch.  Copy & paste to its best.

Thanks very much.