From 37bd4b0f4adcf42331916309ad68ffc6bc147046 Mon Sep 17 00:00:00 2001 From: Andrea Mazzoleni Date: Sat, 29 Aug 2026 21:25:49 +0200 Subject: [PATCH] Make .unrecoverable a persistent one-way recovery quarantine --- cmdline/check.c | 154 +++++++++++++++++++---------------------- cmdline/handle.c | 175 +++++++++++++++++++++++++++++++++++------------ cmdline/handle.h | 10 ++- 3 files changed, 213 insertions(+), 126 deletions(-) diff --git a/cmdline/check.c b/cmdline/check.c index 83ee654e..866fd83d 100644 --- a/cmdline/check.c +++ b/cmdline/check.c @@ -188,16 +188,13 @@ * such data, but the containing file remains marked damaged and is kept under * the .unrecoverable name instead of being reported as successfully recovered. * - * Missing files are created as .unrecoverable from the beginning, and an - * existing .unrecoverable file is reused by later fix runs. This allows a - * recovery to proceed over multiple runs without exposing a partially rebuilt - * file under its final name. + * Missing files are created under their normal names. They are quarantined only + * if recovery produces data that cannot be certified as CURRENT. An existing + * .unrecoverable file is reused by later fix runs and is never promoted back to + * the normal name. * - * Promotion to the final file name is a file-level commit operation. It happens - * only after the complete file has been processed successfully, its metadata - * has been restored, and no block remains damaged. A partial -S/-B fix operates - * only at block level and therefore never performs this file-level promotion - * or other finalization. + * A partial -S/-B fix operates only at block level and therefore never performs + * file-level finalization. */ /****************************************************************************/ @@ -1293,8 +1290,8 @@ static int file_post(struct snapraid_state* state, int fix, int partial, block_o /* * A partial fix operates only at block level. Reaching the last block of a * file doesn't imply that the whole file was processed, so don't perform any - * file-level finalization, including marking it finished, promoting an - * .unrecoverable file, reporting it recovered, or restoring its timestamp. + * file-level finalization, including marking it finished, reporting its final + * status, or restoring its timestamp. * This is intentional also when an explicit block range happens to cover all * the parity blocks, to keep -S/-B behavior independent of the parity size. */ @@ -1309,35 +1306,6 @@ static int file_post(struct snapraid_state* state, int fix, int partial, block_o /* if the file is damaged, meaning that a fix failed */ if (file_flag_has(file, FILE_IS_DAMAGED)) { - char path[PATH_MAX]; - char path_to[PATH_MAX]; - - pathprint(path, sizeof(path), "%s%s", disk->dir, file->sub); - pathprint(path_to, sizeof(path_to), "%s%s.unrecoverable", disk->dir, file->sub); - - /* ensure to close the file before renaming */ - 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_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)); @@ -1401,7 +1369,7 @@ static int file_post(struct snapraid_state* state, int fix, int partial, block_o || collide_file->mtime_sec != file->mtime_sec /* if the mtime is different, the collision is identified */ || collide_file->mtime_nsec != file->mtime_nsec /* same for mtime_nsec */ ) { - /* set the original modification time before promoting/renaming */ + /* set the original modification time */ ret = handle_utime(&handle[j]); if (ret == -1) { /* LCOV_EXCL_START */ @@ -1418,36 +1386,14 @@ static int file_post(struct snapraid_state* state, int fix, int partial, block_o } /* - * This rename is the file-level recovery commit point. Keep the file - * quarantined until every block and metadata was processed without error; - * promoting only after setting the timestamp ensures the final file is - * never exposed with incomplete data or provisional timestamp. + * .unrecoverable is a persistent one-way quarantine marker. + * A later fix may improve its contents, but it must never + * automatically promote it back to the normal filename. */ 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 */ - } + log_tag("status:unrecoverable:%s:%s\n", disk->name, esc_tag(file->sub)); + msg_info("unrecoverable %s\n", fmt_term(disk, file->sub)); + goto close_and_continue; } log_tag("status:recovered:%s:%s\n", disk->name, esc_tag(file->sub)); @@ -2083,8 +2029,28 @@ static int state_check_process(struct snapraid_state* state, int fix, struct sna /* keep track of damaged files */ for (j = 0; j < failed_count; ++j) { - if (failed[j].is_bad) - file_flag_set(failed[j].file, FILE_IS_DAMAGED); + if (!failed[j].is_bad) + continue; + + file_flag_set(failed[j].file, FILE_IS_DAMAGED); + + /* + * Persist the unrecoverable state immediately. + * Previous blocks of this file may already have been + * successfully recovered. + */ + if (fix && !file_flag_has(failed[j].file, FILE_IS_EXCLUDED) && !(state->opt.syncedonly && file_flag_has(failed[j].file, FILE_IS_UNSYNCED))) { + ret = handle_quarantine(failed[j].handle, failed[j].file, state->file_mode); + if (ret == -1) { + /* LCOV_EXCL_START */ + log_tag("%s:%" PRIu64 ":%s:%s: Quarantine error. %s.\n", es(errno), i, failed[j].disk->name, esc_tag(failed[j].file->sub), strerror(errno)); + log_fatal_errno(errno, failed[j].disk->name); + log_fatal(errno, "Stopping at block %" PRIu64 "\n", i); + ++unrecoverable_error; + goto bail; + /* LCOV_EXCL_STOP */ + } + } } } else { /* @@ -2149,32 +2115,56 @@ static int state_check_process(struct snapraid_state* state, int fix, struct sna /* * Write the best recovered candidate even when it is out-of-date. Such data * may still be useful to the user or to a later multistep recovery. - * - * An out-of-date write is not considered a successful fix: the file is marked - * damaged below and file_post() keeps or moves it to .unrecoverable instead - * of promoting it as recovered. */ + /* + * An out-of-date candidate is useful best-effort data, + * but it cannot be certified as the required CURRENT + * contents. Persist this fact before modifying the file + * so that a SIGKILL cannot leave unproven recovered data + * under the normal filename. + */ + if (failed[j].is_outofdate) { + file_flag_set(failed[j].file, FILE_IS_DAMAGED); + + ret = handle_quarantine(failed[j].handle, failed[j].file, state->file_mode); + if (ret == -1) { + /* LCOV_EXCL_START */ + log_tag("%s:%" PRIu64 ":%s:%s: Quarantine error. %s.\n", es(errno), i, failed[j].disk->name, esc_tag(failed[j].file->sub), strerror(errno)); + log_fatal_errno(errno, failed[j].disk->name); + log_fatal(errno, "Stopping at block %" PRIu64 "\n", i); + ++unrecoverable_error; + goto bail; + /* LCOV_EXCL_STOP */ + } + } + ret = handle_write(failed[j].handle, failed[j].file_pos, buffer[failed[j].index], state->block_size); if (ret == -1) { /* LCOV_EXCL_START */ - log_tag("%s:%" PRIu64 ":%s:%s: Write error. %s.\n", es(errno), i, failed[j].disk->name, esc_tag(failed[j].file->sub), strerror(errno)); - log_fatal_errno(errno, failed[j].disk->name); - log_fatal(errno, "Stopping at block %" PRIu64 "\n", i); + int saved_errno = errno; /* mark the file as damaged */ file_flag_set(failed[j].file, FILE_IS_DAMAGED); + ret = handle_quarantine(failed[j].handle, failed[j].file, state->file_mode); + if (ret == -1) { + log_tag("%s:%" PRIu64 ":%s:%s: Quarantine error. %s.\n", es(errno), i, failed[j].disk->name, esc_tag(failed[j].file->sub), strerror(errno)); + log_fatal_errno(errno, failed[j].disk->name); + } + + errno = saved_errno; + log_tag("%s:%" PRIu64 ":%s:%s: Write error. %s.\n", es(errno), i, failed[j].disk->name, esc_tag(failed[j].file->sub), strerror(errno)); + log_fatal_errno(errno, failed[j].disk->name); + log_fatal(errno, "Stopping at block %" PRIu64 "\n", i); + ++unrecoverable_error; goto bail; /* LCOV_EXCL_STOP */ } /* if we are not sure that the recovered content is uptodate */ - if (failed[j].is_outofdate) { - /* mark the file as damaged */ - file_flag_set(failed[j].file, FILE_IS_DAMAGED); + if (failed[j].is_outofdate) continue; - } /* * Mark the file as containing some fixes diff --git a/cmdline/handle.c b/cmdline/handle.c index ff5fdfda..f03d0d14 100644 --- a/cmdline/handle.c +++ b/cmdline/handle.c @@ -14,6 +14,8 @@ int handle_create(struct snapraid_handle* handle, struct snapraid_file* file, in { int ret; int flags; + char path[PATH_MAX]; + char path_unrecoverable[PATH_MAX]; /* if it's the same file, and already opened, nothing to do */ if (handle->file == file && handle->f != -1) { @@ -21,9 +23,11 @@ int handle_create(struct snapraid_handle* handle, struct snapraid_file* file, in } advise_init(&handle->advise, mode); - pathprint(handle->path, sizeof(handle->path), "%s%s", handle->disk->dir, file->sub); - ret = mkancestor(handle->path); + pathprint(path, sizeof(path), "%s%s", handle->disk->dir, file->sub); + pathprint(path_unrecoverable, sizeof(path_unrecoverable), "%s.unrecoverable", path); + + ret = mkancestor(path); if (ret != 0) { /* LCOV_EXCL_START */ return -1; @@ -42,63 +46,61 @@ int handle_create(struct snapraid_handle* handle, struct snapraid_file* file, in */ flags = O_BINARY | O_NOFOLLOW | advise_flags(&handle->advise); - /* open for read write */ - handle->f = open(handle->path, flags | O_RDWR); + /* + * A previous .unrecoverable file is a persistent one-way quarantine. + * Always give it precedence over the normal filename, even if both exist. + */ + handle->f = open(path_unrecoverable, flags | O_RDWR); /* if failed for missing write permission */ if (handle->f == -1 && (errno == EACCES || errno == EROFS)) { int saved_errno = errno; /* open for read-only */ - handle->f = open(handle->path, flags | O_RDONLY); + handle->f = open(path_unrecoverable, flags | O_RDONLY); if (handle->f != -1) { handle->readonly_errno = saved_errno; } } - /* if failed for missing file */ - if (handle->f == -1 && errno == ENOENT) { - char path_unrecoverable[PATH_MAX]; + if (handle->f != -1) { + handle->is_unrecoverable = 1; + pathcpy(handle->path, sizeof(handle->path), path_unrecoverable); + } else if (errno == ENOENT) { + /* no previous quarantine exists, try the normal filename */ + handle->f = open(path, flags | O_RDWR); - /* check if exists a .unrecoverable copy */ - pathprint(path_unrecoverable, sizeof(path_unrecoverable), "%s.unrecoverable", handle->path); - - /* - * 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. - */ - 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 missing file as .unrecoverable so that an interruption or - * crash (e.g. SIGKILL) during recovery never leaves a partially recovered - * file visible under its final name. - */ - handle->f = open(path_unrecoverable, flags | O_RDWR | O_CREAT | O_EXCL, 0600); + /* if failed for missing write permission */ + if (handle->f == -1 && (errno == EACCES || errno == EROFS)) { + int saved_errno = errno; + /* open for read-only */ + handle->f = open(path, flags | O_RDONLY); if (handle->f != -1) { - handle->is_unrecoverable = 1; - pathcpy(handle->path, sizeof(handle->path), path_unrecoverable); + handle->readonly_errno = saved_errno; + } + } + + /* if failed for missing file */ + if (handle->f == -1 && errno == ENOENT) { + /* + * The file is genuinely missing and there is no previous + * quarantine. Create it using the normal filename. + * + * It will be moved to .unrecoverable only if recovery later + * encounters data that cannot be certified CURRENT. + */ + handle->f = open(path, flags | O_RDWR | O_CREAT | O_EXCL, 0600); + if (handle->f != -1) { + handle->created = 1; } else if (errno == EEXIST) { /* if created concurrently, reopen safely */ - 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 { - pathcpy(handle->path, sizeof(handle->path), path_unrecoverable); - } - } else { - pathcpy(handle->path, sizeof(handle->path), path_unrecoverable); + handle->f = open(path, flags | O_RDWR); } - } else { - /* report the path that failed to open */ - pathcpy(handle->path, sizeof(handle->path), path_unrecoverable); } + + pathcpy(handle->path, sizeof(handle->path), path); + } else { + /* report the .unrecoverable path that failed to open */ + pathcpy(handle->path, sizeof(handle->path), path_unrecoverable); } if (handle->f == -1) { @@ -137,6 +139,93 @@ int handle_create(struct snapraid_handle* handle, struct snapraid_file* file, in return 0; } +/** + * Move an opened file to its persistent .unrecoverable name. + */ +int handle_quarantine(struct snapraid_handle* handle, struct snapraid_file* file, int mode) +{ + char path[PATH_MAX]; + char path_to[PATH_MAX]; + struct stat st; + int ret; + + (void)mode; + + /* already persistently quarantined */ + if (handle->is_unrecoverable) + return 0; + + pathprint(path, sizeof(path), "%s%s", handle->disk->dir, file->sub); + pathprint(path_to, sizeof(path_to), "%s%s.unrecoverable", handle->disk->dir, file->sub); + + /* + * Never overwrite a previous quarantine. + */ + ret = lstat(path_to, &st); + if (ret == 0) { + errno = EEXIST; + log_error(errno, "Unrecoverable file '%s' already exists.\n", path_to); + return -1; + } + if (errno != ENOENT) { + /* LCOV_EXCL_START */ + log_error(errno, "Error accessing file '%s'. %s.\n", path_to, strerror(errno)); + return -1; + /* LCOV_EXCL_STOP */ + } + +#ifdef _WIN32 + /* + * Windows may not allow renaming an opened file. Close it before + * the rename and reopen the quarantined file afterward. + */ + ret = handle_close(handle); + if (ret != 0) + return -1; + + ret = rename(path, path_to); + if (ret != 0) { + /* LCOV_EXCL_START */ + log_error(errno, "Error renaming file '%s' to '%s'. %s.\n", path, path_to, strerror(errno)); + return -1; + /* LCOV_EXCL_STOP */ + } + + /* + * handle_create() gives precedence to the existing .unrecoverable + * file, so it reopens path_to and sets is_unrecoverable. + */ + ret = handle_create(handle, file, mode); + if (ret != 0) + return -1; + + if (!handle->is_unrecoverable) { + /* LCOV_EXCL_START */ + errno = ENXIO; + log_error(errno, "Error renaming file '%s' to '%s'. %s.\n", path, path_to, strerror(errno)); + return -1; + /* LCOV_EXCL_STOP */ + } +#else + /* + * POSIX allows renaming an opened file. The descriptor continues + * to reference the same inode, so no close/reopen is required. + */ + ret = rename(path, path_to); + if (ret != 0) { + /* LCOV_EXCL_START */ + log_error(errno, "Error renaming file '%s' to '%s'. %s.\n", path, path_to, strerror(errno)); + return -1; + /* LCOV_EXCL_STOP */ + } + + handle->is_unrecoverable = 1; + pathcpy(handle->path, sizeof(handle->path), path_to); +#endif + + return 0; +} + int handle_truncate(struct snapraid_handle* handle, struct snapraid_file* file) { int ret; diff --git a/cmdline/handle.h b/cmdline/handle.h index cc35ee3b..22ab158b 100644 --- a/cmdline/handle.h +++ b/cmdline/handle.h @@ -37,7 +37,8 @@ struct snapraid_handle { /** * Create a file. - * The file is created if missing, and opened with write access. + * An existing .unrecoverable file takes precedence over the normal filename. + * The normal file is created if both names are 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. @@ -46,6 +47,13 @@ struct snapraid_handle { */ int handle_create(struct snapraid_handle* handle, struct snapraid_file* file, int mode); +/** + * Move an opened file to its persistent .unrecoverable name. + * If already quarantined, do nothing. + * On error, log the failure and return -1 with errno set. + */ +int handle_quarantine(struct snapraid_handle* handle, struct snapraid_file* file, int mode); + /** * Truncate a file if required. */