From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60805) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cDrmN-00054R-G5 for qemu-devel@nongnu.org; Mon, 05 Dec 2016 06:50:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cDrmM-0007RW-5f for qemu-devel@nongnu.org; Mon, 05 Dec 2016 06:49:55 -0500 From: "Pavel Dovgalyuk" References: <001101d218c8$67aa1010$36fe3030$@ru> <20160928083620.GD5236@noname.redhat.com> <001501d2196b$496b4e90$dc41ebb0$@ru> <20160928094313.GI5236@noname.redhat.com> <000001d21986$b68cd280$23a67780$@ru> <001d01d23fee$b962ec40$2c28c4c0$@ru> <20161116122001.GA5407@noname.str.redhat.com> <001b01d243f1$e2313260$a6939720$@ru> <20161121155458.GD5876@noname.redhat.com> <001201d24ecb$36f94880$a4ebd980$@ru> <20161205103416.GA4870@noname.redhat.com> In-Reply-To: <20161205103416.GA4870@noname.redhat.com> Date: Mon, 5 Dec 2016 14:49:45 +0300 Message-ID: <002201d24eed$acc39440$064abcc0$@ru> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Language: ru Subject: Re: [Qemu-devel] [PATCH v5 7/9] block: don't make snapshots for filters List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: 'Kevin Wolf' Cc: pbonzini@redhat.com, 'Pavel Dovgalyuk' , 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 > From: Kevin Wolf [mailto:kwolf@redhat.com] > Am 05.12.2016 um 08:43 hat Pavel Dovgalyuk geschrieben: > > 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: > > > > > > > > > > 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. > > 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. > 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 This seems reasonable for manual way. > 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 > 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 Pavel Dovgalyuk