diff --git a/HISTORY b/HISTORY index de2db307..03e96abc 100644 --- a/HISTORY +++ b/HISTORY @@ -2,7 +2,7 @@ SnapRAID HISTORY ================ 10.0 2016/01 -=========== +============ * Increases the speed of the 'sync' and 'scrub' commands with a new multithread implementation. It uses one thread for each disk, dedicated exclusively to read-ahead data or parity early and to write-behind @@ -20,6 +20,18 @@ SnapRAID HISTORY identify files. The 'status' command recommend to run 'nano' if required. +9.3 2016/01 +=========== + * Fixes an invalid assumption in the copy detection mechanism that + could result in an internal inconsistency, and with the impossibility + to run the 'sync' and 'diff' commands. + This was triggered by a very specific pattern of identical files. + At least three of them, with one already in the parity, and at a + higher disk number than the others that should be instead new ones. + This had no bad effect, if not preventing the 'sync' command to run. + A workaround was to just run 'sync' one time with the -N, --force-nocopy + option to disable the copy detection. + 9.2 2016/01 =========== * Fixes support for symlinks pointing to an empty target. Before they diff --git a/Makefile.am b/Makefile.am index 1d0f748a..17a2f211 100644 --- a/Makefile.am +++ b/Makefile.am @@ -307,6 +307,17 @@ endif $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) sync -l ">&1" rm -r bench/disk1/TEST* $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) -E sync +# Now enforce copy detection of a file without parity +# Create a file in a high number disk + head -c 8192 /dev/urandom > bench/disk6/STEP1 + $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) sync +# Create two new copies. The one in disk1 will copy +# the hash from the one in disk6, and the one in disk2 +# will copy the hash from the one in disk1, that +# doesn't have parity + cp -a bench/disk6/STEP1 bench/disk1/STEP1 + cp -a bench/disk6/STEP1 bench/disk2/STEP1 + $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) sync # Now create, duplicate, move and partial sync head -c 8192 /dev/zero > bench/disk1/INVA1 head -c 8192 /dev/zero > bench/disk1/INVA2 diff --git a/cmdline/elem.c b/cmdline/elem.c index 21187320..660294a9 100644 --- a/cmdline/elem.c +++ b/cmdline/elem.c @@ -1194,7 +1194,7 @@ struct snapraid_file* fs_par2file_get(struct snapraid_disk* disk, block_off_t pa return file; } -block_off_t fs_file2par_get(struct snapraid_disk* disk, struct snapraid_file* file, block_off_t file_pos) +block_off_t fs_file2par_maybe(struct snapraid_disk* disk, struct snapraid_file* file, block_off_t file_pos) { struct snapraid_chunk* chunk; block_off_t ret; @@ -1203,10 +1203,8 @@ block_off_t fs_file2par_get(struct snapraid_disk* disk, struct snapraid_file* fi chunk = fs_file2chunk_get_unlock(disk, &disk->fs_last, file, file_pos); if (!chunk) { - /* LCOV_EXCL_START */ - log_fatal("Internal inconsistency when resolving file '%s' at position '%u/%u' in disk '%s'\n", file->sub, file_pos, file->blockmax, disk->name); - os_abort(); - /* LCOV_EXCL_STOP */ + fs_unlock(disk); + return POS_INVALID; } ret = chunk->parity_pos + (file_pos - chunk->file_pos); diff --git a/cmdline/elem.h b/cmdline/elem.h index e1a11a2c..cbf13585 100644 --- a/cmdline/elem.h +++ b/cmdline/elem.h @@ -938,8 +938,27 @@ struct snapraid_file* fs_par2file_get(struct snapraid_disk* disk, block_off_t pa /** * Get the parity position from the file position. + * Return POS_INVALID if no parity is allocated. */ -block_off_t fs_file2par_get(struct snapraid_disk* disk, struct snapraid_file* file, block_off_t file_pos); +block_off_t fs_file2par_maybe(struct snapraid_disk* disk, struct snapraid_file* file, block_off_t file_pos); + +/** + * Get the parity position from the file position. + */ +static inline block_off_t fs_file2par_get(struct snapraid_disk* disk, struct snapraid_file* file, block_off_t file_pos) +{ + block_off_t ret; + + ret = fs_file2par_maybe(disk, file, file_pos); + if (ret == POS_INVALID) { + /* LCOV_EXCL_START */ + log_fatal("Internal inconsistency when resolving file '%s' at position '%u/%u' in disk '%s'\n", file->sub, file_pos, file->blockmax, disk->name); + os_abort(); + /* LCOV_EXCL_STOP */ + } + + return ret; +} /** * Get the block from the parity position. diff --git a/cmdline/scan.c b/cmdline/scan.c index 6cb403bd..c3eeff1f 100644 --- a/cmdline/scan.c +++ b/cmdline/scan.c @@ -378,19 +378,24 @@ static int file_is_full_invalid_parity_and_stable(struct snapraid_state* state, return 0; /* - * Get the parity position, but only after checking - * that the file has invalid parity. - * This is not really required, as we are sure - * that 'kept" files are mapped into parity. + * Get the parity position. + * + * Note that here we expect to always have mapped + * parity, because kept files always have it. + * + * Anyway, checking for POS_INVALID doesn't hurt. */ - parity_pos = fs_file2par_get(disk, file, i); + parity_pos = fs_file2par_maybe(disk, file, i); - /* get block specific info */ - info = info_get(&state->infoarr, parity_pos); + /* if it's not mapped, it cannot have rehash */ + if (parity_pos != POS_INVALID) { + /* get block specific info */ + info = info_get(&state->infoarr, parity_pos); - /* if rehash fails */ - if (info_get_rehash(info)) - return 0; + /* if rehash fails */ + if (info_get_rehash(info)) + return 0; + } } return 1; @@ -419,25 +424,30 @@ static int file_is_full_hashed_and_stable(struct snapraid_state* state, struct s return 0; /* - * Get the parity position, but only AFTER checking - * that the file has an updated hash. - * This is required because not hashed files may be - * not yet mapped into parity. + * Get the parity position. * - * This happens when a new file is in the delayed insertion - * list, and scan finds a copy of it, and it tries to copy - * the hash info. - * As just allocated files have only CHG blocks, checking that - * the hash is updated is enough to exclude such cases. + * Note that it's possible to have files + * not mapped into the parity, even if they + * have a valid hash. + * + * This happens for example, for 'copied' files + * that have REP blocks, but not yet mapped. + * + * If there are multiple copies, it's also possible + * that such files are used as 'source' to copy + * hashes, and then to get them inside this function. */ - parity_pos = fs_file2par_get(disk, file, i); + parity_pos = fs_file2par_maybe(disk, file, i); - /* get block specific info */ - info = info_get(&state->infoarr, parity_pos); + /* if it's not mapped, it cannot have rehash */ + if (parity_pos != POS_INVALID) { + /* get block specific info */ + info = info_get(&state->infoarr, parity_pos); - /* exclude blocks needing a rehash */ - if (info_get_rehash(info)) - return 0; + /* exclude blocks needing a rehash */ + if (info_get_rehash(info)) + return 0; + } } return 1;