qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v3 1/7] block/dirty-bitmaps: add inconsistent bi


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-block] [PATCH v3 1/7] block/dirty-bitmaps: add inconsistent bit
Date: Wed, 6 Mar 2019 12:25:59 +0000

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 <address@hidden>
> ---
>   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

> +    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 <address@hidden>

Sorry for a delay, I'm very busy with our rebase to 2.12 :(

-- 
Best regards,
Vladimir

reply via email to

[Prev in Thread] Current Thread [Next in Thread]