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

[Pavel Dovgalyuk]

On 21.05.2020 16:22, Paolo Bonzini wrote:
> 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)".

I forgot about the changes that prohibited event processing for the 
virtual clock checkpoint.

This allowed to make this part simpler, please check the new version.

However, event processing still waits for refactoring. I'll do it after 
upstreaming the tests for the record/replay to prevent regression.


> 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?

I don't really think that this happens very often, but it worth 
implementing. I'll try it later.


Pavel Dovgalyuk