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

[Benoît Canet]

On Thu, Sep 18, 2014 at 10:57:48AM +0800, Fam Zheng wrote:
> > > > 
> > > > BLOCK_OP_TYPE_MIRROR_REPLACE is checked and blocked by 
> > > > block-job-complete
> > > > during the time the mirror finish when an arbitrary node of the graph 
> > > > must be
> > > > replaced.
> > > 
> > > It seems to me mirror unblocks this operation from the job->blocker when 
> > > job
> > > starts, and never block it (with the job->blocker) again. It's leaked.
> > > 
> > 
> > block-job-complete will block it in mirror_complete.
> > 
> > BLOCK_OP_TYPE_MIRROR_REPLACE is blocked by driver-mirror code triggered by
> > block-job complete to block the "replaces" BDS during the end of the 
> > mirroring.
> > 
> > If you find silly that block-job-complete prevent itself from running twice 
> > on
> > the same BDS by checking the blocker then blocking it then the existing 
> > code is
> > wrong.
> > 
> > Else the code in this current patch is correct.
> > 
> > Could you have a glance at "static void mirror_complete(BlockJob *job, 
> > Error **errp)"
> > and tell me what you think about the situation. You should also look at
> > check_to_replace_node.
> > 
> 
> I'd prefer early error from user's point of view, so maybe checking and
> blocking can be done during mirror_start instead, then we don't need the
> relaxation. What's the advantage to delay the check to block-job-complete?
> 
> Fam

BLOCK_OP_TYPE_MIRROR_REPLACE is the blocker for the replacement work so I 
blocked
it where the replacement happen.
Another point is that until block-job-complete happen we are not completely sure
that this replacement operation will happen.

Best regards

Benoît