From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58589) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WG988-0000e1-Ct for qemu-devel@nongnu.org; Wed, 19 Feb 2014 10:32:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WG983-0001CA-9s for qemu-devel@nongnu.org; Wed, 19 Feb 2014 10:32:12 -0500 Received: from lnantes-156-75-100-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:50336 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WG982-0001C2-W5 for qemu-devel@nongnu.org; Wed, 19 Feb 2014 10:32:07 -0500 Date: Wed, 19 Feb 2014 16:32:06 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140219153206.GF20622@irqsave.net> References: <1392817351-22148-1-git-send-email-famz@redhat.com> <1392817351-22148-7-git-send-email-famz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1392817351-22148-7-git-send-email-famz@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v14 06/14] block: Add backing_blocker in BlockDriverState List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: kwolf@redhat.com, benoit.canet@irqsave.net, armbru@redhat.com, qemu-devel@nongnu.org, rjones@redhat.com, imain@redhat.com, stefanha@redhat.com, pbonzini@redhat.com The Wednesday 19 Feb 2014 =E0 21:42:23 (+0800), Fam Zheng wrote : > This makes use of op_blocker and blocks all the operations except for > commit target, on each BlockDriverState->backing_hd. >=20 > 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. >=20 > Signed-off-by: Fam Zheng > --- > block.c | 19 +++++++++++++++---- > include/block/block_int.h | 3 +++ > 2 files changed, 18 insertions(+), 4 deletions(-) >=20 > diff --git a/block.c b/block.c > index dec44d4..95d8c1f 100644 > --- a/block.c > +++ b/block.c > @@ -1044,19 +1044,33 @@ fail: > void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backi= ng_hd) > { > if (bs->backing_hd) { > + assert(error_is_set(&bs->backing_blocker)); > + bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker); > bdrv_unref(bs->backing_hd); > + } else if (backing_hd) { > + error_setg(&bs->backing_blocker, > + "device is used as backing hd of '%s'", > + bs->device_name); > } > =20 > bs->backing_hd =3D backing_hd; > if (!backing_hd) { > bs->backing_file[0] =3D '\0'; > bs->backing_format[0] =3D '\0'; > + if (error_is_set(&bs->backing_blocker)) { > + error_free(bs->backing_blocker); > + } > goto out; > } > pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_hd->fi= lename); > pstrcpy(bs->backing_format, sizeof(bs->backing_format), > backing_hd->drv ? backing_hd->drv->format_name : ""); > bdrv_ref(bs->backing_hd); > + > + bdrv_op_block_all(bs->backing_hd, bs->backing_blocker); > + /* Otherwise we won't be able to commit due to check in bdrv_commi= t */ > + bdrv_op_unblock(bs->backing_hd, BLOCK_OP_TYPE_COMMIT, > + bs->backing_blocker); > out: > bdrv_refresh_limits(bs); > } > @@ -1696,8 +1710,7 @@ void bdrv_close(BlockDriverState *bs) > =20 > if (bs->drv) { > if (bs->backing_hd) { > - bdrv_unref(bs->backing_hd); > - bs->backing_hd =3D NULL; > + bdrv_set_backing_hd(bs, NULL); > } > bs->drv->bdrv_close(bs); > g_free(bs->opaque); > @@ -1899,7 +1912,6 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDri= verState *bs_old) > assert(QLIST_EMPTY(&bs_new->dirty_bitmaps)); > assert(bs_new->job =3D=3D NULL); > assert(bs_new->dev =3D=3D NULL); > - assert(bdrv_op_blocker_is_empty(bs_new)); > assert(bs_new->io_limits_enabled =3D=3D false); > assert(!throttle_have_timer(&bs_new->throttle_state)); > =20 > @@ -1918,7 +1930,6 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDri= verState *bs_old) > /* Check a few fields that should remain attached to the device */ > assert(bs_new->dev =3D=3D NULL); > assert(bs_new->job =3D=3D NULL); > - assert(bdrv_op_blocker_is_empty(bs_new)); > assert(bs_new->io_limits_enabled =3D=3D false); > assert(!throttle_have_timer(&bs_new->throttle_state)); > =20 > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 1d3f76f..1f4f78b 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -369,6 +369,9 @@ struct BlockDriverState { > BlockJob *job; > =20 > QDict *options; > + > + /* The error object in use for blocking operations on backing_hd *= / > + Error *backing_blocker; > }; > =20 > int get_tmp_filename(char *filename, int size); > --=20 > 1.8.5.4 >=20 Reviewed-by: Benoit Canet