[Qemu-devel] [PATCH v9 00/21] Fixing record/replay and adding reverse debugging

[Qemu-devel] [PATCH v9 00/21] Fixing record/replay and adding reverse debugging

[Pavel Dovgalyuk]

GDB remote protocol supports reverse debugging of the targets.
It includes 'reverse step' and 'reverse continue' operations.
The first one finds the previous step of the execution,
and the second one is intended to stop at the last breakpoint that
would happen when the program is executed normally.

Reverse debugging is possible in the replay mode, when at least
one snapshot was created at the record or replay phase.
QEMU can use these snapshots for travelling back in time with GDB.

Running the execution in replay mode allows using GDB reverse debugging
commands:
 - reverse-stepi (or rsi): Steps one instruction to the past.
   QEMU loads on of the prior snapshots and proceeds to the desired
   instruction forward. When that step is reaches, execution stops.
 - reverse-continue (or rc): Runs execution "backwards".
   QEMU tries to find breakpoint or watchpoint by loaded prior snapshot
   and replaying the execution. Then QEMU loads snapshots again and
   replays to the latest breakpoint. When there are no breakpoints in
   the examined section of the execution, QEMU finds one more snapshot
   and tries again. After the first snapshot is processed, execution
   stops at this snapshot.

The set of patches include the following modifications:
 - gdbstub update for reverse debugging support
 - functions that automatically perform reverse step and reverse
   continue operations
 - hmp/qmp commands for manipulating the replay process
 - improvement of the snapshotting for saving the execution step
   in the snapshot parameters
 - other record/replay fixes

The patches are available in the repository:
https://github.com/ispras/qemu/tree/rr-190109

v9 changes:
 - moved rr qapi stuff to the separate file (suggested by Markus Armbruster)
 - minor coding style fixes

v8 changes:
 - rebased to the new master
 - added missing fix for prior rr patch
 - updated 'since' version number in json-related patches

v7 changes:
 - rebased to the new master with upstreamed patches from the series
 - several improvements in hmp/qmp commands handling (suggested by Markus Armbruster)
 - fixed record/replay with '-rtc base' option enabled
 - added document with virtual hardware requirements

v6 changes:
 - rebased to the new version of master
 - fixed build of linux-user configurations
 - added new clock for slirp and vnc timers

v5 changes:
 - multiple fixes of record/replay bugs appeared after QEMU core update
 - changed reverse debugging to 'since 3.1'

v4 changes:
 - changed 'since 2.13' to 'since 3.0' in json (as suggested by Eric Blake)

v3 changes:
 - Fixed PS/2 bug with save/load vm, which caused failures of the replay.
 - Rebased to the new code base.
 - Minor fixes.

v2 changes:
 - documented reverse debugging
 - fixed start vmstate loading in record mode
 - documented qcow2 changes (as suggested by Eric Blake)
 - made icount SnapshotInfo field optional (as suggested by Eric Blake)
 - renamed qmp commands (as suggested by Eric Blake)
 - minor changes

---

Pavel Dovgalyuk (20):
      block: implement bdrv_snapshot_goto for blkreplay
      replay: disable default snapshot for record/replay
      replay: update docs for record/replay with block devices
      replay: don't drain/flush bdrv queue while RR is working
      replay: finish record/replay before closing the disks
      qcow2: introduce icount field for snapshots
      migration: introduce icount field for snapshots
      replay: provide and accessor for rr filename
      qapi: introduce replay.json for record/replay-related stuff
      replay: introduce info hmp/qmp command
      replay: introduce breakpoint at the specified step
      replay: implement replay-seek command to proceed to the desired step
      replay: refine replay-time module
      replay: flush rr queue before loading the vmstate
      gdbstub: add reverse step support in replay mode
      gdbstub: add reverse continue support in replay mode
      replay: describe reverse debugging in docs/replay.txt
      replay: add BH oneshot event for block layer
      replay: init rtc after enabling the replay
      replay: document development rules

pbonzini@redhat.com (1):
      replay: add missing fix for internal function


 MAINTAINERS               |    1 
 Makefile.objs             |    4 -
 accel/tcg/translator.c    |    1 
 block/blkreplay.c         |    8 +
 block/block-backend.c     |    5 -
 block/io.c                |   28 ++++
 block/qapi.c              |   18 ++-
 block/qcow2-snapshot.c    |    9 +
 block/qcow2.h             |    2 
 blockdev.c                |   10 +
 cpus.c                    |   21 ++-
 docs/devel/replay.txt     |   46 ++++++
 docs/interop/qcow2.txt    |    4 +
 docs/replay.txt           |   45 ++++++
 exec.c                    |    8 +
 gdbstub.c                 |   52 +++++++
 hmp-commands-info.hx      |   14 ++
 hmp-commands.hx           |   53 +++++++
 hmp.h                     |    4 +
 include/block/snapshot.h  |    1 
 include/sysemu/replay.h   |   27 ++++
 migration/savevm.c        |   11 ++
 qapi/block-core.json      |    8 +
 qapi/block.json           |    3 
 qapi/misc.json            |   18 ---
 qapi/qapi-schema.json     |    1 
 qapi/replay.json          |  121 +++++++++++++++++
 replay/Makefile.objs      |    3 
 replay/replay-debugging.c |  322 +++++++++++++++++++++++++++++++++++++++++++++
 replay/replay-events.c    |   16 ++
 replay/replay-internal.c  |    2 
 replay/replay-internal.h  |    7 +
 replay/replay-time.c      |   36 ++---
 replay/replay.c           |   26 +++-
 stubs/Makefile.objs       |    1 
 stubs/replay-user.c       |    9 +
 stubs/replay.c            |   10 +
 vl.c                      |   21 ++-
 38 files changed, 907 insertions(+), 69 deletions(-)
 create mode 100644 docs/devel/replay.txt
 create mode 100644 qapi/replay.json
 create mode 100644 replay/replay-debugging.c
 create mode 100644 stubs/replay-user.c

-- 
Pavel Dovgalyuk

[Qemu-devel] [PATCH v9 01/21] replay: add missing fix for internal function

[Pavel Dovgalyuk]

From: pbonzini@redhat.com <pbonzini@redhat.com>

This is a fix which was missed by patch
74c0b816adfc6aa1b01b4426fdf385e32e35cbac, which added current_step
parameter to the replay_advance_current_step function.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
---
 replay/replay-internal.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/replay/replay-internal.c b/replay/replay-internal.c
index 8f87e9b..7e6de03 100644
--- a/replay/replay-internal.c
+++ b/replay/replay-internal.c
@@ -229,7 +229,7 @@ void replay_mutex_unlock(void)
 
 void replay_advance_current_step(uint64_t current_step)
 {
-    int diff = (int)(replay_get_current_step() - replay_state.current_step);
+    int diff = (int)(current_step - replay_state.current_step);
 
     /* Time can only go forward */
     assert(diff >= 0);

[Qemu-devel] [PATCH v9 02/21] block: implement bdrv_snapshot_goto for blkreplay

[Pavel Dovgalyuk]

From: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>

This patch enables making snapshots with blkreplay used in
block devices.
This function is required to make bdrv_snapshot_goto without
calling .bdrv_open which is not implemented.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 block/blkreplay.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/block/blkreplay.c b/block/blkreplay.c
index b5d9efd..142dfe3 100644
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -126,6 +126,12 @@ static int coroutine_fn blkreplay_co_flush(BlockDriverState *bs)
     return ret;
 }
 
+static int blkreplay_snapshot_goto(BlockDriverState *bs,
+                                   const char *snapshot_id)
+{
+    return bdrv_snapshot_goto(bs->file->bs, snapshot_id, NULL);
+}
+
 static BlockDriver bdrv_blkreplay = {
     .format_name            = "blkreplay",
     .instance_size          = 0,
@@ -140,6 +146,8 @@ static BlockDriver bdrv_blkreplay = {
     .bdrv_co_pwrite_zeroes  = blkreplay_co_pwrite_zeroes,
     .bdrv_co_pdiscard       = blkreplay_co_pdiscard,
     .bdrv_co_flush          = blkreplay_co_flush,
+
+    .bdrv_snapshot_goto     = blkreplay_snapshot_goto,
 };
 
 static void bdrv_blkreplay_init(void)

[Qemu-devel] [PATCH v9 03/21] replay: disable default snapshot for record/replay

[Pavel Dovgalyuk]

From: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>

This patch disables setting '-snapshot' option on by default
in record/replay mode. This is needed for creating vmstates in record
and replay modes.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 vl.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 064872c..7de4f55 100644
--- a/vl.c
+++ b/vl.c
@@ -3196,7 +3196,13 @@ int main(int argc, char **argv, char **envp)
                 drive_add(IF_PFLASH, -1, optarg, PFLASH_OPTS);
                 break;
             case QEMU_OPTION_snapshot:
-                snapshot = 1;
+                {
+                    Error *blocker = NULL;
+                    snapshot = 1;
+                    error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED,
+                               "-snapshot");
+                    replay_add_blocker(blocker);
+                }
                 break;
             case QEMU_OPTION_numa:
                 opts = qemu_opts_parse_noisily(qemu_find_opts("numa"),
@@ -4472,7 +4478,7 @@ int main(int argc, char **argv, char **envp)
         qapi_free_BlockdevOptions(bdo->bdo);
         g_free(bdo);
     }
-    if (snapshot || replay_mode != REPLAY_MODE_NONE) {
+    if (snapshot) {
         qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
                           NULL, NULL);
     }

[Qemu-devel] [PATCH v9 04/21] replay: update docs for record/replay with block devices

[Pavel Dovgalyuk]

This patch updates the description of the command lines for using
record/replay with attached block devices.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
---
 docs/replay.txt |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/docs/replay.txt b/docs/replay.txt
index 3497585..2c2c5f6 100644
--- a/docs/replay.txt
+++ b/docs/replay.txt
@@ -27,7 +27,7 @@ Usage of the record/replay:
  * First, record the execution with the following command line:
     qemu-system-i386 \
      -icount shift=7,rr=record,rrfile=replay.bin \
-     -drive file=disk.qcow2,if=none,id=img-direct \
+     -drive file=disk.qcow2,if=none,snapshot,id=img-direct \
      -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \
      -device ide-hd,drive=img-blkreplay \
      -netdev user,id=net1 -device rtl8139,netdev=net1 \
@@ -35,7 +35,7 @@ Usage of the record/replay:
  * After recording, you can replay it by using another command line:
     qemu-system-i386 \
      -icount shift=7,rr=replay,rrfile=replay.bin \
-     -drive file=disk.qcow2,if=none,id=img-direct \
+     -drive file=disk.qcow2,if=none,snapshot,id=img-direct \
      -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \
      -device ide-hd,drive=img-blkreplay \
      -netdev user,id=net1 -device rtl8139,netdev=net1 \
@@ -223,7 +223,7 @@ Block devices record/replay module intercepts calls of
 bdrv coroutine functions at the top of block drivers stack.
 To record and replay block operations the drive must be configured
 as following:
- -drive file=disk.qcow2,if=none,id=img-direct
+ -drive file=disk.qcow2,if=none,snapshot,id=img-direct
  -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
  -device ide-hd,drive=img-blkreplay
 
@@ -252,6 +252,12 @@ This snapshot is created at start of recording and restored at start
 of replaying. It also can be loaded while replaying to roll back
 the execution.
 
+'snapshot' flag of the disk image must be removed to save the snapshots
+in the overlay (or original image) instead of using the temporary overlay.
+ -drive file=disk.ovl,if=none,id=img-direct
+ -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
+ -device ide-hd,drive=img-blkreplay
+
 Use QEMU monitor to create additional snapshots. 'savevm <name>' command
 created the snapshot and 'loadvm <name>' restores it. To prevent corruption
 of the original disk image, use overlay files linked to the original images.

[Qemu-devel] [PATCH v9 05/21] replay: don't drain/flush bdrv queue while RR is working

[Pavel Dovgalyuk]

In record/replay mode bdrv queue is controlled by replay mechanism.
It does not allow saving or loading the snapshots
when bdrv queue is not empty. Stopping the VM is not blocked by nonempty
queue, but flushing the queue is still impossible there,
because it may cause deadlocks in replay mode.
This patch disables bdrv_drain_all and bdrv_flush_all in
record/replay mode.

Stopping the machine when the IO requests are not finished is needed
for the debugging. E.g., breakpoint may be set at the specified step,
and forcing the IO requests to finish may break the determinism
of the execution.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 block/io.c |   28 ++++++++++++++++++++++++++++
 cpus.c     |    2 --
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/block/io.c b/block/io.c
index bd9d688..f9a9c5a 100644
--- a/block/io.c
+++ b/block/io.c
@@ -32,6 +32,7 @@
 #include "qemu/cutils.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
+#include "sysemu/replay.h"
 
 #define NOT_DONE 0x7fffffff /* used while emulated sync operation in progress */
 
@@ -538,6 +539,15 @@ void bdrv_drain_all_begin(void)
         return;
     }
 
+    /*
+     * bdrv queue is managed by record/replay,
+     * waiting for finishing the I/O requests may
+     * be infinite
+     */
+    if (replay_events_enabled()) {
+        return;
+    }
+
     /* AIO_WAIT_WHILE() with a NULL context can only be called from the main
      * loop AioContext, so make sure we're in the main context. */
     assert(qemu_get_current_aio_context() == qemu_get_aio_context());
@@ -566,6 +576,15 @@ void bdrv_drain_all_end(void)
 {
     BlockDriverState *bs = NULL;
 
+    /*
+     * bdrv queue is managed by record/replay,
+     * waiting for finishing the I/O requests may
+     * be endless
+     */
+    if (replay_events_enabled()) {
+        return;
+    }
+
     while ((bs = bdrv_next_all_states(bs))) {
         AioContext *aio_context = bdrv_get_aio_context(bs);
 
@@ -1997,6 +2016,15 @@ int bdrv_flush_all(void)
     BlockDriverState *bs = NULL;
     int result = 0;
 
+    /*
+     * bdrv queue is managed by record/replay,
+     * creating new flush request for stopping
+     * the VM may break the determinism
+     */
+    if (replay_events_enabled()) {
+        return result;
+    }
+
     for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
         AioContext *aio_context = bdrv_get_aio_context(bs);
         int ret;
diff --git a/cpus.c b/cpus.c
index b09b702..aa33fb1 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1078,7 +1078,6 @@ static int do_vm_stop(RunState state, bool send_stop)
     }
 
     bdrv_drain_all();
-    replay_disable_events();
     ret = bdrv_flush_all();
 
     return ret;
@@ -2149,7 +2148,6 @@ int vm_prepare_start(void)
     /* We are sending this now, but the CPUs will be resumed shortly later */
     qapi_event_send_resume();
 
-    replay_enable_events();
     cpu_enable_ticks();
     runstate_set(RUN_STATE_RUNNING);
     vm_state_notify(1, RUN_STATE_RUNNING);

[Qemu-devel] [PATCH v9 06/21] replay: finish record/replay before closing the disks

[Pavel Dovgalyuk]

After recent updates block devices cannot be closed on qemu exit.
This happens due to the block request polling when replay is not finished.
Therefore now we stop execution recording before closing the block devices.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 replay/replay.c |    2 ++
 vl.c            |    1 +
 2 files changed, 3 insertions(+)

diff --git a/replay/replay.c b/replay/replay.c
index 8b172b2..b75820a 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -385,6 +385,8 @@ void replay_finish(void)
     g_free(replay_snapshot);
     replay_snapshot = NULL;
 
+    replay_mode = REPLAY_MODE_NONE;
+
     replay_finish_events();
 }
 
diff --git a/vl.c b/vl.c
index 7de4f55..95fc44a 100644
--- a/vl.c
+++ b/vl.c
@@ -4678,6 +4678,7 @@ int main(int argc, char **argv, char **envp)
 
     /* No more vcpu or device emulation activity beyond this point */
     vm_shutdown();
+    replay_finish();
 
     job_cancel_sync_all();
     bdrv_close_all();

[Qemu-devel] [PATCH v9 07/21] qcow2: introduce icount field for snapshots

[Pavel Dovgalyuk]

This patch introduces the icount field for saving within the snapshot.
It is required for navigation between the snapshots in record/replay mode.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
Acked-by: Kevin Wolf <kwolf@redhat.com>

--

v2:
 - documented format changes in docs/interop/qcow2.txt
   (suggested by Eric Blake)
---
 block/qcow2-snapshot.c |    7 +++++++
 block/qcow2.h          |    2 ++
 docs/interop/qcow2.txt |    4 ++++
 3 files changed, 13 insertions(+)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index bb6a5b7..d682946 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -103,6 +103,12 @@ int qcow2_read_snapshots(BlockDriverState *bs)
             sn->disk_size = bs->total_sectors * BDRV_SECTOR_SIZE;
         }
 
+        if (extra_data_size >= 24) {
+            sn->icount = be64_to_cpu(extra.icount);
+        } else {
+            sn->icount = -1ULL;
+        }
+
         /* Read snapshot ID */
         sn->id_str = g_malloc(id_str_size + 1);
         ret = bdrv_pread(bs->file, offset, sn->id_str, id_str_size);
@@ -209,6 +215,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
         memset(&extra, 0, sizeof(extra));
         extra.vm_state_size_large = cpu_to_be64(sn->vm_state_size);
         extra.disk_size = cpu_to_be64(sn->disk_size);
+        extra.icount = cpu_to_be64(sn->icount);
 
         id_str_size = strlen(sn->id_str);
         name_size = strlen(sn->name);
diff --git a/block/qcow2.h b/block/qcow2.h
index a98d245..8554629 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -159,6 +159,7 @@ typedef struct QEMU_PACKED QCowSnapshotHeader {
 typedef struct QEMU_PACKED QCowSnapshotExtraData {
     uint64_t vm_state_size_large;
     uint64_t disk_size;
+    uint64_t icount;
 } QCowSnapshotExtraData;
 
 
@@ -172,6 +173,7 @@ typedef struct QCowSnapshot {
     uint32_t date_sec;
     uint32_t date_nsec;
     uint64_t vm_clock_nsec;
+    uint64_t icount;
 } QCowSnapshot;
 
 struct Qcow2Cache;
diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
index fb5cb47..c1edef7 100644
--- a/docs/interop/qcow2.txt
+++ b/docs/interop/qcow2.txt
@@ -540,6 +540,10 @@ Snapshot table entry:
 
                     Byte 48 - 55:   Virtual disk size of the snapshot in bytes
 
+                    Byte 56 - 63:   icount value which corresponds to
+                                    the record/replay step when the snapshot
+                                    was taken
+
                     Version 3 images must include extra data at least up to
                     byte 55.
 

[Qemu-devel] [PATCH v9 08/21] migration: introduce icount field for snapshots

[Pavel Dovgalyuk]

Saving icount as a parameters of the snapshot allows navigation between
them in the execution replay scenario.
This information can be used for finding a specific snapshot for rewinding
the recorded execution to the specific moment of the time.
E.g., 'reverse step' action needs to load the nearest snapshot which is
prior to the current moment of time .

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

--

v2:
 - made icount in SnapshotInfo optional (suggested by Eric Blake)
v7:
 - added more comments for icount member (suggested by Markus Armbruster)
v9:
 - updated icount comment
---
 block/qapi.c             |   18 ++++++++++++++----
 block/qcow2-snapshot.c   |    2 ++
 blockdev.c               |   10 ++++++++++
 include/block/snapshot.h |    1 +
 migration/savevm.c       |    5 +++++
 qapi/block-core.json     |    8 +++++++-
 qapi/block.json          |    3 ++-
 7 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index c66f949..ccf23f3 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -210,6 +210,8 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
         info->date_nsec     = sn_tab[i].date_nsec;
         info->vm_clock_sec  = sn_tab[i].vm_clock_nsec / 1000000000;
         info->vm_clock_nsec = sn_tab[i].vm_clock_nsec % 1000000000;
+        info->icount        = sn_tab[i].icount;
+        info->has_icount    = sn_tab[i].icount != -1ULL;
 
         info_list = g_new0(SnapshotInfoList, 1);
         info_list->value = info;
@@ -663,14 +665,15 @@ void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f,
                         QEMUSnapshotInfo *sn)
 {
     char buf1[128], date_buf[128], clock_buf[128];
+    char icount_buf[128] = {0};
     struct tm tm;
     time_t ti;
     int64_t secs;
 
     if (!sn) {
         func_fprintf(f,
-                     "%-10s%-20s%7s%20s%15s",
-                     "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK");
+                     "%-10s%-18s%7s%20s%13s%11s",
+                     "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK", "ICOUNT");
     } else {
         ti = sn->date_sec;
         localtime_r(&ti, &tm);
@@ -683,13 +686,18 @@ void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f,
                  (int)((secs / 60) % 60),
                  (int)(secs % 60),
                  (int)((sn->vm_clock_nsec / 1000000) % 1000));
+        if (sn->icount != -1ULL) {
+            snprintf(icount_buf, sizeof(icount_buf),
+                "%"PRId64, sn->icount);
+        }
         func_fprintf(f,
-                     "%-10s%-20s%7s%20s%15s",
+                     "%-10s%-18s%7s%20s%13s%11s",
                      sn->id_str, sn->name,
                      get_human_readable_size(buf1, sizeof(buf1),
                                              sn->vm_state_size),
                      date_buf,
-                     clock_buf);
+                     clock_buf,
+                     icount_buf);
     }
 }
 
@@ -857,6 +865,8 @@ void bdrv_image_info_dump(fprintf_function func_fprintf, void *f,
                 .date_nsec = elem->value->date_nsec,
                 .vm_clock_nsec = elem->value->vm_clock_sec * 1000000000ULL +
                                  elem->value->vm_clock_nsec,
+                .icount = elem->value->has_icount ?
+                          elem->value->icount : -1ULL,
             };
 
             pstrcpy(sn.id_str, sizeof(sn.id_str), elem->value->id);
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index d682946..96b57f4 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -379,6 +379,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
     sn->date_sec = sn_info->date_sec;
     sn->date_nsec = sn_info->date_nsec;
     sn->vm_clock_nsec = sn_info->vm_clock_nsec;
+    sn->icount = sn_info->icount;
 
     /* Allocate the L1 table of the snapshot and copy the current one there. */
     l1_table_offset = qcow2_alloc_clusters(bs, s->l1_size * sizeof(uint64_t));
@@ -698,6 +699,7 @@ int qcow2_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
         sn_info->date_sec = sn->date_sec;
         sn_info->date_nsec = sn->date_nsec;
         sn_info->vm_clock_nsec = sn->vm_clock_nsec;
+        sn_info->icount = sn->icount;
     }
     *psn_tab = sn_tab;
     return s->nb_snapshots;
diff --git a/blockdev.c b/blockdev.c
index a6f71f9..f6c5f6c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -57,6 +57,7 @@
 #include "block/trace.h"
 #include "sysemu/arch_init.h"
 #include "sysemu/qtest.h"
+#include "sysemu/replay.h"
 #include "qemu/cutils.h"
 #include "qemu/help_option.h"
 #include "qemu/throttle-options.h"
@@ -1239,6 +1240,10 @@ SnapshotInfo *qmp_blockdev_snapshot_delete_internal_sync(const char *device,
     info->vm_state_size = sn.vm_state_size;
     info->vm_clock_nsec = sn.vm_clock_nsec % 1000000000;
     info->vm_clock_sec = sn.vm_clock_nsec / 1000000000;
+    if (sn.icount != -1ULL) {
+        info->icount = sn.icount;
+        info->has_icount = true;
+    }
 
     return info;
 
@@ -1447,6 +1452,11 @@ static void internal_snapshot_prepare(BlkActionState *common,
     sn->date_sec = tv.tv_sec;
     sn->date_nsec = tv.tv_usec * 1000;
     sn->vm_clock_nsec = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+    if (replay_mode != REPLAY_MODE_NONE) {
+        sn->icount = replay_get_current_step();
+    } else {
+        sn->icount = -1ULL;
+    }
 
     ret1 = bdrv_snapshot_create(bs, sn);
     if (ret1 < 0) {
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index f73d109..c9c8975 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -42,6 +42,7 @@ typedef struct QEMUSnapshotInfo {
     uint32_t date_sec; /* UTC date of the snapshot */
     uint32_t date_nsec;
     uint64_t vm_clock_nsec; /* VM clock relative to boot */
+    uint64_t icount; /* record/replay step */
 } QEMUSnapshotInfo;
 
 int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
diff --git a/migration/savevm.c b/migration/savevm.c
index 9e45fb4..a031e5b 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2509,6 +2509,11 @@ int save_snapshot(const char *name, Error **errp)
     sn->date_sec = tv.tv_sec;
     sn->date_nsec = tv.tv_usec * 1000;
     sn->vm_clock_nsec = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+    if (replay_mode != REPLAY_MODE_NONE) {
+        sn->icount = replay_get_current_step();
+    } else {
+        sn->icount = -1ULL;
+    }
 
     if (name) {
         ret = bdrv_snapshot_find(bs, old_sn, name);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 762000f..a197e5f 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -26,13 +26,19 @@
 #
 # @vm-clock-nsec: fractional part in nano seconds to be used with vm-clock-sec
 #
+# @icount: Current instruction count. Appears when execution record/replay
+#          is enabled. Used for "time-traveling" to match the moment
+#          in the recorded execution with the snapshots. This counter may
+#          be obtained through @query-replay command (since 4.0)
+#
 # Since: 1.3
 #
 ##
 { 'struct': 'SnapshotInfo',
   'data': { 'id': 'str', 'name': 'str', 'vm-state-size': 'int',
             'date-sec': 'int', 'date-nsec': 'int',
-            'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } }
+            'vm-clock-sec': 'int', 'vm-clock-nsec': 'int',
+            '*icount': 'int' } }
 
 ##
 # @ImageInfoSpecificQCow2EncryptionBase:
diff --git a/qapi/block.json b/qapi/block.json
index 11f01f2..a6396a9 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -176,7 +176,8 @@
 #                    "date-sec": 1000012,
 #                    "date-nsec": 10,
 #                    "vm-clock-sec": 100,
-#                    "vm-clock-nsec": 20
+#                    "vm-clock-nsec": 20,
+#                    "icount": 220414
 #      }
 #    }
 #

[Qemu-devel] [PATCH v9 09/21] replay: provide and accessor for rr filename

[Pavel Dovgalyuk]

This patch adds an accessor function for the name of the record/replay
log file. Adding an accessor instead of making variable global,
prevents accidental modification of this variable by other modules.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
---
 include/sysemu/replay.h |    2 ++
 replay/replay.c         |    5 +++++
 2 files changed, 7 insertions(+)

diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 3a7c58e..b3f593f 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -71,6 +71,8 @@ void replay_start(void);
 void replay_finish(void);
 /*! Adds replay blocker with the specified error description */
 void replay_add_blocker(Error *reason);
+/* Returns name of the replay log file */
+const char *replay_get_filename(void);
 
 /* Processing the instructions */
 
diff --git a/replay/replay.c b/replay/replay.c
index b75820a..aa53411 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -394,3 +394,8 @@ void replay_add_blocker(Error *reason)
 {
     replay_blockers = g_slist_prepend(replay_blockers, reason);
 }
+
+const char *replay_get_filename(void)
+{
+    return replay_filename;
+}

[Qemu-devel] [PATCH v9 10/21] qapi: introduce replay.json for record/replay-related stuff

[Pavel Dovgalyuk]

This patch adds replay.json file. It will be
used for adding record/replay-related data structures and commands.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 MAINTAINERS             |    1 +
 Makefile.objs           |    4 ++--
 include/sysemu/replay.h |    1 +
 qapi/misc.json          |   18 ------------------
 qapi/qapi-schema.json   |    1 +
 qapi/replay.json        |   26 ++++++++++++++++++++++++++
 6 files changed, 31 insertions(+), 20 deletions(-)
 create mode 100644 qapi/replay.json

diff --git a/MAINTAINERS b/MAINTAINERS
index 0bfd95a..7b0a1bc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2122,6 +2122,7 @@ F: net/filter-replay.c
 F: include/sysemu/replay.h
 F: docs/replay.txt
 F: stubs/replay.c
+F: qapi/replay.json
 
 IOVA Tree
 M: Peter Xu <peterx@redhat.com>
diff --git a/Makefile.objs b/Makefile.objs
index 4561159..8164b22 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -1,6 +1,6 @@
 QAPI_MODULES = block-core block char common crypto introspect job migration
-QAPI_MODULES += misc net rdma rocker run-state sockets tpm trace transaction
-QAPI_MODULES += ui
+QAPI_MODULES += misc net rdma replay rocker run-state sockets tpm trace
+QAPI_MODULES += transaction ui
 
 #######################################################################
 # Common libraries for tools and emulators
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index b3f593f..2054296 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -13,6 +13,7 @@
  */
 
 #include "sysemu.h"
+#include "qapi/qapi-types-replay.h"
 #include "qapi/qapi-types-misc.h"
 #include "qapi/qapi-types-ui.h"
 
diff --git a/qapi/misc.json b/qapi/misc.json
index 24d20a8..e5e0bea 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -3125,24 +3125,6 @@
   'data': { 'offset': 'int' } }
 
 ##
-# @ReplayMode:
-#
-# Mode of the replay subsystem.
-#
-# @none: normal execution mode. Replay or record are not enabled.
-#
-# @record: record mode. All non-deterministic data is written into the
-#          replay log.
-#
-# @play: replay mode. Non-deterministic data required for system execution
-#        is read from the log.
-#
-# Since: 2.5
-##
-{ 'enum': 'ReplayMode',
-  'data': [ 'none', 'record', 'play' ] }
-
-##
 # @xen-load-devices-state:
 #
 # Load the state of all devices from file. The RAM and the block devices
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index 3bbdfce..b85fd04 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -95,3 +95,4 @@
 { 'include': 'trace.json' }
 { 'include': 'introspect.json' }
 { 'include': 'misc.json' }
+{ 'include': 'replay.json' }
diff --git a/qapi/replay.json b/qapi/replay.json
new file mode 100644
index 0000000..9e13551
--- /dev/null
+++ b/qapi/replay.json
@@ -0,0 +1,26 @@
+# -*- Mode: Python -*-
+#
+
+##
+# = Record/replay
+##
+
+{ 'include': 'common.json' }
+
+##
+# @ReplayMode:
+#
+# Mode of the replay subsystem.
+#
+# @none: normal execution mode. Replay or record are not enabled.
+#
+# @record: record mode. All non-deterministic data is written into the
+#          replay log.
+#
+# @play: replay mode. Non-deterministic data required for system execution
+#        is read from the log.
+#
+# Since: 2.5
+##
+{ 'enum': 'ReplayMode',
+  'data': [ 'none', 'record', 'play' ] }

[Qemu-devel] [PATCH v9 11/21] replay: introduce info hmp/qmp command

[Pavel Dovgalyuk]

This patch introduces 'info replay' monitor command and
corresponding qmp request.
These commands request the current record/replay mode, replay log file name,
and the execution step (number or recorded/replayed instructions).
User may use step number for replay_seek/replay_break commands and
for controlling the execution of replay.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

--

v2:
 - renamed info_replay qmp into query-replay (suggested by Eric Blake)
v7:
 - added empty line (suggested by Markus Armbruster)
v9:
 - changed 'step' parameter name to 'icount'
 - moved json stuff to replay.json and updated the descriptions
   (suggested by Markus Armbruster)
---
 hmp-commands-info.hx      |   14 ++++++++++++++
 hmp.h                     |    1 +
 qapi/replay.json          |   39 +++++++++++++++++++++++++++++++++++++++
 replay/Makefile.objs      |    3 ++-
 replay/replay-debugging.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 98 insertions(+), 1 deletion(-)
 create mode 100644 replay/replay-debugging.c

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index cbee8b9..866bc0f 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -918,6 +918,20 @@ STEXI
 Show SEV information.
 ETEXI
 
+    {
+        .name       = "replay",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show parameters of the record/replay",
+        .cmd        = hmp_info_replay,
+    },
+
+STEXI
+@item info replay
+@findex info replay
+Display the record/replay information: mode and the current icount.
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/hmp.h b/hmp.h
index 5f1addc..d792149 100644
--- a/hmp.h
+++ b/hmp.h
@@ -148,5 +148,6 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict);
 void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
 void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict);
 void hmp_info_sev(Monitor *mon, const QDict *qdict);
+void hmp_info_replay(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi/replay.json b/qapi/replay.json
index 9e13551..d7e76cf 100644
--- a/qapi/replay.json
+++ b/qapi/replay.json
@@ -24,3 +24,42 @@
 ##
 { 'enum': 'ReplayMode',
   'data': [ 'none', 'record', 'play' ] }
+
+##
+# @ReplayInfo:
+#
+# Record/replay information.
+#
+# @mode: current mode.
+#
+# @filename: name of the record/replay log file.
+#            It is present only in record or replay modes, when the log
+#            is recorded or replayed.
+#
+# @icount: current number of executed instructions.
+#
+# Since: 4.0
+#
+##
+{ 'struct': 'ReplayInfo',
+  'data': { 'mode': 'ReplayMode', '*filename': 'str', 'icount': 'int' } }
+
+##
+# @query-replay:
+#
+# Retrieves the record/replay information.
+# It includes current icount which may be used for replay-break and
+# replay-seek commands.
+#
+# Returns: structure with the properties of the record/replay.
+#
+# Since: 4.0
+#
+# Example:
+#
+# -> { "execute": "query-replay" }
+# <- { "return": { "mode": "play", "filename": "log.rr", "icount": 220414 } }
+#
+##
+{ 'command': 'query-replay',
+  'returns': 'ReplayInfo' }
diff --git a/replay/Makefile.objs b/replay/Makefile.objs
index cee6539..6694e3e 100644
--- a/replay/Makefile.objs
+++ b/replay/Makefile.objs
@@ -6,4 +6,5 @@ common-obj-y += replay-input.o
 common-obj-y += replay-char.o
 common-obj-y += replay-snapshot.o
 common-obj-y += replay-net.o
-common-obj-y += replay-audio.o
\ No newline at end of file
+common-obj-y += replay-audio.o
+common-obj-y += replay-debugging.o
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
new file mode 100644
index 0000000..9956405
--- /dev/null
+++ b/replay/replay-debugging.c
@@ -0,0 +1,42 @@
+/*
+ * replay-debugging.c
+ *
+ * Copyright (c) 2010-2018 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 "qemu/osdep.h"
+#include "qapi/error.h"
+#include "sysemu/replay.h"
+#include "replay-internal.h"
+#include "hmp.h"
+#include "monitor/monitor.h"
+#include "qapi/qapi-commands-replay.h"
+
+void hmp_info_replay(Monitor *mon, const QDict *qdict)
+{
+    if (replay_mode == REPLAY_MODE_NONE) {
+        monitor_printf(mon, "No record/replay\n");
+    } else {
+        monitor_printf(mon, "%s execution '%s': current step = %"PRId64"\n",
+            replay_mode == REPLAY_MODE_RECORD ? "Recording" : "Replaying",
+            replay_get_filename(), replay_get_current_step());
+    }
+}
+
+ReplayInfo *qmp_query_replay(Error **errp)
+{
+    ReplayInfo *retval = g_new0(ReplayInfo, 1);
+
+    retval->mode = replay_mode;
+    if (replay_get_filename()) {
+        retval->filename = g_strdup(replay_get_filename());
+        retval->has_filename = true;
+    }
+    retval->icount = replay_get_current_step();
+    return retval;
+}

[Qemu-devel] [PATCH v9 12/21] replay: introduce breakpoint at the specified step

[Pavel Dovgalyuk]

This patch introduces replay_break, replay_delete_break
qmp and hmp commands.
These commands allow stopping at the specified instruction.
It may be useful for debugging when there are some known
events that should be investigated.
replay_break command has one argument - number of instructions
executed since the start of the replay.
replay_delete_break removes previously set breakpoint.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

--

v2:
 - renamed replay_break qmp command into replay-break
   (suggested by Eric Blake)
v7:
 - introduces replay_delete_break command
v9:
 - changed 'step' parameter name to 'icount'
 - moved json stuff to replay.json and updated the description
   (suggested by Markus Armbruster)
---
 hmp-commands.hx           |   34 ++++++++++++++++++
 hmp.h                     |    2 +
 qapi/replay.json          |   36 +++++++++++++++++++
 replay/replay-debugging.c |   85 +++++++++++++++++++++++++++++++++++++++++++++
 replay/replay-internal.h  |    4 ++
 replay/replay.c           |   17 +++++++++
 6 files changed, 178 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index ba71558..6d04c02 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1890,6 +1890,40 @@ Set QOM property @var{property} of object at location @var{path} to value @var{v
 ETEXI
 
     {
+        .name       = "replay_break",
+        .args_type  = "icount:i",
+        .params     = "icount",
+        .help       = "sets breakpoint on the step specified by the icount of the replay",
+        .cmd        = hmp_replay_break,
+    },
+
+STEXI
+@item replay_break @var{icount}
+@findex replay_break
+Set breakpoint on the step of the replay specified by @var{icount}.
+Execution stops when the specified @var{icount} is reached.
+icount for the reference may be observed with 'info replay' command.
+There could be at most one breakpoint. When breakpoint is set, the prior
+one is removed. The breakpoints may be set only in replay mode and only
+at the step in the future.
+ETEXI
+
+    {
+        .name       = "replay_delete_break",
+        .args_type  = "",
+        .params     = "",
+        .help       = "removes replay breakpoint",
+        .cmd        = hmp_replay_delete_break,
+    },
+
+STEXI
+@item replay_delete_break
+@findex replay_delete_break
+Removes replay breakpoint which was previously set with replay_break.
+The command is ignored when there are no replay breakpoints.
+ETEXI
+
+    {
         .name       = "info",
         .args_type  = "item:s?",
         .params     = "[subcommand]",
diff --git a/hmp.h b/hmp.h
index d792149..c9b9b4f 100644
--- a/hmp.h
+++ b/hmp.h
@@ -149,5 +149,7 @@ void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
 void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict);
 void hmp_info_sev(Monitor *mon, const QDict *qdict);
 void hmp_info_replay(Monitor *mon, const QDict *qdict);
+void hmp_replay_break(Monitor *mon, const QDict *qdict);
+void hmp_replay_delete_break(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi/replay.json b/qapi/replay.json
index d7e76cf..a63219c 100644
--- a/qapi/replay.json
+++ b/qapi/replay.json
@@ -63,3 +63,39 @@
 ##
 { 'command': 'query-replay',
   'returns': 'ReplayInfo' }
+
+##
+# @replay-break:
+#
+# Set breakpoint on the step of the replay specified by @icount.
+# Execution stops when the specified @icount is reached.
+# icount for the reference may be obtained with @query-replay command.
+# There could be at most one breakpoint. When breakpoint is set, the prior
+# one is removed. The breakpoints may be set only in replay mode and only
+# at the step in the future.
+#
+# @icount: execution step to stop at
+#
+# Since: 4.0
+#
+# Example:
+#
+# -> { "execute": "replay-break", "data": { "icount": 220414 } }
+#
+##
+{ 'command': 'replay-break', 'data': { 'icount': 'int' } }
+
+##
+# @replay-delete-break:
+#
+# Removes replay breakpoint which was set with @replay-break.
+# The command is ignored when there are no replay breakpoints.
+#
+# Since: 4.0
+#
+# Example:
+#
+# -> { "execute": "replay-delete-break" }
+#
+##
+{ 'command': 'replay-delete-break' }
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index 9956405..8ee64b2 100644
--- a/replay/replay-debugging.c
+++ b/replay/replay-debugging.c
@@ -16,6 +16,8 @@
 #include "hmp.h"
 #include "monitor/monitor.h"
 #include "qapi/qapi-commands-replay.h"
+#include "qapi/qmp/qdict.h"
+#include "qemu/timer.h"
 
 void hmp_info_replay(Monitor *mon, const QDict *qdict)
 {
@@ -40,3 +42,86 @@ ReplayInfo *qmp_query_replay(Error **errp)
     retval->icount = replay_get_current_step();
     return retval;
 }
+
+static void replay_break(uint64_t step, QEMUTimerCB callback, void *opaque)
+{
+    assert(replay_mode == REPLAY_MODE_PLAY);
+    assert(replay_mutex_locked());
+    assert(replay_break_step >= replay_get_current_step());
+    assert(callback);
+
+    replay_break_step = step;
+
+    if (replay_break_timer) {
+        timer_del(replay_break_timer);
+    } else {
+        replay_break_timer = timer_new_ns(QEMU_CLOCK_REALTIME,
+                                          callback, opaque);
+    }
+}
+
+static void replay_delete_break(void)
+{
+    assert(replay_mode == REPLAY_MODE_PLAY);
+    assert(replay_mutex_locked());
+
+    if (replay_break_timer) {
+        timer_del(replay_break_timer);
+        timer_free(replay_break_timer);
+        replay_break_timer = NULL;
+    }
+    replay_break_step = -1ULL;
+}
+
+static void replay_stop_vm(void *opaque)
+{
+    vm_stop(RUN_STATE_PAUSED);
+    replay_delete_break();
+}
+
+void qmp_replay_break(int64_t icount, Error **errp)
+{
+    if (replay_mode == REPLAY_MODE_PLAY) {
+        if (icount >= replay_get_current_step()) {
+            replay_break(icount, replay_stop_vm, NULL);
+        } else {
+            error_setg(errp, "cannot set breakpoint at the step in the past");
+        }
+    } else {
+        error_setg(errp, "setting the breakpoint is allowed only in play mode");
+    }
+}
+
+void hmp_replay_break(Monitor *mon, const QDict *qdict)
+{
+    int64_t step = qdict_get_try_int(qdict, "icount", -1LL);
+    Error *err = NULL;
+
+    qmp_replay_break(step, &err);
+    if (err) {
+        error_report_err(err);
+        error_free(err);
+        return;
+    }
+}
+
+void qmp_replay_delete_break(Error **errp)
+{
+    if (replay_mode == REPLAY_MODE_PLAY) {
+        replay_delete_break();
+    } else {
+        error_setg(errp, "replay breakpoints are allowed only in play mode");
+    }
+}
+
+void hmp_replay_delete_break(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+
+    qmp_replay_delete_break(&err);
+    if (err) {
+        error_report_err(err);
+        error_free(err);
+        return;
+    }
+}
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index af6f4d5..94b7e9b 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -91,6 +91,10 @@ extern ReplayState replay_state;
 
 /* File for replay writing */
 extern FILE *replay_file;
+/* Step of the replay breakpoint */
+extern uint64_t replay_break_step;
+/* Timer for the replay breakpoint callback */
+extern QEMUTimer *replay_break_timer;
 
 void replay_put_byte(uint8_t byte);
 void replay_put_event(uint8_t event);
diff --git a/replay/replay.c b/replay/replay.c
index aa53411..3996499 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -34,6 +34,10 @@ static char *replay_filename;
 ReplayState replay_state;
 static GSList *replay_blockers;
 
+/* Replay breakpoints */
+uint64_t replay_break_step = -1ULL;
+QEMUTimer *replay_break_timer;
+
 bool replay_next_event_is(int event)
 {
     bool res = false;
@@ -73,6 +77,13 @@ int replay_get_instructions(void)
     replay_mutex_lock();
     if (replay_next_event_is(EVENT_INSTRUCTION)) {
         res = replay_state.instructions_count;
+        if (replay_break_step != -1LL) {
+            uint64_t current = replay_get_current_step();
+            assert(replay_break_step >= current);
+            if (current + res > replay_break_step) {
+                res = replay_break_step - current;
+            }
+        }
     }
     replay_mutex_unlock();
     return res;
@@ -99,6 +110,12 @@ void replay_account_executed_instructions(void)
                    will be read from the log. */
                 qemu_notify_event();
             }
+            /* Execution reached the break step */
+            if (replay_break_step == replay_state.current_step) {
+                /* Cannot make callback directly from the vCPU thread */
+                timer_mod_ns(replay_break_timer,
+                    qemu_clock_get_ns(QEMU_CLOCK_REALTIME));
+            }
         }
     }
 }

[Qemu-devel] [PATCH v9 13/21] replay: implement replay-seek command to proceed to the desired step

[Pavel Dovgalyuk]

This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
the execution to the specified step.
The command automatically loads nearest snapshot and replay the execution
to find the desired step.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

--

v2:
 - renamed replay_seek qmp command into replay-seek
   (suggested by Eric Blake)
v7:
 - small fixes related to Markus Armbruster's review
v9:
 - changed 'step' parameter name to 'icount'
 - moved json stuff to replay.json and updated the description
   (suggested by Markus Armbruster)
---
 hmp-commands.hx           |   19 +++++++++
 hmp.h                     |    1 
 qapi/replay.json          |   20 ++++++++++
 replay/replay-debugging.c |   91 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 131 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 6d04c02..2839acc 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1924,6 +1924,25 @@ The command is ignored when there are no replay breakpoints.
 ETEXI
 
     {
+        .name       = "replay_seek",
+        .args_type  = "icount:i",
+        .params     = "icount",
+        .help       = "rewinds replay to the step specified by icount",
+        .cmd        = hmp_replay_seek,
+    },
+
+STEXI
+@item replay_seek @var{icount}
+@findex replay_seek
+Automatically proceeds to the step specified by @var{icount}, when
+replaying the execution. The command automatically loads nearest
+snapshot and replay the execution to find the desired step.
+When there is no preceding snapshot or the execution is not replayed,
+then the command is ignored.
+icount for the reference may be observed with 'info replay' command.
+ETEXI
+
+    {
         .name       = "info",
         .args_type  = "item:s?",
         .params     = "[subcommand]",
diff --git a/hmp.h b/hmp.h
index c9b9b4f..d6e1d7e 100644
--- a/hmp.h
+++ b/hmp.h
@@ -151,5 +151,6 @@ void hmp_info_sev(Monitor *mon, const QDict *qdict);
 void hmp_info_replay(Monitor *mon, const QDict *qdict);
 void hmp_replay_break(Monitor *mon, const QDict *qdict);
 void hmp_replay_delete_break(Monitor *mon, const QDict *qdict);
+void hmp_replay_seek(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi/replay.json b/qapi/replay.json
index a63219c..7390f88 100644
--- a/qapi/replay.json
+++ b/qapi/replay.json
@@ -99,3 +99,23 @@
 #
 ##
 { 'command': 'replay-delete-break' }
+
+##
+# @replay-seek:
+#
+# Automatically proceeds to the step specified by @icount when replaying
+# the execution. The command automatically loads nearest
+# snapshot and replay the execution to find the desired step.
+# When there is no preceding snapshot or the execution is not replayed,
+# then the command is ignored.
+# icount for the reference may be obtained with @query-replay command.
+#
+# @icount: destination execution step
+#
+# Since: 4.0
+#
+# Example:
+#
+# -> { "execute": "replay-seek", "data": { "icount": 220414 } }
+##
+{ 'command': 'replay-seek', 'data': { 'icount': 'int' } }
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index 8ee64b2..39848dc 100644
--- a/replay/replay-debugging.c
+++ b/replay/replay-debugging.c
@@ -18,6 +18,8 @@
 #include "qapi/qapi-commands-replay.h"
 #include "qapi/qmp/qdict.h"
 #include "qemu/timer.h"
+#include "block/snapshot.h"
+#include "migration/snapshot.h"
 
 void hmp_info_replay(Monitor *mon, const QDict *qdict)
 {
@@ -125,3 +127,92 @@ void hmp_replay_delete_break(Monitor *mon, const QDict *qdict)
         return;
     }
 }
+
+static char *replay_find_nearest_snapshot(int64_t step, int64_t* snapshot_step)
+{
+    BlockDriverState *bs;
+    QEMUSnapshotInfo *sn_tab;
+    QEMUSnapshotInfo *nearest = NULL;
+    char *ret = NULL;
+    int nb_sns, i;
+    AioContext *aio_context;
+
+    *snapshot_step = -1;
+
+    bs = bdrv_all_find_vmstate_bs();
+    if (!bs) {
+        goto fail;
+    }
+    aio_context = bdrv_get_aio_context(bs);
+
+    aio_context_acquire(aio_context);
+    nb_sns = bdrv_snapshot_list(bs, &sn_tab);
+    aio_context_release(aio_context);
+
+    for (i = 0; i < nb_sns; i++) {
+        if (bdrv_all_find_snapshot(sn_tab[i].name, &bs) == 0) {
+            if (sn_tab[i].icount != -1ULL
+                && sn_tab[i].icount <= step
+                && (!nearest || nearest->icount < sn_tab[i].icount)) {
+                nearest = &sn_tab[i];
+            }
+        }
+    }
+    if (nearest) {
+        ret = g_strdup(nearest->name);
+        *snapshot_step = nearest->icount;
+    }
+    g_free(sn_tab);
+
+fail:
+    return ret;
+}
+
+static void replay_seek(int64_t step, QEMUTimerCB callback, Error **errp)
+{
+    char *snapshot = NULL;
+    int64_t snapshot_step;
+
+    if (replay_mode != REPLAY_MODE_PLAY) {
+        error_setg(errp, "replay must be enabled to seek");
+        return;
+    }
+    if (!replay_snapshot) {
+        error_setg(errp, "snapshotting is disabled");
+        return;
+    }
+
+    snapshot = replay_find_nearest_snapshot(step, &snapshot_step);
+    if (snapshot) {
+        if (step < replay_get_current_step()
+            || replay_get_current_step() < snapshot_step) {
+            vm_stop(RUN_STATE_RESTORE_VM);
+            load_snapshot(snapshot, errp);
+        }
+        g_free(snapshot);
+    }
+    if (replay_get_current_step() <= step) {
+        replay_break(step, callback, NULL);
+        vm_start();
+    } else {
+        error_setg(errp, "cannot seek to the specified step");
+    }
+}
+
+void qmp_replay_seek(int64_t icount, Error **errp)
+{
+    replay_seek(icount, replay_stop_vm, errp);
+}
+
+void hmp_replay_seek(Monitor *mon, const QDict *qdict)
+{
+    int64_t step = qdict_get_try_int(qdict, "icount", -1LL);
+    Error *err = NULL;
+
+    qmp_replay_seek(step, &err);
+    if (err) {
+        error_report_err(err);
+        error_free(err);
+        return;
+    }
+}

[Qemu-devel] [PATCH v9 14/21] replay: refine replay-time module

[Pavel Dovgalyuk]

This patch removes refactoring artifacts from the replay/replay-time.c

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
---
 replay/replay-time.c |   36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/replay/replay-time.c b/replay/replay-time.c
index 0df1693..60f47b7 100644
--- a/replay/replay-time.c
+++ b/replay/replay-time.c
@@ -15,18 +15,19 @@
 #include "replay-internal.h"
 #include "qemu/error-report.h"
 
-int64_t replay_save_clock(ReplayClockKind kind, int64_t clock, int64_t raw_icount)
+int64_t replay_save_clock(ReplayClockKind kind, int64_t clock,
+                          int64_t raw_icount)
 {
-    if (replay_file) {
-        g_assert(replay_mutex_locked());
+    g_assert(replay_file);
+    g_assert(replay_mutex_locked());
 
-        /* Due to the caller's locking requirements we get the icount from it
-         * instead of using replay_save_instructions().
-         */
-        replay_advance_current_step(raw_icount);
-        replay_put_event(EVENT_CLOCK + kind);
-        replay_put_qword(clock);
-    }
+    /*
+     * Due to the caller's locking requirements we get the icount from it
+     * instead of using replay_save_instructions().
+     */
+    replay_advance_current_step(raw_icount);
+    replay_put_event(EVENT_CLOCK + kind);
+    replay_put_qword(clock);
 
     return clock;
 }
@@ -48,20 +49,15 @@ void replay_read_next_clock(ReplayClockKind kind)
 /*! Reads next clock event from the input. */
 int64_t replay_read_clock(ReplayClockKind kind)
 {
+    int64_t ret;
     g_assert(replay_file && replay_mutex_locked());
 
     replay_account_executed_instructions();
 
-    if (replay_file) {
-        int64_t ret;
-        if (replay_next_event_is(EVENT_CLOCK + kind)) {
-            replay_read_next_clock(kind);
-        }
-        ret = replay_state.cached_clock[kind];
-
-        return ret;
+    if (replay_next_event_is(EVENT_CLOCK + kind)) {
+        replay_read_next_clock(kind);
     }
+    ret = replay_state.cached_clock[kind];
 
-    error_report("REPLAY INTERNAL ERROR %d", __LINE__);
-    exit(1);
+    return ret;
 }

[Qemu-devel] [PATCH v9 15/21] replay: flush rr queue before loading the vmstate

[Pavel Dovgalyuk]

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.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
---
 include/sysemu/replay.h  |    2 ++
 migration/savevm.c       |    6 ++++++
 replay/replay-internal.h |    2 --
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 2054296..1b758ec 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -141,6 +141,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 input event to the queue */
diff --git a/migration/savevm.c b/migration/savevm.c
index a031e5b..5a9902d 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2701,6 +2701,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();
+
     /* Flush all IO requests so they don't interfere with the new state.  */
     bdrv_drain_all_begin();
 
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index 94b7e9b..e37b201 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -146,8 +146,6 @@ void replay_read_next_clock(unsigned int kind);
 void replay_init_events(void);
 /*! Clears internal data structures for events handling */
 void replay_finish_events(void);
-/*! Flushes events queue */
-void replay_flush_events(void);
 /*! Returns true if there are any unsaved events in the queue */
 bool replay_has_events(void);
 /*! Saves events from queue into the file */

[Qemu-devel] [PATCH v9 16/21] gdbstub: add reverse step support in replay mode

[Pavel Dovgalyuk]

GDB remote protocol supports two reverse debugging commands:
reverse step and reverse continue.
This patch adds support of the first one to the gdbstub.
Reverse step is intended to step one instruction in the backwards
direction. This is not possible in regular execution.
But replayed execution is deterministic, therefore we can load one of
the prior snapshots and proceed to the desired step. It is equivalent
to stepping one instruction back.
There should be at least one snapshot preceding the debugged part of
the replay log.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
---
 accel/tcg/translator.c    |    1 +
 cpus.c                    |   14 +++++++++++---
 exec.c                    |    7 +++++++
 gdbstub.c                 |   44 +++++++++++++++++++++++++++++++++++++++++---
 include/sysemu/replay.h   |   11 +++++++++++
 replay/replay-debugging.c |   33 +++++++++++++++++++++++++++++++++
 stubs/replay.c            |    5 +++++
 7 files changed, 109 insertions(+), 6 deletions(-)

diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index afd0a49..33a543e 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -17,6 +17,7 @@
 #include "exec/gen-icount.h"
 #include "exec/log.h"
 #include "exec/translator.h"
+#include "sysemu/replay.h"
 
 /* Pairs with tcg_clear_temp_count.
    To be called by #TranslatorOps.{translate_insn,tb_stop} if
diff --git a/cpus.c b/cpus.c
index aa33fb1..013341c 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1104,9 +1104,17 @@ static bool cpu_can_run(CPUState *cpu)
 
 static void cpu_handle_guest_debug(CPUState *cpu)
 {
-    gdb_set_stop_cpu(cpu);
-    qemu_system_debug_request();
-    cpu->stopped = true;
+    if (!replay_running_debug()) {
+        gdb_set_stop_cpu(cpu);
+        qemu_system_debug_request();
+        cpu->stopped = true;
+    } else {
+        if (!cpu->singlestep_enabled) {
+            cpu_single_step(cpu, SSTEP_ENABLE);
+        } else {
+            cpu_single_step(cpu, 0);
+        }
+    }
 }
 
 #ifdef CONFIG_LINUX
diff --git a/exec.c b/exec.c
index 6e875f0..10f2927 100644
--- a/exec.c
+++ b/exec.c
@@ -2739,6 +2739,13 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags)
     QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) {
         if (cpu_watchpoint_address_matches(wp, vaddr, len)
             && (wp->flags & flags)) {
+            if (replay_running_debug()) {
+                /*
+                 * Don't process the watchpoints when we are
+                 * in a reverse debugging operation.
+                 */
+                return;
+            }
             if (flags == BP_MEM_READ) {
                 wp->flags |= BP_WATCHPOINT_HIT_READ;
             } else {
diff --git a/gdbstub.c b/gdbstub.c
index bfc7afb..442187d 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -39,6 +39,7 @@
 #include "sysemu/kvm.h"
 #include "exec/semihost.h"
 #include "exec/exec-all.h"
+#include "sysemu/replay.h"
 
 #ifdef CONFIG_USER_ONLY
 #define GDB_ATTACHED "0"
@@ -344,6 +345,20 @@ typedef struct GDBState {
  */
 static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER;
 
+/* Retrieves flags for single step mode. */
+static int get_sstep_flags(void)
+{
+    /*
+     * In replay mode all events written into the log should be replayed.
+     * That is why NOIRQ flag is removed in this mode.
+     */
+    if (replay_mode != REPLAY_MODE_NONE) {
+        return SSTEP_ENABLE;
+    } else {
+        return sstep_flags;
+    }
+}
+
 static GDBState *gdbserver_state;
 
 bool gdb_has_xml;
@@ -434,7 +449,7 @@ static int gdb_continue_partial(GDBState *s, char *newstates)
     CPU_FOREACH(cpu) {
         if (newstates[cpu->cpu_index] == 's') {
             trace_gdbstub_op_stepping(cpu->cpu_index);
-            cpu_single_step(cpu, sstep_flags);
+            cpu_single_step(cpu, get_sstep_flags());
         }
     }
     s->running_state = 1;
@@ -453,7 +468,7 @@ static int gdb_continue_partial(GDBState *s, char *newstates)
                 break; /* nothing to do here */
             case 's':
                 trace_gdbstub_op_stepping(cpu->cpu_index);
-                cpu_single_step(cpu, sstep_flags);
+                cpu_single_step(cpu, get_sstep_flags());
                 cpu_resume(cpu);
                 flag = 1;
                 break;
@@ -1433,9 +1448,28 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
             addr = strtoull(p, (char **)&p, 16);
             gdb_set_cpu_pc(s, addr);
         }
-        cpu_single_step(s->c_cpu, sstep_flags);
+        cpu_single_step(s->c_cpu, get_sstep_flags());
         gdb_continue(s);
         return RS_IDLE;
+    case 'b':
+        /* Backward debugging commands */
+        if (replay_mode == REPLAY_MODE_PLAY) {
+            switch (*p) {
+            case 's':
+                if (replay_reverse_step()) {
+                    gdb_continue(s);
+                    return RS_IDLE;
+                } else {
+                    put_packet(s, "E14");
+                    break;
+                }
+            default:
+                goto unknown_command;
+            }
+        } else {
+            put_packet(s, "E22");
+        }
+        goto unknown_command;
     case 'F':
         {
             target_ulong ret;
@@ -1738,6 +1772,10 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
             }
             pstrcat(buf, sizeof(buf), ";multiprocess+");
 
+            if (replay_mode == REPLAY_MODE_PLAY) {
+                pstrcat(buf, sizeof(buf), ";ReverseStep+");
+            }
+
             put_packet(s, buf);
             break;
         }
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 1b758ec..b0e2309 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -74,6 +74,17 @@ void replay_finish(void);
 void replay_add_blocker(Error *reason);
 /* Returns name of the replay log file */
 const char *replay_get_filename(void);
+/*
+ * Start making one step in backward direction.
+ * Used by gdbstub for backwards debugging.
+ * Returns true on success.
+ */
+bool replay_reverse_step(void);
+/*
+ * Returns true if replay module is processing
+ * reverse_continue or reverse_step request
+ */
+bool replay_running_debug(void);
 
 /* Processing the instructions */
 
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index 39848dc..dfade54 100644
--- a/replay/replay-debugging.c
+++ b/replay/replay-debugging.c
@@ -21,6 +21,13 @@
 #include "block/snapshot.h"
 #include "migration/snapshot.h"
 
+static bool replay_is_debugging;
+
+bool replay_running_debug(void)
+{
+    return replay_is_debugging;
+}
+
 void hmp_info_replay(Monitor *mon, const QDict *qdict)
 {
     if (replay_mode == REPLAY_MODE_NONE) {
@@ -216,3 +223,29 @@ void hmp_replay_seek(Monitor *mon, const QDict *qdict)
         return;
     }
 }
+
+static void replay_stop_vm_debug(void *opaque)
+{
+    replay_is_debugging = false;
+    vm_stop(RUN_STATE_DEBUG);
+    replay_break(-1LL, NULL, NULL);
+}
+
+bool replay_reverse_step(void)
+{
+    Error *err = NULL;
+
+    assert(replay_mode == REPLAY_MODE_PLAY);
+
+    if (replay_get_current_step() != 0) {
+        replay_seek(replay_get_current_step() - 1, replay_stop_vm_debug, &err);
+        if (err) {
+            error_free(err);
+            return false;
+        }
+        replay_is_debugging = true;
+        return true;
+    }
+
+    return false;
+}
diff --git a/stubs/replay.c b/stubs/replay.c
index 4ac6078..521552f 100644
--- a/stubs/replay.c
+++ b/stubs/replay.c
@@ -80,3 +80,8 @@ void replay_mutex_lock(void)
 void replay_mutex_unlock(void)
 {
 }
+
+bool replay_reverse_step(void)
+{
+    return false;
+}

[Qemu-devel] [PATCH v9 17/21] gdbstub: add reverse continue support in replay mode

[Pavel Dovgalyuk]

This patch adds support of the reverse continue operation for gdbstub.
Reverse continue finds the last breakpoint that would happen in normal
execution from the beginning to the current moment.
Implementation of the reverse continue replays the execution twice:
to find the breakpoints that were hit and to seek to the last breakpoint.
Reverse continue loads the previous snapshot and tries to find the breakpoint
since that moment. If there are no such breakpoints, it proceeds to
the earlier snapshot, and so on. When no breakpoints or watchpoints were
hit at all, execution stops at the beginning of the replay log.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
---
 cpus.c                    |    5 +++
 exec.c                    |    1 +
 gdbstub.c                 |   10 ++++++
 include/sysemu/replay.h   |    8 +++++
 replay/replay-debugging.c |   71 +++++++++++++++++++++++++++++++++++++++++++++
 stubs/replay.c            |    5 +++
 6 files changed, 99 insertions(+), 1 deletion(-)

diff --git a/cpus.c b/cpus.c
index 013341c..6ef0992 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1110,6 +1110,11 @@ static void cpu_handle_guest_debug(CPUState *cpu)
         cpu->stopped = true;
     } else {
         if (!cpu->singlestep_enabled) {
+            /*
+             * Report about the breakpoint and
+             * make a single step to skip it
+             */
+            replay_breakpoint();
             cpu_single_step(cpu, SSTEP_ENABLE);
         } else {
             cpu_single_step(cpu, 0);
diff --git a/exec.c b/exec.c
index 10f2927..4278fcb 100644
--- a/exec.c
+++ b/exec.c
@@ -2744,6 +2744,7 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags)
                  * Don't process the watchpoints when we are
                  * in a reverse debugging operation.
                  */
+                replay_breakpoint();
                 return;
             }
             if (flags == BP_MEM_READ) {
diff --git a/gdbstub.c b/gdbstub.c
index 442187d..60176d5 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1463,6 +1463,14 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
                     put_packet(s, "E14");
                     break;
                 }
+            case 'c':
+                if (replay_reverse_continue()) {
+                    gdb_continue(s);
+                    return RS_IDLE;
+                } else {
+                    put_packet(s, "E14");
+                    break;
+                }
             default:
                 goto unknown_command;
             }
@@ -1773,7 +1781,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
             pstrcat(buf, sizeof(buf), ";multiprocess+");
 
             if (replay_mode == REPLAY_MODE_PLAY) {
-                pstrcat(buf, sizeof(buf), ";ReverseStep+");
+                pstrcat(buf, sizeof(buf), ";ReverseStep+;ReverseContinue+");
             }
 
             put_packet(s, buf);
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index b0e2309..5baf10a 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -81,10 +81,18 @@ const char *replay_get_filename(void);
  */
 bool replay_reverse_step(void);
 /*
+ * Start searching the last breakpoint/watchpoint.
+ * Used by gdbstub for backwards debugging.
+ * Returns true if the process successfully started.
+ */
+bool replay_reverse_continue(void);
+/*
  * Returns true if replay module is processing
  * reverse_continue or reverse_step request
  */
 bool replay_running_debug(void);
+/* Called in reverse debugging mode to collect breakpoint information */
+void replay_breakpoint(void);
 
 /* Processing the instructions */
 
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index dfade54..98300ae 100644
--- a/replay/replay-debugging.c
+++ b/replay/replay-debugging.c
@@ -22,6 +22,8 @@
 #include "migration/snapshot.h"
 
 static bool replay_is_debugging;
+static int64_t replay_last_breakpoint;
+static int64_t replay_last_snapshot;
 
 bool replay_running_debug(void)
 {
@@ -249,3 +251,72 @@ bool replay_reverse_step(void)
 
     return false;
 }
+
+static void replay_continue_end(void)
+{
+    replay_is_debugging = false;
+    vm_stop(RUN_STATE_DEBUG);
+    replay_break(-1LL, NULL, NULL);
+}
+
+static void replay_continue_stop(void *opaque)
+{
+    Error *err = NULL;
+    if (replay_last_breakpoint != -1LL) {
+        replay_seek(replay_last_breakpoint, replay_stop_vm_debug, &err);
+        if (err) {
+            error_free(err);
+            replay_continue_end();
+        }
+        return;
+    }
+    /*
+     * No breakpoints since the last snapshot.
+     * Find previous snapshot and try again.
+     */
+    if (replay_last_snapshot != 0) {
+        replay_seek(replay_last_snapshot - 1, replay_continue_stop, &err);
+        if (err) {
+            error_free(err);
+            replay_continue_end();
+        }
+        replay_last_snapshot = replay_get_current_step();
+        return;
+    } else {
+        /* Seek to the very first step */
+        replay_seek(0, replay_stop_vm_debug, &err);
+        if (err) {
+            error_free(err);
+            replay_continue_end();
+        }
+        return;
+    }
+    replay_continue_end();
+}
+
+bool replay_reverse_continue(void)
+{
+    Error *err = NULL;
+
+    assert(replay_mode == REPLAY_MODE_PLAY);
+
+    if (replay_get_current_step() != 0) {
+        replay_seek(replay_get_current_step() - 1, replay_continue_stop, &err);
+        if (err) {
+            error_free(err);
+            return false;
+        }
+        replay_last_breakpoint = -1LL;
+        replay_is_debugging = true;
+        replay_last_snapshot = replay_get_current_step();
+        return true;
+    }
+
+    return false;
+}
+
+void replay_breakpoint(void)
+{
+    assert(replay_mode == REPLAY_MODE_PLAY);
+    replay_last_breakpoint = replay_get_current_step();
+}
diff --git a/stubs/replay.c b/stubs/replay.c
index 521552f..ee64972 100644
--- a/stubs/replay.c
+++ b/stubs/replay.c
@@ -85,3 +85,8 @@ bool replay_reverse_step(void)
 {
     return false;
 }
+
+bool replay_reverse_continue(void)
+{
+    return false;
+}

[Qemu-devel] [PATCH v9 18/21] replay: describe reverse debugging in docs/replay.txt

[Pavel Dovgalyuk]

This patch updates the documentation and describes usage of the reverse
debugging in QEMU+GDB.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
---
 docs/replay.txt |   33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/docs/replay.txt b/docs/replay.txt
index 2c2c5f6..8447fdd 100644
--- a/docs/replay.txt
+++ b/docs/replay.txt
@@ -293,6 +293,39 @@ for recording and replaying must contain identical number of ports in record
 and replay modes, but their backends may differ.
 E.g., '-serial stdio' in record mode, and '-serial null' in replay mode.
 
+Reverse debugging
+-----------------
+
+Reverse debugging allows "executing" the program in reverse direction.
+GDB remote protocol supports "reverse step" and "reverse continue"
+commands. The first one steps single instruction backwards in time,
+and the second one finds the last breakpoint in the past.
+
+Recorded executions may be used to enable reverse debugging. QEMU can't
+execute the code in backwards direction, but can load a snapshot and
+replay forward to find the desired position or breakpoint.
+
+The following GDB commands are supported:
+ - reverse-stepi (or rsi) - step one instruction backwards
+ - reverse-continue (or rc) - find last breakpoint in the past
+
+Reverse step loads the nearest snapshot and replays the execution until
+the required instruction is met.
+
+Reverse continue may include several passes of examining the execution
+between the snapshots. Each of the passes include the following steps:
+ 1. loading the snapshot
+ 2. replaying to examine the breakpoints
+ 3. if breakpoint or watchpoint was met
+    - loading the snaphot again
+    - replaying to the required breakpoint
+ 4. else
+    - proceeding to the p.1 with the earlier snapshot
+
+Therefore usage of the reverse debugging requires at least one snapshot
+created in advance. See the "Snapshotting" section to learn about running
+record/replay and creating the snapshot in these modes.
+
 Replay log format
 -----------------
 

[Qemu-devel] [PATCH v9 19/21] replay: add BH oneshot event for block layer

[Pavel Dovgalyuk]

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    |    5 +++--
 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, 34 insertions(+), 3 deletions(-)
 create mode 100644 stubs/replay-user.c

diff --git a/block/block-backend.c b/block/block-backend.c
index 60d37a0..41ee871 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"
@@ -1380,8 +1381,8 @@ 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),
-                                blk_aio_complete_bh, acb);
+        replay_bh_schedule_oneshot_event(blk_get_aio_context(blk),
+                                         blk_aio_complete_bh, acb);
     }
 
     return &acb->common;
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 5baf10a..eaad320 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -164,6 +164,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 d9a2d49..fc9b082 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:
@@ -217,6 +232,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 e37b201..af2b941 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 3996499..fdf1778 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 5dd0aee..8e8df1e 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);
+}

[Qemu-devel] [PATCH v9 20/21] replay: init rtc after enabling the replay

[Pavel Dovgalyuk]

This patch postpones the call of 'configure_rtc' function. This call
uses host clock to configure the rtc, but host clock access should be
recorded when using icount record/replay mode. Therefore now rtc
is configured after switching record/replay mode on.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
---
 vl.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/vl.c b/vl.c
index 95fc44a..c2d5aca 100644
--- a/vl.c
+++ b/vl.c
@@ -3008,6 +3008,7 @@ int main(int argc, char **argv, char **envp)
     DisplayState *ds;
     QemuOpts *opts, *machine_opts;
     QemuOpts *icount_opts = NULL, *accel_opts = NULL;
+    QemuOpts *rtc_opts = NULL;
     QemuOptsList *olist;
     int optind;
     const char *optarg;
@@ -3814,9 +3815,9 @@ int main(int argc, char **argv, char **envp)
                 warn_report("This option is ignored and will be removed soon");
                 break;
             case QEMU_OPTION_rtc:
-                opts = qemu_opts_parse_noisily(qemu_find_opts("rtc"), optarg,
-                                               false);
-                if (!opts) {
+                rtc_opts = qemu_opts_parse_noisily(qemu_find_opts("rtc"),
+                                                   optarg, false);
+                if (!rtc_opts) {
                     exit(1);
                 }
                 break;
@@ -4029,6 +4030,9 @@ int main(int argc, char **argv, char **envp)
     loc_set_none();
 
     replay_configure(icount_opts);
+    if (rtc_opts) {
+        configure_rtc(rtc_opts);
+    }
 
     if (incoming && !preconfig_exit_requested) {
         error_report("'preconfig' and 'incoming' options are "

[Qemu-devel] [PATCH v9 21/21] replay: document development rules

[Pavel Dovgalyuk]

This patch introduces docs/devel/replay.txt which describes the rules
that should be followed to make virtual devices usable in record/replay mode.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgauk@ispras.ru>

--

v9: fixed external virtual clock description (reported by Artem Pisarenko)
---
 docs/devel/replay.txt |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 docs/devel/replay.txt

diff --git a/docs/devel/replay.txt b/docs/devel/replay.txt
new file mode 100644
index 0000000..e641c35
--- /dev/null
+++ b/docs/devel/replay.txt
@@ -0,0 +1,46 @@
+Record/replay mechanism, that could be enabled through icount mode, expects
+the virtual devices to satisfy the following requirements.
+
+The main idea behind this document is that everything that affects
+the guest state during execution in icount mode should be deterministic.
+
+Timers
+======
+
+All virtual devices should use virtual clock for timers that change the guest
+state. Virtual clock is deterministic, therefore such timers are deterministic
+too.
+
+Virtual devices can also use realtime clock for the events that do not change
+the guest state directly. When the clock ticking should depend on VM execution
+speed, use virtual clock with EXTERNAL attribute. It is not deterministic,
+but its speed depends on the guest execution. This clock is used by
+the virtual devices (e.g., slirp routing device) that lie outside the
+replayed guest.
+
+Bottom halves
+=============
+
+Bottom half callbacks, that affect the guest state, should be invoked through
+replay_bh_schedule_event or replay_bh_schedule_oneshot_event functions.
+Their invocations are saved in record mode and synchronized with the existing
+log in replay mode.
+
+Saving/restoring the VM state
+=============================
+
+All fields in the device state structure (including virtual timers)
+should be restored by loadvm to the same values they had before savevm.
+
+Avoid accessing other devices' state, because the order of saving/restoring
+is not defined. It means that you should not call functions like
+'update_irq' in post_load callback. Save everything explicitly to avoid
+the dependencies that may make restoring the VM state non-deterministic.
+
+Stopping the VM
+===============
+
+Stopping the guest should not interfere with its state (with the exception
+of the network connections, that could be broken by the remote timeouts).
+VM can be stopped at any moment of replay by the user. Restarting the VM
+after that stop should not break the replay by the unneeded guest state change.

Re: [Qemu-devel] [PATCH v9 09/21] replay: provide and accessor for rr filename

[Markus Armbruster]

In the subject: "an accessor".

Re: [Qemu-devel] [PATCH v9 10/21] qapi: introduce replay.json for record/replay-related stuff

[Markus Armbruster]

Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> writes:

> This patch adds replay.json file. It will be
> used for adding record/replay-related data structures and commands.
>
> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> ---
>  MAINTAINERS             |    1 +
>  Makefile.objs           |    4 ++--
>  include/sysemu/replay.h |    1 +
>  qapi/misc.json          |   18 ------------------
>  qapi/qapi-schema.json   |    1 +
>  qapi/replay.json        |   26 ++++++++++++++++++++++++++
>  6 files changed, 31 insertions(+), 20 deletions(-)
>  create mode 100644 qapi/replay.json
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0bfd95a..7b0a1bc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2122,6 +2122,7 @@ F: net/filter-replay.c
>  F: include/sysemu/replay.h
>  F: docs/replay.txt
>  F: stubs/replay.c
> +F: qapi/replay.json
>  
>  IOVA Tree
>  M: Peter Xu <peterx@redhat.com>
> diff --git a/Makefile.objs b/Makefile.objs
> index 4561159..8164b22 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -1,6 +1,6 @@
>  QAPI_MODULES = block-core block char common crypto introspect job migration
> -QAPI_MODULES += misc net rdma rocker run-state sockets tpm trace transaction
> -QAPI_MODULES += ui
> +QAPI_MODULES += misc net rdma replay rocker run-state sockets tpm trace
> +QAPI_MODULES += transaction ui
>  
>  #######################################################################
>  # Common libraries for tools and emulators
> diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
> index b3f593f..2054296 100644
> --- a/include/sysemu/replay.h
> +++ b/include/sysemu/replay.h
> @@ -13,6 +13,7 @@
>   */
>  
>  #include "sysemu.h"
> +#include "qapi/qapi-types-replay.h"
>  #include "qapi/qapi-types-misc.h"

I believe you can drop qapi/qapi-types-misc.h now.

>  #include "qapi/qapi-types-ui.h"
>  
> diff --git a/qapi/misc.json b/qapi/misc.json
> index 24d20a8..e5e0bea 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -3125,24 +3125,6 @@
>    'data': { 'offset': 'int' } }
>  
>  ##
> -# @ReplayMode:
> -#
> -# Mode of the replay subsystem.
> -#
> -# @none: normal execution mode. Replay or record are not enabled.
> -#
> -# @record: record mode. All non-deterministic data is written into the
> -#          replay log.
> -#
> -# @play: replay mode. Non-deterministic data required for system execution
> -#        is read from the log.
> -#
> -# Since: 2.5
> -##
> -{ 'enum': 'ReplayMode',
> -  'data': [ 'none', 'record', 'play' ] }
> -
> -##
>  # @xen-load-devices-state:
>  #
>  # Load the state of all devices from file. The RAM and the block devices
> diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
> index 3bbdfce..b85fd04 100644
> --- a/qapi/qapi-schema.json
> +++ b/qapi/qapi-schema.json
> @@ -95,3 +95,4 @@
   # Documentation generated with qapi-gen.py is in source order, with
   # included sub-schemas inserted at the first include directive
   # (subsequent include directives have no effect).  To get a sane and
   # stable order, it's best to include each sub-schema just once, or
   # include it first right here.
[...]
>  { 'include': 'trace.json' }
>  { 'include': 'introspect.json' }
>  { 'include': 'misc.json' }
> +{ 'include': 'replay.json' }

I doubt this the best spot for replay stuff in the generated
documentation.

Is there another chapter you'd consider related?

> diff --git a/qapi/replay.json b/qapi/replay.json
> new file mode 100644
> index 0000000..9e13551
> --- /dev/null
> +++ b/qapi/replay.json
> @@ -0,0 +1,26 @@
> +# -*- Mode: Python -*-
> +#
> +
> +##
> +# = Record/replay
> +##
> +
> +{ 'include': 'common.json' }
> +
> +##
> +# @ReplayMode:
> +#
> +# Mode of the replay subsystem.
> +#
> +# @none: normal execution mode. Replay or record are not enabled.
> +#
> +# @record: record mode. All non-deterministic data is written into the
> +#          replay log.
> +#
> +# @play: replay mode. Non-deterministic data required for system execution
> +#        is read from the log.
> +#
> +# Since: 2.5
> +##
> +{ 'enum': 'ReplayMode',
> +  'data': [ 'none', 'record', 'play' ] }

Re: [Qemu-devel] [PATCH v9 08/21] migration: introduce icount field for snapshots

[Markus Armbruster]

Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> writes:

> Saving icount as a parameters of the snapshot allows navigation between
> them in the execution replay scenario.
> This information can be used for finding a specific snapshot for rewinding
> the recorded execution to the specific moment of the time.
> E.g., 'reverse step' action needs to load the nearest snapshot which is
> prior to the current moment of time .
>
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>
> --
>
> v2:
>  - made icount in SnapshotInfo optional (suggested by Eric Blake)
> v7:
>  - added more comments for icount member (suggested by Markus Armbruster)
> v9:
>  - updated icount comment
> ---
[...]
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 762000f..a197e5f 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -26,13 +26,19 @@
>  #
>  # @vm-clock-nsec: fractional part in nano seconds to be used with vm-clock-sec
>  #
> +# @icount: Current instruction count. Appears when execution record/replay
> +#          is enabled. Used for "time-traveling" to match the moment
> +#          in the recorded execution with the snapshots. This counter may
> +#          be obtained through @query-replay command (since 4.0)

query-replay appears only in PATCH 11.  Please delay the last sentence
until then.

> +#
>  # Since: 1.3
>  #
>  ##
>  { 'struct': 'SnapshotInfo',
>    'data': { 'id': 'str', 'name': 'str', 'vm-state-size': 'int',
>              'date-sec': 'int', 'date-nsec': 'int',
> -            'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } }
> +            'vm-clock-sec': 'int', 'vm-clock-nsec': 'int',
> +            '*icount': 'int' } }
>  
>  ##
>  # @ImageInfoSpecificQCow2EncryptionBase:
> diff --git a/qapi/block.json b/qapi/block.json
> index 11f01f2..a6396a9 100644
> --- a/qapi/block.json
> +++ b/qapi/block.json
> @@ -176,7 +176,8 @@
>  #                    "date-sec": 1000012,
>  #                    "date-nsec": 10,
>  #                    "vm-clock-sec": 100,
> -#                    "vm-clock-nsec": 20
> +#                    "vm-clock-nsec": 20,
> +#                    "icount": 220414
>  #      }
>  #    }
>  #

Re: [Qemu-devel] [PATCH v9 11/21] replay: introduce info hmp/qmp command

[Markus Armbruster]

Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> writes:

> This patch introduces 'info replay' monitor command and
> corresponding qmp request.
> These commands request the current record/replay mode, replay log file name,
> and the execution step (number or recorded/replayed instructions).

s/or/of/

> User may use step number for replay_seek/replay_break commands and
> for controlling the execution of replay.

Dangling reference: these two commands appear only in the next two
patches.  Suggest:

  These commands request the current record/replay mode, replay log file
  name, and the instruction count (number of recorded/replayed
  instructions).  The instruction count can be used with the
  replay_seek/replay_break commands added in the next two patches.

> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> --
>
> v2:
>  - renamed info_replay qmp into query-replay (suggested by Eric Blake)
> v7:
>  - added empty line (suggested by Markus Armbruster)
> v9:
>  - changed 'step' parameter name to 'icount'
>  - moved json stuff to replay.json and updated the descriptions
>    (suggested by Markus Armbruster)
> ---
>  hmp-commands-info.hx      |   14 ++++++++++++++
>  hmp.h                     |    1 +
>  qapi/replay.json          |   39 +++++++++++++++++++++++++++++++++++++++
>  replay/Makefile.objs      |    3 ++-
>  replay/replay-debugging.c |   42 ++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 98 insertions(+), 1 deletion(-)
>  create mode 100644 replay/replay-debugging.c
>
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index cbee8b9..866bc0f 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -918,6 +918,20 @@ STEXI
>  Show SEV information.
>  ETEXI
>  
> +    {
> +        .name       = "replay",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show parameters of the record/replay",

Recommend "show record/replay information", because that's how you
document the underlying QMP command.

> +        .cmd        = hmp_info_replay,
> +    },
> +
> +STEXI
> +@item info replay
> +@findex info replay
> +Display the record/replay information: mode and the current icount.
> +ETEXI
> +
>  STEXI
>  @end table
>  ETEXI
> diff --git a/hmp.h b/hmp.h
> index 5f1addc..d792149 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -148,5 +148,6 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict);
>  void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
>  void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict);
>  void hmp_info_sev(Monitor *mon, const QDict *qdict);
> +void hmp_info_replay(Monitor *mon, const QDict *qdict);
>  
>  #endif
> diff --git a/qapi/replay.json b/qapi/replay.json
> index 9e13551..d7e76cf 100644
> --- a/qapi/replay.json
> +++ b/qapi/replay.json
> @@ -24,3 +24,42 @@
>  ##
>  { 'enum': 'ReplayMode',
>    'data': [ 'none', 'record', 'play' ] }
> +
> +##
> +# @ReplayInfo:
> +#
> +# Record/replay information.
> +#
> +# @mode: current mode.
> +#
> +# @filename: name of the record/replay log file.
> +#            It is present only in record or replay modes, when the log
> +#            is recorded or replayed.
> +#
> +# @icount: current number of executed instructions.
> +#
> +# Since: 4.0
> +#
> +##
> +{ 'struct': 'ReplayInfo',
> +  'data': { 'mode': 'ReplayMode', '*filename': 'str', 'icount': 'int' } }
> +
> +##
> +# @query-replay:
> +#
> +# Retrieves the record/replay information.
> +# It includes current icount which may be used for replay-break and
> +# replay-seek commands.

Let's say "current instruction count, which".

Dangling reference: these two commands appear only in the next two
patches.  Tolerable if the commit message spells it out.

> +#
> +# Returns: structure with the properties of the record/replay.

I'd simply say "Returns: record/replay information".

> +#
> +# Since: 4.0
> +#
> +# Example:
> +#
> +# -> { "execute": "query-replay" }
> +# <- { "return": { "mode": "play", "filename": "log.rr", "icount": 220414 } }
> +#
> +##
> +{ 'command': 'query-replay',
> +  'returns': 'ReplayInfo' }
> diff --git a/replay/Makefile.objs b/replay/Makefile.objs
> index cee6539..6694e3e 100644
> --- a/replay/Makefile.objs
> +++ b/replay/Makefile.objs
> @@ -6,4 +6,5 @@ common-obj-y += replay-input.o
>  common-obj-y += replay-char.o
>  common-obj-y += replay-snapshot.o
>  common-obj-y += replay-net.o
> -common-obj-y += replay-audio.o
> \ No newline at end of file
> +common-obj-y += replay-audio.o
> +common-obj-y += replay-debugging.o
> diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
> new file mode 100644
> index 0000000..9956405
> --- /dev/null
> +++ b/replay/replay-debugging.c

The file name suggests the instruction count is just for debugging.  If
that's true, the documentation should mention it.  Else, the file should
be named less misleadingly, perhaps replay-control.c.

> @@ -0,0 +1,42 @@
> +/*
> + * replay-debugging.c
> + *
> + * Copyright (c) 2010-2018 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 "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "sysemu/replay.h"
> +#include "replay-internal.h"
> +#include "hmp.h"
> +#include "monitor/monitor.h"
> +#include "qapi/qapi-commands-replay.h"
> +
> +void hmp_info_replay(Monitor *mon, const QDict *qdict)
> +{
> +    if (replay_mode == REPLAY_MODE_NONE) {
> +        monitor_printf(mon, "No record/replay\n");

Perhaps something like "Record/replay not active" would be friendlier.

> +    } else {
> +        monitor_printf(mon, "%s execution '%s': current step = %"PRId64"\n",
> +            replay_mode == REPLAY_MODE_RECORD ? "Recording" : "Replaying",
> +            replay_get_filename(), replay_get_current_step());

You improved documentation to consistently say "instruction count".  The
code still calls it current_step.  So does the HMP UI here.  In your
shoes, I'd go all the way.  Consistent terminology really helps
readers.  Advice, not demand.

> +    }
> +}
> +
> +ReplayInfo *qmp_query_replay(Error **errp)
> +{
> +    ReplayInfo *retval = g_new0(ReplayInfo, 1);
> +
> +    retval->mode = replay_mode;
> +    if (replay_get_filename()) {
> +        retval->filename = g_strdup(replay_get_filename());
> +        retval->has_filename = true;
> +    }
> +    retval->icount = replay_get_current_step();
> +    return retval;
> +}

Looks almost ready.

Re: [Qemu-devel] [PATCH v9 12/21] replay: introduce breakpoint at the specified step

[Markus Armbruster]

Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> writes:

> This patch introduces replay_break, replay_delete_break
> qmp and hmp commands.
> These commands allow stopping at the specified instruction.
> It may be useful for debugging when there are some known
> events that should be investigated.
> replay_break command has one argument - number of instructions
> executed since the start of the replay.
> replay_delete_break removes previously set breakpoint.
>
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>
> --
>
> v2:
>  - renamed replay_break qmp command into replay-break
>    (suggested by Eric Blake)
> v7:
>  - introduces replay_delete_break command
> v9:
>  - changed 'step' parameter name to 'icount'
>  - moved json stuff to replay.json and updated the description
>    (suggested by Markus Armbruster)
> ---
>  hmp-commands.hx           |   34 ++++++++++++++++++
>  hmp.h                     |    2 +
>  qapi/replay.json          |   36 +++++++++++++++++++
>  replay/replay-debugging.c |   85 +++++++++++++++++++++++++++++++++++++++++++++
>  replay/replay-internal.h  |    4 ++
>  replay/replay.c           |   17 +++++++++
>  6 files changed, 178 insertions(+)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index ba71558..6d04c02 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1890,6 +1890,40 @@ Set QOM property @var{property} of object at location @var{path} to value @var{v
>  ETEXI
>  
>      {
> +        .name       = "replay_break",
> +        .args_type  = "icount:i",
> +        .params     = "icount",
> +        .help       = "sets breakpoint on the step specified by the icount of the replay",

"set replay breakpoint at the specified instruction count"

> +        .cmd        = hmp_replay_break,
> +    },
> +
> +STEXI
> +@item replay_break @var{icount}
> +@findex replay_break
> +Set breakpoint on the step of the replay specified by @var{icount}.
> +Execution stops when the specified @var{icount} is reached.
> +icount for the reference may be observed with 'info replay' command.
> +There could be at most one breakpoint. When breakpoint is set, the prior
> +one is removed. The breakpoints may be set only in replay mode and only
> +at the step in the future.

This still uses both "step" and "instruction count".

Suggest

   Set replay breakpoint at instruction count @var{icount}.
   Execution stops when the specified instruction is reached.
   There can be at most one breakpoint.  When breakpoint is set, any prior
   one is removed.  The breakpoint may be set only in replay mode and only
   "in the future", i.e. at instruction counts greater than the current one.
   The current instruction count can be observed with 'info replay'.


> +ETEXI
> +
> +    {
> +        .name       = "replay_delete_break",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "removes replay breakpoint",
> +        .cmd        = hmp_replay_delete_break,
> +    },
> +
> +STEXI
> +@item replay_delete_break
> +@findex replay_delete_break
> +Removes replay breakpoint which was previously set with replay_break.
> +The command is ignored when there are no replay breakpoints.
> +ETEXI
> +
> +    {
>          .name       = "info",
>          .args_type  = "item:s?",
>          .params     = "[subcommand]",
> diff --git a/hmp.h b/hmp.h
> index d792149..c9b9b4f 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -149,5 +149,7 @@ void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
>  void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict);
>  void hmp_info_sev(Monitor *mon, const QDict *qdict);
>  void hmp_info_replay(Monitor *mon, const QDict *qdict);
> +void hmp_replay_break(Monitor *mon, const QDict *qdict);
> +void hmp_replay_delete_break(Monitor *mon, const QDict *qdict);
>  
>  #endif
> diff --git a/qapi/replay.json b/qapi/replay.json
> index d7e76cf..a63219c 100644
> --- a/qapi/replay.json
> +++ b/qapi/replay.json
> @@ -63,3 +63,39 @@
>  ##
>  { 'command': 'query-replay',
>    'returns': 'ReplayInfo' }
> +
> +##
> +# @replay-break:
> +#
> +# Set breakpoint on the step of the replay specified by @icount.
> +# Execution stops when the specified @icount is reached.
> +# icount for the reference may be obtained with @query-replay command.
> +# There could be at most one breakpoint. When breakpoint is set, the prior
> +# one is removed. The breakpoints may be set only in replay mode and only
> +# at the step in the future.

My comment on hmp-commands.hx applies.

> +#
> +# @icount: execution step to stop at

s/execution step/instruction count/

> +#
> +# Since: 4.0
> +#
> +# Example:
> +#
> +# -> { "execute": "replay-break", "data": { "icount": 220414 } }
> +#
> +##
> +{ 'command': 'replay-break', 'data': { 'icount': 'int' } }
> +
> +##
> +# @replay-delete-break:
> +#
> +# Removes replay breakpoint which was set with @replay-break.
> +# The command is ignored when there are no replay breakpoints.
> +#
> +# Since: 4.0
> +#
> +# Example:
> +#
> +# -> { "execute": "replay-delete-break" }
> +#
> +##
> +{ 'command': 'replay-delete-break' }
[...]

We're down to phrasing help and documentation.  Almost done :)

Re: [Qemu-devel] [PATCH v9 13/21] replay: implement replay-seek command to proceed to the desired step

[Markus Armbruster]

Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> writes:

> This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
> the execution to the specified step.
> The command automatically loads nearest snapshot and replay the execution
> to find the desired step.

"step" again.

>
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>
> --
>
> v2:
>  - renamed replay_seek qmp command into replay-seek
>    (suggested by Eric Blake)
> v7:
>  - small fixes related to Markus Armbruster's review
> v9:
>  - changed 'step' parameter name to 'icount'
>  - moved json stuff to replay.json and updated the description
>    (suggested by Markus Armbruster)
> ---
>  hmp-commands.hx           |   19 +++++++++
>  hmp.h                     |    1 
>  qapi/replay.json          |   20 ++++++++++
>  replay/replay-debugging.c |   91 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 131 insertions(+)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 6d04c02..2839acc 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1924,6 +1924,25 @@ The command is ignored when there are no replay breakpoints.
>  ETEXI
>  
>      {
> +        .name       = "replay_seek",
> +        .args_type  = "icount:i",
> +        .params     = "icount",
> +        .help       = "rewinds replay to the step specified by icount",

We use imperative mood in .help: "rewind".

"Rewind" suggests going backward.  The command can do that.  But it can
also go forward.  What about:

"replay execution to the specified intruction count"

> +        .cmd        = hmp_replay_seek,
> +    },
> +
> +STEXI
> +@item replay_seek @var{icount}
> +@findex replay_seek
> +Automatically proceeds to the step specified by @var{icount}, when
> +replaying the execution. The command automatically loads nearest
> +snapshot and replay the execution to find the desired step.
> +When there is no preceding snapshot or the execution is not replayed,
> +then the command is ignored.

Uh, why isn't that an error?

> +icount for the reference may be observed with 'info replay' command.

This still uses both "step" and "instruction count".

I'm happy to help rephrasing, but first we need to nail down the failure
modes.

> +ETEXI
> +
> +    {
>          .name       = "info",
>          .args_type  = "item:s?",
>          .params     = "[subcommand]",
> diff --git a/hmp.h b/hmp.h
> index c9b9b4f..d6e1d7e 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -151,5 +151,6 @@ void hmp_info_sev(Monitor *mon, const QDict *qdict);
>  void hmp_info_replay(Monitor *mon, const QDict *qdict);
>  void hmp_replay_break(Monitor *mon, const QDict *qdict);
>  void hmp_replay_delete_break(Monitor *mon, const QDict *qdict);
> +void hmp_replay_seek(Monitor *mon, const QDict *qdict);
>  
>  #endif
> diff --git a/qapi/replay.json b/qapi/replay.json
> index a63219c..7390f88 100644
> --- a/qapi/replay.json
> +++ b/qapi/replay.json
> @@ -99,3 +99,23 @@
>  #
>  ##
>  { 'command': 'replay-delete-break' }
> +
> +##
> +# @replay-seek:
> +#
> +# Automatically proceeds to the step specified by @icount when replaying
> +# the execution. The command automatically loads nearest
> +# snapshot and replay the execution to find the desired step.
> +# When there is no preceding snapshot or the execution is not replayed,
> +# then the command is ignored.
> +# icount for the reference may be obtained with @query-replay command.

My comments on hmp-commands.hx apply.

> +#
> +# @icount: destination execution step

s/execution step/instruction count/

Maybe s/destination/target/

> +#
> +# Since: 4.0
> +#
> +# Example:
> +#
> +# -> { "execute": "replay-seek", "data": { "icount": 220414 } }
> +##
> +{ 'command': 'replay-seek', 'data': { 'icount': 'int' } }
[...]

Re: [Qemu-devel] [PATCH v9 19/21] replay: add BH oneshot event for block layer

[Kevin Wolf]

Am 09.01.2019 um 13:13 hat Pavel Dovgalyuk geschrieben:
> 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>

This still doesn't come even close to catching all BHs that need to be
caught. While you managed to show a few BHs that actually don't need to
be considered for recording when I asked for this in v7, most BHs in the
block layer can in some way lead to device callbacks and must therefore
be recorded.

How bad would it be to record some BHs even if recording them isn't
necessary? I'd definitely try to err on the safe side here. Having two
different sets of BH functions, you can't expect that people always use
the right one (especially if you don't even make the existing code base
consistently use the right one intially).

Kevin

Re: [Qemu-devel] [PATCH v9 00/21] Fixing record/replay and adding reverse debugging

[no-reply]

Patchew URL: https://patchew.org/QEMU/154703587757.13472.3898702635363120794.stgit@pasha-VirtualBox/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-quick@centos7 SHOW_ENV=1 J=8
=== TEST SCRIPT END ===




The full log is available at
http://patchew.org/logs/154703587757.13472.3898702635363120794.stgit@pasha-VirtualBox/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH v9 10/21] qapi: introduce replay.json for record/replay-related stuff

[Pavel Dovgalyuk]

> From: Markus Armbruster [mailto:armbru@redhat.com]
> Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> writes:
> 
> > This patch adds replay.json file. It will be
> > used for adding record/replay-related data structures and commands.
> >
> > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> > ---
> >  MAINTAINERS             |    1 +
> >  Makefile.objs           |    4 ++--
> >  include/sysemu/replay.h |    1 +
> >  qapi/misc.json          |   18 ------------------
> >  qapi/qapi-schema.json   |    1 +
> >  qapi/replay.json        |   26 ++++++++++++++++++++++++++
> >  6 files changed, 31 insertions(+), 20 deletions(-)
> >  create mode 100644 qapi/replay.json
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 0bfd95a..7b0a1bc 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -2122,6 +2122,7 @@ F: net/filter-replay.c
> >  F: include/sysemu/replay.h
> >  F: docs/replay.txt
> >  F: stubs/replay.c
> > +F: qapi/replay.json
> >
> >  IOVA Tree
> >  M: Peter Xu <peterx@redhat.com>
> > diff --git a/Makefile.objs b/Makefile.objs
> > index 4561159..8164b22 100644
> > --- a/Makefile.objs
> > +++ b/Makefile.objs
> > @@ -1,6 +1,6 @@
> >  QAPI_MODULES = block-core block char common crypto introspect job migration
> > -QAPI_MODULES += misc net rdma rocker run-state sockets tpm trace transaction
> > -QAPI_MODULES += ui
> > +QAPI_MODULES += misc net rdma replay rocker run-state sockets tpm trace
> > +QAPI_MODULES += transaction ui
> >
> >  #######################################################################
> >  # Common libraries for tools and emulators
> > diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
> > index b3f593f..2054296 100644
> > --- a/include/sysemu/replay.h
> > +++ b/include/sysemu/replay.h
> > @@ -13,6 +13,7 @@
> >   */
> >
> >  #include "sysemu.h"
> > +#include "qapi/qapi-types-replay.h"
> >  #include "qapi/qapi-types-misc.h"
> 
> I believe you can drop qapi/qapi-types-misc.h now.

Missed this one, thanks.

> 
> >  #include "qapi/qapi-types-ui.h"
> >
> > diff --git a/qapi/misc.json b/qapi/misc.json
> > index 24d20a8..e5e0bea 100644
> > --- a/qapi/misc.json
> > +++ b/qapi/misc.json
> > @@ -3125,24 +3125,6 @@
> >    'data': { 'offset': 'int' } }
> >
> >  ##
> > -# @ReplayMode:
> > -#
> > -# Mode of the replay subsystem.
> > -#
> > -# @none: normal execution mode. Replay or record are not enabled.
> > -#
> > -# @record: record mode. All non-deterministic data is written into the
> > -#          replay log.
> > -#
> > -# @play: replay mode. Non-deterministic data required for system execution
> > -#        is read from the log.
> > -#
> > -# Since: 2.5
> > -##
> > -{ 'enum': 'ReplayMode',
> > -  'data': [ 'none', 'record', 'play' ] }
> > -
> > -##
> >  # @xen-load-devices-state:
> >  #
> >  # Load the state of all devices from file. The RAM and the block devices
> > diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
> > index 3bbdfce..b85fd04 100644
> > --- a/qapi/qapi-schema.json
> > +++ b/qapi/qapi-schema.json
> > @@ -95,3 +95,4 @@
>    # Documentation generated with qapi-gen.py is in source order, with
>    # included sub-schemas inserted at the first include directive
>    # (subsequent include directives have no effect).  To get a sane and
>    # stable order, it's best to include each sub-schema just once, or
>    # include it first right here.
> [...]
> >  { 'include': 'trace.json' }
> >  { 'include': 'introspect.json' }
> >  { 'include': 'misc.json' }
> > +{ 'include': 'replay.json' }
> 
> I doubt this the best spot for replay stuff in the generated
> documentation.
> 
> Is there another chapter you'd consider related?

Do you mean, that replay should be located near some other topic?
I don't think, that it related to anything.
But I can put replay above misc, because it related to one concrete topic.

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v9 11/21] replay: introduce info hmp/qmp command

[Pavel Dovgalyuk]

> From: Markus Armbruster [mailto:armbru@redhat.com]
> Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> writes:


> > diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
> > new file mode 100644
> > index 0000000..9956405
> > --- /dev/null
> > +++ b/replay/replay-debugging.c
> 
> The file name suggests the instruction count is just for debugging.  If
> that's true, the documentation should mention it.  Else, the file should
> be named less misleadingly, perhaps replay-control.c.

The main purpose of this file is providing an implementation of reverse
debugging methods.
Querying the rr information is mostly for debugging/problem solving too.
(until someone suggests any other manner of using these functions)

> 
> > @@ -0,0 +1,42 @@
> > +/*
> > + * replay-debugging.c
> > + *
> > + * Copyright (c) 2010-2018 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 "qemu/osdep.h"
> > +#include "qapi/error.h"
> > +#include "sysemu/replay.h"
> > +#include "replay-internal.h"
> > +#include "hmp.h"
> > +#include "monitor/monitor.h"
> > +#include "qapi/qapi-commands-replay.h"
> > +
> > +void hmp_info_replay(Monitor *mon, const QDict *qdict)
> > +{
> > +    if (replay_mode == REPLAY_MODE_NONE) {
> > +        monitor_printf(mon, "No record/replay\n");
> 
> Perhaps something like "Record/replay not active" would be friendlier.
> 
> > +    } else {
> > +        monitor_printf(mon, "%s execution '%s': current step = %"PRId64"\n",
> > +            replay_mode == REPLAY_MODE_RECORD ? "Recording" : "Replaying",
> > +            replay_get_filename(), replay_get_current_step());
> 
> You improved documentation to consistently say "instruction count".  The
> code still calls it current_step.  So does the HMP UI here.  In your
> shoes, I'd go all the way.  Consistent terminology really helps
> readers.  Advice, not demand.

Sounds reasonable. I'll probably refactor replay_get_current_step() too.

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v9 13/21] replay: implement replay-seek command to proceed to the desired step

[Pavel Dovgalyuk]

> From: Markus Armbruster [mailto:armbru@redhat.com]
> Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> writes:
> 
> > This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
> > the execution to the specified step.
> > The command automatically loads nearest snapshot and replay the execution
> > to find the desired step.
> 
> "step" again.
> 
> >
> > Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> >
> > --
> >
> > v2:
> >  - renamed replay_seek qmp command into replay-seek
> >    (suggested by Eric Blake)
> > v7:
> >  - small fixes related to Markus Armbruster's review
> > v9:
> >  - changed 'step' parameter name to 'icount'
> >  - moved json stuff to replay.json and updated the description
> >    (suggested by Markus Armbruster)
> > ---
> >  hmp-commands.hx           |   19 +++++++++
> >  hmp.h                     |    1
> >  qapi/replay.json          |   20 ++++++++++
> >  replay/replay-debugging.c |   91 +++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 131 insertions(+)
> >
> > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > index 6d04c02..2839acc 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -1924,6 +1924,25 @@ The command is ignored when there are no replay breakpoints.
> >  ETEXI
> >
> >      {
> > +        .name       = "replay_seek",
> > +        .args_type  = "icount:i",
> > +        .params     = "icount",
> > +        .help       = "rewinds replay to the step specified by icount",
> 
> We use imperative mood in .help: "rewind".
> 
> "Rewind" suggests going backward.  The command can do that.  But it can
> also go forward.  What about:
> 
> "replay execution to the specified intruction count"
> 
> > +        .cmd        = hmp_replay_seek,
> > +    },
> > +
> > +STEXI
> > +@item replay_seek @var{icount}
> > +@findex replay_seek
> > +Automatically proceeds to the step specified by @var{icount}, when
> > +replaying the execution. The command automatically loads nearest
> > +snapshot and replay the execution to find the desired step.
> > +When there is no preceding snapshot or the execution is not replayed,
> > +then the command is ignored.
> 
> Uh, why isn't that an error?

What about this one?

Automatically proceeds to the instruction count @var{icount}, when
replaying the execution. The command automatically loads nearest
snapshot and replays the execution to find the desired instruction.
When there is no preceding snapshot or the execution is not replayed,
then the command is ignored and error message is displayed.
icount for the reference may be observed with 'info replay' command.


Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v9 19/21] replay: add BH oneshot event for block layer

[Pavel Dovgalyuk]

> From: Kevin Wolf [mailto:kwolf@redhat.com]
> Am 09.01.2019 um 13:13 hat Pavel Dovgalyuk geschrieben:
> > 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>
> 
> This still doesn't come even close to catching all BHs that need to be
> caught. While you managed to show a few BHs that actually don't need to
> be considered for recording when I asked for this in v7, most BHs in the
> block layer can in some way lead to device callbacks and must therefore
> be recorded.

Let's have a brief review. I can change all the places, but how
should I make a test case to be sure, that all of them are working ok?

aio_bh_schedule_oneshot is used in:
 - blk_abort_aio_request
 - bdrv_co_yield_to_drain
 - iscsi_co_generic_cb
 - nfs_co_generic_cb
 - null_aio_common
 - nvme_process_completion
 - nvme_rw_cb
 - rbd_finish_aiocb
 - vxhs_iio_callback
 - (and couple of others not in the block layer)

We must change this call to replay_bh_schedule_oneshot_event when
the result of the BH execution affects the replayed guest state
(e.g., interrupt request is generated or memory is written)

If you think that all of these can do that, then I should change
such function calls.

> How bad would it be to record some BHs even if recording them isn't
> necessary? I'd definitely try to err on the safe side here. Having two
> different sets of BH functions, you can't expect that people always use
> the right one (especially if you don't even make the existing code base
> consistently use the right one intially).

There are two possible options:
1. Execution hangs when recording. Kind of deadlock caused by the incorrect
   management of the events. E.g., adding stopping the VM and trying to flush
   the block layer queue.
2. Execution hangs when replaying. 
   One of the events that affect the guest state is missed or generated
   at the other moment (e.g., when BH is not linked to the execution step).
   Then the guest behaves differently and the order of the events in the log
   does not match the guest state (e.g., interrupt processing is not matched).

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v9 19/21] replay: add BH oneshot event for block layer

[Kevin Wolf]

Am 14.01.2019 um 12:10 hat Pavel Dovgalyuk geschrieben:
> > From: Kevin Wolf [mailto:kwolf@redhat.com]
> > Am 09.01.2019 um 13:13 hat Pavel Dovgalyuk geschrieben:
> > > 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>
> > 
> > This still doesn't come even close to catching all BHs that need to be
> > caught. While you managed to show a few BHs that actually don't need to
> > be considered for recording when I asked for this in v7, most BHs in the
> > block layer can in some way lead to device callbacks and must therefore
> > be recorded.
> 
> Let's have a brief review. I can change all the places, but how
> should I make a test case to be sure, that all of them are working ok?

The list is changing all the time. This is why I am so concerned about
special-casing a few callers instead of having a generic solution. I
don't know how we could make sure that we call the right function
everywhere.

> aio_bh_schedule_oneshot is used in:
>  - blk_abort_aio_request
>  - bdrv_co_yield_to_drain
>  - iscsi_co_generic_cb
>  - nfs_co_generic_cb
>  - null_aio_common
>  - nvme_process_completion
>  - nvme_rw_cb
>  - rbd_finish_aiocb
>  - vxhs_iio_callback
>  - (and couple of others not in the block layer)

In addition to these, we have at least a few functions that just resume
block layer coroutines rather than directly scheduling a BH with a
callback somewhere in the block layer.

> We must change this call to replay_bh_schedule_oneshot_event when
> the result of the BH execution affects the replayed guest state
> (e.g., interrupt request is generated or memory is written)
> 
> If you think that all of these can do that, then I should change
> such function calls.

I haven't reviewed the code, but these names look like all of them can
eventually call back into the guest devices. They won't do that always,
but potentially.

> > How bad would it be to record some BHs even if recording them isn't
> > necessary? I'd definitely try to err on the safe side here. Having two
> > different sets of BH functions, you can't expect that people always use
> > the right one (especially if you don't even make the existing code base
> > consistently use the right one intially).
> 
> There are two possible options:
> 1. Execution hangs when recording. Kind of deadlock caused by the incorrect
>    management of the events. E.g., adding stopping the VM and trying to flush
>    the block layer queue.
> 2. Execution hangs when replaying. 
>    One of the events that affect the guest state is missed or generated
>    at the other moment (e.g., when BH is not linked to the execution step).
>    Then the guest behaves differently and the order of the events in the log
>    does not match the guest state (e.g., interrupt processing is not matched).

So basically when you have two events that are kind of nested? Operation
A triggers event A, but in order to complete the operation, you call
operation B with event B internally, which isn't available yet because
we're still handling event A?

Could this be solved by not having an order of events, but an order of
sets of events?

Kevin

Re: [Qemu-devel] [PATCH v9 19/21] replay: add BH oneshot event for block layer

[Pavel Dovgalyuk]

> From: Kevin Wolf [mailto:kwolf@redhat.com]
> Am 14.01.2019 um 12:10 hat Pavel Dovgalyuk geschrieben:
> > > From: Kevin Wolf [mailto:kwolf@redhat.com]
> > > Am 09.01.2019 um 13:13 hat Pavel Dovgalyuk geschrieben:
> > > > 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>
> > >
> > > This still doesn't come even close to catching all BHs that need to be
> > > caught. While you managed to show a few BHs that actually don't need to
> > > be considered for recording when I asked for this in v7, most BHs in the
> > > block layer can in some way lead to device callbacks and must therefore
> > > be recorded.
> >
> > Let's have a brief review. I can change all the places, but how
> > should I make a test case to be sure, that all of them are working ok?
> 
> The list is changing all the time. This is why I am so concerned about
> special-casing a few callers instead of having a generic solution. I
> don't know how we could make sure that we call the right function
> everywhere.
> 
> > aio_bh_schedule_oneshot is used in:
> >  - blk_abort_aio_request
> >  - bdrv_co_yield_to_drain
> >  - iscsi_co_generic_cb
> >  - nfs_co_generic_cb
> >  - null_aio_common
> >  - nvme_process_completion
> >  - nvme_rw_cb
> >  - rbd_finish_aiocb
> >  - vxhs_iio_callback
> >  - (and couple of others not in the block layer)
> 
> In addition to these, we have at least a few functions that just resume
> block layer coroutines rather than directly scheduling a BH with a
> callback somewhere in the block layer.
> 
> > We must change this call to replay_bh_schedule_oneshot_event when
> > the result of the BH execution affects the replayed guest state
> > (e.g., interrupt request is generated or memory is written)
> >
> > If you think that all of these can do that, then I should change
> > such function calls.
> 
> I haven't reviewed the code, but these names look like all of them can
> eventually call back into the guest devices. They won't do that always,
> but potentially.

Ok, then I'll try making all of them deterministic.

> > > How bad would it be to record some BHs even if recording them isn't
> > > necessary? I'd definitely try to err on the safe side here. Having two
> > > different sets of BH functions, you can't expect that people always use
> > > the right one (especially if you don't even make the existing code base
> > > consistently use the right one intially).
> >
> > There are two possible options:
> > 1. Execution hangs when recording. Kind of deadlock caused by the incorrect
> >    management of the events. E.g., adding stopping the VM and trying to flush
> >    the block layer queue.
> > 2. Execution hangs when replaying.
> >    One of the events that affect the guest state is missed or generated
> >    at the other moment (e.g., when BH is not linked to the execution step).
> >    Then the guest behaves differently and the order of the events in the log
> >    does not match the guest state (e.g., interrupt processing is not matched).
> 
> So basically when you have two events that are kind of nested? 

Can you give a concrete example?

> Operation
> A triggers event A, but in order to complete the operation, you call
> operation B with event B internally, which isn't available yet because
> we're still handling event A?
> 
> Could this be solved by not having an order of events, but an order of
> sets of events?

There are two kind of operations: synchronous and not.
E.g., querying the disk contents through the PIO waits until the disk operation
is finished, and does not need other synchronization. DMA operations are processed
by coroutines and bottom halves. Synchronization of these objects should be
recorded, because the moment of invocation may be different, and it affects
the guest state.

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v9 19/21] replay: add BH oneshot event for block layer

[Pavel Dovgalyuk]

> From: Kevin Wolf [mailto:kwolf@redhat.com]
> Am 14.01.2019 um 12:10 hat Pavel Dovgalyuk geschrieben:
> > > From: Kevin Wolf [mailto:kwolf@redhat.com]
> > > Am 09.01.2019 um 13:13 hat Pavel Dovgalyuk geschrieben:
> > > > 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>
> > >
> > > This still doesn't come even close to catching all BHs that need to be
> > > caught. While you managed to show a few BHs that actually don't need to
> > > be considered for recording when I asked for this in v7, most BHs in the
> > > block layer can in some way lead to device callbacks and must therefore
> > > be recorded.
> >
> > Let's have a brief review. I can change all the places, but how
> > should I make a test case to be sure, that all of them are working ok?
> 
> The list is changing all the time. This is why I am so concerned about
> special-casing a few callers instead of having a generic solution. I
> don't know how we could make sure that we call the right function
> everywhere.

I changed all oneshot invocations in the block layer in the new version.
Can you review it and other block-related patches?

Pavel Dovgalyuk