From 3bf691ff7d1d15f81dbf6ec4091d5d215d646ad9 Mon Sep 17 00:00:00 2001 From: Andrea Mazzoleni Date: Sat, 12 Sep 2015 15:19:52 +0200 Subject: [PATCH] On data error, prints the number of different bits --- TODO | 6 ------ cmdline/check.c | 8 ++++++-- cmdline/scrub.c | 12 ++++++++---- cmdline/sync.c | 5 +++-- cmdline/util.c | 19 +++++++++++++++++++ cmdline/util.h | 5 +++++ 6 files changed, 41 insertions(+), 14 deletions(-) diff --git a/TODO b/TODO index e3be8bf2..375c7edb 100644 --- a/TODO +++ b/TODO @@ -14,12 +14,6 @@ because disk d6 disappeared, like I:\ drive missing, including its mount point a content file. In Linux likely the mount point would be still present, but not in Windows. The content file is instead missing in all platforms. -- In log, when hash doesn't match in recovering, make -sense to print such hash, to check why it doesn't match. -* A single bit error in hash ? -* Check if all the recovering attempts are recoving -the same bad data, meaning that likely the original hash was wrong ? - - Minor * Check how to reduce memory occupation using smaller memory block. diff --git a/cmdline/check.c b/cmdline/check.c index e2f2d7ad..1aa74ea3 100644 --- a/cmdline/check.c +++ b/cmdline/check.c @@ -1137,6 +1137,8 @@ static int state_check_process(struct snapraid_state* state, int fix, struct sna /* compare the hash */ if (memcmp(hash, block->hash, HASH_SIZE) != 0) { + unsigned diff = memdiff(hash, block->hash, HASH_SIZE); + /* save the failed block for the check/fix */ failed[failed_count].is_bad = 1; /* it's bad because the hash doesn't match */ failed[failed_count].is_outofdate = 0; @@ -1145,7 +1147,7 @@ static int state_check_process(struct snapraid_state* state, int fix, struct sna failed[failed_count].handle = &handle[j]; ++failed_count; - log_tag("error:%u:%s:%s: Data error at position %u\n", i, disk->name, esc(file->sub), block_file_pos(block)); + log_tag("error:%u:%s:%s: Data error at position %u, diff bits %u\n", i, disk->name, esc(file->sub), block_file_pos(block), diff); ++error; continue; } @@ -1237,9 +1239,11 @@ static int state_check_process(struct snapraid_state* state, int fix, struct sna /* check the parity */ for (l = 0; l < state->level; ++l) { if (buffer_recov[l] != 0 && memcmp(buffer_recov[l], buffer[diskmax + l], state->block_size) != 0) { + unsigned diff = memdiff(buffer_recov[l], buffer[diskmax + l], state->block_size); + buffer_recov[l] = 0; - log_tag("parity_error:%u:%s: Data error\n", i, lev_config_name(l)); + log_tag("parity_error:%u:%s: Data error, diff bits %u\n", i, lev_config_name(l), diff); ++error; } } diff --git a/cmdline/scrub.c b/cmdline/scrub.c index c90f148c..6795ce96 100644 --- a/cmdline/scrub.c +++ b/cmdline/scrub.c @@ -330,14 +330,16 @@ static int state_scrub_process(struct snapraid_state* state, struct snapraid_par if (block_has_updated_hash(block)) { /* compare the hash */ if (memcmp(hash, block->hash, HASH_SIZE) != 0) { - log_tag("error:%u:%s:%s: Data error at position %u\n", i, disk->name, esc(handle[j].file->sub), block_file_pos(block)); + unsigned diff = memdiff(hash, block->hash, HASH_SIZE); + + log_tag("error:%u:%s:%s: Data error at position %u, diff bits %u\n", i, disk->name, esc(handle[j].file->sub), block_file_pos(block), diff); /* it's a silent error only if we are dealing with synced files */ if (file_is_unsynced) { ++error; error_on_this_block = 1; } else { - log_error("Data error in file '%s' at position '%u'\n", handle[j].path, block_file_pos(block)); + log_error("Data error in file '%s' at position '%u', diff bits %u\n", handle[j].path, block_file_pos(block), diff); ++silent_error; silent_error_on_this_block = 1; } @@ -399,14 +401,16 @@ static int state_scrub_process(struct snapraid_state* state, struct snapraid_par /* compare the parity */ for (l = 0; l < state->level; ++l) { if (buffer_recov[l] && memcmp(buffer[diskmax + l], buffer_recov[l], state->block_size) != 0) { - log_tag("parity_error:%u:%s: Data error\n", i, lev_config_name(l)); + unsigned diff = memdiff(buffer[diskmax + l], buffer_recov[l], state->block_size); + + log_tag("parity_error:%u:%s: Data error, diff bits %u\n", i, lev_config_name(l), diff); /* it's a silent error only if we are dealing with synced blocks */ if (block_is_unsynced) { ++error; error_on_this_block = 1; } else { - log_fatal("Data error in parity '%s' at position '%u'\n", lev_config_name(l), i); + log_fatal("Data error in parity '%s' at position '%u', diff bits %u\n", lev_config_name(l), i, diff); ++silent_error; silent_error_on_this_block = 1; } diff --git a/cmdline/sync.c b/cmdline/sync.c index 05613e3f..df3b7804 100644 --- a/cmdline/sync.c +++ b/cmdline/sync.c @@ -772,8 +772,9 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari error_on_this_block = 1; continue; } else { /* otherwise it's a BLK with silent error */ - log_tag("error:%u:%s:%s: Data error at position %u\n", i, disk->name, esc(file->sub), block_file_pos(block)); - log_error("Data error in file '%s' at position '%u'\n", handle[j].path, block_file_pos(block)); + unsigned diff = memdiff(hash, block->hash, HASH_SIZE); + log_tag("error:%u:%s:%s: Data error at position %u, diff bits %u\n", i, disk->name, esc(file->sub), block_file_pos(block), diff); + log_error("Data error in file '%s' at position '%u', diff bits %u\n", handle[j].path, block_file_pos(block), diff); /* save the failed block for the fix */ failed[failed_count].index = j; diff --git a/cmdline/util.c b/cmdline/util.c index 2a3baf4f..d1ae0026 100644 --- a/cmdline/util.c +++ b/cmdline/util.c @@ -512,6 +512,25 @@ const char* hash_config_name(unsigned kind) } } +unsigned memdiff(const unsigned char* data1, const unsigned char* data2, unsigned size) +{ + unsigned i; + unsigned count; + + count = 0; + for (i = 0; i < size; ++i) { + unsigned j; + unsigned char diff = data1[i] ^ data2[i]; + for (j = 0; j < 8; ++j) { + if ((diff & 1) != 0) + ++count; + diff >>= 1; + } + } + + return 0; +} + /****************************************************************************/ /* lock */ diff --git a/cmdline/util.h b/cmdline/util.h index 7d394522..1a10ee8a 100644 --- a/cmdline/util.h +++ b/cmdline/util.h @@ -123,6 +123,11 @@ void memhash(unsigned kind, const unsigned char* seed, void* digest, const void* */ const char* hash_config_name(unsigned kind); +/** + * Count the number of different bits in the two buffers. + */ +unsigned memdiff(const unsigned char* data1, const unsigned char* data2, size_t size); + /****************************************************************************/ /* lock */