Fix an invalid assumption about allocated parity

In the copy detection it's possible to need to
copy from a file wihout parity. This could result
in the parity position not found.
This commit is contained in:
Andrea Mazzoleni 2016-01-15 19:50:21 +01:00
parent 2f5c6a39a9
commit 56bef0d90f
4 changed files with 47 additions and 29 deletions

View file

@ -303,6 +303,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

View file

@ -1160,12 +1160,8 @@ block_off_t fs_file2par_get_ts(struct snapraid_disk* disk, struct snapraid_chunk
struct snapraid_chunk* chunk;
chunk = fs_file2chunk_get_ts(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 */
}
if (!chunk)
return POS_INVALID;
return chunk->parity_pos + (file_pos - chunk->file_pos);
}

View file

@ -941,6 +941,7 @@ block_off_t fs_file2par_get_ts(struct snapraid_disk* disk, struct snapraid_chunk
/**
* Get the parity position from the file position.
* Return POS_INVALID if no parity is allocated.
* \note This function is NOT thread-safe as it uses the the disk cache.
*/
static inline block_off_t fs_file2par_get(struct snapraid_disk* disk, struct snapraid_file* file, block_off_t file_pos)

View file

@ -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);
/* 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);
/* 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;