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

[Fam Zheng]

On Thu, 04/20 14:00, Paolo Bonzini wrote:
> @@ -400,7 +431,8 @@ BlockDirtyInfoList 
> *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
>      return list;
>  }
>  
> -int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
> +/* Called within bdrv_dirty_bitmap_lock..unlock */
> +int bdrv_get_dirty_locked(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
>                     int64_t sector)

Parameter indentation is off now.

>  {
>      if (bitmap) {
> @@ -410,6 +442,18 @@ int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap 
> *bitmap,
>      }
>  }
>  
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 03db2cf..c264ead 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -598,8 +598,8 @@ struct BlockDriverState {
>  
>      /* 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 requires the AioContext
> -     * lock.  */
> +     * dirty_bitmap_mutex.  Modifying a bitmap only requires
> +     * dirty_bitmap_mutex.  */

I'm confused by this comment. What's added in this patch is bitmap->mutex, not
dirty_bitmap_mutex. Is it a mistake?

>      QemuMutex dirty_bitmap_mutex;
>      QLIST_HEAD(, BdrvDirtyBitmap) dirty_bitmaps;
>  

Fam