Re: [Qemu-devel] [PATCH v5 7/9] block: don't make snapshots for filters

[Pavel Dovgalyuk]

Paolo,

> From: Kevin Wolf [mailto:address@hidden
> Am 21.11.2016 um 13:22 hat Pavel Dovgalyuk geschrieben:
> > > From: Kevin Wolf [mailto:address@hidden
> > > Am 16.11.2016 um 10:49 hat Pavel Dovgalyuk geschrieben:
> > > > > From: Pavel Dovgalyuk [mailto:address@hidden
> > > >
> > > > 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