[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v14 06/14] block: Add backing_blocker in BlockDr
From: |
Jeff Cody |
Subject: |
Re: [Qemu-devel] [PATCH v14 06/14] block: Add backing_blocker in BlockDriverState |
Date: |
Wed, 19 Feb 2014 16:17:32 -0500 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Wed, Feb 19, 2014 at 09:42:23PM +0800, 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 <address@hidden>
> ---
> block.c | 19 +++++++++++++++----
> include/block/block_int.h | 3 +++
> 2 files changed, 18 insertions(+), 4 deletions(-)
>
> 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 *backing_hd)
> {
> if (bs->backing_hd) {
> + assert(error_is_set(&bs->backing_blocker));
When I run block-commit, on either the active or non-active layer, I
get an assertion here. The qemu-iotests do not catch it, and I
presume it is because happens a couple of seconds or so after the
success message is returned over QMP.
> + 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);
> }
>
> bs->backing_hd = backing_hd;
> if (!backing_hd) {
> bs->backing_file[0] = '\0';
> bs->backing_format[0] = '\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->filename);
> 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_commit */
> + 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)
>
> if (bs->drv) {
> if (bs->backing_hd) {
> - bdrv_unref(bs->backing_hd);
> - bs->backing_hd = 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,
> BlockDriverState *bs_old)
> assert(QLIST_EMPTY(&bs_new->dirty_bitmaps));
> assert(bs_new->job == NULL);
> assert(bs_new->dev == NULL);
> - assert(bdrv_op_blocker_is_empty(bs_new));
> assert(bs_new->io_limits_enabled == false);
> assert(!throttle_have_timer(&bs_new->throttle_state));
>
> @@ -1918,7 +1930,6 @@ void bdrv_swap(BlockDriverState *bs_new,
> BlockDriverState *bs_old)
> /* Check a few fields that should remain attached to the device */
> assert(bs_new->dev == NULL);
> assert(bs_new->job == NULL);
> - assert(bdrv_op_blocker_is_empty(bs_new));
> assert(bs_new->io_limits_enabled == false);
> assert(!throttle_have_timer(&bs_new->throttle_state));
>
> 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;
>
> QDict *options;
> +
> + /* The error object in use for blocking operations on backing_hd */
> + Error *backing_blocker;
> };
>
> int get_tmp_filename(char *filename, int size);
> --
> 1.8.5.4
>
>
- [Qemu-devel] [PATCH v14 02/14] block: Introduce op_blockers to BlockDriverState, (continued)
- [Qemu-devel] [PATCH v14 02/14] block: Introduce op_blockers to BlockDriverState, Fam Zheng, 2014/02/19
- [Qemu-devel] [PATCH v14 03/14] block: Replace in_use with operation blocker, Fam Zheng, 2014/02/19
- [Qemu-devel] [PATCH v14 04/14] block: Move op_blocker check from block_job_create to its caller, Fam Zheng, 2014/02/19
- [Qemu-devel] [PATCH v14 05/14] block: Add bdrv_set_backing_hd(), Fam Zheng, 2014/02/19
- [Qemu-devel] [PATCH v14 06/14] block: Add backing_blocker in BlockDriverState, Fam Zheng, 2014/02/19
[Qemu-devel] [PATCH v14 07/14] block: Parse "backing" option to reference existing BDS, Fam Zheng, 2014/02/19
[Qemu-devel] [PATCH v14 08/14] block: Support dropping active in bdrv_drop_intermediate, Fam Zheng, 2014/02/19