From 49bcad544810b4d0c349b82047098a6ec855cd96 Mon Sep 17 00:00:00 2001 From: Andrea Mazzoleni Date: Tue, 29 Sep 2015 22:31:13 +0200 Subject: [PATCH] Extend the -h option to check also copied/moved files --- HISTORY | 4 ++- Makefile.am | 3 ++- cmdline/sync.c | 67 ++++++++++++++++++++++++++++++++++++-------------- snapraid.1 | 15 ++++++----- snapraid.d | 15 ++++++----- snapraid.txt | 15 ++++++----- 6 files changed, 80 insertions(+), 39 deletions(-) diff --git a/HISTORY b/HISTORY index e821f011..faf61de4 100644 --- a/HISTORY +++ b/HISTORY @@ -13,9 +13,11 @@ SnapRAID HISTORY This allows to verify as early as possible that the written parity during sync is really correct. The "status" command reports the amount of scrub necessary to - ensure to scrub all the synched blocks. + ensure to scrub all the synced blocks. * The -p, --percentage option supports the new the arguments: "bad", "sync" and "full" to define the kind of scrub to do. + * Extends the -h, --pre-hash option to also verify moved and + copied files into the array before running a 'sync'. * Update 'best' RAID functions for recent Atom CPUs. * Validate filters specifications rejecting relative paths. diff --git a/Makefile.am b/Makefile.am index 32449adf..50880983 100644 --- a/Makefile.am +++ b/Makefile.am @@ -257,7 +257,8 @@ endif # Corrupt and move it to another disk $(TESTENV) ./mktest$(EXEEXT) damage 1 1 1 bench/disk1/COPY mv bench/disk1/COPY bench/disk2/COPY -# Now sync with failure as the data won't match +# Now sync with failure as the data won't match. We have two points of failure, pre-hash and sync + $(FAILENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) --test-expect-failure -h sync $(FAILENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) --test-expect-failure sync # Now sync with force-nocopy $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) --force-nocopy sync diff --git a/cmdline/sync.c b/cmdline/sync.c index df3b7804..8da6946b 100644 --- a/cmdline/sync.c +++ b/cmdline/sync.c @@ -63,15 +63,15 @@ static int state_hash_process(struct snapraid_state* state, block_off_t blocksta for (i = blockstart; i < blockmax; ++i) { struct snapraid_block* block; + unsigned block_state; block = disk_block_get(disk, i); - /* if no file in this block, skip it */ - if (!block_has_file(block)) - continue; + /* get the state of the block */ + block_state = block_state_get(block); - /* if the block already has a hash, skip it */ - if (block_has_updated_hash(block)) + /* process REP and CHG blocks */ + if (block_state != BLOCK_STATE_REP && block_state != BLOCK_STATE_CHG) continue; ++countmax; @@ -99,20 +99,17 @@ static int state_hash_process(struct snapraid_state* state, block_off_t blocksta struct snapraid_block* block; int read_size; unsigned char hash[HASH_SIZE]; + unsigned block_state; block = disk_block_get(disk, i); - /* if no file in this block, skip it */ - if (!block_has_file(block)) - continue; + /* get the state of the block */ + block_state = block_state_get(block); - /* if the block already has a hash, skip it */ - if (block_has_updated_hash(block)) + /* process REP and CHG blocks */ + if (block_state != BLOCK_STATE_REP && block_state != BLOCK_STATE_CHG) continue; - /* here only CHG blocks are allowed */ - assert(block_state_get(block) == BLOCK_STATE_CHG); - /* get block specific info */ info = info_get(&state->infoarr, i); @@ -256,14 +253,45 @@ static int state_hash_process(struct snapraid_state* state, block_off_t blocksta memhash(state->hash, state->hashseed, hash, buffer, read_size); } - /* copy the hash in the block */ - memcpy(block->hash, hash, HASH_SIZE); + if (block_state == BLOCK_STATE_REP) { + /* compare the hash */ + if (memcmp(hash, block->hash, HASH_SIZE) != 0) { + /* file we are processing for error reporting */ + struct snapraid_file* file = block_file_get(block); - /* and mark the block as hashed */ - block_state_set(block, BLOCK_STATE_REP); + log_tag("error:%u:%s:%s: Unexpected data change\n", i, disk->name, esc(file->sub)); + log_error("Data change at file '%s' at position '%u'\n", handle[j].path, block_file_pos(block)); + log_error("WARNING! Unexpected data modification of a file without parity!\n"); - /* mark the state as needing write */ - state->need_write = 1; + if (file_flag_has(file, FILE_IS_COPY)) { + log_error("This file was detected as a copy of another file with the same name, size,\n"); + log_error("and timestamp, but the file data isn't matching the assumed copy.\n"); + log_error("If this is a false positive, and the files are expected to be different,\n"); + log_error("you can 'sync' anyway using 'snapraid --force-nocopy sync'\n"); + } else { + log_error("Try removing the file from the array and rerun the 'sync' command!\n"); + } + + /* block sync to allow a recovery before overwriting */ + /* the parity needed to make such recovery */ + *skip_sync = 1; /* avoid to run the next sync */ + + ++error; + continue; + } + } else { + /* the only other case is BLOCK_STATE_CHG */ + assert(block_state == BLOCK_STATE_CHG); + + /* copy the hash in the block */ + memcpy(block->hash, hash, HASH_SIZE); + + /* and mark the block as hashed */ + block_state_set(block, BLOCK_STATE_REP); + + /* mark the state as needing write */ + state->need_write = 1; + } /* count the number of processed block */ ++countpos; @@ -1307,6 +1335,7 @@ int state_sync(struct snapraid_state* state, block_off_t blockstart, block_off_t /* LCOV_EXCL_STOP */ } + /* save with the new hash information */ if (state->need_write) state_write(state); } diff --git a/snapraid.1 b/snapraid.1 index 7c5ce32d..fb44e22b 100644 --- a/snapraid.1 +++ b/snapraid.1 @@ -1000,18 +1000,21 @@ option can speedup a lot the checking process. This option can be used only with \[dq]check\[dq]. .TP .B \-h, \-\-pre\-hash -In \[dq]sync\[dq] runs a preliminary hashing phase of all -the new data to verify the data used in the parity computation. +In \[dq]sync\[dq] runs a preliminary hashing phase of all the new data +to have an additional verification before the parity computation. Usually in \[dq]sync\[dq] no preliminary hashing is done, and the new data is hashed just before the parity computation when it\'s read -for the first time, +for the first time. Unfortunately, this process happens when the system is under -heavy load, with all disks spinning and a busy CPU. -This is an extreme condition for your machine, and if it has a +heavy load, with all disks spinning and with a busy CPU. +This is an extreme condition for the machine, and if it has a latent hardware problem, it\'s possible to encounter silent errors what cannot be detected because the data is not yet hashed. To avoid this risk, you can enable the \[dq]pre\-hash\[dq] mode and have -all the data hashed two times to ensure its integrity. +all the data read two times to ensure its integrity. +This option also verifies the files moved inside the array, +to ensure that the move operation went succesfully, and in case +to block the sync and to allow to run a fix operation. This option can be used only with \[dq]sync\[dq]. .TP .B \-i, \-\-import DIR diff --git a/snapraid.d b/snapraid.d index 59e5dd8c..c656d086 100644 --- a/snapraid.d +++ b/snapraid.d @@ -714,18 +714,21 @@ Options This option can be used only with "check". -h, --pre-hash - In "sync" runs a preliminary hashing phase of all - the new data to verify the data used in the parity computation. + In "sync" runs a preliminary hashing phase of all the new data + to have an additional verification before the parity computation. Usually in "sync" no preliminary hashing is done, and the new data is hashed just before the parity computation when it's read - for the first time, + for the first time. Unfortunately, this process happens when the system is under - heavy load, with all disks spinning and a busy CPU. - This is an extreme condition for your machine, and if it has a + heavy load, with all disks spinning and with a busy CPU. + This is an extreme condition for the machine, and if it has a latent hardware problem, it's possible to encounter silent errors what cannot be detected because the data is not yet hashed. To avoid this risk, you can enable the "pre-hash" mode and have - all the data hashed two times to ensure its integrity. + all the data read two times to ensure its integrity. + This option also verifies the files moved inside the array, + to ensure that the move operation went succesfully, and in case + to block the sync and to allow to run a fix operation. This option can be used only with "sync". -i, --import DIR diff --git a/snapraid.txt b/snapraid.txt index fb05c735..be6c8544 100644 --- a/snapraid.txt +++ b/snapraid.txt @@ -773,18 +773,21 @@ SnapRAID provides the following options: This option can be used only with "check". -h, --pre-hash - In "sync" runs a preliminary hashing phase of all - the new data to verify the data used in the parity computation. + In "sync" runs a preliminary hashing phase of all the new data + to have an additional verification before the parity computation. Usually in "sync" no preliminary hashing is done, and the new data is hashed just before the parity computation when it's read - for the first time, + for the first time. Unfortunately, this process happens when the system is under - heavy load, with all disks spinning and a busy CPU. - This is an extreme condition for your machine, and if it has a + heavy load, with all disks spinning and with a busy CPU. + This is an extreme condition for the machine, and if it has a latent hardware problem, it's possible to encounter silent errors what cannot be detected because the data is not yet hashed. To avoid this risk, you can enable the "pre-hash" mode and have - all the data hashed two times to ensure its integrity. + all the data read two times to ensure its integrity. + This option also verifies the files moved inside the array, + to ensure that the move operation went succesfully, and in case + to block the sync and to allow to run a fix operation. This option can be used only with "sync". -i, --import DIR