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

[Eric Blake]

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.

> 
> 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

signature.asc
Description: OpenPGP digital signature