John Snow <js...@redhat.com> writes: > On 07/07/2017 09:53 AM, Markus Armbruster wrote: >> Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> writes: >> >>> 07.07.2017 12:00, Markus Armbruster wrote: >>>> "Daniel P. Berrange" <berra...@redhat.com> writes: >>>> >>>>> On Fri, Jul 07, 2017 at 10:05:22AM +0200, Markus Armbruster wrote: >>>>>> Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> writes: >>>>>> >>>>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >>>>>>> --- >>>>>>> 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. >> > > I... should we, though? It's a debug interface for testing only, > basically. Couldn't think of a better way to test it, and people > demanded tests. > > How's this for documentation: > > "The hash will be in an arbitrary format that changes every time you > look away from this specification. Any similarity, real or imagined, to > a canonical SHA-256 ASCII string is purely coincidental." > > Basically, I would actually rather go out of my way to obfuscate this > command, not document it... > > Maybe that's wrong-headed of me, but I still maintain that it's not > terribly important because I'd rather people never, ever try to use this > in production.
It is wrong-headed of you :) We mark stuff not covered by QMP's compatibility promise with x- prefixes, not with incomplete documentation, and not by obsfuscating it. People trying to use this in non-production need to know how it works, too. If you think people need to be scared away some more, feel free to write something like "currently SHA-256 ASCII". If that's not enough of a hint for you, you might add "but you shouldn't rely on that".