Re: [PATCH] replay: synchronize on every virtual timer callback

[Paolo Bonzini]

On 06/05/20 10:17, Pavel Dovgalyuk wrote:
> Sometimes virtual timer callbacks depend on order
> of virtual timer processing and warping of virtual clock.
> Therefore every callback should be logged to make replay deterministic.
> This patch creates a checkpoint before every virtual timer callback.
> With these checkpoints virtual timers processing and clock warping
> events order is completely deterministic.
> 
> Signed-off-by: Pavel Dovgalyuk <address@hidden>
> ---
>  util/qemu-timer.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/util/qemu-timer.c b/util/qemu-timer.c
> index d548d3c1ad..47833f338f 100644
> --- a/util/qemu-timer.c
> +++ b/util/qemu-timer.c
> @@ -588,6 +588,11 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
>          qemu_mutex_lock(&timer_list->active_timers_lock);
>  
>          progress = true;
> +        /*
> +         * Callback may insert new checkpoints, therefore add new checkpoint
> +         * for the virtual timers.
> +         */
> +        need_replay_checkpoint = timer_list->clock->type == 
> QEMU_CLOCK_VIRTUAL;

You need to check replay_mode != REPLAY_MODE_NONE, either here or in the
"if (need_replay_checkpoint)" above.  If you choose the latter, you can
remove the other "if (replay_mode != REPLAY_MODE_NONE)".

Also, there is a comment that says that checkpointing "must only be done
once since the clock value stays the same".  Is that actually a "can"
rather than a "must"?  Should the central replay logic have something
like a checkpoint count, that prevents adding back-to-back equal
checkpoints?

Thanks,

Paolo

>      }
>      qemu_mutex_unlock(&timer_list->active_timers_lock);
>  
>