On Mon, Aug 25, 2014 at 05:37:37PM +0800, Fam Zheng wrote: > On Mon, 08/25 09:06, Benoît Canet wrote: > > On Mon, Aug 25, 2014 at 02:04:24PM +0800, Fam Zheng wrote: > > > On Fri, 08/22 18:11, Benoît Canet wrote: > > > > Since the block layer code is starting to modify the BDS graph right in > > > > the > > > > middle of BDS chains (block-mirror's replace parameter for example) > > > > QEMU needs > > > > to properly block and unblock whole BDS subtrees; recursion is a neat > > > > way to > > > > achieve this task. > > > > > > > > This patch also takes care of modifying the op blockers users. > > > > > > Is this going to replace backing_blocker? > > > > > > I think it is too general an approach to control the operation properly, > > > because the op blocker may not work in the same way for all types of BDS > > > connections. In other words, the choosing of op blockers are likely > > > conditional on graph edge types, that's why backing_blocker was added > > > here. For > > > example, A VMDK extent connection will probably need a different set of > > > blockers than bs->file connection. > > > > > > So could you explain in which cases is the recursive blocking/unblocking > > > useful? > > > > It's designed for the new crop of block operations operating on BDS located > > in > > the middle of the backing chain: Jeff's patches, intermediate live > > streaming or > > intermediate mirroring. > > Recursively blocking BDS allows to do these operations safely. > > Sorry I may be slow on this, but it's still not clear to me. > > That doesn't immediately show how backing_blocker doesn't work. These > operations are in the category of operations that update graph topology, > meaning that they drop, add or swap some nodes in the middle of the chain. It > is not safe because there are used by the other nodes, but they are supposed > to > be protected by backing_blocker. Could you be more specific?
I don't know particularly about the backing blocker case. > > I can think of something more than backing_hd: there are also link types other > than backing_hd, for example ->file, (vmdk)->extents or (quorum)->qcrs, etc. > They should be protected as well. This patch takes cares of recursing everywhere. I can give you an example for quorum. If a streaming operation is running on a quorum block backend the recursive blocking will help to block any operation done directly on any of the children. It's usefull since we introduced drive-mirror replace which will replace an arbitrary node of a quorum at the end of the mirroring operation. > > But it seems to me that these are not recursive association, so we don't need > to apply the blocker recursively. Shouldn't it be enough to only block > bs->file > when assigning this pointer, and unblock it when unassigning? Maybe their is other reason to implements recursive blocker: I have not decided on my own to implements them. It's a decision that took place at the Stuttgart meeting. Best regards Benoît > > I'm not trying to push back this series. I am asking just because that my > understanding to the question still needs some forging. > > Fam
