[for-5.0 PATCH 03/11] migration: introduce icount field for snapshots

Pavel Dovgalyuk posted 11 patches 5 years, 10 months ago
[for-5.0 PATCH 03/11] migration: introduce icount field for snapshots
Posted by Pavel Dovgalyuk 5 years, 10 months ago
From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>

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 proceeding
the recorded execution to the specific moment of the time.
E.g., 'reverse step' action (introduced in one of the following patches)
needs to load the nearest snapshot which is prior to the current moment
of time.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
Acked-by: Markus Armbruster <armbru@redhat.com>

--

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

diff --git a/block/qapi.c b/block/qapi.c
index 9a5d0c9b27..110ac253ab 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -219,6 +219,8 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
         info->date_nsec     = sn_tab[i].date_nsec;
         info->vm_clock_sec  = sn_tab[i].vm_clock_nsec / 1000000000;
         info->vm_clock_nsec = sn_tab[i].vm_clock_nsec % 1000000000;
+        info->icount        = sn_tab[i].icount;
+        info->has_icount    = sn_tab[i].icount != -1ULL;
 
         info_list = g_new0(SnapshotInfoList, 1);
         info_list->value = info;
@@ -651,14 +653,15 @@ BlockStatsList *qmp_query_blockstats(bool has_query_nodes,
 void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
 {
     char date_buf[128], clock_buf[128];
+    char icount_buf[128] = {0};
     struct tm tm;
     time_t ti;
     int64_t secs;
     char *sizing = NULL;
 
     if (!sn) {
-        qemu_printf("%-10s%-20s%7s%20s%15s",
-                    "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK");
+        qemu_printf("%-10s%-18s%7s%20s%13s%11s",
+                    "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK", "ICOUNT");
     } else {
         ti = sn->date_sec;
         localtime_r(&ti, &tm);
@@ -672,11 +675,16 @@ void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
                  (int)(secs % 60),
                  (int)((sn->vm_clock_nsec / 1000000) % 1000));
         sizing = size_to_str(sn->vm_state_size);
-        qemu_printf("%-10s%-20s%7s%20s%15s",
+        if (sn->icount != -1ULL) {
+            snprintf(icount_buf, sizeof(icount_buf),
+                "%"PRId64, sn->icount);
+        }
+        qemu_printf("%-10s%-18s%7s%20s%13s%11s",
                     sn->id_str, sn->name,
                     sizing,
                     date_buf,
-                    clock_buf);
+                    clock_buf,
+                    icount_buf);
     }
     g_free(sizing);
 }
@@ -838,6 +846,8 @@ void bdrv_image_info_dump(ImageInfo *info)
                 .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 b04b3e1634..2c003514ef 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -662,6 +662,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;
     sn->extra_data_size = sizeof(QCowSnapshotExtraData);
 
     /* Allocate the L1 table of the snapshot and copy the current one there. */
@@ -995,6 +996,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 8e029e9c01..6383a64ddd 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -59,6 +59,7 @@
 #include "sysemu/arch_init.h"
 #include "sysemu/qtest.h"
 #include "sysemu/runstate.h"
+#include "sysemu/replay.h"
 #include "qemu/cutils.h"
 #include "qemu/help_option.h"
 #include "qemu/main-loop.h"
@@ -1242,6 +1243,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;
 
@@ -1449,6 +1454,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_icount();
+    } 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 2bfcd57578..b0fe42993d 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 a71b930b91..ae84bf6ab0 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2681,6 +2681,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_icount();
+    } 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 0cf68fea14..db3e435c74 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -26,13 +26,18 @@
 #
 # @vm-clock-nsec: fractional part in nano seconds to be used with vm-clock-sec
 #
+# @icount: Current instruction count. Appears when execution record/replay
+#          is enabled. Used for "time-traveling" to match the moment
+#          in the recorded execution with the snapshots. (since 5.0)
+#
 # Since: 1.3
 #
 ##
 { 'struct': 'SnapshotInfo',
   'data': { 'id': 'str', 'name': 'str', 'vm-state-size': 'int',
             'date-sec': 'int', 'date-nsec': 'int',
-            'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } }
+            'vm-clock-sec': 'int', 'vm-clock-nsec': 'int',
+            '*icount': 'int' } }
 
 ##
 # @ImageInfoSpecificQCow2EncryptionBase:
diff --git a/qapi/block.json b/qapi/block.json
index 145c268bb6..f389bb6f1a 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -176,7 +176,8 @@
 #                    "date-sec": 1000012,
 #                    "date-nsec": 10,
 #                    "vm-clock-sec": 100,
-#                    "vm-clock-nsec": 20
+#                    "vm-clock-nsec": 20,
+#                    "icount": 220414
 #      }
 #    }
 #


Re: [for-5.0 PATCH 03/11] migration: introduce icount field for snapshots
Posted by Kevin Wolf 5 years, 10 months ago
Am 23.12.2019 um 10:47 hat Pavel Dovgalyuk geschrieben:
> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> 
> 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 proceeding
> the recorded execution to the specific moment of the time.
> E.g., 'reverse step' action (introduced in one of the following patches)
> needs to load the nearest snapshot which is prior to the current moment
> of time.
> 
> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> Acked-by: Markus Armbruster <armbru@redhat.com>

Acked-by: Kevin Wolf <kwolf@redhat.com>


Re: [for-5.0 PATCH 03/11] migration: introduce icount field for snapshots
Posted by Alex Bennée 5 years, 10 months ago
Kevin Wolf <kwolf@redhat.com> writes:

> Am 23.12.2019 um 10:47 hat Pavel Dovgalyuk geschrieben:
>> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>> 
>> 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 proceeding
>> the recorded execution to the specific moment of the time.
>> E.g., 'reverse step' action (introduced in one of the following patches)
>> needs to load the nearest snapshot which is prior to the current moment
>> of time.
>> 
>> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>> Acked-by: Markus Armbruster <armbru@redhat.com>
>
> Acked-by: Kevin Wolf <kwolf@redhat.com>

Apologies my mailer ignored my replay-all: 

This commit breaks when of the iotests for me:

 git bisect run /bin/sh -c "cd builds/all && make -j4 \
     && cd tests/qemu-iotests && ./check -qcow2 267"
 

Gives:

  make[1]: Entering directory '/home/alex.bennee/lsrc/qemu.git/slirp'
  make[1]: Nothing to be done for 'all'.
  make[1]: Leaving directory '/home/alex.bennee/lsrc/qemu.git/slirp'
  QEMU          -- "/home/alex.bennee/lsrc/qemu.git/builds/all/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64" -nodefaults -display none -accel qtest
  QEMU_IMG      -- "/home/alex.bennee/lsrc/qemu.git/builds/all/tests/qemu-iotests/../../qemu-img"
  QEMU_IO       -- "/home/alex.bennee/lsrc/qemu.git/builds/all/tests/qemu-iotests/../../qemu-io"  --cache writeback -f qcow2
  QEMU_NBD      -- "/home/alex.bennee/lsrc/qemu.git/builds/all/tests/qemu-iotests/../../qemu-nbd"
  IMGFMT        -- qcow2 (compat=1.1)
  IMGPROTO      -- file
  PLATFORM      -- Linux/x86_64 hackbox2 4.15.0-66-generic
  TEST_DIR      -- /home/alex.bennee/lsrc/qemu.git/builds/all/tests/qemu-iotests/scratch
  SOCK_DIR      -- /tmp/tmp.NV0n5HqCUs
  SOCKET_SCM_HELPER -- /home/alex.bennee/lsrc/qemu.git/builds/all/tests/qemu-iotests/socket_scm_helper

  267      fail       [12:17:36] [12:17:38]      (last: 1s)    output mismatch (see 267.out.bad)
  --- /home/alex.bennee/lsrc/qemu.git/tests/qemu-iotests/267.out  2019-10-31 10:46:30.559805129 +0000
  +++ /home/alex.bennee/lsrc/qemu.git/builds/all/tests/qemu-iotests/267.out.bad   2020-01-13 12:17:38.096181947 +0000
  @@ -33,7 +33,7 @@
   (qemu) savevm snap0
   (qemu) info snapshots
   List of snapshots present on all disks:
  -ID        TAG                 VM SIZE                DATE       VM CLOCK
  +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
   --        snap0                  SIZE yyyy-mm-dd hh:mm:ss   00:00:00.000
   (qemu) loadvm snap0
   (qemu) quit
  @@ -44,7 +44,7 @@
   (qemu) savevm snap0
   (qemu) info snapshots
   List of snapshots present on all disks:
  -ID        TAG                 VM SIZE                DATE       VM CLOCK
  +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT

But I've also seen:

  ERROR:/home/.../qemu.git/replay/replay-events.c:80:replay_flush_events:
     assertion failed: (replay_mutex_locked())

-- 
Alex Bennée

RE: [for-5.0 PATCH 03/11] migration: introduce icount field for snapshots
Posted by Pavel Dovgalyuk 5 years, 10 months ago
> From: Alex Bennée [mailto:alex.bennee@linaro.org]
> Kevin Wolf <kwolf@redhat.com> writes:
> 
> > Am 23.12.2019 um 10:47 hat Pavel Dovgalyuk geschrieben:
> >> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> >>
> >> 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 proceeding
> >> the recorded execution to the specific moment of the time.
> >> E.g., 'reverse step' action (introduced in one of the following patches)
> >> needs to load the nearest snapshot which is prior to the current moment
> >> of time.
> >>
> >> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> >> Acked-by: Markus Armbruster <armbru@redhat.com>
> >
> > Acked-by: Kevin Wolf <kwolf@redhat.com>
> 
> Apologies my mailer ignored my replay-all:
> 
> This commit breaks when of the iotests for me:
> 
>  git bisect run /bin/sh -c "cd builds/all && make -j4 \
>      && cd tests/qemu-iotests && ./check -qcow2 267"
> 
> 
>    List of snapshots present on all disks:
>   -ID        TAG                 VM SIZE                DATE       VM CLOCK
>   +ID        TAG               VM SIZE                DATE     VM CLOCK     ICOUNT
> 
> But I've also seen:
> 
>   ERROR:/home/.../qemu.git/replay/replay-events.c:80:replay_flush_events:
>      assertion failed: (replay_mutex_locked())

Thank you, I've updated the code.
I also added a patch for fixing the test output.

Pavel Dovgalyuk