[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block: Make op blockers recursive
From: |
Benoît Canet |
Subject: |
Re: [Qemu-devel] [PATCH] block: Make op blockers recursive |
Date: |
Mon, 25 Aug 2014 12:12:19 +0000 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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