[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 04/36] block: bdrv_append(): don't consume reference
From: |
Alberto Garcia |
Subject: |
Re: [PATCH v3 04/36] block: bdrv_append(): don't consume reference |
Date: |
Wed, 07 Apr 2021 19:46:38 +0200 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
On Wed 17 Mar 2021 03:34:57 PM CET, Vladimir Sementsov-Ogievskiy
<vsementsov@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 <vsementsov@virtuozzo.com>
Reviewed-by: Alberto Garcia <berto@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
- Re: [PATCH v3 04/36] block: bdrv_append(): don't consume reference,
Alberto Garcia <=