| Message ID | 1471343175-14945-16-git-send-email-vsementsov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Aug 16, 2016 at 01:26:12PM +0300, Vladimir Sementsov-Ogievskiy wrote: > Reviewed-by: John Snow <jsnow@redhat.com> > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > block/dirty-bitmap.c | 1 + > include/qemu/hbitmap.h | 8 ++++++++ > qapi/block-core.json | 5 ++++- > util/hbitmap.c | 8 ++++++++ > 4 files changed, 21 insertions(+), 1 deletion(-) > > diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c > index 32aa6eb..f895fce 100644 > --- a/block/dirty-bitmap.c > +++ b/block/dirty-bitmap.c > @@ -365,6 +365,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs) > info->has_name = !!bm->name; > info->name = g_strdup(bm->name); > info->status = bdrv_dirty_bitmap_status(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 eb46475..8d4f836 100644 > --- a/include/qemu/hbitmap.h > +++ b/include/qemu/hbitmap.h > @@ -225,6 +225,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 2bbc027..5642a8a 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -419,11 +419,14 @@ > # > # @status: current status of the dirty bitmap (since 2.4) > # > +# @md5: md5 checksum (as a hexadecimal string) of the last bitmap level > +# (since 2.6) > +# > # Since: 1.3 > ## > { 'struct': 'BlockDirtyInfo', > 'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32', > - 'status': 'DirtyBitmapStatus'} } > + 'status': 'DirtyBitmapStatus', 'md5': 'str'} } > > ## > # @BlockInfo: > diff --git a/util/hbitmap.c b/util/hbitmap.c > index 6a13c12..4afb3d5 100644 > --- a/util/hbitmap.c > +++ b/util/hbitmap.c > @@ -669,3 +669,11 @@ void hbitmap_free_meta(HBitmap *hb) > hbitmap_free(hb->meta); > hb->meta = NULL; > } > + > +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); > +} Please use one of the qcrypto_hash_* functions, not GChecksum directly, as this ensures we use the hash impls that have been through crypto certification when needed by people shipping QEM. Regards, Daniel
On Tue, Aug 16, 2016 at 11:37:28AM +0100, Daniel P. Berrange wrote: > On Tue, Aug 16, 2016 at 01:26:12PM +0300, Vladimir Sementsov-Ogievskiy wrote: > > Reviewed-by: John Snow <jsnow@redhat.com> > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > > --- > > block/dirty-bitmap.c | 1 + > > include/qemu/hbitmap.h | 8 ++++++++ > > qapi/block-core.json | 5 ++++- > > util/hbitmap.c | 8 ++++++++ > > 4 files changed, 21 insertions(+), 1 deletion(-) > > > > diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c > > index 32aa6eb..f895fce 100644 > > --- a/block/dirty-bitmap.c > > +++ b/block/dirty-bitmap.c > > @@ -365,6 +365,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs) > > info->has_name = !!bm->name; > > info->name = g_strdup(bm->name); > > info->status = bdrv_dirty_bitmap_status(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 eb46475..8d4f836 100644 > > --- a/include/qemu/hbitmap.h > > +++ b/include/qemu/hbitmap.h > > @@ -225,6 +225,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 2bbc027..5642a8a 100644 > > --- a/qapi/block-core.json > > +++ b/qapi/block-core.json > > @@ -419,11 +419,14 @@ > > # > > # @status: current status of the dirty bitmap (since 2.4) > > # > > +# @md5: md5 checksum (as a hexadecimal string) of the last bitmap level > > +# (since 2.6) > > +# > > # Since: 1.3 > > ## > > { 'struct': 'BlockDirtyInfo', > > 'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32', > > - 'status': 'DirtyBitmapStatus'} } > > + 'status': 'DirtyBitmapStatus', 'md5': 'str'} } > > > > ## > > # @BlockInfo: > > diff --git a/util/hbitmap.c b/util/hbitmap.c > > index 6a13c12..4afb3d5 100644 > > --- a/util/hbitmap.c > > +++ b/util/hbitmap.c > > @@ -669,3 +669,11 @@ void hbitmap_free_meta(HBitmap *hb) > > hbitmap_free(hb->meta); > > hb->meta = NULL; > > } > > + > > +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); > > +} > > Please use one of the qcrypto_hash_* functions, not GChecksum directly, > as this ensures we use the hash impls that have been through crypto > certification when needed by people shipping QEM. Oh and why MD5 here. Can we use a modern strong algorithm SHA256 instead of MD5 which is known to be susceptible to collisions Regards, Daniel
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index 32aa6eb..f895fce 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -365,6 +365,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs) info->has_name = !!bm->name; info->name = g_strdup(bm->name); info->status = bdrv_dirty_bitmap_status(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 eb46475..8d4f836 100644 --- a/include/qemu/hbitmap.h +++ b/include/qemu/hbitmap.h @@ -225,6 +225,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 2bbc027..5642a8a 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -419,11 +419,14 @@ # # @status: current status of the dirty bitmap (since 2.4) # +# @md5: md5 checksum (as a hexadecimal string) of the last bitmap level +# (since 2.6) +# # Since: 1.3 ## { 'struct': 'BlockDirtyInfo', 'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32', - 'status': 'DirtyBitmapStatus'} } + 'status': 'DirtyBitmapStatus', 'md5': 'str'} } ## # @BlockInfo: diff --git a/util/hbitmap.c b/util/hbitmap.c index 6a13c12..4afb3d5 100644 --- a/util/hbitmap.c +++ b/util/hbitmap.c @@ -669,3 +669,11 @@ void hbitmap_free_meta(HBitmap *hb) hbitmap_free(hb->meta); hb->meta = NULL; } + +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); +}