Change "diff" to return an error if a "sync" is required

HISTORY

@@ -13,8 +13,10 @@ SnapRAID HISTORY
  * The -e, --filter-error option doesn't write anymore fixes to
    unsynced files. This helps in case you are running it on a not
    synced array, removing the risk to revert some files to an old state.
- * The -e, --filter-error option is now optimal and read only the minimal
-   amount of data necessary to fix the errors.
+ * The -e, --filter-error option is now optimal and read only the
+   minimal amount of data necessary to fix the errors.
+ * The "diff" command returns with an error code if a "sync" is
+   required.
  * Adds new "smart" command to print a SMART report of the array.
  * Adds new "up" and "down" commands to spin up and down the disks of
    the array.

Makefile.am

@@ -229,13 +229,13 @@ endif
 	echo DUP > bench/disk1/DUP2
 	echo --- Some commands with a not empty array
 # Run a sync using physical offset
-	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS_PHYSICAL) -c $(CONF) diff > output.log
+	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS_PHYSICAL) -c $(CONF) --test-expect-failure diff > output.log
 	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS_PHYSICAL) -c $(CONF) sync
 # Now reset the array, as we normally test with alpha order and murmur3
 	rm bench/content bench/?-content
 # Now rebuild the array with alpha order and murmur3 and do some commands
 # Later we will convert it to spooky2 to test both hashes
-	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS_VERBOSE) -c $(CONF) diff > output.log
+	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS_VERBOSE) -c $(CONF) --test-expect-failure  diff > output.log
 	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) --test-force-murmur3 --test-force-autosave-at 100 sync
 	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS_VERBOSE) -c $(CONF) dup -l test.log
 	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS_VERBOSE) -c $(CONF) list -l test.log > output.log
@@ -272,14 +272,14 @@ endif
 	rm bench/disk4/a/9*
 	rm bench/disk5/a/9*
 	rm bench/disk6/a/9*
-	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS_VERBOSE) -c $(CONF) diff > output.log
+	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS_VERBOSE) -c $(CONF) --test-expect-failure diff > output.log
 	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS_VERBOSE) -c $(CONF) sync -l test.log
 	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) check
 	echo --- Move some files, sync and check
 	mv bench/disk1/a/9* bench/disk4/a
 	mv bench/disk2/a/9* bench/disk5/a
 	mv bench/disk3/a/9* bench/disk6/a
-	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS_VERBOSE) -c $(CONF) diff > output.log
+	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS_VERBOSE) -c $(CONF) --test-expect-failure diff > output.log
 	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) sync
 	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) check
 	echo --- Create some more files, hardlinks and empty directories, delete others, sync PAR1 and check
@@ -301,7 +301,7 @@ endif
 	echo 333 > bench/disk3/file_hardlink1
 	ln bench/disk3/file_hardlink1 bench/disk3/file_hardlink2
 	ln bench/disk3/file_hardlink1 bench/disk3/file_hardlink3
-	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS_VERBOSE) -c $(PAR1) diff > output.log
+	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS_VERBOSE) -c $(PAR1) --test-expect-failure diff > output.log
 	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) --test-skip-fallocate -c $(PAR1) sync
 	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) status
 	$(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) --test-skip-sequential -c $(PAR1) check

TODO

@@ -5,11 +5,6 @@ This is the list of TODO items for SnapRAID.
 
 - Minor
 
-* Should "diff" warn about not completed sync ?
-At now neither "diff" nor "status" return with an error code
-if this happen... Something better cold be done.
-See: https://sourceforge.net/p/snapraid/discussion/1677233/thread/c96cdd31/
-
 * Restore ownership and permissions, at least in Unix.
 
 * Restore directory timestamps.

cmdline/parity.c

@@ -78,6 +78,39 @@ block_off_t parity_used_size(struct snapraid_state* state)
 	return parity_block;
 }
 
+int parity_is_invalid(struct snapraid_state* state)
+{
+	block_off_t blockmax;
+	block_off_t i;
+
+	blockmax = parity_allocated_size(state);
+
+	for (i = 0; i < blockmax; ++i) {
+		tommy_node* node_disk;
+		int one_invalid;
+		int one_valid;
+
+		/* for each disk */
+		one_invalid = 0;
+		one_valid = 0;
+		for (node_disk = state->disklist; node_disk != 0; node_disk = node_disk->next) {
+			struct snapraid_disk* disk = node_disk->data;
+			struct snapraid_block* block = disk_block_get(disk, i);
+
+			if (block_has_file(block))
+				one_valid = 1;
+			if (block_has_invalid_parity(block))
+				one_invalid = 1;
+		}
+
+		/* if both valid and invalid, we need to update */
+		if (one_invalid && one_valid)
+			return 1;
+	}
+
+	return 0;
+}
+
 void parity_overflow(struct snapraid_state* state, data_off_t size)
 {
 	tommy_node* i;

cmdline/parity.h

@@ -51,6 +51,20 @@ block_off_t parity_allocated_size(struct snapraid_state* state);
  */
 block_off_t parity_used_size(struct snapraid_state* state);
 
+/**
+ * Checks if the parity needs to be updated with a "sync".
+ *
+ * This is the same logic used in "status" to detect an incomplete "sync",
+ * that ignores invalid block, if they are not used by a file in any disk.
+ * This means that DELETED blocks won't necessarely imply an invalid parity.
+ */
+int parity_is_invalid(struct snapraid_state* state);
+
+/**
+ * If parity has some
+ */
+int parity_is_invalid(struct snapraid_state* state);
+
 /**
  * Reports all the files outside the specified parity size.
  */

cmdline/scan.c

@@ -20,6 +20,7 @@
 #include "support.h"
 #include "elem.h"
 #include "state.h"
+#include "parity.h"
 
 struct snapraid_scan {
 	struct snapraid_state* state; /**< State used. */
@@ -1340,7 +1341,7 @@ static int scan_dir(struct snapraid_scan* scan, int is_diff, const char* dir, co
 	return processed;
 }
 
-void state_scan(struct snapraid_state* state, int is_diff)
+static int state_diffscan(struct snapraid_state* state, int is_diff)
 {
 	tommy_node* i;
 	tommy_node* j;
@@ -1743,5 +1744,27 @@ void state_scan(struct snapraid_state* state, int is_diff)
 	msg_flush();
 
 	tommy_list_foreach(&scanlist, (tommy_foreach_func*)free);
+
+	if (is_diff) {
+		/* check for file difference */
+		if (!no_difference)
+			return -1;
+
+		/* check also for incomplete "sync" */
+		if (parity_is_invalid(state))
+			return -1;
+	}
+
+	return 0;
+}
+
+int state_diff(struct snapraid_state* state)
+{
+	return state_diffscan(state, 1);
+}
+
+void state_scan(struct snapraid_state* state)
+{
+	(void)state_diffscan(state, 0); /* ignore return value */
 }
 

cmdline/snapraid.c

@@ -957,7 +957,14 @@ int main(int argc, char* argv[])
 	if (operation == OPERATION_DIFF) {
 		state_read(&state);
 
-		state_scan(&state, 1);
+		ret = state_diff(&state);
+
+		/* abort if required */
+		if (ret != 0) {
+			/* LCOV_EXCL_START */
+			exit(EXIT_FAILURE);
+			/* LCOV_EXCL_STOP */
+		}
 	} else if (operation == OPERATION_SYNC) {
 
 		/* in the next state read ensures to clear all the past hashes in case */
@@ -970,7 +977,7 @@ int main(int argc, char* argv[])
 
 		state_read(&state);
 
-		state_scan(&state, 0);
+		state_scan(&state);
 
 		/* refresh the size info before the content write */
 		state_refresh(&state);

cmdline/state.h

@@ -191,10 +191,15 @@ void state_read(struct snapraid_state* state);
  */
 void state_write(struct snapraid_state* state);
 
+/**
+ * Diff all the disks.
+ */
+int state_diff(struct snapraid_state* state);
+
 /**
  * Scans all the disks to update the state.
  */
-void state_scan(struct snapraid_state* state, int is_diff);
+void state_scan(struct snapraid_state* state);
 
 /**
  * Sets the nanosecond timestamp of all files that have a zero value.

snapraid.1

@@ -686,6 +686,9 @@ their parity data recomputed.
 This command doesn\'t check the file data, but only the file timestamp
 size and inode.
 .PP
+If a \[dq]sync\[dq] is not required, the return error code is 0. Otherwise,
+it\'s 1.
+.PP
 Nothing is modified.
 .SS sync 
 Updates the parity information. All the modified files

snapraid.d

@@ -416,6 +416,9 @@ Commands
 	This command doesn't check the file data, but only the file timestamp
 	size and inode.
 
+	If a "sync" is not required, the return error code is 0. Otherwise,
+	it's 1.
+
 	Nothing is modified.
 
   sync

snapraid.txt

@@ -450,6 +450,9 @@ their parity data recomputed.
 This command doesn't check the file data, but only the file timestamp
 size and inode.
 
+If a "sync" is not required, the return error code is 0. Otherwise,
+it's 1.
+
 Nothing is modified.
 
 5.6 sync