> From: Kevin Wolf [mailto:kw...@redhat.com] > Am 05.12.2016 um 08:43 hat Pavel Dovgalyuk geschrieben: > > Paolo, > > > > > From: Kevin Wolf [mailto:kw...@redhat.com] > > > Am 21.11.2016 um 13:22 hat Pavel Dovgalyuk geschrieben: > > > > > From: Kevin Wolf [mailto:kw...@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