From: "Pavel Dovgalyuk" <dovgaluk@ispras.ru>
To: 'Kevin Wolf' <kwolf@redhat.com>, pbonzini@redhat.com
Cc: 'Pavel Dovgalyuk' <pavel.dovgaluk@ispras.ru>,
qemu-devel@nongnu.org, peter.maydell@linaro.org,
quintela@redhat.com, jasowang@redhat.com, mst@redhat.com,
mreitz@redhat.com, qemu-block@nongnu.org, armbru@redhat.com
Subject: Re: [Qemu-devel] [PATCH v5 7/9] block: don't make snapshots for filters
Date: Mon, 5 Dec 2016 10:43:05 +0300 [thread overview]
Message-ID: <001201d24ecb$36f94880$a4ebd980$@ru> (raw)
In-Reply-To: <20161121155458.GD5876@noname.redhat.com>
Paolo,
> From: Kevin Wolf [mailto:kwolf@redhat.com]
> Am 21.11.2016 um 13:22 hat Pavel Dovgalyuk geschrieben:
> > > From: Kevin Wolf [mailto:kwolf@redhat.com]
> > > Am 16.11.2016 um 10:49 hat Pavel Dovgalyuk geschrieben:
> > > > > From: Pavel Dovgalyuk [mailto:dovgaluk@ispras.ru]
> > > >
> > > > 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
> > > > ,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.
> > >
> > > 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?
>
> 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.
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.
> 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.
Pavel Dovgalyuk
next prev parent reply other threads:[~2016-12-05 7:43 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-26 8:07 [Qemu-devel] [PATCH v5 0/9] replay additions Pavel Dovgalyuk
2016-09-26 8:08 ` [Qemu-devel] [PATCH v5 1/9] replay: move internal data to the structure Pavel Dovgalyuk
2016-09-26 8:08 ` [Qemu-devel] [PATCH v5 2/9] replay: vmstate for replay module Pavel Dovgalyuk
2016-09-26 8:08 ` [Qemu-devel] [PATCH v5 3/9] replay: allow replay stopping and restarting Pavel Dovgalyuk
2016-09-26 8:08 ` [Qemu-devel] [PATCH v5 4/9] record/replay: add network support Pavel Dovgalyuk
2016-10-27 3:36 ` Jason Wang
2016-10-27 7:58 ` Paolo Bonzini
2016-10-31 7:41 ` Jason Wang
2016-09-26 8:08 ` [Qemu-devel] [PATCH v5 5/9] savevm: add public save_vmstate function Pavel Dovgalyuk
2016-09-26 8:15 ` Paolo Bonzini
2016-09-26 8:08 ` [Qemu-devel] [PATCH v5 6/9] replay: save/load initial state Pavel Dovgalyuk
2016-09-26 8:08 ` [Qemu-devel] [PATCH v5 7/9] block: don't make snapshots for filters Pavel Dovgalyuk
2016-09-26 9:23 ` Kevin Wolf
2016-09-26 9:51 ` Pavel Dovgalyuk
2016-09-26 13:17 ` Kevin Wolf
2016-09-27 14:06 ` Pavel Dovgalyuk
2016-09-28 8:36 ` Kevin Wolf
2016-09-28 9:32 ` Pavel Dovgalyuk
2016-09-28 9:43 ` Kevin Wolf
2016-09-28 12:49 ` Pavel Dovgalyuk
2016-09-29 10:32 ` Kevin Wolf
2016-11-16 9:49 ` Pavel Dovgalyuk
2016-11-16 12:15 ` Paolo Bonzini
2016-11-16 13:50 ` Pavel Dovgalyuk
2016-11-16 12:20 ` Kevin Wolf
2016-11-21 12:22 ` Pavel Dovgalyuk
2016-11-21 15:54 ` Kevin Wolf
2016-12-05 7:43 ` Pavel Dovgalyuk [this message]
2016-12-05 10:34 ` Kevin Wolf
2016-12-05 11:49 ` Pavel Dovgalyuk
2016-12-05 12:44 ` Kevin Wolf
2016-12-06 9:37 ` Pavel Dovgalyuk
2016-12-22 6:58 ` Pavel Dovgalyuk
2016-09-26 8:08 ` [Qemu-devel] [PATCH v5 8/9] block: implement bdrv_recurse_is_first_non_filter for blkreplay Pavel Dovgalyuk
2016-09-26 8:08 ` [Qemu-devel] [PATCH v5 9/9] integratorcp: adding vmstate for save/restore Pavel Dovgalyuk
2016-09-26 8:15 ` [Qemu-devel] [PATCH v5 0/9] replay additions Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='001201d24ecb$36f94880$a4ebd980$@ru' \
--to=dovgaluk@ispras.ru \
--cc=armbru@redhat.com \
--cc=jasowang@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=mst@redhat.com \
--cc=pavel.dovgaluk@ispras.ru \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=quintela@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).