Re: [Qemu-devel] loading bitmaps in invalidate_cache fails

[Kevin Wolf <kwolf@redhat.com>]

Am 11.09.2017 um 18:51 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Hi Kevin!
> 
> I'm confused with relations of permissions and invalidation, can you please
> help?
> 
> Now dirty bitmaps are loaded in invalidate_cache. Here is a problem with
> migration:
> 
> 1. destination starts (inactive)
> 
> 2. load bitmaps readonly
> 
> ...
> 
> 3. invalidate_cache: here we should make our loaded bitmaps RW, ie set
> BdrvDirtyBitmap->readonly
> 
>   to false and set IN_USE bit in the image. But the latter goes into
> "bdrv_aligned_pwritev: Assertion `child->perm & BLK_PERM_WRITE' failed",
> 
>   because in bdrv_invalidate_cache we call bdrv_set_perm after
> drv->bdrv_invalidate_cache.
> 
> 
> What is the true way of fixing this?

It's all still a bit of a mess. :-(

I think it makes a lot of sense that we should activate the lower layers
first, so the order in bdrv_invalidate_cache() looks wrong. It should be
something like this:

1. invalidate_cache() for the children
2. Update permissions for non-BDRV_O_INACTIVE
3. Call drv->bdrv_invalidate_cache()

I'm currently working on some fixes related to bdrv_reopen() where
things become tricky because the updated permissions shouldn't depend on
the current state, but on the state after the operation has finished.

You get something similar here, but I think just making sure that we
clear BDRV_O_INACTIVE before updating the permissions is enough here.
The only thing to be careful is that in error cases, we need to revert
both the flag and the permissions then.

Kevin