diff --git a/HISTORY b/HISTORY index 02dcfc22..362fa3d8 100644 --- a/HISTORY +++ b/HISTORY @@ -3,6 +3,8 @@ SnapRAID HISTORY 11.1 2017/04 ============ + * Fixed the check command to correctly ignore errors on unused parity. + This was broken in version 9.0. * Allow increasing the number of parity splits of existing parity. * Fixed quoting when printing in Linux. This fixes the UTF-8 screen output. Windows version was not affected. diff --git a/cmdline/check.c b/cmdline/check.c index 63aae376..e266f9de 100644 --- a/cmdline/check.c +++ b/cmdline/check.c @@ -815,14 +815,16 @@ static int block_is_enabled(struct snapraid_state* state, block_off_t i, struct return 0; } - /* for each parity */ + /* now apply the filters */ + + /* if a parity is not excluded, include all blocks, even unused ones */ for (l = 0; l < state->level; ++l) { - if (!state->parity[l].is_excluded) { + if (!state->parity[l].is_excluded_by_filter) { return 1; } } - /* for each disk */ + /* otherwise include only used blocks */ for (j = 0; j < diskmax; ++j) { struct snapraid_block* block; @@ -902,6 +904,7 @@ static int state_check_process(struct snapraid_state* state, int fix, struct sna for (i = blockstart; i < blockmax; ++i) { unsigned failed_count; int valid_parity; + int used_parity; snapraid_info info; int rehash; @@ -927,6 +930,9 @@ static int state_check_process(struct snapraid_state* state, int fix, struct sna /* because we also don't read it at all */ valid_parity = 1; + /* If the parity is used by at least one file */ + used_parity = 0; + /* keep track of the number of failed blocks */ failed_count = 0; @@ -992,6 +998,9 @@ static int state_check_process(struct snapraid_state* state, int fix, struct sna continue; } + /* here we are sure that the parity is used by a file */ + used_parity = 1; + /* get the file of this block */ file = fs_par2file_get(disk, i, &file_pos); @@ -1270,10 +1279,16 @@ static int state_check_process(struct snapraid_state* state, int fix, struct sna ++unrecoverable_error; } - /* now check parities, but only if all the blocks have it computed */ - /* if you check/fix after a partial sync, it's OK to have parity errors on the blocks with invalid parity */ - /* and doesn't make sense to try to fix it */ - if (valid_parity) { + /* + * Check parities, but only if all the blocks have it computed and it's used. + * + * If you check/fix after a partial sync, it's OK to have parity errors + * on the blocks with invalid parity and doesn't make sense to try to fix it. + * + * It's also OK to have data errors on unused parity, because sync doesn't + * update it. + */ + if (used_parity && valid_parity) { /* 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) { @@ -1334,10 +1349,17 @@ static int state_check_process(struct snapraid_state* state, int fix, struct sna ++recovered_error; } - /* update parity only if all the blocks have it computed */ - /* if you check/fix after a partial sync, you do not want to fix parity */ - /* for blocks that are going to have it computed in the sync completion */ - if (valid_parity) { + /* + * Update parity only if all the blocks have it computed and it's used. + * + * If you check/fix after a partial sync, you do not want to fix parity + * for blocks that are going to have it computed in the sync completion. + * + * For unused parity there is no need to write it, because when fixing + * we already have allocated space for it on parity file creation, + * and its content doesn't matter. + */ + if (used_parity && valid_parity) { /* update the parity */ for (l = 0; l < state->level; ++l) { /* if the parity on disk is wrong */ @@ -1345,7 +1367,7 @@ static int state_check_process(struct snapraid_state* state, int fix, struct sna /* and we have access at the parity */ && parity[l] != 0 /* and the parity is not excluded */ - && !state->parity[l].is_excluded + && !state->parity[l].is_excluded_by_filter ) { ret = parity_write(parity[l], i, buffer[diskmax + l], state->block_size); if (ret == -1) { diff --git a/cmdline/elem.h b/cmdline/elem.h index f3e339bc..00a8b8f3 100644 --- a/cmdline/elem.h +++ b/cmdline/elem.h @@ -463,7 +463,7 @@ struct snapraid_parity { char smartctl[PATH_MAX]; /**< Custom command for smartctl. Empty means auto. */ block_off_t total_blocks; /**< Number of total blocks. */ block_off_t free_blocks; /**< Number of free blocks at the last sync. */ - int is_excluded; /**< If the parity is excluded by disk filter. */ + int is_excluded_by_filter; /**< If the parity is excluded by filters. */ int skip_access; /**< If at least one of the parity disk is inaccessible and it should be skipped. */ uint64_t tick; /**< Usage time. */ uint64_t progress_tick[PROGRESS_MAX]; /**< Last cpu ticks of progress. */ diff --git a/cmdline/state.c b/cmdline/state.c index 62c6a291..d9edb786 100644 --- a/cmdline/state.c +++ b/cmdline/state.c @@ -162,7 +162,7 @@ void state_init(struct snapraid_state* state) state->parity[l].skip_access = 0; state->parity[l].tick = 0; state->parity[l].cached = 0; - state->parity[l].is_excluded = 0; + state->parity[l].is_excluded_by_filter = 0; } state->tick_io = 0; state->tick_misc = 0; @@ -4061,14 +4061,14 @@ void state_filter(struct snapraid_state* state, tommy_list* filterlist_file, tom } } - /* if we are filtering by disk */ + /* if we are filtering by disk, exclude any parity not explicitely included */ if (!tommy_list_empty(filterlist_disk)) { /* for each parity disk */ for (l = 0; l < state->level; ++l) { /* check if the parity is excluded by name */ if (filter_path(filterlist_disk, 0, lev_config_name(l), 0) != 0) { /* excluded the parity from further operation */ - state->parity[l].is_excluded = 1; + state->parity[l].is_excluded_by_filter = 1; } } } else { @@ -4076,7 +4076,7 @@ void state_filter(struct snapraid_state* state, tommy_list* filterlist_file, tom if (filter_missing || !tommy_list_empty(filterlist_file)) { /* for each parity disk */ for (l = 0; l < state->level; ++l) { - state->parity[l].is_excluded = 1; + state->parity[l].is_excluded_by_filter = 1; } } }