Re: [PATCH v2 4/7] block-copy: add a CoMutex to the BlockCopyTask list

[Paolo Bonzini]

On 25/05/21 12:25, Vladimir Sementsov-Ogievskiy wrote:
> > > Next, even if we take bitmaps lock in 
> > > bdrv_dirty_bitmap_next_dirty_area() or around it, it doesn't bring 
> > > thread-safety to block_copy_task_create():
> >
> > The simplest solution here seems to protect 
> > bdrv_dirty_bitmap_next_dirty_area and also bdrv_reset_dirty_bitmap 
> > with the tasks lock, so that once it is released the bitmap is updated 
> > accordingly and from my understanding no other task can get that region.
>
> Yes, we just need to protect larger areas by outer lock, to protect the 
> logic, not only structures itself.

Locks protects data, not code; code must ensure invariants are respected 
at the end of critical sections.  Here we have both problems:

- it's protecting too little data (the bitmap is not protected).  This 
is a block/dirty-bitmap.c bug.

- it's not respecting the invariant that tasks always reflected a 
superset of what is set in the dirty bitmap.  This is solved by making 
the critical section larger.

Paolo