From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55403) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFD0t-0003qG-TU for qemu-devel@nongnu.org; Tue, 14 Jul 2015 23:05:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZFD0p-0003aP-FH for qemu-devel@nongnu.org; Tue, 14 Jul 2015 23:05:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35910) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFD0p-0003Zz-7u for qemu-devel@nongnu.org; Tue, 14 Jul 2015 23:05:35 -0400 Date: Wed, 15 Jul 2015 11:05:32 +0800 From: Fam Zheng Message-ID: <20150715030532.GB2412@ad.nay.redhat.com> References: <55A5BA2D.4060402@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55A5BA2D.4060402@cn.fujitsu.com> Subject: Re: [Qemu-devel] [PATCH v2] more check for replaced node List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wen Congyang Cc: Jeff Cody , qemu-devl , Stefan Hajnoczi On Wed, 07/15 09:41, Wen Congyang wrote: > We use mirror+replace to fix quorum's broken child. bs/s->common.bs > is quorum, and to_replace is the broken child. The new child is target_bs. > Without this patch, the replace node can be any node, and it can be > top BDS with BB, or another quorum's child. We just check if the broken > child is part of the quorum BDS in this patch. > > Signed-off-by: Wen Congyang > --- > block.c | 5 +++-- > block/mirror.c | 3 ++- > blockdev.c | 2 +- > include/block/block.h | 3 ++- > 4 files changed, 8 insertions(+), 5 deletions(-) > > diff --git a/block.c b/block.c > index d088ee0..090923c 100644 > --- a/block.c > +++ b/block.c > @@ -4077,7 +4077,8 @@ bool bdrv_is_first_non_filter(BlockDriverState *candidate) > return false; > } > > -BlockDriverState *check_to_replace_node(const char *node_name, Error **errp) > +BlockDriverState *check_to_replace_node(BlockDriverState *parent_bs, > + const char *node_name, Error **errp) > { > BlockDriverState *to_replace_bs = bdrv_find_node(node_name); > AioContext *aio_context; > @@ -4100,7 +4101,7 @@ BlockDriverState *check_to_replace_node(const char *node_name, Error **errp) > * Another benefit is that this tests exclude backing files which are > * blocked by the backing blockers. > */ > - if (!bdrv_is_first_non_filter(to_replace_bs)) { > + if (!bdrv_recurse_is_first_non_filter(parent_bs, to_replace_bs)) { > error_setg(errp, "Only top most non filter can be replaced"); > to_replace_bs = NULL; > goto out; > diff --git a/block/mirror.c b/block/mirror.c > index 238a070..b81077e 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -626,7 +626,8 @@ static void mirror_complete(BlockJob *job, Error **errp) > if (s->replaces) { > AioContext *replace_aio_context; > > - s->to_replace = check_to_replace_node(s->replaces, &local_err); > + s->to_replace = check_to_replace_node(s->common.bs, s->replaces, > + &local_err); Why is the check in qmp_drive_mirror not enough? Isn't this redundant? Fam > if (!s->to_replace) { > error_propagate(errp, local_err); > return; > diff --git a/blockdev.c b/blockdev.c > index c11611d..bf12e2e 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -2757,7 +2757,7 @@ void qmp_drive_mirror(const char *device, const char *target, > goto out; > } > > - to_replace_bs = check_to_replace_node(replaces, &local_err); > + to_replace_bs = check_to_replace_node(bs, replaces, &local_err); > > if (!to_replace_bs) { > error_propagate(errp, local_err); > diff --git a/include/block/block.h b/include/block/block.h > index 37916f7..608cd4e 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -317,7 +317,8 @@ bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs, > bool bdrv_is_first_non_filter(BlockDriverState *candidate); > > /* check if a named node can be replaced when doing drive-mirror */ > -BlockDriverState *check_to_replace_node(const char *node_name, Error **errp); > +BlockDriverState *check_to_replace_node(BlockDriverState *parent_bs, > + const char *node_name, Error **errp); > > /* async block I/O */ > typedef void BlockDriverDirtyHandler(BlockDriverState *bs, int64_t sector, > -- > 2.4.3 >