Re: [Qemu-block] [Qemu-devel] [PATCH v22 25/30] qmp: add x-debug-block-dirty-bitmap-sha256

[Vladimir Sementsov-Ogievskiy]

07.07.2017 12:00, Markus Armbruster wrote:
> "Daniel P. Berrange" <address@hidden> writes:
>
> > On Fri, Jul 07, 2017 at 10:05:22AM +0200, Markus Armbruster wrote:
> > > Vladimir Sementsov-Ogievskiy <address@hidden> writes:
> > >
> > > > Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> > > > ---
> > > >   block/dirty-bitmap.c         |  5 +++++
> > > >   blockdev.c                   | 25 +++++++++++++++++++++++++
> > > >   include/block/dirty-bitmap.h |  1 +
> > > >   include/qemu/hbitmap.h       |  8 ++++++++
> > > >   qapi/block-core.json         | 27 +++++++++++++++++++++++++++
> > > >   tests/Makefile.include       |  2 +-
> > > >   util/hbitmap.c               | 11 +++++++++++
> > > >   7 files changed, 78 insertions(+), 1 deletion(-)
> > > [...]
> > > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > > index 5c42cc7790..6ad8585400 100644
> > > > --- a/qapi/block-core.json
> > > > +++ b/qapi/block-core.json
> > > > @@ -1644,6 +1644,33 @@
> > > >     'data': 'BlockDirtyBitmap' }
> > > >   
> > > >   ##
> > > > +# @BlockDirtyBitmapSha256:
> > > > +#
> > > > +# SHA256 hash of dirty bitmap data
> > > > +#
> > > > +# @sha256: ASCII representation of SHA256 bitmap hash
> > > Spell it SHA-256, please.  The member name @sha256 can stay.
> > >
> > > SHA-256 is 256 binary bits.  Please specify how they are represented in
> > > ASCII.  It better be base64 (RFC 4648), because we use that elsewhere.
> > It is filled later in this patch using qcrypto_hash_digest, so it is just
> > a hex string representing the hash, not base64. For the latter you can
> > use qcrypto_hash_base64
> I got two points:
>
> 1. Whatever encoding we use, it needs to be documented.
>
> 2. The fewer binary -> ASCII encodings we use, the better.  We already
> use base64.


ASCII format for check sum is more common as it is more readable. It is 
used in the internet to check downloads, it is used by standard utility 
sha256sum. So, it may be better for the monitor.

However, if it is needed, I can make a follow-up patch, it is very easy, 
just s/qcrypto_hash_digest/qcrypto_hash_base64/ in util/hbitmap.c. 
iotest 165 - the only user of the feature - doesn't need any changes.



--
Best regards,
Vladimir