[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 09/25] block/dirty-bitmap: add read
From: |
John Snow |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 09/25] block/dirty-bitmap: add readonly field to BdrvDirtyBitmap |
Date: |
Wed, 31 May 2017 18:58:02 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 |
On 05/19/2017 07:02 PM, John Snow wrote:
>
>
> On 05/03/2017 08:25 AM, Vladimir Sementsov-Ogievskiy wrote:
>> It will be needed in following commits for persistent bitmaps.
>> If bitmap is loaded from read-only storage (and we can't mark it
>> "in use" in this storage) corresponding BdrvDirtyBitmap should be
>> read-only.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>> ---
>> block/dirty-bitmap.c | 16 ++++++++++++++++
>> include/block/dirty-bitmap.h | 3 +++
>> 2 files changed, 19 insertions(+)
>>
>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>> index 90af37287f..ab6a95cf41 100644
>> --- a/block/dirty-bitmap.c
>> +++ b/block/dirty-bitmap.c
>> @@ -44,6 +44,8 @@ struct BdrvDirtyBitmap {
>> int64_t size; /* Size of the bitmap (Number of sectors) */
>> bool disabled; /* Bitmap is read-only */
>> int active_iterators; /* How many iterators are active */
>> + bool readonly; /* Bitmap is read-only and may be changed
>> only
>> + by deserialize* functions */
>> QLIST_ENTRY(BdrvDirtyBitmap) list;
>> };
>>
>> @@ -436,6 +438,7 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>> int64_t cur_sector, int64_t nr_sectors)
>> {
>> assert(bdrv_dirty_bitmap_enabled(bitmap));
>> + assert(!bdrv_dirty_bitmap_readonly(bitmap));
>
> Doesn't this change the nature of the assertion?
>
> We're going from
> return !(bitmap->disabled || bitmap->successor);
> to
> !bitmap->readonly
>
> That makes me a little nervous to ACK this patch, because the
> correctness depends on how readonly is updated and manipulated in the
> future, which makes it more prone to error.
>
I must have been *REALLY* tired when I sent this; I had thought you were
replacing an assertion instead of just adding one.
I'm sorry about that.
>> hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
>> }
>>
>> @@ -443,12 +446,14 @@ void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
>> int64_t cur_sector, int64_t nr_sectors)
>> {
>> assert(bdrv_dirty_bitmap_enabled(bitmap));
>> + assert(!bdrv_dirty_bitmap_readonly(bitmap));
>> hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors);
>> }
>>
>> void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out)
>> {
>> assert(bdrv_dirty_bitmap_enabled(bitmap));
>> + assert(!bdrv_dirty_bitmap_readonly(bitmap));
>> if (!out) {
>> hbitmap_reset_all(bitmap->bitmap);
>> } else {
>> @@ -519,6 +524,7 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t
>> cur_sector,
>> if (!bdrv_dirty_bitmap_enabled(bitmap)) {
>> continue;
>> }
>> + assert(!bdrv_dirty_bitmap_readonly(bitmap));
>> hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
>> }
>> }
>> @@ -540,3 +546,13 @@ int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap
>> *bitmap)
>> {
>> return hbitmap_count(bitmap->meta);
>> }
>> +
>> +bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap)
>> +{
>> + return bitmap->readonly;
>> +}
>> +
>> +void bdrv_dirty_bitmap_set_readonly(BdrvDirtyBitmap *bitmap)
>> +{
>> + bitmap->readonly = true;
>> +}
>> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
>> index 1e17729ac2..0aab5841f5 100644
>> --- a/include/block/dirty-bitmap.h
>> +++ b/include/block/dirty-bitmap.h
>> @@ -75,4 +75,7 @@ void bdrv_dirty_bitmap_deserialize_ones(BdrvDirtyBitmap
>> *bitmap,
>> bool finish);
>> void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap);
>>
>> +bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);
>> +void bdrv_dirty_bitmap_set_readonly(BdrvDirtyBitmap *bitmap);
>> +
>> #endif
>>
- [Qemu-block] [PATCH v18 00/25] qcow2: persistent dirty bitmaps, Vladimir Sementsov-Ogievskiy, 2017/05/03
- [Qemu-block] [PATCH 13/25] block: introduce persistent dirty bitmaps, Vladimir Sementsov-Ogievskiy, 2017/05/03
- [Qemu-block] [PATCH 09/25] block/dirty-bitmap: add readonly field to BdrvDirtyBitmap, Vladimir Sementsov-Ogievskiy, 2017/05/03
- Re: [Qemu-block] [Qemu-devel] [PATCH 09/25] block/dirty-bitmap: add readonly field to BdrvDirtyBitmap, John Snow, 2017/05/19
- Re: [Qemu-block] [PATCH 09/25] block/dirty-bitmap: add readonly field to BdrvDirtyBitmap, Max Reitz, 2017/05/29
- Re: [Qemu-block] [PATCH 09/25] block/dirty-bitmap: add readonly field to BdrvDirtyBitmap, Max Reitz, 2017/05/29
- Re: [Qemu-block] [PATCH 09/25] block/dirty-bitmap: add readonly field to BdrvDirtyBitmap, Max Reitz, 2017/05/31
- Re: [Qemu-block] [PATCH 09/25] block/dirty-bitmap: add readonly field to BdrvDirtyBitmap, Vladimir Sementsov-Ogievskiy, 2017/05/31
- Re: [Qemu-block] [PATCH 09/25] block/dirty-bitmap: add readonly field to BdrvDirtyBitmap, Max Reitz, 2017/05/31
- Re: [Qemu-block] [PATCH 09/25] block/dirty-bitmap: add readonly field to BdrvDirtyBitmap, Vladimir Sementsov-Ogievskiy, 2017/05/31
- Re: [Qemu-block] [PATCH 09/25] block/dirty-bitmap: add readonly field to BdrvDirtyBitmap, Max Reitz, 2017/05/31