|
From: | Vladimir Sementsov-Ogievskiy |
Subject: | Re: [Qemu-devel] [PATCH 1/4 for-2.11?] block/dirty-bitmap: add lock to bdrv_enable/disable_dirty_bitmap |
Date: | Thu, 16 Nov 2017 10:56:15 +0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
13.11.2017 20:50, Eric Blake wrote:
On 11/13/2017 10:20 AM, Vladimir Sementsov-Ogievskiy wrote:Like other setters here these functions should take a lock. Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden> --- block/dirty-bitmap.c | 4 ++++ 1 file changed, 4 insertions(+)Should this patch be in 2.11?diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index bd04e991b1..2a0bcd9e51 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -397,15 +397,19 @@ void bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, /* Called with BQL taken. */ void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap) { + bdrv_dirty_bitmap_lock(bitmap); assert(!bdrv_dirty_bitmap_frozen(bitmap)); bitmap->disabled = true; + bdrv_dirty_bitmap_unlock(bitmap);Why do we need this lock in addition to BQL?
in fact, comment about BQL should be dropped. 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;
}/* Called with BQL taken. */void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap) { + bdrv_dirty_bitmap_lock(bitmap); assert(!bdrv_dirty_bitmap_frozen(bitmap)); bitmap->disabled = false; + bdrv_dirty_bitmap_unlock(bitmap);Again, why do we need this in addition to BQL?
and here.
The commit message needs more details about a scenario where our existing BQL lock is insufficient to prevent misuse of bitmap->disabled.
-- Best regards, Vladimir
[Prev in Thread] | Current Thread | [Next in Thread] |