Merge branch 'b9'

Conflicts:
	HISTORY
	cmdline/elem.c
	cmdline/elem.h

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

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

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);

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.

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;