From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43726) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wmati-0004tz-Ag for qemu-devel@nongnu.org; Mon, 19 May 2014 23:39:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wmatd-0006H6-Gf for qemu-devel@nongnu.org; Mon, 19 May 2014 23:39:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:6359) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wmatd-0006H1-7U for qemu-devel@nongnu.org; Mon, 19 May 2014 23:39:21 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s4K3dJls001973 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Mon, 19 May 2014 23:39:19 -0400 Date: Tue, 20 May 2014 11:39:27 +0800 From: Fam Zheng Message-ID: <20140520033927.GD7688@T430.nay.redhat.com> References: <1399858555-9672-1-git-send-email-famz@redhat.com> <1399858555-9672-8-git-send-email-famz@redhat.com> <537A5CE9.9020801@redhat.com> <871tvpa56o.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <871tvpa56o.fsf@blackfin.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH v19 07/16] block: Add backing_blocker in BlockDriverState List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: kwolf@redhat.com, jcody@redhat.com, hbrock@redhat.com, qemu-devel@nongnu.org, rjones@redhat.com, imain@redhat.com, stefanha@redhat.com, pbonzini@redhat.com On Mon, 05/19 22:23, Markus Armbruster wrote: > Eric Blake writes: > > > On 05/11/2014 07:35 PM, Fam Zheng wrote: > >> This makes use of op_blocker and blocks all the operations except for > >> commit target, on each BlockDriverState->backing_hd. > >> > >> The asserts for op_blocker in bdrv_swap are removed because with this > >> change, the target of block commit has at least the backing blocker of > >> its child, so the assertion is not true. Callers should do their check. > >> > >> Signed-off-by: Fam Zheng > >> --- > >> block.c | 24 ++++++++++++++++++++---- > >> block/mirror.c | 1 + > >> include/block/block_int.h | 3 +++ > >> 3 files changed, 24 insertions(+), 4 deletions(-) > >> > >> diff --git a/block.c b/block.c > >> index ec26a2b..8155e68 100644 > >> --- a/block.c > >> +++ b/block.c > >> @@ -1097,14 +1097,31 @@ fail: > >> void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd) > >> { > >> > >> + if (bs->backing_hd) { > >> + assert(error_is_set(&bs->backing_blocker)); > > > > error_is_set() is going away. Please don't use it. > > https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03801.html > > > > This can just be assert(bs->backing_blocker). > > Yes. See commit 0fb6395. Another instance in PATCH 14. Yes. > > >> + bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker); > >> + } else if (backing_hd) { > >> + error_setg(&bs->backing_blocker, > >> + "device is used as backing hd of '%s'", > >> + bs->device_name); > >> + } > >> + > >> bs->backing_hd = backing_hd; > >> if (!backing_hd) { > >> + if (error_is_set(&bs->backing_blocker)) { > >> + error_free(bs->backing_blocker); > > > > if (bs->backing_blocker) { > > Actually, unconditional error_free(bs->backing_blocker) is fine. > Thanks, will update. Fam