Re: [PATCH v2 3/7] block: make bdrv_drop_intermediate() less wrong

[Alberto Garcia]

On Fri 06 Nov 2020 01:42:37 PM CET, Vladimir Sementsov-Ogievskiy wrote:
> First, permission update loop tries to do iterations transactionally,
> but the whole update is not transactional: nobody roll-back successful
> loop iterations when some iteration fails.
>
> Second, in the iteration we have nested permission update:
> c->klass->update_filename may point to bdrv_child_cb_update_filename()
> which calls bdrv_backing_update_filename(), which may do node reopen to
> RW.
>
> Permission update system is not prepared to nested updates, at least it
> has intermediate permission-update state stored in BdrvChild
> structures: has_backup_perm, backup_perm and backup_shared_perm.
>
> So, let's first do bdrv_replace_node_common() (which is more
> transactional than open-coded update in bdrv_drop_intermediate()) and
> then call update_filename() in separate. We still do not rollback
> changes in case of update_filename() failure but it's not much worse
> than pre-patch behavior.
>
> Note that bdrv_replace_node_common() does check for frozen children,
> so corresponding check is dropped in bdrv_drop_intermediate().
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto