qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH for-4.1] block: Use bdrv_unref_child() for all c


From: Max Reitz
Subject: Re: [Qemu-block] [PATCH for-4.1] block: Use bdrv_unref_child() for all children in bdrv_close()
Date: Fri, 26 Apr 2019 23:44:50 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1

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.

Hm.  Sounds reasonable.  But the commit dates are interesting.  The TODO
was introduced in commit 33a604075c51, so actually Kevin wrote that
patch just one day before the patches for vmdk, quorum, and blkverify.
They hit the mailing list two months apart, though (July vs. September
2015).

But still I wonder whether there was really no other reason nobody
resolved this TODO until now.

Hm.  Perhaps because it looks like we want to keep the children list
central to the block layer so no block driver would have to ever even
call bdrv_unref_child().  But that sounds overly complicated for no
gain; and as long as e.g. quorum has the children list in its own state
object, it should unref all children when it deletes the list, because
that's the right thing to do.

So, yeah, I'm a bit wary, but I can't see why not.

> 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(-)

Thanks, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]