Re: [Qemu-devel] [PATCH v5 23/24] replay: add BH oneshot event for block layer

[Alex Bennée]

Pavel Dovgalyuk <address@hidden> writes:

> Replay is capable of recording normal BH events, but sometimes
> there are single use callbacks scheduled with aio_bh_schedule_oneshot
> function. This patch enables recording and replaying such callbacks.
> Block layer uses these events for calling the completion function.
> Replaying these calls makes the execution deterministic.
>
> Signed-off-by: Pavel Dovgalyuk <address@hidden>
> ---
>  block/block-backend.c    |    3 ++-
>  include/sysemu/replay.h  |    3 +++
>  replay/replay-events.c   |   16 ++++++++++++++++
>  replay/replay-internal.h |    1 +
>  replay/replay.c          |    2 +-
>  stubs/replay.c           |    6 ++++++
>  6 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/block/block-backend.c b/block/block-backend.c
> index f2f75a9..232d114 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -17,6 +17,7 @@
>  #include "block/throttle-groups.h"
>  #include "sysemu/blockdev.h"
>  #include "sysemu/sysemu.h"
> +#include "sysemu/replay.h"
>  #include "qapi/error.h"
>  #include "qapi/qapi-events-block.h"
>  #include "qemu/id.h"
> @@ -1370,7 +1371,7 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, 
> int64_t offset, int bytes,
>
>      acb->has_returned = true;
>      if (acb->rwco.ret != NOT_DONE) {
> -        aio_bh_schedule_oneshot(blk_get_aio_context(blk),
> +        replay_bh_schedule_oneshot_event(blk_get_aio_context(blk),
>                                  blk_aio_complete_bh, acb);
>      }
>
> diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
> index 8118b00..945bc74 100644
> --- a/include/sysemu/replay.h
> +++ b/include/sysemu/replay.h
> @@ -152,6 +152,9 @@ bool replay_events_enabled(void);
>  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 */
> +void replay_bh_schedule_oneshot_event(AioContext *ctx,
> +    QEMUBHFunc *cb, void *opaque);
>  /*! Adds input event to the queue */
>  void replay_input_event(QemuConsole *src, InputEvent *evt);
>  /*! Adds input sync event to the queue */
> diff --git a/replay/replay-events.c b/replay/replay-events.c
> index 0964a82..0ac8a5c 100644
> --- a/replay/replay-events.c
> +++ b/replay/replay-events.c
> @@ -37,6 +37,9 @@ static void replay_run_event(Event *event)
>      case REPLAY_ASYNC_EVENT_BH:
>          aio_bh_call(event->opaque);
>          break;
> +    case REPLAY_ASYNC_EVENT_BH_ONESHOT:
> +        ((QEMUBHFunc*)event->opaque)(event->opaque2);
> +        break;
>      case REPLAY_ASYNC_EVENT_INPUT:
>          qemu_input_event_send_impl(NULL, (InputEvent *)event->opaque);
>          qapi_free_InputEvent((InputEvent *)event->opaque);
> @@ -132,6 +135,17 @@ void replay_bh_schedule_event(QEMUBH *bh)
>      }
>  }
>
> +void replay_bh_schedule_oneshot_event(AioContext *ctx,
> +    QEMUBHFunc *cb,void *opaque)
> +{
> +    if (events_enabled) {
> +        uint64_t id = replay_get_current_step();
> +        replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id);
> +    } else {
> +        aio_bh_schedule_oneshot(ctx, cb, opaque);
> +    }
> +}
> +
>  void replay_add_input_event(struct InputEvent *event)
>  {
>      replay_add_event(REPLAY_ASYNC_EVENT_INPUT, event, NULL, 0);
> @@ -162,6 +176,7 @@ static void replay_save_event(Event *event, int 
> checkpoint)
>          /* save event-specific data */
>          switch (event->event_kind) {
>          case REPLAY_ASYNC_EVENT_BH:
> +        case REPLAY_ASYNC_EVENT_BH_ONESHOT:
>              replay_put_qword(event->id);
>              break;
>          case REPLAY_ASYNC_EVENT_INPUT:
> @@ -216,6 +231,7 @@ static Event *replay_read_event(int checkpoint)
>      /* Events that has not to be in the queue */
>      switch (replay_state.read_event_kind) {
>      case REPLAY_ASYNC_EVENT_BH:
> +    case REPLAY_ASYNC_EVENT_BH_ONESHOT:
>          if (replay_state.read_event_id == -1) {
>              replay_state.read_event_id = replay_get_qword();
>          }
> diff --git a/replay/replay-internal.h b/replay/replay-internal.h
> index 08ef2ec..0c0ed16 100644
> --- a/replay/replay-internal.h
> +++ b/replay/replay-internal.h
> @@ -51,6 +51,7 @@ enum ReplayEvents {
>
>  enum ReplayAsyncEventKind {
>      REPLAY_ASYNC_EVENT_BH,
> +    REPLAY_ASYNC_EVENT_BH_ONESHOT,
>      REPLAY_ASYNC_EVENT_INPUT,
>      REPLAY_ASYNC_EVENT_INPUT_SYNC,
>      REPLAY_ASYNC_EVENT_CHAR_READ,
> diff --git a/replay/replay.c b/replay/replay.c
> index 6e82764..061b1e2 100644
> --- a/replay/replay.c
> +++ b/replay/replay.c
> @@ -22,7 +22,7 @@
>
>  /* Current version of the replay mechanism.
>     Increase it when file format changes. */
> -#define REPLAY_VERSION              0xe02007
> +#define REPLAY_VERSION              0xe02008
>  /* Size of replay log header */
>  #define HEADER_SIZE                 (sizeof(uint32_t) + sizeof(uint64_t))
>
> diff --git a/stubs/replay.c b/stubs/replay.c
> index 781974e..cbdac80 100644
> --- a/stubs/replay.c
> +++ b/stubs/replay.c
> @@ -90,3 +90,9 @@ bool replay_reverse_continue(void)
>  {
>      return false;
>  }
> +
> +void replay_bh_schedule_oneshot_event(AioContext *ctx,
> +    QEMUBHFunc *cb,void *opaque)
> +{
> +    aio_bh_schedule_oneshot(ctx, cb, opaque);
> +}

It seems wrong to have something in stubs that actively does stuff.
Isn't this meant to be a bunch of NOPs?

--
Alex Bennée