06.03.2019 16:06, Vladimir Sementsov-Ogievskiy wrote:
> 06.03.2019 15:25, Vladimir Sementsov-Ogievskiy wrote:
>> 01.03.2019 22:15, John Snow wrote:
>>> Add an inconsistent bit to dirty-bitmaps that allows us to report a bitmap
>>> as
>>> persistent but potentially inconsistent, i.e. if we find bitmaps on a qcow2
>>> that have been marked as "in use".
>>>
>>> Signed-off-by: John Snow <js...@redhat.com>
>>> ---
>>> qapi/block-core.json | 13 +++++++++----
>>> include/block/dirty-bitmap.h | 2 ++
>>> block/dirty-bitmap.c | 19 +++++++++++++++++++
>>> 3 files changed, 30 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>>> index 6e543594b3..e639ef6d1c 100644
>>> --- a/qapi/block-core.json
>>> +++ b/qapi/block-core.json
>>> @@ -467,15 +467,20 @@
>>> # and cannot be modified via QMP or used by another operation.
>>> # Replaces `locked` and `frozen` statuses. (since 4.0)
>>> #
>>> -# @persistent: true if the bitmap will eventually be flushed to persistent
>>> -# storage (since 4.0)
>>> +# @persistent: true if the bitmap was stored on disk, is scheduled to be
>>> stored
>>> +# on disk, or both. (since 4.0)
>>> +#
>>> +# @inconsistent: true if this is a persistent bitmap that was improperly
>>> +# stored. Implies @persistent to be true; @recording and
>>> +# @busy to be false. This bitmap cannot be used. To remove
>>> +# it, use @block-dirty-bitmap-remove. (Since 4.0)
>>> #
>>> # Since: 1.3
>>> ##
>>> { 'struct': 'BlockDirtyInfo',
>>> 'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32',
>>> - 'recording': 'bool', 'busy': 'bool',
>>> - 'status': 'DirtyBitmapStatus', 'persistent': 'bool' } }
>>> + 'recording': 'bool', 'busy': 'bool', 'status':
>>> 'DirtyBitmapStatus',
>>> + 'persistent': 'bool', '*inconsistent': 'bool' } }
>>> ##
>>> # @Qcow2BitmapInfoFlags:
>>> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
>>> index ba8477b73f..bd1b6479df 100644
>>> --- a/include/block/dirty-bitmap.h
>>> +++ b/include/block/dirty-bitmap.h
>>> @@ -68,6 +68,7 @@ void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap
>>> *bitmap);
>>> void bdrv_dirty_bitmap_set_readonly(BdrvDirtyBitmap *bitmap, bool value);
>>> void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap,
>>> bool persistent);
>>> +void bdrv_dirty_bitmap_set_inconsistent(BdrvDirtyBitmap *bitmap);
>>> void bdrv_dirty_bitmap_set_busy(BdrvDirtyBitmap *bitmap, bool busy);
>>> void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap
>>> *src,
>>> HBitmap **backup, Error **errp);
>>> @@ -91,6 +92,7 @@ bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap
>>> *bitmap);
>>> bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
>>> bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
>>> bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap);
>>> +bool bdrv_dirty_bitmap_inconsistent(const BdrvDirtyBitmap *bitmap);
>>> bool bdrv_dirty_bitmap_busy(BdrvDirtyBitmap *bitmap);
>>> bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs);
>>> BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
>>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>>> index 980cae4fa3..9e8630e1ac 100644
>>> --- a/block/dirty-bitmap.c
>>> +++ b/block/dirty-bitmap.c
>>> @@ -46,6 +46,9 @@ struct BdrvDirtyBitmap {
>>> and this bitmap must remain unchanged
>>> while
>>> this flag is set. */
>>> bool persistent; /* bitmap must be saved to owner disk
>>> image */
>>> + bool inconsistent; /* bitmap is persistent, but inconsistent.
>>> + * It cannot be used at all in any way,
>>> except
>>> + * a QMP user can remove it. */
>>> bool migration; /* Bitmap is selected for migration, it
>>> should
>>> not be stored on the next inactivation
>>> (persistent flag doesn't matter until
>>> next
>>> @@ -464,6 +467,8 @@ BlockDirtyInfoList
>>> *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
>>> info->recording = bdrv_dirty_bitmap_recording(bm);
>>> info->busy = bdrv_dirty_bitmap_busy(bm);
>>> info->persistent = bm->persistent;
>>> + info->has_inconsistent = bm->inconsistent;
>>> + info->inconsistent = bm->inconsistent;
>>> entry->value = info;
>>> *plist = entry;
>>> plist = &entry->next;
>>> @@ -711,6 +716,15 @@ void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap
>>> *bitmap, bool persistent)
>>> qemu_mutex_unlock(bitmap->mutex);
>>> }
>>> +/* Called with BQL taken. */
>>> +void bdrv_dirty_bitmap_set_inconsistent(BdrvDirtyBitmap *bitmap)
>>> +{
>>> + qemu_mutex_lock(bitmap->mutex);
>>> + bitmap->inconsistent = true;
>>> + bitmap->disabled = true;
>>
>> Agree, that it worth to assert persistance
>
> Hmm, didn't we want to make it readonly too?
May be better not doing it to not interfere these two things.
>
>>
>>> + qemu_mutex_unlock(bitmap->mutex);
>>> +}
>>> +
>>> /* Called with BQL taken. */
>>> void bdrv_dirty_bitmap_set_migration(BdrvDirtyBitmap *bitmap, bool
>>> migration)
>>> {
>>> @@ -724,6 +738,11 @@ bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap
>>> *bitmap)
>>> return bitmap->persistent && !bitmap->migration;
>>> }
>>> +bool bdrv_dirty_bitmap_inconsistent(const BdrvDirtyBitmap *bitmap)
>>> +{
>>> + return bitmap->inconsistent;
>>> +}
>>> +
>>> bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs)
>>> {
>>> BdrvDirtyBitmap *bm;
>>>
>>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
>>
>> Sorry for a delay, I'm very busy with our rebase to 2.12 :(
>>
>
>
--
Best regards,
Vladimir
