Re: [Qemu-block] [PATCH v2 2/3] block/fleecing-filter: new filter driver for fleecing

[Kevin Wolf]

Am 03.07.2018 um 18:11 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 03.07.2018 14:15, Kevin Wolf wrote:
> > We'll have to figure out where to fix this problem (or what it really
> > is, once you look more than just at fleecing), but I think requiring the
> > user to add a filter driver to work around missing serialisation in
> > other code, and corrupting their image if they forget to, is not a
> > reasonable solution.
> > 
> > I see at least two things wrong in this context:
> > 
> > * The permissions don't seem to match reality. The NBD server
> >    unconditionally shares PERM_WRITE, which is wrong in this case. The
> >    client wants to see a point-in-time snapshot that never changes. This
> >    should become an option so that it can be properly reflected in the
> >    permissions used.
> > 
> > * Once we have proper permissions, the fleecing setup breaks down
> >    because the guest needs PERM_WRITE on the backing file, but the
> >    fleecing overlay allows that only if the NBD client allows it (which
> >    it doesn't for fleecing).
> > 
> >    Now we can implement an exception right into backup that installs a
> >    backup filter driver between source and target if the source is the
> >    backing file of the target. The filter driver would be similar to the
> >    commit filter driver in that it simply promises !PERM_WRITE to its
> >    parents, but allows PERM_WRITE on the source because it has installed
> >    the before_write_notifier that guarantees this condition.
> > 
> >    All writes to the target that are made by the backup job in this setup
> >    (including before_write_notifier writes) need to be marked as
> >    serialising so that any concurrent reads are completed first.
> > 
> > And if we decide to add a target filter to backup, we should probably at
> > the same time use a filter driver for intercepting source writes instead
> > of using before_write_notifier.
> 
> Hmm, is it possible to do all the staff in one super filter driver, which we
> insert into the tree like this:
> 
> top blk        fleecing qcow2
>      +           +
>      |           |backing
>      v     <-----+
>    super filter
>      +
>      |file
>      v
>    active image
> 
> 
> And super filter do the following:
> 
> 1. copy-on-write, before forwarding write to file, it do serializing write
> to fleecing qcow2

This is where it breaks down. The filter driver in your graph doesn't
know fleecing.qcow2, so it can't write to it. Attaching fleecing.qcow2
as an additional child to the super filter doesn't work either because
you would create a loop then.

I think we need two separate nodes (and probably it's better to have
them managed by a block job so that both together can be checked to
result in a consistent setup).

> 2. fake .bdrv_child_perm for fleecing qcow2, like in block commit
> 
> and no block job is needed.

Kevin