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