diff mbox series

[v6,22/25] replay: add BH oneshot event for block layer

Message ID 20180912081956.3228.17903.stgit@pasha-VirtualBox
State New
Headers show
Series Fixing record/replay and adding reverse debugging | expand

Commit Message

Pavel Dovgalyuk Sept. 12, 2018, 8:19 a.m. UTC 
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 <Pavel.Dovgaluk@ispras.ru>

--

v6:
 - moved stub function to the separate file for fixing linux-user build
---
 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/Makefile.objs      |    1 +
 stubs/replay-user.c      |    9 +++++++++
 7 files changed, 33 insertions(+), 2 deletions(-)
 create mode 100644 stubs/replay-user.c

Comments

Paolo Bonzini Sept. 13, 2018, 10:22 a.m. UTC | #1 
On 12/09/2018 10:19, Pavel Dovgalyuk wrote:
> + uint64_t id = replay_get_current_step();
> + replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id);

Why does it need an id, while REPLAY_ASYNC_EVENT_BH does not?

Paolo
Pavel Dovgalyuk Sept. 13, 2018, 11:13 a.m. UTC | #2 
> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> On 12/09/2018 10:19, Pavel Dovgalyuk wrote:
> > + uint64_t id = replay_get_current_step();
> > + replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id);
> 
> Why does it need an id, while REPLAY_ASYNC_EVENT_BH does not?

Because _oneshot() function takes only context and callback,
and BH object is created inside that function.
Therefore to use the normal bh scheduling events we have to refactor
block code - add bh creation, scheduling, and freeing.

Pavel Dovgalyuk
Paolo Bonzini Sept. 13, 2018, 1:45 p.m. UTC | #3 
On 13/09/2018 13:13, Pavel Dovgalyuk wrote:
>> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
>> On 12/09/2018 10:19, Pavel Dovgalyuk wrote:
>>> + uint64_t id = replay_get_current_step();
>>> + replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id);
>>
>> Why does it need an id, while REPLAY_ASYNC_EVENT_BH does not?
> 
> Because _oneshot() function takes only context and callback,
> and BH object is created inside that function.
> Therefore to use the normal bh scheduling events we have to refactor
> block code - add bh creation, scheduling, and freeing.

No, I'm asking only why the id is 0 for REPLAY_ASYNC_EVENT_BH and
replay_get_current_step() for REPLAY_ASYNC_EVENT_BH_ONESHOT.

Thanks,

Paolo
Pavel Dovgalyuk Sept. 14, 2018, 6:17 a.m. UTC | #4 
> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> On 13/09/2018 13:13, Pavel Dovgalyuk wrote:
> >> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> >> On 12/09/2018 10:19, Pavel Dovgalyuk wrote:
> >>> + uint64_t id = replay_get_current_step();
> >>> + replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id);
> >>
> >> Why does it need an id, while REPLAY_ASYNC_EVENT_BH does not?
> >
> > Because _oneshot() function takes only context and callback,
> > and BH object is created inside that function.
> > Therefore to use the normal bh scheduling events we have to refactor
> > block code - add bh creation, scheduling, and freeing.
> 
> No, I'm asking only why the id is 0 for REPLAY_ASYNC_EVENT_BH and
> replay_get_current_step() for REPLAY_ASYNC_EVENT_BH_ONESHOT.
> 
> Thanks,
> 
> Paolo

Sorry, I missed your point.
Maybe you looked at the wrong function, because both of these events have an id:

void replay_bh_schedule_event(QEMUBH *bh)
{
    if (events_enabled) {
        uint64_t id = replay_get_current_step();
        replay_add_event(REPLAY_ASYNC_EVENT_BH, bh, NULL, id);
    } else {
        qemu_bh_schedule(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);
    }
}

Pavel Dovgalyuk
diff mbox series

Patch

diff --git a/block/block-backend.c b/block/block-backend.c
index fa12063..712bafd 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"
@@ -1369,7 +1370,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 3130e45..9da90e1 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -160,6 +160,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..83a7d81 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 a857ca4..548d4d8 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 2145686..355e5ae 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/Makefile.objs b/stubs/Makefile.objs
index 53d3f32..2acbac3 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -24,6 +24,7 @@  stub-obj-y += monitor.o
 stub-obj-y += notify-event.o
 stub-obj-y += qtest.o
 stub-obj-y += replay.o
+stub-obj-y += replay-user.o
 stub-obj-y += runstate-check.o
 stub-obj-y += set-fd-handler.o
 stub-obj-y += slirp.o
diff --git a/stubs/replay-user.c b/stubs/replay-user.c
new file mode 100644
index 0000000..2ad9e27
--- /dev/null
+++ b/stubs/replay-user.c
@@ -0,0 +1,9 @@ 
+#include "qemu/osdep.h"
+#include "sysemu/replay.h"
+#include "sysemu/sysemu.h"
+
+void replay_bh_schedule_oneshot_event(AioContext *ctx,
+    QEMUBHFunc *cb, void *opaque)
+{
+    aio_bh_schedule_oneshot(ctx, cb, opaque);
+}