From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38425) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFjKc-0004sc-W3 for qemu-devel@nongnu.org; Thu, 16 Jul 2015 09:36:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZFjKX-0001OG-En for qemu-devel@nongnu.org; Thu, 16 Jul 2015 09:36:10 -0400 Received: from mail-wg0-x22a.google.com ([2a00:1450:400c:c00::22a]:35400) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFjKX-0001OA-7p for qemu-devel@nongnu.org; Thu, 16 Jul 2015 09:36:05 -0400 Received: by wgjx7 with SMTP id x7so58662493wgj.2 for ; Thu, 16 Jul 2015 06:36:04 -0700 (PDT) Date: Thu, 16 Jul 2015 14:36:01 +0100 From: Stefan Hajnoczi Message-ID: <20150716133601.GF16984@stefanha-thinkpad.redhat.com> References: <55A5BA2D.4060402@cn.fujitsu.com> <20150715030532.GB2412@ad.nay.redhat.com> <55A5D20C.8010605@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Mjqg7Yu+0hL22rav" Content-Disposition: inline In-Reply-To: <55A5D20C.8010605@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: benoit.canet@irqsave.net, Jeff Cody , Fam Zheng , qemu-devl , Stefan Hajnoczi --Mjqg7Yu+0hL22rav Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 > >> --- > >> 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; > >> } > >> =20 > >> -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 =3D bdrv_find_node(node_name); > >> AioContext *aio_context; > >> @@ -4100,7 +4101,7 @@ BlockDriverState *check_to_replace_node(const ch= ar *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 =3D 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; > >> =20 > >> - s->to_replace =3D check_to_replace_node(s->replaces, &local_e= rr); > >> + s->to_replace =3D check_to_replace_node(s->common.bs, s->repl= aces, > >> + &local_err); > >=20 > > Why is the check in qmp_drive_mirror not enough? Isn't this redundant? >=20 > 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? --Mjqg7Yu+0hL22rav Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVp7NBAAoJEJykq7OBq3PIzUEH/1kkinTPe639HmuWKUZrIngW 8JJ9gIFGcROx/wDLDDPNwY2B1sOgkioWty5JuD0Mo3ugDpOv9zbfo24ImwQMnbcG ToD2KSvGfXvsvXUz8iJ4IiRtNeM5S+BhD8kzD5F6secGXX/GY/yzWK9ZFmUq8oGA 4JD1swc3KVkJHPCyOOLolyiHDsjpA/WQTv+0WG0UWxa5S9iUGKUx9GuI++nxyUCr wSbFNmTuZS4350bR18gUktw2DrK2Vo+hEzTM0fYai5jxkO1MCYja72C9gBiguhSn Rvw0Q3I++K6LV+LHSHgPjo/IyMAJqsVvmYvyqefpn+BSOvV6KKYWB89osZHziBM= =cvSK -----END PGP SIGNATURE----- --Mjqg7Yu+0hL22rav--