Re: [Qemu-devel] [PATCH] block: Make op blockers recursive

[Kevin Wolf]

Am 10.09.2014 um 17:49 hat Benoît Canet geschrieben:
> On Wed, Sep 10, 2014 at 09:14:35AM -0600, Eric Blake wrote:
> > On 09/10/2014 02:54 AM, Fam Zheng wrote:
> > 
> > >> Let's think of a situation that recursive blockers protect but
> > >> backing_blocker does not:
> > >>
> > >> a <- b <- c <- d
> > >>
> > >> c is the backing file and is therefore protected by the op blocker.
> > >>
> > >> The block-commit command works with node-names, however, so we can
> > >> manipulate any nodes in the graph, not just the topmost one.  Try this:
> > >>
> > >> block-commit d
> > >> block-commit b
> > >>
> > >> I haven't checked yet but I suspect it will launch two block-commit jobs
> > >> on the same partial chain (that's a bad thing because it can lead to
> > >> corruption).
> > > 
> > > 1) Does block-commit work with node-names already? In other words, is
> > >    block-commit b possible now? I only see drive-mirror works with it, 
> > > but not
> > >    drive-backup, block-mirror or block-commit.
> > 
> > IIRC, Jeff Cody proposed patches for qemu 2.1 that would have done this,
> > but we dropped them for that release in order to get the recursive
> > blockers sorted out first.
> > 
> >  >
> > > 2) Regardless of the answer to 1), I think we could use a similar 
> > > approach as
> > >    drive-backup here: split BLOCK_OP_TYPE_COMMIT to
> > >    BLOCK_OP_TYPE_COMMIT_{SOURCE,TARGET}, and only unblock
> > >    BLOCK_OP_TYPE_COMMIT_TARGET in bdrv_set_backing_hd.
> > 
> > In that earlier thread, Jeff had some ideas that it is not so much the
> > operation name that should be the blocker, but the lower-level action(s)
> > implied by each operation (read metadata, write metadata, read image,
> > write image)
> 
> Does it mean I should pause this current series and task switch to another
> infrastucture task ?
> I could switch to the block I/O accouting work.
> 
> What does the other developpers and maintainers think about it ?

No, I think we should get this series, with comments addressed, merged.
It's not a solution for all eternity because it tends to be more
restrictive than necessary, but that's okay for now and it makes things
safer.

We'll have to discuss more about blockers at KVM Forum, but that's the
second part: Lifting unnecessary restrictions. This is tricky and won't
be done for 2.2, so in the meantime we want this patch (and I expect we
can reuse some parts of it even with Jeff's approach, so it won't be
wasted work).

We just shouldn't try to sink much time here because we know that it's
only preliminary. Let's just get something that works good enough for
now, it doesn't have to be perfect. Splitting into SOURCE/TARGET and
similar things to make it less restrictive are probably not worth it.

Kevin