Re: [PATCH v3 3/4] migration: Deprecate block migration

2023-10-12 Thread Juan Quintela
Markus Armbruster  wrote:
> Juan Quintela  writes:
>
>> It is obsolete.  It is better to use driver-mirror with NBD instead.
>
> drive-mirror
>
> Several more below.

Done.

>> +# Features:
>> +#
>> +# @deprecated: @disk migration is deprecated.  Use driver-mirror
>> +# with NBD instead.
>> +#
>
> Suggest:
>
># @deprecated: Member @disk is deprecated because block migration is.

Done.

>>  # Since: 0.14
>>  ##
>>  { 'struct': 'MigrationInfo',
>>'data': {'*status': 'MigrationStatus', '*ram': 'MigrationStats',
>> -   '*disk': 'MigrationStats',
>> +   '*disk': { 'type': 'MigrationStats', 'features': ['deprecated'] 
>> },
>> '*vfio': 'VfioStats',
>> '*xbzrle-cache': 'XBZRLECacheStats',
>> '*total-time': 'int',
>> @@ -526,6 +531,9 @@
>>  #
>>  # Features:
>>  #
>> +# @deprecated: @block migration is deprecated.  Use driver-mirror
>> +# with NBD instead.
>> +#
>
> Suggest:
>
># @deprecated: Member @block is deprecated.  Use drrive-mirror with
># NBD instead.

Done.

>>  # @unstable: Members @x-colo and @x-ignore-shared are experimental.
>>  #
>>  # Since: 1.2
>> @@ -535,7 +543,8 @@
>> 'compress', 'events', 'postcopy-ram',
>> { 'name': 'x-colo', 'features': [ 'unstable' ] },
>> 'release-ram',
>> -   'block', 'return-path', 'pause-before-switchover', 'multifd',
>> +   { 'name': 'block', 'features': [ 'deprecated' ] },
>> +   'return-path', 'pause-before-switchover', 'multifd',
>> 'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',
>> { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
>> 'validate-uuid', 'background-snapshot',
>> @@ -826,6 +835,9 @@
>>  #
>>  # Features:
>>  #
>> +# @deprecated: Member @block-incremental is obsolete. Use
>> +# driver-mirror with NBD instead.
>> +#
>
> Wait!  This is what PATCH 1 tells users to use instead of deprecated
> @inc.  You need to update that deprecation note to point to a
> non-deprecated replacement.

I don't have.
I want to deprecate the whole thing.
But the options are more urgent to deprecate/remove.  They are
implemented in a very hacky way.  Even for migration/qemu standards.

Later, Juan.



Re: [PATCH v3 3/4] migration: Deprecate block migration

2023-10-12 Thread Kevin Wolf
Am 12.10.2023 um 12:01 hat Markus Armbruster geschrieben:
> Juan Quintela  writes:
> 
> > It is obsolete.  It is better to use driver-mirror with NBD instead.
> 
> drive-mirror
> 
> Several more below.

Actually, blockdev-mirror, please. We don't want to move people from the
oldest way to the next oldest one. Pointing them to the modern way makes
more sense.

> >
> > CC: Kevin Wolf 
> > CC: Eric Blake 
> > CC: Stefan Hajnoczi 
> > CC: Hanna Czenczek 
> >
> > Signed-off-by: Juan Quintela 

Kevin



Re: [PATCH v3 3/4] migration: Deprecate block migration

2023-10-12 Thread Markus Armbruster
Juan Quintela  writes:

> It is obsolete.  It is better to use driver-mirror with NBD instead.

drive-mirror

Several more below.

>
> CC: Kevin Wolf 
> CC: Eric Blake 
> CC: Stefan Hajnoczi 
> CC: Hanna Czenczek 
>
> Signed-off-by: Juan Quintela 
> ---
>  docs/about/deprecated.rst | 10 ++
>  qapi/migration.json   | 30 +-
>  migration/block.c |  3 +++
>  migration/options.c   |  9 -
>  4 files changed, 46 insertions(+), 6 deletions(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index b2b7e11742..e46f3df376 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -431,3 +431,13 @@ The new way to modify migration is using migration 
> parameters.
>  ``blk`` functionality can be achieved by setting the
>  ``block`` migration capability to ``true``.
>  
> +block migration (since 8.2)
> +'''
> +
> +Block migration is too inflexible.  It needs to migrate all block
> +devices or none.
> +
> +Please see "QMP invocation for live storage migration with
> +``driver-mirror`` + NBD" in docs/interop/live-block-operations.rst for
> +a detailed explanation.
> +
> diff --git a/qapi/migration.json b/qapi/migration.json
> index e4949e0d8e..bf6e1ac5b5 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -269,11 +269,16 @@
>  # average memory load of the virtual CPU indirectly.  Note that
>  # zero means guest doesn't dirty memory.  (Since 8.1)
>  #
> +# Features:
> +#
> +# @deprecated: @disk migration is deprecated.  Use driver-mirror
> +# with NBD instead.
> +#

Suggest:

   # @deprecated: Member @disk is deprecated because block migration is.

>  # Since: 0.14
>  ##
>  { 'struct': 'MigrationInfo',
>'data': {'*status': 'MigrationStatus', '*ram': 'MigrationStats',
> -   '*disk': 'MigrationStats',
> +   '*disk': { 'type': 'MigrationStats', 'features': ['deprecated'] },
> '*vfio': 'VfioStats',
> '*xbzrle-cache': 'XBZRLECacheStats',
> '*total-time': 'int',
> @@ -526,6 +531,9 @@
>  #
>  # Features:
>  #
> +# @deprecated: @block migration is deprecated.  Use driver-mirror
> +# with NBD instead.
> +#

Suggest:

   # @deprecated: Member @block is deprecated.  Use drrive-mirror with
   # NBD instead.

>  # @unstable: Members @x-colo and @x-ignore-shared are experimental.
>  #
>  # Since: 1.2
> @@ -535,7 +543,8 @@
> 'compress', 'events', 'postcopy-ram',
> { 'name': 'x-colo', 'features': [ 'unstable' ] },
> 'release-ram',
> -   'block', 'return-path', 'pause-before-switchover', 'multifd',
> +   { 'name': 'block', 'features': [ 'deprecated' ] },
> +   'return-path', 'pause-before-switchover', 'multifd',
> 'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',
> { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
> 'validate-uuid', 'background-snapshot',
> @@ -826,6 +835,9 @@
>  #
>  # Features:
>  #
> +# @deprecated: Member @block-incremental is obsolete. Use
> +# driver-mirror with NBD instead.
> +#

Wait!  This is what PATCH 1 tells users to use instead of deprecated
@inc.  You need to update that deprecation note to point to a
non-deprecated replacement.

>  # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period
>  # are experimental.
>  #
> @@ -841,7 +853,7 @@
> 'tls-creds', 'tls-hostname', 'tls-authz', 'max-bandwidth',
> 'downtime-limit',
> { 'name': 'x-checkpoint-delay', 'features': [ 'unstable' ] },
> -   'block-incremental',
> +   { 'name': 'block-incremental', 'features': [ 'deprecated' ] },
> 'multifd-channels',
> 'xbzrle-cache-size', 'max-postcopy-bandwidth',
> 'max-cpu-throttle', 'multifd-compression',
> @@ -992,6 +1004,9 @@
>  #
>  # Features:
>  #
> +# @deprecated: Member @block-incremental is obsolete. Use
> +# driver-mirror with NBD instead.
> +#
>  # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period
>  # are experimental.
>  #
> @@ -1020,7 +1035,8 @@
>  '*downtime-limit': 'uint64',
>  '*x-checkpoint-delay': { 'type': 'uint32',
>   'features': [ 'unstable' ] },
> -'*block-incremental': 'bool',
> +'*block-incremental': { 'type': 'bool',
> +'features': [ 'deprecated' ] },
>  '*multifd-channels': 'uint8',
>  '*xbzrle-cache-size': 'size',
>  '*max-postcopy-bandwidth': 'size',
> @@ -1195,6 +1211,9 @@
>  #
>  # Features:
>  #
> +# @deprecated: Member @block-incremental is obsolete. Use
> +# driver-mirror with NBD instead.
> +#
>  # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period
>  # are experimental.
>  #
> @@ -1220,7 +1239,8 @@
>  '*downtime-limit': 'uint64',
>