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

[Pavel Dovgalyuk]

Ping?

Pavel Dovgalyuk

> -----Original Message-----
> From: Pavel Dovgalyuk [mailto:address@hidden
> Sent: Monday, June 20, 2016 9:27 AM
> To: 'Paolo Bonzini'; 'Pavel Dovgalyuk'
> Cc: address@hidden; address@hidden; address@hidden; address@hidden
> Subject: RE: [PATCH 2/3] replay: allow replay stopping and restarting
> 
> > From: Paolo Bonzini [mailto:address@hidden
> > > From: "Pavel Dovgalyuk" <address@hidden>
> > > This patch fixes bug with stopping and restarting replay
> > > through monitor.
> > >
> > > Signed-off-by: Pavel Dovgalyuk <address@hidden>
> > > ---
> > >  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