Re: [Qemu-devel] [PATCH RFC v3 12/14] qapi: add md5 checksum of last dirty bitmap level to query-block

[John Snow]

On 02/18/2015 09:00 AM, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>   block.c                | 1 +
>   include/qemu/hbitmap.h | 8 ++++++++
>   qapi/block-core.json   | 4 +++-
>   util/hbitmap.c         | 8 ++++++++
>   4 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/block.c b/block.c
> index 4cca55d..9532ccc 100644
> --- a/block.c
> +++ b/block.c
> @@ -5600,6 +5600,7 @@ BlockDirtyInfoList 
> *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
>           info->name = g_strdup(bm->name);
>           info->disabled = bm->disabled;
>           info->frozen = bdrv_dirty_bitmap_frozen(bm);
> +        info->md5 = hbitmap_md5(bm->bitmap);
>           entry->value = info;
>           *plist = entry;
>           plist = &entry->next;
> diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
> index 10ce05b..2fb748a 100644
> --- a/include/qemu/hbitmap.h
> +++ b/include/qemu/hbitmap.h
> @@ -188,6 +188,14 @@ void hbitmap_deserialize_zeroes(HBitmap *hb, uint64_t 
> start, uint64_t count);
>   void hbitmap_deserialize_finish(HBitmap *hb);
>
>   /**
> + * hbitmap_md5:
> + * @bitmap: HBitmap to operate on.
> + *
> + * Returns md5 checksum of the last level.
> + */
> +char *hbitmap_md5(const HBitmap *bitmap);
> +
> +/**
>    * hbitmap_free:
>    * @hb: HBitmap to operate on.
>    *
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 25dea80..2028d37 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -336,11 +336,13 @@
>   #
>   # @frozen: whether the dirty bitmap is frozen (Since 2.3)
>   #
> +# @md5: md5 checksum of the last bitmap level (since 2.3)
> +#
>   # Since: 1.3
>   ##
>   { 'type': 'BlockDirtyInfo',
>     'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32',
> -           'disabled': 'bool', 'frozen': 'bool'} }
> +           'disabled': 'bool', 'frozen': 'bool', 'md5': 'str'} }
>
>   ##
>   # @BlockInfo:
> diff --git a/util/hbitmap.c b/util/hbitmap.c
> index 1a736e7..8063dce 100644
> --- a/util/hbitmap.c
> +++ b/util/hbitmap.c
> @@ -523,3 +523,11 @@ bool hbitmap_merge(HBitmap *a, const HBitmap *b)
>
>       return true;
>   }
> +
> +char *hbitmap_md5(const HBitmap *bitmap)
> +{
> +    uint64_t size =
> +        MAX((bitmap->size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
> +    const guchar *data = (const guchar *)bitmap->levels[HBITMAP_LEVELS - 1];
> +    return g_compute_checksum_for_data(G_CHECKSUM_MD5, data, size);
> +}

It strikes me as somewhat odd to introduce a feature for the explicit 
purpose of regression testing, but I can't think of how else we'd do it 
simply, so this makes the most sense to me right now.

Reviewed-by: John Snow <address@hidden>