diff --git a/HISTORY b/HISTORY index 273528e6..1ee38c84 100644 --- a/HISTORY +++ b/HISTORY @@ -12,6 +12,8 @@ SnapRAID HISTORY possible, but they would affect badly the performance. * Removed the detailed list of errors from the screen output. To get it you must explicitely uses the -l, --log option. + * For files with changes only in some blocks, updates the parity + only for blocks that really are changed. * Reduced memory usage avoiding to allocate unnecessary memory. 4.1 2013/9 diff --git a/Makefile.am b/Makefile.am index 80c224c8..1a762930 100644 --- a/Makefile.am +++ b/Makefile.am @@ -350,7 +350,7 @@ endif rm -r test/disk2 mkdir test/disk2 $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(RAID5) fix -l test.log - echo --- Fixes again with RAIDTP to restore all the parity + echo --- Fixes again to restore all the parity $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) fix -l test.log rm -r test/disk1/_c $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) sync @@ -362,7 +362,7 @@ endif rm -r test/disk2 mkdir test/disk2 $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(RAID5) fix -l test.log - echo --- Fixes again with RAIDTP to restore all the parity + echo --- Fixes again to restore all the parity $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) fix -l test.log -rm -r test/disk1/_a -rm -r test/disk1/_b @@ -370,6 +370,7 @@ endif mv test/_a test/disk1/_a mv test/_b test/disk1/_b $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) sync + $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) check echo --- Abort sync early with more deletions than additions and then delete some unchanged stuff and fix with RAID6 cp -pR test/disk1/_a test/disk1/_c mv test/disk1/_a test/_a @@ -385,6 +386,7 @@ endif mv test/_a test/disk1/_a mv test/_b test/disk1/_b $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) sync + $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) -c $(CONF) check echo --- Delete some files, fix with -m and check with RAID5 rm test/disk1/8* $(TESTENV) ./snapraid$(EXEEXT) $(CHECKFLAGS) --test-expect-recoverable -c $(RAID5) check -l test.log diff --git a/elem.h b/elem.h index b5671c8b..8079ebb6 100644 --- a/elem.h +++ b/elem.h @@ -95,10 +95,14 @@ struct snapraid_filter { * but only if it's different by all 0. * The parity for this disk is not updated, but it contains the old data referenced by the hash. * - * If the hash is completely filled with 0, it means that the hash is lost. - * This could happen if a NEW/CHG block is first DELETED, - * and then reallocated in a CHG one after an interrupted sync. - * See scan_file_remove() and scan_file_insert() for the exact location where this could happen. + * If the state is read from an incomplete sync, we don't really know if the hash is referring at the + * data used to compute the parity, because the sync process was interrupted at an unknown point, + * and the parity may or may not be updated. + * + * For this reson we clear all such hashes when reading the state from an incomplete sync when + * starting a new sync, because sync is affected by sych hashes. + * Check and fix are instead able to work with unsynched hashes. + * Scrub is not affected because it ignores CHG/DELETED blocks. */ #define BLOCK_STATE_CHG 3 @@ -110,9 +114,14 @@ struct snapraid_filter { * but only if it's different by all 0. * The parity for this disk is not updated, but it contains the old data referenced by the hash. * - * If the hash is completely filled with 0, it means that the hash is lost. - * This could happen if a NEW/CHG block is DELETED after an interrupted sync. - * See scan_file_remove() for the exact location where this could happen. + * If the state is read from an incomplete sync, we don't really know if the hash is referring at the + * data used to compute the parity, because the sync process was interrupted at an unknown point, + * and the parity may or may not be updated. + * + * For this reson we clear all such hashes when reading the state from an incomplete sync when + * starting a new sync, because sync is affected by sych hashes. + * Check and fix are instead able to work with unsynched hashes. + * Scrub is not affected because it ignores CHG/DELETED blocks. */ #define BLOCK_STATE_DELETED 4 @@ -504,6 +513,20 @@ static inline int block_has_file(const struct snapraid_block* block) return state == BLOCK_STATE_BLK || state == BLOCK_STATE_NEW || state == BLOCK_STATE_CHG; } +/** + * Checks if the specified block has the same presence state, + * meaning that it's now present when before it was also present, + * or it's now absent and before it was also absent. + * + * Note that NEW / DELETED return 0. + */ +static inline int block_has_same_presence(const struct snapraid_block* block) +{ + unsigned state = block_state_get(block); + + return state == BLOCK_STATE_EMPTY || state == BLOCK_STATE_BLK || state == BLOCK_STATE_CHG; +} + /** * Checks if the block has an invalid parity than needs to be updated. * diff --git a/scan.c b/scan.c index a69d3ad4..5b1ebb54 100644 --- a/scan.c +++ b/scan.c @@ -179,15 +179,18 @@ static void scan_file_remove(struct snapraid_state* state, struct snapraid_disk* break; case BLOCK_STATE_CHG : case BLOCK_STATE_NEW : - /* in these cases we don't know if the old state is still the one */ - /* stored inside the parity, because after an aborted sync, the parity */ - /* may be or may be not have been updated with the new data */ - /* Then we reset the hash to a bogus value */ - /* Note that this condition is possible only if: */ - /* - new files added/modified */ - /* - aborted sync, without saving the content file */ - /* - files deleted after the aborted sync */ - memset(block->hash, 0, HASH_SIZE); + /* if we have not already cleared the undeterminated hash */ + if (!state->clear_undeterminate_hash) { + /* in these cases we don't know if the old state is still the one */ + /* stored inside the parity, because after an aborted sync, the parity */ + /* may be or may be not have been updated with the new data */ + /* Then we reset the hash to a bogus value */ + /* Note that this condition is possible only if: */ + /* - new files added/modified */ + /* - aborted sync, without saving the content file */ + /* - files deleted after the aborted sync */ + memset(block->hash, 0, HASH_SIZE); + } break; default: fprintf(stderr, "Internal state inconsistency in scanning for block %u state %u\n", block->parity_pos, block_state); @@ -254,7 +257,21 @@ static void scan_file_insert(struct snapraid_state* state, struct snapraid_disk* /* we just overwrite it with a NEW one */ block_state_set(&file->blockvec[i], BLOCK_STATE_NEW); } else { - /* otherwise it's a DELETED one, that we convert in CHG keeping the hash */ + /* otherwise it's a DELETED one */ + + /* if we have not already cleared the undeterminated hash */ + if (!state->clear_undeterminate_hash) { + /* in this case we don't know if the old state is still the one */ + /* stored inside the parity, because after an aborted sync, the parity */ + /* may be or may be not have been updated with the new data */ + /* Then we reset the hash to a bogus value */ + /* Note that this condition is possible only if: */ + /* - files are deleted */ + /* - aborted sync, without saving the content file */ + /* - files are readded after the aborted sync */ + memset(block->hash, 0, HASH_SIZE); + } + block_state_set(&file->blockvec[i], BLOCK_STATE_CHG); memcpy(file->blockvec[i].hash, block->hash, HASH_SIZE); } @@ -296,7 +313,7 @@ static void scan_file(struct snapraid_scan* scan, struct snapraid_state* state, * - Linux VFAT kernel (3.2) driver. Inodes are fully reassigned at every mount. * * In such cases, to avoid possible random collisions, it's better to disable the moved - * file recognizition. + * file recognition. * * We do this implicitely removing all the inode before searching for files. * This ensure that no file is found with an old inode, but at the same time, diff --git a/snapraid.c b/snapraid.c index e7a51564..a27d91fb 100644 --- a/snapraid.c +++ b/snapraid.c @@ -581,6 +581,15 @@ int main(int argc, char* argv[]) memory(state.opt.gui); } else if (operation == OPERATION_SYNC) { + + /* in the next state read ensures to clear all the undeterminated hashes in case */ + /* we are reading from an incomplete sync */ + /* The undeterminated hash are only for CHG/DELETED blocks for which we don't */ + /* know if the previous interrupted sync was able to update or not the parity */ + /* The sync process instead needs to trust this information because it's used */ + /* to avoid to recompute the parity if all the input are equals as before. */ + state.clear_undeterminate_hash = 1; + state_read(&state); state_scan(&state, 0); diff --git a/state.c b/state.c index 761aedfc..d6d8f0a6 100644 --- a/state.c +++ b/state.c @@ -66,6 +66,7 @@ void state_init(struct snapraid_state* state) state->lockfile[0] = 0; state->level = 1; /* default is the lowest protection */ state->loaded_blockmax = 0; + state->clear_undeterminate_hash = 0; state->no_conf = 0; tommy_list_init(&state->disklist); @@ -994,6 +995,11 @@ static void state_read_text(struct snapraid_state* state, const char* path, STRE fprintf(stderr, "Invalid '%s' specification in '%s' at line %u\n", tag, path, line); exit(EXIT_FAILURE); } + + /* clear undeterminated hashes before a sync */ + if (tag[0] != 'b' && state->clear_undeterminate_hash) { + memset(block->hash, 0, HASH_SIZE); + } } /* we must not overwrite existing blocks */ @@ -1129,6 +1135,11 @@ static void state_read_text(struct snapraid_state* state, const char* path, STRE exit(EXIT_FAILURE); } + /* clear undeterminated hashes before a sync */ + if (state->clear_undeterminate_hash) { + memset(deleted->block.hash, 0, HASH_SIZE); + } + /* we must not overwrite existing blocks */ if (disk_block_get(disk, v_pos) != BLOCK_EMPTY) { fprintf(stderr, "Internal inconsistency for 'off' specification in '%s' at line %u\n", path, line); @@ -2224,6 +2235,10 @@ static void state_read_binary(struct snapraid_state* state, const char* path, ST decoding_error(path, f); exit(EXIT_FAILURE); } + + /* clear undeterminated hashes before a sync */ + if (c != 'b' && state->clear_undeterminate_hash) + memset(block->hash, 0, HASH_SIZE); } /* we must not overwrite existing blocks */ @@ -2391,6 +2406,11 @@ static void state_read_binary(struct snapraid_state* state, const char* path, ST exit(EXIT_FAILURE); } + /* clear undeterminated hashes before a sync */ + if (state->clear_undeterminate_hash) { + memset(deleted->block.hash, 0, HASH_SIZE); + } + /* we must not overwrite existing blocks */ if (disk_block_get(disk, v_pos) != BLOCK_EMPTY) { decoding_error(path, f); diff --git a/state.h b/state.h index 46049b83..03e38ab0 100644 --- a/state.h +++ b/state.h @@ -105,6 +105,7 @@ struct snapraid_state { tommy_hashdyn previmportset; /**< Hashtable by prevhash of all the import blocks. Valid only if we are in a rehash state. */ tommy_arrayblkof infoarr; /**< Block information array. */ block_off_t loaded_blockmax; /**< Previous size of the parity file, computed from the loaded state. */ + int clear_undeterminate_hash; /**< Clear all the hash from CHG and DELETED blocks when reading the state from an incomplete sync. */ time_t progress_start; /**< Start of processing for progress visualization. */ time_t progress_last; /**< Last update of progress visualization. */ time_t progress_interruption; /**< Start of the measure interruption. */ diff --git a/sync.c b/sync.c index 026d84dc..4c5ae81a 100644 --- a/sync.c +++ b/sync.c @@ -58,6 +58,10 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari time_t now; unsigned l; + /* the sync process assumes that all the hashes are correct */ + /* including the ones from CHG and DELETED blocks */ + assert(state->clear_undeterminate_hash != 0); + /* get the present time */ now = time(0); @@ -119,6 +123,7 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari int one_invalid; int one_valid; int skip_this_block; + int parity_needs_to_be_updated; int ret; snapraid_info info; int rehash; @@ -172,6 +177,16 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari info = info_get(&state->infoarr, i); rehash = info_get_rehash(info); + /* it could happens that all the blocks are EMPTY/BLK and CHG but with the hash */ + /* still matching because the specific CHG block was not modified. */ + /* In such case, we can avoid to update parity, because it would be the same as before */ + parity_needs_to_be_updated = 0; + + /* if the block is marked as bad, we force the parity update */ + /* because the bad block may be the result of a wrong parity */ + if (info_get_bad(info)) + parity_needs_to_be_updated = 1; + /* for each disk, process the block */ for(j=0;jblock_size); @@ -288,6 +314,26 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari goto bail; } } else { + /* if until now the parity doesn't need to be update */ + if (!parity_needs_to_be_updated) { + /* for sure it's a CHG block, because EMPTY are processed before with "continue" */ + /* and BLK have "block_has_updated_hash()" as 1, and all the others */ + /* have "parity_needs_to_be_updated" already at 1 */ + assert(block_state_get(block) == BLOCK_STATE_CHG); + + /* if there is an hash */ + if (block_has_any_hash(block)) { + /* check if the hash is changed */ + if (memcmp(hash, block->hash, HASH_SIZE) != 0) { + /* the block is different, and we must update parity */ + parity_needs_to_be_updated = 1; + } + } else { + /* if the hash is already invalid, we update parity */ + parity_needs_to_be_updated = 1; + } + } + /* copy the hash in the block, but doesn't mark the block as hashed */ /* this allow in case of skipped block to do not save the failed computation */ memcpy(block->hash, hash, HASH_SIZE); @@ -300,17 +346,20 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari /* if we have read all the data required, proceed with the parity */ if (!skip_this_block) { - /* compute the parity */ - raid_gen(state->level, buffer, diskmax, state->block_size); + /* updates the parity only if really needed */ + if (parity_needs_to_be_updated) { + /* compute the parity */ + raid_gen(state->level, buffer, diskmax, state->block_size); - /* write the parity */ - for(l=0;llevel;++l) { - ret = parity_write(parity[l], i, buffer[diskmax+l], state->block_size); - if (ret == -1) { - fprintf(stderr, "DANGER! Write error in the %s disk, it isn't possible to sync.\n", lev_name(l)); - printf("Stopping at block %u\n", i); - ++error; - goto bail; + /* write the parity */ + for(l=0;llevel;++l) { + ret = parity_write(parity[l], i, buffer[diskmax+l], state->block_size); + if (ret == -1) { + fprintf(stderr, "DANGER! Write error in the %s disk, it isn't possible to sync.\n", lev_name(l)); + printf("Stopping at block %u\n", i); + ++error; + goto bail; + } } } @@ -336,18 +385,23 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari block_state_set(block, BLOCK_STATE_BLK); } - /* if rehash is neeed */ - if (rehash) { - /* store all the new hash already computed */ - for(j=0;jhash, rehandle[j].hash, HASH_SIZE); + /* we update the info block only if we really have updated the parity */ + /* because otherwise the time info would be misleading as we didn't */ + /* wrote the parity at this time */ + if (parity_needs_to_be_updated) { + /* if rehash is neeed */ + if (rehash) { + /* store all the new hash already computed */ + for(j=0;jhash, rehandle[j].hash, HASH_SIZE); + } } - } - /* update the time info of the block */ - /* we are also clearing any previous bad and rehash flag */ - info_set(&state->infoarr, i, info_make(now, 0, 0)); + /* update the time info of the block */ + /* we are also clearing any previous bad and rehash flag */ + info_set(&state->infoarr, i, info_make(now, 0, 0)); + } } /* mark the state as needing write */