Re: [Qemu-devel] [for-4.2 PATCH 3/6] replay: update docs for record/replay with block devices
Am 19.09.2019 um 14:10 hat Pavel Dovgalyuk geschrieben:
> > From: Kevin Wolf [mailto:kw...@redhat.com]
> > Am 19.09.2019 um 11:05 hat Pavel Dovgalyuk geschrieben:
> > > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > > > >
> > > > > > However, global -snapshot is just a convenient shortcut for
> > > > > > specifying
> > > > > > snapshot=on for all -drive arguments. So if -snapshot is
> > > > > > incompatible
> > > > > > with replay, shouldn't manually marking all drives as snapshot=on be
> > > > > > incompatible as well?
> > > > > >
> > > > > > Maybe you're really interested in some specific drive not having
> > > > > > snapshot=on? But then it might be better to check that specific
> > > > > > drive
> > > > > > instad of forbidding just the shortcut for setting it.
> > > > >
> > > > > -snapshot adds the flag for top-level drive, making driver operations
> > > > > dependent on temporary file structure.
> > > > >
> > > > > Moving this overlay beneath blkreplay driver makes drive operations
> > > > > deterministic for the top-level device.
> > > >
> > > > So the real requirement is that blkreplay is the top-level node of any
> > > > guest device, right? And only because of this, you can't use -snapshot
> > > > (or snapshot=on on the blkreplay driver).
> > > >
> > > > If we instead check e.g. in blk_insert_bs() or blk_attach_dev() that in
> > > > record/replay mode, the root node of the BlockBackend is blkreplay,
> > > > wouldn't we catch many more incorrect setups?
> > >
> > > That sounds interesting.
> > > Will it help to check that every backend is connected to blkreplay?
> >
> > Yes, it would return an error when you try to attach a non-blkreplay
> > node to a BlockBackend (and every guest device uses a BlockBackend).
> >
> > Note that this restriction would currently make block jobs unavailable
> > on non-blkreplay nodes as they also use BlockBackends internally (though
> > this is going to change in the long run). I believe this restriction is
> > harmless and the typical replay use case doesn't involve any block jobs,
> > but if you do think it's a problem, blk_attach_dev() would be the place
> > that affects only devices.
> >
> > > How then this check has to be done?
> >
> > Only compile-tested, but maybe something like below?
> >
> > Kevin
> >
> > diff --git a/include/block/block_int.h b/include/block/block_int.h
> > index 0422acdf1c..9fa72bea51 100644
> > --- a/include/block/block_int.h
> > +++ b/include/block/block_int.h
> > @@ -955,6 +955,7 @@ static inline BlockDriverState
> > *backing_bs(BlockDriverState *bs)
> > extern BlockDriver bdrv_file;
> > extern BlockDriver bdrv_raw;
> > extern BlockDriver bdrv_qcow2;
> > +extern BlockDriver bdrv_blkreplay;
> >
> > int coroutine_fn bdrv_co_preadv(BdrvChild *child,
> > int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
> > diff --git a/block/blkreplay.c b/block/blkreplay.c
> > index 2b7931b940..16a4f1df6a 100644
> > --- a/block/blkreplay.c
> > +++ b/block/blkreplay.c
> > @@ -126,7 +126,7 @@ static int coroutine_fn
> > blkreplay_co_flush(BlockDriverState *bs)
> > return ret;
> > }
> >
> > -static BlockDriver bdrv_blkreplay = {
> > +BlockDriver bdrv_blkreplay = {
> > .format_name= "blkreplay",
> > .instance_size = 0,
> >
> > diff --git a/block/block-backend.c b/block/block-backend.c
> > index 1c605d5444..c57d3d9fdf 100644
> > --- a/block/block-backend.c
> > +++ b/block/block-backend.c
> > @@ -17,6 +17,7 @@
> > #include "block/throttle-groups.h"
> > #include "hw/qdev-core.h"
> > #include "sysemu/blockdev.h"
> > +#include "sysemu/replay.h"
> > #include "sysemu/runstate.h"
> > #include "qapi/error.h"
> > #include "qapi/qapi-events-block.h"
> > @@ -808,6 +809,12 @@ void blk_remove_bs(BlockBackend *blk)
> > int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp)
> > {
> > ThrottleGroupMember *tgm = >public.throttle_group_member;
> > +
> > +if (replay_mode != REPLAY_MODE_NONE && bs->drv != _blkreplay) {
> > +error_setg(errp, "Root node must be blkreplay");
> > +return -ENOTSUP;
> > +}
>
> I guess this is opposite direction - bs->drv is bdrv_file.
> And we should check its parent.
If bs->drv is bdrv_file, you want this to fail because only
bdrv_blkreplay should be able to be attached to devices.
bs doesn't have any parents here in the common case, it's the root node
of the BlockBackend.
Kevin
Re: [Qemu-devel] [for-4.2 PATCH 3/6] replay: update docs for record/replay with block devices
> From: Kevin Wolf [mailto:kw...@redhat.com]
> Am 19.09.2019 um 11:05 hat Pavel Dovgalyuk geschrieben:
> > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > > >
> > > > > However, global -snapshot is just a convenient shortcut for specifying
> > > > > snapshot=on for all -drive arguments. So if -snapshot is incompatible
> > > > > with replay, shouldn't manually marking all drives as snapshot=on be
> > > > > incompatible as well?
> > > > >
> > > > > Maybe you're really interested in some specific drive not having
> > > > > snapshot=on? But then it might be better to check that specific drive
> > > > > instad of forbidding just the shortcut for setting it.
> > > >
> > > > -snapshot adds the flag for top-level drive, making driver operations
> > > > dependent on temporary file structure.
> > > >
> > > > Moving this overlay beneath blkreplay driver makes drive operations
> > > > deterministic for the top-level device.
> > >
> > > So the real requirement is that blkreplay is the top-level node of any
> > > guest device, right? And only because of this, you can't use -snapshot
> > > (or snapshot=on on the blkreplay driver).
> > >
> > > If we instead check e.g. in blk_insert_bs() or blk_attach_dev() that in
> > > record/replay mode, the root node of the BlockBackend is blkreplay,
> > > wouldn't we catch many more incorrect setups?
> >
> > That sounds interesting.
> > Will it help to check that every backend is connected to blkreplay?
>
> Yes, it would return an error when you try to attach a non-blkreplay
> node to a BlockBackend (and every guest device uses a BlockBackend).
>
> Note that this restriction would currently make block jobs unavailable
> on non-blkreplay nodes as they also use BlockBackends internally (though
> this is going to change in the long run). I believe this restriction is
> harmless and the typical replay use case doesn't involve any block jobs,
> but if you do think it's a problem, blk_attach_dev() would be the place
> that affects only devices.
>
> > How then this check has to be done?
>
> Only compile-tested, but maybe something like below?
>
> Kevin
>
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 0422acdf1c..9fa72bea51 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -955,6 +955,7 @@ static inline BlockDriverState
> *backing_bs(BlockDriverState *bs)
> extern BlockDriver bdrv_file;
> extern BlockDriver bdrv_raw;
> extern BlockDriver bdrv_qcow2;
> +extern BlockDriver bdrv_blkreplay;
>
> int coroutine_fn bdrv_co_preadv(BdrvChild *child,
> int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
> diff --git a/block/blkreplay.c b/block/blkreplay.c
> index 2b7931b940..16a4f1df6a 100644
> --- a/block/blkreplay.c
> +++ b/block/blkreplay.c
> @@ -126,7 +126,7 @@ static int coroutine_fn
> blkreplay_co_flush(BlockDriverState *bs)
> return ret;
> }
>
> -static BlockDriver bdrv_blkreplay = {
> +BlockDriver bdrv_blkreplay = {
> .format_name= "blkreplay",
> .instance_size = 0,
>
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 1c605d5444..c57d3d9fdf 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -17,6 +17,7 @@
> #include "block/throttle-groups.h"
> #include "hw/qdev-core.h"
> #include "sysemu/blockdev.h"
> +#include "sysemu/replay.h"
> #include "sysemu/runstate.h"
> #include "qapi/error.h"
> #include "qapi/qapi-events-block.h"
> @@ -808,6 +809,12 @@ void blk_remove_bs(BlockBackend *blk)
> int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp)
> {
> ThrottleGroupMember *tgm = >public.throttle_group_member;
> +
> +if (replay_mode != REPLAY_MODE_NONE && bs->drv != _blkreplay) {
> +error_setg(errp, "Root node must be blkreplay");
> +return -ENOTSUP;
> +}
I guess this is opposite direction - bs->drv is bdrv_file.
And we should check its parent.
> +
> bdrv_ref(bs);
> blk->root = bdrv_root_attach_child(bs, "root", _root, blk->ctx,
> blk->perm, blk->shared_perm, blk,
> errp);
Pavel Dovgalyuk
Re: [Qemu-devel] [for-4.2 PATCH 3/6] replay: update docs for record/replay with block devices
Am 19.09.2019 um 11:05 hat Pavel Dovgalyuk geschrieben:
> > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > >
> > > > However, global -snapshot is just a convenient shortcut for specifying
> > > > snapshot=on for all -drive arguments. So if -snapshot is incompatible
> > > > with replay, shouldn't manually marking all drives as snapshot=on be
> > > > incompatible as well?
> > > >
> > > > Maybe you're really interested in some specific drive not having
> > > > snapshot=on? But then it might be better to check that specific drive
> > > > instad of forbidding just the shortcut for setting it.
> > >
> > > -snapshot adds the flag for top-level drive, making driver operations
> > > dependent on temporary file structure.
> > >
> > > Moving this overlay beneath blkreplay driver makes drive operations
> > > deterministic for the top-level device.
> >
> > So the real requirement is that blkreplay is the top-level node of any
> > guest device, right? And only because of this, you can't use -snapshot
> > (or snapshot=on on the blkreplay driver).
> >
> > If we instead check e.g. in blk_insert_bs() or blk_attach_dev() that in
> > record/replay mode, the root node of the BlockBackend is blkreplay,
> > wouldn't we catch many more incorrect setups?
>
> That sounds interesting.
> Will it help to check that every backend is connected to blkreplay?
Yes, it would return an error when you try to attach a non-blkreplay
node to a BlockBackend (and every guest device uses a BlockBackend).
Note that this restriction would currently make block jobs unavailable
on non-blkreplay nodes as they also use BlockBackends internally (though
this is going to change in the long run). I believe this restriction is
harmless and the typical replay use case doesn't involve any block jobs,
but if you do think it's a problem, blk_attach_dev() would be the place
that affects only devices.
> How then this check has to be done?
Only compile-tested, but maybe something like below?
Kevin
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 0422acdf1c..9fa72bea51 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -955,6 +955,7 @@ static inline BlockDriverState *backing_bs(BlockDriverState
*bs)
extern BlockDriver bdrv_file;
extern BlockDriver bdrv_raw;
extern BlockDriver bdrv_qcow2;
+extern BlockDriver bdrv_blkreplay;
int coroutine_fn bdrv_co_preadv(BdrvChild *child,
int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
diff --git a/block/blkreplay.c b/block/blkreplay.c
index 2b7931b940..16a4f1df6a 100644
--- a/block/blkreplay.c
+++ b/block/blkreplay.c
@@ -126,7 +126,7 @@ static int coroutine_fn blkreplay_co_flush(BlockDriverState
*bs)
return ret;
}
-static BlockDriver bdrv_blkreplay = {
+BlockDriver bdrv_blkreplay = {
.format_name= "blkreplay",
.instance_size = 0,
diff --git a/block/block-backend.c b/block/block-backend.c
index 1c605d5444..c57d3d9fdf 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -17,6 +17,7 @@
#include "block/throttle-groups.h"
#include "hw/qdev-core.h"
#include "sysemu/blockdev.h"
+#include "sysemu/replay.h"
#include "sysemu/runstate.h"
#include "qapi/error.h"
#include "qapi/qapi-events-block.h"
@@ -808,6 +809,12 @@ void blk_remove_bs(BlockBackend *blk)
int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp)
{
ThrottleGroupMember *tgm = >public.throttle_group_member;
+
+if (replay_mode != REPLAY_MODE_NONE && bs->drv != _blkreplay) {
+error_setg(errp, "Root node must be blkreplay");
+return -ENOTSUP;
+}
+
bdrv_ref(bs);
blk->root = bdrv_root_attach_child(bs, "root", _root, blk->ctx,
blk->perm, blk->shared_perm, blk, errp);
Re: [Qemu-devel] [for-4.2 PATCH 3/6] replay: update docs for record/replay with block devices
> From: Kevin Wolf [mailto:kw...@redhat.com] > > > > > > However, global -snapshot is just a convenient shortcut for specifying > > > snapshot=on for all -drive arguments. So if -snapshot is incompatible > > > with replay, shouldn't manually marking all drives as snapshot=on be > > > incompatible as well? > > > > > > Maybe you're really interested in some specific drive not having > > > snapshot=on? But then it might be better to check that specific drive > > > instad of forbidding just the shortcut for setting it. > > > > -snapshot adds the flag for top-level drive, making driver operations > > dependent on temporary file structure. > > > > Moving this overlay beneath blkreplay driver makes drive operations > > deterministic for the top-level device. > > So the real requirement is that blkreplay is the top-level node of any > guest device, right? And only because of this, you can't use -snapshot > (or snapshot=on on the blkreplay driver). > > If we instead check e.g. in blk_insert_bs() or blk_attach_dev() that in > record/replay mode, the root node of the BlockBackend is blkreplay, > wouldn't we catch many more incorrect setups? That sounds interesting. Will it help to check that every backend is connected to blkreplay? How then this check has to be done? Pavel Dovgalyuk
Re: [Qemu-devel] [for-4.2 PATCH 3/6] replay: update docs for record/replay with block devices
Am 18.09.2019 um 11:52 hat Pavel Dovgalyuk geschrieben: > > From: Kevin Wolf [mailto:kw...@redhat.com] > > Am 18.09.2019 um 11:37 hat Pavel Dovgalyuk geschrieben: > > > > From: Kevin Wolf [mailto:kw...@redhat.com] > > > > Am 18.09.2019 um 11:22 hat Pavel Dovgalyuk geschrieben: > > > > > > From: Kevin Wolf [mailto:kw...@redhat.com] > > > > > > Am 17.09.2019 um 13:58 hat Pavel Dovgalyuk geschrieben: > > > > > > > From: Pavel Dovgalyuk > > > > > > > > > > > > > > This patch updates the description of the command lines for using > > > > > > > record/replay with attached block devices. > > > > > > > > > > > > > > Signed-off-by: Pavel Dovgalyuk > > > > > > > --- > > > > > > > docs/replay.txt | 12 +--- > > > > > > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > > > > > > > > > > > diff --git a/docs/replay.txt b/docs/replay.txt > > > > > > > index ee6aee9861..ce97c3f72f 100644 > > > > > > > --- a/docs/replay.txt > > > > > > > +++ b/docs/replay.txt > > > > > > > @@ -27,7 +27,7 @@ Usage of the record/replay: > > > > > > > * First, record the execution with the following command line: > > > > > > > qemu-system-i386 \ > > > > > > > -icount shift=7,rr=record,rrfile=replay.bin \ > > > > > > > - -drive file=disk.qcow2,if=none,id=img-direct \ > > > > > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \ > > > > > > > -drive > > > > > > > driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \ > > > > > > > -device ide-hd,drive=img-blkreplay \ > > > > > > > -netdev user,id=net1 -device rtl8139,netdev=net1 \ > > > > > > > @@ -35,7 +35,7 @@ Usage of the record/replay: > > > > > > > * After recording, you can replay it by using another command > > > > > > > line: > > > > > > > qemu-system-i386 \ > > > > > > > -icount shift=7,rr=replay,rrfile=replay.bin \ > > > > > > > - -drive file=disk.qcow2,if=none,id=img-direct \ > > > > > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \ > > > > > > > -drive > > > > > > > driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \ > > > > > > > -device ide-hd,drive=img-blkreplay \ > > > > > > > -netdev user,id=net1 -device rtl8139,netdev=net1 \ > > > > > > > @@ -223,7 +223,7 @@ Block devices record/replay module intercepts > > > > > > > calls of > > > > > > > bdrv coroutine functions at the top of block drivers stack. > > > > > > > To record and replay block operations the drive must be > > > > > > > configured > > > > > > > as following: > > > > > > > - -drive file=disk.qcow2,if=none,id=img-direct > > > > > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct > > > > > > > -drive > > > > > > > driver=blkreplay,if=none,image=img-direct,id=img-blkreplay > > > > > > > -device ide-hd,drive=img-blkreplay > > > > > > > > > > > > > > @@ -252,6 +252,12 @@ This snapshot is created at start of > > > > > > > recording and restored at > > > > start > > > > > > > of replaying. It also can be loaded while replaying to roll back > > > > > > > the execution. > > > > > > > > > > > > > > +'snapshot' flag of the disk image must be removed to save the > > > > > > > snapshots > > > > > > > +in the overlay (or original image) instead of using the > > > > > > > temporary overlay. > > > > > > > + -drive file=disk.ovl,if=none,id=img-direct > > > > > > > + -drive > > > > > > > driver=blkreplay,if=none,image=img-direct,id=img-blkreplay > > > > > > > + -device ide-hd,drive=img-blkreplay > > > > > > > > > > > > Wait, didn't patch 2 just make -snapshot unconditionally > > > > > > incompatible > > > > > > with replay? So isn't saving the snapshot in the original image the > > > > > > only > > > > > > supported mode now? > > > > > > > > > > There are two ways to run record/replay: > > > > > 1. Disk with snapshot option and any image to make it unchanged > > > > > 2. Disk with overlay without snapshot option to save VM snapshots on > > > > > it > > > > > > > > Yes, I think I understand the two options that you intend to make > > > > available, but when -snapshot sets a replay blocker, how can 1. still > > > > work? Is there some code that ignores replay blockers? > > > > > > I checked the text and don't see anything about global "-snapshot" option. > > > All references are about drive snapshot flag. > > > Can you point me where did I missed that? > > > > Oh, sorry, you're right and I misread. > > > > However, global -snapshot is just a convenient shortcut for specifying > > snapshot=on for all -drive arguments. So if -snapshot is incompatible > > with replay, shouldn't manually marking all drives as snapshot=on be > > incompatible as well? > > > > Maybe you're really interested in some specific drive not having > > snapshot=on? But then it might be better to check that specific drive > > instad of forbidding just the shortcut for setting it. > > -snapshot adds the flag for top-level drive, making driver operations > dependent on temporary file structure. > >
Re: [Qemu-devel] [for-4.2 PATCH 3/6] replay: update docs for record/replay with block devices
> From: Kevin Wolf [mailto:kw...@redhat.com] > Am 18.09.2019 um 11:37 hat Pavel Dovgalyuk geschrieben: > > > From: Kevin Wolf [mailto:kw...@redhat.com] > > > Am 18.09.2019 um 11:22 hat Pavel Dovgalyuk geschrieben: > > > > > From: Kevin Wolf [mailto:kw...@redhat.com] > > > > > Am 17.09.2019 um 13:58 hat Pavel Dovgalyuk geschrieben: > > > > > > From: Pavel Dovgalyuk > > > > > > > > > > > > This patch updates the description of the command lines for using > > > > > > record/replay with attached block devices. > > > > > > > > > > > > Signed-off-by: Pavel Dovgalyuk > > > > > > --- > > > > > > docs/replay.txt | 12 +--- > > > > > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > > > > > > > > > diff --git a/docs/replay.txt b/docs/replay.txt > > > > > > index ee6aee9861..ce97c3f72f 100644 > > > > > > --- a/docs/replay.txt > > > > > > +++ b/docs/replay.txt > > > > > > @@ -27,7 +27,7 @@ Usage of the record/replay: > > > > > > * First, record the execution with the following command line: > > > > > > qemu-system-i386 \ > > > > > > -icount shift=7,rr=record,rrfile=replay.bin \ > > > > > > - -drive file=disk.qcow2,if=none,id=img-direct \ > > > > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \ > > > > > > -drive > > > > > > driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \ > > > > > > -device ide-hd,drive=img-blkreplay \ > > > > > > -netdev user,id=net1 -device rtl8139,netdev=net1 \ > > > > > > @@ -35,7 +35,7 @@ Usage of the record/replay: > > > > > > * After recording, you can replay it by using another command > > > > > > line: > > > > > > qemu-system-i386 \ > > > > > > -icount shift=7,rr=replay,rrfile=replay.bin \ > > > > > > - -drive file=disk.qcow2,if=none,id=img-direct \ > > > > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \ > > > > > > -drive > > > > > > driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \ > > > > > > -device ide-hd,drive=img-blkreplay \ > > > > > > -netdev user,id=net1 -device rtl8139,netdev=net1 \ > > > > > > @@ -223,7 +223,7 @@ Block devices record/replay module intercepts > > > > > > calls of > > > > > > bdrv coroutine functions at the top of block drivers stack. > > > > > > To record and replay block operations the drive must be configured > > > > > > as following: > > > > > > - -drive file=disk.qcow2,if=none,id=img-direct > > > > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct > > > > > > -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay > > > > > > -device ide-hd,drive=img-blkreplay > > > > > > > > > > > > @@ -252,6 +252,12 @@ This snapshot is created at start of recording > > > > > > and restored at > > > start > > > > > > of replaying. It also can be loaded while replaying to roll back > > > > > > the execution. > > > > > > > > > > > > +'snapshot' flag of the disk image must be removed to save the > > > > > > snapshots > > > > > > +in the overlay (or original image) instead of using the temporary > > > > > > overlay. > > > > > > + -drive file=disk.ovl,if=none,id=img-direct > > > > > > + -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay > > > > > > + -device ide-hd,drive=img-blkreplay > > > > > > > > > > Wait, didn't patch 2 just make -snapshot unconditionally incompatible > > > > > with replay? So isn't saving the snapshot in the original image the > > > > > only > > > > > supported mode now? > > > > > > > > There are two ways to run record/replay: > > > > 1. Disk with snapshot option and any image to make it unchanged > > > > 2. Disk with overlay without snapshot option to save VM snapshots on it > > > > > > Yes, I think I understand the two options that you intend to make > > > available, but when -snapshot sets a replay blocker, how can 1. still > > > work? Is there some code that ignores replay blockers? > > > > I checked the text and don't see anything about global "-snapshot" option. > > All references are about drive snapshot flag. > > Can you point me where did I missed that? > > Oh, sorry, you're right and I misread. > > However, global -snapshot is just a convenient shortcut for specifying > snapshot=on for all -drive arguments. So if -snapshot is incompatible > with replay, shouldn't manually marking all drives as snapshot=on be > incompatible as well? > > Maybe you're really interested in some specific drive not having > snapshot=on? But then it might be better to check that specific drive > instad of forbidding just the shortcut for setting it. -snapshot adds the flag for top-level drive, making driver operations dependent on temporary file structure. Moving this overlay beneath blkreplay driver makes drive operations deterministic for the top-level device. Pavel Dovgalyuk
Re: [Qemu-devel] [for-4.2 PATCH 3/6] replay: update docs for record/replay with block devices
Am 18.09.2019 um 11:37 hat Pavel Dovgalyuk geschrieben: > > From: Kevin Wolf [mailto:kw...@redhat.com] > > Am 18.09.2019 um 11:22 hat Pavel Dovgalyuk geschrieben: > > > > From: Kevin Wolf [mailto:kw...@redhat.com] > > > > Am 17.09.2019 um 13:58 hat Pavel Dovgalyuk geschrieben: > > > > > From: Pavel Dovgalyuk > > > > > > > > > > This patch updates the description of the command lines for using > > > > > record/replay with attached block devices. > > > > > > > > > > Signed-off-by: Pavel Dovgalyuk > > > > > --- > > > > > docs/replay.txt | 12 +--- > > > > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > > > > > > > diff --git a/docs/replay.txt b/docs/replay.txt > > > > > index ee6aee9861..ce97c3f72f 100644 > > > > > --- a/docs/replay.txt > > > > > +++ b/docs/replay.txt > > > > > @@ -27,7 +27,7 @@ Usage of the record/replay: > > > > > * First, record the execution with the following command line: > > > > > qemu-system-i386 \ > > > > > -icount shift=7,rr=record,rrfile=replay.bin \ > > > > > - -drive file=disk.qcow2,if=none,id=img-direct \ > > > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \ > > > > > -drive > > > > > driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \ > > > > > -device ide-hd,drive=img-blkreplay \ > > > > > -netdev user,id=net1 -device rtl8139,netdev=net1 \ > > > > > @@ -35,7 +35,7 @@ Usage of the record/replay: > > > > > * After recording, you can replay it by using another command line: > > > > > qemu-system-i386 \ > > > > > -icount shift=7,rr=replay,rrfile=replay.bin \ > > > > > - -drive file=disk.qcow2,if=none,id=img-direct \ > > > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \ > > > > > -drive > > > > > driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \ > > > > > -device ide-hd,drive=img-blkreplay \ > > > > > -netdev user,id=net1 -device rtl8139,netdev=net1 \ > > > > > @@ -223,7 +223,7 @@ Block devices record/replay module intercepts > > > > > calls of > > > > > bdrv coroutine functions at the top of block drivers stack. > > > > > To record and replay block operations the drive must be configured > > > > > as following: > > > > > - -drive file=disk.qcow2,if=none,id=img-direct > > > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct > > > > > -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay > > > > > -device ide-hd,drive=img-blkreplay > > > > > > > > > > @@ -252,6 +252,12 @@ This snapshot is created at start of recording > > > > > and restored at > > start > > > > > of replaying. It also can be loaded while replaying to roll back > > > > > the execution. > > > > > > > > > > +'snapshot' flag of the disk image must be removed to save the > > > > > snapshots > > > > > +in the overlay (or original image) instead of using the temporary > > > > > overlay. > > > > > + -drive file=disk.ovl,if=none,id=img-direct > > > > > + -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay > > > > > + -device ide-hd,drive=img-blkreplay > > > > > > > > Wait, didn't patch 2 just make -snapshot unconditionally incompatible > > > > with replay? So isn't saving the snapshot in the original image the only > > > > supported mode now? > > > > > > There are two ways to run record/replay: > > > 1. Disk with snapshot option and any image to make it unchanged > > > 2. Disk with overlay without snapshot option to save VM snapshots on it > > > > Yes, I think I understand the two options that you intend to make > > available, but when -snapshot sets a replay blocker, how can 1. still > > work? Is there some code that ignores replay blockers? > > I checked the text and don't see anything about global "-snapshot" option. > All references are about drive snapshot flag. > Can you point me where did I missed that? Oh, sorry, you're right and I misread. However, global -snapshot is just a convenient shortcut for specifying snapshot=on for all -drive arguments. So if -snapshot is incompatible with replay, shouldn't manually marking all drives as snapshot=on be incompatible as well? Maybe you're really interested in some specific drive not having snapshot=on? But then it might be better to check that specific drive instad of forbidding just the shortcut for setting it. Kevin
Re: [Qemu-devel] [for-4.2 PATCH 3/6] replay: update docs for record/replay with block devices
> From: Kevin Wolf [mailto:kw...@redhat.com] > Am 18.09.2019 um 11:22 hat Pavel Dovgalyuk geschrieben: > > > From: Kevin Wolf [mailto:kw...@redhat.com] > > > Am 17.09.2019 um 13:58 hat Pavel Dovgalyuk geschrieben: > > > > From: Pavel Dovgalyuk > > > > > > > > This patch updates the description of the command lines for using > > > > record/replay with attached block devices. > > > > > > > > Signed-off-by: Pavel Dovgalyuk > > > > --- > > > > docs/replay.txt | 12 +--- > > > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/docs/replay.txt b/docs/replay.txt > > > > index ee6aee9861..ce97c3f72f 100644 > > > > --- a/docs/replay.txt > > > > +++ b/docs/replay.txt > > > > @@ -27,7 +27,7 @@ Usage of the record/replay: > > > > * First, record the execution with the following command line: > > > > qemu-system-i386 \ > > > > -icount shift=7,rr=record,rrfile=replay.bin \ > > > > - -drive file=disk.qcow2,if=none,id=img-direct \ > > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \ > > > > -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay > > > > \ > > > > -device ide-hd,drive=img-blkreplay \ > > > > -netdev user,id=net1 -device rtl8139,netdev=net1 \ > > > > @@ -35,7 +35,7 @@ Usage of the record/replay: > > > > * After recording, you can replay it by using another command line: > > > > qemu-system-i386 \ > > > > -icount shift=7,rr=replay,rrfile=replay.bin \ > > > > - -drive file=disk.qcow2,if=none,id=img-direct \ > > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \ > > > > -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay > > > > \ > > > > -device ide-hd,drive=img-blkreplay \ > > > > -netdev user,id=net1 -device rtl8139,netdev=net1 \ > > > > @@ -223,7 +223,7 @@ Block devices record/replay module intercepts calls > > > > of > > > > bdrv coroutine functions at the top of block drivers stack. > > > > To record and replay block operations the drive must be configured > > > > as following: > > > > - -drive file=disk.qcow2,if=none,id=img-direct > > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct > > > > -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay > > > > -device ide-hd,drive=img-blkreplay > > > > > > > > @@ -252,6 +252,12 @@ This snapshot is created at start of recording and > > > > restored at > start > > > > of replaying. It also can be loaded while replaying to roll back > > > > the execution. > > > > > > > > +'snapshot' flag of the disk image must be removed to save the snapshots > > > > +in the overlay (or original image) instead of using the temporary > > > > overlay. > > > > + -drive file=disk.ovl,if=none,id=img-direct > > > > + -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay > > > > + -device ide-hd,drive=img-blkreplay > > > > > > Wait, didn't patch 2 just make -snapshot unconditionally incompatible > > > with replay? So isn't saving the snapshot in the original image the only > > > supported mode now? > > > > There are two ways to run record/replay: > > 1. Disk with snapshot option and any image to make it unchanged > > 2. Disk with overlay without snapshot option to save VM snapshots on it > > Yes, I think I understand the two options that you intend to make > available, but when -snapshot sets a replay blocker, how can 1. still > work? Is there some code that ignores replay blockers? I checked the text and don't see anything about global "-snapshot" option. All references are about drive snapshot flag. Can you point me where did I missed that? Pavel Dovgalyuk
Re: [Qemu-devel] [for-4.2 PATCH 3/6] replay: update docs for record/replay with block devices
Am 18.09.2019 um 11:22 hat Pavel Dovgalyuk geschrieben: > > From: Kevin Wolf [mailto:kw...@redhat.com] > > Am 17.09.2019 um 13:58 hat Pavel Dovgalyuk geschrieben: > > > From: Pavel Dovgalyuk > > > > > > This patch updates the description of the command lines for using > > > record/replay with attached block devices. > > > > > > Signed-off-by: Pavel Dovgalyuk > > > --- > > > docs/replay.txt | 12 +--- > > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > > > diff --git a/docs/replay.txt b/docs/replay.txt > > > index ee6aee9861..ce97c3f72f 100644 > > > --- a/docs/replay.txt > > > +++ b/docs/replay.txt > > > @@ -27,7 +27,7 @@ Usage of the record/replay: > > > * First, record the execution with the following command line: > > > qemu-system-i386 \ > > > -icount shift=7,rr=record,rrfile=replay.bin \ > > > - -drive file=disk.qcow2,if=none,id=img-direct \ > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \ > > > -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \ > > > -device ide-hd,drive=img-blkreplay \ > > > -netdev user,id=net1 -device rtl8139,netdev=net1 \ > > > @@ -35,7 +35,7 @@ Usage of the record/replay: > > > * After recording, you can replay it by using another command line: > > > qemu-system-i386 \ > > > -icount shift=7,rr=replay,rrfile=replay.bin \ > > > - -drive file=disk.qcow2,if=none,id=img-direct \ > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \ > > > -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \ > > > -device ide-hd,drive=img-blkreplay \ > > > -netdev user,id=net1 -device rtl8139,netdev=net1 \ > > > @@ -223,7 +223,7 @@ Block devices record/replay module intercepts calls of > > > bdrv coroutine functions at the top of block drivers stack. > > > To record and replay block operations the drive must be configured > > > as following: > > > - -drive file=disk.qcow2,if=none,id=img-direct > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct > > > -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay > > > -device ide-hd,drive=img-blkreplay > > > > > > @@ -252,6 +252,12 @@ This snapshot is created at start of recording and > > > restored at start > > > of replaying. It also can be loaded while replaying to roll back > > > the execution. > > > > > > +'snapshot' flag of the disk image must be removed to save the snapshots > > > +in the overlay (or original image) instead of using the temporary > > > overlay. > > > + -drive file=disk.ovl,if=none,id=img-direct > > > + -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay > > > + -device ide-hd,drive=img-blkreplay > > > > Wait, didn't patch 2 just make -snapshot unconditionally incompatible > > with replay? So isn't saving the snapshot in the original image the only > > supported mode now? > > There are two ways to run record/replay: > 1. Disk with snapshot option and any image to make it unchanged > 2. Disk with overlay without snapshot option to save VM snapshots on it Yes, I think I understand the two options that you intend to make available, but when -snapshot sets a replay blocker, how can 1. still work? Is there some code that ignores replay blockers? Kevin
Re: [Qemu-devel] [for-4.2 PATCH 3/6] replay: update docs for record/replay with block devices
> From: Kevin Wolf [mailto:kw...@redhat.com] > Am 17.09.2019 um 13:58 hat Pavel Dovgalyuk geschrieben: > > From: Pavel Dovgalyuk > > > > This patch updates the description of the command lines for using > > record/replay with attached block devices. > > > > Signed-off-by: Pavel Dovgalyuk > > --- > > docs/replay.txt | 12 +--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/docs/replay.txt b/docs/replay.txt > > index ee6aee9861..ce97c3f72f 100644 > > --- a/docs/replay.txt > > +++ b/docs/replay.txt > > @@ -27,7 +27,7 @@ Usage of the record/replay: > > * First, record the execution with the following command line: > > qemu-system-i386 \ > > -icount shift=7,rr=record,rrfile=replay.bin \ > > - -drive file=disk.qcow2,if=none,id=img-direct \ > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \ > > -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \ > > -device ide-hd,drive=img-blkreplay \ > > -netdev user,id=net1 -device rtl8139,netdev=net1 \ > > @@ -35,7 +35,7 @@ Usage of the record/replay: > > * After recording, you can replay it by using another command line: > > qemu-system-i386 \ > > -icount shift=7,rr=replay,rrfile=replay.bin \ > > - -drive file=disk.qcow2,if=none,id=img-direct \ > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \ > > -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \ > > -device ide-hd,drive=img-blkreplay \ > > -netdev user,id=net1 -device rtl8139,netdev=net1 \ > > @@ -223,7 +223,7 @@ Block devices record/replay module intercepts calls of > > bdrv coroutine functions at the top of block drivers stack. > > To record and replay block operations the drive must be configured > > as following: > > - -drive file=disk.qcow2,if=none,id=img-direct > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct > > -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay > > -device ide-hd,drive=img-blkreplay > > > > @@ -252,6 +252,12 @@ This snapshot is created at start of recording and > > restored at start > > of replaying. It also can be loaded while replaying to roll back > > the execution. > > > > +'snapshot' flag of the disk image must be removed to save the snapshots > > +in the overlay (or original image) instead of using the temporary overlay. > > + -drive file=disk.ovl,if=none,id=img-direct > > + -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay > > + -device ide-hd,drive=img-blkreplay > > Wait, didn't patch 2 just make -snapshot unconditionally incompatible > with replay? So isn't saving the snapshot in the original image the only > supported mode now? There are two ways to run record/replay: 1. Disk with snapshot option and any image to make it unchanged 2. Disk with overlay without snapshot option to save VM snapshots on it Pavel Dovgalyuk
Re: [Qemu-devel] [for-4.2 PATCH 3/6] replay: update docs for record/replay with block devices
Am 17.09.2019 um 13:58 hat Pavel Dovgalyuk geschrieben: > From: Pavel Dovgalyuk > > This patch updates the description of the command lines for using > record/replay with attached block devices. > > Signed-off-by: Pavel Dovgalyuk > --- > docs/replay.txt | 12 +--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/docs/replay.txt b/docs/replay.txt > index ee6aee9861..ce97c3f72f 100644 > --- a/docs/replay.txt > +++ b/docs/replay.txt > @@ -27,7 +27,7 @@ Usage of the record/replay: > * First, record the execution with the following command line: > qemu-system-i386 \ > -icount shift=7,rr=record,rrfile=replay.bin \ > - -drive file=disk.qcow2,if=none,id=img-direct \ > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \ > -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \ > -device ide-hd,drive=img-blkreplay \ > -netdev user,id=net1 -device rtl8139,netdev=net1 \ > @@ -35,7 +35,7 @@ Usage of the record/replay: > * After recording, you can replay it by using another command line: > qemu-system-i386 \ > -icount shift=7,rr=replay,rrfile=replay.bin \ > - -drive file=disk.qcow2,if=none,id=img-direct \ > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \ > -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \ > -device ide-hd,drive=img-blkreplay \ > -netdev user,id=net1 -device rtl8139,netdev=net1 \ > @@ -223,7 +223,7 @@ Block devices record/replay module intercepts calls of > bdrv coroutine functions at the top of block drivers stack. > To record and replay block operations the drive must be configured > as following: > - -drive file=disk.qcow2,if=none,id=img-direct > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct > -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay > -device ide-hd,drive=img-blkreplay > > @@ -252,6 +252,12 @@ This snapshot is created at start of recording and > restored at start > of replaying. It also can be loaded while replaying to roll back > the execution. > > +'snapshot' flag of the disk image must be removed to save the snapshots > +in the overlay (or original image) instead of using the temporary overlay. > + -drive file=disk.ovl,if=none,id=img-direct > + -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay > + -device ide-hd,drive=img-blkreplay Wait, didn't patch 2 just make -snapshot unconditionally incompatible with replay? So isn't saving the snapshot in the original image the only supported mode now? Kevin
