Am 18.01.2021 um 18:21 hat Vladimir Sementsov-Ogievskiy geschrieben: > 18.01.2021 17:18, Kevin Wolf wrote: > > Am 27.11.2020 um 15:44 hat Vladimir Sementsov-Ogievskiy geschrieben: > > > 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>
> > > diff --git a/blockdev.c b/blockdev.c > > > index b5f11c524b..96c96f8ba6 100644 > > > --- a/blockdev.c > > > +++ b/blockdev.c > > > @@ -1587,10 +1587,6 @@ static void > > > external_snapshot_prepare(BlkActionState *common, > > > goto out; > > > } > > > - /* This removes our old bs and adds the new bs. This is an operation > > > that > > > - * can fail, so we need to do it in .prepare; undoing it for abort is > > > - * always possible. */ > > > > This comment is still relevant, it's unrelated to the bdrv_ref(). > > I described in commit message, why I dislike this comment :) I can > keep it of course, it's not critical Ah, right, I missed this bit in the commit message (or forgot it until I reached this hunk) and thought it was an accidental removal. If it's intentional, no reason to change the patch. Kevin