qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/3] qemu-img: Add checksum command


From: Nir Soffer
Subject: Re: [PATCH 1/3] qemu-img: Add checksum command
Date: Mon, 28 Nov 2022 12:37:31 +0200

On Mon, Nov 7, 2022 at 12:20 PM Hanna Reitz <hreitz@redhat.com> wrote:
On 30.10.22 18:37, Nir Soffer wrote:
> On Wed, Oct 26, 2022 at 4:00 PM Hanna Reitz <hreitz@redhat.com> wrote:
>
>     On 01.09.22 16:32, Nir Soffer wrote:
[...]
>     > ---
>     >   docs/tools/qemu-img.rst |  22 +++++
>     >   meson.build             |  10 ++-
>     >   meson_options.txt       |   2 +
>     >   qemu-img-cmds.hx        |   8 ++
>     >   qemu-img.c              | 191
>     ++++++++++++++++++++++++++++++++++++++++
>     >   5 files changed, 232 insertions(+), 1 deletion(-)
>     >
>     > diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
>     > index 85a6e05b35..8be9c45cbf 100644
>     > --- a/docs/tools/qemu-img.rst
>     > +++ b/docs/tools/qemu-img.rst
>     > @@ -347,20 +347,42 @@ Command description:
>     >       Check completed, image is corrupted
>     >     3
>     >       Check completed, image has leaked clusters, but is not
>     corrupted
>     >     63
>     >       Checks are not supported by the image format
>     >
>     >     If ``-r`` is specified, exit codes representing the image
>     state refer to the
>     >     state after (the attempt at) repairing it. That is, a
>     successful ``-r all``
>     >     will yield the exit code 0, independently of the image state
>     before.
>     >
>     > +.. option:: checksum [--object OBJECTDEF] [--image-opts] [-f
>     FMT] [-T SRC_CACHE] [-p] FILENAME
>     > +
>     > +  Print a checksum for image *FILENAME* guest visible content.
>
>     Why not say which kind of checksum it is?
>
>
> Do you mean the algorithm used? This may be confusing, for example we
> write
>
>    Print a sha256 checksum ...
>
> User will expect to get the same result from "sha256sum disk.img". How
> about
>
>    Print a blkhash checksum ...
>
> And add a link to the blkhash project?

I did mean sha256, but if it isn’t pure sha256, then a link to any
description how it is computed would be good, I think.

Ok, will link to https://gitlab.com/nirs/blkhash
 
[...]

>     > +  The checksum is not compatible with other tools such as
>     *sha256sum*.
>
>     Why not?  I can see it differs even for raw images, but why?  I would
>     have very much assumed that this gives me exactly what sha256sum
>     in the
>     guest on the guest device would yield.
>
>
> The blkhash is a construction based on other cryptographic hash
> functions (e.g. sha256).
> The way the hash is constructed is explained here:
> https://gitlab.com/nirs/blkhash/-/blob/master/blkhash.py#L52
>
> We can provide a very slow version using a single thread and no zero
> optimization
> that will create the same hash as sha256sum for raw image.

Ah, right.  Yes, especially zero optimization is likely to make a huge
difference.  Thanks for the explanation!

Maybe that could be mentioned here as a side note, though?  E.g. “The
checksum is not compatible with other tools such as *sha256sum* for
optimization purposes (to allow multithreading and optimized handling of
zero areas).”?

Ok, I will improve the text in the next version.
 
[...]
> In blksum I do not allow changing the block size.
>
> I'll add an assert in the next version to keeps this default optimal.

Thanks!  (Static assert should work, right?)

I think it should
 
Nir

reply via email to

[Prev in Thread] Current Thread [Next in Thread]