Re: [Qemu-devel] [PATCH for-4.1] block: Use bdrv_unref_child() for all children in bdrv_close()

[Max Reitz]

On 31.03.19 13:17, Alberto Garcia wrote:
> bdrv_unref_child() does the following things:
> 
>   - Updates the child->bs->inherits_from pointer.
>   - Calls bdrv_detach_child() to remove the BdrvChild from bs->children.
>   - Calls bdrv_unref() to unref the child BlockDriverState.
> 
> When bdrv_unref_child() was introduced in commit 33a604075c it was not
> used in bdrv_close() because the drivers that had additional children
> (like quorum or blkverify) had already called bdrv_unref() on their
> children during their own close functions.
> 
> This was changed later (in 0bd6e91a7e for quorum, in 3e586be0b2 for
> blkverify) so there's no reason not to use bdrv_unref_child() in
> bdrv_close() anymore.
> 
> After this there's also no need to remove bs->backing and bs->file
> separately from the rest of the children, so bdrv_close() can be
> simplified.
> 
> Signed-off-by: Alberto Garcia <address@hidden>
> ---
>  block.c | 16 +++-------------
>  1 file changed, 3 insertions(+), 13 deletions(-)

Hm, I’m afraid this breaks make check:

$ make -j 4 && make tests/test-bdrv-drain && gtester tests/test-bdrv-drain

TEST: tests/test-bdrv-drain... (pid=22321)
test-bdrv-drain: block.c:5397: bdrv_unref: Assertion `bs->refcnt > 0'
failed.

(gdb) bt
#0  0x00007f15c7ffc57f in raise () from /lib64/libc.so.6
#1  0x00007f15c7fe6895 in abort () from /lib64/libc.so.6
#2  0x00007f15c7fe6769 in __assert_fail_base.cold.0 () from /lib64/libc.so.6
#3  0x00007f15c7ff4a26 in __assert_fail () from /lib64/libc.so.6
#4  0x00005562a11b0a00 in bdrv_unref (bs=0x5562a3370c30) at block.c:5397
#5  bdrv_unref (bs=0x5562a3370c30) at block.c:5392
#6  0x00005562a11a7f8f in test_detach_indirect (by_parent_cb=<optimized
out>) at tests/test-bdrv-drain.c:1442
#7  0x00007f15c8766fca in ?? () from /lib64/libglib-2.0.so.0
#8  0x00007f15c8766e84 in ?? () from /lib64/libglib-2.0.so.0
#9  0x00007f15c8766e84 in ?? () from /lib64/libglib-2.0.so.0
#10 0x00007f15c8767282 in g_test_run_suite () from /lib64/libglib-2.0.so.0
#11 0x00007f15c87672a5 in g_test_run () from /lib64/libglib-2.0.so.0
#12 0x00005562a11a5b77 in main (argc=<optimized out>, argv=<optimized
out>) at tests/test-bdrv-drain.c:1617

So I’ll dequeue this patch for now.

Max

signature.asc
Description: OpenPGP digital signature