On Wed 17 Mar 2021 03:34:57 PM CET, Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> wrote: > We have too much comments for this feature. It seems better just don't > do it. Most of real users (tests don't count) have to create additional > reference. > > Drop also comment in external_snapshot_prepare: > - bdrv_append doesn't "remove" old bs in common sense, it sounds > strange > - the fact that bdrv_append can fail is obvious from the context > - the fact that we must rollback all changes in transaction abort is > known (it's the direct role of abort) > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
Reviewed-by: Alberto Garcia <be...@igalia.com> > @@ -4645,36 +4640,22 @@ int bdrv_replace_node(BlockDriverState *from, > BlockDriverState *to, > * bs_new must not be attached to a BlockBackend. > * > * This function does not create any image files. > - * > - * bdrv_append() takes ownership of a bs_new reference and unrefs it because > - * that's what the callers commonly need. bs_new will be referenced by the > old > - * parents of bs_top after bdrv_append() returns. If the caller needs to > keep a > - * reference of its own, it must call bdrv_ref(). > */ > int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top, > Error **errp) You could still mention explicitly that the old parents of @bs_top will add a new reference to @bs_new. Berto
