[Qemu-devel] [PATCH v2 0/4] Deterministic replay extensions

[Qemu-devel] [PATCH v2 0/4] Deterministic replay extensions

[Pavel Dovgalyuk]

This set of patches is related to the reverse execution and deterministic 
replay of qemu execution. It includes recording and replaying of serial devices
and block devices operations.

With these patches one can record and deterministically replay behavior
of the system with connected disk drives and serial communication ports
(e.g., telnet terminal).

Patches for deterministic replay of the block devices intercept calls of
blk_aio_ functions. These functions are used only by virtual devices.
Replay cannot be embedded as block driver functions layer, because these
functions are also used for qemu internal operations (e.g., migration).

v2 changes:
 - removed obsolete call of qemu_clock_warp
 - fixed record/replay of aio_cancel
 - simplified call sequence for blk_aio_ functions in non-replay mode (as suggested by Kevin Wolf)

---

Pavel Dovgalyuk (4):
      replay: character devices
      icount: remove obsolete warp call
      replay: introduce new checkpoint for icount warp
      replay: introduce block devices record/replay


 block/block-backend.c    |   54 ++++++++++++--
 cpus.c                   |   10 +--
 gdbstub.c                |    2 -
 include/qemu/timer.h     |    3 +
 include/sysemu/char.h    |   26 +++++++
 include/sysemu/replay.h  |   43 ++++++++++++
 main-loop.c              |    2 -
 qemu-char.c              |   56 +++++++++++++--
 qemu-timer.c             |    2 -
 replay/Makefile.objs     |    2 +
 replay/replay-block.c    |  172 ++++++++++++++++++++++++++++++++++++++++++++++
 replay/replay-char.c     |   97 ++++++++++++++++++++++++++
 replay/replay-events.c   |   68 +++++++++++++++++-
 replay/replay-internal.h |   39 ++++++++++
 replay/replay.c          |   25 ++++++-
 stubs/clock-warp.c       |    2 -
 stubs/replay.c           |   50 +++++++++++++
 17 files changed, 622 insertions(+), 31 deletions(-)
 create mode 100755 replay/replay-block.c
 create mode 100755 replay/replay-char.c

-- 
Pavel Dovgalyuk

[Qemu-devel] [PATCH v2 1/4] replay: character devices

[Pavel Dovgalyuk]

This patch implements record and replay of character devices.
It records chardevs communication in replay mode. Recorded information
include data read from backend and counter of bytes written
from frontend to backend to preserve frontend internal state.
If character device was configured through the command line in record mode,
then in replay mode it should be also added to command line. Backend of
the character device could be changed in replay mode.
Replaying of devices that perform ioctl and get_msgfd operations is not
supported.
gdbstub which also acts as a backend is not recorded to allow controlling
the replaying through gdb.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 gdbstub.c                |    2 -
 include/sysemu/char.h    |   26 ++++++++++++
 include/sysemu/replay.h  |   12 ++++++
 qemu-char.c              |   56 ++++++++++++++++++++++++---
 replay/Makefile.objs     |    1 
 replay/replay-char.c     |   97 ++++++++++++++++++++++++++++++++++++++++++++++
 replay/replay-events.c   |   17 +++++++-
 replay/replay-internal.h |   15 +++++++
 replay/replay.c          |   25 +++++++++++-
 9 files changed, 240 insertions(+), 11 deletions(-)
 create mode 100755 replay/replay-char.c

diff --git a/gdbstub.c b/gdbstub.c
index 61c12b1..fdcb0ee 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1752,7 +1752,7 @@ int gdbserver_start(const char *device)
             sigaction(SIGINT, &act, NULL);
         }
 #endif
-        chr = qemu_chr_new("gdb", device, NULL);
+        chr = qemu_chr_new_noreplay("gdb", device, NULL);
         if (!chr)
             return -1;
 
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index e035d1c..03fc165 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -86,6 +86,7 @@ struct CharDriverState {
     int is_mux;
     guint fd_in_tag;
     QemuOpts *opts;
+    bool replay;
     QTAILQ_ENTRY(CharDriverState) next;
 };
 
@@ -129,6 +130,22 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename,
                               void (*init)(struct CharDriverState *s));
 
 /**
+ * @qemu_chr_new_noreplay:
+ *
+ * Create a new character backend from a URI.
+ * Character device communications are not written
+ * into the replay log.
+ *
+ * @label the name of the backend
+ * @filename the URI
+ * @init not sure..
+ *
+ * Returns: a new character backend
+ */
+CharDriverState *qemu_chr_new_noreplay(const char *label, const char *filename,
+                                       void (*init)(struct CharDriverState *s));
+
+/**
  * @qemu_chr_delete:
  *
  * Destroy a character backend and remove it from the list of
@@ -331,6 +348,15 @@ int qemu_chr_be_can_write(CharDriverState *s);
  */
 void qemu_chr_be_write(CharDriverState *s, uint8_t *buf, int len);
 
+/**
+ * @qemu_chr_be_write_impl:
+ *
+ * Implementation of back end writing. Used by replay module.
+ *
+ * @buf a buffer to receive data from the front end
+ * @len the number of bytes to receive from the front end
+ */
+void qemu_chr_be_write_impl(CharDriverState *s, uint8_t *buf, int len);
 
 /**
  * @qemu_chr_be_event:
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index abb4688..3c4a988 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -117,4 +117,16 @@ void replay_input_event(QemuConsole *src, InputEvent *evt);
 /*! Adds input sync event to the queue */
 void replay_input_sync_event(void);
 
+/* Character device */
+
+/*! Registers char driver to save it's events */
+void replay_register_char_driver(struct CharDriverState *chr);
+/*! Saves write to char device event to the log */
+void replay_chr_be_write(struct CharDriverState *s, uint8_t *buf, int len);
+
+/* Other data */
+
+/*! Writes or reads integer value to/from replay log. */
+void replay_data_int(int *data);
+
 #endif
diff --git a/qemu-char.c b/qemu-char.c
index 927c47e..e285f9f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -37,6 +37,7 @@
 #include "io/channel-socket.h"
 #include "io/channel-file.h"
 #include "io/channel-tls.h"
+#include "sysemu/replay.h"
 
 #include <zlib.h>
 
@@ -245,6 +246,9 @@ int qemu_chr_fe_write(CharDriverState *s, const uint8_t *buf, int len)
         qemu_chr_fe_write_log(s, buf, ret);
     }
 
+    if (s->replay) {
+        replay_data_int(&ret);
+    }
     qemu_mutex_unlock(&s->chr_write_lock);
     return ret;
 }
@@ -318,9 +322,19 @@ int qemu_chr_fe_read_all(CharDriverState *s, uint8_t *buf, int len)
 
 int qemu_chr_fe_ioctl(CharDriverState *s, int cmd, void *arg)
 {
-    if (!s->chr_ioctl)
-        return -ENOTSUP;
-    return s->chr_ioctl(s, cmd, arg);
+    int res;
+    if (!s->chr_ioctl) {
+        res = -ENOTSUP;
+    } else {
+        res = s->chr_ioctl(s, cmd, arg);
+        if (s->replay) {
+            fprintf(stderr,
+                    "Replay: ioctl is not supported for serial devices yet\n");
+            exit(1);
+        }
+    }
+
+    return res;
 }
 
 int qemu_chr_be_can_write(CharDriverState *s)
@@ -330,17 +344,35 @@ int qemu_chr_be_can_write(CharDriverState *s)
     return s->chr_can_read(s->handler_opaque);
 }
 
-void qemu_chr_be_write(CharDriverState *s, uint8_t *buf, int len)
+void qemu_chr_be_write_impl(CharDriverState *s, uint8_t *buf, int len)
 {
     if (s->chr_read) {
         s->chr_read(s->handler_opaque, buf, len);
     }
 }
 
+void qemu_chr_be_write(CharDriverState *s, uint8_t *buf, int len)
+{
+    if (s->replay) {
+        if (replay_mode == REPLAY_MODE_PLAY) {
+            return;
+        }
+        replay_chr_be_write(s, buf, len);
+    } else {
+        qemu_chr_be_write_impl(s, buf, len);
+    }
+}
+
 int qemu_chr_fe_get_msgfd(CharDriverState *s)
 {
     int fd;
-    return (qemu_chr_fe_get_msgfds(s, &fd, 1) == 1) ? fd : -1;
+    int res = (qemu_chr_fe_get_msgfds(s, &fd, 1) == 1) ? fd : -1;
+    if (s->replay) {
+        fprintf(stderr,
+                "Replay: get msgfd is not supported for serial devices yet\n");
+        exit(1);
+    }
+    return res;
 }
 
 int qemu_chr_fe_get_msgfds(CharDriverState *s, int *fds, int len)
@@ -3834,7 +3866,8 @@ err:
     return NULL;
 }
 
-CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*init)(struct CharDriverState *s))
+CharDriverState *qemu_chr_new_noreplay(const char *label, const char *filename,
+                                       void (*init)(struct CharDriverState *s))
 {
     const char *p;
     CharDriverState *chr;
@@ -3860,6 +3893,17 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in
     return chr;
 }
 
+CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*init)(struct CharDriverState *s))
+{
+    CharDriverState *chr;
+    chr = qemu_chr_new_noreplay(label, filename, init);
+    if (chr) {
+        chr->replay = replay_mode != REPLAY_MODE_NONE;
+        replay_register_char_driver(chr);
+    }
+    return chr;
+}
+
 void qemu_chr_fe_set_echo(struct CharDriverState *chr, bool echo)
 {
     if (chr->chr_set_echo) {
diff --git a/replay/Makefile.objs b/replay/Makefile.objs
index 232193a..70e5572 100644
--- a/replay/Makefile.objs
+++ b/replay/Makefile.objs
@@ -3,3 +3,4 @@ common-obj-y += replay-internal.o
 common-obj-y += replay-events.o
 common-obj-y += replay-time.o
 common-obj-y += replay-input.o
+common-obj-y += replay-char.o
diff --git a/replay/replay-char.c b/replay/replay-char.c
new file mode 100755
index 0000000..78bd3be
--- /dev/null
+++ b/replay/replay-char.c
@@ -0,0 +1,97 @@
+/*
+ * replay-char.c
+ *
+ * Copyright (c) 2010-2016 Institute for System Programming
+ *                         of the Russian Academy of Sciences.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "sysemu/replay.h"
+#include "replay-internal.h"
+#include "sysemu/sysemu.h"
+#include "sysemu/char.h"
+
+/* Char drivers that generate qemu_chr_be_write events
+   that should be saved into the log. */
+static CharDriverState **char_drivers;
+static int drivers_count;
+
+/* Char event attributes. */
+typedef struct CharEvent {
+    int id;
+    uint8_t *buf;
+    size_t len;
+} CharEvent;
+
+static int find_char_driver(CharDriverState *chr)
+{
+    int i = 0;
+    for ( ; i < drivers_count ; ++i) {
+        if (char_drivers[i] == chr) {
+            return i;
+        }
+    }
+    return -1;
+}
+
+void replay_register_char_driver(CharDriverState *chr)
+{
+    if (replay_mode == REPLAY_MODE_NONE) {
+        return;
+    }
+    char_drivers = g_realloc(char_drivers,
+                             sizeof(*char_drivers) * (drivers_count + 1));
+    char_drivers[drivers_count++] = chr;
+}
+
+void replay_chr_be_write(CharDriverState *s, uint8_t *buf, int len)
+{
+    CharEvent *event = g_malloc0(sizeof(CharEvent));
+
+    event->id = find_char_driver(s);
+    if (event->id < 0) {
+        fprintf(stderr, "Replay: cannot find char driver\n");
+        exit(1);
+    }
+    event->buf = g_malloc(len);
+    memcpy(event->buf, buf, len);
+    event->len = len;
+
+    replay_add_event(REPLAY_ASYNC_EVENT_CHAR, event, NULL, 0);
+}
+
+void replay_event_char_run(void *opaque)
+{
+    CharEvent *event = (CharEvent *)opaque;
+
+    qemu_chr_be_write_impl(char_drivers[event->id], event->buf,
+                           (int)event->len);
+
+    g_free(event->buf);
+    g_free(event);
+}
+
+void replay_event_char_save(void *opaque)
+{
+    CharEvent *event = (CharEvent *)opaque;
+
+    replay_put_byte(event->id);
+    replay_put_array(event->buf, event->len);
+}
+
+void *replay_event_char_read(void)
+{
+    CharEvent *event = g_malloc0(sizeof(CharEvent));
+
+    event->id = replay_get_byte();
+    replay_get_array_alloc(&event->buf, &event->len);
+
+    return event;
+}
diff --git a/replay/replay-events.c b/replay/replay-events.c
index 2628109..59d467f 100644
--- a/replay/replay-events.c
+++ b/replay/replay-events.c
@@ -48,6 +48,9 @@ static void replay_run_event(Event *event)
     case REPLAY_ASYNC_EVENT_INPUT_SYNC:
         qemu_input_event_sync_impl();
         break;
+    case REPLAY_ASYNC_EVENT_CHAR:
+        replay_event_char_run(event->opaque);
+        break;
     default:
         error_report("Replay: invalid async event ID (%d) in the queue",
                     event->event_kind);
@@ -102,9 +105,9 @@ void replay_clear_events(void)
 }
 
 /*! Adds specified async event to the queue */
-static void replay_add_event(ReplayAsyncEventKind event_kind,
-                             void *opaque,
-                             void *opaque2, uint64_t id)
+void replay_add_event(ReplayAsyncEventKind event_kind,
+                      void *opaque,
+                      void *opaque2, uint64_t id)
 {
     assert(event_kind < REPLAY_ASYNC_COUNT);
 
@@ -168,6 +171,9 @@ static void replay_save_event(Event *event, int checkpoint)
             break;
         case REPLAY_ASYNC_EVENT_INPUT_SYNC:
             break;
+        case REPLAY_ASYNC_EVENT_CHAR:
+            replay_event_char_save(event->opaque);
+            break;
         default:
             error_report("Unknown ID %d of replay event", read_event_kind);
             exit(1);
@@ -221,6 +227,11 @@ static Event *replay_read_event(int checkpoint)
         event->event_kind = read_event_kind;
         event->opaque = 0;
         return event;
+    case REPLAY_ASYNC_EVENT_CHAR:
+        event = g_malloc0(sizeof(Event));
+        event->event_kind = read_event_kind;
+        event->opaque = replay_event_char_read();
+        return event;
     default:
         error_report("Unknown ID %d of replay event", read_event_kind);
         exit(1);
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index 77e0d29..34eee5b 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -25,6 +25,8 @@ enum ReplayEvents {
     EVENT_ASYNC,
     /* for shutdown request */
     EVENT_SHUTDOWN,
+    /* for int data */
+    EVENT_DATA_INT,
     /* for clock read/writes */
     /* some of greater codes are reserved for clocks */
     EVENT_CLOCK,
@@ -44,6 +46,7 @@ enum ReplayAsyncEventKind {
     REPLAY_ASYNC_EVENT_BH,
     REPLAY_ASYNC_EVENT_INPUT,
     REPLAY_ASYNC_EVENT_INPUT_SYNC,
+    REPLAY_ASYNC_EVENT_CHAR,
     REPLAY_ASYNC_COUNT
 };
 
@@ -125,6 +128,9 @@ bool replay_has_events(void);
 void replay_save_events(int checkpoint);
 /*! Read events from the file into the input queue */
 void replay_read_events(int checkpoint);
+/*! Adds specified async event to the queue */
+void replay_add_event(ReplayAsyncEventKind event_kind, void *opaque,
+                      void *opaque2, uint64_t id);
 
 /* Input events */
 
@@ -137,4 +143,13 @@ void replay_add_input_event(struct InputEvent *event);
 /*! Adds input sync event to the queue */
 void replay_add_input_sync_event(void);
 
+/* Character devices */
+
+/*! Called to run char device event. */
+void replay_event_char_run(void *opaque);
+/*! Writes char event to the file. */
+void replay_event_char_save(void *opaque);
+/*! Reads char event from the file. */
+void *replay_event_char_read(void);
+
 #endif
diff --git a/replay/replay.c b/replay/replay.c
index 9cac178..dfb37dc 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -20,7 +20,7 @@
 
 /* Current version of the replay mechanism.
    Increase it when file format changes. */
-#define REPLAY_VERSION              0xe02002
+#define REPLAY_VERSION              0xe02003
 /* Size of replay log header */
 #define HEADER_SIZE                 (sizeof(uint32_t) + sizeof(uint64_t))
 
@@ -340,3 +340,26 @@ void replay_add_blocker(Error *reason)
 {
     replay_blockers = g_slist_prepend(replay_blockers, reason);
 }
+
+void replay_data_int(int *data)
+{
+    if (replay_mode == REPLAY_MODE_PLAY) {
+        replay_account_executed_instructions();
+        replay_mutex_lock();
+        if (replay_next_event_is(EVENT_DATA_INT)) {
+            *data = replay_get_dword();
+            replay_finish_event();
+            replay_mutex_unlock();
+        } else {
+            replay_mutex_unlock();
+            error_report("Missing data int event in the replay log");
+            exit(1);
+        }
+    } else if (replay_mode == REPLAY_MODE_RECORD) {
+        replay_save_instructions();
+        replay_mutex_lock();
+        replay_put_event(EVENT_DATA_INT);
+        replay_put_dword(*data);
+        replay_mutex_unlock();
+    }
+}

[Qemu-devel] [PATCH v2 2/4] icount: remove obsolete warp call

[Pavel Dovgalyuk]

qemu_clock_warp call in qemu_tcg_wait_io_event function is not needed
anymore, because it is called in every iteration of main_loop_wait.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 cpus.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/cpus.c b/cpus.c
index 898426c..01c9809 100644
--- a/cpus.c
+++ b/cpus.c
@@ -995,9 +995,6 @@ static void qemu_wait_io_event_common(CPUState *cpu)
 static void qemu_tcg_wait_io_event(CPUState *cpu)
 {
     while (all_cpu_threads_idle()) {
-       /* Start accounting real time to the virtual clock if the CPUs
-          are idle.  */
-        qemu_clock_warp(QEMU_CLOCK_VIRTUAL);
         qemu_cond_wait(cpu->halt_cond, &qemu_global_mutex);
     }
 

[Qemu-devel] [PATCH v2 3/4] replay: introduce new checkpoint for icount warp

[Pavel Dovgalyuk]

qemu_clock_warp function is called to update virtual clock when CPU
is sleeping. This function includes replay checkpoint to make execution
deterministic in icount mode.
Record/replay module flushes async event queue at checkpoints.
Some of the events (e.g., block devices operations) include interaction
with hardware. E.g., APIC polled by block devices sets one of IRQ flags.
Flag to be set depends on currently executed thread (CPU or iothread).
Therefore in replay mode we have to process the checkpoints in the same thread
as they were recorded.
qemu_clock_warp function (and its checkpoint) may be called from different
thread. This patch introduces new checkpoint which distinguished warp
checkpoint calls from different threads.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 cpus.c                  |    7 ++++---
 include/qemu/timer.h    |    3 ++-
 include/sysemu/replay.h |    1 +
 main-loop.c             |    2 +-
 qemu-timer.c            |    2 +-
 stubs/clock-warp.c      |    2 +-
 6 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/cpus.c b/cpus.c
index 01c9809..c2d9cfe 100644
--- a/cpus.c
+++ b/cpus.c
@@ -396,7 +396,7 @@ void qtest_clock_warp(int64_t dest)
     qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
 }
 
-void qemu_clock_warp(QEMUClockType type)
+void qemu_clock_warp(QEMUClockType type, bool in_tcg)
 {
     int64_t clock;
     int64_t deadline;
@@ -418,7 +418,8 @@ void qemu_clock_warp(QEMUClockType type)
     }
 
     /* warp clock deterministically in record/replay mode */
-    if (!replay_checkpoint(CHECKPOINT_CLOCK_WARP)) {
+    if (!replay_checkpoint(in_tcg ? CHECKPOINT_CLOCK_WARP_TCG
+                                  : CHECKPOINT_CLOCK_WARP)) {
         return;
     }
 
@@ -1496,7 +1497,7 @@ static void tcg_exec_all(void)
     int r;
 
     /* Account partial waits to QEMU_CLOCK_VIRTUAL.  */
-    qemu_clock_warp(QEMU_CLOCK_VIRTUAL);
+    qemu_clock_warp(QEMU_CLOCK_VIRTUAL, true);
 
     if (next_cpu == NULL) {
         next_cpu = first_cpu;
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index d0946cb..c58192c 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -212,10 +212,11 @@ void qemu_clock_enable(QEMUClockType type, bool enabled);
 /**
  * qemu_clock_warp:
  * @type: the clock type
+ * @in_tcg: true if function is called from TCG CPU thread
  *
  * Warp a clock to a new value
  */
-void qemu_clock_warp(QEMUClockType type);
+void qemu_clock_warp(QEMUClockType type, bool in_tcg);
 
 /**
  * qemu_clock_register_reset_notifier:
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 3c4a988..c879231 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -31,6 +31,7 @@ typedef enum ReplayClockKind ReplayClockKind;
 /* IDs of the checkpoints */
 enum ReplayCheckpoint {
     CHECKPOINT_CLOCK_WARP,
+    CHECKPOINT_CLOCK_WARP_TCG,
     CHECKPOINT_RESET_REQUESTED,
     CHECKPOINT_SUSPEND_REQUESTED,
     CHECKPOINT_CLOCK_VIRTUAL,
diff --git a/main-loop.c b/main-loop.c
index 19beae7..cd8415f 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -509,7 +509,7 @@ int main_loop_wait(int nonblocking)
 
     /* CPU thread can infinitely wait for event after
        missing the warp */
-    qemu_clock_warp(QEMU_CLOCK_VIRTUAL);
+    qemu_clock_warp(QEMU_CLOCK_VIRTUAL, false);
     qemu_clock_run_all_timers();
 
     return ret;
diff --git a/qemu-timer.c b/qemu-timer.c
index e98ecc9..980fe7e 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -394,7 +394,7 @@ static bool timer_mod_ns_locked(QEMUTimerList *timer_list,
 static void timerlist_rearm(QEMUTimerList *timer_list)
 {
     /* Interrupt execution to force deadline recalculation.  */
-    qemu_clock_warp(timer_list->clock->type);
+    qemu_clock_warp(timer_list->clock->type, false);
     timerlist_notify(timer_list);
 }
 
diff --git a/stubs/clock-warp.c b/stubs/clock-warp.c
index 5ae32b9..24ae0f8 100644
--- a/stubs/clock-warp.c
+++ b/stubs/clock-warp.c
@@ -2,7 +2,7 @@
 #include "qemu-common.h"
 #include "qemu/timer.h"
 
-void qemu_clock_warp(QEMUClockType type)
+void qemu_clock_warp(QEMUClockType type, bool in_tcg)
 {
 }
 

[Qemu-devel] [PATCH v2 4/4] replay: introduce block devices record/replay

[Pavel Dovgalyuk]

This patch introduces a set of functions that implement recording
and replaying of block devices' operations. These functions form a thin
layer between blk_aio_ functions and replay subsystem.
All asynchronous block requests are added to the queue to be processed
at deterministically invoked record/replay checkpoints.
Queue is flushed at checkpoints and information about processed requests
is recorded to the log. In replay phase the queue is matched with
events read from the log. Therefore block devices requests are processed
deterministically.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 block/block-backend.c    |   54 ++++++++++++--
 include/sysemu/replay.h  |   30 ++++++++
 replay/Makefile.objs     |    1 
 replay/replay-block.c    |  172 ++++++++++++++++++++++++++++++++++++++++++++++
 replay/replay-events.c   |   51 ++++++++++++++
 replay/replay-internal.h |   24 ++++++
 stubs/replay.c           |   50 +++++++++++++
 7 files changed, 372 insertions(+), 10 deletions(-)
 create mode 100755 replay/replay-block.c

diff --git a/block/block-backend.c b/block/block-backend.c
index ebdf78a..1765369 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-event.h"
 
 /* Number of coroutines to reserve per attached device model */
@@ -688,7 +689,7 @@ BlockAIOCB *blk_abort_aio_request(BlockBackend *blk,
 
     bh = aio_bh_new(blk_get_aio_context(blk), error_callback_bh, acb);
     acb->bh = bh;
-    qemu_bh_schedule(bh);
+    replay_bh_schedule_event(bh);
 
     return &acb->common;
 }
@@ -702,8 +703,13 @@ BlockAIOCB *blk_aio_write_zeroes(BlockBackend *blk, int64_t sector_num,
         return blk_abort_aio_request(blk, cb, opaque, ret);
     }
 
-    return bdrv_aio_write_zeroes(blk->bs, sector_num, nb_sectors, flags,
-                                 cb, opaque);
+    if (replay_mode == REPLAY_MODE_NONE) {
+        return bdrv_aio_write_zeroes(blk->bs, sector_num, nb_sectors, flags,
+                                     cb, opaque);
+    } else {
+        return replay_aio_write_zeroes(blk->bs, sector_num, nb_sectors, flags,
+                                       cb, opaque);
+    }
 }
 
 int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int count)
@@ -762,7 +768,11 @@ BlockAIOCB *blk_aio_readv(BlockBackend *blk, int64_t sector_num,
         return blk_abort_aio_request(blk, cb, opaque, ret);
     }
 
-    return bdrv_aio_readv(blk->bs, sector_num, iov, nb_sectors, cb, opaque);
+    if (replay_mode == REPLAY_MODE_NONE) {
+        return bdrv_aio_readv(blk->bs, sector_num, iov, nb_sectors, cb, opaque);
+    } else {
+        return replay_aio_readv(blk->bs, sector_num, iov, nb_sectors, cb, opaque);
+    }
 }
 
 BlockAIOCB *blk_aio_writev(BlockBackend *blk, int64_t sector_num,
@@ -774,7 +784,11 @@ BlockAIOCB *blk_aio_writev(BlockBackend *blk, int64_t sector_num,
         return blk_abort_aio_request(blk, cb, opaque, ret);
     }
 
-    return bdrv_aio_writev(blk->bs, sector_num, iov, nb_sectors, cb, opaque);
+    if (replay_mode == REPLAY_MODE_NONE) {
+        return bdrv_aio_writev(blk->bs, sector_num, iov, nb_sectors, cb, opaque);
+    } else {
+        return replay_aio_writev(blk->bs, sector_num, iov, nb_sectors, cb, opaque);
+    }
 }
 
 BlockAIOCB *blk_aio_flush(BlockBackend *blk,
@@ -784,7 +798,11 @@ BlockAIOCB *blk_aio_flush(BlockBackend *blk,
         return blk_abort_aio_request(blk, cb, opaque, -ENOMEDIUM);
     }
 
-    return bdrv_aio_flush(blk->bs, cb, opaque);
+    if (replay_mode == REPLAY_MODE_NONE) {
+        return bdrv_aio_flush(blk->bs, cb, opaque);
+    } else {
+        return replay_aio_flush(blk->bs, cb, opaque);
+    }
 }
 
 BlockAIOCB *blk_aio_discard(BlockBackend *blk,
@@ -796,17 +814,29 @@ BlockAIOCB *blk_aio_discard(BlockBackend *blk,
         return blk_abort_aio_request(blk, cb, opaque, ret);
     }
 
-    return bdrv_aio_discard(blk->bs, sector_num, nb_sectors, cb, opaque);
+    if (replay_mode == REPLAY_MODE_NONE) {
+        return bdrv_aio_discard(blk->bs, sector_num, nb_sectors, cb, opaque);
+    } else {
+        return replay_aio_discard(blk->bs, sector_num, nb_sectors, cb, opaque);
+    }
 }
 
 void blk_aio_cancel(BlockAIOCB *acb)
 {
-    bdrv_aio_cancel(acb);
+    if (replay_mode == REPLAY_MODE_NONE) {
+        bdrv_aio_cancel(acb);
+    } else {
+        replay_aio_cancel(acb);
+    }
 }
 
 void blk_aio_cancel_async(BlockAIOCB *acb)
 {
-    bdrv_aio_cancel_async(acb);
+    if (replay_mode == REPLAY_MODE_NONE) {
+        bdrv_aio_cancel_async(acb);
+    } else {
+        replay_aio_cancel(acb);
+    }
 }
 
 int blk_aio_multiwrite(BlockBackend *blk, BlockRequest *reqs, int num_reqs)
@@ -839,7 +869,11 @@ BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
         return blk_abort_aio_request(blk, cb, opaque, -ENOMEDIUM);
     }
 
-    return bdrv_aio_ioctl(blk->bs, req, buf, cb, opaque);
+    if (replay_mode == REPLAY_MODE_NONE) {
+        return bdrv_aio_ioctl(blk->bs, req, buf, cb, opaque);
+    } else {
+        return replay_aio_ioctl(blk->bs, req, buf, cb, opaque);
+    }
 }
 
 int blk_co_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors)
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index c879231..75ad4aa 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -17,6 +17,8 @@
 #include "qapi-types.h"
 #include "qapi/error.h"
 #include "qemu/typedefs.h"
+#include "block/aio.h"
+#include "block/block.h"
 
 /* replay clock kinds */
 enum ReplayClockKind {
@@ -125,6 +127,34 @@ void replay_register_char_driver(struct CharDriverState *chr);
 /*! Saves write to char device event to the log */
 void replay_chr_be_write(struct CharDriverState *s, uint8_t *buf, int len);
 
+/* Block devices */
+
+/*! Asynchronous read function. Adds read request to the queue. */
+BlockAIOCB *replay_aio_readv(BlockDriverState *bs, int64_t sector_num,
+                             QEMUIOVector *iov, int nb_sectors,
+                             BlockCompletionFunc *cb, void *opaque);
+/*! Asynchronous write function. Adds write request to the queue. */
+BlockAIOCB *replay_aio_writev(BlockDriverState *bs, int64_t sector_num,
+                              QEMUIOVector *iov, int nb_sectors,
+                              BlockCompletionFunc *cb, void *opaque);
+/*! Asynchronous write zeroes function. Adds this request to the queue. */
+BlockAIOCB *replay_aio_write_zeroes(BlockDriverState *bs, int64_t sector_num,
+                                    int nb_sectors, BdrvRequestFlags flags,
+                                    BlockCompletionFunc *cb, void *opaque);
+/*! Asynchronous flush function. Adds flush request to the queue. */
+BlockAIOCB *replay_aio_flush(BlockDriverState *bs,
+                             BlockCompletionFunc *cb, void *opaque);
+/*! Asynchronous discard function. Adds discard request to the queue. */
+BlockAIOCB *replay_aio_discard(BlockDriverState *bs, int64_t sector_num,
+                               int nb_sectors, BlockCompletionFunc *cb,
+                               void *opaque);
+/*! Asynchronous ioctl function. Adds ioctl request to the queue. */
+BlockAIOCB *replay_aio_ioctl(BlockDriverState *bs, unsigned long int req, void *buf,
+                             BlockCompletionFunc *cb, void *opaque);
+/*! Cancels pending asynchronous IO request. */
+void replay_aio_cancel(BlockAIOCB *acb);
+
+
 /* Other data */
 
 /*! Writes or reads integer value to/from replay log. */
diff --git a/replay/Makefile.objs b/replay/Makefile.objs
index 70e5572..050c0ea 100644
--- a/replay/Makefile.objs
+++ b/replay/Makefile.objs
@@ -4,3 +4,4 @@ common-obj-y += replay-events.o
 common-obj-y += replay-time.o
 common-obj-y += replay-input.o
 common-obj-y += replay-char.o
+common-obj-y += replay-block.o
diff --git a/replay/replay-block.c b/replay/replay-block.c
new file mode 100755
index 0000000..f927580
--- /dev/null
+++ b/replay/replay-block.c
@@ -0,0 +1,172 @@
+/*
+ * replay-block.c
+ *
+ * Copyright (c) 2010-2016 Institute for System Programming
+ *                         of the Russian Academy of Sciences.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#include "sysemu/replay.h"
+#include "sysemu/block-backend.h"
+#include "replay-internal.h"
+
+typedef struct ReplayAIOCB {
+    BlockAIOCB common;
+    BlockRequest req;
+    ReplayAsyncEventKind kind;
+} ReplayAIOCB;
+
+static const AIOCBInfo replay_aiocb_info = {
+    .aiocb_size   = sizeof(ReplayAIOCB),
+    .cancel_async = replay_aio_cancel
+};
+
+static ReplayAIOCB *replay_aio_create(ReplayAsyncEventKind kind,
+                                      BlockDriverState *bs,
+                                      BlockCompletionFunc *cb, void *opaque)
+{
+    ReplayAIOCB *acb = g_malloc0(sizeof(*acb));
+    acb->kind = kind;
+    /* Make correct AIOCB to return to the caller */
+    acb->common.aiocb_info = &replay_aiocb_info;
+    acb->common.bs = bs;
+    acb->common.cb = cb;
+    acb->common.opaque = opaque;
+    acb->common.refcnt = 1;
+    acb->req.error = -EINPROGRESS;
+    return acb;
+}
+
+BlockAIOCB *replay_aio_readv(BlockDriverState *bs, int64_t sector_num,
+                             QEMUIOVector *iov, int nb_sectors,
+                             BlockCompletionFunc *cb, void *opaque)
+{
+    ReplayAIOCB *acb = replay_aio_create(REPLAY_ASYNC_EVENT_BLOCK_READV,
+                                         bs, cb, opaque);
+    acb->req.sector = sector_num;
+    acb->req.nb_sectors = nb_sectors;
+    acb->req.qiov = iov;
+    replay_add_event(REPLAY_ASYNC_EVENT_BLOCK_READV, acb, NULL, 0);
+
+    return &acb->common;
+}
+
+void replay_event_block_readv_run(void *opaque)
+{
+    ReplayAIOCB *acb = opaque;
+    bdrv_aio_readv(acb->common.bs, acb->req.sector, acb->req.qiov,
+                   acb->req.nb_sectors, acb->common.cb,
+                   acb->common.opaque);
+    blk_drain_all();
+}
+
+BlockAIOCB *replay_aio_writev(BlockDriverState *bs, int64_t sector_num,
+                              QEMUIOVector *iov, int nb_sectors,
+                              BlockCompletionFunc *cb, void *opaque)
+{
+    ReplayAIOCB *acb = replay_aio_create(REPLAY_ASYNC_EVENT_BLOCK_WRITEV,
+                                         bs, cb, opaque);
+    acb->req.sector = sector_num;
+    acb->req.nb_sectors = nb_sectors;
+    acb->req.qiov = iov;
+    replay_add_event(REPLAY_ASYNC_EVENT_BLOCK_WRITEV, acb, NULL, 0);
+
+    return &acb->common;
+}
+
+void replay_event_block_writev_run(void *opaque)
+{
+    ReplayAIOCB *acb = opaque;
+    bdrv_aio_writev(acb->common.bs, acb->req.sector, acb->req.qiov,
+                    acb->req.nb_sectors, acb->common.cb,
+                    acb->common.opaque);
+    blk_drain_all();
+}
+
+BlockAIOCB *replay_aio_write_zeroes(BlockDriverState *bs, int64_t sector_num,
+                                    int nb_sectors, BdrvRequestFlags flags,
+                                    BlockCompletionFunc *cb, void *opaque)
+{
+    ReplayAIOCB *acb = replay_aio_create(REPLAY_ASYNC_EVENT_BLOCK_WRITE_ZEROES,
+                                         bs, cb, opaque);
+    acb->req.sector = sector_num;
+    acb->req.nb_sectors = nb_sectors;
+    acb->req.flags = flags;
+    replay_add_event(REPLAY_ASYNC_EVENT_BLOCK_WRITE_ZEROES, acb, NULL, 0);
+
+    return &acb->common;
+}
+
+void replay_event_block_write_zeroes_run(void *opaque)
+{
+    ReplayAIOCB *acb = opaque;
+    bdrv_aio_write_zeroes(acb->common.bs, acb->req.sector, acb->req.nb_sectors,
+                          acb->req.flags, acb->common.cb, acb->common.opaque);
+    blk_drain_all();
+}
+
+BlockAIOCB *replay_aio_flush(BlockDriverState *bs,
+                             BlockCompletionFunc *cb, void *opaque)
+{
+    ReplayAIOCB *acb = replay_aio_create(REPLAY_ASYNC_EVENT_BLOCK_FLUSH,
+                                         bs, cb, opaque);
+    replay_add_event(REPLAY_ASYNC_EVENT_BLOCK_FLUSH, acb, NULL, 0);
+
+    return &acb->common;
+}
+
+void replay_event_block_flush_run(void *opaque)
+{
+    ReplayAIOCB *acb = opaque;
+    bdrv_aio_flush(acb->common.bs, acb->common.cb, acb->common.opaque);
+    blk_drain_all();
+}
+
+void replay_aio_cancel(BlockAIOCB *acb)
+{
+    ReplayAIOCB *racb = container_of(acb, ReplayAIOCB, common);;
+    replay_remove_event(racb->kind, acb, NULL, 0);
+}
+
+BlockAIOCB *replay_aio_discard(BlockDriverState *bs, int64_t sector_num,
+                               int nb_sectors, BlockCompletionFunc *cb,
+                               void *opaque)
+{
+    ReplayAIOCB *acb = replay_aio_create(REPLAY_ASYNC_EVENT_BLOCK_DISCARD,
+                                         bs, cb, opaque);
+    acb->req.sector = sector_num;
+    acb->req.nb_sectors = nb_sectors;
+    replay_add_event(REPLAY_ASYNC_EVENT_BLOCK_DISCARD, acb, NULL, 0);
+
+    return &acb->common;
+}
+
+void replay_event_block_discard_run(void *opaque)
+{
+    ReplayAIOCB *acb = opaque;
+    bdrv_aio_discard(acb->common.bs, acb->req.sector, acb->req.nb_sectors,
+                     acb->common.cb, acb->common.opaque);
+    blk_drain_all();
+}
+
+BlockAIOCB *replay_aio_ioctl(BlockDriverState *bs, unsigned long int req, void *buf,
+                             BlockCompletionFunc *cb, void *opaque)
+{
+    ReplayAIOCB *acb = replay_aio_create(REPLAY_ASYNC_EVENT_BLOCK_IOCTL,
+                                         bs, cb, opaque);
+    acb->req.req = req;
+    acb->req.buf = buf;
+    replay_add_event(REPLAY_ASYNC_EVENT_BLOCK_IOCTL, acb, NULL, 0);
+
+    return &acb->common;
+}
+
+void replay_event_block_ioctl_run(void *opaque)
+{
+    ReplayAIOCB *acb = opaque;
+    bdrv_aio_ioctl(acb->common.bs, acb->req.req, acb->req.buf,
+                   acb->common.cb, acb->common.opaque);
+    blk_drain_all();
+}
diff --git a/replay/replay-events.c b/replay/replay-events.c
index 59d467f..14aca62 100644
--- a/replay/replay-events.c
+++ b/replay/replay-events.c
@@ -51,6 +51,24 @@ static void replay_run_event(Event *event)
     case REPLAY_ASYNC_EVENT_CHAR:
         replay_event_char_run(event->opaque);
         break;
+    case REPLAY_ASYNC_EVENT_BLOCK_READV:
+        replay_event_block_readv_run(event->opaque);
+        break;
+    case REPLAY_ASYNC_EVENT_BLOCK_WRITEV:
+        replay_event_block_writev_run(event->opaque);
+        break;
+    case REPLAY_ASYNC_EVENT_BLOCK_WRITE_ZEROES:
+        replay_event_block_write_zeroes_run(event->opaque);
+        break;
+    case REPLAY_ASYNC_EVENT_BLOCK_FLUSH:
+        replay_event_block_flush_run(event->opaque);
+        break;
+    case REPLAY_ASYNC_EVENT_BLOCK_DISCARD:
+        replay_event_block_discard_run(event->opaque);
+        break;
+    case REPLAY_ASYNC_EVENT_BLOCK_IOCTL:
+        replay_event_block_ioctl_run(event->opaque);
+        break;
     default:
         error_report("Replay: invalid async event ID (%d) in the queue",
                     event->event_kind);
@@ -133,6 +151,24 @@ void replay_add_event(ReplayAsyncEventKind event_kind,
     replay_mutex_unlock();
 }
 
+void replay_remove_event(ReplayAsyncEventKind event_kind,
+                         void *opaque,
+                         void *opaque2, uint64_t id)
+{
+    Event *event;
+    replay_mutex_lock();
+    QTAILQ_FOREACH(event, &events_list, events) {
+        if (event->event_kind == event_kind
+            && event->opaque == opaque
+            && event->opaque2 == opaque2
+            && event->id == id) {
+            QTAILQ_REMOVE(&events_list, event, events);
+            break;
+        }
+    }
+    replay_mutex_unlock();
+}
+
 void replay_bh_schedule_event(QEMUBH *bh)
 {
     if (replay_mode != REPLAY_MODE_NONE) {
@@ -174,6 +210,13 @@ static void replay_save_event(Event *event, int checkpoint)
         case REPLAY_ASYNC_EVENT_CHAR:
             replay_event_char_save(event->opaque);
             break;
+        case REPLAY_ASYNC_EVENT_BLOCK_READV:
+        case REPLAY_ASYNC_EVENT_BLOCK_WRITEV:
+        case REPLAY_ASYNC_EVENT_BLOCK_WRITE_ZEROES:
+        case REPLAY_ASYNC_EVENT_BLOCK_FLUSH:
+        case REPLAY_ASYNC_EVENT_BLOCK_DISCARD:
+        case REPLAY_ASYNC_EVENT_BLOCK_IOCTL:
+            break;
         default:
             error_report("Unknown ID %d of replay event", read_event_kind);
             exit(1);
@@ -232,6 +275,14 @@ static Event *replay_read_event(int checkpoint)
         event->event_kind = read_event_kind;
         event->opaque = replay_event_char_read();
         return event;
+    case REPLAY_ASYNC_EVENT_BLOCK_READV:
+    case REPLAY_ASYNC_EVENT_BLOCK_WRITEV:
+    case REPLAY_ASYNC_EVENT_BLOCK_WRITE_ZEROES:
+    case REPLAY_ASYNC_EVENT_BLOCK_FLUSH:
+    case REPLAY_ASYNC_EVENT_BLOCK_DISCARD:
+    case REPLAY_ASYNC_EVENT_BLOCK_IOCTL:
+        /* search in the queue */
+        break;
     default:
         error_report("Unknown ID %d of replay event", read_event_kind);
         exit(1);
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index 34eee5b..8f705d1 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -47,6 +47,12 @@ enum ReplayAsyncEventKind {
     REPLAY_ASYNC_EVENT_INPUT,
     REPLAY_ASYNC_EVENT_INPUT_SYNC,
     REPLAY_ASYNC_EVENT_CHAR,
+    REPLAY_ASYNC_EVENT_BLOCK_READV,
+    REPLAY_ASYNC_EVENT_BLOCK_WRITEV,
+    REPLAY_ASYNC_EVENT_BLOCK_WRITE_ZEROES,
+    REPLAY_ASYNC_EVENT_BLOCK_FLUSH,
+    REPLAY_ASYNC_EVENT_BLOCK_DISCARD,
+    REPLAY_ASYNC_EVENT_BLOCK_IOCTL,
     REPLAY_ASYNC_COUNT
 };
 
@@ -131,6 +137,9 @@ void replay_read_events(int checkpoint);
 /*! Adds specified async event to the queue */
 void replay_add_event(ReplayAsyncEventKind event_kind, void *opaque,
                       void *opaque2, uint64_t id);
+/*! Removes specified async event from the queue */
+void replay_remove_event(ReplayAsyncEventKind event_kind,
+                         void *opaque, void *opaque2, uint64_t id);
 
 /* Input events */
 
@@ -152,4 +161,19 @@ void replay_event_char_save(void *opaque);
 /*! Reads char event from the file. */
 void *replay_event_char_read(void);
 
+/* Block devices */
+
+/*! Called to run block device readv event. */
+void replay_event_block_readv_run(void *opaque);
+/*! Called to run block device writev event. */
+void replay_event_block_writev_run(void *opaque);
+/*! Called to run block device write zeroes event. */
+void replay_event_block_write_zeroes_run(void *opaque);
+/*! Called to run block device flush event. */
+void replay_event_block_flush_run(void *opaque);
+/*! Called to run block device discard event. */
+void replay_event_block_discard_run(void *opaque);
+/*! Called to run block device ioctl event. */
+void replay_event_block_ioctl_run(void *opaque);
+
 #endif
diff --git a/stubs/replay.c b/stubs/replay.c
index 00ca01f..635e7dd 100644
--- a/stubs/replay.c
+++ b/stubs/replay.c
@@ -1,6 +1,7 @@
 #include "qemu/osdep.h"
 #include "sysemu/replay.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/block-backend.h"
 
 ReplayMode replay_mode;
 
@@ -29,3 +30,52 @@ bool replay_events_enabled(void)
 void replay_finish(void)
 {
 }
+
+BlockAIOCB *replay_aio_readv(BlockDriverState *bs, int64_t sector_num,
+                             QEMUIOVector *iov, int nb_sectors,
+                             BlockCompletionFunc *cb, void *opaque)
+{
+    return NULL;
+}
+
+BlockAIOCB *replay_aio_writev(BlockDriverState *bs, int64_t sector_num,
+                              QEMUIOVector *iov, int nb_sectors,
+                              BlockCompletionFunc *cb, void *opaque)
+{
+    return NULL;
+}
+
+BlockAIOCB *replay_aio_flush(BlockDriverState *bs,
+                             BlockCompletionFunc *cb, void *opaque)
+{
+    return NULL;
+}
+
+BlockAIOCB *replay_aio_discard(BlockDriverState *bs,
+                               int64_t sector_num, int nb_sectors,
+                               BlockCompletionFunc *cb, void *opaque)
+{
+    return NULL;
+}
+
+BlockAIOCB *replay_aio_ioctl(BlockDriverState *bs, unsigned long int req, void *buf,
+                             BlockCompletionFunc *cb, void *opaque)
+{
+    return NULL;
+}
+
+BlockAIOCB *replay_aio_write_zeroes(BlockDriverState *bs, int64_t sector_num,
+                                    int nb_sectors, BdrvRequestFlags flags,
+                                    BlockCompletionFunc *cb, void *opaque)
+{
+    return NULL;
+}
+
+void replay_aio_cancel(BlockAIOCB *acb)
+{
+}
+
+void replay_bh_schedule_event(QEMUBH *bh)
+{
+    qemu_bh_schedule(bh);
+}

Re: [Qemu-devel] [PATCH v2 4/4] replay: introduce block devices record/replay

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 906 bytes --]

On Wed, Feb 10, 2016 at 12:13:23PM +0300, Pavel Dovgalyuk wrote:
> @@ -784,7 +798,11 @@ BlockAIOCB *blk_aio_flush(BlockBackend *blk,
>          return blk_abort_aio_request(blk, cb, opaque, -ENOMEDIUM);
>      }
>  
> -    return bdrv_aio_flush(blk->bs, cb, opaque);
> +    if (replay_mode == REPLAY_MODE_NONE) {
> +        return bdrv_aio_flush(blk->bs, cb, opaque);
> +    } else {
> +        return replay_aio_flush(blk->bs, cb, opaque);
> +    }
>  }

I am only looking at this patch in isolation and am not familiar with
the record/replay work, but these changes appear invasive.  In order for
record/replay to keep working in the future, everyone touching block
layer code must be familiar with the principles of how record/replay
works.  This patch does not include documentation.

Can you point me to documentation that explains the how record replay
works?

Thanks,
Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v2 4/4] replay: introduce block devices record/replay

[Pavel Dovgalyuk]

> From: Stefan Hajnoczi [mailto:stefanha@redhat.com]
> On Wed, Feb 10, 2016 at 12:13:23PM +0300, Pavel Dovgalyuk wrote:
> > @@ -784,7 +798,11 @@ BlockAIOCB *blk_aio_flush(BlockBackend *blk,
> >          return blk_abort_aio_request(blk, cb, opaque, -ENOMEDIUM);
> >      }
> >
> > -    return bdrv_aio_flush(blk->bs, cb, opaque);
> > +    if (replay_mode == REPLAY_MODE_NONE) {
> > +        return bdrv_aio_flush(blk->bs, cb, opaque);
> > +    } else {
> > +        return replay_aio_flush(blk->bs, cb, opaque);
> > +    }
> >  }
> 
> I am only looking at this patch in isolation and am not familiar with
> the record/replay work, but these changes appear invasive.  In order for
> record/replay to keep working in the future, everyone touching block
> layer code must be familiar with the principles of how record/replay
> works.  This patch does not include documentation.

We've already discussed it with Kevin.
He proposes adding new block driver for record/replay.

> Can you point me to documentation that explains the how record replay
> works?

There is some information about it in docs/replay.txt and in
http://wiki.qemu.org/Features/record-replay

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v2 4/4] replay: introduce block devices record/replay

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2270 bytes --]

On Thu, Feb 11, 2016 at 04:52:42PM +0300, Pavel Dovgalyuk wrote:
> > From: Stefan Hajnoczi [mailto:stefanha@redhat.com]
> > On Wed, Feb 10, 2016 at 12:13:23PM +0300, Pavel Dovgalyuk wrote:
> > > @@ -784,7 +798,11 @@ BlockAIOCB *blk_aio_flush(BlockBackend *blk,
> > >          return blk_abort_aio_request(blk, cb, opaque, -ENOMEDIUM);
> > >      }
> > >
> > > -    return bdrv_aio_flush(blk->bs, cb, opaque);
> > > +    if (replay_mode == REPLAY_MODE_NONE) {
> > > +        return bdrv_aio_flush(blk->bs, cb, opaque);
> > > +    } else {
> > > +        return replay_aio_flush(blk->bs, cb, opaque);
> > > +    }
> > >  }
> > 
> > I am only looking at this patch in isolation and am not familiar with
> > the record/replay work, but these changes appear invasive.  In order for
> > record/replay to keep working in the future, everyone touching block
> > layer code must be familiar with the principles of how record/replay
> > works.  This patch does not include documentation.
> 
> We've already discussed it with Kevin.
> He proposes adding new block driver for record/replay.
> 
> > Can you point me to documentation that explains the how record replay
> > works?
> 
> There is some information about it in docs/replay.txt and in
> http://wiki.qemu.org/Features/record-replay

I was thinking about developer documentation.  A feature like this is
"cross-cutting" - it affects many components.  There should be
documentation that explains the semantics so everyone modifying code can
follow the rules to keep record/replay working.

Live migration is similar in that it touches many components.  The
docs/migration.txt file explains the APIs and general idea.  It
communicates the assumptions, limitations, and requirements that live
migration imposes.  Many QEMU developers understand migration basics and
keep them in mind during code review.  This way we can prevent most (but
not all) migration breakage.

Record/replay should aim for the same level of education/awareness,
otherwise the feature will break and bitrot.

The alternative is to implement it in a way that isn't invasive.  Then
other developers don't need to understand how it works.  Maybe
implementing it in a block driver can achieve this.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]