> -----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; Zhang, Hailiang
> <zhanghaili...@xfusion.com>; Peter Xu <pet...@redhat.com>; Leonardo
> Bras <leob...@redhat.com>
> Subject: [PATCH v4 02/10] colo: make colo_checkpoint_notify static and
> provide simpler API
>
> colo_checkpoint_notify() is mostly used in colo.c. Outside we use it once
> when x-checkpoint-delay migration parameter is set. So, let's simplify the
> external API to only that function - notify COLO that parameter was set. This
> make external API more robust and hides implementation details from
> external callers. Also this helps us to make COLO module optional in further
> patch (i.e. we are going to add possibility not build the COLO module).
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
Reviewed-by: Zhang Chen <chen.zh...@intel.com>
Thanks
Chen
> ---
> include/migration/colo.h | 9 ++++++++-
> migration/colo.c | 29 ++++++++++++++++++-----------
> migration/options.c | 4 +---
> 3 files changed, 27 insertions(+), 15 deletions(-)
>
> diff --git a/include/migration/colo.h b/include/migration/colo.h index
> 5fbe1a6d5d..7ef315473e 100644
> --- a/include/migration/colo.h
> +++ b/include/migration/colo.h
> @@ -36,6 +36,13 @@ COLOMode get_colo_mode(void);
> /* failover */
> void colo_do_failover(void);
>
> -void colo_checkpoint_notify(void *opaque);
> +/*
> + * colo_checkpoint_delay_set
> + *
> + * Handles change of x-checkpoint-delay migration parameter, called
> +from
> + * migrate_params_apply() to notify COLO module about the change.
> + */
> +void colo_checkpoint_delay_set(void);
> +
> void colo_shutdown(void);
> #endif
> diff --git a/migration/colo.c b/migration/colo.c index
> 07bfa21fea..c9e0b909b9 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -65,6 +65,24 @@ static bool colo_runstate_is_stopped(void)
> return runstate_check(RUN_STATE_COLO) || !runstate_is_running(); }
>
> +static void colo_checkpoint_notify(void *opaque) {
> + MigrationState *s = opaque;
> + int64_t next_notify_time;
> +
> + qemu_event_set(&s->colo_checkpoint_event);
> + s->colo_checkpoint_time = qemu_clock_get_ms(QEMU_CLOCK_HOST);
> + next_notify_time = s->colo_checkpoint_time +
> migrate_checkpoint_delay();
> + timer_mod(s->colo_delay_timer, next_notify_time); }
> +
> +void colo_checkpoint_delay_set(void)
> +{
> + if (migration_in_colo_state()) {
> + colo_checkpoint_notify(migrate_get_current());
> + }
> +}
> +
> static void secondary_vm_do_failover(void) {
> /* COLO needs enable block-replication */ @@ -644,17 +662,6 @@ out:
> }
> }
>
> -void colo_checkpoint_notify(void *opaque) -{
> - MigrationState *s = opaque;
> - int64_t next_notify_time;
> -
> - qemu_event_set(&s->colo_checkpoint_event);
> - s->colo_checkpoint_time = qemu_clock_get_ms(QEMU_CLOCK_HOST);
> - next_notify_time = s->colo_checkpoint_time +
> migrate_checkpoint_delay();
> - timer_mod(s->colo_delay_timer, next_notify_time);
> -}
> -
> void migrate_start_colo_process(MigrationState *s) {
> qemu_mutex_unlock_iothread();
> diff --git a/migration/options.c b/migration/options.c index
> 53b7fc5d5d..865a0214d8 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -1246,9 +1246,7 @@ static void
> migrate_params_apply(MigrateSetParameters *params, Error **errp)
>
> if (params->has_x_checkpoint_delay) {
> s->parameters.x_checkpoint_delay = params->x_checkpoint_delay;
> - if (migration_in_colo_state()) {
> - colo_checkpoint_notify(s);
> - }
> + colo_checkpoint_delay_set();
> }
>
> if (params->has_block_incremental) {
> --
> 2.34.1
