bdrv_attach_child_common() doesn't require tran_finalize() on failure (it does tran_add() only on success path). Still tran_new() must be paired with tran_finalize() anyway, at least to free empty Transaction object itself.
So, refactor the function for clean finalization code, same on all paths. While being here, also leave a comment on unobvious background zeroing of child pointer on failure path. Reported-by: Coverity (CID 1452773) Reported-by: Peter Maydell <peter.mayd...@linaro.org> Fixes: 548a74c0dbc858edd1a7ee3045b5f2fe710bd8b1 Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> --- block.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index 874c22c43e..728aa34b2f 100644 --- a/block.c +++ b/block.c @@ -2918,12 +2918,18 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs, child_role, perm, shared_perm, opaque, &child, tran, errp); if (ret < 0) { - bdrv_unref(child_bs); - return NULL; + goto out; } ret = bdrv_refresh_perms(child_bs, errp); + if (ret < 0) { + goto out; + } + +out: tran_finalize(tran, ret); + /* child is unset on failure by bdrv_attach_child_common_abort() */ + assert((ret < 0) == !child); bdrv_unref(child_bs); return child; -- 2.29.2