Re: [PATCH 1/3] migration/colo: Deprecate COLO migration framework

[Markus Armbruster <armbru@redhat.com>]

Peter Xu <peterx@redhat.com> writes:

> COLO was broken for QEMU release 10.0/10.1 without anyone noticed.

We could arguably drop this right away.  I'm not demanding we do, just
pointing out.

First, COLO is marked 'unstable' in the QAPI schema:

* MigrationCapability member x-colo:

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

* MigrationParameter and MigrationParameters member x-checkpoint-delay:

    # @unstable: Members @x-checkpoint-delay and
    #     @x-vcpu-dirty-limit-period are experimental.

* Command x-colo-lost-heartbeat:

    # @unstable: This command is experimental.

There's more COLO stuff we neglected to mark, e.g. MigrationStatus
member @colo, event COLO_EXIT, commands xen-colo-do-checkpoint,
query-colo-status.  We should clean that up.  More on that below.

Second, it's been broken for two releases, our deprecation grace period.
In my opinion, "broken" is even stronger notice than "deprecated".

>                                                                     One
> reason might be that we don't have an unit test for COLO (which we
> explicitly require now for any new migration feature).  The other reason
> should be that there are just no more active COLO users, at least based on
> the latest development of QEMU.
>
> I don't remember seeing anything really active in the past few years in
> COLO development.
>
> Meanwhile, COLO migration framework maintainer (Hailiang Zhang)'s last
> email to qemu-devel is in Dec 2021 where the patch proposed an email
> change (<20211214075424.6920-1-zhanghailiang@xfusion.com>).
>
> We've discussed this for a while, see latest discussions here (our thoughts
> of deprecating COLO framework might be earlier than that, but still):
>
> https://lore.kernel.org/r/aQu6bDAA7hnIPg-y@x1.local/
> https://lore.kernel.org/r/20251230-colo_unit_test_multifd-v1-0-f9734bc74c71@web.de
>
> Let's make it partly official and put COLO into deprecation list.  If
> anyone cares about COLO and is deploying it, please send an email to
> qemu-devel to discuss.
>
> Otherwise, let's try to save some energy for either maintainers or
> developers who is looking after QEMU. Let's save the work if we don't even
> know what the work is for.
>
> Cc: Lukáš Doktor <ldoktor@redhat.com>
> Cc: Juan Quintela <quintela@trasno.org>
> Cc: Dr. David Alan Gilbert <dave@treblig.org>
> Cc: Zhang Chen <zhangckid@gmail.com>
> Cc: zhanghailiang@xfusion.com
> Cc: Li Zhijian <lizhijian@fujitsu.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  docs/about/deprecated.rst | 6 ++++++
>  qapi/migration.json       | 5 ++---
>  migration/options.c       | 4 ++++
>  3 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 7abb3dab59..b499b2acb0 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -580,3 +580,9 @@ command documentation for details on the ``fdset`` usage.
>  
>  The ``zero-blocks`` capability was part of the block migration which
>  doesn't exist anymore since it was removed in QEMU v9.1.
> +
> +COLO migration framework (since 11.0)
> +'''''''''''''''''''''''''''''''''''''
> +
> +To be removed with no replacement, as the COLO migration framework doesn't
> +seem to have any active user for a while.
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 201dedd982..3c868efe38 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -531,8 +531,7 @@
>  #
>  # @unstable: Members @x-colo and @x-ignore-shared are experimental.
>  #
> -# @deprecated: Member @zero-blocks is deprecated as being part of
> -#     block migration which was already removed.
> +# @deprecated: Member @zero-blocks and @x-colo are deprecated.
>  #
>  # Since: 1.2
>  ##
> @@ -540,7 +539,7 @@
>    'data': ['xbzrle', 'rdma-pin-all', 'auto-converge',
>             { 'name': 'zero-blocks', 'features': [ 'deprecated' ] },
>             'events', 'postcopy-ram',
> -           { 'name': 'x-colo', 'features': [ 'unstable' ] },
> +           { 'name': 'x-colo', 'features': [ 'unstable', 'deprecated' ] },
>             'release-ram',
>             'return-path', 'pause-before-switchover', 'multifd',
>             'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',

Issues / doubts:

1. We delete the text why @zero-blocks is deprecated.  Harmless; the
next patch drops @zero-blocks entirely.  Better: swap the patches.

2. The text for @x-colo is lacking.  Suggest something like "Member
@x-colo" is deprecated without replacement."

3. Does it make sense to keep x-colo @unstable?

4. Shouldn't we mark *all* the COLO interfaces the same way?

> diff --git a/migration/options.c b/migration/options.c
> index 9a5a39c886..318850ba94 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -580,6 +580,10 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
>          warn_report("zero-blocks capability is deprecated");
>      }
>  
> +    if (new_caps[MIGRATION_CAPABILITY_X_COLO]) {
> +        warn_report("COLO migration framework is deprecated");
> +    }
> +
>  #ifndef CONFIG_REPLICATION
>      if (new_caps[MIGRATION_CAPABILITY_X_COLO]) {
>          error_setg(errp, "QEMU compiled without replication module"