[Qemu-devel] [PATCH v5 00/24] Fixing record/replay and adding reverse debugging

[Qemu-devel] [PATCH v5 00/24] 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:
 - fixes of record/replay caused by the QEMU core changes
 - 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-180725

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 (24):
      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: introduce info hmp/qmp command
      replay: introduce breakpoint at the specified step
      replay: implement replay-seek command to proceed to the desired step
      replay: flush events when exiting
      timer: remove replay clock probe in deadline calculation
      replay: refine replay-time module
      translator: fix breakpoint processing
      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: allow loading any snapshots before recording
      ps2: prevent changing irq state on save and load
      replay: wake up vCPU when replaying
      replay: replay BH for IDE trim operation
      replay: add BH oneshot event for block layer
      slirp: fix ipv6 timers


 accel/tcg/translator.c    |    9 +
 block/blkreplay.c         |    8 +
 block/block-backend.c     |    3 
 block/io.c                |   22 +++
 block/qapi.c              |   17 ++-
 block/qcow2-snapshot.c    |    9 +
 block/qcow2.h             |    2 
 blockdev.c                |   10 ++
 cpus.c                    |   50 +++++---
 docs/interop/qcow2.txt    |    4 +
 docs/replay.txt           |   45 +++++++
 exec.c                    |    6 +
 gdbstub.c                 |   50 +++++++-
 hmp-commands-info.hx      |   14 ++
 hmp-commands.hx           |   30 +++++
 hmp.h                     |    3 
 hw/ide/core.c             |    3 
 hw/input/ps2.c            |    8 +
 include/block/snapshot.h  |    1 
 include/sysemu/replay.h   |   24 ++++
 migration/savevm.c        |   15 +-
 qapi/block-core.json      |    5 +
 qapi/block.json           |    3 
 qapi/misc.json            |   68 +++++++++++
 replay/Makefile.objs      |    3 
 replay/replay-debugging.c |  287 +++++++++++++++++++++++++++++++++++++++++++++
 replay/replay-events.c    |   30 +++--
 replay/replay-internal.h  |   11 +-
 replay/replay-snapshot.c  |   17 ++-
 replay/replay-time.c      |   27 ++--
 replay/replay.c           |   36 +++++-
 slirp/ip6_icmp.c          |    6 -
 stubs/replay.c            |   16 +++
 util/qemu-timer.c         |   11 --
 vl.c                      |   18 ++-
 35 files changed, 772 insertions(+), 99 deletions(-)
 create mode 100644 replay/replay-debugging.c

-- 
Pavel Dovgalyuk

[Qemu-devel] [PATCH v5 01/24] 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 766150a..ad287f7 100755
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -130,6 +130,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,
@@ -145,6 +151,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 v5 02/24] 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 16b913f..2a6d987 100644
--- a/vl.c
+++ b/vl.c
@@ -3111,7 +3111,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"),
@@ -4443,7 +4449,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 v5 03/24] 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 2e21e9c..f7def53 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 v5 04/24] 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.

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

diff --git a/block/io.c b/block/io.c
index 7100344..4d7a6e4 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 */
 
@@ -533,6 +534,13 @@ 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());
@@ -561,6 +569,13 @@ 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);
 
@@ -1993,6 +2008,13 @@ 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 b5844b7..9b98b1f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1016,7 +1016,6 @@ static int do_vm_stop(RunState state, bool send_stop)
     }
 
     bdrv_drain_all();
-    replay_disable_events();
     ret = bdrv_flush_all();
 
     return ret;
@@ -2067,7 +2066,6 @@ int vm_prepare_start(void)
     /* We are sending this now, but the CPUs will be resumed shortly later */
     qapi_event_send_resume(&error_abort);
 
-    replay_enable_events();
     cpu_enable_ticks();
     runstate_set(RUN_STATE_RUNNING);
     vm_state_notify(1, RUN_STATE_RUNNING);

[Qemu-devel] [PATCH v5 05/24] 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 8228261..58a986f 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -366,6 +366,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 2a6d987..e86d295 100644
--- a/vl.c
+++ b/vl.c
@@ -4653,6 +4653,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 v5 06/24] 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>

--

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 81b844e..b24ec93 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -148,6 +148,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;
 
 
@@ -161,6 +162,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 845d40a..aa1101d 100644
--- a/docs/interop/qcow2.txt
+++ b/docs/interop/qcow2.txt
@@ -506,6 +506,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 v5 07/24] 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)
---
 block/qapi.c             |   17 +++++++++++++----
 block/qcow2-snapshot.c   |    2 ++
 blockdev.c               |   10 ++++++++++
 include/block/snapshot.h |    1 +
 migration/savevm.c       |    5 +++++
 qapi/block-core.json     |    5 ++++-
 qapi/block.json          |    3 ++-
 7 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index e12968f..a89e180 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -210,6 +210,7 @@ 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_list = g_new0(SnapshotInfoList, 1);
         info_list->value = info;
@@ -648,14 +649,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);
@@ -668,13 +670,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);
     }
 }
 
@@ -842,6 +849,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 dcf8c8d..f09d5e5 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"
@@ -1349,6 +1350,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;
 
@@ -1557,6 +1562,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 7f92567..6be4f80 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2454,6 +2454,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 d40d5ec..29de85c 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -26,13 +26,16 @@
 #
 # @vm-clock-nsec: fractional part in nano seconds to be used with vm-clock-sec
 #
+# @icount: current instruction count for execution record/replay (since 3.1)
+#
 # 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 v5 08/24] 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).

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

--

v2:
 - renamed info_replay qmp into query-replay (suggested by Eric Blake)
---
 hmp-commands-info.hx      |   14 ++++++++++++++
 hmp.h                     |    1 +
 qapi/misc.json            |   35 +++++++++++++++++++++++++++++++++++
 replay/Makefile.objs      |    3 ++-
 replay/replay-debugging.c |   41 +++++++++++++++++++++++++++++++++++++++++
 replay/replay-internal.h  |    2 ++
 replay/replay.c           |    3 +--
 7 files changed, 96 insertions(+), 3 deletions(-)
 create mode 100644 replay/replay-debugging.c

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 70639f6..1b24714 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -896,6 +896,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 current record/replay mode and the currently executing step.
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/hmp.h b/hmp.h
index 33354f1..9d12c63 100644
--- a/hmp.h
+++ b/hmp.h
@@ -147,5 +147,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/misc.json b/qapi/misc.json
index d450cfe..e246ce3 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -3100,6 +3100,41 @@
   'data': [ 'none', 'record', 'play' ] }
 
 ##
+# @ReplayInfo:
+#
+# Status of the record/replay mode.
+#
+# @mode: current mode.
+#
+# @filename: name of the record/replay log file.
+#
+# @step: current step number.
+#
+# Since: 3.1
+#
+##
+{ 'struct': 'ReplayInfo',
+  'data': { 'mode': 'ReplayMode', '*filename': 'str', 'step': 'int' } }
+
+##
+# @query-replay:
+#
+# Retrieves the status of the execution record/replay.
+#
+# Returns: structure with the properties of the record/replay.
+#
+# Since: 3.1
+#
+# Example:
+#
+# -> { "execute": "query-replay" }
+# <- { "return": { "mode": "play", "filename": "log.rr", "step": 220414 } }
+#
+##
+{ 'command': 'query-replay',
+  'returns': 'ReplayInfo' }
+
+##
 # @xen-load-devices-state:
 #
 # Load the state of all devices from file. The RAM and the block devices
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..03e7db8
--- /dev/null
+++ b/replay/replay-debugging.c
@@ -0,0 +1,41 @@
+/*
+ * 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-misc.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_filename, replay_get_current_step());
+    }
+}
+
+ReplayInfo *qmp_query_replay(Error **errp)
+{
+    ReplayInfo *retval = g_new0(ReplayInfo, 1);
+    retval->mode = replay_mode;
+    if (replay_filename) {
+        retval->filename = g_strdup(replay_filename);
+        retval->has_filename = true;
+    }
+    retval->step = replay_get_current_step();
+    return retval;
+}
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index ac4b27b..ef82b5e 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -91,6 +91,8 @@ extern ReplayState replay_state;
 
 /* File for replay writing */
 extern FILE *replay_file;
+/*! Name of replay file  */
+extern char *replay_filename;
 
 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 58a986f..8b70d7d 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -29,8 +29,7 @@
 ReplayMode replay_mode = REPLAY_MODE_NONE;
 char *replay_snapshot;
 
-/* Name of replay file  */
-static char *replay_filename;
+char *replay_filename;
 ReplayState replay_state;
 static GSList *replay_blockers;
 

Re: [Qemu-devel] [PATCH v5 08/24] replay: introduce info hmp/qmp command

[Dr. David Alan Gilbert]

* Pavel Dovgalyuk (Pavel.Dovgaluk@ispras.ru) wrote:
> 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).
> 
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

ACK for HMP

Note you might want to make an accessor for the filename rather than
making it a global?

Dave

> 
> --
> 
> v2:
>  - renamed info_replay qmp into query-replay (suggested by Eric Blake)
> ---
>  hmp-commands-info.hx      |   14 ++++++++++++++
>  hmp.h                     |    1 +
>  qapi/misc.json            |   35 +++++++++++++++++++++++++++++++++++
>  replay/Makefile.objs      |    3 ++-
>  replay/replay-debugging.c |   41 +++++++++++++++++++++++++++++++++++++++++
>  replay/replay-internal.h  |    2 ++
>  replay/replay.c           |    3 +--
>  7 files changed, 96 insertions(+), 3 deletions(-)
>  create mode 100644 replay/replay-debugging.c
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 70639f6..1b24714 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -896,6 +896,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 current record/replay mode and the currently executing step.
> +ETEXI
> +
>  STEXI
>  @end table
>  ETEXI
> diff --git a/hmp.h b/hmp.h
> index 33354f1..9d12c63 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -147,5 +147,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/misc.json b/qapi/misc.json
> index d450cfe..e246ce3 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -3100,6 +3100,41 @@
>    'data': [ 'none', 'record', 'play' ] }
>  
>  ##
> +# @ReplayInfo:
> +#
> +# Status of the record/replay mode.
> +#
> +# @mode: current mode.
> +#
> +# @filename: name of the record/replay log file.
> +#
> +# @step: current step number.
> +#
> +# Since: 3.1
> +#
> +##
> +{ 'struct': 'ReplayInfo',
> +  'data': { 'mode': 'ReplayMode', '*filename': 'str', 'step': 'int' } }
> +
> +##
> +# @query-replay:
> +#
> +# Retrieves the status of the execution record/replay.
> +#
> +# Returns: structure with the properties of the record/replay.
> +#
> +# Since: 3.1
> +#
> +# Example:
> +#
> +# -> { "execute": "query-replay" }
> +# <- { "return": { "mode": "play", "filename": "log.rr", "step": 220414 } }
> +#
> +##
> +{ 'command': 'query-replay',
> +  'returns': 'ReplayInfo' }
> +
> +##
>  # @xen-load-devices-state:
>  #
>  # Load the state of all devices from file. The RAM and the block devices
> 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..03e7db8
> --- /dev/null
> +++ b/replay/replay-debugging.c
> @@ -0,0 +1,41 @@
> +/*
> + * 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-misc.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_filename, replay_get_current_step());
> +    }
> +}
> +
> +ReplayInfo *qmp_query_replay(Error **errp)
> +{
> +    ReplayInfo *retval = g_new0(ReplayInfo, 1);
> +    retval->mode = replay_mode;
> +    if (replay_filename) {
> +        retval->filename = g_strdup(replay_filename);
> +        retval->has_filename = true;
> +    }
> +    retval->step = replay_get_current_step();
> +    return retval;
> +}
> diff --git a/replay/replay-internal.h b/replay/replay-internal.h
> index ac4b27b..ef82b5e 100644
> --- a/replay/replay-internal.h
> +++ b/replay/replay-internal.h
> @@ -91,6 +91,8 @@ extern ReplayState replay_state;
>  
>  /* File for replay writing */
>  extern FILE *replay_file;
> +/*! Name of replay file  */
> +extern char *replay_filename;
>  
>  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 58a986f..8b70d7d 100644
> --- a/replay/replay.c
> +++ b/replay/replay.c
> @@ -29,8 +29,7 @@
>  ReplayMode replay_mode = REPLAY_MODE_NONE;
>  char *replay_snapshot;
>  
> -/* Name of replay file  */
> -static char *replay_filename;
> +char *replay_filename;
>  ReplayState replay_state;
>  static GSList *replay_blockers;
>  
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v5 08/24] replay: introduce info hmp/qmp command

[Pavel Dovgalyuk]

> From: Dr. David Alan Gilbert [mailto:dgilbert@redhat.com]
> * Pavel Dovgalyuk (Pavel.Dovgaluk@ispras.ru) wrote:
> > 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).
> >
> > Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> 
> ACK for HMP
> 
> Note you might want to make an accessor for the filename rather than
> making it a global?

Good idea, thanks.


> 
> Dave
> 
> >
> > --
> >
> > v2:
> >  - renamed info_replay qmp into query-replay (suggested by Eric Blake)
> > ---
> >  hmp-commands-info.hx      |   14 ++++++++++++++
> >  hmp.h                     |    1 +
> >  qapi/misc.json            |   35 +++++++++++++++++++++++++++++++++++
> >  replay/Makefile.objs      |    3 ++-
> >  replay/replay-debugging.c |   41 +++++++++++++++++++++++++++++++++++++++++
> >  replay/replay-internal.h  |    2 ++
> >  replay/replay.c           |    3 +--
> >  7 files changed, 96 insertions(+), 3 deletions(-)
> >  create mode 100644 replay/replay-debugging.c
> >
> > diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> > index 70639f6..1b24714 100644
> > --- a/hmp-commands-info.hx
> > +++ b/hmp-commands-info.hx
> > @@ -896,6 +896,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 current record/replay mode and the currently executing step.
> > +ETEXI
> > +
> >  STEXI
> >  @end table
> >  ETEXI
> > diff --git a/hmp.h b/hmp.h
> > index 33354f1..9d12c63 100644
> > --- a/hmp.h
> > +++ b/hmp.h
> > @@ -147,5 +147,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/misc.json b/qapi/misc.json
> > index d450cfe..e246ce3 100644
> > --- a/qapi/misc.json
> > +++ b/qapi/misc.json
> > @@ -3100,6 +3100,41 @@
> >    'data': [ 'none', 'record', 'play' ] }
> >
> >  ##
> > +# @ReplayInfo:
> > +#
> > +# Status of the record/replay mode.
> > +#
> > +# @mode: current mode.
> > +#
> > +# @filename: name of the record/replay log file.
> > +#
> > +# @step: current step number.
> > +#
> > +# Since: 3.1
> > +#
> > +##
> > +{ 'struct': 'ReplayInfo',
> > +  'data': { 'mode': 'ReplayMode', '*filename': 'str', 'step': 'int' } }
> > +
> > +##
> > +# @query-replay:
> > +#
> > +# Retrieves the status of the execution record/replay.
> > +#
> > +# Returns: structure with the properties of the record/replay.
> > +#
> > +# Since: 3.1
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "query-replay" }
> > +# <- { "return": { "mode": "play", "filename": "log.rr", "step": 220414 } }
> > +#
> > +##
> > +{ 'command': 'query-replay',
> > +  'returns': 'ReplayInfo' }
> > +
> > +##
> >  # @xen-load-devices-state:
> >  #
> >  # Load the state of all devices from file. The RAM and the block devices
> > 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..03e7db8
> > --- /dev/null
> > +++ b/replay/replay-debugging.c
> > @@ -0,0 +1,41 @@
> > +/*
> > + * 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-misc.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_filename, replay_get_current_step());
> > +    }
> > +}
> > +
> > +ReplayInfo *qmp_query_replay(Error **errp)
> > +{
> > +    ReplayInfo *retval = g_new0(ReplayInfo, 1);
> > +    retval->mode = replay_mode;
> > +    if (replay_filename) {
> > +        retval->filename = g_strdup(replay_filename);
> > +        retval->has_filename = true;
> > +    }
> > +    retval->step = replay_get_current_step();
> > +    return retval;
> > +}
> > diff --git a/replay/replay-internal.h b/replay/replay-internal.h
> > index ac4b27b..ef82b5e 100644
> > --- a/replay/replay-internal.h
> > +++ b/replay/replay-internal.h
> > @@ -91,6 +91,8 @@ extern ReplayState replay_state;
> >
> >  /* File for replay writing */
> >  extern FILE *replay_file;
> > +/*! Name of replay file  */
> > +extern char *replay_filename;
> >
> >  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 58a986f..8b70d7d 100644
> > --- a/replay/replay.c
> > +++ b/replay/replay.c
> > @@ -29,8 +29,7 @@
> >  ReplayMode replay_mode = REPLAY_MODE_NONE;
> >  char *replay_snapshot;
> >
> > -/* Name of replay file  */
> > -static char *replay_filename;
> > +char *replay_filename;
> >  ReplayState replay_state;
> >  static GSList *replay_blockers;
> >
> >
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Pavel Dovgalyuk

[Qemu-devel] [PATCH v5 09/24] replay: introduce breakpoint at the specified step

[Pavel Dovgalyuk]

This patch introduces replay_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.
The commands have one argument - number of instructions
executed since the start of the replay.

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

--

v2:
 - renamed replay_break qmp command into replay-break
   (suggested by Eric Blake)
---
 hmp-commands.hx           |   15 ++++++++++++
 hmp.h                     |    1 +
 include/sysemu/replay.h   |    3 ++
 qapi/misc.json            |   17 ++++++++++++++
 replay/replay-debugging.c |   55 +++++++++++++++++++++++++++++++++++++++++++++
 replay/replay-internal.h  |    4 +++
 replay/replay.c           |   17 ++++++++++++++
 7 files changed, 112 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 91dfe51..ec58ca4 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1874,6 +1874,21 @@ Set QOM property @var{property} of object at location @var{path} to value @var{v
 ETEXI
 
     {
+        .name       = "replay_break",
+        .args_type  = "step:i",
+        .params     = "step",
+        .help       = "sets breakpoint on the specified step of the replay",
+        .cmd        = hmp_replay_break,
+    },
+
+STEXI
+@item replay_break @var{step}
+@findex replay_break
+Set breakpoint on the specified step of the replay.
+Execution stops when the specified step is reached.
+ETEXI
+
+    {
         .name       = "info",
         .args_type  = "item:s?",
         .params     = "[subcommand]",
diff --git a/hmp.h b/hmp.h
index 9d12c63..3eeea7d 100644
--- a/hmp.h
+++ b/hmp.h
@@ -148,5 +148,6 @@ 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);
 
 #endif
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 3ced6bc..98d709c 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -71,6 +71,9 @@ void replay_start(void);
 void replay_finish(void);
 /*! Adds replay blocker with the specified error description */
 void replay_add_blocker(Error *reason);
+/*! Sets breakpoint at the specified step.
+    If step = -1LL the existing breakpoint is removed. */
+void replay_break(int64_t step, QEMUTimerCB callback, void *opaque);
 
 /* Processing the instructions */
 
diff --git a/qapi/misc.json b/qapi/misc.json
index e246ce3..4fcd211 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -3135,6 +3135,23 @@
   'returns': 'ReplayInfo' }
 
 ##
+# @replay-break:
+#
+# Set breakpoint on the specified step of the replay.
+# Execution stops when the specified step is reached.
+#
+# @step: execution step to stop at
+#
+# Since: 3.1
+#
+# Example:
+#
+# -> { "execute": "replay-break", "data": { "step": 220414 } }
+#
+##
+{ 'command': 'replay-break', 'data': { 'step': 'int' } }
+
+##
 # @xen-load-devices-state:
 #
 # Load the state of all devices from file. The RAM and the block devices
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index 03e7db8..819017e 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-misc.h"
+#include "qapi/qmp/qdict.h"
+#include "qemu/timer.h"
 
 void hmp_info_replay(Monitor *mon, const QDict *qdict)
 {
@@ -39,3 +41,56 @@ ReplayInfo *qmp_query_replay(Error **errp)
     retval->step = replay_get_current_step();
     return retval;
 }
+
+void replay_break(int64_t step, QEMUTimerCB callback, void *opaque)
+{
+    assert(replay_mode == REPLAY_MODE_PLAY);
+    assert(replay_mutex_locked());
+
+    replay_break_step = step;
+    if (replay_break_timer) {
+        timer_del(replay_break_timer);
+        timer_free(replay_break_timer);
+        replay_break_timer = NULL;
+    }
+
+    if (replay_break_step == -1LL) {
+        return;
+    }
+    assert(replay_break_step >= replay_get_current_step());
+    assert(callback);
+
+    replay_break_timer = timer_new_ns(QEMU_CLOCK_REALTIME, callback, opaque);
+}
+
+static void replay_stop_vm(void *opaque)
+{
+    vm_stop(RUN_STATE_PAUSED);
+    replay_break(-1LL, NULL, NULL);
+}
+
+void qmp_replay_break(int64_t step, Error **errp)
+{
+    if (replay_mode ==  REPLAY_MODE_PLAY) {
+        if (step >= replay_get_current_step()) {
+            replay_break(step, replay_stop_vm, NULL);
+        } else {
+            error_setg(errp, "cannot set break at the step in the past");
+        }
+    } else {
+        error_setg(errp, "setting the break is allowed only in play mode");
+    }
+}
+
+void hmp_replay_break(Monitor *mon, const QDict *qdict)
+{
+    int64_t step = qdict_get_try_int(qdict, "step", -1LL);
+    Error *err = NULL;
+
+    qmp_replay_break(step, &err);
+    if (err) {
+        monitor_printf(mon, "replay_break error: %s\n", error_get_pretty(err));
+        error_free(err);
+        return;
+    }
+}
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index ef82b5e..34d19eb 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -93,6 +93,10 @@ extern ReplayState replay_state;
 extern FILE *replay_file;
 /*! Name of replay file  */
 extern char *replay_filename;
+/*! Step of the replay breakpoint */
+extern int64_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 8b70d7d..dcce902 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -33,6 +33,10 @@ char *replay_filename;
 ReplayState replay_state;
 static GSList *replay_blockers;
 
+/* Replay breakpoints */
+int64_t replay_break_step = -1LL;
+QEMUTimer *replay_break_timer;
+
 bool replay_next_event_is(int event)
 {
     bool res = false;
@@ -72,6 +76,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;
@@ -98,6 +109,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 v5 10/24] 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 commands 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)
---
 hmp-commands.hx           |   15 ++++++++
 hmp.h                     |    1 +
 qapi/misc.json            |   16 ++++++++
 replay/replay-debugging.c |   89 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 121 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index ec58ca4..cdae207 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1889,6 +1889,21 @@ Execution stops when the specified step is reached.
 ETEXI
 
     {
+        .name       = "replay_seek",
+        .args_type  = "step:i",
+        .params     = "step",
+        .help       = "rewinds replay to the specified step",
+        .cmd        = hmp_replay_seek,
+    },
+
+STEXI
+@item replay_seek @var{step}
+@findex replay_seek
+Automatically proceeds to the specified step, when replaying
+the execution.
+ETEXI
+
+    {
         .name       = "info",
         .args_type  = "item:s?",
         .params     = "[subcommand]",
diff --git a/hmp.h b/hmp.h
index 3eeea7d..b6b31b0 100644
--- a/hmp.h
+++ b/hmp.h
@@ -149,5 +149,6 @@ 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_seek(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi/misc.json b/qapi/misc.json
index 4fcd211..7389cc6 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -3152,6 +3152,22 @@
 { 'command': 'replay-break', 'data': { 'step': 'int' } }
 
 ##
+# @replay-seek:
+#
+# Automatically proceeds to the specified step, when replaying
+# the execution.
+#
+# @step: destination execution step
+#
+# Since: 3.1
+#
+# Example:
+#
+# -> { "execute": "replay-seek", "data": { "step": 220414 } }
+##
+{ 'command': 'replay-seek', 'data': { 'step': 'int' } }
+
+##
 # @xen-load-devices-state:
 #
 # Load the state of all devices from file. The RAM and the block devices
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index 819017e..8d6c03d 100644
--- a/replay/replay-debugging.c
+++ b/replay/replay-debugging.c
@@ -18,6 +18,8 @@
 #include "qapi/qapi-commands-misc.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)
 {
@@ -94,3 +96,90 @@ void hmp_replay_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, Error **errp, QEMUTimerCB callback)
+{
+    char *snapshot = NULL;
+    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;
+    }
+    int64_t snapshot_step = -1;
+    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 step, Error **errp)
+{
+    replay_seek(step, errp, replay_stop_vm);
+}
+
+void hmp_replay_seek(Monitor *mon, const QDict *qdict)
+{
+    int64_t step = qdict_get_try_int(qdict, "step", -1LL);
+    Error *err = NULL;
+
+    qmp_replay_seek(step, &err);
+    if (err) {
+        monitor_printf(mon, "replay_seek error: %s\n", error_get_pretty(err));
+        error_free(err);
+        return;
+    }
+}

[Qemu-devel] [PATCH v5 11/24] replay: flush events when exiting

[Pavel Dovgalyuk]

This patch adds events processing when emulation finishes instead
of just cleaning the queue. Now the bdrv coroutines will be in consistent
state when emulator closes. It allows correct polling of the block layer
at exit.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
---
 replay/replay-events.c   |   14 +-------------
 replay/replay-internal.h |    2 --
 2 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/replay/replay-events.c b/replay/replay-events.c
index 707de38..0964a82 100644
--- a/replay/replay-events.c
+++ b/replay/replay-events.c
@@ -94,18 +94,6 @@ void replay_disable_events(void)
     }
 }
 
-void replay_clear_events(void)
-{
-    g_assert(replay_mutex_locked());
-
-    while (!QTAILQ_EMPTY(&events_list)) {
-        Event *event = QTAILQ_FIRST(&events_list);
-        QTAILQ_REMOVE(&events_list, event, events);
-
-        g_free(event);
-    }
-}
-
 /*! Adds specified async event to the queue */
 void replay_add_event(ReplayAsyncEventKind event_kind,
                       void *opaque,
@@ -308,7 +296,7 @@ void replay_init_events(void)
 void replay_finish_events(void)
 {
     events_enabled = false;
-    replay_clear_events();
+    replay_flush_events();
 }
 
 bool replay_events_enabled(void)
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index 34d19eb..a2221e5 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -148,8 +148,6 @@ void replay_init_events(void);
 void replay_finish_events(void);
 /*! Flushes events queue */
 void replay_flush_events(void);
-/*! Clears events list before loading new VM state */
-void replay_clear_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 v5 12/24] timer: remove replay clock probe in deadline calculation

[Pavel Dovgalyuk]

Ciro Santilli reported that commit a5ed352596a8b7eb2f9acce34371b944ac3056c4
breaks the execution replay. It happens due to the probing the clock
for the new instances of iothread.
However, this probing was made in replay mode for the timer lists that
are empty.
This patch removes clock probing in replay mode.
It is an artifact of the old version with another thread model.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
---
 util/qemu-timer.c |   11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index 2ed1bf2..86bfe84 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -578,17 +578,10 @@ int64_t timerlistgroup_deadline_ns(QEMUTimerListGroup *tlg)
 {
     int64_t deadline = -1;
     QEMUClockType type;
-    bool play = replay_mode == REPLAY_MODE_PLAY;
     for (type = 0; type < QEMU_CLOCK_MAX; type++) {
         if (qemu_clock_use_for_deadline(type)) {
-            if (!play || type == QEMU_CLOCK_REALTIME) {
-                deadline = qemu_soonest_timeout(deadline,
-                                                timerlist_deadline_ns(tlg->tl[type]));
-            } else {
-                /* Read clock from the replay file and
-                   do not calculate the deadline, based on virtual clock. */
-                qemu_clock_get_ns(type);
-            }
+            deadline = qemu_soonest_timeout(deadline,
+                                            timerlist_deadline_ns(tlg->tl[type]));
         }
     }
     return deadline;

[Qemu-devel] [PATCH v5 13/24] 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 |   27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/replay/replay-time.c b/replay/replay-time.c
index 6a7565e..40030b8 100644
--- a/replay/replay-time.c
+++ b/replay/replay-time.c
@@ -17,14 +17,12 @@
 
 int64_t replay_save_clock(ReplayClockKind kind, int64_t clock)
 {
+    g_assert(replay_file);
+    g_assert(replay_mutex_locked());
 
-    if (replay_file) {
-        g_assert(replay_mutex_locked());
-
-        replay_save_instructions();
-        replay_put_event(EVENT_CLOCK + kind);
-        replay_put_qword(clock);
-    }
+    replay_save_instructions();
+    replay_put_event(EVENT_CLOCK + kind);
+    replay_put_qword(clock);
 
     return clock;
 }
@@ -46,20 +44,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 v5 14/24] translator: fix breakpoint processing

[Pavel Dovgalyuk]

QEMU cannot pass through the breakpoints when 'si' command is used
in remote gdb. This patch disables inserting the breakpoints
when we are already single stepping though the gdb remote protocol.
This patch also fixes icount calculation for the blocks that include
breakpoints - instruction with breakpoint is not executed and shouldn't
be used in icount calculation.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
---
 accel/tcg/translator.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 0f9dca9..afd0a49 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -34,6 +34,8 @@ void translator_loop_temp_check(DisasContextBase *db)
 void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
                      CPUState *cpu, TranslationBlock *tb)
 {
+    int bp_insn = 0;
+
     /* Initialize DisasContext */
     db->tb = tb;
     db->pc_first = tb->pc;
@@ -71,11 +73,13 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
         tcg_debug_assert(db->is_jmp == DISAS_NEXT);  /* no early exit */
 
         /* Pass breakpoint hits to target for further processing */
-        if (unlikely(!QTAILQ_EMPTY(&cpu->breakpoints))) {
+        if (!db->singlestep_enabled
+            && unlikely(!QTAILQ_EMPTY(&cpu->breakpoints))) {
             CPUBreakpoint *bp;
             QTAILQ_FOREACH(bp, &cpu->breakpoints, entry) {
                 if (bp->pc == db->pc_next) {
                     if (ops->breakpoint_check(db, cpu, bp)) {
+                        bp_insn = 1;
                         break;
                     }
                 }
@@ -118,7 +122,7 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
 
     /* Emit code to exit the TB, as indicated by db->is_jmp.  */
     ops->tb_stop(db, cpu);
-    gen_tb_end(db->tb, db->num_insns);
+    gen_tb_end(db->tb, db->num_insns - bp_insn);
 
     /* The disas_log hook may use these values rather than recompute.  */
     db->tb->size = db->pc_next - db->pc_first;

[Qemu-devel] [PATCH v5 15/24] 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       |   10 ++++------
 replay/replay-internal.h |    2 --
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 98d709c..84a1ec5 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -132,6 +132,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 6be4f80..834dd25 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2607,12 +2607,6 @@ int load_snapshot(const char *name, Error **errp)
     AioContext *aio_context;
     MigrationIncomingState *mis = migration_incoming_get_current();
 
-    if (!replay_can_snapshot()) {
-        error_report("Record/replay does not allow loading snapshot "
-                     "right now. Try once more later.");
-        return -EINVAL;
-    }
-
     if (!bdrv_all_can_snapshot(&bs)) {
         error_setg(errp,
                    "Device '%s' is writable but does not support snapshots",
@@ -2646,6 +2640,10 @@ 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 a2221e5..08ef2ec 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 v5 16/24] 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                    |    5 +++++
 gdbstub.c                 |   42 +++++++++++++++++++++++++++++++++++++++---
 include/sysemu/replay.h   |    7 +++++++
 replay/replay-debugging.c |   33 +++++++++++++++++++++++++++++++++
 stubs/replay.c            |    5 +++++
 7 files changed, 101 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 9b98b1f..e53a364 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1042,9 +1042,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 4f5df07..c4b5125 100644
--- a/exec.c
+++ b/exec.c
@@ -2761,6 +2761,11 @@ 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 d6ab950..fa108f4 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"
@@ -334,6 +335,19 @@ 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;
@@ -424,7 +438,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;
@@ -443,7 +457,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;
@@ -1082,9 +1096,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;
@@ -1347,6 +1380,9 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
             if (cc->gdb_core_xml_file != NULL) {
                 pstrcat(buf, sizeof(buf), ";qXfer:features:read+");
             }
+            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 84a1ec5..611eabb 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -74,6 +74,13 @@ void replay_add_blocker(Error *reason);
 /*! Sets breakpoint at the specified step.
     If step = -1LL the existing breakpoint is removed. */
 void replay_break(int64_t step, QEMUTimerCB callback, void *opaque);
+/*! 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 8d6c03d..388cf12 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) {
@@ -183,3 +190,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, &err, replay_stop_vm_debug);
+        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 04279ab..b0fba0e 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 v5 17/24] 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                    |    3 ++
 exec.c                    |    1 +
 gdbstub.c                 |   10 ++++++-
 include/sysemu/replay.h   |    6 ++++
 replay/replay-debugging.c |   69 +++++++++++++++++++++++++++++++++++++++++++++
 stubs/replay.c            |    5 +++
 6 files changed, 93 insertions(+), 1 deletion(-)

diff --git a/cpus.c b/cpus.c
index e53a364..181ce33 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1048,6 +1048,9 @@ 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 c4b5125..151e4f1 100644
--- a/exec.c
+++ b/exec.c
@@ -2764,6 +2764,7 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags)
             if (replay_running_debug()) {
                 /* 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 fa108f4..ecdc0b2 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1111,6 +1111,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;
             }
@@ -1381,7 +1389,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
                 pstrcat(buf, sizeof(buf), ";qXfer:features:read+");
             }
             if (replay_mode == REPLAY_MODE_PLAY) {
-                pstrcat(buf, sizeof(buf), ";ReverseStep+");
+                pstrcat(buf, sizeof(buf), ";ReverseStep+;ReverseContinue+");
             }
             put_packet(s, buf);
             break;
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 611eabb..a3113c1 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -78,9 +78,15 @@ void replay_break(int64_t step, QEMUTimerCB callback, void *opaque);
     Used by gdbstub for backwards debugging.
     Returns true on success. */
 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 388cf12..edab98e 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)
 {
@@ -216,3 +218,70 @@ 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, &err, replay_stop_vm_debug);
+        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, &err, replay_continue_stop);
+        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, &err, replay_stop_vm_debug);
+        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, &err, replay_continue_stop);
+        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 b0fba0e..781974e 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 v5 18/24] 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 f7def53..086d3f8 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 v5 19/24] replay: allow loading any snapshots before recording

[Pavel Dovgalyuk]

This patch enables using -loadvm in recording mode to allow starting
the execution recording from any of the available snapshots.
It also fixes loading of the record/replay state, therefore snapshots
created in replay mode may also be used for starting the new recording.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
---
 replay/replay-snapshot.c |   17 ++++++++++++-----
 vl.c                     |    7 ++++---
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
index 2ab85cf..16bacc9 100644
--- a/replay/replay-snapshot.c
+++ b/replay/replay-snapshot.c
@@ -33,11 +33,18 @@ static int replay_pre_save(void *opaque)
 static int replay_post_load(void *opaque, int version_id)
 {
     ReplayState *state = opaque;
-    fseek(replay_file, state->file_offset, SEEK_SET);
-    qemu_clock_set_last(QEMU_CLOCK_HOST, state->host_clock_last);
-    /* If this was a vmstate, saved in recording mode,
-       we need to initialize replay data fields. */
-    replay_fetch_data_kind();
+    if (replay_mode == REPLAY_MODE_PLAY) {
+        fseek(replay_file, state->file_offset, SEEK_SET);
+        qemu_clock_set_last(QEMU_CLOCK_HOST, state->host_clock_last);
+        /* If this was a vmstate, saved in recording mode,
+           we need to initialize replay data fields. */
+        replay_fetch_data_kind();
+    } else if (replay_mode == REPLAY_MODE_RECORD) {
+        /* This is only useful for loading the initial state.
+           Therefore reset all the counters. */
+        state->instructions_count = 0;
+        state->block_request_id = 0;
+    }
 
     return 0;
 }
diff --git a/vl.c b/vl.c
index e86d295..59b018d 100644
--- a/vl.c
+++ b/vl.c
@@ -4616,15 +4616,16 @@ int main(int argc, char **argv, char **envp)
     replay_checkpoint(CHECKPOINT_RESET);
     qemu_system_reset(SHUTDOWN_CAUSE_NONE);
     register_global_state();
-    if (replay_mode != REPLAY_MODE_NONE) {
-        replay_vmstate_init();
-    } else if (loadvm) {
+    if (loadvm) {
         Error *local_err = NULL;
         if (load_snapshot(loadvm, &local_err) < 0) {
             error_report_err(local_err);
             autostart = 0;
         }
     }
+    if (replay_mode != REPLAY_MODE_NONE) {
+        replay_vmstate_init();
+    }
 
     qdev_prop_check_globals();
     if (vmstate_dump_file) {

[Qemu-devel] [PATCH v5 20/24] ps2: prevent changing irq state on save and load

[Pavel Dovgalyuk]

Commit 2858ab09e6f708e381fc1a1cc87e747a690c4884 changed
PS/2 keyboard/mouse buffers to the standard size. However, its state
may change when migrating from the old buffer size and therefore irq needs
updating. But this change made wrong, because it throws the whole queue
if there are too much data instead of cropping it.

That commit also updates irq (because the queue state may change).
But updating the irq may change the VM state (and determinism of
the execution). E.g., when replaying the execution, one may save
the VM state and the state of the interrupt controller will be updated
at the moment of saving, instead of using the recorded update events.

This patch makes the queue update deterministic: it removes the update_irq
call and crops the queue to prevent losing the characters and changing
the required irq status.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
---
 hw/input/ps2.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index fdfcadf..6c43fc2 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -914,7 +914,12 @@ static void ps2_common_post_load(PS2State *s)
     uint8_t tmp_data[PS2_QUEUE_SIZE];
 
     /* set the useful data buffer queue size, < PS2_QUEUE_SIZE */
-    size = (q->count < 0 || q->count > PS2_QUEUE_SIZE) ? 0 : q->count;
+    size = q->count;
+    if (q->count < 0) {
+        size = 0;
+    } else if (q->count > PS2_QUEUE_SIZE) {
+        size = PS2_QUEUE_SIZE;
+    }
 
     /* move the queue elements to the start of data array */
     for (i = 0; i < size; i++) {
@@ -929,7 +934,6 @@ static void ps2_common_post_load(PS2State *s)
     q->rptr = 0;
     q->wptr = (size == PS2_QUEUE_SIZE) ? 0 : size;
     q->count = size;
-    s->update_irq(s->update_arg, q->count != 0);
 }
 
 static void ps2_kbd_reset(void *opaque)

[Qemu-devel] [PATCH v5 21/24] replay: wake up vCPU when replaying

[Pavel Dovgalyuk]

In record/replay icount mode vCPU thread and iothread synchronize
the execution using the checkpoints.
vCPU thread processes the virtual timers and iothread processes all others.
When iothread wants to wake up sleeping vCPU thread, it sends dummy queued
work. Therefore it could be the following sequence of the events in
record mode:
 - IO: sending dummy work
 - IO: processing timers
 - CPU: wakeup
 - CPU: clearing dummy work
 - CPU: processing virtual timers

But due to the races in replay mode the sequence may change:
 - IO: sending dummy work
 - CPU: wakeup
 - CPU: clearing dummy work
 - CPU: sleeping again because nothing to do
 - IO: Processing timers
 - CPU: zzzz

In this case vCPU will not wake up, because dummy work is not to be set up
again.

This patch tries to wake up the vCPU when it sleeps and the icount warp
checkpoint isn't met. It means that vCPU has something to do, because
there are no other reasons of non-matching warp checkpoint.

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

--

v5: improve checking that vCPU is still sleeping
---
 cpus.c                  |   31 +++++++++++++++++++++----------
 include/sysemu/replay.h |    3 +++
 replay/replay.c         |   12 ++++++++++++
 3 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/cpus.c b/cpus.c
index 181ce33..38bd521 100644
--- a/cpus.c
+++ b/cpus.c
@@ -539,18 +539,29 @@ void qemu_start_warp_timer(void)
         return;
     }
 
-    /* warp clock deterministically in record/replay mode */
-    if (!replay_checkpoint(CHECKPOINT_CLOCK_WARP_START)) {
-        return;
-    }
+    if (replay_mode != REPLAY_MODE_PLAY) {
+        if (!all_cpu_threads_idle()) {
+            return;
+        }
 
-    if (!all_cpu_threads_idle()) {
-        return;
-    }
+        if (qtest_enabled()) {
+            /* When testing, qtest commands advance icount.  */
+            return;
+        }
 
-    if (qtest_enabled()) {
-        /* When testing, qtest commands advance icount.  */
-        return;
+        replay_checkpoint(CHECKPOINT_CLOCK_WARP_START);
+    } else {
+        /* warp clock deterministically in record/replay mode */
+        if (!replay_checkpoint(CHECKPOINT_CLOCK_WARP_START)) {
+            /* vCPU is sleeping and warp can't be started.
+               It is probably a race condition: notification sent
+               to vCPU was processed in advance and vCPU went to sleep.
+               Therefore we have to wake it up for doing someting. */
+            if (replay_has_checkpoint()) {
+                qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
+            }
+            return;
+        }
     }
 
     /* We want to use the earliest deadline from ALL vm_clocks */
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index a3113c1..8118b00 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -136,6 +136,9 @@ void replay_shutdown_request(ShutdownCause cause);
     Returns 0 in PLAY mode if checkpoint was not found.
     Returns 1 in all other cases. */
 bool replay_checkpoint(ReplayCheckpoint checkpoint);
+/*! Used to determine that checkpoint is pending.
+    Does not proceed to the next event in the log. */
+bool replay_has_checkpoint(void);
 
 /* Asynchronous events queue */
 
diff --git a/replay/replay.c b/replay/replay.c
index dcce902..6e82764 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -240,6 +240,18 @@ out:
     return res;
 }
 
+bool replay_has_checkpoint(void)
+{
+    bool res = false;
+    if (replay_mode == REPLAY_MODE_PLAY) {
+        g_assert(replay_mutex_locked());
+        replay_account_executed_instructions();
+        res = EVENT_CHECKPOINT <= replay_state.data_kind
+              && replay_state.data_kind <= EVENT_CHECKPOINT_LAST;
+    }
+    return res;
+}
+
 static void replay_enable(const char *fname, int mode)
 {
     const char *fmode = NULL;

[Qemu-devel] [PATCH v5 22/24] replay: replay BH for IDE trim operation

[Pavel Dovgalyuk]

This patch makes IDE trim BH deterministic, because it affects
the device state. Therefore its invocation should be replayed
instead of running at the random moment.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
---
 hw/ide/core.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 2c62efc..04e22e7 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -35,6 +35,7 @@
 #include "sysemu/block-backend.h"
 #include "qapi/error.h"
 #include "qemu/cutils.h"
+#include "sysemu/replay.h"
 
 #include "hw/ide/internal.h"
 #include "trace.h"
@@ -479,7 +480,7 @@ static void ide_issue_trim_cb(void *opaque, int ret)
 done:
     iocb->aiocb = NULL;
     if (iocb->bh) {
-        qemu_bh_schedule(iocb->bh);
+        replay_bh_schedule_event(iocb->bh);
     }
 }
 

Re: [Qemu-devel] [PATCH v5 22/24] replay: replay BH for IDE trim operation

[Paolo Bonzini]

On 25/07/2018 14:16, Pavel Dovgalyuk wrote:
> This patch makes IDE trim BH deterministic, because it affects
> the device state. Therefore its invocation should be replayed
> instead of running at the random moment.
> 
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> ---
>  hw/ide/core.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 2c62efc..04e22e7 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -35,6 +35,7 @@
>  #include "sysemu/block-backend.h"
>  #include "qapi/error.h"
>  #include "qemu/cutils.h"
> +#include "sysemu/replay.h"
>  
>  #include "hw/ide/internal.h"
>  #include "trace.h"
> @@ -479,7 +480,7 @@ static void ide_issue_trim_cb(void *opaque, int ret)
>  done:
>      iocb->aiocb = NULL;
>      if (iocb->bh) {
> -        qemu_bh_schedule(iocb->bh);
> +        replay_bh_schedule_event(iocb->bh);
>      }
>  }
>  
> 
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

[Qemu-devel] [PATCH v5 23/24] 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>
---
 block/block-backend.c    |    3 ++-
 include/sysemu/replay.h  |    3 +++
 replay/replay-events.c   |   16 ++++++++++++++++
 replay/replay-internal.h |    1 +
 replay/replay.c          |    2 +-
 stubs/replay.c           |    6 ++++++
 6 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index f2f75a9..232d114 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -17,6 +17,7 @@
 #include "block/throttle-groups.h"
 #include "sysemu/blockdev.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/replay.h"
 #include "qapi/error.h"
 #include "qapi/qapi-events-block.h"
 #include "qemu/id.h"
@@ -1370,7 +1371,7 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int bytes,
 
     acb->has_returned = true;
     if (acb->rwco.ret != NOT_DONE) {
-        aio_bh_schedule_oneshot(blk_get_aio_context(blk),
+        replay_bh_schedule_oneshot_event(blk_get_aio_context(blk),
                                 blk_aio_complete_bh, acb);
     }
 
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 8118b00..945bc74 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -152,6 +152,9 @@ bool replay_events_enabled(void);
 void replay_flush_events(void);
 /*! Adds bottom half event to the queue */
 void replay_bh_schedule_event(QEMUBH *bh);
+/*! Adds oneshot bottom half event to the queue */
+void replay_bh_schedule_oneshot_event(AioContext *ctx,
+    QEMUBHFunc *cb, void *opaque);
 /*! Adds input event to the queue */
 void replay_input_event(QemuConsole *src, InputEvent *evt);
 /*! Adds input sync event to the queue */
diff --git a/replay/replay-events.c b/replay/replay-events.c
index 0964a82..0ac8a5c 100644
--- a/replay/replay-events.c
+++ b/replay/replay-events.c
@@ -37,6 +37,9 @@ static void replay_run_event(Event *event)
     case REPLAY_ASYNC_EVENT_BH:
         aio_bh_call(event->opaque);
         break;
+    case REPLAY_ASYNC_EVENT_BH_ONESHOT:
+        ((QEMUBHFunc*)event->opaque)(event->opaque2);
+        break;
     case REPLAY_ASYNC_EVENT_INPUT:
         qemu_input_event_send_impl(NULL, (InputEvent *)event->opaque);
         qapi_free_InputEvent((InputEvent *)event->opaque);
@@ -132,6 +135,17 @@ void replay_bh_schedule_event(QEMUBH *bh)
     }
 }
 
+void replay_bh_schedule_oneshot_event(AioContext *ctx,
+    QEMUBHFunc *cb,void *opaque)
+{
+    if (events_enabled) {
+        uint64_t id = replay_get_current_step();
+        replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id);
+    } else {
+        aio_bh_schedule_oneshot(ctx, cb, opaque);
+    }
+}
+
 void replay_add_input_event(struct InputEvent *event)
 {
     replay_add_event(REPLAY_ASYNC_EVENT_INPUT, event, NULL, 0);
@@ -162,6 +176,7 @@ static void replay_save_event(Event *event, int checkpoint)
         /* save event-specific data */
         switch (event->event_kind) {
         case REPLAY_ASYNC_EVENT_BH:
+        case REPLAY_ASYNC_EVENT_BH_ONESHOT:
             replay_put_qword(event->id);
             break;
         case REPLAY_ASYNC_EVENT_INPUT:
@@ -216,6 +231,7 @@ static Event *replay_read_event(int checkpoint)
     /* Events that has not to be in the queue */
     switch (replay_state.read_event_kind) {
     case REPLAY_ASYNC_EVENT_BH:
+    case REPLAY_ASYNC_EVENT_BH_ONESHOT:
         if (replay_state.read_event_id == -1) {
             replay_state.read_event_id = replay_get_qword();
         }
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index 08ef2ec..0c0ed16 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -51,6 +51,7 @@ enum ReplayEvents {
 
 enum ReplayAsyncEventKind {
     REPLAY_ASYNC_EVENT_BH,
+    REPLAY_ASYNC_EVENT_BH_ONESHOT,
     REPLAY_ASYNC_EVENT_INPUT,
     REPLAY_ASYNC_EVENT_INPUT_SYNC,
     REPLAY_ASYNC_EVENT_CHAR_READ,
diff --git a/replay/replay.c b/replay/replay.c
index 6e82764..061b1e2 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -22,7 +22,7 @@
 
 /* Current version of the replay mechanism.
    Increase it when file format changes. */
-#define REPLAY_VERSION              0xe02007
+#define REPLAY_VERSION              0xe02008
 /* Size of replay log header */
 #define HEADER_SIZE                 (sizeof(uint32_t) + sizeof(uint64_t))
 
diff --git a/stubs/replay.c b/stubs/replay.c
index 781974e..cbdac80 100644
--- a/stubs/replay.c
+++ b/stubs/replay.c
@@ -90,3 +90,9 @@ bool replay_reverse_continue(void)
 {
     return false;
 }
+
+void replay_bh_schedule_oneshot_event(AioContext *ctx,
+    QEMUBHFunc *cb,void *opaque)
+{
+    aio_bh_schedule_oneshot(ctx, cb, opaque);
+}

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

[Alex Bennée]

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

> Replay is capable of recording normal BH events, but sometimes
> there are single use callbacks scheduled with aio_bh_schedule_oneshot
> function. This patch enables recording and replaying such callbacks.
> Block layer uses these events for calling the completion function.
> Replaying these calls makes the execution deterministic.
>
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

I'm not sure what about this commit causes the compile breakage I'm
seeing:

  LINK    aarch64-linux-user/qemu-aarch64
../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount'
exec.o:(.bss+0x58): first defined here
collect2: error: ld returned 1 exit status
Makefile:199: recipe for target 'qemu-aarch64' failed
make[1]: *** [qemu-aarch64] Error 1
Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed
make: *** [subdir-aarch64-linux-user] Error 2

It only occurs on a make clean && make -j on that commit though. It's
hidden if you do incremental builds.



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


--
Alex Bennée

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

[Alex Bennée]

Alex Bennée <alex.bennee@linaro.org> writes:

> Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> writes:
>
>> Replay is capable of recording normal BH events, but sometimes
>> there are single use callbacks scheduled with aio_bh_schedule_oneshot
>> function. This patch enables recording and replaying such callbacks.
>> Block layer uses these events for calling the completion function.
>> Replaying these calls makes the execution deterministic.
>>
>> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>
> I'm not sure what about this commit causes the compile breakage I'm
> seeing:
>
>   LINK    aarch64-linux-user/qemu-aarch64
> ../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount'
> exec.o:(.bss+0x58): first defined here
> collect2: error: ld returned 1 exit status
> Makefile:199: recipe for target 'qemu-aarch64' failed
> make[1]: *** [qemu-aarch64] Error 1
> Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed
> make: *** [subdir-aarch64-linux-user] Error 2
>
> It only occurs on a make clean && make -j on that commit though. It's
> hidden if you do incremental builds.

And it seems to be the same failure across all the cross builds:

  https://app.shippable.com/github/stsquad/qemu/runs/538/summary/console

--
Alex Bennée

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

[Pavel Dovgalyuk]

> From: Alex Bennée [mailto:alex.bennee@linaro.org]
> > 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>
> 
> I'm not sure what about this commit causes the compile breakage I'm
> seeing:
> 
>   LINK    aarch64-linux-user/qemu-aarch64
> ../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount'
> exec.o:(.bss+0x58): first defined here
> collect2: error: ld returned 1 exit status
> Makefile:199: recipe for target 'qemu-aarch64' failed
> make[1]: *** [qemu-aarch64] Error 1
> Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed
> make: *** [subdir-aarch64-linux-user] Error 2
> 
> It only occurs on a make clean && make -j on that commit though. It's
> hidden if you do incremental builds.

make distclean works for me in such cases.

Pavel Dovgalyuk

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

[Alex Bennée]

Pavel Dovgalyuk <dovgaluk@ispras.ru> writes:

>> From: Alex Bennée [mailto:alex.bennee@linaro.org]
>> > 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>
>>
>> I'm not sure what about this commit causes the compile breakage I'm
>> seeing:
>>
>>   LINK    aarch64-linux-user/qemu-aarch64
>> ../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount'
>> exec.o:(.bss+0x58): first defined here
>> collect2: error: ld returned 1 exit status
>> Makefile:199: recipe for target 'qemu-aarch64' failed
>> make[1]: *** [qemu-aarch64] Error 1
>> Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed
>> make: *** [subdir-aarch64-linux-user] Error 2
>>
>> It only occurs on a make clean && make -j on that commit though. It's
>> hidden if you do incremental builds.
>
> make distclean works for me in such cases.

Hmmm

make distclean
./configure
make

And I get:

LINK    aarch64-linux-user/qemu-aarch64
../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount'
exec.o:(.bss+0x58): first defined here
collect2: error: ld returned 1 exit status
Makefile:199: recipe for target 'qemu-aarch64' failed
make[1]: *** [qemu-aarch64] Error 1
Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed
make: *** [subdir-aarch64-linux-user] Error 2

The CI builds all look pretty broken too:

  https://travis-ci.org/stsquad/qemu/builds/408537385
  https://app.shippable.com/github/stsquad/qemu/runs/538/summary/console


>
> Pavel Dovgalyuk


--
Alex Bennée

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

[Pavel Dovgalyuk]

> -----Original Message-----
> From: Alex Bennée [mailto:alex.bennee@linaro.org]
> Sent: Friday, July 27, 2018 7:45 PM
> To: Pavel Dovgalyuk
> Cc: 'Pavel Dovgalyuk'; qemu-devel@nongnu.org; kwolf@redhat.com; peter.maydell@linaro.org;
> war2jordan@live.com; crosthwaite.peter@gmail.com; boost.lists@gmail.com; quintela@redhat.com;
> ciro.santilli@gmail.com; jasowang@redhat.com; mst@redhat.com; zuban32s@gmail.com;
> armbru@redhat.com; maria.klimushenkova@ispras.ru; kraxel@redhat.com;
> thomas.dullien@googlemail.com; pbonzini@redhat.com; mreitz@redhat.com; dgilbert@redhat.com;
> rth@twiddle.net
> Subject: Re: [PATCH v5 23/24] replay: add BH oneshot event for block layer
> 
> 
> Pavel Dovgalyuk <dovgaluk@ispras.ru> writes:
> 
> >> From: Alex Bennée [mailto:alex.bennee@linaro.org]
> >> > 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>
> >>
> >> I'm not sure what about this commit causes the compile breakage I'm
> >> seeing:
> >>
> >>   LINK    aarch64-linux-user/qemu-aarch64
> >> ../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount'
> >> exec.o:(.bss+0x58): first defined here
> >> collect2: error: ld returned 1 exit status
> >> Makefile:199: recipe for target 'qemu-aarch64' failed
> >> make[1]: *** [qemu-aarch64] Error 1
> >> Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed
> >> make: *** [subdir-aarch64-linux-user] Error 2
> >>
> >> It only occurs on a make clean && make -j on that commit though. It's
> >> hidden if you do incremental builds.
> >
> > make distclean works for me in such cases.
> 
> Hmmm
> 
> make distclean
> ./configure
> make
> 
> And I get:
> 
> LINK    aarch64-linux-user/qemu-aarch64
> ../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount'
> exec.o:(.bss+0x58): first defined here
> collect2: error: ld returned 1 exit status
> Makefile:199: recipe for target 'qemu-aarch64' failed
> make[1]: *** [qemu-aarch64] Error 1
> Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed
> make: *** [subdir-aarch64-linux-user] Error 2

That's strange. As I understand, linux-user emulators does not include block layer.
They also don't include record/replay.
Why then block- and record-related stub affects the build?

Pavel Dovgalyuk

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

[Alex Bennée]

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

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

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

--
Alex Bennée

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

[Pavel Dovgalyuk]

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

I thinks stubs are meant to be functions that are used by qemu-img and other tools.
As I replaced aio_bh_schedule_oneshot with replay_bh_schedule_oneshot_event,
the tools should not notice this. Therefore the stub performs this call.

Pavel Dovgalyuk

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

[Paolo Bonzini]

On 26/07/2018 19:17, Alex Bennée wrote:
>> +
>> +void replay_bh_schedule_oneshot_event(AioContext *ctx,
>> +    QEMUBHFunc *cb,void *opaque)
>> +{
>> +    aio_bh_schedule_oneshot(ctx, cb, opaque);
>> +}
> It seems wrong to have something in stubs that actively does stuff.
> Isn't this meant to be a bunch of NOPs?

No, not necessarily, for example

int64_t cpu_get_clock(void)
{
    return get_clock_realtime();
}

or even

void error_vprintf(const char *fmt, va_list ap)
{
    if (g_test_initialized() && !g_test_subprocess() &&
        getenv("QTEST_SILENT_ERRORS")) {
        char *msg = g_strdup_vprintf(fmt, ap);
        g_test_message("%s", msg);
        g_free(msg);
    } else {
        vfprintf(stderr, fmt, ap);
    }
}

Paolo

[Qemu-devel] [PATCH v5 24/24] slirp: fix ipv6 timers

[Pavel Dovgalyuk]

ICMP implementation for IPv6 uses timers based on virtual clock.
This is incorrect because this service is not related to the guest state.
This patch changes using virtual clock to the realtime.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
---
 slirp/ip6_icmp.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/slirp/ip6_icmp.c b/slirp/ip6_icmp.c
index ee333d0..e25818e 100644
--- a/slirp/ip6_icmp.c
+++ b/slirp/ip6_icmp.c
@@ -17,7 +17,7 @@ static void ra_timer_handler(void *opaque)
 {
     Slirp *slirp = opaque;
     timer_mod(slirp->ra_timer,
-              qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + NDP_Interval);
+              qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + NDP_Interval);
     ndp_send_ra(slirp);
 }
 
@@ -27,9 +27,9 @@ void icmp6_init(Slirp *slirp)
         return;
     }
 
-    slirp->ra_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, ra_timer_handler, slirp);
+    slirp->ra_timer = timer_new_ms(QEMU_CLOCK_REALTIME, ra_timer_handler, slirp);
     timer_mod(slirp->ra_timer,
-              qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + NDP_Interval);
+              qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + NDP_Interval);
 }
 
 void icmp6_cleanup(Slirp *slirp)

Re: [Qemu-devel] [PATCH v5 24/24] slirp: fix ipv6 timers

[Samuel Thibault]

Pavel Dovgalyuk, le mer. 25 juil. 2018 15:17:06 +0300, a ecrit:
> ICMP implementation for IPv6 uses timers based on virtual clock.
> This is incorrect because this service is not related to the guest state.

? Why not?  The RAs are seen by the guest.  As documented:

 * @QEMU_CLOCK_REALTIME: Real time clock
 *
 * The real time clock should be used only for stuff which does not
 * change the virtual machine state, as it runs even if the virtual
 * machine is stopped.

There is no reason to "send RAs" while the machine is stopped.

Samuel

Re: [Qemu-devel] [PATCH v5 24/24] slirp: fix ipv6 timers

[Pavel Dovgalyuk]

> From: Samuel Thibault [mailto:samuel.thibault@gnu.org]
> Pavel Dovgalyuk, le mer. 25 juil. 2018 15:17:06 +0300, a ecrit:
> > ICMP implementation for IPv6 uses timers based on virtual clock.
> > This is incorrect because this service is not related to the guest state.
> 
> ? Why not?  The RAs are seen by the guest.  

Because virtual clock should be used by the virtual devices.
slirp module is not the virtual device. Therefore processed packets
become visible to the guest after passing to the virtual network card.
Before that it can create any timers that should not change the state of the guest.

> As documented:
> 
>  * @QEMU_CLOCK_REALTIME: Real time clock
>  *
>  * The real time clock should be used only for stuff which does not
>  * change the virtual machine state, as it runs even if the virtual
>  * machine is stopped.
> 
> There is no reason to "send RAs" while the machine is stopped.

I see.
Then we'll need one more clock. Which works like realtime+virtual:
intended to be used for the internal QEMU purposes, but stops when
VM is stopped.

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v5 24/24] slirp: fix ipv6 timers

[Samuel Thibault]

Pavel Dovgalyuk, le jeu. 26 juil. 2018 10:08:29 +0300, a ecrit:
> > As documented:
> > 
> >  * @QEMU_CLOCK_REALTIME: Real time clock
> >  *
> >  * The real time clock should be used only for stuff which does not
> >  * change the virtual machine state, as it runs even if the virtual
> >  * machine is stopped.
> > 
> > There is no reason to "send RAs" while the machine is stopped.
> 
> I see.
> Then we'll need one more clock. Which works like realtime+virtual:
> intended to be used for the internal QEMU purposes, but stops when
> VM is stopped.

Just to be sure: what is meant by "is stopped"? Is it a pause (thus time
does not advance within the guest), or is it just sleeping because it
has nothing to do?

Samuel

Re: [Qemu-devel] [PATCH v5 24/24] slirp: fix ipv6 timers

[Pavel Dovgalyuk]

> From: Samuel Thibault [mailto:samuel.thibault@gnu.org]
> Pavel Dovgalyuk, le jeu. 26 juil. 2018 10:08:29 +0300, a ecrit:
> > > As documented:
> > >
> > >  * @QEMU_CLOCK_REALTIME: Real time clock
> > >  *
> > >  * The real time clock should be used only for stuff which does not
> > >  * change the virtual machine state, as it runs even if the virtual
> > >  * machine is stopped.
> > >
> > > There is no reason to "send RAs" while the machine is stopped.
> >
> > I see.
> > Then we'll need one more clock. Which works like realtime+virtual:
> > intended to be used for the internal QEMU purposes, but stops when
> > VM is stopped.
> 
> Just to be sure: what is meant by "is stopped"? Is it a pause (thus time
> does not advance within the guest), or is it just sleeping because it
> has nothing to do?

Paused with HMP/QMP command.
As virtual clock runs only if VM is not paused.

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v5 24/24] slirp: fix ipv6 timers

[Samuel Thibault]

Pavel Dovgalyuk, le jeu. 26 juil. 2018 10:37:03 +0300, a ecrit:
> > From: Samuel Thibault [mailto:samuel.thibault@gnu.org]
> > Pavel Dovgalyuk, le jeu. 26 juil. 2018 10:08:29 +0300, a ecrit:
> > > > As documented:
> > > >
> > > >  * @QEMU_CLOCK_REALTIME: Real time clock
> > > >  *
> > > >  * The real time clock should be used only for stuff which does not
> > > >  * change the virtual machine state, as it runs even if the virtual
> > > >  * machine is stopped.
> > > >
> > > > There is no reason to "send RAs" while the machine is stopped.
> > >
> > > I see.
> > > Then we'll need one more clock. Which works like realtime+virtual:
> > > intended to be used for the internal QEMU purposes, but stops when
> > > VM is stopped.
> > 
> > Just to be sure: what is meant by "is stopped"? Is it a pause (thus time
> > does not advance within the guest), or is it just sleeping because it
> > has nothing to do?
> 
> Paused with HMP/QMP command.
> As virtual clock runs only if VM is not paused.

Then all other uses of qemu_clock in slirp are bogus and need to be
fixed like ip6_icmp: they are using QEMU_CLOCK_REALTIME, but they want
it not to progress while the guest time is not advancing. Otherwise on
guest resume after a long pause basically all TCP/UDP/ARP timings will
have expired.

Samuel

Re: [Qemu-devel] [PATCH v5 24/24] slirp: fix ipv6 timers

[Samuel Thibault]

Pavel Dovgalyuk, le jeu. 26 juil. 2018 10:08:29 +0300, a ecrit:
> virtual clock should be used by the virtual devices.
> slirp module is not the virtual device. Therefore processed packets
> become visible to the guest after passing to the virtual network card.
> Before that it can create any timers that should not change the state of the guest.

I'm not sure I understand that part correctly. slirp is not a "device"
strictly speaking, but it has a whole foot in the virtual world. All
TCP/UDP/ARP/RA timings are related to the guest timing, so

> > > this service is not related to the guest state.

seems incorrect. At the moment the ip6_icmp timer's current value is not
saved in the guest state, but in principle it should, so that the guest
does see the RAs at a regular rate. In practice we don't care because
the timing is randomized anyway.

> intended to be used for the internal QEMU purposes, but stops when VM
> is stopped.

I again don't understand this. The ip6_icmp timing is not for internal
QEMU purpose, its whole point is how often RAs are sent to the guest.

slirp's guest part is not a device as directly seen by guest I/O, but
it's a router device as seen through guest packets. Think of it like a
USB device, which is seen by the guest through USB packets.

Samuel

Re: [Qemu-devel] [PATCH v5 24/24] slirp: fix ipv6 timers

[Pavel Dovgalyuk]

> From: Samuel Thibault [mailto:samuel.thibault@gnu.org]
> Pavel Dovgalyuk, le jeu. 26 juil. 2018 10:08:29 +0300, a ecrit:
> > virtual clock should be used by the virtual devices.
> > slirp module is not the virtual device. Therefore processed packets
> > become visible to the guest after passing to the virtual network card.
> > Before that it can create any timers that should not change the state of the guest.
> 
> I'm not sure I understand that part correctly. slirp is not a "device"
> strictly speaking, but it has a whole foot in the virtual world. All
> TCP/UDP/ARP/RA timings are related to the guest timing, so

I don't know all details of slirp, so let me ask:
if the virtual timer runs very slowly (when it configured this way with icount option),
should the timings relate this speed? Or the timers are related to the network devices
(e.g., servers in the outer world)?

> > > > this service is not related to the guest state.
> 
> seems incorrect. At the moment the ip6_icmp timer's current value is not
> saved in the guest state, but in principle it should, so that the guest
> does see the RAs at a regular rate. In practice we don't care because
> the timing is randomized anyway.

Isn't this just a side effect?
I mean that slirp may be replaced by, say, tap, and the guest should not notice
the difference.

> > intended to be used for the internal QEMU purposes, but stops when VM
> > is stopped.
> 
> I again don't understand this. The ip6_icmp timing is not for internal
> QEMU purpose, its whole point is how often RAs are sent to the guest.
> 
> slirp's guest part is not a device as directly seen by guest I/O, but
> it's a router device as seen through guest packets. Think of it like a
> USB device, which is seen by the guest through USB packets.

Record/replay implementation creates a line between the guest state and
the outer world. Everything crossing this line is saved in the log replayed.
In case of network, this line is implemented with the network filter.
It takes packets from slirp(or anything) and passes(or not) them to the guest nic.
When replaying, the saved packets are injected into the filter directly.
Slirp is the part of the outer world, so it shouldn't affect the guest state directly.

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v5 24/24] slirp: fix ipv6 timers

[Samuel Thibault]

Pavel Dovgalyuk, le jeu. 26 juil. 2018 11:37:57 +0300, a ecrit:
> > From: Samuel Thibault [mailto:samuel.thibault@gnu.org]
> > Pavel Dovgalyuk, le jeu. 26 juil. 2018 10:08:29 +0300, a ecrit:
> > > virtual clock should be used by the virtual devices.
> > > slirp module is not the virtual device. Therefore processed packets
> > > become visible to the guest after passing to the virtual network card.
> > > Before that it can create any timers that should not change the state of the guest.
> > 
> > I'm not sure I understand that part correctly. slirp is not a "device"
> > strictly speaking, but it has a whole foot in the virtual world. All
> > TCP/UDP/ARP/RA timings are related to the guest timing, so
> 
> I don't know all details of slirp, so let me ask:
> if the virtual timer runs very slowly (when it configured this way with icount option),
> should the timings relate this speed?

Yes. Otherwise the guest will not be fast enough to answer promptly
according to slirp's TCP delays.

> Or the timers are related to the network devices (e.g., servers in the
> outer world)?

No.

> > > > > this service is not related to the guest state.
> > 
> > seems incorrect. At the moment the ip6_icmp timer's current value is not
> > saved in the guest state, but in principle it should, so that the guest
> > does see the RAs at a regular rate. In practice we don't care because
> > the timing is randomized anyway.
> 
> Isn't this just a side effect?
> I mean that slirp may be replaced by, say, tap, and the guest should not notice
> the difference.

Well, if a guest is connected through a tap, the virtual time should
really run as fast as the realtime, and it should not be paused.
Otherwise TCP connections will break since the guest won't be able to
reply fast enough, without even knowing about the issue. Slirp can
compensate this thanks to a buffer between what happens in the real
world and what happens in the virtual world. Real world timings are
handled by the OS socket implementation, and virtual world timings are
handled with the qemu timer.

> > > intended to be used for the internal QEMU purposes, but stops when VM
> > > is stopped.
> > 
> > I again don't understand this. The ip6_icmp timing is not for internal
> > QEMU purpose, its whole point is how often RAs are sent to the guest.
> > 
> > slirp's guest part is not a device as directly seen by guest I/O, but
> > it's a router device as seen through guest packets. Think of it like a
> > USB device, which is seen by the guest through USB packets.
> 
> Record/replay implementation creates a line between the guest state and
> the outer world. Everything crossing this line is saved in the log replayed.
> In case of network, this line is implemented with the network filter.
> It takes packets from slirp(or anything) and passes(or not) them to the guest nic.
> When replaying, the saved packets are injected into the filter directly.

> Slirp is the part of the outer world,

In normal uses it is not. It is a virtual world (its DHCP server, tftp
server, TCP connexions, etc.) that lives along the guest.

Now, I understand that for record/replay it's simpler to put the line
after slirp.

Ideally slirp's state should ideally be split it two: the part connected
to the real world (data from/to the sockets), and the part connected to
the virtual world (TCP buffering with the guest). So that when pausing,
going back, going forward etc. the slirp buffers act accordingly, TCP
knowing exactly what is supposed to be sent or not (otherwise, TCP
would for instance be really astonished if the guest happens to insist
requesting old data that it has already ACKed).

But that's tricky, and I understand it's simpler to just put the line
after slirp, and let the replay of frames provide the guest (which for
instance has been reset to an older time) with the missing data, and TCP
will nicely cope with duplicate ACKs and spurious re-emissions from the
guest.

That being said, there will be problems with TCP connections if you
pause the guest for a long time: slirp's TCP will timeout and reset the
connexion. Yes, that happens with tap devices anyway, but slirp acting
as a buffer seems more useful to me.

Samuel

Re: [Qemu-devel] [PATCH v5 24/24] slirp: fix ipv6 timers

[Pavel Dovgalyuk]

> From: Samuel Thibault [mailto:samuel.thibault@gnu.org]
> Pavel Dovgalyuk, le jeu. 26 juil. 2018 11:37:57 +0300, a ecrit:
> > > From: Samuel Thibault [mailto:samuel.thibault@gnu.org]
> > > Pavel Dovgalyuk, le jeu. 26 juil. 2018 10:08:29 +0300, a ecrit:
> > > > virtual clock should be used by the virtual devices.
> > > > slirp module is not the virtual device. Therefore processed packets
> > > > become visible to the guest after passing to the virtual network card.
> > > > Before that it can create any timers that should not change the state of the guest.
> > >
> > > I'm not sure I understand that part correctly. slirp is not a "device"
> > > strictly speaking, but it has a whole foot in the virtual world. All
> > > TCP/UDP/ARP/RA timings are related to the guest timing, so
> >
> > I don't know all details of slirp, so let me ask:
> > if the virtual timer runs very slowly (when it configured this way with icount option),
> > should the timings relate this speed?
> 
> Yes. Otherwise the guest will not be fast enough to answer promptly
> according to slirp's TCP delays.
> 
> > Or the timers are related to the network devices (e.g., servers in the
> > outer world)?
> 
> No.
> 
> > > > > > this service is not related to the guest state.
> > >
> > > seems incorrect. At the moment the ip6_icmp timer's current value is not
> > > saved in the guest state, but in principle it should, so that the guest
> > > does see the RAs at a regular rate. In practice we don't care because
> > > the timing is randomized anyway.
> >
> > Isn't this just a side effect?
> > I mean that slirp may be replaced by, say, tap, and the guest should not notice
> > the difference.
> 
> Well, if a guest is connected through a tap, the virtual time should
> really run as fast as the realtime, and it should not be paused.
> Otherwise TCP connections will break since the guest won't be able to
> reply fast enough, without even knowing about the issue. Slirp can
> compensate this thanks to a buffer between what happens in the real
> world and what happens in the virtual world. Real world timings are
> handled by the OS socket implementation, and virtual world timings are
> handled with the qemu timer.

Then maybe the solution is the new clock with the frequency of the virtual
clock, but which does not affect the replayed core?
This clock should stop when VM is paused.
It also could be saved in vmstate. As it does not affect the replay,
saving and restoring its state won't break anything.

> > > > intended to be used for the internal QEMU purposes, but stops when VM
> > > > is stopped.
> > >
> > > I again don't understand this. The ip6_icmp timing is not for internal
> > > QEMU purpose, its whole point is how often RAs are sent to the guest.
> > >
> > > slirp's guest part is not a device as directly seen by guest I/O, but
> > > it's a router device as seen through guest packets. Think of it like a
> > > USB device, which is seen by the guest through USB packets.
> >
> > Record/replay implementation creates a line between the guest state and
> > the outer world. Everything crossing this line is saved in the log replayed.
> > In case of network, this line is implemented with the network filter.
> > It takes packets from slirp(or anything) and passes(or not) them to the guest nic.
> > When replaying, the saved packets are injected into the filter directly.
> 
> > Slirp is the part of the outer world,
> 
> In normal uses it is not. It is a virtual world (its DHCP server, tftp
> server, TCP connexions, etc.) that lives along the guest.
> 
> Now, I understand that for record/replay it's simpler to put the line
> after slirp.
> 
> Ideally slirp's state should ideally be split it two: the part connected
> to the real world (data from/to the sockets), and the part connected to
> the virtual world (TCP buffering with the guest). So that when pausing,
> going back, going forward etc. the slirp buffers act accordingly, TCP
> knowing exactly what is supposed to be sent or not (otherwise, TCP
> would for instance be really astonished if the guest happens to insist
> requesting old data that it has already ACKed).
> 
> But that's tricky, and I understand it's simpler to just put the line
> after slirp, and let the replay of frames provide the guest (which for
> instance has been reset to an older time) with the missing data, and TCP
> will nicely cope with duplicate ACKs and spurious re-emissions from the
> guest.
> 
> That being said, there will be problems with TCP connections if you
> pause the guest for a long time: slirp's TCP will timeout and reset the
> connexion. Yes, that happens with tap devices anyway, but slirp acting
> as a buffer seems more useful to me.

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v5 24/24] slirp: fix ipv6 timers

[Samuel Thibault]

Pavel Dovgalyuk, le mar. 31 juil. 2018 09:58:26 +0300, a ecrit:
> > From: Samuel Thibault [mailto:samuel.thibault@gnu.org]
> > Pavel Dovgalyuk, le jeu. 26 juil. 2018 11:37:57 +0300, a ecrit:
> > > Or the timers are related to the network devices (e.g., servers in the
> > > outer world)?
> > 
> > No.
> > 
> > > > > > > this service is not related to the guest state.
> > > >
> > > > seems incorrect. At the moment the ip6_icmp timer's current value is not
> > > > saved in the guest state, but in principle it should, so that the guest
> > > > does see the RAs at a regular rate. In practice we don't care because
> > > > the timing is randomized anyway.
> > >
> > > Isn't this just a side effect?
> > > I mean that slirp may be replaced by, say, tap, and the guest should not notice
> > > the difference.
> > 
> > Well, if a guest is connected through a tap, the virtual time should
> > really run as fast as the realtime, and it should not be paused.
> > Otherwise TCP connections will break since the guest won't be able to
> > reply fast enough, without even knowing about the issue. Slirp can
> > compensate this thanks to a buffer between what happens in the real
> > world and what happens in the virtual world. Real world timings are
> > handled by the OS socket implementation, and virtual world timings are
> > handled with the qemu timer.
> 
> Then maybe the solution is the new clock with the frequency of the virtual
> clock, but which does not affect the replayed core?
> This clock should stop when VM is paused.
> It also could be saved in vmstate. As it does not affect the replay,
> saving and restoring its state won't break anything.

I guess so.

Samuel

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

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180725121311.12867.21729.stgit@pasha-VirtualBox
Subject: [Qemu-devel] [PATCH v5 00/24] Fixing record/replay and adding reverse debugging

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
b933359fcb slirp: fix ipv6 timers
08f5dcb6f9 replay: add BH oneshot event for block layer
1d795aa6f9 replay: replay BH for IDE trim operation
22a0a68431 replay: wake up vCPU when replaying
8a458d20ac ps2: prevent changing irq state on save and load
c076be246d replay: allow loading any snapshots before recording
10622de164 replay: describe reverse debugging in docs/replay.txt
a9f8b005f0 gdbstub: add reverse continue support in replay mode
c1b2f4385e gdbstub: add reverse step support in replay mode
9bd4685704 replay: flush rr queue before loading the vmstate
cffb0d860d translator: fix breakpoint processing
05e4bd25b6 replay: refine replay-time module
ed42025371 timer: remove replay clock probe in deadline calculation
36f5132987 replay: flush events when exiting
ee8c956c92 replay: implement replay-seek command to proceed to the desired step
9aade36782 replay: introduce breakpoint at the specified step
36fb64416b replay: introduce info hmp/qmp command
84f414e5bf migration: introduce icount field for snapshots
67e35a07df qcow2: introduce icount field for snapshots
74d11dda54 replay: finish record/replay before closing the disks
eea5cde9f5 replay: don't drain/flush bdrv queue while RR is working
a96d8d5e35 replay: update docs for record/replay with block devices
e673d40571 replay: disable default snapshot for record/replay
1ea43d85a7 block: implement bdrv_snapshot_goto for blkreplay

=== OUTPUT BEGIN ===
Checking PATCH 1/24: block: implement bdrv_snapshot_goto for blkreplay...
Checking PATCH 2/24: replay: disable default snapshot for record/replay...
Checking PATCH 3/24: replay: update docs for record/replay with block devices...
Checking PATCH 4/24: replay: don't drain/flush bdrv queue while RR is working...
Checking PATCH 5/24: replay: finish record/replay before closing the disks...
Checking PATCH 6/24: qcow2: introduce icount field for snapshots...
Checking PATCH 7/24: migration: introduce icount field for snapshots...
Checking PATCH 8/24: replay: introduce info hmp/qmp command...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#114: 
new file mode 100644

total: 0 errors, 1 warnings, 132 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 9/24: replay: introduce breakpoint at the specified step...
Checking PATCH 10/24: replay: implement replay-seek command to proceed to the desired step...
Checking PATCH 11/24: replay: flush events when exiting...
Checking PATCH 12/24: timer: remove replay clock probe in deadline calculation...
WARNING: line over 80 characters
#37: FILE: util/qemu-timer.c:584:
+                                            timerlist_deadline_ns(tlg->tl[type]));

total: 0 errors, 1 warnings, 19 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 13/24: replay: refine replay-time module...
Checking PATCH 14/24: translator: fix breakpoint processing...
Checking PATCH 15/24: replay: flush rr queue before loading the vmstate...
Checking PATCH 16/24: gdbstub: add reverse step support in replay mode...
Checking PATCH 17/24: gdbstub: add reverse continue support in replay mode...
Checking PATCH 18/24: replay: describe reverse debugging in docs/replay.txt...
Checking PATCH 19/24: replay: allow loading any snapshots before recording...
Checking PATCH 20/24: ps2: prevent changing irq state on save and load...
Checking PATCH 21/24: replay: wake up vCPU when replaying...
Checking PATCH 22/24: replay: replay BH for IDE trim operation...
Checking PATCH 23/24: replay: add BH oneshot event for block layer...
ERROR: "(foo*)" should be "(foo *)"
#59: FILE: replay/replay-events.c:41:
+        ((QEMUBHFunc*)event->opaque)(event->opaque2);

ERROR: space required after that ',' (ctx:VxV)
#69: FILE: replay/replay-events.c:139:
+    QEMUBHFunc *cb,void *opaque)
                   ^

ERROR: space required after that ',' (ctx:VxV)
#133: FILE: stubs/replay.c:95:
+    QEMUBHFunc *cb,void *opaque)
                   ^

total: 3 errors, 0 warnings, 88 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 24/24: slirp: fix ipv6 timers...
WARNING: line over 80 characters
#31: FILE: slirp/ip6_icmp.c:30:
+    slirp->ra_timer = timer_new_ms(QEMU_CLOCK_REALTIME, ra_timer_handler, slirp);

total: 0 errors, 1 warnings, 19 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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

[Ciro Santilli]

OK, finally got some time to try it out, I'm using
c42634d8e3428cfa60672c3ba89cabefc720cde9 from rr-180725.

Replay works well as far as I can tell, so I moved to the reverse debugging:

/home/ciro/bak/git/linux-kernel-module-cheat/out/x86_
64/buildroot/build/host-qemu-custom.rr/x86_64-softmmu/qemu-system-x86_64 \
-M pc \
-append 'root=/dev/sda nopat console_msg_format=syslog nokaslr norandmaps
printk.devkmsg=on printk.time=y console=ttyS0 -  lkmc_eval_base64="
L3JhbmRfY2hlY2sub3V0Oy9wb3dlcm9mZi5vdXQ7"' \
-kernel '/home/ciro/bak/git/linux-kernel-module-cheat/out/x86_
64/buildroot/build/linux-custom.default/arch/x86/boot/bzImage' \
-m '256M' \
-monitor 'telnet::45454,server,nowait' \
-nographic \
-serial mon:stdio \
-smp '1' \
\
-drive 'file=/home/ciro/bak/git/linux-kernel-module-cheat/out/
x86_64/buildroot/images/rootfs.ext2.qcow2,format=qcow2,if=none,id=img-direct'
\
-drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \
-device ide-hd,drive=img-blkreplay \
\
-object filter-replay,id=replay,netdev=net0 \
-device rtl8139,netdev=net0 \
-netdev 'user,hostfwd=tcp::45455-:45455,hostfwd=tcp::45456-:22,id=net0' \
\
-icount 'shift=7,rr=record,rrfile=/home/ciro/bak/git/linux-
kernel-module-cheat/out/x86_64/qemu/0/rrfile' \

and replay with:

-icount 'shift=7,rr=replay,rrfile=/home/ciro/bak/git/linux-
kernel-module-cheat/out/x86_64/qemu/0/rrfile' \
-gdb 'tcp::45457' \
-S \

Then, I do

/home/ciro/bak/git/linux-kernel-module-cheat/out/x86_
64/buildroot/host/usr/bin/x86_64-linux-gdb \
 -q \
-ex 'add-auto-load-safe-path /home/ciro/bak/git/linux-kernel-module-cheat' \
-ex 'file vmlinux' \
-ex 'target remote localhost:45457' \
-ex 'break start_kernel' \
  -ex continue \
-ex 'lx-symbols ../kernel_module-1.0/' \

Then in GDB:

n
n
n
n
reverse-continue

expecting it to return me to start_kernel, but instead it left me in the
same place that I'm at.

I also tried to manually checkpoint from qemu monitor at the very start,
but it didn't change anything.

bzImage at: https://github.com/cirosantilli/linux-kernel-
module-cheat/releases/download/sha-19f4d00f9b13aa67369e32ec7cd351
8950c6f30e/bzImage and docs at: https://github.com/
cirosantilli/linux-kernel-module-cheat/tree/19f4d00f9b13aa67369e32ec7cd351
8950c6f30e#qemu-record-and-replay


On Wed, Jul 25, 2018 at 1:13 PM, Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
wrote:

> 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:
>  - fixes of record/replay caused by the QEMU core changes
>  - 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-180725
>
> 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 (24):
>       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: introduce info hmp/qmp command
>       replay: introduce breakpoint at the specified step
>       replay: implement replay-seek command to proceed to the desired step
>       replay: flush events when exiting
>       timer: remove replay clock probe in deadline calculation
>       replay: refine replay-time module
>       translator: fix breakpoint processing
>       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: allow loading any snapshots before recording
>       ps2: prevent changing irq state on save and load
>       replay: wake up vCPU when replaying
>       replay: replay BH for IDE trim operation
>       replay: add BH oneshot event for block layer
>       slirp: fix ipv6 timers
>
>
>  accel/tcg/translator.c    |    9 +
>  block/blkreplay.c         |    8 +
>  block/block-backend.c     |    3
>  block/io.c                |   22 +++
>  block/qapi.c              |   17 ++-
>  block/qcow2-snapshot.c    |    9 +
>  block/qcow2.h             |    2
>  blockdev.c                |   10 ++
>  cpus.c                    |   50 +++++---
>  docs/interop/qcow2.txt    |    4 +
>  docs/replay.txt           |   45 +++++++
>  exec.c                    |    6 +
>  gdbstub.c                 |   50 +++++++-
>  hmp-commands-info.hx      |   14 ++
>  hmp-commands.hx           |   30 +++++
>  hmp.h                     |    3
>  hw/ide/core.c             |    3
>  hw/input/ps2.c            |    8 +
>  include/block/snapshot.h  |    1
>  include/sysemu/replay.h   |   24 ++++
>  migration/savevm.c        |   15 +-
>  qapi/block-core.json      |    5 +
>  qapi/block.json           |    3
>  qapi/misc.json            |   68 +++++++++++
>  replay/Makefile.objs      |    3
>  replay/replay-debugging.c |  287 ++++++++++++++++++++++++++++++
> +++++++++++++++
>  replay/replay-events.c    |   30 +++--
>  replay/replay-internal.h  |   11 +-
>  replay/replay-snapshot.c  |   17 ++-
>  replay/replay-time.c      |   27 ++--
>  replay/replay.c           |   36 +++++-
>  slirp/ip6_icmp.c          |    6 -
>  stubs/replay.c            |   16 +++
>  util/qemu-timer.c         |   11 --
>  vl.c                      |   18 ++-
>  35 files changed, 772 insertions(+), 99 deletions(-)
>  create mode 100644 replay/replay-debugging.c
>
> --
> Pavel Dovgalyuk
>

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

[dovgaluk]

Hi, Ciro!

I found several issues in your command lines.

Ciro Santilli писал 2018-08-08 02:13:
> OK, finally got some time to try it out, I'm using
> c42634d8e3428cfa60672c3ba89cabefc720cde9 from rr-180725.
> 
> Replay works well as far as I can tell, so I moved to the reverse
> debugging:
> 
> /home/ciro/bak/git/linux-kernel-module-cheat/out/x86_64/buildroot/build/host-qemu-custom.rr/x86_64-softmmu/qemu-system-x86_64
> \
> -M pc \
> -append 'root=/dev/sda nopat console_msg_format=syslog nokaslr
> norandmaps printk.devkmsg=on printk.time=y console=ttyS0 -
> lkmc_eval_base64="L3JhbmRfY2hlY2sub3V0Oy9wb3dlcm9mZi5vdXQ7"' \
> -kernel
> '/home/ciro/bak/git/linux-kernel-module-cheat/out/x86_64/buildroot/build/linux-custom.default/arch/x86/boot/bzImage'
> \
> -m '256M' \
> -monitor 'telnet::45454,server,nowait' \
> -nographic \
> -serial mon:stdio \
> -smp '1' \
> \
> -drive
> 'file=/home/ciro/bak/git/linux-kernel-module-cheat/out/x86_64/buildroot/images/rootfs.ext2.qcow2,format=qcow2,if=none,id=img-direct'

You'll probably need an overlay, it you want this file to be unchanged 
by VM.

Can you also provide this file for testing? I found only bzImage.

> \
> -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \
> -device ide-hd,drive=img-blkreplay \
> \
> -object filter-replay,id=replay,netdev=net0 \
> -device rtl8139,netdev=net0 \
> -netdev
> 'user,hostfwd=tcp::45455-:45455,hostfwd=tcp::45456-:22,id=net0' \
> \
> -icount
> 'shift=7,rr=record,rrfile=/home/ciro/bak/git/linux-kernel-module-cheat/out/x86_64/qemu/0/rrfile'

You need to specify rrsnapshot=<name> option for creating the initial VM 
snapshot.
This option creates snapshot at record and loads it at replay. GDB can 
also use this snapshot for reverse execution.

> \
> 
> and replay with:
> 
> -icount
> 'shift=7,rr=replay,rrfile=/home/ciro/bak/git/linux-kernel-module-cheat/out/x86_64/qemu/0/rrfile'
> \
> -gdb 'tcp::45457' \
> -S \
> 
> Then, I do
> 
> /home/ciro/bak/git/linux-kernel-module-cheat/out/x86_64/buildroot/host/usr/bin/x86_64-linux-gdb
> \
>  -q \
> -ex 'add-auto-load-safe-path
> /home/ciro/bak/git/linux-kernel-module-cheat' \
> -ex 'file vmlinux' \
> -ex 'target remote localhost:45457' \
> -ex 'break start_kernel' \
>   -ex continue \
> -ex 'lx-symbols ../kernel_module-1.0/' \
> 
> Then in GDB:
> 
> n
> n
> n
> n
> reverse-continue
> 
> expecting it to return me to start_kernel, but instead it left me in
> the same place that I'm at.

Right, because there were no checkpoints. The initial one must be 
created at recording phase.



Pavel Dovgalyuk