diff --git a/HISTORY b/HISTORY index 87857d87..9da2af95 100644 --- a/HISTORY +++ b/HISTORY @@ -1,6 +1,18 @@ SnapRAID HISTORY ================ +6.0 2014/1 (BETA) +================= + * In "sync", even if a silent error is found, still updates the + parity if it's possible to correct the error. + Note that the block will still be marked bad, and the data + will be fixed only at the next "fix -e" call. + But any new data added will be protected if you are using + enough parity to fix both the silent error and at least + another potential error. + * New source code organization. The RAID engine is now + an external component usable also in other projects. + 5.3 2014/1 (BETA) ================= * Don't warn about UUID changed if it's for an empty disk. diff --git a/Makefile.am b/Makefile.am index ff633af5..37ab17ba 100644 --- a/Makefile.am +++ b/Makefile.am @@ -226,6 +226,14 @@ endif $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) -o 0 scrub $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) -p 100 scrub $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) sync + echo --- Corrupt some files, and sync with error presents + $(TESTENV) ./mktest$(EXEEXT) change 2 10 1 test/disk1/_a/_a/* + $(TESTENV) ./mktest$(EXEEXT) change 2 10 1 test/disk1/_a/_b/* + $(TESTENV) ./mktest$(EXEEXT) damage 2 10 1 test/disk2/_a/_a/* + $(TESTENV) ./mktest$(EXEEXT) damage 2 10 1 test/disk2/_a/_b/* + $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) --test-expect-recoverable sync + $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) status + $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) fix -e echo --- Make a hole in the disk array and sync with --force-empty mv test/disk2 test/disk2.old mkdir test/disk2 diff --git a/TODO b/TODO index ecea3b8a..2bc27d8e 100644 --- a/TODO +++ b/TODO @@ -5,12 +5,6 @@ This is the list of TODO items for SnapRAID. - Next -* When a silent error is found in sync, try to recover the data, -and if succesfull compute the new parity as if data was read correctly. -But avoid to write the corrected data, and still mark the block as bad. -Keep the present behaviour of not proceeding with sync only if recovering -is not possible. - * When renaming to .unrecoverable at now we always overwrite a previous copy losing partial, but potentially better, file recovery. Is it possible a better strategy ? diff --git a/check.c b/check.c index 7655fd4d..c1466e82 100644 --- a/check.c +++ b/check.c @@ -483,8 +483,8 @@ static int state_check_process(struct snapraid_state* state, int check, int fix, handle = handle_map(state, &diskmax); - /* we need disk + 2 for each parity level buffers + 1 zero buffer */ - buffermax = diskmax + state->level * 2 + 1; + /* we need 1 * data + 2 * parity + + 1 * zero */ + buffermax = diskmax + 2 * state->level + 1; buffer = malloc_nofail_vector_align(diskmax, buffermax, state->block_size, &buffer_alloc); if (!state->opt.skip_self) @@ -610,7 +610,7 @@ static int state_check_process(struct snapraid_state* state, int check, int fix, valid_parity = 0; } - /* if the block is deleted */ + /* if the block is DELETED */ if (block_state == BLOCK_STATE_DELETED) { /* use an empty block */ memset(buffer[j], 0, state->block_size); diff --git a/mktest.c b/mktest.c index c4c3f6fe..ecfc3a73 100644 --- a/mktest.c +++ b/mktest.c @@ -211,7 +211,7 @@ void fallback(int f, struct stat* st) } } -void change(const char* path, int size, int restore_time) +void change(const char* path, int size, int silent_error) { struct stat st; @@ -226,13 +226,15 @@ void change(const char* path, int size, int restore_time) } if (S_ISLNK(st.st_mode)) { - /* symlink */ - int ret; - - ret = remove(path); - if (ret != 0) { - fprintf(stderr, "Error removing %s\n", path); - exit(EXIT_FAILURE); + if (!silent_error) { + /* symlink */ + int ret; + + ret = remove(path); + if (ret != 0) { + fprintf(stderr, "Error removing %s\n", path); + exit(EXIT_FAILURE); + } } } else { /* file */ @@ -267,7 +269,7 @@ void change(const char* path, int size, int restore_time) fflush(f); /* restore the previous modification time */ - if (restore_time) + if (silent_error) fallback(fileno(f), &st); fclose(f); @@ -421,7 +423,7 @@ int main(int argc, char* argv[]) } } else if (strcmp(argv[1], "damage") == 0 || strcmp(argv[1], "change") == 0) { int fail, size; - int restore_time = strcmp(argv[1], "damage") == 0; + int silent_error = strcmp(argv[1], "damage") == 0; if (argc < 6) { help(); @@ -438,7 +440,7 @@ int main(int argc, char* argv[]) for(i=b;ilevel; + /* we need 2 * data + 1 * parity + 1 * zero */ + buffermax = 2 * diskmax + state->level + 1; buffer = malloc_nofail_vector_align(diskmax, buffermax, state->block_size, &buffer_alloc); if (!state->opt.skip_self) mtest_vector(buffermax, state->block_size, buffer); + /* fill up the zero buffer */ + memset(buffer[buffermax-1], 0, state->block_size); + raid_zero(buffer[buffermax-1]); + + failed = malloc_nofail(diskmax * sizeof(struct failed_struct)); + failed_map = malloc_nofail(diskmax * sizeof(unsigned)); + error = 0; silent_error = 0; @@ -122,10 +141,12 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari countpos = 0; if (state_progress_begin(state, blockstart, blockmax, countmax)) for(i=blockstart;iinfoarr, i); @@ -222,9 +247,24 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari /* it's important to check this before any other check */ /* because for DELETED block, we skip at the next check */ + /* now continue with processing */ } - /* if the block has no file, it doesn't partecipate in the new parity computation */ + /* if the block is NEW, DELETED or CHG, */ + /* we have to take care of it in case of recover */ + if (block_has_invalid_parity(block)) { + /* store it in the failed set, because */ + /* the parity is still computed with the previous content */ + failed[failed_count].index = j; + failed[failed_count].size = state->block_size; + failed[failed_count].block = block; + ++failed_count; + + /* now continue with processing */ + } + + /* if the block has no file, meanining that it's EMPTY or DELETED, */ + /* it doesn't partecipate in the new parity computation */ if (!block_has_file(block)) { /* use an empty block */ memset(buffer[j], 0, state->block_size); @@ -343,6 +383,12 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari fprintf(stderr, "WARNING! Unexpected data error in a data disk! The block is now marked as bad!\n"); fprintf(stderr, "Try with 'snapraid -e fix' to recover!\n"); + /* save the failed block for the fix */ + failed[failed_count].index = j; + 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 */ @@ -380,9 +426,100 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari } } - /* if we have read all the data required and it's correct, proceed with the parity */ - if (!error_on_this_block && !silent_error_on_this_block) { + /* if we have only silent errors we can try to fix them on-the-fly */ + if (!error_on_this_block && silent_error_on_this_block) { + unsigned failed_mac; + /* setup the blocks to recover */ + failed_mac = 0; + for(j=0;jlevel + failed[j].index]; + unsigned block_state = block_state_get(failed[j].block); + + /* make a copy of new content just read */ + memcpy(block_copy, block_buffer, state->block_size); + + if (block_state == BLOCK_STATE_NEW) { + /* restore the data to parity state */ + memset(block_buffer, 0, state->block_size); + } else { + /* if we have too many failure */ + if (failed_mac >= state->level) { + /* we cannot recover */ + break; + } + + /* otherwise it has to be recovered */ + failed_map[failed_mac++] = failed[j].index; + } + } + + /* if we have enough parity to fix */ + if (j == failed_count) { + /* read the parity */ + /* we are sure that parity exists because a silent error */ + /* implies a BLK block with compute parity */ + for(l=0;llevel;++l) { + ret = parity_read(parity[l], i, buffer[diskmax+l], state->block_size, stdlog); + if (ret == -1) { + fprintf(stdlog, "parity_error:%u:%s: Read error\n", i, lev_config_name(l)); + fprintf(stderr, "DANGER! Read error in the %s disk, it isn't possible to sync.\n", lev_name(l)); + fprintf(stderr, "Ensure that disk '%s' is sane.\n", lev_config_name(l)); + printf("Stopping at block %u\n", i); + ++error; + goto bail; + } + } + + /* try to fix the data */ + raid_rec(failed_mac, failed_map, diskmax, state->level, state->block_size, buffer); + + /* check the result and prepare the data */ + for(j=0;jlevel + failed[j].index]; + unsigned block_state = block_state_get(failed[j].block); + + if (block_state == BLOCK_STATE_BLK) { + unsigned size = failed[j].size; + + /* compute the hash of the recovered block */ + if (rehash) { + memhash(state->prevhash, state->prevhashseed, hash, block_buffer, size); + } else { + memhash(state->hash, state->hashseed, hash, block_buffer, size); + } + + /* if the hash doesn't match */ + if (memcmp(hash, failed[j].block->hash, HASH_SIZE) != 0) { + /* we have not recovered */ + break; + } + + /* pad with 0 if needed */ + if (size < state->block_size) + memset(block_buffer + size, 0, state->block_size - size); + } else { + /* otherwise restore the new content */ + /* because we are not interested in recovered */ + /* state for CHG and DELETED blocks */ + /* and we have to restore also NEW zeroed blocks */ + memcpy(block_buffer, block_copy, state->block_size); + } + } + + /* if all is processed, we have fixed it */ + if (j == failed_count) + fixed_error_on_this_block = 1; + } + } + + /* if we have read all the data required and it's correct, proceed with the parity */ + if (!error_on_this_block + && (!silent_error_on_this_block || fixed_error_on_this_block) + ) { /* updates the parity only if really needed */ if (parity_needs_to_be_updated) { /* compute the parity */ @@ -427,7 +564,11 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari /* we update the info block only if we really have updated the parity */ /* because otherwise the time info would be misleading as we didn't */ /* wrote the parity at this time */ - if (parity_needs_to_be_updated) { + /* we also update the info block only if no silent error was found */ + /* because has no sense to refresh the time for data that we know bad */ + if (parity_needs_to_be_updated + && !silent_error_on_this_block + ) { /* if rehash is neeed */ if (rehash) { /* store all the new hash already computed */ @@ -441,7 +582,12 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari /* we are also clearing any previous bad and rehash flag */ info_set(&state->infoarr, i, info_make(now, 0, 0)); } - } else if (silent_error_on_this_block) { + } + + /* if a silent error was found, even if corrected */ + /* mark the block as bad to have check/fix to handle it */ + /* because our correction is in memory only and not yet written */ + if (silent_error_on_this_block) { /* set the error status keeping the other info */ info_set(&state->infoarr, i, info_set_bad(info)); } @@ -508,6 +654,8 @@ bail: free(buffer_alloc); free(buffer); free(rehandle_alloc); + free(failed); + free(failed_map); if (state->opt.expect_recoverable) { if (error + silent_error == 0)