Re: [Qemu-devel] [PATCH v3 0/4] Introduce attributes for timers subsystem and remove QEMU_CLOCK_VIRTUAL_EXT clock type

["Pavel Dovgalyuk" <dovgaluk@ispras.ru>]

> From: Artem Pisarenko [mailto:artem.k.pisarenko@gmail.com]
> ср, 16 янв. 2019 г. в 12:15, Pavel Dovgalyuk <dovgaluk@ispras.ru>:
> >
> > > From: Artem Pisarenko [mailto:artem.k.pisarenko@gmail.com]
> > > > It seems, that this approach is not always correct.
> > > > Now timerlist_deadline_ns uses all virtual timers for deadline calculation (including
> > > external
> > > > ones).
> > > > qemu_start_warp_timer uses the deadline for setting warp timer (which should be
> > > deterministic).
> > > > Therefore warp timer may become non-deterministic and replay may behave differently
> compared
> > > to
> > > > recording phase.
> > > >
> > > > We have to rollback these or improve somehow to avoid non-determinism.
> > >
> > > I dont understand how this approach would even introduce
> > > non-determinism. I'm not sure about aspects of timers subsystem you
> > > mentioned, assuming that maybe we missed something when tried to
> > > optimize. But this has nothing to do with determinism as long as we
> > > treat all virtual clock timers as deterministic, regardless of
> > > EXTERNAL attribute being set or not. They are intended to be used as
> > > such by design, aren't? This attribute was introduced purely to avoid
> > > extra events in log.
> >
> > Right, but deadline calculation and icount warp events (that are written into the log too)
> > depend on the active virtual timers. Therefore external ones may affect icount warp
> > operation sequence.
> >
> > Pavel Dovgalyuk
> >
> 
> Sorry, but I still don't understand the source of non-determinism.

The source is the external timers, that depend on "outer world", e.g., slirp.
These timers are not recorded/replayed, but may affect the icount warping
(which should remain deterministic).

> From what you said I may understand that replay module actually has
> some additional non-trivial (for me) dependencies on EXTERNAL
> attribute other than one, introduced in 'timerlist_run_timers()' (i.e.
> it expects deadline calculations somewhere else to match decisions
> made in this function, or something like that). I would agree that
> these patch series might introduce some incorrect behavior. But it
> must be identical in all emulations running in same conditions,
> because deadline, warp timer and their effects are all dependent only
> on virtual timers, and, therefore, are deterministic too. Of course,
> it needs to be fixed. Did you find solution ?

Please check the new version of the series.
Patch 22

Pavel Dovgalyuk