From 83d967e2c0792a5fd9a6543ff1d7d53815d291d8 Mon Sep 17 00:00:00 2001 From: Andrea Mazzoleni Date: Mon, 2 Mar 2015 23:01:15 +0100 Subject: [PATCH] Change "diff" to return an error if a "sync" is required --- HISTORY | 6 ++++-- Makefile.am | 10 +++++----- TODO | 5 ----- cmdline/parity.c | 33 +++++++++++++++++++++++++++++++++ cmdline/parity.h | 14 ++++++++++++++ cmdline/scan.c | 25 ++++++++++++++++++++++++- cmdline/snapraid.c | 11 +++++++++-- cmdline/state.h | 7 ++++++- snapraid.1 | 3 +++ snapraid.d | 3 +++ snapraid.txt | 3 +++ 11 files changed, 104 insertions(+), 16 deletions(-) diff --git a/HISTORY b/HISTORY index 4ca75d54..1e20c24d 100644 --- a/HISTORY +++ b/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. diff --git a/Makefile.am b/Makefile.am index 27d537da..bfef721c 100644 --- a/Makefile.am +++ b/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 diff --git a/TODO b/TODO index 17138f1e..bafd1d0c 100644 --- a/TODO +++ b/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. diff --git a/cmdline/parity.c b/cmdline/parity.c index 40457128..51138819 100644 --- a/cmdline/parity.c +++ b/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; diff --git a/cmdline/parity.h b/cmdline/parity.h index 39f75b31..f0b32a36 100644 --- a/cmdline/parity.h +++ b/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. */ diff --git a/cmdline/scan.c b/cmdline/scan.c index 16bc98b6..19ff18e2 100644 --- a/cmdline/scan.c +++ b/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 */ } diff --git a/cmdline/snapraid.c b/cmdline/snapraid.c index 053b7fd4..f58223bb 100644 --- a/cmdline/snapraid.c +++ b/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); diff --git a/cmdline/state.h b/cmdline/state.h index 7969084b..ce30ada6 100644 --- a/cmdline/state.h +++ b/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. diff --git a/snapraid.1 b/snapraid.1 index accfaedb..8d1a90f4 100644 --- a/snapraid.1 +++ b/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 diff --git a/snapraid.d b/snapraid.d index 66752cef..6f9ed018 100644 --- a/snapraid.d +++ b/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 diff --git a/snapraid.txt b/snapraid.txt index 3b6afafb..d56ca38e 100644 --- a/snapraid.txt +++ b/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