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

[Benoît Canet]

On Tue, Aug 26, 2014 at 12:42:04PM +0800, Fam Zheng wrote:
> On Mon, 08/25 12:12, Benoît Canet wrote:
> > 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.
> 
> At what points should block layer recursively block/unblock the operations in
> this quorum case?

When the streaming starts it should block all the top bs children.
So after when an operation tries to operate on a child of the top bs it will be
forbidden.

The beauty of it is that recursive blockers can easily replace regular blockers.

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