> -----Original Message-----
> From: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
> Sent: Saturday, April 29, 2023 3:49 AM
> To: qemu-devel@nongnu.org
> Cc: lukasstra...@web.de; quint...@redhat.com; Zhang, Chen
> <chen.zh...@intel.com>; vsement...@yandex-team.ru; Peter Xu
> <pet...@redhat.com>; Leonardo Bras <leob...@redhat.com>
> Subject: [PATCH v4 09/10] migration: disallow change capabilities in COLO
> state
>
> COLO is not listed as running state in migrate_is_running(), so, it's
> theoretically possible to disable colo capability in COLO state and the
> unexpected error in migration_iteration_finish() is reachable.
>
> Let's disallow that in qmp_migrate_set_capabilities. Than the error becomes
> absolutely unreachable: we can get into COLO state only with enabled
> capability and can't disable it while we are in COLO state. So substitute the
> error by simple assertion.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
> ---
> migration/migration.c | 5 +----
> migration/options.c | 2 +-
> 2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c index
> 0d912ee0d7..8c5bbf3e94 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2751,10 +2751,7 @@ static void
> migration_iteration_finish(MigrationState *s)
> runstate_set(RUN_STATE_POSTMIGRATE);
> break;
> case MIGRATION_STATUS_COLO:
> - if (!migrate_colo()) {
> - error_report("%s: critical error: calling COLO code without "
> - "COLO enabled", __func__);
> - }
> + assert(migrate_colo());
> migrate_start_colo_process(s);
> s->vm_was_running = true;
> /* Fallthrough */
> diff --git a/migration/options.c b/migration/options.c index
> 865a0214d8..f461d02eeb 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -598,7 +598,7 @@ void
> qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
> MigrationCapabilityStatusList *cap;
> bool new_caps[MIGRATION_CAPABILITY__MAX];
>
> - if (migration_is_running(s->state)) {
> + if (migration_is_running(s->state) || migration_in_colo_state()) {
Make the "MIGRATION_STATUS_COLO" into the " migration_is_running()" is a better
way?
Like the "migration_is_setup_ot_active()".
Thanks
Chen
> error_setg(errp, QERR_MIGRATION_ACTIVE);
> return;
> }
> --
> 2.34.1
