On Wed, Jul 15, 2015 at 11:22:52AM +0800, Wen Congyang wrote:
> On 07/15/2015 11:05 AM, Fam Zheng wrote:
> > 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 <we...@cn.fujitsu.com>
> >> ---
> >> 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?
>
> I don't know why we check it twice. And I think it is redundant too.Okay, does that mean you are dropping this patch now?
pgpKENas1DRIF.pgp
Description: PGP signature
