Re: [PATCH v5 06/16] colo: Don't send ENABLE_COLO command

[Peter Xu <peterx@redhat.com>]

On Tue, Feb 03, 2026 at 11:15:12AM +0100, Lukas Straub wrote:
> We only support COLO with the same version on both sides so this is
> not needed anymore.
> 
> Reviewed-by: Fabiano Rosas <farosas@suse.de>
> Signed-off-by: Lukas Straub <lukasstraub2@web.de>

Besides squashing with prior patch, another nitpick inline:

> ---
>  migration/migration.c  | 5 -----
>  migration/savevm.c     | 8 +-------
>  migration/savevm.h     | 1 -
>  migration/trace-events | 1 -
>  4 files changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 3f3fc5276bb067ae1960e4b675b33208ad641b23..5515be1bf305b40ba0b590136df18a53451872c5 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3496,11 +3496,6 @@ static void *migration_thread(void *opaque)
>          qemu_savevm_send_postcopy_advise(s->to_dst_file);
>      }
>  
> -    if (migrate_colo()) {
> -        /* Notify migration destination that we enable COLO */
> -        qemu_savevm_send_colo_enable(s->to_dst_file);
> -    }
> -
>      if (migrate_auto_converge()) {
>          /* Start RAMBlock dirty bitmap sync timer */
>          cpu_throttle_dirty_sync_timer(true);
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 413688b75f4bee6cb10878eb51886cf6ba14872d..a3af09616a7bd22194ffba3cfb7cc4cf15fc88e0 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -90,7 +90,7 @@ enum qemu_vm_cmd {
>                                        were previously sent during
>                                        precopy but are dirty. */
>      MIG_CMD_PACKAGED,          /* Send a wrapped stream within this stream */
> -    MIG_CMD_ENABLE_COLO,       /* Enable COLO */
> +    MIG_CMD_UNUSED_0,          /* Unused since 11.0 */

IMHO it's not "when unused" that matters, but "when it was used, and used
as what" that matters.  E.g. if we received this unused command in some
future QEMU debugging session, we can guess where it came from with that info.

Hence, I'd suggest:

       MIG_CMD_DEPRECATED_0,      /* Prior to 10.2, used as MIG_CMD_ENABLE_COLO */

I still think DEPRECATED is better here, as it reminds people we shouldn't
"reuse" it and better be left untouched to catch surprises, where "UNSED"
may imply "you can use it now".

Other than that looks all good, thanks.

>      MIG_CMD_POSTCOPY_RESUME,   /* resume postcopy on dest */
>      MIG_CMD_RECV_BITMAP,       /* Request for recved bitmap on dst */
>      MIG_CMD_SWITCHOVER_START,  /* Switchover start notification */
> @@ -1092,12 +1092,6 @@ static void qemu_savevm_command_send(QEMUFile *f,
>      qemu_fflush(f);
>  }
>  
> -void qemu_savevm_send_colo_enable(QEMUFile *f)
> -{
> -    trace_savevm_send_colo_enable();
> -    qemu_savevm_command_send(f, MIG_CMD_ENABLE_COLO, 0, NULL);
> -}
> -
>  void qemu_savevm_send_ping(QEMUFile *f, uint32_t value)
>  {
>      uint32_t buf;
> diff --git a/migration/savevm.h b/migration/savevm.h
> index 125a2507b7279412bcb0745b95a774874c31c54f..0a1e5bfd1ca125565a4c90c6f31b2f8c94404117 100644
> --- a/migration/savevm.h
> +++ b/migration/savevm.h
> @@ -62,7 +62,6 @@ void qemu_savevm_send_postcopy_ram_discard(QEMUFile *f, const char *name,
>                                             uint16_t len,
>                                             uint64_t *start_list,
>                                             uint64_t *length_list);
> -void qemu_savevm_send_colo_enable(QEMUFile *f);
>  void qemu_savevm_live_state(QEMUFile *f);
>  int qemu_save_device_state(QEMUFile *f);
>  
> diff --git a/migration/trace-events b/migration/trace-events
> index 91d7506634c9f110e8f0b5f9183728058fe6542a..cfd4d58a0f82ec299ca9e8a9260dd3c3a210cece 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -37,7 +37,6 @@ savevm_send_ping(uint32_t val) "0x%x"
>  savevm_send_postcopy_listen(void) ""
>  savevm_send_postcopy_run(void) ""
>  savevm_send_postcopy_resume(void) ""
> -savevm_send_colo_enable(void) ""
>  savevm_send_recv_bitmap(char *name) "%s"
>  savevm_send_switchover_start(void) ""
>  savevm_state_setup(void) ""
> 
> -- 
> 2.39.5
> 

-- 
Peter Xu