rt/snapraid
1
0
Fork 0
mirror of https://github.com/amadvance/snapraid.git synced 2026-10-26 19:01:57 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

Treat trusted REBUILD hash mismatches as persistent silent data corruption

Browse source
This commit is contained in:
Andrea Mazzoleni 2026-09-09 14:49:31 +02:00
parent 62ae6c3552
commit 4a616558ec
2 changed files with 59 additions and 45 deletions
Download patch file Download diff file Expand all files Collapse all files

46
cmdline/scrub.c
View file

@ -39,9 +39,10 @@
* CHG yes OLD not hash checked unsynced
* REP yes NEW hash checked unsynced
* DELETED no OLD/INVALID zero input unsynced
* REBUILD yes CURRENT hash checked unsynced
*
* BLK and REP have a hash describing their current file data and can therefore
* be checked directly.
* BLK, REP and REBUILD have a hash describing their current file data and can
* therefore be checked directly.
*
* CHG is deliberately different. Its stored hash describes the OLD data, so
* comparing the current file contents against block->hash would not validate
@ -65,18 +66,25 @@
* Error classification model
* --------------------------
*
* A hash or parity mismatch is a silent data error only when the corresponding
* data and parity position are expected to be synchronized.
* A current-data hash mismatch is a silent data error for BLK and REBUILD when
* the file metadata still matches the content state. REBUILD retains the
* trusted CURRENT hash inherited from BLK even though its physical parity is
* untrusted.
*
* If the file metadata differs from the content state, or any block at the
* position is CHG, REP or DELETED, a mismatch can be the normal consequence
* of an unsynchronized array. It is therefore reported as a soft error rather
* than as silent corruption.
* REP mismatches remain soft errors because REP represents a pending
* unsynchronized state. If the file metadata differs from the content state,
* a current-data hash mismatch may likewise be the normal consequence of a
* legitimate file change and is therefore reported as a soft error.
*
* The unsynced classification is conservative and affects only the meaning of
* a mismatch. It does not make the position fail automatically. If the data
* that can be checked is correct and physical parity already agrees with the
* current logical inputs, the scrub of that position can still complete.
* A parity mismatch is a silent parity error only when the complete position is
* expected to be synchronized. CHG, REP, DELETED and REBUILD make physical
* parity untrusted, so a parity mismatch at such a position is reported as a
* soft synchronization error.
*
* These classifications affect only the meaning of a mismatch. They do not
* make the position fail automatically. If the data that can be checked is
* correct and physical parity already agrees with the current logical inputs,
* the scrub of that position can still complete.
*
* I/O errors are tracked separately from logical or hash mismatches. A
* recoverable per-position I/O failure marks the position bad; errors severe
@ -633,14 +641,16 @@ static int state_scrub_process(struct snapraid_state* state, struct snapraid_par
/*
* Do this before skipping blocks without a current file.
*
* DELETED has no file contribution but still has invalid parity: physical
* parity may contain its OLD data. It must therefore make the whole position
* unsynced so a later parity mismatch is not reported as silent corruption.
* Invalid parity makes the whole position unsynced for parity comparison.
* It also makes data mismatches soft for pending states such as REP, except
* for REBUILD whose stored hash is the trusted CURRENT hash inherited from
* the synchronized BLK state.
*/
if (block_has_invalid_parity(block)) {
/* report that the block and the file are not synced */
/* report that the parity position is not synced */
block_is_unsynced = 1;
file_is_unsynced = 1;
if (block_state_get(block) != BLOCK_STATE_REBUILD)
file_is_unsynced = 1;
/* follow */
}
@ -724,7 +734,7 @@ static int state_scrub_process(struct snapraid_state* state, struct snapraid_par
state_usage_hash(state);
/*
* Only BLK and REP contain a hash for the current file data.
* BLK, REP and REBUILD contain a hash for the current file data.
*
* CHG deliberately skips this comparison because block->hash describes its
* OLD parity contribution. A CHG can still participate in the parity check

58
cmdline/sync.c
View file

@ -1142,13 +1142,17 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari
*/
if (block_has_invalid_parity(block)) {
/*
* Store it in the failed set, because
* the parity may be still computed with the previous content
* CHG, REP and DELETED may differ from the data represented by
* physical parity, so treat them as erasures during recovery.
* REBUILD instead has a trusted CURRENT hash and can remain a known
* input when it matches. Add it only below if that hash check fails.
*/
failed[failed_count].index = diskcur;
failed[failed_count].size = state->block_size;
failed[failed_count].block = block;
++failed_count;
if (block_state != BLOCK_STATE_REBUILD) {
failed[failed_count].index = diskcur;
failed[failed_count].size = state->block_size;
failed[failed_count].block = block;
++failed_count;
}
/*
* If the block has invalid parity, we have to update the parity
@ -1248,8 +1252,7 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari
if (block_has_updated_hash(block)) {
/* compare the hash */
if (memcmp(hash, block->hash, BLOCK_HASH_SIZE) != 0) {
/* if the file has invalid parity, it's a REP changed during the sync */
if (block_has_invalid_parity(block)) {
if (block_state == BLOCK_STATE_REP) {
log_tag("error:%" PRIu64 ":%s:%s: Unexpected data change\n", blockcur, disk->name, esc_tag(file->sub));
log_error(ESOFT, "Data change at file '%s' at position '%" PRIu64 "'\n", task->path, file_pos);
log_error(ESOFT, "WARNING! Unexpected data modification of a file without parity!\n");
@ -1264,28 +1267,25 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari
}
++soft_error;
/*
* If the file is changed, it means that it was modified during sync
* this isn't a serious error, so we skip this block, and continue with others
*/
error_on_this_block = 1;
continue;
} else { /* otherwise it's a BLK with silent error */
} else {
unsigned diff = memdiff(hash, block->hash, BLOCK_HASH_SIZE);
log_tag("error_data:%" PRIu64 ":%s:%s: Data error at position %" PRIu64 ", diff hash bits %u/%zu\n", blockcur, disk->name, esc_tag(file->sub), file_pos, diff, BLOCK_HASH_SIZE * 8);
log_error(EDATA, "Data error in file '%s' at position '%" PRIu64 "', diff hash bits %u/%zu\n", task->path, file_pos, diff, BLOCK_HASH_SIZE * 8);
/* save the failed block for the fix */
assert(block_state == BLOCK_STATE_BLK || block_state == BLOCK_STATE_REBUILD);
/* recover only data that failed its trusted CURRENT hash */
failed[failed_count].index = diskcur;
failed[failed_count].size = read_size;
failed[failed_count].block = block;
++failed_count;
/*
* Silent errors are very rare, and are not a signal that a disk
* is going to fail. So, we just continue marking the block as bad
* just like in scrub
* BLK and REBUILD contain trusted CURRENT hashes. When recovery is
* attempted, accept either reconstructed buffer only after validating
* it against that hash below.
*/
++silent_error;
silent_error_on_this_block = 1;
@ -1297,7 +1297,7 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari
if (!parity_needs_to_be_updated) {
/*
* For sure it's a CHG block, because EMPTY are processed before with "continue"
* and BLK and REP have "block_has_updated_hash()" as 1, and all the others
* and BLK, REP and REBUILD have "block_has_updated_hash()" as 1, and all the others
* have "parity_needs_to_be_updated" already at 1
*/
assert(block_state_get(block) == BLOCK_STATE_CHG);
@ -1362,9 +1362,11 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari
* new parity with bad data. Repaired data is kept in memory only.
*
* Recovery reconstructs the state represented by on-disk parity. For
* CHG/REP/DELETED blocks, current buffers may differ from on-disk parity,
* especially after an interrupted sync. Treating them as erasures prevents
* raid_rec() from trusting their current contents to recover a bad BLK.
* CHG/REP/DELETED blocks, current buffers cannot be assumed to represent
* the state encoded by physical parity, especially after an interrupted
* sync. Treating them as erasures prevents raid_rec() from trusting their
* current contents to recover a bad BLK. A REBUILD is an erasure only when
* its trusted CURRENT hash check failed.
*
* CHG blocks with known ZERO old values supply zero directly without
* consuming a parity equation. If an interrupted sync had already moved
@ -1372,7 +1374,9 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari
* discarded.
*
* After recovery, current buffers are restored for CHG/REP/DELETED:
* reconstructed old values must not roll back pending filesystem changes.
* reconstructed parity-history values must not replace current filesystem data.
* Recovered BLK and REBUILD buffers are instead accepted only after their
* trusted CURRENT hash is validated below.
*/
if (!error_on_this_block && !io_error_on_this_block && silent_error_on_this_block) {
unsigned failed_mac;
@ -1488,7 +1492,7 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari
unsigned char* block_copy = copy[failed[j].index];
unsigned block_state = block_state_get(failed[j].block);
if (block_state == BLOCK_STATE_BLK) {
if (block_state == BLOCK_STATE_BLK || block_state == BLOCK_STATE_REBUILD) {
size_t size = failed[j].size;
/* compute the hash of the recovered block */
@ -1512,9 +1516,9 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari
memset(block_buffer + size, 0, state->block_size - size);
} else {
/*
* Otherwise restore the content
* because we are not interested in the old state
* that it's recovered for CHG, REP and DELETED blocks
* CHG, REP and DELETED are not direct recovery targets. Restore
* current filesystem data after using the buffer only as an
* intermediate parity-history recovery input.
*/
memcpy(block_buffer, block_copy, state->block_size);
}