Re: [PATCH 1/3] backup: Remove nodes from job in .clean()

[Kevin Wolf]

Am 19.02.2021 um 16:33 hat Max Reitz geschrieben:
> The block job holds a reference to the backup-top node (because it is
> passed as the main job BDS to block_job_create()).  Therefore,
> bdrv_backup_top_drop() cannot delete the backup-top node (replacing it
> by its child does not affect the job parent, because that has
> .stay_at_node set).  That is a problem, because all of its I/O functions
> assume the BlockCopyState (s->bcs) to be valid and that it has a
> filtered child; but after bdrv_backup_top_drop(), neither of those
> things are true.

This kind of suggests that block_copy_state_free() doesn't really belong
in bdrv_backup_top_drop(), but in a .bdrv_close callback.

Doesn't make this patch less correct, of course. We still want to have
all references dropped at the end of bdrv_backup_top_drop().

> It does not make sense to add new parents to backup-top after
> backup_clean(), so we should detach it from the job before
> bdrv_backup_top_drop().  Because there is no function to do that for a
> single node, just detach all of the job's nodes -- the job does not do
> anything past backup_clean() anyway.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>