Re: [Qemu-devel] [PATCH v6 21/25] replay: replay BH for IDE trim operation

[Pavel Dovgalyuk]

> From: John Snow [mailto:address@hidden
> On 09/12/2018 04:19 AM, Pavel Dovgalyuk wrote:
> > This patch makes IDE trim BH deterministic, because it affects
> > the device state. Therefore its invocation should be replayed
> > instead of running at the random moment.
> >
> > Signed-off-by: Pavel Dovgalyuk <address@hidden>
> > Reviewed-by: Paolo Bonzini <address@hidden>
> > ---
> >  hw/ide/core.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/ide/core.c b/hw/ide/core.c
> > index 2c62efc..04e22e7 100644
> > --- a/hw/ide/core.c
> > +++ b/hw/ide/core.c
> > @@ -35,6 +35,7 @@
> >  #include "sysemu/block-backend.h"
> >  #include "qapi/error.h"
> >  #include "qemu/cutils.h"
> > +#include "sysemu/replay.h"
> >
> >  #include "hw/ide/internal.h"
> >  #include "trace.h"
> > @@ -479,7 +480,7 @@ static void ide_issue_trim_cb(void *opaque, int ret)
> >  done:
> >      iocb->aiocb = NULL;
> >      if (iocb->bh) {
> > -        qemu_bh_schedule(iocb->bh);
> > +        replay_bh_schedule_event(iocb->bh);
> >      }
> >  }
> >
> Just passing by: Why do we need to change this call, but nothing else in
> IDE?

This call is responsible for a bug that was reproducible.

> I don't mind conceptually, but it's odd to me that of all the calls I
> make in this emulator that change state somewhere that this is the only
> one you need to hijack for the replay feature.
> 
> Is this a necessarily complete change?

Maybe not. We can hardly analyze all peripheral devices code and fix all the 
calls.
But I think we can improve that patch and at least look through ide core to fix 
other calls.

Pavel Dovgalyuk