Re: [PATCH v2 08/10] block: Let replace_child_tran keep indirect pointer

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

11.11.2021 15:08, Hanna Reitz wrote:
> As of a future commit, bdrv_replace_child_noperm() will clear the
> indirect BdrvChild pointer passed to it if the new child BDS is NULL.
> bdrv_replace_child_tran() will want to let it do that, but revert this
> change in its abort handler.  For that, we need to have it receive a
> BdrvChild ** pointer, too, and keep it stored in the
> BdrvReplaceChildState object that we attach to the transaction.
> 
> Note that we do not need to store it in the BdrvReplaceChildState when
> new_bs is not NULL, because then there is nothing to revert.  This is
> important so that bdrv_replace_node_noperm() can pass a pointer to a
> loop-local variable to bdrv_replace_child_tran() without worrying that
> this pointer will outlive one loop iteration.
> 
> (Of course, for that to work, bdrv_replace_node_noperm() and in turn
> bdrv_replace_node() and its relatives may not be called with a NULL @to
> node.  Luckily, they already are not, but now we should assert this.)
> 
> bdrv_remove_file_or_backing_child() on the other hand needs to ensure
> that the indirect pointer it passes will stay valid for the duration of
> the transaction.  Ensure this by keeping a strong reference to the BDS
> whose &bs->backing or &bs->file it passes to bdrv_replace_child_tran(),
> and giving up that reference only in the transaction .clean() handler.
> 
> Signed-off-by: Hanna Reitz<hreitz@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>


-- 
Best regards,
Vladimir