[Qemu-devel] [PATCH v3 0/8] replay additions

[Qemu-devel] [PATCH v3 0/8] 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:
 - VM start/stop in replay mode
 - Network interaction record/replay
 - overlay option for blkreplay filter
 - rrsnapshot option for record/replay
 - vmstate fix for integratorcp ARM platform

v3 changes:
 - Added rrsnapshot option for specifying the initial snapshot name (as suggested by Paolo Bonzini)
 - Minor changes

---

Pavel Dovgalyuk (8):
      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
      integratorcp: adding vmstate for save/restore


 block/blkreplay.c        |  132 +++++++++++++++++++++++++++++++++++++++++++---
 block/snapshot.c         |    3 +
 cpus.c                   |    1 
 docs/replay.txt          |   34 ++++++++++++
 hw/arm/integratorcp.c    |   62 ++++++++++++++++++++++
 include/sysemu/replay.h  |   22 ++++++++
 net/Makefile.objs        |    1 
 net/filter-replay.c      |   90 +++++++++++++++++++++++++++++++
 qemu-options.hx          |    8 ++-
 replay/Makefile.objs     |    2 +
 replay/replay-events.c   |   21 +++++++
 replay/replay-internal.c |   19 +++----
 replay/replay-internal.h |   36 +++++++++++--
 replay/replay-net.c      |  110 ++++++++++++++++++++++++++++++++++++++
 replay/replay-snapshot.c |   72 +++++++++++++++++++++++++
 replay/replay-time.c     |    2 -
 replay/replay.c          |   25 ++++++---
 stubs/replay.c           |    5 ++
 vl.c                     |   14 ++++-
 19 files changed, 618 insertions(+), 41 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 v3 1/8] 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 fca0487..fd7f17e 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 v3 2/8] 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..e90d2c6 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 = "blkreplay",
+    .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 fd7f17e..9adca19 100644
--- a/vl.c
+++ b/vl.c
@@ -4410,7 +4410,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 v3 3/8] 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 v3 4/8] 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.

v3 changes:
 - added rrsnapshot option

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 docs/replay.txt          |   12 ++++++++++++
 include/sysemu/replay.h  |    6 ++++++
 qemu-options.hx          |    8 +++++---
 replay/Makefile.objs     |    1 +
 replay/replay-internal.h |    3 +++
 replay/replay-snapshot.c |   31 +++++++++++++++++++++++++++++++
 replay/replay.c          |    7 +++++++
 vl.c                     |    8 +++++++-
 8 files changed, 72 insertions(+), 4 deletions(-)
 create mode 100644 replay/replay-snapshot.c

diff --git a/docs/replay.txt b/docs/replay.txt
index 5be8f25..ce3c6b8 100644
--- a/docs/replay.txt
+++ b/docs/replay.txt
@@ -204,6 +204,18 @@ Overlay file may be specified as follows:
  -drive driver=blkreplay,if=none,image=img-direct,
         overlay=overlay.qcow2,id=img-blkreplay
 
+Snapshotting
+------------
+
+New VM snapshots may be created in replay mode. They can be used later
+to recover the desired VM state. All VM states created in replay mode
+are associated with the moment of time in the replay scenario.
+After recovering the VM state replay will start from that position.
+
+Default starting snapshot name may be overridden with icount field
+rrsnapshot as follows:
+ -icount shift=7,rr=record,rrfile=replay.bin,rrsnapshot=non_default_snapshot
+
 Network devices
 ---------------
 
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/qemu-options.hx b/qemu-options.hx
index 0b621bb..1483ad8 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3367,12 +3367,12 @@ re-inject them.
 ETEXI
 
 DEF("icount", HAS_ARG, QEMU_OPTION_icount, \
-    "-icount [shift=N|auto][,align=on|off][,sleep=on|off,rr=record|replay,rrfile=<filename>]\n" \
+    "-icount [shift=N|auto][,align=on|off][,sleep=on|off,rr=record|replay,rrfile=<filename>,rrsnapshot=<snapshot>]\n" \
     "                enable virtual instruction counter with 2^N clock ticks per\n" \
     "                instruction, enable aligning the host and virtual clocks\n" \
     "                or disable real time cpu sleeping\n", QEMU_ARCH_ALL)
 STEXI
-@item -icount [shift=@var{N}|auto][,rr=record|replay,rrfile=@var{filename}]
+@item -icount [shift=@var{N}|auto][,rr=record|replay,rrfile=@var{filename},rrsnapshot=@var{snapshot}]
 @findex -icount
 Enable virtual instruction counter.  The virtual cpu will execute one
 instruction every 2^@var{N} ns of virtual time.  If @code{auto} is specified
@@ -3404,7 +3404,9 @@ when the shift value is high (how high depends on the host machine).
 
 When @option{rr} option is specified deterministic record/replay is enabled.
 Replay log is written into @var{filename} file in record mode and
-read from this file in replay mode.
+read from this file in replay mode. At the start of record new snapshot
+is created. It get 'replay_init' name or the one specified with @{snapshot}
+option. In replay mode this option is used to load the initial VM state.
 ETEXI
 
 DEF("watchdog", HAS_ARG, QEMU_OPTION_watchdog, \
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-internal.h b/replay/replay-internal.h
index d28cfb7..6f2f6e2 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -71,6 +71,9 @@ extern unsigned int replay_data_kind;
 /* File for replay writing */
 extern FILE *replay_file;
 
+/* Default name of the initial VM snapshot */
+extern char *replay_snapshot;
+
 void replay_put_byte(uint8_t byte);
 void replay_put_event(uint8_t event);
 void replay_put_word(uint16_t word);
diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
new file mode 100644
index 0000000..6079b29
--- /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_snapshot));
+        hmp_savevm(cur_mon, opts);
+        QDECREF(opts);
+    } else if (replay_mode == REPLAY_MODE_PLAY) {
+        load_vmstate(replay_snapshot);
+    }
+}
diff --git a/replay/replay.c b/replay/replay.c
index e040f6f..ef1e5e9 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -26,6 +26,7 @@
 #define HEADER_SIZE                 (sizeof(uint32_t) + sizeof(uint64_t))
 
 ReplayMode replay_mode = REPLAY_MODE_NONE;
+char *replay_snapshot;
 
 /* Name of replay file  */
 static char *replay_filename;
@@ -291,6 +292,8 @@ void replay_configure(QemuOpts *opts)
         exit(1);
     }
 
+    replay_snapshot = g_strdup(qemu_opt_get(opts, "rrsnapshot"));
+
     replay_enable(fname, mode);
 
 out:
@@ -343,6 +346,10 @@ void replay_finish(void)
         g_free(replay_filename);
         replay_filename = NULL;
     }
+    if (replay_snapshot) {
+        g_free(replay_snapshot);
+        replay_snapshot = NULL;
+    }
 
     replay_finish_events();
     replay_mutex_destroy();
diff --git a/vl.c b/vl.c
index 9adca19..c0d43f0 100644
--- a/vl.c
+++ b/vl.c
@@ -460,6 +460,10 @@ static QemuOptsList qemu_icount_opts = {
         }, {
             .name = "rrfile",
             .type = QEMU_OPT_STRING,
+        }, {
+            .name = "rrsnapshot",
+            .type = QEMU_OPT_STRING,
+            .def_value_str = "replay_init"
         },
         { /* end of list */ }
     },
@@ -4590,7 +4594,9 @@ int main(int argc, char **argv, char **envp)
     replay_checkpoint(CHECKPOINT_RESET);
     qemu_system_reset(VMRESET_SILENT);
     register_global_state();
-    if (loadvm) {
+    if (replay_mode != REPLAY_MODE_NONE) {
+        replay_vmstate_init();
+    } else if (loadvm) {
         if (load_vmstate(loadvm) < 0) {
             autostart = 0;
         }

[Qemu-devel] [PATCH v3 5/8] 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.

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

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 6f2f6e2..b586f88 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;
 
@@ -102,7 +104,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 ef1e5e9..c357be5 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -39,15 +39,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();
@@ -86,7 +86,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
@@ -189,7 +189,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;
         }
@@ -197,7 +197,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);
@@ -238,9 +238,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 v3 6/8] 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>
---
 replay/replay-internal.h |    9 +++++++++
 replay/replay-snapshot.c |   40 ++++++++++++++++++++++++++++++++++++++++
 replay/replay.c          |    1 +
 3 files changed, 50 insertions(+)

diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index b586f88..726e3d5 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;
 
@@ -170,4 +172,11 @@ void replay_event_net_save(void *opaque);
 /*! Reads network from the file. */
 void *replay_event_net_load(void);
 
+/* VMState-related functions */
+
+/* Registers replay VMState.
+   Should be called before virtual devices initialization
+   to make cached timers available for post_load functions. */
+void replay_vmstate_register(void);
+
 #endif
diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
index 6079b29..708e4bf 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/replay/replay.c b/replay/replay.c
index c357be5..e679f0d 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -295,6 +295,7 @@ void replay_configure(QemuOpts *opts)
 
     replay_snapshot = g_strdup(qemu_opt_get(opts, "rrsnapshot"));
 
+    replay_vmstate_register();
     replay_enable(fname, mode);
 
 out:

[Qemu-devel] [PATCH v3 7/8] 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 e90d2c6..e9a0d4d 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 e39ccb7..2f5684f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -751,6 +751,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 aa378ce..9984df4 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 726e3d5..f626101 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;
 
@@ -127,8 +131,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 708e4bf..2ee509d 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 c0d43f0..41aa27b 100644
--- a/vl.c
+++ b/vl.c
@@ -750,6 +750,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 v3 8/8] 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 v3 2/8] block: set snapshot option for block devices in blkreplay module

[Paolo Bonzini]

On 20/09/2016 14:31, Pavel Dovgalyuk wrote:
>      /* 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;
> +        }

No, this is unnecessary.  The image is unused in this case, so you can
specify the overlay as image=foo.qcow2.


Paolo

Re: [Qemu-devel] [PATCH v3 4/8] replay: save/load initial state

[Paolo Bonzini]

On 20/09/2016 14:31, Pavel Dovgalyuk wrote:
> @@ -291,6 +292,8 @@ void replay_configure(QemuOpts *opts)
>          exit(1);
>      }
>  
> +    replay_snapshot = g_strdup(qemu_opt_get(opts, "rrsnapshot"));
> +
>      replay_enable(fname, mode);
>  

Should you set snapshot = 1 here if there is no rrsnapshot option?

Paolo

Re: [Qemu-devel] [PATCH v3 4/8] replay: save/load initial state

[Pavel Dovgalyuk]

> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> On 20/09/2016 14:31, Pavel Dovgalyuk wrote:
> > @@ -291,6 +292,8 @@ void replay_configure(QemuOpts *opts)
> >          exit(1);
> >      }
> >
> > +    replay_snapshot = g_strdup(qemu_opt_get(opts, "rrsnapshot"));
> > +
> >      replay_enable(fname, mode);
> >
> 
> Should you set snapshot = 1 here if there is no rrsnapshot option?

No, because there is default snapshot name for the case when user 
specifies overlay for the drives.

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v3 2/8] block: set snapshot option for block devices in blkreplay module

[Pavel Dovgalyuk]

> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> On 20/09/2016 14:31, Pavel Dovgalyuk wrote:
> >      /* 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;
> > +        }
> 
> No, this is unnecessary.  The image is unused in this case, so you can
> specify the overlay as image=foo.qcow2.

This branch allows user do not bother about overlays at all.
Driver will automatically create temporary snapshot.

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v3 4/8] replay: save/load initial state

[Paolo Bonzini]

On 20/09/2016 14:39, Pavel Dovgalyuk wrote:
> > > +    replay_snapshot = g_strdup(qemu_opt_get(opts, "rrsnapshot"));
> > > +
> > >      replay_enable(fname, mode);
> > >
> > 
> > Should you set snapshot = 1 here if there is no rrsnapshot option?
> 
> No, because there is default snapshot name for the case when user 
> specifies overlay for the drives.

There are three possibilities:

a) these patches:
   with implicit overlay:
      -drive file=disk.raw,if=none,id=img-direct
      -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay

   with explicit overlay:
     -drive file=disk.raw,if=none,id=img-direct
     -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay,overlay=foo.qcow2

   Advantages:
   - does the right thing in the "implicit overlay" case.

   Disadvantages:
   - no need really to specify disk.raw in the "explicit overlay" case, since
     it's already specified when you create the overlay with qemu-img.

   Vote for implicit overlay: excellent
   Vote for explicit overlay: bad (need to track two file names)

b) always specify -snapshot manually:
   with implicit overlay:
      -drive file=disk.raw,if=none,id=img-direct -snapshot
      -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay

   with explicit overlay:
     -drive file=foo.qcow2,if=none,id=img-direct
     -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay

   Advantages:
   - can use default snapshot name in the "explicit overlay" case

   Disadvantages:
   - need to specify -snapshot explicitly for the "implicit overlay" case

   Vote for implicit overlay: awful (disk.raw destroy if you forget -snapshot)
   Vote for explicit overlay: excellent

c) no rrsnapshot implies -snapshot:
   without overlay:
      -drive file=disk.raw,if=none,id=img-direct
      -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay

   with overlay:
     -drive file=foo.qcow2,if=none,id=img-direct
     -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
     -icount ...,rrsnapshot=snapname

   Advantages:
   - does the right thing in the "implicit overlay" case.

   Disadvantages:
   - always have to specify snapshot name in the "explicit overlay" case

   Vote for implicit overlay: excellent (same as a)
   Vote for explicit overlay: decent

We have to choose between (a) and (c) I think, because the risk of corruption
for (b) is too high.  I prefer (c).  Kevin, what do you think?

Thanks,

Paolo

Re: [Qemu-devel] [PATCH v3 2/8] block: set snapshot option for block devices in blkreplay module

[Paolo Bonzini]

On 20/09/2016 14:43, Pavel Dovgalyuk wrote:
> > No, this is unnecessary.  The image is unused in this case, so you can
> > specify the overlay as image=foo.qcow2.
>
> This branch allows user do not bother about overlays at all.
> Driver will automatically create temporary snapshot.

See other message.  There is already a way to do this in QEMU, which is
-snapshot.  I think it's a requirement that -snapshot works, and also
that your temporary overlay is implemented on top of -snapshot.

You can add some magic ways to enable -snapshot if you want, of course.

Paolo

Re: [Qemu-devel] [PATCH v3 0/8] replay additions

[no-reply]

Hi,

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

Message-id: 20160920123126.5400.29283.stgit@PASHA-ISP.def.inno
Subject: [Qemu-devel] [PATCH v3 0/8] replay additions
Type: series

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

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

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
1e5f827 integratorcp: adding vmstate for save/restore
ce7f8d8 replay: allow replay stopping and restarting
71b30fd replay: vmstate for replay module
a4dceb0 replay: move internal data to the structure
4a54806 replay: save/load initial state
3a8d660 block: don't make snapshots for filters
9c93c5a block: set snapshot option for block devices in blkreplay module
b84caf1 record/replay: add network support

=== OUTPUT BEGIN ===
Checking PATCH 1/8: record/replay: add network support...
WARNING: line over 80 characters
#122: FILE: net/filter-replay.c:35:
+static ssize_t filter_replay_receive_iov(NetFilterState *nf, NetClientState *sndr,

WARNING: line over 80 characters
#123: FILE: net/filter-replay.c:36:
+                                         unsigned flags, const struct iovec *iov,

WARNING: line over 80 characters
#296: FILE: replay/replay-net.c:42:
+                                network_filters_count * sizeof(*network_filters));

total: 0 errors, 3 warnings, 313 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/8: block: set snapshot option for block devices in blkreplay module...
Checking PATCH 3/8: block: don't make snapshots for filters...
Checking PATCH 4/8: replay: save/load initial state...
ERROR: g_free(NULL) is safe this check is probably not required
#169: FILE: replay/replay.c:350:
+    if (replay_snapshot) {
+        g_free(replay_snapshot);

total: 1 errors, 0 warnings, 141 lines checked

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

Checking PATCH 5/8: replay: move internal data to the structure...
WARNING: line over 80 characters
#62: FILE: replay/replay-internal.c:158:
+                error_report("Replay: unknown event kind %d", replay_state.data_kind);

total: 0 errors, 1 warnings, 133 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 6/8: replay: vmstate for replay module...
Checking PATCH 7/8: replay: allow replay stopping and restarting...
Checking PATCH 8/8: integratorcp: adding vmstate for save/restore...
=== OUTPUT END ===

Test command exited with code: 1


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

Re: [Qemu-devel] [PATCH v3 4/8] replay: save/load initial state

[Pavel Dovgalyuk]

> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> On 20/09/2016 14:39, Pavel Dovgalyuk wrote:
> > > > +    replay_snapshot = g_strdup(qemu_opt_get(opts, "rrsnapshot"));
> > > > +
> > > >      replay_enable(fname, mode);
> > > >
> > >
> > > Should you set snapshot = 1 here if there is no rrsnapshot option?
> >
> > No, because there is default snapshot name for the case when user
> > specifies overlay for the drives.
> 
> There are three possibilities:
> 
> a) these patches:
>    with implicit overlay:
>       -drive file=disk.raw,if=none,id=img-direct
>       -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
> 
>    with explicit overlay:
>      -drive file=disk.raw,if=none,id=img-direct
>      -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay,overlay=foo.qcow2
> 
>    Advantages:
>    - does the right thing in the "implicit overlay" case.

     - automatically creates overlay

> 
>    Disadvantages:
>    - no need really to specify disk.raw in the "explicit overlay" case, since
>      it's already specified when you create the overlay with qemu-img.
> 
>    Vote for implicit overlay: excellent
>    Vote for explicit overlay: bad (need to track two file names)

Disadvantage is for replay only. Running QEMU in record mode automatically
creates overlay. Therefore two filenames are required.

> c) no rrsnapshot implies -snapshot:
>    without overlay:
>       -drive file=disk.raw,if=none,id=img-direct
>       -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
> 
>    with overlay:
>      -drive file=foo.qcow2,if=none,id=img-direct
>      -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
>      -icount ...,rrsnapshot=snapname

But how record will create this overlay?
This method requires creating overlay manually, because backing file is
not specified at all.


Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v3 4/8] replay: save/load initial state

[Paolo Bonzini]

On 20/09/2016 15:37, Pavel Dovgalyuk wrote:
>> c) no rrsnapshot implies -snapshot:
>>    without overlay:
>>       -drive file=disk.raw,if=none,id=img-direct
>>       -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
>>
>>    with overlay:
>>      -drive file=foo.qcow2,if=none,id=img-direct
>>      -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
>>      -icount ...,rrsnapshot=snapname
> 
> But how record will create this overlay?
> This method requires creating overlay manually, because backing file is
> not specified at all.

You create it manually, or you just use a .qcow2 file to begin with for
your image.  Then:

1) if you specify no snapshot, a temporary .qcow2 file is created on top
so data is not destroyed

2) if you specify a snapshot, that snapshot is preserved (so you don't
lose the base state even though the file changes)

Paolo

Re: [Qemu-devel] [PATCH v3 4/8] replay: save/load initial state

[Pavel Dovgalyuk]

> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> On 20/09/2016 15:37, Pavel Dovgalyuk wrote:
> >> c) no rrsnapshot implies -snapshot:
> >>    without overlay:
> >>       -drive file=disk.raw,if=none,id=img-direct
> >>       -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
> >>
> >>    with overlay:
> >>      -drive file=foo.qcow2,if=none,id=img-direct
> >>      -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
> >>      -icount ...,rrsnapshot=snapname
> >
> > But how record will create this overlay?
> > This method requires creating overlay manually, because backing file is
> > not specified at all.
> 
> You create it manually, or you just use a .qcow2 file to begin with for
> your image.  Then:
> 
> 1) if you specify no snapshot, a temporary .qcow2 file is created on top
> so data is not destroyed
> 
> 2) if you specify a snapshot, that snapshot is preserved (so you don't
> lose the base state even though the file changes)

Now I see.
This seems ok, but:
 - this approach adds some garbage to original disk image
 - won't work with raw images

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v3 4/8] replay: save/load initial state

[Paolo Bonzini]

On 20/09/2016 15:51, Pavel Dovgalyuk wrote:
>> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
>> On 20/09/2016 15:37, Pavel Dovgalyuk wrote:
>>>> c) no rrsnapshot implies -snapshot:
>>>>    without overlay:
>>>>       -drive file=disk.raw,if=none,id=img-direct
>>>>       -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
>>>>
>>>>    with overlay:
>>>>      -drive file=foo.qcow2,if=none,id=img-direct
>>>>      -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
>>>>      -icount ...,rrsnapshot=snapname
>>>
>>> But how record will create this overlay?
>>> This method requires creating overlay manually, because backing file is
>>> not specified at all.
>>
>> You create it manually, or you just use a .qcow2 file to begin with for
>> your image.  Then:
>>
>> 1) if you specify no snapshot, a temporary .qcow2 file is created on top
>> so data is not destroyed
>>
>> 2) if you specify a snapshot, that snapshot is preserved (so you don't
>> lose the base state even though the file changes)
> 
> Now I see.
> This seems ok, but:
>  - this approach adds some garbage to original disk image
>  - won't work with raw images

Yes, for raw images or if you want to keep the pristine image you have
to do a "qemu-img create -f qcow2 -b disk.raw foo.qcow2".

Paolo

Re: [Qemu-devel] [PATCH v3 2/8] block: set snapshot option for block devices in blkreplay module

[Kevin Wolf]

[ Cc: qemu-block ]

Am 20.09.2016 um 14:31 hat Pavel Dovgalyuk geschrieben:
> 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..e90d2c6 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 = "blkreplay",
> +    .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;
> +    }

Starting from here...

> +    /* 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;
> +    }

...to here, this is code that is written according to a fundamentally
wrong design.

The problem here is that you're hard-coding the options that are used
for the overlay image. This restricts users to using only qcow2 (there
are many other formats that support backing files), only on local files
(there are quite a few more protocols over which qcow2 works), only with
default options (e.g. cache mode, discard behaviour, lazy refcounts).

The correct way would be to get not a filename option, but what is
called a BlockdevRef in QAPI: Either a node-name of a separately created
BDS or an inline definition of a new BDS. Have a look at other filter
drivers for how to do this. The thing to look for is bdrv_open_child()
(though of course an overlay may look a bit different from this).

> +
>      ret = 0;
>  fail:
>      if (ret < 0) {

Kevin

Re: [Qemu-devel] [PATCH v3 2/8] block: set snapshot option for block devices in blkreplay module

[Pavel Dovgalyuk]

> From: Kevin Wolf [mailto:kwolf@redhat.com]
> Am 20.09.2016 um 14:31 hat Pavel Dovgalyuk geschrieben:
> > 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..e90d2c6 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 = "blkreplay",
> > +    .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;
> > +    }
> 
> Starting from here...
> 
> > +    /* 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;
> > +    }
> 
> ...to here, this is code that is written according to a fundamentally
> wrong design.
> 
> The problem here is that you're hard-coding the options that are used
> for the overlay image. This restricts users to using only qcow2 (there
> are many other formats that support backing files), only on local files
> (there are quite a few more protocols over which qcow2 works), only with
> default options (e.g. cache mode, discard behaviour, lazy refcounts).
> 
> The correct way would be to get not a filename option, but what is
> called a BlockdevRef in QAPI: Either a node-name of a separately created
> BDS or an inline definition of a new BDS. Have a look at other filter
> drivers for how to do this. The thing to look for is bdrv_open_child()
> (though of course an overlay may look a bit different from this).

Thanks.
Then I'll better apply Paolo's approach to creating overlays.

Pavel Dovgalyuk