diff --git a/cmdline/check.c b/cmdline/check.c index 69f93ca5..4de90be2 100644 --- a/cmdline/check.c +++ b/cmdline/check.c @@ -690,6 +690,17 @@ static int file_post(struct snapraid_state* state, int fix, block_off_t i, struc /* finish the fix process if it's the last block of the files */ if (fix) { + int was_unrecoverable; + + if (handle[j].file != file) { + /* LCOV_EXCL_START */ + log_fatal(EINTERNAL, "Internal inconsistency in file handle '%s' instead of '%s'\n", + handle[j].file != 0 ? handle[j].file->sub : "", file->sub); + os_abort(); + /* LCOV_EXCL_STOP */ + } + was_unrecoverable = handle[j].is_unrecoverable; + /* * Mark that we finished with this file * to identify later any NOT finished ones @@ -698,7 +709,6 @@ static int file_post(struct snapraid_state* state, int fix, block_off_t i, struc /* if the file is damaged, meaning that a fix failed */ if (file_flag_has(file, FILE_IS_DAMAGED)) { - /* rename it to .unrecoverable */ char path[PATH_MAX]; char path_to[PATH_MAX]; @@ -706,27 +716,28 @@ static int file_post(struct snapraid_state* state, int fix, block_off_t i, struc pathprint(path_to, sizeof(path_to), "%s%s.unrecoverable", disk->dir, file->sub); /* ensure to close the file before renaming */ - if (handle[j].file == file) { - ret = handle_close(&handle[j]); + ret = handle_close(&handle[j]); + if (ret != 0) { + /* LCOV_EXCL_START */ + log_tag("%s:%" PRIu64 ":%s:%s: Close error. %s.\n", es(errno), i, disk->name, esc_tag(file->sub), strerror(errno)); + log_fatal_errno(errno, disk->name); + return -1; + /* LCOV_EXCL_STOP */ + } + + /* a previous recovery is already quarantined */ + if (!was_unrecoverable) { + ret = rename(path, path_to); if (ret != 0) { /* LCOV_EXCL_START */ - log_tag("%s:%" PRIu64 ":%s:%s: Close error. %s.\n", es(errno), i, disk->name, esc_tag(file->sub), strerror(errno)); + log_fatal(errno, "Error renaming '%s%s'. %s.\n", disk->dir, file->sub, strerror(errno)); + log_tag("%s:%" PRIu64 ":%s:%s: Rename error. %s.\n", es(errno), i, disk->name, esc_tag(file->sub), strerror(errno)); log_fatal_errno(errno, disk->name); return -1; /* LCOV_EXCL_STOP */ } } - ret = rename(path, path_to); - if (ret != 0) { - /* LCOV_EXCL_START */ - log_fatal(errno, "Error renaming '%s%s'. %s.\n", disk->dir, file->sub, strerror(errno)); - log_tag("%s:%" PRIu64 ":%s:%s: Rename error. %s.\n", es(errno), i, disk->name, esc_tag(file->sub), strerror(errno)); - log_fatal_errno(errno, disk->name); - return -1; - /* LCOV_EXCL_STOP */ - } - log_tag("status:unrecoverable:%s:%s\n", disk->name, esc_tag(file->sub)); msg_info("unrecoverable %s\n", fmt_term(disk, file->sub)); @@ -734,8 +745,40 @@ static int file_post(struct snapraid_state* state, int fix, block_off_t i, struc goto close_and_continue; } + /* + * This rename is the file-level recovery commit point. Keep the file + * quarantined until every block was processed without FILE_IS_DAMAGED; + * block-by-block promotion could expose an incomplete recovery. + */ + if (was_unrecoverable) { + char path[PATH_MAX]; + char path_from[PATH_MAX]; + + pathprint(path, sizeof(path), "%s%s", disk->dir, file->sub); + pathprint(path_from, sizeof(path_from), "%s%s.unrecoverable", disk->dir, file->sub); + + ret = handle_close(&handle[j]); + if (ret != 0) { + /* LCOV_EXCL_START */ + log_tag("%s:%" PRIu64 ":%s:%s: Close error. %s.\n", es(errno), i, disk->name, esc_tag(file->sub), strerror(errno)); + log_fatal_errno(errno, disk->name); + return -1; + /* LCOV_EXCL_STOP */ + } + + ret = rename(path_from, path); + if (ret != 0) { + /* LCOV_EXCL_START */ + log_fatal(errno, "Error renaming '%s'. %s.\n", path_from, strerror(errno)); + log_tag("%s:%" PRIu64 ":%s:%s: Rename error. %s.\n", es(errno), i, disk->name, esc_tag(file->sub), strerror(errno)); + log_fatal_errno(errno, disk->name); + return -1; + /* LCOV_EXCL_STOP */ + } + } + /* if the file is not fixed, meaning that it is untouched */ - if (!file_flag_has(file, FILE_IS_FIXED)) { + if (!file_flag_has(file, FILE_IS_FIXED) && !was_unrecoverable) { /* nothing to do, but close the file */ goto close_and_continue; } @@ -756,11 +799,8 @@ static int file_post(struct snapraid_state* state, int fix, block_off_t i, struc /* LCOV_EXCL_STOP */ } - /* - * Reopen it as readonly, as to set the mtime readonly access it's enough - * we know that the file exists because it has the FILE_IS_FIXED tag - */ - ret = handle_open(&handle[j], file, state->file_mode, 0); + /* reopen the promoted file for writing, as required to set the mtime on Windows */ + ret = handle_create(&handle[j], file, state->file_mode); if (ret != 0) { /* LCOV_EXCL_START */ log_tag("%s:%" PRIu64 ":%s:%s: Open error. %s.\n", es(errno), i, disk->name, esc_tag(file->sub), strerror(errno)); @@ -2341,4 +2381,3 @@ int state_check(struct snapraid_state* state, int fix, block_off_t blockstart, b return 0; } - diff --git a/cmdline/handle.c b/cmdline/handle.c index b71afd8d..ae53ad4d 100644 --- a/cmdline/handle.c +++ b/cmdline/handle.c @@ -32,6 +32,7 @@ int handle_create(struct snapraid_handle* handle, struct snapraid_file* file, in /* initial values, changed later if required */ handle->created = 0; + handle->is_unrecoverable = 0; handle->readonly_errno = 0; /* @@ -56,26 +57,32 @@ int handle_create(struct snapraid_handle* handle, struct snapraid_file* file, in /* if failed for missing file */ if (handle->f == -1 && errno == ENOENT) { - char path_from[PATH_MAX]; + char path_unrecoverable[PATH_MAX]; - /* check if exists a .unrecoverable copy, and rename to the real one */ - pathprint(path_from, sizeof(path_from), "%s.unrecoverable", handle->path); + /* check if exists a .unrecoverable copy */ + pathprint(path_unrecoverable, sizeof(path_unrecoverable), "%s.unrecoverable", handle->path); /* - * Reuse a previous partial recovery as the starting point for a - * multistep fix. Readable CHG blocks in this file are intentionally - * preserved because their current hash is not available. + * Open a previous recovery directly as .unrecoverable, keeping it + * quarantined until the whole file is processed successfully. Promoting + * it now could expose an incomplete or unverified file if this fix aborts + * or fails again. Reuse it for a multistep fix, preserving readable CHG + * blocks because their current hash is not available. */ - if (rename(path_from, handle->path) == 0) { - /* open for read write */ - handle->f = open(handle->path, flags | O_RDWR); - } else { + handle->f = open(path_unrecoverable, flags | O_RDWR); + if (handle->f != -1) { + handle->is_unrecoverable = 1; + pathcpy(handle->path, sizeof(handle->path), path_unrecoverable); + } else if (errno == ENOENT) { /* create it */ handle->f = open(handle->path, flags | O_RDWR | O_CREAT, 0600); if (handle->f != -1) { /* mark it as created if really done */ handle->created = 1; } + } else { + /* report the path that failed to open */ + pathcpy(handle->path, sizeof(handle->path), path_unrecoverable); } } @@ -165,6 +172,7 @@ int handle_open(struct snapraid_handle* handle, struct snapraid_file* file, int /* for sure not created */ handle->created = 0; + handle->is_unrecoverable = 0; handle->readonly_errno = 0; /* @@ -232,6 +240,7 @@ int handle_close(struct snapraid_handle* handle) handle->file = 0; handle->f = -1; handle->valid_size = 0; + handle->is_unrecoverable = 0; return -1; /* LCOV_EXCL_STOP */ } @@ -241,6 +250,7 @@ int handle_close(struct snapraid_handle* handle) handle->file = 0; handle->f = -1; handle->valid_size = 0; + handle->is_unrecoverable = 0; handle->readonly_errno = 0; return 0; @@ -424,6 +434,7 @@ struct snapraid_handle* handle_mapping(struct snapraid_state* state, unsigned* h handle[j].file = 0; handle[j].f = -1; handle[j].valid_size = 0; + handle[j].is_unrecoverable = 0; handle[j].readonly_errno = 0; handle[j].bw = 0; } diff --git a/cmdline/handle.h b/cmdline/handle.h index cfac05b6..9ea4dbbc 100644 --- a/cmdline/handle.h +++ b/cmdline/handle.h @@ -20,6 +20,7 @@ struct snapraid_handle { struct advise_struct advise; /**< Advise information. */ data_off_t valid_size; /**< Size of the valid data. */ int created; /**< If the file was created, otherwise it was already existing. */ + int is_unrecoverable; /**< If the open descriptor refers to the .unrecoverable path. */ int readonly_errno; /**< Non-zero if opened read-only as fallback. */ struct snapraid_bw* bw; /**< Context for bandwidth limiting. */ }; @@ -28,6 +29,7 @@ struct snapraid_handle { * Create a file. * The file is created if missing, and opened with write access. * If the file is created, the handle->created is set. + * If a previous .unrecoverable file is opened, the handle->is_unrecoverable is set. * The initial size of the file is stored in the file->st struct. * If the file cannot be opened for write access, it's opened with read-only access. * The read-only access works only if the file has already the correct size and doesn't need to be modified.