Re: [Qemu-devel] [PATCH 2/3] replay: allow replay stopping and restarting

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

> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> > From: "Pavel Dovgalyuk" <pavel.dovgaluk@ispras.ru>
> > This patch fixes bug with stopping and restarting replay
> > through monitor.
> >
> > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> > ---
> >  block/blkreplay.c        |   18 +++++++++++++-----
> >  cpus.c                   |    1 +
> >  include/sysemu/replay.h  |    2 ++
> >  replay/replay-internal.h |    2 --
> >  vl.c                     |    1 +
> >  5 files changed, 17 insertions(+), 7 deletions(-)
> >
> > diff --git a/block/blkreplay.c b/block/blkreplay.c
> > index 42f1813..438170c 100644
> > --- a/block/blkreplay.c
> > +++ b/block/blkreplay.c
> > @@ -70,6 +70,14 @@ static void blkreplay_bh_cb(void *opaque)
> >      g_free(req);
> >  }
> >
> > +static uint64_t blkreplay_next_id(void)
> > +{
> > +    if (replay_events_enabled()) {
> > +        return request_id++;
> > +    }
> > +    return 0;
> > +}
> 
> What happens if 0 is returned?  

It could be any value. When replay events are disables,
it means that either replay is disabled or execution is stopped.
In first case we won't pass this requests through the replay queue
and therefore id is useless.
In stopped mode we have to keep request_id unchanged to make
record/replay deterministic.

> I think that you want to call
> replay_disable_events...
> 
> >      bdrv_drain_all();
> 
> ... after this bdrv_drain_all.

Why? We disable replay events to avoid adding new block requests
to the queue. How this is related to drain all?

> 
> I was going to suggest using qemu_add_vm_change_state_handler
> in replay_start (which could have replaced the existing call
> to replay_enable_events), but that's not possible if you have
> to do your calls after bdrv_drain_all.

Pavel Dovgalyuk