From e80009334b28b4cec57ad62d18274692fbdceb31 Mon Sep 17 00:00:00 2001 From: Andrea Mazzoleni Date: Sat, 19 Mar 2011 22:00:08 +0100 Subject: [PATCH] Even better error management. More precise and explicative. --- check.c | 29 +++++++++++--------- handle.c | 16 +++++++---- handle.h | 1 + snapraid.c | 13 +++++++-- state.c | 3 ++ state.h | 3 +- sync.c | 80 ++++++++++++++++++++++++++++++++++++++++++++---------- 7 files changed, 108 insertions(+), 37 deletions(-) diff --git a/check.c b/check.c index 530cafc1..12787fb0 100644 --- a/check.c +++ b/check.c @@ -106,6 +106,7 @@ static int state_check_process(struct snapraid_state* state, int fix, int parity if (ret == -1) { fprintf(stderr, "WARNING! Without a working data disk, it isn't possible to fix errors on it.\n"); fprintf(stderr, "Stopping at block %u\n", i); + ++unrecoverable_error; goto bail; } @@ -115,6 +116,7 @@ static int state_check_process(struct snapraid_state* state, int fix, int parity if (ret == -1) { fprintf(stderr, "WARNING! Without a working data disk, it isn't possible to fix errors on it.\n"); fprintf(stderr, "Stopping at block %u\n", i); + ++unrecoverable_error; goto bail; } } else { @@ -203,6 +205,7 @@ static int state_check_process(struct snapraid_state* state, int fix, int parity if (ret == -1) { fprintf(stderr, "WARNING! Without a working parity disk, it isn't possible to fix errors on it.\n"); fprintf(stderr, "Stopping at block %u\n", i); + ++unrecoverable_error; goto bail; } @@ -252,6 +255,7 @@ static int state_check_process(struct snapraid_state* state, int fix, int parity if (ret == -1) { fprintf(stderr, "WARNING! Without a working data disk, it isn't possible to fix errors on it.\n"); fprintf(stderr, "Stopping at block %u\n", i); + ++unrecoverable_error; goto bail; } @@ -281,6 +285,7 @@ bail: if (ret == -1) { if (fix) { fprintf(stderr, "WARNING! A 'snapraid check' command is highly suggested.\n"); + ++unrecoverable_error; /* continue, as we are already exiting */ } } @@ -309,10 +314,8 @@ bail: free(block_buffer); free(xor_buffer); - /* signal if there are unrecoverable errors */ if (unrecoverable_error != 0) return -1; - return 0; } @@ -323,6 +326,7 @@ void state_check(struct snapraid_state* state, int fix, block_off_t blockstart) data_off_t size; int ret; int f; + unsigned unrecoverable_error; if (fix) printf("Checking and fixing...\n"); @@ -356,19 +360,14 @@ void state_check(struct snapraid_state* state, int fix, block_off_t blockstart) } } + unrecoverable_error = 0; + /* skip degenerated cases of empty parity, or skipping all */ if (blockstart < blockmax) { ret = state_check_process(state, fix, f, blockstart, blockmax); if (ret == -1) { - if (f != -1) { - ret = parity_close(path, f); - if (ret != -1) { - fprintf(stderr, "WARNING! A 'snapraid check' command is highly suggested.\n"); - /* continue, as we are already exiting */ - } - } - /* exit with a failure code */ - exit(EXIT_FAILURE); + ++unrecoverable_error; + /* continue, as we are already exiting */ } } @@ -377,8 +376,12 @@ void state_check(struct snapraid_state* state, int fix, block_off_t blockstart) ret = parity_close(path, f); if (ret == -1) { fprintf(stderr, "WARNING! A 'snapraid check' command is highly suggested.\n"); - exit(EXIT_FAILURE); + ++unrecoverable_error; + /* continue, as we are already exiting */ } } -} + /* abort if required */ + if (unrecoverable_error != 0) + exit(EXIT_FAILURE); +} diff --git a/handle.c b/handle.c index 8af386a0..5b97cb57 100644 --- a/handle.c +++ b/handle.c @@ -56,7 +56,6 @@ int handle_close_if_different(struct snapraid_handle* handle, struct snapraid_fi int handle_create(struct snapraid_handle* handle, struct snapraid_file* file) { - struct stat st; int ret; /* if it's the same file, and already opened, nothing to do */ @@ -79,14 +78,14 @@ int handle_create(struct snapraid_handle* handle, struct snapraid_file* file) /* just opened */ handle->file = file; - /* set the size */ - ret = fstat(handle->f, &st); + /* get the stat info */ + ret = fstat(handle->f, &handle->st); if (ret != 0) { fprintf(stderr, "Error accessing file '%s'. %s.\n", handle->path, strerror(errno)); return -1; } - if (st.st_size < file->size) { + if (handle->st.st_size < file->size) { #if HAVE_POSIX_FALLOCATE /* allocate real space */ ret = posix_fallocate(handle->f, 0, file->size); @@ -102,7 +101,7 @@ int handle_create(struct snapraid_handle* handle, struct snapraid_file* file) } return -1; } - } else if (st.st_size > file->size) { + } else if (handle->st.st_size > file->size) { ret = ftruncate(handle->f, file->size); if (ret != 0) { fprintf(stderr, "Error truncating file '%s'. %s.\n", handle->path, strerror(errno)); @@ -146,6 +145,13 @@ int handle_open(struct snapraid_handle* handle, struct snapraid_file* file) /* just opened */ handle->file = file; + /* get the stat info */ + ret = fstat(handle->f, &handle->st); + if (ret != 0) { + fprintf(stderr, "Error accessing file '%s'. %s.\n", handle->path, strerror(errno)); + return -1; + } + #if HAVE_POSIX_FADVISE /* advise sequential access */ ret = posix_fadvise(handle->f, 0, 0, POSIX_FADV_SEQUENTIAL); diff --git a/handle.h b/handle.h index ef53915b..19833201 100644 --- a/handle.h +++ b/handle.h @@ -26,6 +26,7 @@ struct snapraid_handle { struct snapraid_disk* disk; /**< Disk of the file. */ struct snapraid_file* file; /**< File opened. */ int f; /**< Handle of the file. */ + struct stat st; /**< Stat info of the opened file. */ }; /** diff --git a/snapraid.c b/snapraid.c index beab6a78..87ffa1a4 100644 --- a/snapraid.c +++ b/snapraid.c @@ -83,6 +83,7 @@ int main(int argc, char* argv[]) struct snapraid_state state; int operation; block_off_t blockstart; + int ret; /* intercept Ctrl+C */ signal(SIGINT, &signal_handler); @@ -158,10 +159,16 @@ int main(int argc, char* argv[]) state_scan(&state); - state_sync(&state, blockstart); + ret = state_sync(&state, blockstart); - state_write(&state); - } else if (operation == OPERATION_CHECK || operation == OPERATION_FIX) { + /* save the new state if required */ + if (state.need_write) + state_write(&state); + + /* abort if required by the sync command */ + if (ret == -1) + exit(EXIT_FAILURE); + } else { state_read(&state); state_check(&state, operation == OPERATION_FIX, blockstart); diff --git a/state.c b/state.c index f0c7370c..e011d089 100644 --- a/state.c +++ b/state.c @@ -29,6 +29,9 @@ void state_init(struct snapraid_state* state) { state->verbose = 0; + state->force_zero = 0; + state->force_empty = 0; + state->need_write = 0; state->block_size = 256 * 1024; /* default 256 kB */ state->content[0] = 0; state->parity[0] = 0; diff --git a/state.h b/state.h index 3d16e0b5..b79ba1a5 100644 --- a/state.h +++ b/state.h @@ -30,6 +30,7 @@ struct snapraid_state { int verbose; /**< Verbose output. */ int force_zero; /**< Forced dangerous operations of synching file now with zero size. */ int force_empty; /**< Forced dangerous operations of synching disk now empty. */ + int need_write; /**< If the state is changed. */ uint32_t block_size; /**< Block size in bytes. */ char content[PATH_MAX]; /**< Path of the content file. */ char parity[PATH_MAX]; /**< Path of the parity file. */ @@ -70,7 +71,7 @@ void state_scan(struct snapraid_state* state); /** * Syncs the parity data. */ -void state_sync(struct snapraid_state* state, block_off_t blockstart); +int state_sync(struct snapraid_state* state, block_off_t blockstart); /** * Checks (and fixes) all the files and the parity data. diff --git a/sync.c b/sync.c index d9c0a4ec..a3e11ca8 100644 --- a/sync.c +++ b/sync.c @@ -26,7 +26,7 @@ /****************************************************************************/ /* sync */ -static void state_sync_process(struct snapraid_state* state, int parity_f, block_off_t blockstart, block_off_t blockmax) +static int state_sync_process(struct snapraid_state* state, int parity_f, block_off_t blockstart, block_off_t blockmax) { struct snapraid_handle* handle; unsigned diskmax = tommy_array_size(&state->diskarr); @@ -39,6 +39,7 @@ static void state_sync_process(struct snapraid_state* state, int parity_f, block time_t start; time_t last; int ret; + unsigned unrecoverable_error; block_buffer = malloc_nofail(state->block_size); xor_buffer = malloc_nofail(state->block_size); @@ -49,6 +50,7 @@ static void state_sync_process(struct snapraid_state* state, int parity_f, block handle[i].file = 0; handle[i].f = -1; } + unrecoverable_error = 0; count_size = 0; count_block = 0; @@ -88,22 +90,46 @@ static void state_sync_process(struct snapraid_state* state, int parity_f, block ret = handle_close_if_different(&handle[j], block->file); if (ret == -1) { - fprintf(stderr, "WARNING! Without an accessible data disk, it isn't possible to sync.\n"); + /* This one is really an unexpected error, because we are only reading */ + /* and closing a descriptor should never fail */ + fprintf(stderr, "DANGER! Unexpected close error in a data disk, it isn't possible to sync.\n"); fprintf(stderr, "Stopping at block %u\n", i); + ++unrecoverable_error; goto bail; } ret = handle_open(&handle[j], block->file); if (ret == -1) { - fprintf(stderr, "WARNING! Without an accessible data disk, it isn't possible to sync.\n"); + if (errno == ENOENT) { + fprintf(stderr, "Missing file '%s'.\n", handle[j].path); + fprintf(stderr, "WARNING! You cannot modify data disk during a sync. Rerun the sync command when finished.\n"); + fprintf(stderr, "Stopping at block %u\n", i); + } else if (errno == EACCES) { + fprintf(stderr, "No access at file '%s'.\n", handle[j].path); + fprintf(stderr, "WARNING! Please fix the access permission in the data disk.\n"); + fprintf(stderr, "Stopping at block %u\n", i); + } else { + fprintf(stderr, "DANGER! Unexpected open error in a data disk, it isn't possible to sync.\n"); + fprintf(stderr, "Stopping to allow recovery. Try with 'snapraid check'\n"); + } + ++unrecoverable_error; + goto bail; + } + + /* check if the file is changed */ + if (handle[j].st.st_size != block->file->size || handle[j].st.st_mtime != block->file->mtime) { + fprintf(stderr, "Unexpected change at file '%s'.\n", handle[j].path); + fprintf(stderr, "WARNING! You cannot modify data disk during a sync. Rerun the sync command when finished.\n"); fprintf(stderr, "Stopping at block %u\n", i); + ++unrecoverable_error; goto bail; } read_size = handle_read(&handle[j], block, block_buffer, state->block_size); if (read_size == -1) { - fprintf(stderr, "WARNING! Without an accessible data disk, it isn't possible to sync.\n"); - fprintf(stderr, "Stopping at block %u\n", i); + fprintf(stderr, "DANGER! Unexpected read error in a data disk, it isn't possible to sync.\n"); + fprintf(stderr, "Stopping to allow recovery. Try with 'snapraid check'\n"); + ++unrecoverable_error; goto bail; } @@ -116,8 +142,9 @@ static void state_sync_process(struct snapraid_state* state, int parity_f, block /* compare the hash */ if (memcmp(hash, block->hash, HASH_MAX) != 0) { fprintf(stderr, "%u: Data error for file %s at position %u\n", i, block->file->sub, block_file_pos(block)); - fprintf(stderr, "WARNING! With errors in the data disk it isn't possible to sync.\n"); - fprintf(stderr, "Stopping to allow recovery. Try with 'snapraid -s %u fix'\n", i); + fprintf(stderr, "DANGER! Unexpected data error in a data disk, it isn't possible to sync.\n"); + fprintf(stderr, "Stopping to allow recovery. Try with 'snapraid -s %u check'\n", i); + ++unrecoverable_error; goto bail; } } else { @@ -135,8 +162,9 @@ static void state_sync_process(struct snapraid_state* state, int parity_f, block /* write the parity */ ret = parity_write(state->parity, parity_f, i, xor_buffer, state->block_size); if (ret == -1) { - fprintf(stderr, "WARNING! Without an accessible parity file, it isn't possible to sync.\n"); + fprintf(stderr, "DANGER! Write error in the parity disk, it isn't possible to sync.\n"); fprintf(stderr, "Stopping at block %u\n", i); + ++unrecoverable_error; goto bail; } @@ -151,6 +179,9 @@ static void state_sync_process(struct snapraid_state* state, int parity_f, block block->is_hashed = 1; } + /* mark the state as needing write */ + state->need_write = 1; + /* count the number of processed block */ ++count_block; @@ -167,7 +198,8 @@ bail: for(j=0;j