diff mbox

[15/18] qapi: add md5 checksum of last dirty bitmap level to query-block

Message ID 1471343175-14945-16-git-send-email-vsementsov@virtuozzo.com
State New
Headers show

Commit Message

Vladimir Sementsov-Ogievskiy Aug. 16, 2016, 10:26 a.m. UTC 
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(-)

Comments

Daniel P. Berrangé Aug. 16, 2016, 10:37 a.m. UTC | #1 
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
Daniel P. Berrangé Aug. 16, 2016, 10:42 a.m. UTC | #2 
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 mbox

Patch

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);
+}