Skip to content
  • Vladimir Sementsov-Ogievskiy's avatar
    d669ed6a
    block: make bdrv_drop_intermediate() less wrong · d669ed6a
    Vladimir Sementsov-Ogievskiy authored
    
    
    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: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
    Message-Id: <20201106124241.16950-4-vsementsov@virtuozzo.com>
    Reviewed-by: default avatarAlberto Garcia <berto@igalia.com>
    Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
    d669ed6a
    block: make bdrv_drop_intermediate() less wrong
    Vladimir Sementsov-Ogievskiy authored
    
    
    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: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
    Message-Id: <20201106124241.16950-4-vsementsov@virtuozzo.com>
    Reviewed-by: default avatarAlberto Garcia <berto@igalia.com>
    Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
Loading