[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v8 04/14] block/dirty-bitmap: add bdrv_dirty_bit
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH v8 04/14] block/dirty-bitmap: add bdrv_dirty_bitmap_set_frozen |
Date: |
Fri, 17 Nov 2017 12:20:31 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
On 11/17/2017 09:46 AM, Vladimir Sementsov-Ogievskiy wrote:
> 14.11.2017 02:32, John Snow wrote:
>>
>> On 10/30/2017 12:32 PM, Vladimir Sementsov-Ogievskiy wrote:
>>> Make it possible to set bitmap 'frozen' without a successor.
>>> This is needed to protect the bitmap during outgoing bitmap postcopy
>>> migration.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>>> ---
>>> include/block/dirty-bitmap.h | 1 +
>>> block/dirty-bitmap.c | 22 ++++++++++++++++++++--
>>> 2 files changed, 21 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
>>> index a9e2a92e4f..ae6d697850 100644
>>> --- a/include/block/dirty-bitmap.h
>>> +++ b/include/block/dirty-bitmap.h
>>> @@ -39,6 +39,7 @@ uint32_t
>>> bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
>>> uint32_t bdrv_dirty_bitmap_granularity(const BdrvDirtyBitmap *bitmap);
>>> bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
>>> bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap);
>>> +void bdrv_dirty_bitmap_set_frozen(BdrvDirtyBitmap *bitmap, bool
>>> frozen);
>>> const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap);
>>> int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap);
>>> DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap);
>>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>>> index 7578863aa1..67fc6bd6e0 100644
>>> --- a/block/dirty-bitmap.c
>>> +++ b/block/dirty-bitmap.c
>>> @@ -40,6 +40,8 @@ struct BdrvDirtyBitmap {
>>> QemuMutex *mutex;
>>> HBitmap *bitmap; /* Dirty bitmap implementation */
>>> HBitmap *meta; /* Meta dirty bitmap */
>>> + bool frozen; /* Bitmap is frozen, it can't be
>>> modified
>>> + through QMP */
>> I hesitate, because this now means that we have two independent bits of
>> state we need to update and maintain consistency with.
>
> it was your proposal)))
>
> https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg01172.html
>
> "
> Your new use case here sounds like Frozen to me, but it simply does not
> have an anonymous successor to force it to be recognized as "frozen." We
> can add a `bool protected` or `bool frozen` field to force recognition
> of this status and adjust the json documentation accordingly.
>
> I think then we'd have four recognized states:
>
> FROZEN: Cannot be reset/deleted. Bitmap is in-use by a block job or
> other internal process. Bitmap is otherwise ACTIVE.
> DISABLED: Not recording any writes (by choice.)
> READONLY: Not able to record any writes (by necessity.)
> ACTIVE: Normal bitmap status.
>
> Sound right?
> "
>
>
I was afraid you'd say that :(
It's okay, anyway. I shouldn't let myself go so long between reviews
like this, because you catch me changing my mind. Anyway, please go
ahead with it. I don't want to delay you on something that works because
I can't make up *my* mind.