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

[Markus Armbruster]

Vladimir Sementsov-Ogievskiy <address@hidden> writes:

> 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.

If the is a standard way to represent SHA-256 in ASCII, use it.

Whatever you use, document it clearly in the QAPI schema.