From 9c985612ea7d5f2a40a798469e0fa9d0fd3d8bbb Mon Sep 17 00:00:00 2001 From: Andrea Mazzoleni Date: Tue, 8 Sep 2026 22:19:55 +0200 Subject: [PATCH] Make snapshot replacement crash-safe using retiring snapshots --- cmdline/app.h | 15 +++++- cmdline/mingwapp.c | 121 +++++++++++++++++++++++++++++++++++++---- cmdline/state.c | 34 +++--------- cmdline/unixapp.c | 132 +++++++++++++++++++++++++++++++++++++++++---- 4 files changed, 256 insertions(+), 46 deletions(-) diff --git a/cmdline/app.h b/cmdline/app.h index 4d9b523e..9edf6b5b 100644 --- a/cmdline/app.h +++ b/cmdline/app.h @@ -85,6 +85,7 @@ int fsinfo(const char* path, int* has_persistent_inode, int* has_syncronized_har #define SNAPSHOT_PENDING "pending" #define SNAPSHOT_STABLE "stable" #define SNAPSHOT_SCAN "scan" +#define SNAPSHOT_RETIRING_SUFFIX ".retiring" /* * Snapshots context @@ -138,7 +139,7 @@ struct fssnapshot_struct { * * @param dir Full path inside the filesystem (must end with '/') * @param fss Output snapshot context - * @return 0 on success, -1 on failure + * @return 0 on success, 1 if snapshots are unsupported, -1 on failure */ int fssnapshot_mount(const char* dir, struct fssnapshot_struct* fss); @@ -189,6 +190,18 @@ int fssnapshot_delete(const struct fssnapshot_struct* fss, const char* name); */ int fssnapshot_rename(const struct fssnapshot_struct* fss, const char* old_name, const char* new_name); +/** + * Replace a snapshot while preserving the previous destination until the new one is published. + * + * An interrupted replacement is recovered automatically by fssnapshot_mount(). + * + * @param fss Snapshot context + * @param old_name Existing source snapshot name + * @param new_name Destination snapshot name + * @return 0 on success, -1 on failure + */ +int fssnapshot_replace(struct fssnapshot_struct* fss, const char* old_name, const char* new_name); + /** * Get the path where a logical snapshot is accessible. * diff --git a/cmdline/mingwapp.c b/cmdline/mingwapp.c index 772a7589..028aa03e 100644 --- a/cmdline/mingwapp.c +++ b/cmdline/mingwapp.c @@ -485,6 +485,55 @@ static int windows_delete_link(const struct fssnapshot_struct* fss, const char* return 0; } +static void fssnapshot_retiring_name(const char* name, char* retiring_name, size_t retiring_name_size) +{ + pathcpy(retiring_name, retiring_name_size, name); + pathcat(retiring_name, retiring_name_size, SNAPSHOT_RETIRING_SUFFIX); +} + +static int fssnapshot_exists(struct fssnapshot_struct* fss, const char* name) +{ + struct stat st; + + if (fssnapshot_stat(fss, name, &st) == 0) + return 1; + + if (errno == ENOENT) + return 0; + + return -1; +} + +/** + * Recover an interrupted snapshot replacement. + * + * The .retiring snapshot is the previous destination and must remain available until the new destination is published. If the destination exists, publication completed and the retiring snapshot is deleted. Otherwise the previous destination is restored. + * + * Any failure is propagated to fssnapshot_mount(). Callers may then fall back to the live filesystem with an explicit warning when snapshots are used only as a read source, while snapshot-modifying operations fail. + */ +static int fssnapshot_recover_replace(struct fssnapshot_struct* fss, const char* name) +{ + char retiring_name[PATH_MAX]; + + fssnapshot_retiring_name(name, retiring_name, sizeof(retiring_name)); + + int retiring = fssnapshot_exists(fss, retiring_name); + if (retiring <= 0) + return retiring; + + int current = fssnapshot_exists(fss, name); + if (current < 0) + return -1; + + if (current != 0) { + /* the new destination was published, so the previous one can now be retired */ + return fssnapshot_delete(fss, retiring_name); + } + + /* the new destination was not published, so restore the previous one */ + return fssnapshot_rename(fss, retiring_name, name); +} + int fssnapshot_mount(const char* dir, struct fssnapshot_struct* fss) { wchar_t conv_buf_vol[CONV_MAX]; @@ -493,6 +542,8 @@ int fssnapshot_mount(const char* dir, struct fssnapshot_struct* fss) wchar_t volume_name[PATH_MAX]; wchar_t fs_name[32]; uint32_t magic; + char pending_retiring[PATH_MAX]; + char stable_retiring[PATH_MAX]; /* * GetVolumePathNameW() accepts any path: file, directory, or deep @@ -560,18 +611,31 @@ int fssnapshot_mount(const char* dir, struct fssnapshot_struct* fss) return -1; } - /* refresh the links, after reboots they gets invalidated */ - if (windows_rebuild_link(fss, SNAPSHOT_PENDING) != 0) { - return -1; - } + fssnapshot_retiring_name(SNAPSHOT_PENDING, pending_retiring, sizeof(pending_retiring)); + fssnapshot_retiring_name(SNAPSHOT_STABLE, stable_retiring, sizeof(stable_retiring)); - if (windows_rebuild_link(fss, SNAPSHOT_STABLE) != 0) { + /* refresh the links, after reboots their DeviceObject paths are invalidated */ + if (windows_rebuild_link(fss, SNAPSHOT_PENDING) != 0) return -1; - } - if (windows_rebuild_link(fss, SNAPSHOT_SCAN) != 0) { + if (windows_rebuild_link(fss, SNAPSHOT_STABLE) != 0) + return -1; + + if (windows_rebuild_link(fss, SNAPSHOT_SCAN) != 0) + return -1; + + if (windows_rebuild_link(fss, pending_retiring) != 0) + return -1; + + if (windows_rebuild_link(fss, stable_retiring) != 0) + return -1; + + /* never expose an interrupted replacement to the state layer */ + if (fssnapshot_recover_replace(fss, SNAPSHOT_PENDING) != 0) + return -1; + + if (fssnapshot_recover_replace(fss, SNAPSHOT_STABLE) != 0) return -1; - } fss->magic = magic; @@ -591,8 +655,10 @@ int fssnapshot_stat(struct fssnapshot_struct* fss, const char* name, struct stat return -1; /* it must be a link to a directory */ - if (!S_ISLNKDIR(st->st_mode)) + if (!S_ISLNKDIR(st->st_mode)) { + errno = ENOTDIR; return -1; + } return 0; } @@ -760,6 +826,35 @@ int fssnapshot_rename(const struct fssnapshot_struct* fss, const char* old_name, return 0; } +int fssnapshot_replace(struct fssnapshot_struct* fss, const char* old_name, const char* new_name) +{ + char retiring_name[PATH_MAX]; + + fssnapshot_retiring_name(new_name, retiring_name, sizeof(retiring_name)); + + /* resolve a replacement interrupted by an earlier invocation before starting another one */ + if (fssnapshot_recover_replace(fss, new_name) != 0) + return -1; + + int current = fssnapshot_exists(fss, new_name); + if (current < 0) + return -1; + + if (current == 0) + return fssnapshot_rename(fss, old_name, new_name); + + /* keep the previous destination recoverable until the source is published under the final name */ + if (fssnapshot_rename(fss, new_name, retiring_name) != 0) + return -1; + + /* on failure leave .retiring in place; fssnapshot_mount() will restore or complete the transition */ + if (fssnapshot_rename(fss, old_name, new_name) != 0) + return -1; + + /* the new destination is now published, so the previous destination is no longer required */ + return fssnapshot_delete(fss, retiring_name); +} + int fssnapshot_path(const struct fssnapshot_struct* fss, const char* name, char* path, size_t path_size) { pathcpy(path, path_size, fss->snapshot_dir); @@ -772,6 +867,9 @@ int fssnapshot_path(const struct fssnapshot_struct* fss, const char* name, char* void fssnapshot_unmount(const struct fssnapshot_struct* fss) { + char pending_retiring[PATH_MAX]; + char stable_retiring[PATH_MAX]; + /* * Remove symlinks on exit to prevent "Dangling Links." * Since \Device\HarddiskVolumeShadowCopyN paths are reassigned by the kernel @@ -781,9 +879,14 @@ void fssnapshot_unmount(const struct fssnapshot_struct* fss) * Deleting the link ensures system hygiene and prevents users from seeing * "Location not available" errors or incorrect data after a reboot. */ + fssnapshot_retiring_name(SNAPSHOT_PENDING, pending_retiring, sizeof(pending_retiring)); + fssnapshot_retiring_name(SNAPSHOT_STABLE, stable_retiring, sizeof(stable_retiring)); + windows_delete_link(fss, SNAPSHOT_PENDING); windows_delete_link(fss, SNAPSHOT_STABLE); windows_delete_link(fss, SNAPSHOT_SCAN); + windows_delete_link(fss, pending_retiring); + windows_delete_link(fss, stable_retiring); } /****************************************************************************/ diff --git a/cmdline/state.c b/cmdline/state.c index ce134153..3a4d0066 100644 --- a/cmdline/state.c +++ b/cmdline/state.c @@ -6506,18 +6506,9 @@ int state_snapshot_pending(struct snapraid_state* state) msg_progress("Creating disk %s pending snapshot...\n", disk->name); - /* delete the pending snapshot only after the content state was saved */ - if (state_snapshot_dir(&disk->fss, SNAPSHOT_PENDING, 0) == 0) { - if (fssnapshot_delete(&disk->fss, SNAPSHOT_PENDING) != 0) { - log_fatal(errno, "Failed to delete pending snapshot in '%s'. %s.\n", disk->fss.snapshot_dir, strerror(errno)); - error = -1; - continue; - } - } - - /* make the scan snapshot available for recovery */ - if (fssnapshot_rename(&disk->fss, SNAPSHOT_SCAN, SNAPSHOT_PENDING) != 0) { - log_fatal(errno, "Failed to rename scan snapshot in '%s'. %s.\n", disk->fss.snapshot_dir, strerror(errno)); + /* publish the scan snapshot without dropping the previous recovery source first */ + if (fssnapshot_replace(&disk->fss, SNAPSHOT_SCAN, SNAPSHOT_PENDING) != 0) { + log_fatal(errno, "Failed to replace pending snapshot in '%s'. %s.\n", disk->fss.snapshot_dir, strerror(errno)); error = -1; continue; } @@ -6549,18 +6540,9 @@ int state_snapshot_commit(struct snapraid_state* state) msg_progress("Committing disk %s stable snapshot...\n", disk->name); - /* delete a potential previous stable snapshot */ - if (state_snapshot_dir(&disk->fss, SNAPSHOT_STABLE, 0) == 0) { - if (fssnapshot_delete(&disk->fss, SNAPSHOT_STABLE) != 0) { - log_fatal(errno, "Failed to delete stable snapshot in '%s'. %s.\n", disk->fss.snapshot_dir, strerror(errno)); - error = -1; - continue; - } - } - - /* rename pending to stable */ - if (fssnapshot_rename(&disk->fss, SNAPSHOT_PENDING, SNAPSHOT_STABLE) != 0) { - log_fatal(errno, "Failed to rename snapshot in '%s'. %s.\n", disk->fss.snapshot_dir, strerror(errno)); + /* publish pending without dropping the previous stable deallocation source first */ + if (fssnapshot_replace(&disk->fss, SNAPSHOT_PENDING, SNAPSHOT_STABLE) != 0) { + log_fatal(errno, "Failed to replace stable snapshot in '%s'. %s.\n", disk->fss.snapshot_dir, strerror(errno)); error = -1; continue; } @@ -6582,7 +6564,7 @@ void state_snapshot_read(struct snapraid_state* state) if (res > 0) continue; if (res < 0) { - log_error(EUSER, "WARNING! Disk %s snapshot mount failed, falling back to live filesystem.\n", disk->name); + log_error(EUSER, "WARNING! Disk %s snapshot setup or recovery failed, falling back to live filesystem.\n", disk->name); log_error(EUSER, "Recovery capability may be reduced if files have changed since the last successful sync.\n"); continue; } @@ -6615,7 +6597,7 @@ void state_snapshot_write(struct snapraid_state* state, tommy_list* filterlist_d if (res > 0) continue; if (res < 0) { - log_error(EUSER, "WARNING! Disk %s snapshot mount failed, falling back to live filesystem.\n", disk->name); + log_error(EUSER, "WARNING! Disk %s snapshot setup or recovery failed, falling back to live filesystem.\n", disk->name); log_error(EUSER, "Recovery capability may be reduced if files have changed since the last successful sync.\n"); continue; } diff --git a/cmdline/unixapp.c b/cmdline/unixapp.c index 16e13a76..e4b86032 100644 --- a/cmdline/unixapp.c +++ b/cmdline/unixapp.c @@ -2377,10 +2377,69 @@ static int fssnapshot_zfs(const char* dir, uint32_t magic, struct fssnapshot_str } #endif +static void fssnapshot_retiring_name(const char* name, char* retiring_name, size_t retiring_name_size) +{ + pathcpy(retiring_name, retiring_name_size, name); + pathcat(retiring_name, retiring_name_size, SNAPSHOT_RETIRING_SUFFIX); +} + +static int fssnapshot_exists(struct fssnapshot_struct* fss, const char* name) +{ + struct stat st; + + if (fssnapshot_stat(fss, name, &st) == 0) + return 1; + + if (errno == ENOENT) + return 0; + + return -1; +} + +/** + * Recover an interrupted snapshot replacement. + * + * The .retiring snapshot is the previous destination and must remain available until the new destination is published. If the destination exists, publication completed and the retiring snapshot is deleted. Otherwise the previous destination is restored. + * + * Any failure is propagated to fssnapshot_mount(). Callers may then fall back to the live filesystem with an explicit warning when snapshots are used only as a read source, while snapshot-modifying operations fail. + */ +static int fssnapshot_recover_replace(struct fssnapshot_struct* fss, const char* name) +{ + char retiring_name[PATH_MAX]; + + fssnapshot_retiring_name(name, retiring_name, sizeof(retiring_name)); + + int retiring = fssnapshot_exists(fss, retiring_name); + if (retiring <= 0) + return retiring; + + int current = fssnapshot_exists(fss, name); + if (current < 0) + return -1; + + if (current != 0) { + /* the new destination was published, so the previous one can now be retired */ + if (fssnapshot_delete(fss, retiring_name) != 0) { + log_error(errno, "Failed to delete retiring snapshot '%s'. %s.\n", retiring_name, strerror(errno)); + return -1; + } + return 0; + } + + /* the new destination was not published, so restore the previous one */ + if (fssnapshot_rename(fss, retiring_name, name) != 0) { + log_error(errno, "Failed to restore retiring snapshot '%s' as '%s'. %s.\n", retiring_name, name, strerror(errno)); + return -1; + } + + return 0; +} + int fssnapshot_mount(const char* path, struct fssnapshot_struct* fss) { #if HAVE_LINUX_DEVICE struct statfs sfs; + int ret; if (statfs(path, &sfs) != 0) { /* LCOV_EXCL_START */ @@ -2393,15 +2452,27 @@ int fssnapshot_mount(const char* path, struct fssnapshot_struct* fss) if (statfs_type(&sfs) == BTRFS_SUPER_MAGIC) { /* btrfs reserved inode 256 for subvolume roots */ - return fssnapshot_inode(path, BTRFS_SUPER_MAGIC, 256, fss); + ret = fssnapshot_inode(path, BTRFS_SUPER_MAGIC, 256, fss); } else if (statfs_type(&sfs) == BCACHEFS_SUPER_MAGIC) { /* Bcachefs reserves inode 4096 for each subvolume's root directory */ - return fssnapshot_inode(path, BCACHEFS_SUPER_MAGIC, 4096, fss); + ret = fssnapshot_inode(path, BCACHEFS_SUPER_MAGIC, 4096, fss); } else if (statfs_type(&sfs) == ZFS_SUPER_MAGIC) { - return fssnapshot_zfs(path, ZFS_SUPER_MAGIC, fss); + ret = fssnapshot_zfs(path, ZFS_SUPER_MAGIC, fss); } else { return 1; } + + if (ret != 0) + return ret; + + /* never expose an interrupted replacement to the state layer */ + if (fssnapshot_recover_replace(fss, SNAPSHOT_PENDING) != 0) + return -1; + + if (fssnapshot_recover_replace(fss, SNAPSHOT_STABLE) != 0) + return -1; + + return 0; #else (void)path; (void)fss; @@ -2421,8 +2492,10 @@ static int fssnapshot_stat_fs(const struct fssnapshot_struct* fss, const char* n return -1; /* it must be a directory */ - if (!S_ISDIR(st->st_mode)) + if (!S_ISDIR(st->st_mode)) { + errno = ENOTDIR; return -1; + } return 0; } @@ -2458,8 +2531,10 @@ static int fssnapshot_stat_zfs(const struct fssnapshot_struct* fss, const char* return -1; /* it must be a directory */ - if (!S_ISDIR(st->st_mode)) + if (!S_ISDIR(st->st_mode)) { + errno = ENOTDIR; return -1; + } /** * When verifying if a ZFS snapshot has been successfully deleted, a direct @@ -2492,18 +2567,26 @@ static int fssnapshot_stat_zfs(const struct fssnapshot_struct* fss, const char* } while (1) { + errno = 0; struct dirent* entry = readdir(d); - if (!entry) - break; + if (!entry) { + int error = errno; + closedir(d); + + if (error != 0) { + errno = error; + return -1; + } + + errno = ENOENT; + return -1; + } if (strcmp(entry->d_name, native_name) == 0) { closedir(d); return 0; } } - - closedir(d); - return -1; } #endif @@ -2880,6 +2963,35 @@ int fssnapshot_rename(const struct fssnapshot_struct* fss, const char* old_name, #endif } +int fssnapshot_replace(struct fssnapshot_struct* fss, const char* old_name, const char* new_name) +{ + char retiring_name[PATH_MAX]; + + fssnapshot_retiring_name(new_name, retiring_name, sizeof(retiring_name)); + + /* resolve a replacement interrupted by an earlier invocation before starting another one */ + if (fssnapshot_recover_replace(fss, new_name) != 0) + return -1; + + int current = fssnapshot_exists(fss, new_name); + if (current < 0) + return -1; + + if (current == 0) + return fssnapshot_rename(fss, old_name, new_name); + + /* keep the previous destination recoverable until the source is published under the final name */ + if (fssnapshot_rename(fss, new_name, retiring_name) != 0) + return -1; + + /* on failure leave .retiring in place; fssnapshot_mount() will restore or complete the transition */ + if (fssnapshot_rename(fss, old_name, new_name) != 0) + return -1; + + /* the new destination is now published, so the previous destination is no longer required */ + return fssnapshot_delete(fss, retiring_name); +} + int fssnapshot_path(const struct fssnapshot_struct* fss, const char* name, char* path, size_t path_size) { pathcpy(path, path_size, fss->snapshot_dir);