Re: [Qemu-block] [Qemu-devel] [PATCH v2 3/3] block/qcow2: proper locking on bitmap add/remove paths

[John Snow]

On 9/11/19 11:00 AM, Vladimir Sementsov-Ogievskiy wrote:
> qmp_block_dirty_bitmap_add and do_block_dirty_bitmap_remove do acquire
> aio context since 0a6c86d024c52b. But this is not enough: we also must
> lock qcow2 mutex when access in-image metadata. Especially it concerns
> freeing qcow2 clusters.
> 
> To achieve this, move qcow2_can_store_new_dirty_bitmap and
> qcow2_remove_persistent_dirty_bitmap to coroutine context.
> 
> Since we work in coroutines in correct aio context, we don't need
> context acquiring in blockdev.c anymore, drop it.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>

It makes sense to me given the context, but I'll want Kevin or Max to
eye it over for locking correctness, admittedly.

Here's my good faith RB:

Reviewed-by: John Snow <address@hidden>


(I'm not so sure about the aio_wait_kick. Why is that needed here? ...

oh, 4720cbeea1f42fd905fc69338fd42b191e58b412 explains why.
I suppose any synchronous caller of a co_ function needs to make sure
that the entry shim kicks on the way out. This seems very easy to get
wrong, doesn't it?)