[Qemu-devel] [PATCH v2 00/10] replay additions

[Qemu-devel] [PATCH v2 00/10] replay additions

[Pavel Dovgalyuk]

This set of patches includes several fixes for replay and
adds network record/replay for network devices. It also makes possible
saving/restoring vmstate in replay mode.

Record and replay for network interactions is performed with the network filter.
Each backend must have its own instance of the replay filter as follows:
 -netdev user,id=net1 -device rtl8139,netdev=net1
 -object filter-replay,id=replay,netdev=net1

This patches add overlay option for blkreplay block driver. Using persistent
overlay file allows saving and reloading VM snapshots in replay mode.
Replay mechanism automatically creates one snapshot named 'replay_init' to
allow rewinding execution while replaying.
Overlay file may be specified as follows:
 -drive file=disk.qcow,if=none,id=img-direct 
 -drive driver=blkreplay,if=none,image=img-direct,overlay=overlay.qcow2,id=img-blkreplay 
 -device ide-hd,drive=img-blkreplay

This set of patches includes fixes and additions for icount and
record/replay implementation:
 - Enabling VM start/stop in replay mode
 - Adding network interaction record/replay
 - Adding overlay option for blkreplay filter
 - Fixes of the vmstate for several virtual devices

---

Pavel Dovgalyuk (10):
      record/replay: add network support
      block: set snapshot option for block devices in blkreplay module
      block: don't make snapshots for filters
      replay: save/load initial state
      replay: move internal data to the structure
      replay: vmstate for replay module
      replay: allow replay stopping and restarting
      kvmvapic: fix state change handler
      pcspk: adding vmstate for save/restore
      integratorcp: adding vmstate for save/restore


 block/blkreplay.c        |  132 +++++++++++++++++++++++++++++++++++++++++++---
 block/snapshot.c         |    3 +
 cpus.c                   |    1 
 docs/replay.txt          |   22 ++++++++
 hw/arm/integratorcp.c    |   62 ++++++++++++++++++++++
 hw/audio/pcspk.c         |   17 +++++-
 hw/i386/kvmvapic.c       |    1 
 include/sysemu/replay.h  |   26 +++++++++
 net/Makefile.objs        |    1 
 net/filter-replay.c      |   90 +++++++++++++++++++++++++++++++
 replay/Makefile.objs     |    2 +
 replay/replay-events.c   |   21 +++++++
 replay/replay-internal.c |   19 +++----
 replay/replay-internal.h |   26 +++++++--
 replay/replay-net.c      |  110 ++++++++++++++++++++++++++++++++++++++
 replay/replay-snapshot.c |   72 +++++++++++++++++++++++++
 replay/replay-time.c     |    2 -
 replay/replay.c          |   17 +++---
 stubs/replay.c           |    5 ++
 vl.c                     |    9 ++-
 20 files changed, 599 insertions(+), 39 deletions(-)
 create mode 100644 net/filter-replay.c
 create mode 100644 replay/replay-net.c
 create mode 100644 replay/replay-snapshot.c

-- 
Pavel Dovgalyuk

[Qemu-devel] [PATCH v2 01/10] record/replay: add network support

[Pavel Dovgalyuk]

This patch adds support of recording and replaying network packets in
irount rr mode.

Record and replay for network interactions is performed with the network filter.
Each backend must have its own instance of the replay filter as follows:
 -netdev user,id=net1 -device rtl8139,netdev=net1
 -object filter-replay,id=replay,netdev=net1

Replay network filter is used to record and replay network packets. While
recording the virtual machine this filter puts all packets coming from
the outer world into the log. In replay mode packets from the log are
injected into the network device. All interactions with network backend
in replay mode are disabled.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 docs/replay.txt          |   14 ++++++
 include/sysemu/replay.h  |   12 +++++
 net/Makefile.objs        |    1 
 net/filter-replay.c      |   90 ++++++++++++++++++++++++++++++++++++++
 replay/Makefile.objs     |    1 
 replay/replay-events.c   |   11 +++++
 replay/replay-internal.h |   10 ++++
 replay/replay-net.c      |  110 ++++++++++++++++++++++++++++++++++++++++++++++
 replay/replay.c          |    2 -
 vl.c                     |    3 +
 10 files changed, 252 insertions(+), 2 deletions(-)
 create mode 100644 net/filter-replay.c
 create mode 100644 replay/replay-net.c

diff --git a/docs/replay.txt b/docs/replay.txt
index 779c6c0..347b2ff 100644
--- a/docs/replay.txt
+++ b/docs/replay.txt
@@ -195,3 +195,17 @@ Queue is flushed at checkpoints and information about processed requests
 is recorded to the log. In replay phase the queue is matched with
 events read from the log. Therefore block devices requests are processed
 deterministically.
+
+Network devices
+---------------
+
+Record and replay for network interactions is performed with the network filter.
+Each backend must have its own instance of the replay filter as follows:
+ -netdev user,id=net1 -device rtl8139,netdev=net1
+ -object filter-replay,id=replay,netdev=net1
+
+Replay network filter is used to record and replay network packets. While
+recording the virtual machine this filter puts all packets coming from
+the outer world into the log. In replay mode packets from the log are
+injected into the network device. All interactions with network backend
+in replay mode are disabled.
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 0a88393..a408633 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -39,6 +39,8 @@ enum ReplayCheckpoint {
 };
 typedef enum ReplayCheckpoint ReplayCheckpoint;
 
+typedef struct ReplayNetState ReplayNetState;
+
 extern ReplayMode replay_mode;
 
 /* Replay process control functions */
@@ -133,4 +135,14 @@ void replay_char_read_all_save_error(int res);
 /*! Writes character read_all execution result into the replay log. */
 void replay_char_read_all_save_buf(uint8_t *buf, int offset);
 
+/* Network */
+
+/*! Registers replay network filter attached to some backend. */
+ReplayNetState *replay_register_net(NetFilterState *nfs);
+/*! Unregisters replay network filter. */
+void replay_unregister_net(ReplayNetState *rns);
+/*! Called to write network packet to the replay log. */
+void replay_net_packet_event(ReplayNetState *rns, unsigned flags,
+                             const struct iovec *iov, int iovcnt);
+
 #endif
diff --git a/net/Makefile.objs b/net/Makefile.objs
index b7c22fd..f787ba4 100644
--- a/net/Makefile.objs
+++ b/net/Makefile.objs
@@ -16,3 +16,4 @@ common-obj-$(CONFIG_NETMAP) += netmap.o
 common-obj-y += filter.o
 common-obj-y += filter-buffer.o
 common-obj-y += filter-mirror.o
+common-obj-y += filter-replay.o
diff --git a/net/filter-replay.c b/net/filter-replay.c
new file mode 100644
index 0000000..7d93dc9
--- /dev/null
+++ b/net/filter-replay.c
@@ -0,0 +1,90 @@
+/*
+ * filter-replay.c
+ *
+ * Copyright (c) 2010-2016 Institute for System Programming
+ *                         of the Russian Academy of Sciences.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "clients.h"
+#include "qapi/error.h"
+#include "qemu-common.h"
+#include "qemu/error-report.h"
+#include "qemu/iov.h"
+#include "qemu/log.h"
+#include "qemu/timer.h"
+#include "qapi/visitor.h"
+#include "net/filter.h"
+#include "sysemu/replay.h"
+
+#define TYPE_FILTER_REPLAY "filter-replay"
+
+#define FILTER_REPLAY(obj) \
+    OBJECT_CHECK(NetFilterReplayState, (obj), TYPE_FILTER_REPLAY)
+
+struct NetFilterReplayState {
+    NetFilterState nfs;
+    ReplayNetState *rns;
+};
+typedef struct NetFilterReplayState NetFilterReplayState;
+
+static ssize_t filter_replay_receive_iov(NetFilterState *nf, NetClientState *sndr,
+                                         unsigned flags, const struct iovec *iov,
+                                         int iovcnt, NetPacketSent *sent_cb)
+{
+    NetFilterReplayState *nfrs = FILTER_REPLAY(nf);
+    switch (replay_mode) {
+    case REPLAY_MODE_RECORD:
+        if (nf->netdev == sndr) {
+            replay_net_packet_event(nfrs->rns, flags, iov, iovcnt);
+            return iov_size(iov, iovcnt);
+        }
+        return 0;
+    case REPLAY_MODE_PLAY:
+        /* Drop all packets in replay mode.
+           Packets from the log will be injected by the replay module. */
+        return iov_size(iov, iovcnt);
+    default:
+        /* Pass all the packets. */
+        return 0;
+    }
+}
+
+static void filter_replay_instance_init(Object *obj)
+{
+    NetFilterReplayState *nfrs = FILTER_REPLAY(obj);
+    nfrs->rns = replay_register_net(&nfrs->nfs);
+}
+
+static void filter_replay_instance_finalize(Object *obj)
+{
+    NetFilterReplayState *nfrs = FILTER_REPLAY(obj);
+    replay_unregister_net(nfrs->rns);
+}
+
+static void filter_replay_class_init(ObjectClass *oc, void *data)
+{
+    NetFilterClass *nfc = NETFILTER_CLASS(oc);
+
+    nfc->receive_iov = filter_replay_receive_iov;
+}
+
+static const TypeInfo filter_replay_info = {
+    .name = TYPE_FILTER_REPLAY,
+    .parent = TYPE_NETFILTER,
+    .class_init = filter_replay_class_init,
+    .instance_init = filter_replay_instance_init,
+    .instance_finalize = filter_replay_instance_finalize,
+    .instance_size = sizeof(NetFilterReplayState),
+};
+
+static void filter_replay_register_types(void)
+{
+    type_register_static(&filter_replay_info);
+}
+
+type_init(filter_replay_register_types);
diff --git a/replay/Makefile.objs b/replay/Makefile.objs
index fcb3f74..f55a6b5 100644
--- a/replay/Makefile.objs
+++ b/replay/Makefile.objs
@@ -4,3 +4,4 @@ common-obj-y += replay-events.o
 common-obj-y += replay-time.o
 common-obj-y += replay-input.o
 common-obj-y += replay-char.o
+common-obj-y += replay-net.o
diff --git a/replay/replay-events.c b/replay/replay-events.c
index 3807245..9ce9e51 100644
--- a/replay/replay-events.c
+++ b/replay/replay-events.c
@@ -54,6 +54,9 @@ static void replay_run_event(Event *event)
     case REPLAY_ASYNC_EVENT_BLOCK:
         aio_bh_call(event->opaque);
         break;
+    case REPLAY_ASYNC_EVENT_NET:
+        replay_event_net_run(event->opaque);
+        break;
     default:
         error_report("Replay: invalid async event ID (%d) in the queue",
                     event->event_kind);
@@ -189,6 +192,9 @@ static void replay_save_event(Event *event, int checkpoint)
         case REPLAY_ASYNC_EVENT_BLOCK:
             replay_put_qword(event->id);
             break;
+        case REPLAY_ASYNC_EVENT_NET:
+            replay_event_net_save(event->opaque);
+            break;
         default:
             error_report("Unknown ID %" PRId64 " of replay event", event->id);
             exit(1);
@@ -252,6 +258,11 @@ static Event *replay_read_event(int checkpoint)
             read_id = replay_get_qword();
         }
         break;
+    case REPLAY_ASYNC_EVENT_NET:
+        event = g_malloc0(sizeof(Event));
+        event->event_kind = read_event_kind;
+        event->opaque = replay_event_net_load();
+        return event;
     default:
         error_report("Unknown ID %d of replay event", read_event_kind);
         exit(1);
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index efbf14c..d28cfb7 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -50,6 +50,7 @@ enum ReplayAsyncEventKind {
     REPLAY_ASYNC_EVENT_INPUT_SYNC,
     REPLAY_ASYNC_EVENT_CHAR_READ,
     REPLAY_ASYNC_EVENT_BLOCK,
+    REPLAY_ASYNC_EVENT_NET,
     REPLAY_ASYNC_COUNT
 };
 
@@ -155,4 +156,13 @@ void replay_event_char_read_save(void *opaque);
 /*! Reads char event read from the file. */
 void *replay_event_char_read_load(void);
 
+/* Network devices */
+
+/*! Called to run network event. */
+void replay_event_net_run(void *opaque);
+/*! Writes network event to the file. */
+void replay_event_net_save(void *opaque);
+/*! Reads network from the file. */
+void *replay_event_net_load(void);
+
 #endif
diff --git a/replay/replay-net.c b/replay/replay-net.c
new file mode 100644
index 0000000..e3ba0f2
--- /dev/null
+++ b/replay/replay-net.c
@@ -0,0 +1,110 @@
+/*
+ * replay-net.c
+ *
+ * Copyright (c) 2010-2016 Institute for System Programming
+ *                         of the Russian Academy of Sciences.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "sysemu/replay.h"
+#include "replay-internal.h"
+#include "sysemu/sysemu.h"
+#include "net/net.h"
+#include "net/filter.h"
+#include "qemu/iov.h"
+
+struct ReplayNetState {
+    NetFilterState *nfs;
+    int id;
+};
+
+typedef struct NetEvent {
+    uint8_t id;
+    uint32_t flags;
+    uint8_t *data;
+    size_t size;
+} NetEvent;
+
+static NetFilterState **network_filters;
+static int network_filters_count;
+
+ReplayNetState *replay_register_net(NetFilterState *nfs)
+{
+    ReplayNetState *rns = g_new0(ReplayNetState, 1);
+    rns->nfs = nfs;
+    rns->id = network_filters_count++;
+    network_filters = g_realloc(network_filters,
+                                network_filters_count * sizeof(*network_filters));
+    network_filters[network_filters_count - 1] = nfs;
+    return rns;
+}
+
+void replay_unregister_net(ReplayNetState *rns)
+{
+    network_filters[rns->id] = NULL;
+    g_free(rns);
+}
+
+void replay_net_packet_event(ReplayNetState *rns, unsigned flags,
+                             const struct iovec *iov, int iovcnt)
+{
+    NetEvent *event;
+    int i;
+
+    event = g_new(NetEvent, 1);
+    event->flags = flags;
+    event->data = g_malloc(iov_size(iov, iovcnt));
+    event->size = 0;
+    event->id = rns->id;
+
+    for (i = 0; i < iovcnt; i++) {
+        size_t len = iov[i].iov_len;
+
+        memcpy(event->data + event->size, iov[i].iov_base, len);
+        event->size += len;
+    }
+
+    replay_add_event(REPLAY_ASYNC_EVENT_NET, event, NULL, 0);
+}
+
+void replay_event_net_run(void *opaque)
+{
+    NetEvent *event = opaque;
+    struct iovec iov = {
+        .iov_base = (void *)event->data,
+        .iov_len = event->size
+    };
+
+    assert(event->id < network_filters_count);
+
+    qemu_netfilter_pass_to_next(network_filters[event->id]->netdev,
+        event->flags, &iov, 1, network_filters[event->id]);
+
+    g_free(event->data);
+    g_free(event);
+}
+
+void replay_event_net_save(void *opaque)
+{
+    NetEvent *event = opaque;
+
+    replay_put_byte(event->id);
+    replay_put_dword(event->flags);
+    replay_put_array(event->data, event->size);
+}
+
+void *replay_event_net_load(void)
+{
+    NetEvent *event = g_new(NetEvent, 1);
+
+    event->id = replay_get_byte();
+    event->flags = replay_get_dword();
+    replay_get_array_alloc(&event->data, &event->size);
+
+    return event;
+}
diff --git a/replay/replay.c b/replay/replay.c
index 167fd29..e040f6f 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -21,7 +21,7 @@
 
 /* Current version of the replay mechanism.
    Increase it when file format changes. */
-#define REPLAY_VERSION              0xe02004
+#define REPLAY_VERSION              0xe02005
 /* Size of replay log header */
 #define HEADER_SIZE                 (sizeof(uint32_t) + sizeof(uint64_t))
 
diff --git a/vl.c b/vl.c
index ad2664b..cb0fedc 100644
--- a/vl.c
+++ b/vl.c
@@ -2807,7 +2807,8 @@ static bool object_create_initial(const char *type)
     if (g_str_equal(type, "filter-buffer") ||
         g_str_equal(type, "filter-dump") ||
         g_str_equal(type, "filter-mirror") ||
-        g_str_equal(type, "filter-redirector")) {
+        g_str_equal(type, "filter-redirector") ||
+        g_str_equal(type, "filter-replay")) {
         return false;
     }
 

[Qemu-devel] [PATCH v2 02/10] block: set snapshot option for block devices in blkreplay module

[Pavel Dovgalyuk]

This patch adds overlay option for blkreplay filter.
It allows creating persistent overlay file for saving and reloading
VM snapshots in record/replay modes.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 block/blkreplay.c |  119 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 docs/replay.txt   |    8 ++++
 vl.c              |    2 -
 3 files changed, 128 insertions(+), 1 deletion(-)

diff --git a/block/blkreplay.c b/block/blkreplay.c
index 30f9d5f..62ae861 100644
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -14,6 +14,7 @@
 #include "block/block_int.h"
 #include "sysemu/replay.h"
 #include "qapi/error.h"
+#include "qapi/qmp/qstring.h"
 
 typedef struct Request {
     Coroutine *co;
@@ -25,11 +26,82 @@ typedef struct Request {
    block devices should not get overlapping ids. */
 static uint64_t request_id;
 
+static BlockDriverState *blkreplay_append_snapshot(BlockDriverState *bs,
+                                                   int flags,
+                                                   QDict *snapshot_options,
+                                                   Error **errp)
+{
+    int ret;
+    BlockDriverState *bs_snapshot;
+
+    /* Create temporary file, if needed */
+    if ((flags & BDRV_O_TEMPORARY) || replay_mode == REPLAY_MODE_RECORD) {
+        int64_t total_size;
+        QemuOpts *opts = NULL;
+        const char *tmp_filename = qdict_get_str(snapshot_options,
+                                                 "file.filename");
+
+        /* Get the required size from the image */
+        total_size = bdrv_getlength(bs);
+        if (total_size < 0) {
+            error_setg_errno(errp, -total_size, "Could not get image size");
+            goto out;
+        }
+
+        opts = qemu_opts_create(bdrv_qcow2.create_opts, NULL, 0,
+                                &error_abort);
+        qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size, &error_abort);
+        ret = bdrv_create(&bdrv_qcow2, tmp_filename, opts, errp);
+        qemu_opts_del(opts);
+        if (ret < 0) {
+            error_prepend(errp, "Could not create temporary overlay '%s': ",
+                          tmp_filename);
+            goto out;
+        }
+    }
+
+    bs_snapshot = bdrv_open(NULL, NULL, snapshot_options, flags, errp);
+    snapshot_options = NULL;
+    if (!bs_snapshot) {
+        ret = -EINVAL;
+        goto out;
+    }
+
+    /* bdrv_append() consumes a strong reference to bs_snapshot (i.e. it will
+     * call bdrv_unref() on it), so in order to be able to return one, we have
+     * to increase bs_snapshot's refcount here */
+    bdrv_ref(bs_snapshot);
+    bdrv_append(bs_snapshot, bs);
+
+    return bs_snapshot;
+
+out:
+    QDECREF(snapshot_options);
+    return NULL;
+}
+
+static QemuOptsList blkreplay_runtime_opts = {
+    .name = "quorum",
+    .head = QTAILQ_HEAD_INITIALIZER(blkreplay_runtime_opts.head),
+    .desc = {
+        {
+            .name = "overlay",
+            .type = QEMU_OPT_STRING,
+            .help = "Optional overlay file for snapshots",
+        },
+        { /* end of list */ }
+    },
+};
+
 static int blkreplay_open(BlockDriverState *bs, QDict *options, int flags,
                           Error **errp)
 {
     Error *local_err = NULL;
     int ret;
+    QDict *snapshot_options = qdict_new();
+    int snapshot_flags = BDRV_O_RDWR;
+    const char *snapshot;
+    QemuOpts *opts = NULL;
 
     /* Open the image file */
     bs->file = bdrv_open_child(NULL, options, "image",
@@ -40,6 +112,43 @@ static int blkreplay_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
+    opts = qemu_opts_create(&blkreplay_runtime_opts, NULL, 0, &error_abort);
+    qemu_opts_absorb_qdict(opts, options, &local_err);
+    if (local_err) {
+        ret = -EINVAL;
+        goto fail;
+    }
+
+    /* Prepare options QDict for the overlay file */
+    qdict_put(snapshot_options, "file.driver",
+              qstring_from_str("file"));
+    qdict_put(snapshot_options, "driver",
+              qstring_from_str("qcow2"));
+
+    snapshot = qemu_opt_get(opts, "overlay");
+    if (snapshot) {
+        qdict_put(snapshot_options, "file.filename",
+                  qstring_from_str(snapshot));
+    } else {
+        char tmp_filename[PATH_MAX + 1];
+        ret = get_tmp_filename(tmp_filename, PATH_MAX + 1);
+        if (ret < 0) {
+            error_setg_errno(errp, -ret, "Could not get temporary filename");
+            goto fail;
+        }
+        qdict_put(snapshot_options, "file.filename",
+                  qstring_from_str(tmp_filename));
+        snapshot_flags |= BDRV_O_TEMPORARY;
+    }
+
+    /* Add temporary snapshot to preserve the image */
+    if (!blkreplay_append_snapshot(bs->file->bs, snapshot_flags,
+                      snapshot_options, &local_err)) {
+        ret = -EINVAL;
+        error_propagate(errp, local_err);
+        goto fail;
+    }
+
     ret = 0;
 fail:
     if (ret < 0) {
@@ -50,6 +159,7 @@ fail:
 
 static void blkreplay_close(BlockDriverState *bs)
 {
+    bdrv_unref(bs->file->bs);
 }
 
 static int64_t blkreplay_getlength(BlockDriverState *bs)
@@ -135,6 +245,12 @@ static int coroutine_fn blkreplay_co_flush(BlockDriverState *bs)
     return ret;
 }
 
+static bool blkreplay_recurse_is_first_non_filter(BlockDriverState *bs,
+                                                  BlockDriverState *candidate)
+{
+    return bdrv_recurse_is_first_non_filter(bs->file->bs, candidate);
+}
+
 static BlockDriver bdrv_blkreplay = {
     .format_name            = "blkreplay",
     .protocol_name          = "blkreplay",
@@ -150,6 +266,9 @@ static BlockDriver bdrv_blkreplay = {
     .bdrv_co_pwrite_zeroes  = blkreplay_co_pwrite_zeroes,
     .bdrv_co_pdiscard       = blkreplay_co_pdiscard,
     .bdrv_co_flush          = blkreplay_co_flush,
+
+    .is_filter              = true,
+    .bdrv_recurse_is_first_non_filter = blkreplay_recurse_is_first_non_filter,
 };
 
 static void bdrv_blkreplay_init(void)
diff --git a/docs/replay.txt b/docs/replay.txt
index 347b2ff..5be8f25 100644
--- a/docs/replay.txt
+++ b/docs/replay.txt
@@ -196,6 +196,14 @@ is recorded to the log. In replay phase the queue is matched with
 events read from the log. Therefore block devices requests are processed
 deterministically.
 
+blkdriver also supports overlay option, which allows creating persistent
+overlay file for saving and reloading VM snapshots in record/replay modes.
+Replay mechanism automatically creates one snapshot named 'replay_init' to
+allow rewinding execution while replaying.
+Overlay file may be specified as follows:
+ -drive driver=blkreplay,if=none,image=img-direct,
+        overlay=overlay.qcow2,id=img-blkreplay
+
 Network devices
 ---------------
 
diff --git a/vl.c b/vl.c
index cb0fedc..1c68779 100644
--- a/vl.c
+++ b/vl.c
@@ -4409,7 +4409,7 @@ int main(int argc, char **argv, char **envp)
     }
 
     /* open the virtual block devices */
-    if (snapshot || replay_mode != REPLAY_MODE_NONE) {
+    if (snapshot) {
         qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
                           NULL, NULL);
     }

[Qemu-devel] [PATCH v2 03/10] block: don't make snapshots for filters

[Pavel Dovgalyuk]

This patch disables snapshotting for block driver filters.
It is needed, because snapshots should be created
in underlying disk images, not in filters itself.

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

diff --git a/block/snapshot.c b/block/snapshot.c
index bf5c2ca..8998b8b 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -184,6 +184,9 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
     if (!drv) {
         return -ENOMEDIUM;
     }
+    if (drv->is_filter) {
+        return 0;
+    }
     if (drv->bdrv_snapshot_goto) {
         return drv->bdrv_snapshot_goto(bs, snapshot_id);
     }

[Qemu-devel] [PATCH v2 04/10] replay: save/load initial state

[Pavel Dovgalyuk]

This patch implements initial vmstate creation or loading at the start
of record/replay. It is needed for rewinding the execution in the replay mode.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 include/sysemu/replay.h  |    6 ++++++
 replay/Makefile.objs     |    1 +
 replay/replay-snapshot.c |   31 +++++++++++++++++++++++++++++++
 vl.c                     |    2 ++
 4 files changed, 40 insertions(+)
 create mode 100644 replay/replay-snapshot.c

diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index a408633..aa378ce 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -145,4 +145,10 @@ void replay_unregister_net(ReplayNetState *rns);
 void replay_net_packet_event(ReplayNetState *rns, unsigned flags,
                              const struct iovec *iov, int iovcnt);
 
+/* VM state operations */
+
+/*! Called at the start of execution.
+    Loads or saves initial vmstate depending on execution mode. */
+void replay_vmstate_init(void);
+
 #endif
diff --git a/replay/Makefile.objs b/replay/Makefile.objs
index f55a6b5..4600d74 100644
--- a/replay/Makefile.objs
+++ b/replay/Makefile.objs
@@ -5,3 +5,4 @@ common-obj-y += replay-time.o
 common-obj-y += replay-input.o
 common-obj-y += replay-char.o
 common-obj-y += replay-net.o
+common-obj-y += replay-snapshot.o
diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
new file mode 100644
index 0000000..a48c6fc
--- /dev/null
+++ b/replay/replay-snapshot.c
@@ -0,0 +1,31 @@
+/*
+ * replay-snapshot.c
+ *
+ * Copyright (c) 2010-2016 Institute for System Programming
+ *                         of the Russian Academy of Sciences.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu-common.h"
+#include "sysemu/replay.h"
+#include "replay-internal.h"
+#include "sysemu/sysemu.h"
+#include "monitor/monitor.h"
+#include "qapi/qmp/qstring.h"
+
+void replay_vmstate_init(void)
+{
+    if (replay_mode == REPLAY_MODE_RECORD) {
+        QDict *opts = qdict_new();
+        qdict_put(opts, "name", qstring_from_str("replay_init"));
+        hmp_savevm(cur_mon, opts);
+        QDECREF(opts);
+    } else if (replay_mode == REPLAY_MODE_PLAY) {
+        load_vmstate("replay_init");
+    }
+}
diff --git a/vl.c b/vl.c
index 1c68779..6698d88 100644
--- a/vl.c
+++ b/vl.c
@@ -4593,6 +4593,8 @@ int main(int argc, char **argv, char **envp)
         if (load_vmstate(loadvm) < 0) {
             autostart = 0;
         }
+    } else {
+        replay_vmstate_init();
     }
 
     qdev_prop_check_globals();

[Qemu-devel] [PATCH v2 05/10] replay: move internal data to the structure

[Pavel Dovgalyuk]

This patch moves replay static variables into the structure
to allow saving and loading them with savevm/loadvm.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 replay/replay-events.c   |    2 +-
 replay/replay-internal.c |   19 ++++++++-----------
 replay/replay-internal.h |    8 +++++---
 replay/replay-time.c     |    2 +-
 replay/replay.c          |   15 ++++++++-------
 5 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/replay/replay-events.c b/replay/replay-events.c
index 9ce9e51..4ee2f5d 100644
--- a/replay/replay-events.c
+++ b/replay/replay-events.c
@@ -290,7 +290,7 @@ static Event *replay_read_event(int checkpoint)
 /* Called with replay mutex locked */
 void replay_read_events(int checkpoint)
 {
-    while (replay_data_kind == EVENT_ASYNC) {
+    while (replay_state.data_kind == EVENT_ASYNC) {
         Event *event = replay_read_event(checkpoint);
         if (!event) {
             break;
diff --git a/replay/replay-internal.c b/replay/replay-internal.c
index 5835e8d..f4e9caa 100644
--- a/replay/replay-internal.c
+++ b/replay/replay-internal.c
@@ -16,11 +16,8 @@
 #include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
 
-unsigned int replay_data_kind = -1;
-static unsigned int replay_has_unread_data;
-
 /* Mutex to protect reading and writing events to the log.
-   replay_data_kind and replay_has_unread_data are also protected
+   data_kind and has_unread_data are also protected
    by this mutex.
    It also protects replay events queue which stores events to be
    written or read to the log. */
@@ -150,15 +147,15 @@ void replay_check_error(void)
 void replay_fetch_data_kind(void)
 {
     if (replay_file) {
-        if (!replay_has_unread_data) {
-            replay_data_kind = replay_get_byte();
-            if (replay_data_kind == EVENT_INSTRUCTION) {
+        if (!replay_state.has_unread_data) {
+            replay_state.data_kind = replay_get_byte();
+            if (replay_state.data_kind == EVENT_INSTRUCTION) {
                 replay_state.instructions_count = replay_get_dword();
             }
             replay_check_error();
-            replay_has_unread_data = 1;
-            if (replay_data_kind >= EVENT_COUNT) {
-                error_report("Replay: unknown event kind %d", replay_data_kind);
+            replay_state.has_unread_data = 1;
+            if (replay_state.data_kind >= EVENT_COUNT) {
+                error_report("Replay: unknown event kind %d", replay_state.data_kind);
                 exit(1);
             }
         }
@@ -167,7 +164,7 @@ void replay_fetch_data_kind(void)
 
 void replay_finish_event(void)
 {
-    replay_has_unread_data = 0;
+    replay_state.has_unread_data = 0;
     replay_fetch_data_kind();
 }
 
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index d28cfb7..3147d66 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -63,11 +63,13 @@ typedef struct ReplayState {
     uint64_t current_step;
     /*! Number of instructions to be executed before other events happen. */
     int instructions_count;
+    /*! Type of the currently executed event. */
+    unsigned int data_kind;
+    /*! Flag which indicates that event is not processed yet. */
+    unsigned int has_unread_data;
 } ReplayState;
 extern ReplayState replay_state;
 
-extern unsigned int replay_data_kind;
-
 /* File for replay writing */
 extern FILE *replay_file;
 
@@ -99,7 +101,7 @@ void replay_check_error(void);
     the next event from the log. */
 void replay_finish_event(void);
 /*! Reads data type from the file and stores it in the
-    replay_data_kind variable. */
+    data_kind variable. */
 void replay_fetch_data_kind(void);
 
 /*! Saves queued events (like instructions and sound). */
diff --git a/replay/replay-time.c b/replay/replay-time.c
index fffe072..f70382a 100644
--- a/replay/replay-time.c
+++ b/replay/replay-time.c
@@ -31,7 +31,7 @@ int64_t replay_save_clock(ReplayClockKind kind, int64_t clock)
 
 void replay_read_next_clock(ReplayClockKind kind)
 {
-    unsigned int read_kind = replay_data_kind - EVENT_CLOCK;
+    unsigned int read_kind = replay_state.data_kind - EVENT_CLOCK;
 
     assert(read_kind == kind);
 
diff --git a/replay/replay.c b/replay/replay.c
index e040f6f..56b4237 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -38,15 +38,15 @@ bool replay_next_event_is(int event)
 
     /* nothing to skip - not all instructions used */
     if (replay_state.instructions_count != 0) {
-        assert(replay_data_kind == EVENT_INSTRUCTION);
+        assert(replay_state.data_kind == EVENT_INSTRUCTION);
         return event == EVENT_INSTRUCTION;
     }
 
     while (true) {
-        if (event == replay_data_kind) {
+        if (event == replay_state.data_kind) {
             res = true;
         }
-        switch (replay_data_kind) {
+        switch (replay_state.data_kind) {
         case EVENT_SHUTDOWN:
             replay_finish_event();
             qemu_system_shutdown_request();
@@ -85,7 +85,7 @@ void replay_account_executed_instructions(void)
             replay_state.instructions_count -= count;
             replay_state.current_step += count;
             if (replay_state.instructions_count == 0) {
-                assert(replay_data_kind == EVENT_INSTRUCTION);
+                assert(replay_state.data_kind == EVENT_INSTRUCTION);
                 replay_finish_event();
                 /* Wake up iothread. This is required because
                    timers will not expire until clock counters
@@ -188,7 +188,7 @@ bool replay_checkpoint(ReplayCheckpoint checkpoint)
     if (replay_mode == REPLAY_MODE_PLAY) {
         if (replay_next_event_is(EVENT_CHECKPOINT + checkpoint)) {
             replay_finish_event();
-        } else if (replay_data_kind != EVENT_ASYNC) {
+        } else if (replay_state.data_kind != EVENT_ASYNC) {
             res = false;
             goto out;
         }
@@ -196,7 +196,7 @@ bool replay_checkpoint(ReplayCheckpoint checkpoint)
         /* replay_read_events may leave some unread events.
            Return false if not all of the events associated with
            checkpoint were processed */
-        res = replay_data_kind != EVENT_ASYNC;
+        res = replay_state.data_kind != EVENT_ASYNC;
     } else if (replay_mode == REPLAY_MODE_RECORD) {
         replay_put_event(EVENT_CHECKPOINT + checkpoint);
         replay_save_events(checkpoint);
@@ -237,9 +237,10 @@ static void replay_enable(const char *fname, int mode)
     replay_filename = g_strdup(fname);
 
     replay_mode = mode;
-    replay_data_kind = -1;
+    replay_state.data_kind = -1;
     replay_state.instructions_count = 0;
     replay_state.current_step = 0;
+    replay_state.has_unread_data = 0;
 
     /* skip file header for RECORD and check it for PLAY */
     if (replay_mode == REPLAY_MODE_RECORD) {

[Qemu-devel] [PATCH v2 06/10] replay: vmstate for replay module

[Pavel Dovgalyuk]

This patch introduces vmstate for replay data structures.
It allows saving and loading vmstate while replaying.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 include/sysemu/replay.h  |    4 ++++
 replay/replay-internal.h |    2 ++
 replay/replay-snapshot.c |   40 ++++++++++++++++++++++++++++++++++++++++
 vl.c                     |    1 +
 4 files changed, 47 insertions(+)

diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index aa378ce..1123fc2 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -147,6 +147,10 @@ void replay_net_packet_event(ReplayNetState *rns, unsigned flags,
 
 /* VM state operations */
 
+/* Registers replay VMState.
+   Should be called before virtual devices initialization
+   to make cached timers available for post_load functions. */
+void replay_vmstate_register(void);
 /*! Called at the start of execution.
     Loads or saves initial vmstate depending on execution mode. */
 void replay_vmstate_init(void);
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index 3147d66..9bc4a29 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -67,6 +67,8 @@ typedef struct ReplayState {
     unsigned int data_kind;
     /*! Flag which indicates that event is not processed yet. */
     unsigned int has_unread_data;
+    /*! Temporary variable for saving current log offset. */
+    uint64_t file_offset;
 } ReplayState;
 extern ReplayState replay_state;
 
diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
index a48c6fc..4933bdd 100644
--- a/replay/replay-snapshot.c
+++ b/replay/replay-snapshot.c
@@ -17,6 +17,46 @@
 #include "sysemu/sysemu.h"
 #include "monitor/monitor.h"
 #include "qapi/qmp/qstring.h"
+#include "migration/vmstate.h"
+
+static void replay_pre_save(void *opaque)
+{
+    ReplayState *state = opaque;
+    state->file_offset = ftello64(replay_file);
+}
+
+static int replay_post_load(void *opaque, int version_id)
+{
+    ReplayState *state = opaque;
+    fseeko64(replay_file, state->file_offset, SEEK_SET);
+    /* If this was a vmstate, saved in recording mode,
+       we need to initialize replay data fields. */
+    replay_fetch_data_kind();
+
+    return 0;
+}
+
+static const VMStateDescription vmstate_replay = {
+    .name = "replay",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .pre_save = replay_pre_save,
+    .post_load = replay_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_INT64_ARRAY(cached_clock, ReplayState, REPLAY_CLOCK_COUNT),
+        VMSTATE_UINT64(current_step, ReplayState),
+        VMSTATE_INT32(instructions_count, ReplayState),
+        VMSTATE_UINT32(data_kind, ReplayState),
+        VMSTATE_UINT32(has_unread_data, ReplayState),
+        VMSTATE_UINT64(file_offset, ReplayState),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
+void replay_vmstate_register(void)
+{
+    vmstate_register(NULL, 0, &vmstate_replay, &replay_state);
+}
 
 void replay_vmstate_init(void)
 {
diff --git a/vl.c b/vl.c
index 6698d88..f2193cb 100644
--- a/vl.c
+++ b/vl.c
@@ -4361,6 +4361,7 @@ int main(int argc, char **argv, char **envp)
         configure_icount(icount_opts, &error_abort);
         qemu_opts_del(icount_opts);
     }
+    replay_vmstate_register();
 
     if (default_net) {
         QemuOptsList *net = qemu_find_opts("net");

[Qemu-devel] [PATCH v2 07/10] replay: allow replay stopping and restarting

[Pavel Dovgalyuk]

This patch fixes bug with stopping and restarting replay
through monitor.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 block/blkreplay.c        |   15 +++++----------
 cpus.c                   |    1 +
 include/sysemu/replay.h  |    4 ++++
 replay/replay-events.c   |    8 ++++++++
 replay/replay-internal.h |    6 ++++--
 replay/replay-snapshot.c |    1 +
 stubs/replay.c           |    5 +++++
 vl.c                     |    1 +
 8 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/block/blkreplay.c b/block/blkreplay.c
index 62ae861..c2c3615 100644
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -21,11 +21,6 @@ typedef struct Request {
     QEMUBH *bh;
 } Request;
 
-/* Next request id.
-   This counter is global, because requests from different
-   block devices should not get overlapping ids. */
-static uint64_t request_id;
-
 static BlockDriverState *blkreplay_append_snapshot(BlockDriverState *bs,
                                                    int flags,
                                                    QDict *snapshot_options,
@@ -194,7 +189,7 @@ static void block_request_create(uint64_t reqid, BlockDriverState *bs,
 static int coroutine_fn blkreplay_co_preadv(BlockDriverState *bs,
     uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
 {
-    uint64_t reqid = request_id++;
+    uint64_t reqid = blkreplay_next_id();
     int ret = bdrv_co_preadv(bs->file, offset, bytes, qiov, flags);
     block_request_create(reqid, bs, qemu_coroutine_self());
     qemu_coroutine_yield();
@@ -205,7 +200,7 @@ static int coroutine_fn blkreplay_co_preadv(BlockDriverState *bs,
 static int coroutine_fn blkreplay_co_pwritev(BlockDriverState *bs,
     uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
 {
-    uint64_t reqid = request_id++;
+    uint64_t reqid = blkreplay_next_id();
     int ret = bdrv_co_pwritev(bs->file, offset, bytes, qiov, flags);
     block_request_create(reqid, bs, qemu_coroutine_self());
     qemu_coroutine_yield();
@@ -216,7 +211,7 @@ static int coroutine_fn blkreplay_co_pwritev(BlockDriverState *bs,
 static int coroutine_fn blkreplay_co_pwrite_zeroes(BlockDriverState *bs,
     int64_t offset, int count, BdrvRequestFlags flags)
 {
-    uint64_t reqid = request_id++;
+    uint64_t reqid = blkreplay_next_id();
     int ret = bdrv_co_pwrite_zeroes(bs->file, offset, count, flags);
     block_request_create(reqid, bs, qemu_coroutine_self());
     qemu_coroutine_yield();
@@ -227,7 +222,7 @@ static int coroutine_fn blkreplay_co_pwrite_zeroes(BlockDriverState *bs,
 static int coroutine_fn blkreplay_co_pdiscard(BlockDriverState *bs,
                                               int64_t offset, int count)
 {
-    uint64_t reqid = request_id++;
+    uint64_t reqid = blkreplay_next_id();
     int ret = bdrv_co_pdiscard(bs->file->bs, offset, count);
     block_request_create(reqid, bs, qemu_coroutine_self());
     qemu_coroutine_yield();
@@ -237,7 +232,7 @@ static int coroutine_fn blkreplay_co_pdiscard(BlockDriverState *bs,
 
 static int coroutine_fn blkreplay_co_flush(BlockDriverState *bs)
 {
-    uint64_t reqid = request_id++;
+    uint64_t reqid = blkreplay_next_id();
     int ret = bdrv_co_flush(bs->file->bs);
     block_request_create(reqid, bs, qemu_coroutine_self());
     qemu_coroutine_yield();
diff --git a/cpus.c b/cpus.c
index 0ab4ab1..3938213 100644
--- a/cpus.c
+++ b/cpus.c
@@ -745,6 +745,7 @@ static int do_vm_stop(RunState state)
     }
 
     bdrv_drain_all();
+    replay_disable_events();
     ret = blk_flush_all();
 
     return ret;
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 1123fc2..d893c63 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -107,6 +107,8 @@ bool replay_checkpoint(ReplayCheckpoint checkpoint);
 
 /*! Disables storing events in the queue */
 void replay_disable_events(void);
+/*! Enables storing events in the queue */
+void replay_enable_events(void);
 /*! Returns true when saving events is enabled */
 bool replay_events_enabled(void);
 /*! Adds bottom half event to the queue */
@@ -117,6 +119,8 @@ void replay_input_event(QemuConsole *src, InputEvent *evt);
 void replay_input_sync_event(void);
 /*! Adds block layer event to the queue */
 void replay_block_event(QEMUBH *bh, uint64_t id);
+/*! Returns ID for the next block event */
+uint64_t blkreplay_next_id(void);
 
 /* Character device */
 
diff --git a/replay/replay-events.c b/replay/replay-events.c
index 4ee2f5d..94a6dcc 100644
--- a/replay/replay-events.c
+++ b/replay/replay-events.c
@@ -320,3 +320,11 @@ bool replay_events_enabled(void)
 {
     return events_enabled;
 }
+
+uint64_t blkreplay_next_id(void)
+{
+    if (replay_events_enabled()) {
+        return replay_state.block_request_id++;
+    }
+    return 0;
+}
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index 9bc4a29..c5afc58 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -69,6 +69,10 @@ typedef struct ReplayState {
     unsigned int has_unread_data;
     /*! Temporary variable for saving current log offset. */
     uint64_t file_offset;
+    /*! Next block operation id.
+        This counter is global, because requests from different
+        block devices should not get overlapping ids. */
+    uint64_t block_request_id;
 } ReplayState;
 extern ReplayState replay_state;
 
@@ -124,8 +128,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);
-/*! Enables storing events in the queue */
-void replay_enable_events(void);
 /*! Flushes events queue */
 void replay_flush_events(void);
 /*! Clears events list before loading new VM state */
diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
index 4933bdd..b7b75d6 100644
--- a/replay/replay-snapshot.c
+++ b/replay/replay-snapshot.c
@@ -49,6 +49,7 @@ static const VMStateDescription vmstate_replay = {
         VMSTATE_UINT32(data_kind, ReplayState),
         VMSTATE_UINT32(has_unread_data, ReplayState),
         VMSTATE_UINT64(file_offset, ReplayState),
+        VMSTATE_UINT64(block_request_id, ReplayState),
         VMSTATE_END_OF_LIST()
     },
 };
diff --git a/stubs/replay.c b/stubs/replay.c
index de9fa1e..d9a6da9 100644
--- a/stubs/replay.c
+++ b/stubs/replay.c
@@ -67,3 +67,8 @@ void replay_char_read_all_save_buf(uint8_t *buf, int offset)
 void replay_block_event(QEMUBH *bh, uint64_t id)
 {
 }
+
+uint64_t blkreplay_next_id(void)
+{
+    return 0;
+}
diff --git a/vl.c b/vl.c
index f2193cb..32155f5 100644
--- a/vl.c
+++ b/vl.c
@@ -746,6 +746,7 @@ void vm_start(void)
     if (runstate_is_running()) {
         qapi_event_send_stop(&error_abort);
     } else {
+        replay_enable_events();
         cpu_enable_ticks();
         runstate_set(RUN_STATE_RUNNING);
         vm_state_notify(1, RUN_STATE_RUNNING);

[Qemu-devel] [PATCH v2 08/10] kvmvapic: fix state change handler

[Pavel Dovgalyuk]

This patch fixes kvmvapic state change handler.
It clears vmsentry field to allow recreating it
at further vmstate loads.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 hw/i386/kvmvapic.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 3bf1ddd..a1cd9b5 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -768,6 +768,7 @@ static void kvmvapic_vm_state_change(void *opaque, int running,
     }
 
     qemu_del_vm_change_state_handler(s->vmsentry);
+    s->vmsentry = NULL;
 }
 
 static int vapic_post_load(void *opaque, int version_id)

[Qemu-devel] [PATCH v2 09/10] pcspk: adding vmstate for save/restore

[Pavel Dovgalyuk]

VMState added by this patch preserves correct
loading of the PC speaker device state.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 hw/audio/pcspk.c |   17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
index 42a6f48..984534b 100644
--- a/hw/audio/pcspk.c
+++ b/hw/audio/pcspk.c
@@ -52,8 +52,8 @@ typedef struct {
     unsigned int pit_count;
     unsigned int samples;
     unsigned int play_pos;
-    int data_on;
-    int dummy_refresh_clock;
+    uint8_t data_on;
+    uint8_t dummy_refresh_clock;
 } PCSpkState;
 
 static const char *s_spk = "pcspk";
@@ -187,6 +187,18 @@ static void pcspk_realizefn(DeviceState *dev, Error **errp)
     pcspk_state = s;
 }
 
+static const VMStateDescription vmstate_spk = {
+    .name = "pcspk",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT8(data_on, PCSpkState),
+        VMSTATE_UINT8(dummy_refresh_clock, PCSpkState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static Property pcspk_properties[] = {
     DEFINE_PROP_UINT32("iobase", PCSpkState, iobase,  -1),
     DEFINE_PROP_END_OF_LIST(),
@@ -198,6 +210,7 @@ static void pcspk_class_initfn(ObjectClass *klass, void *data)
 
     dc->realize = pcspk_realizefn;
     set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
+    dc->vmsd = &vmstate_spk;
     dc->props = pcspk_properties;
     /* Reason: realize sets global pcspk_state */
     dc->cannot_instantiate_with_device_add_yet = true;

[Qemu-devel] [PATCH v2 10/10] integratorcp: adding vmstate for save/restore

[Pavel Dovgalyuk]

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

VMState added by this patch preserves correct
loading of the integratorcp device state.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 hw/arm/integratorcp.c |   62 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index 96dc150..3653df2 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -53,6 +53,27 @@ static uint8_t integrator_spd[128] = {
    0xe, 4, 0x1c, 1, 2, 0x20, 0xc0, 0, 0, 0, 0, 0x30, 0x28, 0x30, 0x28, 0x40
 };
 
+static const VMStateDescription vmstate_integratorcm = {
+    .name = "integratorcm",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT32(cm_osc, IntegratorCMState),
+        VMSTATE_UINT32(cm_ctrl, IntegratorCMState),
+        VMSTATE_UINT32(cm_lock, IntegratorCMState),
+        VMSTATE_UINT32(cm_auxosc, IntegratorCMState),
+        VMSTATE_UINT32(cm_sdram, IntegratorCMState),
+        VMSTATE_UINT32(cm_init, IntegratorCMState),
+        VMSTATE_UINT32(cm_flags, IntegratorCMState),
+        VMSTATE_UINT32(cm_nvflags, IntegratorCMState),
+        VMSTATE_UINT32(int_level, IntegratorCMState),
+        VMSTATE_UINT32(irq_enabled, IntegratorCMState),
+        VMSTATE_UINT32(fiq_enabled, IntegratorCMState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static uint64_t integratorcm_read(void *opaque, hwaddr offset,
                                   unsigned size)
 {
@@ -303,6 +324,19 @@ typedef struct icp_pic_state {
     qemu_irq parent_fiq;
 } icp_pic_state;
 
+static const VMStateDescription vmstate_icp_pic = {
+    .name = "icp_pic",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT32(level, icp_pic_state),
+        VMSTATE_UINT32(irq_enabled, icp_pic_state),
+        VMSTATE_UINT32(fiq_enabled, icp_pic_state),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static void icp_pic_update(icp_pic_state *s)
 {
     uint32_t flags;
@@ -432,6 +466,17 @@ typedef struct ICPCtrlRegsState {
 #define ICP_INTREG_WPROT        (1 << 0)
 #define ICP_INTREG_CARDIN       (1 << 3)
 
+static const VMStateDescription vmstate_icp_control = {
+    .name = "icp_control",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT32(intreg_state, ICPCtrlRegsState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static uint64_t icp_control_read(void *opaque, hwaddr offset,
                                  unsigned size)
 {
@@ -633,6 +678,21 @@ static void core_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->props = core_properties;
+    dc->vmsd = &vmstate_integratorcm;
+}
+
+static void icp_pic_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->vmsd = &vmstate_icp_pic;
+}
+
+static void icp_control_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->vmsd = &vmstate_icp_control;
 }
 
 static const TypeInfo core_info = {
@@ -648,6 +708,7 @@ static const TypeInfo icp_pic_info = {
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(icp_pic_state),
     .instance_init = icp_pic_init,
+    .class_init    = icp_pic_class_init,
 };
 
 static const TypeInfo icp_ctrl_regs_info = {
@@ -655,6 +716,7 @@ static const TypeInfo icp_ctrl_regs_info = {
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(ICPCtrlRegsState),
     .instance_init = icp_control_init,
+    .class_init    = icp_control_class_init,
 };
 
 static void integratorcp_register_types(void)

Re: [Qemu-devel] [PATCH v2 00/10] replay additions

[Paolo Bonzini]

On 15/09/2016 11:00, Pavel Dovgalyuk wrote:
> This set of patches includes several fixes for replay and
> adds network record/replay for network devices. It also makes possible
> saving/restoring vmstate in replay mode.
> 
> Record and replay for network interactions is performed with the network filter.
> Each backend must have its own instance of the replay filter as follows:
>  -netdev user,id=net1 -device rtl8139,netdev=net1
>  -object filter-replay,id=replay,netdev=net1
> 
> This patches add overlay option for blkreplay block driver. Using persistent
> overlay file allows saving and reloading VM snapshots in replay mode.
> Replay mechanism automatically creates one snapshot named 'replay_init' to
> allow rewinding execution while replaying.
> Overlay file may be specified as follows:
>  -drive file=disk.qcow,if=none,id=img-direct 
>  -drive driver=blkreplay,if=none,image=img-direct,overlay=overlay.qcow2,id=img-blkreplay 
>  -device ide-hd,drive=img-blkreplay
> 
> This set of patches includes fixes and additions for icount and
> record/replay implementation:
>  - Enabling VM start/stop in replay mode
>  - Adding network interaction record/replay
>  - Adding overlay option for blkreplay filter
>  - Fixes of the vmstate for several virtual devices

Queued patches 8 and 9.  Patch 9 will break migration from 2.8 to 2.7,
unfortunately, same as what happened for parallel in the past.

Paolo

> ---
> 
> Pavel Dovgalyuk (10):
>       record/replay: add network support
>       block: set snapshot option for block devices in blkreplay module
>       block: don't make snapshots for filters
>       replay: save/load initial state
>       replay: move internal data to the structure
>       replay: vmstate for replay module
>       replay: allow replay stopping and restarting
>       kvmvapic: fix state change handler
>       pcspk: adding vmstate for save/restore
>       integratorcp: adding vmstate for save/restore
> 
> 
>  block/blkreplay.c        |  132 +++++++++++++++++++++++++++++++++++++++++++---
>  block/snapshot.c         |    3 +
>  cpus.c                   |    1 
>  docs/replay.txt          |   22 ++++++++
>  hw/arm/integratorcp.c    |   62 ++++++++++++++++++++++
>  hw/audio/pcspk.c         |   17 +++++-
>  hw/i386/kvmvapic.c       |    1 
>  include/sysemu/replay.h  |   26 +++++++++
>  net/Makefile.objs        |    1 
>  net/filter-replay.c      |   90 +++++++++++++++++++++++++++++++
>  replay/Makefile.objs     |    2 +
>  replay/replay-events.c   |   21 +++++++
>  replay/replay-internal.c |   19 +++----
>  replay/replay-internal.h |   26 +++++++--
>  replay/replay-net.c      |  110 ++++++++++++++++++++++++++++++++++++++
>  replay/replay-snapshot.c |   72 +++++++++++++++++++++++++
>  replay/replay-time.c     |    2 -
>  replay/replay.c          |   17 +++---
>  stubs/replay.c           |    5 ++
>  vl.c                     |    9 ++-
>  20 files changed, 599 insertions(+), 39 deletions(-)
>  create mode 100644 net/filter-replay.c
>  create mode 100644 replay/replay-net.c
>  create mode 100644 replay/replay-snapshot.c
> 

Re: [Qemu-devel] [PATCH v2 04/10] replay: save/load initial state

[Paolo Bonzini]

On 15/09/2016 11:01, Pavel Dovgalyuk wrote:
> +{
> +    if (replay_mode == REPLAY_MODE_RECORD) {
> +        QDict *opts = qdict_new();
> +        qdict_put(opts, "name", qstring_from_str("replay_init"));
> +        hmp_savevm(cur_mon, opts);
> +        QDECREF(opts);
> +    } else if (replay_mode == REPLAY_MODE_PLAY) {
> +        load_vmstate("replay_init");

See my other message about a suggestion to remove the hardcoded snapshot
name.

Also, I think the return value of load_vmstate and hmp_savevm should be
checked.

> +    }
> +}
> diff --git a/vl.c b/vl.c
> index 1c68779..6698d88 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4593,6 +4593,8 @@ int main(int argc, char **argv, char **envp)
>          if (load_vmstate(loadvm) < 0) {
>              autostart = 0;
>          }
> +    } else {
> +        replay_vmstate_init();
>      }

Should -loadvm be incompatible with -rr?

Paolo

Re: [Qemu-devel] [PATCH v2 02/10] block: set snapshot option for block devices in blkreplay module

[Paolo Bonzini]

On 15/09/2016 11:00, Pavel Dovgalyuk wrote:
> diff --git a/docs/replay.txt b/docs/replay.txt
> index 347b2ff..5be8f25 100644
> --- a/docs/replay.txt
> +++ b/docs/replay.txt
> @@ -196,6 +196,14 @@ is recorded to the log. In replay phase the queue is matched with
>  events read from the log. Therefore block devices requests are processed
>  deterministically.
>  
> +blkdriver also supports overlay option, which allows creating persistent
> +overlay file for saving and reloading VM snapshots in record/replay modes.
> +Replay mechanism automatically creates one snapshot named 'replay_init' to
> +allow rewinding execution while replaying.
> +Overlay file may be specified as follows:
> + -drive driver=blkreplay,if=none,image=img-direct,
> +        overlay=overlay.qcow2,id=img-blkreplay

So in this case the image is actually overlay.qcow2, and it is created 
with img-direct as the backing file?  Since you have to create 
overlay.qcow2 outside QEMU anyway, overlay.qcow2 might as well be the 
"image".  That is, you could choose between:

   -drive driver=blkreplay,if=none,image=overlay.qcow2,id=img-blkreplay \
   -rr snapshot=replay_init,...

   -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay

The temporary snapshot would be created if there's no "-rr snapshot" option
on the command line.

Does this make sense?

Paolo

> 
> +static QemuOptsList blkreplay_runtime_opts = {
> +    .name = "quorum",

Pasto. ;)

> +    .head = QTAILQ_HEAD_INITIALIZER(blkreplay_runtime_opts.head),
> +    .desc = {
> +        {
> +            .name = "overlay",
> +            .type = QEMU_OPT_STRING,
> +            .help = "Optional overlay file for snapshots",
> +        },
> +        { /* end of list */ }
> +    },
> +};

Re: [Qemu-devel] [PATCH v2 05/10] replay: move internal data to the structure

[Paolo Bonzini]

On 15/09/2016 11:01, Pavel Dovgalyuk wrote:
> This patch moves replay static variables into the structure
> to allow saving and loading them with savevm/loadvm.
> 
> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> ---
>  replay/replay-events.c   |    2 +-
>  replay/replay-internal.c |   19 ++++++++-----------
>  replay/replay-internal.h |    8 +++++---
>  replay/replay-time.c     |    2 +-
>  replay/replay.c          |   15 ++++++++-------
>  5 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/replay/replay-events.c b/replay/replay-events.c
> index 9ce9e51..4ee2f5d 100644
> --- a/replay/replay-events.c
> +++ b/replay/replay-events.c
> @@ -290,7 +290,7 @@ static Event *replay_read_event(int checkpoint)
>  /* Called with replay mutex locked */
>  void replay_read_events(int checkpoint)
>  {
> -    while (replay_data_kind == EVENT_ASYNC) {
> +    while (replay_state.data_kind == EVENT_ASYNC) {
>          Event *event = replay_read_event(checkpoint);
>          if (!event) {
>              break;
> diff --git a/replay/replay-internal.c b/replay/replay-internal.c
> index 5835e8d..f4e9caa 100644
> --- a/replay/replay-internal.c
> +++ b/replay/replay-internal.c
> @@ -16,11 +16,8 @@
>  #include "qemu/error-report.h"
>  #include "sysemu/sysemu.h"
>  
> -unsigned int replay_data_kind = -1;
> -static unsigned int replay_has_unread_data;
> -
>  /* Mutex to protect reading and writing events to the log.
> -   replay_data_kind and replay_has_unread_data are also protected
> +   data_kind and has_unread_data are also protected
>     by this mutex.
>     It also protects replay events queue which stores events to be
>     written or read to the log. */
> @@ -150,15 +147,15 @@ void replay_check_error(void)
>  void replay_fetch_data_kind(void)
>  {
>      if (replay_file) {
> -        if (!replay_has_unread_data) {
> -            replay_data_kind = replay_get_byte();
> -            if (replay_data_kind == EVENT_INSTRUCTION) {
> +        if (!replay_state.has_unread_data) {
> +            replay_state.data_kind = replay_get_byte();
> +            if (replay_state.data_kind == EVENT_INSTRUCTION) {
>                  replay_state.instructions_count = replay_get_dword();
>              }
>              replay_check_error();
> -            replay_has_unread_data = 1;
> -            if (replay_data_kind >= EVENT_COUNT) {
> -                error_report("Replay: unknown event kind %d", replay_data_kind);
> +            replay_state.has_unread_data = 1;
> +            if (replay_state.data_kind >= EVENT_COUNT) {
> +                error_report("Replay: unknown event kind %d", replay_state.data_kind);
>                  exit(1);
>              }
>          }
> @@ -167,7 +164,7 @@ void replay_fetch_data_kind(void)
>  
>  void replay_finish_event(void)
>  {
> -    replay_has_unread_data = 0;
> +    replay_state.has_unread_data = 0;
>      replay_fetch_data_kind();
>  }
>  
> diff --git a/replay/replay-internal.h b/replay/replay-internal.h
> index d28cfb7..3147d66 100644
> --- a/replay/replay-internal.h
> +++ b/replay/replay-internal.h
> @@ -63,11 +63,13 @@ typedef struct ReplayState {
>      uint64_t current_step;
>      /*! Number of instructions to be executed before other events happen. */
>      int instructions_count;
> +    /*! Type of the currently executed event. */
> +    unsigned int data_kind;
> +    /*! Flag which indicates that event is not processed yet. */
> +    unsigned int has_unread_data;
>  } ReplayState;
>  extern ReplayState replay_state;
>  
> -extern unsigned int replay_data_kind;
> -
>  /* File for replay writing */
>  extern FILE *replay_file;
>  
> @@ -99,7 +101,7 @@ void replay_check_error(void);
>      the next event from the log. */
>  void replay_finish_event(void);
>  /*! Reads data type from the file and stores it in the
> -    replay_data_kind variable. */
> +    data_kind variable. */
>  void replay_fetch_data_kind(void);
>  
>  /*! Saves queued events (like instructions and sound). */
> diff --git a/replay/replay-time.c b/replay/replay-time.c
> index fffe072..f70382a 100644
> --- a/replay/replay-time.c
> +++ b/replay/replay-time.c
> @@ -31,7 +31,7 @@ int64_t replay_save_clock(ReplayClockKind kind, int64_t clock)
>  
>  void replay_read_next_clock(ReplayClockKind kind)
>  {
> -    unsigned int read_kind = replay_data_kind - EVENT_CLOCK;
> +    unsigned int read_kind = replay_state.data_kind - EVENT_CLOCK;
>  
>      assert(read_kind == kind);
>  
> diff --git a/replay/replay.c b/replay/replay.c
> index e040f6f..56b4237 100644
> --- a/replay/replay.c
> +++ b/replay/replay.c
> @@ -38,15 +38,15 @@ bool replay_next_event_is(int event)
>  
>      /* nothing to skip - not all instructions used */
>      if (replay_state.instructions_count != 0) {
> -        assert(replay_data_kind == EVENT_INSTRUCTION);
> +        assert(replay_state.data_kind == EVENT_INSTRUCTION);
>          return event == EVENT_INSTRUCTION;
>      }
>  
>      while (true) {
> -        if (event == replay_data_kind) {
> +        if (event == replay_state.data_kind) {
>              res = true;
>          }
> -        switch (replay_data_kind) {
> +        switch (replay_state.data_kind) {
>          case EVENT_SHUTDOWN:
>              replay_finish_event();
>              qemu_system_shutdown_request();
> @@ -85,7 +85,7 @@ void replay_account_executed_instructions(void)
>              replay_state.instructions_count -= count;
>              replay_state.current_step += count;
>              if (replay_state.instructions_count == 0) {
> -                assert(replay_data_kind == EVENT_INSTRUCTION);
> +                assert(replay_state.data_kind == EVENT_INSTRUCTION);
>                  replay_finish_event();
>                  /* Wake up iothread. This is required because
>                     timers will not expire until clock counters
> @@ -188,7 +188,7 @@ bool replay_checkpoint(ReplayCheckpoint checkpoint)
>      if (replay_mode == REPLAY_MODE_PLAY) {
>          if (replay_next_event_is(EVENT_CHECKPOINT + checkpoint)) {
>              replay_finish_event();
> -        } else if (replay_data_kind != EVENT_ASYNC) {
> +        } else if (replay_state.data_kind != EVENT_ASYNC) {
>              res = false;
>              goto out;
>          }
> @@ -196,7 +196,7 @@ bool replay_checkpoint(ReplayCheckpoint checkpoint)
>          /* replay_read_events may leave some unread events.
>             Return false if not all of the events associated with
>             checkpoint were processed */
> -        res = replay_data_kind != EVENT_ASYNC;
> +        res = replay_state.data_kind != EVENT_ASYNC;
>      } else if (replay_mode == REPLAY_MODE_RECORD) {
>          replay_put_event(EVENT_CHECKPOINT + checkpoint);
>          replay_save_events(checkpoint);
> @@ -237,9 +237,10 @@ static void replay_enable(const char *fname, int mode)
>      replay_filename = g_strdup(fname);
>  
>      replay_mode = mode;
> -    replay_data_kind = -1;
> +    replay_state.data_kind = -1;
>      replay_state.instructions_count = 0;
>      replay_state.current_step = 0;
> +    replay_state.has_unread_data = 0;
>  
>      /* skip file header for RECORD and check it for PLAY */
>      if (replay_mode == REPLAY_MODE_RECORD) {
> 

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

Re: [Qemu-devel] [PATCH v2 02/10] block: set snapshot option for block devices in blkreplay module

[Paolo Bonzini]

On 15/09/2016 11:25, Paolo Bonzini wrote:
> So in this case the image is actually overlay.qcow2, and it is created 
> with img-direct as the backing file?  Since you have to create 
> overlay.qcow2 outside QEMU anyway, overlay.qcow2 might as well be the 
> "image".  That is, you could choose between:
> 
>    -drive driver=blkreplay,if=none,image=overlay.qcow2,id=img-blkreplay \
>    -rr snapshot=replay_init,...
> 
>    -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
> 
> The temporary snapshot would be created if there's no "-rr snapshot" option
> on the command line.

Sorry, that would be "-icount rrsnapshot=" of course.

Paolo

Re: [Qemu-devel] [PATCH v2 06/10] replay: vmstate for replay module

[Paolo Bonzini]

On 15/09/2016 11:01, Pavel Dovgalyuk wrote:
> This patch introduces vmstate for replay data structures.
> It allows saving and loading vmstate while replaying.
> 
> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> ---
>  include/sysemu/replay.h  |    4 ++++
>  replay/replay-internal.h |    2 ++
>  replay/replay-snapshot.c |   40 ++++++++++++++++++++++++++++++++++++++++
>  vl.c                     |    1 +
>  4 files changed, 47 insertions(+)
> 
> diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
> index aa378ce..1123fc2 100644
> --- a/include/sysemu/replay.h
> +++ b/include/sysemu/replay.h
> @@ -147,6 +147,10 @@ void replay_net_packet_event(ReplayNetState *rns, unsigned flags,
>  
>  /* VM state operations */
>  
> +/* Registers replay VMState.
> +   Should be called before virtual devices initialization
> +   to make cached timers available for post_load functions. */
> +void replay_vmstate_register(void);

Can this be done simply in replay_configure?

Paolo

>  /*! Called at the start of execution.
>      Loads or saves initial vmstate depending on execution mode. */
>  void replay_vmstate_init(void);
> diff --git a/replay/replay-internal.h b/replay/replay-internal.h
> index 3147d66..9bc4a29 100644
> --- a/replay/replay-internal.h
> +++ b/replay/replay-internal.h
> @@ -67,6 +67,8 @@ typedef struct ReplayState {
>      unsigned int data_kind;
>      /*! Flag which indicates that event is not processed yet. */
>      unsigned int has_unread_data;
> +    /*! Temporary variable for saving current log offset. */
> +    uint64_t file_offset;
>  } ReplayState;
>  extern ReplayState replay_state;
>  
> diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
> index a48c6fc..4933bdd 100644
> --- a/replay/replay-snapshot.c
> +++ b/replay/replay-snapshot.c
> @@ -17,6 +17,46 @@
>  #include "sysemu/sysemu.h"
>  #include "monitor/monitor.h"
>  #include "qapi/qmp/qstring.h"
> +#include "migration/vmstate.h"
> +
> +static void replay_pre_save(void *opaque)
> +{
> +    ReplayState *state = opaque;
> +    state->file_offset = ftello64(replay_file);
> +}
> +
> +static int replay_post_load(void *opaque, int version_id)
> +{
> +    ReplayState *state = opaque;
> +    fseeko64(replay_file, state->file_offset, SEEK_SET);
> +    /* If this was a vmstate, saved in recording mode,
> +       we need to initialize replay data fields. */
> +    replay_fetch_data_kind();
> +
> +    return 0;
> +}
> +
> +static const VMStateDescription vmstate_replay = {
> +    .name = "replay",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .pre_save = replay_pre_save,
> +    .post_load = replay_post_load,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_INT64_ARRAY(cached_clock, ReplayState, REPLAY_CLOCK_COUNT),
> +        VMSTATE_UINT64(current_step, ReplayState),
> +        VMSTATE_INT32(instructions_count, ReplayState),
> +        VMSTATE_UINT32(data_kind, ReplayState),
> +        VMSTATE_UINT32(has_unread_data, ReplayState),
> +        VMSTATE_UINT64(file_offset, ReplayState),
> +        VMSTATE_END_OF_LIST()
> +    },
> +};
> +
> +void replay_vmstate_register(void)
> +{
> +    vmstate_register(NULL, 0, &vmstate_replay, &replay_state);
> +}
>  
>  void replay_vmstate_init(void)
>  {
> diff --git a/vl.c b/vl.c
> index 6698d88..f2193cb 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4361,6 +4361,7 @@ int main(int argc, char **argv, char **envp)
>          configure_icount(icount_opts, &error_abort);
>          qemu_opts_del(icount_opts);
>      }
> +    replay_vmstate_register();
>  
>      if (default_net) {
>          QemuOptsList *net = qemu_find_opts("net");
> 
> 
> 

Re: [Qemu-devel] [PATCH v2 07/10] replay: allow replay stopping and restarting

[Paolo Bonzini]

On 15/09/2016 11:01, Pavel Dovgalyuk wrote:
> This patch fixes bug with stopping and restarting replay
> through monitor.
> 
> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> ---
>  block/blkreplay.c        |   15 +++++----------
>  cpus.c                   |    1 +
>  include/sysemu/replay.h  |    4 ++++
>  replay/replay-events.c   |    8 ++++++++
>  replay/replay-internal.h |    6 ++++--
>  replay/replay-snapshot.c |    1 +
>  stubs/replay.c           |    5 +++++
>  vl.c                     |    1 +
>  8 files changed, 29 insertions(+), 12 deletions(-)
> 
> diff --git a/block/blkreplay.c b/block/blkreplay.c
> index 62ae861..c2c3615 100644
> --- a/block/blkreplay.c
> +++ b/block/blkreplay.c
> @@ -21,11 +21,6 @@ typedef struct Request {
>      QEMUBH *bh;
>  } Request;
>  
> -/* Next request id.
> -   This counter is global, because requests from different
> -   block devices should not get overlapping ids. */
> -static uint64_t request_id;
> -
>  static BlockDriverState *blkreplay_append_snapshot(BlockDriverState *bs,
>                                                     int flags,
>                                                     QDict *snapshot_options,
> @@ -194,7 +189,7 @@ static void block_request_create(uint64_t reqid, BlockDriverState *bs,
>  static int coroutine_fn blkreplay_co_preadv(BlockDriverState *bs,
>      uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
>  {
> -    uint64_t reqid = request_id++;
> +    uint64_t reqid = blkreplay_next_id();
>      int ret = bdrv_co_preadv(bs->file, offset, bytes, qiov, flags);
>      block_request_create(reqid, bs, qemu_coroutine_self());
>      qemu_coroutine_yield();
> @@ -205,7 +200,7 @@ static int coroutine_fn blkreplay_co_preadv(BlockDriverState *bs,
>  static int coroutine_fn blkreplay_co_pwritev(BlockDriverState *bs,
>      uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
>  {
> -    uint64_t reqid = request_id++;
> +    uint64_t reqid = blkreplay_next_id();
>      int ret = bdrv_co_pwritev(bs->file, offset, bytes, qiov, flags);
>      block_request_create(reqid, bs, qemu_coroutine_self());
>      qemu_coroutine_yield();
> @@ -216,7 +211,7 @@ static int coroutine_fn blkreplay_co_pwritev(BlockDriverState *bs,
>  static int coroutine_fn blkreplay_co_pwrite_zeroes(BlockDriverState *bs,
>      int64_t offset, int count, BdrvRequestFlags flags)
>  {
> -    uint64_t reqid = request_id++;
> +    uint64_t reqid = blkreplay_next_id();
>      int ret = bdrv_co_pwrite_zeroes(bs->file, offset, count, flags);
>      block_request_create(reqid, bs, qemu_coroutine_self());
>      qemu_coroutine_yield();
> @@ -227,7 +222,7 @@ static int coroutine_fn blkreplay_co_pwrite_zeroes(BlockDriverState *bs,
>  static int coroutine_fn blkreplay_co_pdiscard(BlockDriverState *bs,
>                                                int64_t offset, int count)
>  {
> -    uint64_t reqid = request_id++;
> +    uint64_t reqid = blkreplay_next_id();
>      int ret = bdrv_co_pdiscard(bs->file->bs, offset, count);
>      block_request_create(reqid, bs, qemu_coroutine_self());
>      qemu_coroutine_yield();
> @@ -237,7 +232,7 @@ static int coroutine_fn blkreplay_co_pdiscard(BlockDriverState *bs,
>  
>  static int coroutine_fn blkreplay_co_flush(BlockDriverState *bs)
>  {
> -    uint64_t reqid = request_id++;
> +    uint64_t reqid = blkreplay_next_id();
>      int ret = bdrv_co_flush(bs->file->bs);
>      block_request_create(reqid, bs, qemu_coroutine_self());
>      qemu_coroutine_yield();
> diff --git a/cpus.c b/cpus.c
> index 0ab4ab1..3938213 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -745,6 +745,7 @@ static int do_vm_stop(RunState state)
>      }
>  
>      bdrv_drain_all();
> +    replay_disable_events();
>      ret = blk_flush_all();
>  
>      return ret;
> diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
> index 1123fc2..d893c63 100644
> --- a/include/sysemu/replay.h
> +++ b/include/sysemu/replay.h
> @@ -107,6 +107,8 @@ bool replay_checkpoint(ReplayCheckpoint checkpoint);
>  
>  /*! Disables storing events in the queue */
>  void replay_disable_events(void);
> +/*! Enables storing events in the queue */
> +void replay_enable_events(void);
>  /*! Returns true when saving events is enabled */
>  bool replay_events_enabled(void);
>  /*! Adds bottom half event to the queue */
> @@ -117,6 +119,8 @@ void replay_input_event(QemuConsole *src, InputEvent *evt);
>  void replay_input_sync_event(void);
>  /*! Adds block layer event to the queue */
>  void replay_block_event(QEMUBH *bh, uint64_t id);
> +/*! Returns ID for the next block event */
> +uint64_t blkreplay_next_id(void);
>  
>  /* Character device */
>  
> diff --git a/replay/replay-events.c b/replay/replay-events.c
> index 4ee2f5d..94a6dcc 100644
> --- a/replay/replay-events.c
> +++ b/replay/replay-events.c
> @@ -320,3 +320,11 @@ bool replay_events_enabled(void)
>  {
>      return events_enabled;
>  }
> +
> +uint64_t blkreplay_next_id(void)
> +{
> +    if (replay_events_enabled()) {
> +        return replay_state.block_request_id++;
> +    }
> +    return 0;
> +}
> diff --git a/replay/replay-internal.h b/replay/replay-internal.h
> index 9bc4a29..c5afc58 100644
> --- a/replay/replay-internal.h
> +++ b/replay/replay-internal.h
> @@ -69,6 +69,10 @@ typedef struct ReplayState {
>      unsigned int has_unread_data;
>      /*! Temporary variable for saving current log offset. */
>      uint64_t file_offset;
> +    /*! Next block operation id.
> +        This counter is global, because requests from different
> +        block devices should not get overlapping ids. */
> +    uint64_t block_request_id;
>  } ReplayState;
>  extern ReplayState replay_state;
>  
> @@ -124,8 +128,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);
> -/*! Enables storing events in the queue */
> -void replay_enable_events(void);
>  /*! Flushes events queue */
>  void replay_flush_events(void);
>  /*! Clears events list before loading new VM state */
> diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
> index 4933bdd..b7b75d6 100644
> --- a/replay/replay-snapshot.c
> +++ b/replay/replay-snapshot.c
> @@ -49,6 +49,7 @@ static const VMStateDescription vmstate_replay = {
>          VMSTATE_UINT32(data_kind, ReplayState),
>          VMSTATE_UINT32(has_unread_data, ReplayState),
>          VMSTATE_UINT64(file_offset, ReplayState),
> +        VMSTATE_UINT64(block_request_id, ReplayState),
>          VMSTATE_END_OF_LIST()
>      },
>  };
> diff --git a/stubs/replay.c b/stubs/replay.c
> index de9fa1e..d9a6da9 100644
> --- a/stubs/replay.c
> +++ b/stubs/replay.c
> @@ -67,3 +67,8 @@ void replay_char_read_all_save_buf(uint8_t *buf, int offset)
>  void replay_block_event(QEMUBH *bh, uint64_t id)
>  {
>  }
> +
> +uint64_t blkreplay_next_id(void)
> +{
> +    return 0;
> +}
> diff --git a/vl.c b/vl.c
> index f2193cb..32155f5 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -746,6 +746,7 @@ void vm_start(void)
>      if (runstate_is_running()) {
>          qapi_event_send_stop(&error_abort);
>      } else {
> +        replay_enable_events();

Can this be done with a change state notifier?

Paolo

>          cpu_enable_ticks();
>          runstate_set(RUN_STATE_RUNNING);
>          vm_state_notify(1, RUN_STATE_RUNNING);
> 
> 
> 

Re: [Qemu-devel] [PATCH v2 07/10] replay: allow replay stopping and restarting

[Pavel Dovgalyuk]

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> On 15/09/2016 11:01, Pavel Dovgalyuk wrote:
> > This patch fixes bug with stopping and restarting replay
> > through monitor.
> >
> > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> > ---
> > diff --git a/vl.c b/vl.c
> > index f2193cb..32155f5 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -746,6 +746,7 @@ void vm_start(void)
> >      if (runstate_is_running()) {
> >          qapi_event_send_stop(&error_abort);
> >      } else {
> > +        replay_enable_events();
> 
> Can this be done with a change state notifier?

We have to be sure that events are enabled before any of the callbacks will try to
use events subsystem. Therefore enabling events must be the first thing to do.

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v2 06/10] replay: vmstate for replay module

[Pavel Dovgalyuk]

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> On 15/09/2016 11:01, Pavel Dovgalyuk wrote:
> > This patch introduces vmstate for replay data structures.
> > It allows saving and loading vmstate while replaying.
> >
> > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> > ---
> >  include/sysemu/replay.h  |    4 ++++
> >  replay/replay-internal.h |    2 ++
> >  replay/replay-snapshot.c |   40 ++++++++++++++++++++++++++++++++++++++++
> >  vl.c                     |    1 +
> >  4 files changed, 47 insertions(+)
> >
> > diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
> > index aa378ce..1123fc2 100644
> > --- a/include/sysemu/replay.h
> > +++ b/include/sysemu/replay.h
> > @@ -147,6 +147,10 @@ void replay_net_packet_event(ReplayNetState *rns, unsigned flags,
> >
> >  /* VM state operations */
> >
> > +/* Registers replay VMState.
> > +   Should be called before virtual devices initialization
> > +   to make cached timers available for post_load functions. */
> > +void replay_vmstate_register(void);
> 
> Can this be done simply in replay_configure?

I guess it can. Configuring is performed early enough to allow vmsd be added
first into the list.

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v2 02/10] block: set snapshot option for block devices in blkreplay module

[Pavel Dovgalyuk]

> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> On 15/09/2016 11:00, Pavel Dovgalyuk wrote:
> > diff --git a/docs/replay.txt b/docs/replay.txt
> > index 347b2ff..5be8f25 100644
> > --- a/docs/replay.txt
> > +++ b/docs/replay.txt
> > @@ -196,6 +196,14 @@ is recorded to the log. In replay phase the queue is matched with
> >  events read from the log. Therefore block devices requests are processed
> >  deterministically.
> >
> > +blkdriver also supports overlay option, which allows creating persistent
> > +overlay file for saving and reloading VM snapshots in record/replay modes.
> > +Replay mechanism automatically creates one snapshot named 'replay_init' to
> > +allow rewinding execution while replaying.
> > +Overlay file may be specified as follows:
> > + -drive driver=blkreplay,if=none,image=img-direct,
> > +        overlay=overlay.qcow2,id=img-blkreplay
> 
> So in this case the image is actually overlay.qcow2, and it is created
> with img-direct as the backing file? 

Right.

> Since you have to create
> overlay.qcow2 outside QEMU anyway, overlay.qcow2 might as well be the
> "image".  That is, you could choose between:
> 
>    -drive driver=blkreplay,if=none,image=overlay.qcow2,id=img-blkreplay \
>    -rr snapshot=replay_init,...
> 
>    -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
> 
> The temporary snapshot would be created if there's no "-rr snapshot" option
> on the command line.
> 
> Does this make sense?

There are two different parts:
 - creating an overlay
 - creating the snapshot

Overlay is needed to preserve the state of the original backing file.
In the current version temporary overlay is always created at start of qemu.
Then all changes are destroyed at exit and disk remain the same.
It allows replaying the execution from the same disk state.

To allow reverse execution we have to create some snapshots, that will allow
rewinding the execution. Snapshots have to be written on some non-temporary overlay.

I don't think that it is convenient forcing user to create overlay manually.
Common debugging scenario includes multiple recording passes until the bug manifests
itself. Every new execution recorded should be accompanied by creating an overlay
to assure that the execution is started from the same disk state.

Specifying initial snapshot name makes sense if we want to suppress -loadvm option.
Then replay may be started from any state without using -loadvm.

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v2 04/10] replay: save/load initial state

[Pavel Dovgalyuk]

> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> On 15/09/2016 11:01, Pavel Dovgalyuk wrote:
> > +{
> > +    if (replay_mode == REPLAY_MODE_RECORD) {
> > +        QDict *opts = qdict_new();
> > +        qdict_put(opts, "name", qstring_from_str("replay_init"));
> > +        hmp_savevm(cur_mon, opts);
> > +        QDECREF(opts);
> > +    } else if (replay_mode == REPLAY_MODE_PLAY) {
> > +        load_vmstate("replay_init");
> 
> See my other message about a suggestion to remove the hardcoded snapshot
> name.
> 
> Also, I think the return value of load_vmstate and hmp_savevm should be
> checked.

Load may be unsuccessful when the system is started with temporary overlay file.
This is ok.

> > +    }
> > +}
> > diff --git a/vl.c b/vl.c
> > index 1c68779..6698d88 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -4593,6 +4593,8 @@ int main(int argc, char **argv, char **envp)
> >          if (load_vmstate(loadvm) < 0) {
> >              autostart = 0;
> >          }
> > +    } else {
> > +        replay_vmstate_init();
> >      }
> 
> Should -loadvm be incompatible with -rr?

User either loads some initial state or relies on replay which uses 'replay_init'.

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v2 07/10] replay: allow replay stopping and restarting

[Paolo Bonzini]

On 16/09/2016 08:35, Pavel Dovgalyuk wrote:
>> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
>> On 15/09/2016 11:01, Pavel Dovgalyuk wrote:
>>> This patch fixes bug with stopping and restarting replay
>>> through monitor.
>>>
>>> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
>>> ---
>>> diff --git a/vl.c b/vl.c
>>> index f2193cb..32155f5 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -746,6 +746,7 @@ void vm_start(void)
>>>      if (runstate_is_running()) {
>>>          qapi_event_send_stop(&error_abort);
>>>      } else {
>>> +        replay_enable_events();
>>
>> Can this be done with a change state notifier?
> 
> We have to be sure that events are enabled before any of the callbacks will try to
> use events subsystem. Therefore enabling events must be the first thing to do.

Ok.

Paolo

Re: [Qemu-devel] [PATCH v2 02/10] block: set snapshot option for block devices in blkreplay module

[Paolo Bonzini]

On 16/09/2016 09:55, Pavel Dovgalyuk wrote:
>> Since you have to create
>> overlay.qcow2 outside QEMU anyway, overlay.qcow2 might as well be the
>> "image".  That is, you could choose between:
>>
>>    -drive driver=blkreplay,if=none,image=overlay.qcow2,id=img-blkreplay \
>>    -rr snapshot=replay_init,...
>>
>>    -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
>>
>> The temporary snapshot would be created if there's no "-rr snapshot" option
>> on the command line.
>>
>> Does this make sense?
> 
> There are two different parts:
>  - creating an overlay
>  - creating the snapshot
> 
> Overlay is needed to preserve the state of the original backing file.
> In the current version temporary overlay is always created at start of qemu.

Yes, this would still be the default for rr mode.

> I don't think that it is convenient forcing user to create overlay manually.

Note that all I'm only saying that _only for the case where the user
creates the overlay manually anyway_ there is no need to specify both
image and overlay.  (I also don't like particularly the hard-coded
snapshot name replay_init, which can be overridden by -loadvm but not
when saving).

So there are various possibilites:

First proposal:

- automatically created overlay is -icount rr=record|replay (then
snapshot name doesn't matter, it can be replay_init)

- manually created overlay is -icount
rr=record|replay,rrsnapshot=snapname (then snapshot name matters because
you can have different snapshots in the same file)

If rr is enabled but rrsnapshot is absent, configure_icount can just set
"snapshot = 1" to force creation of the temporary overlay.  This
requires no change to the blkreplay driver


Second proposal:

- automatically created overlay is -icount rr=record|replay -snapshot

- manually created overlay is -icount rr=record|replay and an rrsnapshot
suboption can be added anyway if considered useful.

This requires no change to the blkreplay driver either.  It's a little
more verbose in the common case, but perhaps less surprising if you're
already used to -snapshot.

> Common debugging scenario includes multiple recording passes until the bug manifests
> itself. Every new execution recorded should be accompanied by creating an overlay
> to assure that the execution is started from the same disk state.
> 
> Specifying initial snapshot name makes sense if we want to suppress -loadvm option.

My rationale was to have similar command lines between record and replay
modes (just changing rr=record to rr=replay).

Paolo

> Then replay may be started from any state without using -loadvm.
> 
> Pavel Dovgalyuk
> 
> 
> 

Re: [Qemu-devel] [PATCH v2 04/10] replay: save/load initial state

[Paolo Bonzini]

On 16/09/2016 09:56, Pavel Dovgalyuk wrote:
>>> > > +{
>>> > > +    if (replay_mode == REPLAY_MODE_RECORD) {
>>> > > +        QDict *opts = qdict_new();
>>> > > +        qdict_put(opts, "name", qstring_from_str("replay_init"));
>>> > > +        hmp_savevm(cur_mon, opts);
>>> > > +        QDECREF(opts);
>>> > > +    } else if (replay_mode == REPLAY_MODE_PLAY) {
>>> > > +        load_vmstate("replay_init");
>> > 
>> > See my other message about a suggestion to remove the hardcoded snapshot
>> > name.
>> > 
>> > Also, I think the return value of load_vmstate and hmp_savevm should be
>> > checked.
> 
> Load may be unsuccessful when the system is started with temporary overlay file.
> This is ok.

This seems to be in favor of using a rrsnapshot option instead.  Using
rrsnapshot for temporary overlay would make little sense.

Paolo

Re: [Qemu-devel] [PATCH v2 02/10] block: set snapshot option for block devices in blkreplay module

[Pavel Dovgalyuk]

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> On 16/09/2016 09:55, Pavel Dovgalyuk wrote:
> >> Since you have to create
> >> overlay.qcow2 outside QEMU anyway, overlay.qcow2 might as well be the
> >> "image".  That is, you could choose between:
> >>
> >>    -drive driver=blkreplay,if=none,image=overlay.qcow2,id=img-blkreplay \
> >>    -rr snapshot=replay_init,...
> >>
> >>    -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
> >>
> >> The temporary snapshot would be created if there's no "-rr snapshot" option
> >> on the command line.
> >>
> >> Does this make sense?
> >
> > There are two different parts:
> >  - creating an overlay
> >  - creating the snapshot
> >
> > Overlay is needed to preserve the state of the original backing file.
> > In the current version temporary overlay is always created at start of qemu.
> 
> Yes, this would still be the default for rr mode.
> 
> > I don't think that it is convenient forcing user to create overlay manually.
> 
> Note that all I'm only saying that _only for the case where the user
> creates the overlay manually anyway_ there is no need to specify both
> image and overlay.  (I also don't like particularly the hard-coded
> snapshot name replay_init, which can be overridden by -loadvm but not
> when saving).

Ok, this seems reasonable to fix.

> 
> So there are various possibilites:
> 
> First proposal:
> 
> - automatically created overlay is -icount rr=record|replay (then
> snapshot name doesn't matter, it can be replay_init)
> 
> - manually created overlay is -icount
> rr=record|replay,rrsnapshot=snapname (then snapshot name matters because
> you can have different snapshots in the same file)

We can't create overlay with icount suboptions, because there could be several
block devices. Each one needs its own overlay.

> If rr is enabled but rrsnapshot is absent, configure_icount can just set
> "snapshot = 1" to force creation of the temporary overlay.  This
> requires no change to the blkreplay driver
> 
> 
> Second proposal:
> 
> - automatically created overlay is -icount rr=record|replay -snapshot
> 
> - manually created overlay is -icount rr=record|replay and an rrsnapshot
> suboption can be added anyway if considered useful.

See above.

> This requires no change to the blkreplay driver either.  It's a little
> more verbose in the common case, but perhaps less surprising if you're
> already used to -snapshot.

Our internal version automatically creates overlays with generated names.
But now I think, that it is inconvenient and manual name specification is better.

> > Common debugging scenario includes multiple recording passes until the bug manifests
> > itself. Every new execution recorded should be accompanied by creating an overlay
> > to assure that the execution is started from the same disk state.
> >
> > Specifying initial snapshot name makes sense if we want to suppress -loadvm option.
> 
> My rationale was to have similar command lines between record and replay
> modes (just changing rr=record to rr=replay).

This seems reasonable, I'll try to fix this part.

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v2 02/10] block: set snapshot option for block devices in blkreplay module

[Paolo Bonzini]

On 16/09/2016 11:36, Pavel Dovgalyuk wrote:
>> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
>> On 16/09/2016 09:55, Pavel Dovgalyuk wrote:
>>>> Since you have to create
>>>> overlay.qcow2 outside QEMU anyway, overlay.qcow2 might as well be the
>>>> "image".  That is, you could choose between:
>>>>
>>>>    -drive driver=blkreplay,if=none,image=overlay.qcow2,id=img-blkreplay \
>>>>    -rr snapshot=replay_init,...
>>>>
>>>>    -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
>>>>
>>>> The temporary snapshot would be created if there's no "-rr snapshot" option
>>>> on the command line.
>>>>
>>>> Does this make sense?
>>>
>>> There are two different parts:
>>>  - creating an overlay
>>>  - creating the snapshot
>>>
>>> Overlay is needed to preserve the state of the original backing file.
>>> In the current version temporary overlay is always created at start of qemu.
>>
>> Yes, this would still be the default for rr mode.
>>
>>> I don't think that it is convenient forcing user to create overlay manually.
>>
>> Note that all I'm only saying that _only for the case where the user
>> creates the overlay manually anyway_ there is no need to specify both
>> image and overlay.  (I also don't like particularly the hard-coded
>> snapshot name replay_init, which can be overridden by -loadvm but not
>> when saving).
> 
> Ok, this seems reasonable to fix.
> 
>>
>> So there are various possibilites:
>>
>> First proposal:
>>
>> - automatically created overlay is -icount rr=record|replay (then
>> snapshot name doesn't matter, it can be replay_init)
>>
>> - manually created overlay is -icount
>> rr=record|replay,rrsnapshot=snapname (then snapshot name matters because
>> you can have different snapshots in the same file)
> 
> We can't create overlay with icount suboptions, because there could be several
> block devices. Each one needs its own overlay.

Right, so the overlay name is specified in each -drive option the
blkreplay image.  rrsnapshot is just the name of the first snapshot that
is created (for rr=record, instead of requiring manual interaction with
the monitor) or loaded (for rr=replay; in this case it's a convenience
only).

Paolo