27.06.2017 16:58, Paolo Bonzini wrote:
On 27/06/2017 15:51, Vladimir Sementsov-Ogievskiy wrote:
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.
If you have the big QEMU lock (you do if you are in the QEMU monitor),
you are protected from changes to the list of bitmaps.
Paolo
but you wrote "Writing to the list requires the BQL _and_ the
dirty_bitmap_mutex".
should it be BQL only?
bdrv_dirty_bitmaps_lock/unlock is (should be) called by the functions
that write to the list. I hope I can post the patch today already.
Paolo
I'm likely not right, but for me introducing this mutex looks like dirty
bitmaps may be accessed from concurrent threads. So for me it looks
strange that only accessors are protected by the mutex and not the whole
transactions.
One example I've wrote above, other examples from code: are
qmp_block_dirty_bitmap** functions:
bdrv_create_dirty_bitmap() {
bdrv_find_dirty_bitmap()
....
bdrv_dirty_bitmaps_lock(bs);
QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
bdrv_dirty_bitmaps_unlock(bs);
}
- we protect inserting into list from other threads, but what prevent
creating bitmap with the same name from other thread after
bdrv_find_dirty_bitmap() and before bdrv_dirty_bitmaps_lock() ?
-----
qmp_block_dirty_bitmap_clear() {
bitmap = block_dirty_bitmap_lookup()
....
bdrv_clear_dirty_bitmap()
}
- bdrv_clear_dirty_bitmap is protected by lock, but what prevent
deleting this bitmap by other thread between block_dirty_bitmap_lookup()
and bdrv_clear_dirty_bitmap() ?
--- it is not the whole list ---
--
Best regards,
Vladimir
