Re: [PATCH 2/2] block: bdrv_inactivate_recurse(): check for permissions and fix crash

[Hanna Reitz]

On 10.09.21 13:01, Vladimir Sementsov-Ogievskiy wrote:
> We must not inactivate child when parent has write permissions on
> it.
>
> Calling .bdrv_inactivate() doesn't help: actually only qcow2 has this
> handler and it is used to flush caches, not for permission
> manipulations.

I guess we could ask whether block jobs should implement 
.bdrv_inactivate() to cancel themselves, but I believe it’s indeed 
better to have the migration fail and thus force the user to manually 
cancel the job (should that be what they want).

> So, let's simply check cumulative parent permissions before
> inactivating the node.
>
> This commit fixes a crash when we do migration during backup: prior to
> the commit nothing prevents all nodes inactivation at migration finish
> and following backup write to the target crashes on assertion
> "assert(!(bs->open_flags & BDRV_O_INACTIVE));" in
> bdrv_co_write_req_prepare().
>
> After the commit, we rely on the fact that copy-before-write filter
> keeps write permission on target node to be able to write to it. So
> inactivation fails and migration fails as expected.
>
> Corresponding test now passes, so, enable it.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block.c                                        | 8 ++++++++
>   tests/qemu-iotests/tests/migrate-during-backup | 2 +-
>   2 files changed, 9 insertions(+), 1 deletion(-)

Reviewed-by: Hanna Reitz <hreitz@redhat.com>