[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 08/16] block: Manage backing file references
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-devel] [PATCH v2 08/16] block: Manage backing file references in bdrv_set_backing_hd() |
Date: |
Mon, 05 Oct 2015 15:31:29 +0200 |
User-agent: |
Notmuch/0.13.2 (http://notmuchmail.org) Emacs/23.2.1 (i486-pc-linux-gnu) |
On Thu 01 Oct 2015 03:13:26 PM CEST, Kevin Wolf wrote:
> @@ -2428,12 +2434,9 @@ int bdrv_drop_intermediate(BlockDriverState *active,
> BlockDriverState *top,
> BlockDriverState *intermediate;
> BlockDriverState *base_bs = NULL;
> BlockDriverState *new_top_bs = NULL;
> - BlkIntermediateStates *intermediate_state, *next;
> + BlkIntermediateStates *intermediate_state;
> int ret = -EIO;
>
> - QSIMPLEQ_HEAD(states_to_delete, BlkIntermediateStates) states_to_delete;
> - QSIMPLEQ_INIT(&states_to_delete);
> -
> if (!top->drv || !base->drv) {
> goto exit;
> }
> @@ -2460,7 +2463,6 @@ int bdrv_drop_intermediate(BlockDriverState *active,
> BlockDriverState *top,
> while (intermediate) {
> intermediate_state = g_new0(BlkIntermediateStates, 1);
> intermediate_state->bs = intermediate;
> - QSIMPLEQ_INSERT_TAIL(&states_to_delete, intermediate_state, entry);
>
> if (backing_bs(intermediate) == base) {
> base_bs = backing_bs(intermediate);
The purpose of this while() loop in the original code was to make a list
of all the intermediate states whose references need to be dropped after
the bdrv_set_backing_hd() call. Since now bdrv_set_backing_hd() is the
one who manages the backing references, this list is no longer
necessary, and you are actually leaking the BlkIntermediateStates
objects here (see the rest of the patch below).
The loop is also useful to check that 'base' is indeed part of the
backing chain, so that we should keep.
> @@ -2483,17 +2485,8 @@ int bdrv_drop_intermediate(BlockDriverState *active,
> BlockDriverState *top,
> }
> bdrv_set_backing_hd(new_top_bs, base_bs);
>
> - QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_delete, entry,
> next) {
> - /* so that bdrv_close() does not recursively close the chain */
> - bdrv_set_backing_hd(intermediate_state->bs, NULL);
> - bdrv_unref(intermediate_state->bs);
> - }
> ret = 0;
> -
> exit:
> - QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_delete, entry,
> next) {
> - g_free(intermediate_state);
> - }
> return ret;
> }
Berto
- [Qemu-devel] [PATCH v2 15/16] block: Add and use bdrv_replace_in_backing_chain(), (continued)
- [Qemu-devel] [PATCH v2 15/16] block: Add and use bdrv_replace_in_backing_chain(), Kevin Wolf, 2015/10/08
- [Qemu-devel] [PATCH v2 03/16] blkverify: Convert s->test_file to BdrvChild, Kevin Wolf, 2015/10/08
- [Qemu-devel] [PATCH v2 06/16] block: Remove bdrv_open_image(), Kevin Wolf, 2015/10/08
- [Qemu-devel] [PATCH v2 13/16] block: Implement bdrv_append() without bdrv_swap(), Kevin Wolf, 2015/10/08
- [Qemu-devel] [PATCH v2 16/16] block: Remove bdrv_swap(), Kevin Wolf, 2015/10/08
- [Qemu-devel] [PATCH v2 08/16] block: Manage backing file references in bdrv_set_backing_hd(), Kevin Wolf, 2015/10/08
- [Qemu-devel] [PATCH v2 07/16] block: Convert bs->backing_hd to BdrvChild, Kevin Wolf, 2015/10/08
- [Qemu-devel] [PATCH v2 09/16] block: Split bdrv_move_feature_fields(), Kevin Wolf, 2015/10/08
- [Qemu-devel] [PATCH v2 02/16] vmdk: Use BdrvChild instead of BDS for references to extents, Kevin Wolf, 2015/10/08
- [Qemu-devel] [PATCH v2 04/16] quorum: Convert to BdrvChild, Kevin Wolf, 2015/10/08