RE: [for-5.0 PATCH 08/11] replay: flush rr queue before loading the vmstate

[Pavel Dovgalyuk]

> From: Alex Bennée [mailto:address@hidden]
> Pavel Dovgalyuk <address@hidden> writes:
> 
> > From: Pavel Dovgalyuk <address@hidden>
> >
> > Non-empty record/replay queue prevents saving and loading the VM state,
> > because it includes pending bottom halves and block coroutines.
> > But when the new VM state is loaded, we don't have to preserve the 
> > consistency
> > of the current state anymore. Therefore this patch just flushes the queue
> > allowing the coroutines to finish and removes checking for empty rr queue
> > for load_snapshot function.
> >
> > Signed-off-by: Pavel Dovgalyuk <address@hidden>
> > ---
> >  include/sysemu/replay.h  |    2 ++
> >  migration/savevm.c       |   12 ++++++------
> >  replay/replay-internal.h |    2 --
> >  3 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
> > index e00ed2f4a5..239c01e7df 100644
> > --- a/include/sysemu/replay.h
> > +++ b/include/sysemu/replay.h
> > @@ -149,6 +149,8 @@ void replay_disable_events(void);
> >  void replay_enable_events(void);
> >  /*! Returns true when saving events is enabled */
> >  bool replay_events_enabled(void);
> > +/* Flushes events queue */
> > +void replay_flush_events(void);
> >  /*! Adds bottom half event to the queue */
> >  void replay_bh_schedule_event(QEMUBH *bh);
> >  /* Adds oneshot bottom half event to the queue */
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index ae84bf6ab0..0c5cac372a 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -2834,12 +2834,6 @@ int load_snapshot(const char *name, Error **errp)
> >      AioContext *aio_context;
> >      MigrationIncomingState *mis = migration_incoming_get_current();
> >
> > -    if (!replay_can_snapshot()) {
> > -        error_setg(errp, "Record/replay does not allow loading snapshot "
> > -                   "right now. Try once more later.");
> > -        return -EINVAL;
> > -    }
> > -
> >      if (!bdrv_all_can_snapshot(&bs)) {
> >          error_setg(errp,
> >                     "Device '%s' is writable but does not support 
> > snapshots",
> > @@ -2873,6 +2867,12 @@ int load_snapshot(const char *name, Error **errp)
> >          return -EINVAL;
> >      }
> >
> > +    /*
> > +     * Flush the record/replay queue. Now the VM state is going
> > +     * to change. Therefore we don't need to preserve its consistency
> > +     */
> > +    replay_flush_events();
> > +
> <snip>
> 
> This is the commit that introduces:
> 
>   
> ERROR:/home/alex.bennee/lsrc/qemu.git/replay/replay-events.c:80:replay_flush_events:
>   assertion failed: (replay_mutex_locked())
> 
> To the already failing:
> 
>   /bin/sh -c "cd builds/all && make -j4 && cd tests/qemu-iotests && ./check 
> -qcow2 267"
> 
> test case.

Please apply the following update to continue the testing:

--- a/replay/replay-events.c
+++ b/replay/replay-events.c
@@ -77,6 +77,10 @@ bool replay_has_events(void)
 
 void replay_flush_events(void)
 {
+    if (replay_mode == REPLAY_MODE_NONE) {
+        return;
+    }
+

Pavel Dovgalyuk