Re: [PATCH] btrfs-progs: Output proper csum string if csum mismatch

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1956 bytes --]



On 2020/3/4 下午10:28, David Sterba wrote:
> On Mon, Jan 20, 2020 at 05:32:05PM +0800, Qu Wenruo wrote:
>> Introduce a new helper, csum_to_string(), to convert binary csum to
>> string.
>>
>> And use it in check_extent_csums(), so that
>> "btrfs check --check-data-csum" could report the following human
>> readable output:
>>   mirror 1 bytenr 13631488 csum 0x13fec125 expected csum 0x98757625
>>
>> Other than the original octane one:
>>   mirror 1 bytenr 13631488 csum 19 expected csum 152
>>
>> It also has the extra handling for 32 bytes csum (e.g. SHA256).
>> For such long csum, it needs 66 characters (+2 for "0x") to just output
>> one hash, so this function would truncate them into the following
>> format:
>>  0xaabb...ccdd
>>     |       \- The tailing 2 bytes
>>     \--------- The leading 2 bytes
> 
> Kernel prints the whole checksum and also 2 on one line,
> btrfs_print_data_csum_error. The short version is ok for quick comparing
> but for consistency do you think it's too bad to print the whole
> checksum? Eg. when I see errors, copy&paste the checksum and can cross
> check with the kernel messages/logs.
> 

Personally speaking, for kernel I'm in the middle land, leaning a little
more towards shorter csum.
But I'm not confident enough for kernel, thus there is no such patch for
kernel submitted.

For longer csum, unless we have some intentional conflicting algo, it's
really hard to craft csum which makes the leading and tailing 8 bytes.

Furthermore, the only csum human can really use is the csum for all 0
page. Otherwise all csum makes not much sense to human.

But still, full csum may still make sense for certain corner case, and I
don't want even a small chance that user reports csum matches in dmesg
but still causing EIO.
So I hesitate for kernel patch.

As usual, you have the final say, and if kernel patch is needed I'm
pretty happy to submit.

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]