replay additions

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.

v5 changes:
- using iov_to_buf function instead of loop

Signed-off-by Pavel Dovgalyuk<[email protected]>

---

Quoted 15 lines · 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.

Quoted 5 lines

v5 changes:
- using iov_to_buf function instead of loop

Signed-off-by Pavel Dovgalyuk<[email protected]>

---

Reviewed-by Jason Wang <[email protected]>

Quoted 22 lines · Jason Wang

Quoted 20 lines · 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.

v5 changes:
- using iov_to_buf function instead of loop

Signed-off-by Pavel Dovgalyuk<[email protected]>

---

Quoted 1 line

Reviewed-by Jason Wang <[email protected]>

Jason,

please merge this yourself through the net tree.

Thanks,

Paolo

 {
     BlockDriverState *bs, *bs1;
     QEMUSnapshotInfo sn1, *sn = &sn1, old_sn1, *old_sn = &old_sn1;

migration/savevm.c +11 -9

-    int ret;
+    int ret = -1;
     QEMUFile *f;
     int saved_vm_running;
     uint64_t vm_state_size;
     qemu_timeval tv;
     struct tm tm;
-    const char *name = qdict_get_try_str(qdict, "name");
     Error *local_err = NULL;
     AioContext *aio_context;
 
     if (!bdrv_all_can_snapshot(&bs)) {
         monitor_printf(mon, "Device '%s' is writable but does not "
                        "support snapshots.\n", bdrv_get_device_name(bs));
-        return;
+        return ret;
     }
 
     /* Delete old snapshots of the same name */
-    if (name && bdrv_all_delete_snapshot(name, &bs1, &local_err) < 0) {
-        error_reportf_err(local_err,
-                          "Error while deleting snapshot on device '%s': ",
-                          bdrv_get_device_name(bs1));
-        return;
+    if (name) {
+        ret = bdrv_all_delete_snapshot(name, &bs1, &local_err);
+        if (ret < 0) {
+            error_reportf_err(local_err,
+                              "Error while deleting snapshot on device '%s': ",
+                              bdrv_get_device_name(bs1));
+            return ret;
+        }
     }
 
     bs = bdrv_all_find_vmstate_bs();
     if (bs == NULL) {
         monitor_printf(mon, "No block device can accept snapshots\n");
-        return;
+        return ret;
     }

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 <[email protected]>

 block/snapshot.c |    3 +++
 1 file changed, 3 insertions(+)

block/snapshot.c +0 -0

index bf5c2ca..8998b8b 100644

From: Kevin Wolf [mailto:[email protected]]
Am 26.09.2016 um 10:08 hat Pavel Dovgalyuk geschrieben:

Quoted 5 lines

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 <[email protected]>

But that's exactly what the existing code implements? If a driver doesn't provide .bdrv_snapshot_goto, the request is redirected to bs->file.

Quoted 3 lines

 block/snapshot.c |    3 +++
 1 file changed, 3 insertions(+)

block/snapshot.c int bdrv_snapshot_goto(BlockDriverState *bs, · line 189

Quoted 11 lines

block/snapshot.c +3 -0

index bf5c2ca..8998b8b 100644
@@ -184,6 +184,9 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
     if (!drv) {
         return -ENOMEDIUM;
     }
+    if (drv->is_filter) {
+        return 0;
+    }

This, on the other hand, doesn't redirect the request, but silently ignores it. That is, loading the snapshot will apparently succeed, but it wouldn't actually load anything and the disk would stay in its current state.

block/snapshot.c int bdrv_snapshot_goto(BlockDriverState *bs, · line 192

Quoted 4 lines

block/snapshot.c +0 -0

     if (drv->bdrv_snapshot_goto) {
         return drv->bdrv_snapshot_goto(bs, snapshot_id);
     }

Quoted 15 lines

From: Kevin Wolf [mailto:[email protected]]
Am 26.09.2016 um 10:08 hat Pavel Dovgalyuk geschrieben:

Quoted 5 lines

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 <[email protected]>

But that's exactly what the existing code implements? If a driver doesn't provide .bdrv_snapshot_goto, the request is redirected to bs->file.

Quoted 3 lines

 block/snapshot.c |    3 +++
 1 file changed, 3 insertions(+)

Quoted 16 lines

block/snapshot.c int bdrv_snapshot_goto(BlockDriverState *bs, · line 189

Quoted 11 lines

block/snapshot.c +3 -0

index bf5c2ca..8998b8b 100644
@@ -184,6 +184,9 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
     if (!drv) {
         return -ENOMEDIUM;
     }
+    if (drv->is_filter) {
+        return 0;
+    }

This, on the other hand, doesn't redirect the request, but silently ignores it. That is, loading the snapshot will apparently succeed, but it wouldn't actually load anything and the disk would stay in its current state.

In my use case bdrv_all_goto_snapshot iterates all block drivers, including filters and disk images. Therefore skipping goto for images is ok.

From: Kevin Wolf [mailto:[email protected]]
Am 26.09.2016 um 11:51 hat Pavel Dovgalyuk geschrieben:

Quoted 32 lines

Quoted 15 lines

From: Kevin Wolf [mailto:[email protected]]
Am 26.09.2016 um 10:08 hat Pavel Dovgalyuk geschrieben:

Quoted 5 lines

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 <[email protected]>

But that's exactly what the existing code implements? If a driver doesn't provide .bdrv_snapshot_goto, the request is redirected to bs->file.

Quoted 3 lines

 block/snapshot.c |    3 +++
 1 file changed, 3 insertions(+)

Quoted 16 lines

block/snapshot.c int bdrv_snapshot_goto(BlockDriverState *bs, · line 189

Quoted 11 lines

block/snapshot.c +3 -0

index bf5c2ca..8998b8b 100644
@@ -184,6 +184,9 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
     if (!drv) {
         return -ENOMEDIUM;
     }
+    if (drv->is_filter) {
+        return 0;
+    }

This, on the other hand, doesn't redirect the request, but silently ignores it. That is, loading the snapshot will apparently succeed, but it wouldn't actually load anything and the disk would stay in its current state.

Quoted 2 lines

In my use case bdrv_all_goto_snapshot iterates all block drivers, including filters and disk images. Therefore skipping goto for images is ok.

Hm, this can happy today indeed.

Originally, we only called bdrv_goto_snapshot() for all _top level_ BDSes, and this is still what you normally get. However, if you explicitly create a BDS (e.g. with its own -drive option), it is considered a top level BDS without actually being top level for the guest, and therefore the snapshotting function is called for it.

Of course, this is highly inefficient because the goto_snapshot request is passed by the filter driver and then called another time for the lower node, effectively loading the snapshot a second time.

On the other hand if you use a single -drive option to create both the qcow2 BDS and the blkreplay filter, we do need to pass down the goto_snapshot request because it won't be called for the qcow2 layer otherwise.

I'm not completely sure yet what the right behaviour would be here.

Quoted 49 lines

From: Kevin Wolf [mailto:[email protected]]
Am 26.09.2016 um 11:51 hat Pavel Dovgalyuk geschrieben:

Quoted 32 lines

Quoted 15 lines

From: Kevin Wolf [mailto:[email protected]]
Am 26.09.2016 um 10:08 hat Pavel Dovgalyuk geschrieben:

Quoted 5 lines

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 <[email protected]>

But that's exactly what the existing code implements? If a driver doesn't provide .bdrv_snapshot_goto, the request is redirected to bs->file.

Quoted 3 lines

 block/snapshot.c |    3 +++
 1 file changed, 3 insertions(+)

Quoted 16 lines

block/snapshot.c int bdrv_snapshot_goto(BlockDriverState *bs, · line 189

Quoted 11 lines

block/snapshot.c +3 -0

index bf5c2ca..8998b8b 100644
@@ -184,6 +184,9 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
     if (!drv) {
         return -ENOMEDIUM;
     }
+    if (drv->is_filter) {
+        return 0;
+    }

This, on the other hand, doesn't redirect the request, but silently ignores it. That is, loading the snapshot will apparently succeed, but it wouldn't actually load anything and the disk would stay in its current state.

Quoted 2 lines

In my use case bdrv_all_goto_snapshot iterates all block drivers, including filters and disk images. Therefore skipping goto for images is ok.

Hm, this can happy today indeed.

Originally, we only called bdrv_goto_snapshot() for all _top level_ BDSes, and this is still what you normally get. However, if you explicitly create a BDS (e.g. with its own -drive option), it is considered a top level BDS without actually being top level for the guest, and therefore the snapshotting function is called for it.

Of course, this is highly inefficient because the goto_snapshot request is passed by the filter driver and then called another time for the lower node, effectively loading the snapshot a second time.

Quoted 4 lines

On the other hand if you use a single -drive option to create both the qcow2 BDS and the blkreplay filter, we do need to pass down the goto_snapshot request because it won't be called for the qcow2 layer otherwise.

How this can be specified in command line? I believed that separate -drive option is required.

From: Kevin Wolf [mailto:[email protected]]
Am 27.09.2016 um 16:06 hat Pavel Dovgalyuk geschrieben:

Quoted 13 lines

Quoted 13 lines

From: Kevin Wolf [mailto:[email protected]]
Am 26.09.2016 um 11:51 hat Pavel Dovgalyuk geschrieben:

Quoted 2 lines

Quoted 2 lines

From: Kevin Wolf [mailto:[email protected]]
Am 26.09.2016 um 10:08 hat Pavel Dovgalyuk geschrieben:

Originally, we only called bdrv_goto_snapshot() for all _top level_ BDSes, and this is still what you normally get. However, if you explicitly create a BDS (e.g. with its own -drive option), it is considered a top level BDS without actually being top level for the guest, and therefore the snapshotting function is called for it.

Of course, this is highly inefficient because the goto_snapshot request is passed by the filter driver and then called another time for the lower node, effectively loading the snapshot a second time.

Quoted 6 lines

Quoted 5 lines

On the other hand if you use a single -drive option to create both the qcow2 BDS and the blkreplay filter, we do need to pass down the goto_snapshot request because it won't be called for the qcow2 layer otherwise.

Quoted 2 lines

How this can be specified in command line? I believed that separate -drive option is required.

Something like this:

-drive driver=blkreplay,image.driver=file,image.filename=test.img

Quoted 15 lines

From: Kevin Wolf [mailto:[email protected]]
Am 27.09.2016 um 16:06 hat Pavel Dovgalyuk geschrieben:

Quoted 13 lines

Quoted 13 lines

From: Kevin Wolf [mailto:[email protected]]
Am 26.09.2016 um 11:51 hat Pavel Dovgalyuk geschrieben:

Quoted 2 lines

Quoted 2 lines

From: Kevin Wolf [mailto:[email protected]]
Am 26.09.2016 um 10:08 hat Pavel Dovgalyuk geschrieben:

Originally, we only called bdrv_goto_snapshot() for all _top level_ BDSes, and this is still what you normally get. However, if you explicitly create a BDS (e.g. with its own -drive option), it is considered a top level BDS without actually being top level for the guest, and therefore the snapshotting function is called for it.

Of course, this is highly inefficient because the goto_snapshot request is passed by the filter driver and then called another time for the lower node, effectively loading the snapshot a second time.

Maybe double-saving/loading does the smallest damage then? And we should just document how to use blkreplay effectively?

Quoted 6 lines

Quoted 6 lines

Quoted 5 lines

On the other hand if you use a single -drive option to create both the qcow2 BDS and the blkreplay filter, we do need to pass down the goto_snapshot request because it won't be called for the qcow2 layer otherwise.

Quoted 7 lines

Quoted 2 lines

How this can be specified in command line? I believed that separate -drive option is required.

Something like this:

-drive driver=blkreplay,image.driver=file,image.filename=test.img

I tried the following command line, but VM does not detect the hard drive and cannot boot.

From: Kevin Wolf [mailto:[email protected]]
Am 28.09.2016 um 11:32 hat Pavel Dovgalyuk geschrieben:

Quoted 30 lines

Quoted 15 lines

From: Kevin Wolf [mailto:[email protected]]
Am 27.09.2016 um 16:06 hat Pavel Dovgalyuk geschrieben:

Quoted 13 lines

Quoted 13 lines

From: Kevin Wolf [mailto:[email protected]]
Am 26.09.2016 um 11:51 hat Pavel Dovgalyuk geschrieben:

Quoted 2 lines

Quoted 2 lines

From: Kevin Wolf [mailto:[email protected]]
Am 26.09.2016 um 10:08 hat Pavel Dovgalyuk geschrieben:

Originally, we only called bdrv_goto_snapshot() for all _top level_ BDSes, and this is still what you normally get. However, if you explicitly create a BDS (e.g. with its own -drive option), it is considered a top level BDS without actually being top level for the guest, and therefore the snapshotting function is called for it.

Of course, this is highly inefficient because the goto_snapshot request is passed by the filter driver and then called another time for the lower node, effectively loading the snapshot a second time.

Maybe double-saving/loading does the smallest damage then? And we should just document how to use blkreplay effectively?

Quoted 11 lines

Quoted 8 lines

Quoted 5 lines

On the other hand if you use a single -drive option to create both the qcow2 BDS and the blkreplay filter, we do need to pass down the goto_snapshot request because it won't be called for the qcow2 layer otherwise.

How this can be specified in command line? I believed that separate -drive option is required.

Something like this:

Quoted 7 lines

Quoted 2 lines

-drive driver=blkreplay,image.driver=file,image.filename=test.img

I tried the following command line, but VM does not detect the hard drive and cannot boot.

-drive driver=blkreplay,if=none,image.driver=file,image.filename=testdisk.qcow,id=img-

blkreplay

Quoted 1 line

-device ide-hd,drive=img-blkreplay

My command line was assuming a raw image. It looks like you're using a qcow (hopefully qcow2?) image. If so, then you need to include the qcow2 driver:

-drive driver=blkreplay,if=none,image.driver=qcow2,\
image.file.driver=file,image.file.filename=testdisk.qcow,id=img-blkreplay

Quoted 46 lines

From: Kevin Wolf [mailto:[email protected]]
Am 28.09.2016 um 11:32 hat Pavel Dovgalyuk geschrieben:

Quoted 30 lines

Quoted 15 lines

From: Kevin Wolf [mailto:[email protected]]
Am 27.09.2016 um 16:06 hat Pavel Dovgalyuk geschrieben:

Quoted 13 lines

Quoted 13 lines

From: Kevin Wolf [mailto:[email protected]]
Am 26.09.2016 um 11:51 hat Pavel Dovgalyuk geschrieben:

Quoted 2 lines

Quoted 2 lines

From: Kevin Wolf [mailto:[email protected]]
Am 26.09.2016 um 10:08 hat Pavel Dovgalyuk geschrieben:

Originally, we only called bdrv_goto_snapshot() for all _top level_ BDSes, and this is still what you normally get. However, if you explicitly create a BDS (e.g. with its own -drive option), it is considered a top level BDS without actually being top level for the guest, and therefore the snapshotting function is called for it.

Of course, this is highly inefficient because the goto_snapshot request is passed by the filter driver and then called another time for the lower node, effectively loading the snapshot a second time.

Maybe double-saving/loading does the smallest damage then? And we should just document how to use blkreplay effectively?

Quoted 11 lines

Quoted 8 lines

Quoted 5 lines

On the other hand if you use a single -drive option to create both the qcow2 BDS and the blkreplay filter, we do need to pass down the goto_snapshot request because it won't be called for the qcow2 layer otherwise.

How this can be specified in command line? I believed that separate -drive option is required.

Something like this:

Quoted 7 lines

Quoted 2 lines

-drive driver=blkreplay,image.driver=file,image.filename=test.img

I tried the following command line, but VM does not detect the hard drive and cannot boot.

-drive driver=blkreplay,if=none,image.driver=file,image.filename=testdisk.qcow,id=img-

blkreplay

Quoted 1 line

-device ide-hd,drive=img-blkreplay

My command line was assuming a raw image. It looks like you're using a qcow (hopefully qcow2?) image. If so, then you need to include the qcow2 driver:

Quoted 2 lines

-drive driver=blkreplay,if=none,image.driver=qcow2,\
image.file.driver=file,image.file.filename=testdisk.qcow,id=img-blkreplay

This doesn't work for some reason. Replay just hangs at some moment.

Maybe there exists some internal difference between command line with one or two -drive options?

From: Pavel Dovgalyuk [mailto:[email protected]]

Quoted 46 lines

From: Kevin Wolf [mailto:[email protected]]
Am 28.09.2016 um 11:32 hat Pavel Dovgalyuk geschrieben:

Quoted 30 lines

Quoted 15 lines

From: Kevin Wolf [mailto:[email protected]]
Am 27.09.2016 um 16:06 hat Pavel Dovgalyuk geschrieben:

Quoted 13 lines

Quoted 13 lines

From: Kevin Wolf [mailto:[email protected]]
Am 26.09.2016 um 11:51 hat Pavel Dovgalyuk geschrieben:

Quoted 2 lines

Quoted 2 lines

From: Kevin Wolf [mailto:[email protected]]
Am 26.09.2016 um 10:08 hat Pavel Dovgalyuk geschrieben:

Originally, we only called bdrv_goto_snapshot() for all _top level_ BDSes, and this is still what you normally get. However, if you explicitly create a BDS (e.g. with its own -drive option), it is considered a top level BDS without actually being top level for the guest, and therefore the snapshotting function is called for it.

Of course, this is highly inefficient because the goto_snapshot request is passed by the filter driver and then called another time for the lower node, effectively loading the snapshot a second time.

Maybe double-saving/loading does the smallest damage then? And we should just document how to use blkreplay effectively?

Quoted 11 lines

Quoted 8 lines

Quoted 5 lines

On the other hand if you use a single -drive option to create both the qcow2 BDS and the blkreplay filter, we do need to pass down the goto_snapshot request because it won't be called for the qcow2 layer otherwise.

How this can be specified in command line? I believed that separate -drive option is required.

Something like this:

Quoted 7 lines

Quoted 2 lines

-drive driver=blkreplay,image.driver=file,image.filename=test.img

I tried the following command line, but VM does not detect the hard drive and cannot boot.

-drive driver=blkreplay,if=none,image.driver=file,image.filename=testdisk.qcow,id=img-

blkreplay

Quoted 1 line

-device ide-hd,drive=img-blkreplay

My command line was assuming a raw image. It looks like you're using a qcow (hopefully qcow2?) image. If so, then you need to include the qcow2 driver:

Quoted 2 lines

-drive driver=blkreplay,if=none,image.driver=qcow2,\
image.file.driver=file,image.file.filename=testdisk.qcow,id=img-blkreplay

This doesn't work for some reason. Replay just hangs at some moment.

Maybe there exists some internal difference between command line with one or two -drive options?

I've investigated this issue.
This command line works ok:
-drive driver=blkreplay,if=none,image.driver=file,image.filename=testdisk.qcow,id=img-blkreplay -device ide-hd,drive=img-blkreplay

And this does not:
-drive
driver=blkreplay,if=none,image.driver=qcow2,image.file.driver=file,image.file.filename=testdisk.qcow ,id=img-blkreplay
-device ide-hd,drive=img-blkreplay

QEMU hangs at some moment of replay.

I found that some dma requests do not pass through the blkreplay driver due to the following line in block-backend.c:

    return bdrv_co_preadv(blk->root, offset, bytes, qiov, flags);

This line passes read request directly to qcow driver and blkreplay cannot process it to make deterministic.

From: Paolo Bonzini [mailto:[email protected]]

Quoted 9 lines

I've investigated this issue.
This command line works ok:
-drive driver=blkreplay,if=none,image.driver=file,image.filename=testdisk.qcow,id=img-blkreplay -device ide-hd,drive=img-blkreplay

And this does not:
-drive

driver=blkreplay,if=none,image.driver=qcow2,image.file.driver=file,image.file.filename=testdis k.qcow

Quoted 5 lines

,id=img-blkreplay
-device ide-hd,drive=img-blkreplay

QEMU hangs at some moment of replay.

Quoted 6 lines

I found that some dma requests do not pass through the blkreplay driver due to the following line in block-backend.c:

    return bdrv_co_preadv(blk->root, offset, bytes, qiov, flags);

This line passes read request directly to qcow driver and blkreplay cannot process it to make deterministic.

I don't understand, blk->root should be the blkreplay here.

Quoted 6 lines

From: Pavel Dovgalyuk [mailto:[email protected]]

Quoted 5 lines

From: Kevin Wolf [mailto:[email protected]]
My command line was assuming a raw image. It looks like you're using a qcow (hopefully qcow2?) image. If so, then you need to include the qcow2 driver:

Quoted 7 lines

Quoted 2 lines

-drive driver=blkreplay,if=none,image.driver=qcow2,\
image.file.driver=file,image.file.filename=testdisk.qcow,id=img-blkreplay

This doesn't work for some reason. Replay just hangs at some moment.

Maybe there exists some internal difference between command line with one or two -drive options?

I've investigated this issue.
This command line works ok:
-drive driver=blkreplay,if=none,image.driver=file,image.filename=testdisk.qcow,id=img-blkreplay -device ide-hd,drive=img-blkreplay

And this does not:
-drive
driver=blkreplay,if=none,image.driver=qcow2,image.file.driver=file,image.file.filename=testdisk.qcow ,id=img-blkreplay
-device ide-hd,drive=img-blkreplay

QEMU hangs at some moment of replay.

I found that some dma requests do not pass through the blkreplay driver due to the following line in block-backend.c:

    return bdrv_co_preadv(blk->root, offset, bytes, qiov, flags);

This line passes read request directly to qcow driver and blkreplay cannot process it to make deterministic.

From: Kevin Wolf [mailto:[email protected]]
Am 16.11.2016 um 10:49 hat Pavel Dovgalyuk geschrieben:

Quoted 5 lines

Quoted 1 line

From: Pavel Dovgalyuk [mailto:[email protected]]

I've investigated this issue.
This command line works ok:
-drive driver=blkreplay,if=none,image.driver=file,image.filename=testdisk.qcow,id=img-

blkreplay

Quoted 5 lines

-device ide-hd,drive=img-blkreplay

And this does not:
-drive

driver=blkreplay,if=none,image.driver=qcow2,image.file.driver=file,image.file.filename=testdis k.qcow

Quoted 9 lines

,id=img-blkreplay
-device ide-hd,drive=img-blkreplay

QEMU hangs at some moment of replay.

I found that some dma requests do not pass through the blkreplay driver due to the following line in block-backend.c:

    return bdrv_co_preadv(blk->root, offset, bytes, qiov, flags);

Quoted 2 lines

This line passes read request directly to qcow driver and blkreplay cannot process it to make deterministic.

How does that bypass blkreplay? blk->root is supposed to be the blkreply node, do you see something different? If it were the qcow2 node, then I would expect that no requests at all go through the blkreplay layer.

Quoted 24 lines

From: Kevin Wolf [mailto:[email protected]]
Am 16.11.2016 um 10:49 hat Pavel Dovgalyuk geschrieben:

Quoted 5 lines

Quoted 1 line

From: Pavel Dovgalyuk [mailto:[email protected]]

I've investigated this issue.
This command line works ok:
-drive driver=blkreplay,if=none,image.driver=file,image.filename=testdisk.qcow,id=img-

blkreplay

Quoted 5 lines

-device ide-hd,drive=img-blkreplay

And this does not:
-drive

driver=blkreplay,if=none,image.driver=qcow2,image.file.driver=file,image.file.filename=testdis k.qcow

Quoted 9 lines

,id=img-blkreplay
-device ide-hd,drive=img-blkreplay

QEMU hangs at some moment of replay.

I found that some dma requests do not pass through the blkreplay driver due to the following line in block-backend.c:

    return bdrv_co_preadv(blk->root, offset, bytes, qiov, flags);

Quoted 6 lines

Quoted 2 lines

This line passes read request directly to qcow driver and blkreplay cannot process it to make deterministic.

How does that bypass blkreplay? blk->root is supposed to be the blkreply node, do you see something different? If it were the qcow2 node, then I would expect that no requests at all go through the blkreplay layer.

It seems, that the problem is in -snapshot option.
We have one of the following block layers depending on command line:
tmp_overlay1 -> blkreplay -> tmp_overlay2 -> disk_image tmp_overlay1 -> blkreplay -> disk_image

But the correct scheme is intended to be the following:
blkreplay -> tmp_overlay1 -> disk_image

How can we fix it?
Maybe we should add blkreplay automatically to all block devices and not to specify it in the command line?

From: Kevin Wolf [mailto:[email protected]]
Am 21.11.2016 um 13:22 hat Pavel Dovgalyuk geschrieben:

Quoted 7 lines

Quoted 7 lines

From: Kevin Wolf [mailto:[email protected]]
Am 16.11.2016 um 10:49 hat Pavel Dovgalyuk geschrieben:

Quoted 5 lines

Quoted 1 line

From: Pavel Dovgalyuk [mailto:[email protected]]

I've investigated this issue.
This command line works ok:
-drive driver=blkreplay,if=none,image.driver=file,image.filename=testdisk.qcow,id=img-

Quoted 7 lines

Quoted 7 lines

blkreplay

Quoted 5 lines

-device ide-hd,drive=img-blkreplay

And this does not:
-drive

driver=blkreplay,if=none,image.driver=qcow2,image.file.driver=file,image.file.filename=testdis

Quoted 22 lines

Quoted 10 lines

k.qcow

Quoted 9 lines

,id=img-blkreplay
-device ide-hd,drive=img-blkreplay

QEMU hangs at some moment of replay.

I found that some dma requests do not pass through the blkreplay driver due to the following line in block-backend.c:

    return bdrv_co_preadv(blk->root, offset, bytes, qiov, flags);

Quoted 6 lines

Quoted 2 lines

This line passes read request directly to qcow driver and blkreplay cannot process it to make deterministic.

How does that bypass blkreplay? blk->root is supposed to be the blkreply node, do you see something different? If it were the qcow2 node, then I would expect that no requests at all go through the blkreplay layer.

It seems, that the problem is in -snapshot option.
We have one of the following block layers depending on command line:
tmp_overlay1 -> blkreplay -> tmp_overlay2 -> disk_image tmp_overlay1 -> blkreplay -> disk_image

Quoted 6 lines

But the correct scheme is intended to be the following:
blkreplay -> tmp_overlay1 -> disk_image

How can we fix it?
Maybe we should add blkreplay automatically to all block devices and not to specify it in the command line?

I think you found a pretty fundamental design problem with "global" drive options that add a filter node such as -snapshot and replay mode (replay mode isn't one of them today, but your suggestion to make it automatic would turn it into one).

At the core of the problem I think we have two questions:

1. Which block nodes should be affected and get a filter node added, and which nodes shouldn't get one? In your case, disl_image is defined with a -drive option, but shouldn't get the snapshot.

2. In which order should filter nodes be added?

Both of these questions feel hard. As long as we haven't thought through the concept as such (rather than discussing one-off hacks) and we're not completely certain what the right answer to the questions is, we shouldn't add more automatic filter nodes, because chances are that we get it wrong and would regret it.

The obvious answer for a workaround would be: Make everything manual, i.e. don't use -snapshot, but create a qcow2 overlay manually.

For getting the automatic thing right, please give me some time to think about the design. I'll also meet Markus (CCed) in person end of next week and I think this is a design question that would be useful to discuss with him then.

Paolo,

Quoted 59 lines

From: Kevin Wolf [mailto:[email protected]]
Am 21.11.2016 um 13:22 hat Pavel Dovgalyuk geschrieben:

Quoted 7 lines

Quoted 7 lines

From: Kevin Wolf [mailto:[email protected]]
Am 16.11.2016 um 10:49 hat Pavel Dovgalyuk geschrieben:

Quoted 5 lines

Quoted 1 line

From: Pavel Dovgalyuk [mailto:[email protected]]

I've investigated this issue.
This command line works ok:
-drive driver=blkreplay,if=none,image.driver=file,image.filename=testdisk.qcow,id=img-

Quoted 7 lines

Quoted 7 lines

blkreplay

Quoted 5 lines

-device ide-hd,drive=img-blkreplay

And this does not:
-drive

driver=blkreplay,if=none,image.driver=qcow2,image.file.driver=file,image.file.filename=testdis

Quoted 22 lines

Quoted 10 lines

k.qcow

Quoted 9 lines

,id=img-blkreplay
-device ide-hd,drive=img-blkreplay

QEMU hangs at some moment of replay.

I found that some dma requests do not pass through the blkreplay driver due to the following line in block-backend.c:

    return bdrv_co_preadv(blk->root, offset, bytes, qiov, flags);

Quoted 6 lines

Quoted 2 lines

This line passes read request directly to qcow driver and blkreplay cannot process it to make deterministic.

How does that bypass blkreplay? blk->root is supposed to be the blkreply node, do you see something different? If it were the qcow2 node, then I would expect that no requests at all go through the blkreplay layer.

It seems, that the problem is in -snapshot option.
We have one of the following block layers depending on command line:
tmp_overlay1 -> blkreplay -> tmp_overlay2 -> disk_image tmp_overlay1 -> blkreplay -> disk_image

Quoted 6 lines

But the correct scheme is intended to be the following:
blkreplay -> tmp_overlay1 -> disk_image

How can we fix it?
Maybe we should add blkreplay automatically to all block devices and not to specify it in the command line?

I think you found a pretty fundamental design problem with "global" drive options that add a filter node such as -snapshot and replay mode (replay mode isn't one of them today, but your suggestion to make it automatic would turn it into one).

At the core of the problem I think we have two questions:

1. Which block nodes should be affected and get a filter node added, and which nodes shouldn't get one? In your case, disl_image is defined with a -drive option, but shouldn't get the snapshot.

2. In which order should filter nodes be added?

Quoted 8 lines

Both of these questions feel hard. As long as we haven't thought through the concept as such (rather than discussing one-off hacks) and we're not completely certain what the right answer to the questions is, we shouldn't add more automatic filter nodes, because chances are that we get it wrong and would regret it.

The obvious answer for a workaround would be: Make everything manual, i.e. don't use -snapshot, but create a qcow2 overlay manually.

What about to switching to manual overlay creation by default? We can make rrsnapshot option mandatory.
Therefore user will have to create snapshot in image or overlay and the disk image will not be corrupted.

It is not very convenient, but we could disable rrsnapshot again when the solution for -snapshot will be found.

From: Kevin Wolf [mailto:[email protected]]
Am 05.12.2016 um 08:43 hat Pavel Dovgalyuk geschrieben:

Quoted 50 lines

Paolo,

Quoted 34 lines

From: Kevin Wolf [mailto:[email protected]]
Am 21.11.2016 um 13:22 hat Pavel Dovgalyuk geschrieben:

Quoted 12 lines

Quoted 6 lines

From: Kevin Wolf [mailto:[email protected]]
Am 16.11.2016 um 10:49 hat Pavel Dovgalyuk geschrieben:

How does that bypass blkreplay? blk->root is supposed to be the blkreply node, do you see something different? If it were the qcow2 node, then I would expect that no requests at all go through the blkreplay layer.

It seems, that the problem is in -snapshot option.
We have one of the following block layers depending on command line:
tmp_overlay1 -> blkreplay -> tmp_overlay2 -> disk_image tmp_overlay1 -> blkreplay -> disk_image

Quoted 6 lines

But the correct scheme is intended to be the following:
blkreplay -> tmp_overlay1 -> disk_image

How can we fix it?
Maybe we should add blkreplay automatically to all block devices and not to specify it in the command line?

I think you found a pretty fundamental design problem with "global" drive options that add a filter node such as -snapshot and replay mode (replay mode isn't one of them today, but your suggestion to make it automatic would turn it into one).

At the core of the problem I think we have two questions:

1. Which block nodes should be affected and get a filter node added, and which nodes shouldn't get one? In your case, disl_image is defined with a -drive option, but shouldn't get the snapshot.

2. In which order should filter nodes be added?

Quoted 8 lines

Both of these questions feel hard. As long as we haven't thought through the concept as such (rather than discussing one-off hacks) and we're not completely certain what the right answer to the questions is, we shouldn't add more automatic filter nodes, because chances are that we get it wrong and would regret it.

The obvious answer for a workaround would be: Make everything manual, i.e. don't use -snapshot, but create a qcow2 overlay manually.

What about to switching to manual overlay creation by default? We can make rrsnapshot option mandatory.
Therefore user will have to create snapshot in image or overlay and the disk image will not be corrupted.

Quoted 2 lines

It is not very convenient, but we could disable rrsnapshot again when the solution for -snapshot will be found.

Hm, what is this rrsnapshot option? git grep can't find it.

Anyway, it seems that doing things manually is the safe way as long as we don't know the final solution, so I think I agree.

For a slightly more convenient way, one of the problems to solve seems to be that snapshot=on always affects the top level node and you can't create a temporary snapshot in the middle of the chain. Perhaps we should introduce a 'temporary-overlay' driver or something like that, so that you could specify things like this:

-drive if=none,driver=file,filename=test.img,id=orig -drive if=none,driver=temporary-overlay,file=orig,id=snap -drive if=none,driver=blkreplay,image=snap

Which makes me wonder... Is blkreplay usable without the temporary snapshot or is this pretty much a requirement?

Because if it has to be
there, the next step could be that blkreplay creates temporary-overlay internally in its .bdrv_open().

Quoted 52 lines

From: Kevin Wolf [mailto:[email protected]]
Am 05.12.2016 um 08:43 hat Pavel Dovgalyuk geschrieben:

Quoted 50 lines

Paolo,

Quoted 34 lines

From: Kevin Wolf [mailto:[email protected]]
Am 21.11.2016 um 13:22 hat Pavel Dovgalyuk geschrieben:

Quoted 12 lines

Quoted 6 lines

From: Kevin Wolf [mailto:[email protected]]
Am 16.11.2016 um 10:49 hat Pavel Dovgalyuk geschrieben:

How does that bypass blkreplay? blk->root is supposed to be the blkreply node, do you see something different? If it were the qcow2 node, then I would expect that no requests at all go through the blkreplay layer.

It seems, that the problem is in -snapshot option.
We have one of the following block layers depending on command line:
tmp_overlay1 -> blkreplay -> tmp_overlay2 -> disk_image tmp_overlay1 -> blkreplay -> disk_image

Quoted 6 lines

But the correct scheme is intended to be the following:
blkreplay -> tmp_overlay1 -> disk_image

How can we fix it?
Maybe we should add blkreplay automatically to all block devices and not to specify it in the command line?

I think you found a pretty fundamental design problem with "global" drive options that add a filter node such as -snapshot and replay mode (replay mode isn't one of them today, but your suggestion to make it automatic would turn it into one).

At the core of the problem I think we have two questions:

1. Which block nodes should be affected and get a filter node added, and which nodes shouldn't get one? In your case, disl_image is defined with a -drive option, but shouldn't get the snapshot.

2. In which order should filter nodes be added?

Quoted 8 lines

Both of these questions feel hard. As long as we haven't thought through the concept as such (rather than discussing one-off hacks) and we're not completely certain what the right answer to the questions is, we shouldn't add more automatic filter nodes, because chances are that we get it wrong and would regret it.

The obvious answer for a workaround would be: Make everything manual, i.e. don't use -snapshot, but create a qcow2 overlay manually.

What about to switching to manual overlay creation by default? We can make rrsnapshot option mandatory.
Therefore user will have to create snapshot in image or overlay and the disk image will not be corrupted.

Quoted 4 lines

Quoted 2 lines

It is not very convenient, but we could disable rrsnapshot again when the solution for -snapshot will be found.

Hm, what is this rrsnapshot option? git grep can't find it.

It was a patch that was not included yet.
This option creates/loads vm snapshot in record/replay mode leaving original disk image unchanged.

Record/replay without this option uses '-snapshot' to preserve the state of the disk images.

Quoted 9 lines

Anyway, it seems that doing things manually is the safe way as long as we don't know the final solution, so I think I agree.

For a slightly more convenient way, one of the problems to solve seems to be that snapshot=on always affects the top level node and you can't create a temporary snapshot in the middle of the chain. Perhaps we should introduce a 'temporary-overlay' driver or something like that, so that you could specify things like this:

Quoted 3 lines

-drive if=none,driver=file,filename=test.img,id=orig -drive if=none,driver=temporary-overlay,file=orig,id=snap -drive if=none,driver=blkreplay,image=snap

This seems reasonable for manual way.

Quoted 2 lines

Which makes me wonder... Is blkreplay usable without the temporary snapshot or is this pretty much a requirement?

It's not a requirement. But to make replay deterministic we have to start with the same image every time. As I know, this may be achieved by:
1. Restoring original disk image manually
2. Using vm snapshot to start execution from
3. Using -snapshot option
4. Not using disks at all

Quoted 3 lines

Because if it has to be
there, the next step could be that blkreplay creates temporary-overlay internally in its .bdrv_open().

Here is your answer about such an approach :)
https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg04687.html

From: Kevin Wolf [mailto:[email protected]]
Am 05.12.2016 um 12:49 hat Pavel Dovgalyuk geschrieben:

Quoted 15 lines

Quoted 2 lines

From: Kevin Wolf [mailto:[email protected]]
Am 05.12.2016 um 08:43 hat Pavel Dovgalyuk geschrieben:

Record/replay without this option uses '-snapshot' to preserve the state of the disk images.

Quoted 9 lines

Anyway, it seems that doing things manually is the safe way as long as we don't know the final solution, so I think I agree.

For a slightly more convenient way, one of the problems to solve seems to be that snapshot=on always affects the top level node and you can't create a temporary snapshot in the middle of the chain. Perhaps we should introduce a 'temporary-overlay' driver or something like that, so that you could specify things like this:

Quoted 5 lines

Quoted 3 lines

-drive if=none,driver=file,filename=test.img,id=orig -drive if=none,driver=temporary-overlay,file=orig,id=snap -drive if=none,driver=blkreplay,image=snap

This seems reasonable for manual way.

Maybe another, easier to implement option could be something like this:

-drive if=none,driver=file,filename=test.img,snapshot=on,overlay.node-name=snap -drive if=none,driver=blkreplay,image=snap

It would require that we implement support for overlay.* options like we already support backing.* options. Allowing to specify options for the overlay node is probably nice to have anyway.

However, there could be a few tricky parts there. For example, what happens if someone uses overlay.backing=something-else? Perhaps completely disallowing backing and backing.* for overlays would already solve this.

Quoted 10 lines

Quoted 2 lines

Which makes me wonder... Is blkreplay usable without the temporary snapshot or is this pretty much a requirement?

It's not a requirement. But to make replay deterministic we have to start with the same image every time. As I know, this may be achieved by:
1. Restoring original disk image manually
2. Using vm snapshot to start execution from
3. Using -snapshot option
4. Not using disks at all

Quoted 6 lines

Quoted 3 lines

Because if it has to be
there, the next step could be that blkreplay creates temporary-overlay internally in its .bdrv_open().

Here is your answer about such an approach :)
https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg04687.html

Right, and unfortunately these are still good points.

Especially the part where you allowed to give the overlay filename really needs to work the way it does now with the 'image' option. We might not need to be that strict with temporary overlays, restricting to qcow2 with default options could be acceptable there - but whatever I think of to support both cases results in something that isn't really easier than the manual way that we figured out above.

From: Kevin Wolf [mailto:[email protected]]
Am 05.12.2016 um 12:49 hat Pavel Dovgalyuk geschrieben:

Quoted 9 lines

Quoted 1 line

From: Kevin Wolf [mailto:[email protected]]

It's not a requirement. But to make replay deterministic we have to start with the same image every time. As I know, this may be achieved by:
1. Restoring original disk image manually
2. Using vm snapshot to start execution from
3. Using -snapshot option
4. Not using disks at all

Quoted 6 lines

Quoted 3 lines

Because if it has to be
there, the next step could be that blkreplay creates temporary-overlay internally in its .bdrv_open().

Here is your answer about such an approach :)
https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg04687.html

Right, and unfortunately these are still good points.

Especially the part where you allowed to give the overlay filename really needs to work the way it does now with the 'image' option. We might not need to be that strict with temporary overlays, restricting to qcow2 with default options could be acceptable there - but whatever I think of to support both cases results in something that isn't really easier than the manual way that we figured out above.

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 rrsnapshot option for icount. rrshapshot option creates start snapshot at record and loads it at replay. It allows preserving the state of disk images used by virtual machine. This vm state can also use used to roll back the execution while replaying.

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

v5 changes:
- Recording is stopped when initial snapshot cannot be created
- Minor changes