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

["Benoît Canet" <benoit.canet@irqsave.net>]

The Thursday 19 Jun 2014 à 14:26:04 (-0600), Eric Blake wrote :

> On 06/19/2014 02:20 PM, Benoît Canet wrote:
> 
> >> This unconditionally removes all blockers of type op of the subtree
> >>
> >> Yikes - is that really what we want?  Or do we need to start doing
> >> blocker reference counting?
> >>
> >> Consider:
> >>
> >> base <- snap1 <- active
> >>
> >> Looking at Jeff's proposal of making blockers based on access patterns
> >> rather than operations, we want the mere act of being a backing file to
> >> automatically put a guest_write block on base and snap1 (we must not
> >> modify the backing chain out of underneath active).  But now suppose we
> >> do two operations in parallel - we take a fleecing export of active, and
> >> we start a drive-mirror on active.
> >>
> >> base <- snap1 <- active
> >>               |        \-- fleecing
> >>               \-- copy
> >>
> >> Both of those actions should be doable in parallel, and both of them
> >> probably put additional blocker restrictions on the chain.  But if we
> >> unconditionally clear those additional restrictions on the first of the
> >> two jobs ending, that would inappropriately stop blocking that action
> >> from the still on-going second action.  The only way I see around that
> >> is via reference-counted blocking.  Definitely not 2.1 material (but
> >> good to be thinking about it now, so we can get it in early in the 2.2
> >> cycle).
> > 
> > I added this reset function for the case where a whole BDS subtree is detached
> > from the graph and will be destroyed.
> > 
> > It does happen in drive mirror and bdrv_unrefing it would lead to a failed
> > assertion.
> 
> Okay, you may have a use case there.  Or you may just be highlighting a
> bug.  Consider what happens if we have:
> 
> base <- snap1 <- active
> 
> then open a fleecing NBD view of snap1:
> 
> base <- snap1 <- active
>               \-- fleecing
> 
> then do a blockpull into active:
> 
> active
> base <- snap1 <- fleecing
> 
> that is, as long as the fleecing operation is live, we STILL need to
> block base and snap1 from modification; even though active is no longer
> dependent on them.  Dropping the backing chain of active decreases the
> reference count, but does not delete the BDS for base or snap1 because
> those BDS are still in use by the fleecing operation.

Blockers are already refcount like as they are implemented as linked list grouped
by operation types.
As long an error message  is still in a list the BDS is blocker.

Best regards

Benoît

> 
> > 
> > So the reset function take care of removing blocker of dead subtrees.
> > 
> > What would be a cleaner solution ?
> 
> I'm not honestly sure.  Which is why we're thinking about design gotchas
> at the moment ;)
> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>