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 <be...@igalia.com> > --- > 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
signature.asc
Description: OpenPGP digital signature
