From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46626) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bZbpn-00078x-DB for qemu-devel@nongnu.org; Tue, 16 Aug 2016 06:43:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bZbpl-0002LK-40 for qemu-devel@nongnu.org; Tue, 16 Aug 2016 06:43:02 -0400 Date: Tue, 16 Aug 2016 11:42:47 +0100 From: "Daniel P. Berrange" Message-ID: <20160816104247.GA2120@redhat.com> Reply-To: "Daniel P. Berrange" References: <1471343175-14945-1-git-send-email-vsementsov@virtuozzo.com> <1471343175-14945-16-git-send-email-vsementsov@virtuozzo.com> <20160816103728.GE25115@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160816103728.GE25115@redhat.com> Subject: Re: [Qemu-devel] [PATCH 15/18] qapi: add md5 checksum of last dirty bitmap level to query-block List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladimir Sementsov-Ogievskiy Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, kwolf@redhat.com, peter.maydell@linaro.org, famz@redhat.com, lirans@il.ibm.com, quintela@redhat.com, jsnow@redhat.com, armbru@redhat.com, mreitz@redhat.com, stefanha@redhat.com, den@openvz.org, amit.shah@redhat.com, pbonzini@redhat.com, dgilbert@redhat.com 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 > > Signed-off-by: Vladimir Sementsov-Ogievskiy > > --- > > 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 -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|