Re: [Qemu-devel] [PATCH 16/19] block: protect modification of dirty bitmaps with a mutex

[Vladimir Sementsov-Ogievskiy]

27.06.2017 12:27, Paolo Bonzini wrote:
> On 27/06/2017 11:07, Vladimir Sementsov-Ogievskiy wrote:
> > 26.06.2017 19:54, Paolo Bonzini wrote:
> > > On 26/06/2017 18:07, Vladimir Sementsov-Ogievskiy wrote:
> > > > HI!
> > > >
> > > > One question here, should not 'bdrv_undo_clear_dirty_bitmap' be under
> > > > lock too?
> > > Any call to dirty bitmap functions between bdrv_clear_dirty_bitmap and
> > > bdrv_undo_clear_dirty_bitmap is problematic anyway, so
> > > bdrv_clear_dirty_bitmap really only needs the lock in the !out case;
> > > bdrv_undo_clear_dirty_bitmap is only called when out != NULL.
> > >
> > > However, I agree it would be cleaner to add the lock there, too.
> > Also, you've added comment "Called with BQL taken" both to functions
> > that calls bdrv_dirty_bitmaps_lock and that do not... What is BQL?
> It's the "big QEMU lock", also known as "iothread lock".  The locking policy
> is documented in block_int.h:
>
>      /* Writing to the list requires the BQL _and_ the dirty_bitmap_mutex.
>       * Reading from the list can be done with either the BQL or the
>       * dirty_bitmap_mutex.  Modifying a bitmap only requires
>       * dirty_bitmap_mutex.  */
>      QemuMutex dirty_bitmap_mutex;
>      QLIST_HEAD(, BdrvDirtyBitmap) dirty_bitmaps;
>
> and the comments in block/dirty-bitmap.c reflect the above comment.
>
> Paolo

bdrv_enable_dirty_bitmap - writes to the list, it changes 'disabled' 
field. So it requires both BQL and dirty_bitmap_mutex? But the comment 
says only about BQL.

Also, for example, if I want to create a new bitmap and than somehow 
change it, should I do it like this:

bdrv_create_dirty_bitmap(...)

bdrv_dirty_bitmaps_lock(bs)

bitmap  = bdrv_find_dirty_bitmap(bs, name)

<some changes>

bdrv_dirty_bitmaps_unlock(bs)

- because we can't now trust the pointer returned by 
bdrv_create_dirty_bitmap, as it releases bitmap lock before return.




--
Best regards,
Vladimir