Conversation
Adds RollbackMethod enum (SET_DEFAULT, SUBVOL_RENAME) and two detection functions to AppUtil: - detect_rollback_method_from_options(): pure function over already-parsed mount options, testable without /proc/mounts access - detect_rollback_method(): reads /proc/mounts via getMtabData() and delegates to the above Returns SUBVOL_RENAME when a "subvol=<name>" mount option is present and the name is not "/", SET_DEFAULT otherwise. This distinguishes systems using named subvolumes (subvol=@root) from those relying on the btrfs default subvolume id. Also adds KEY_ROLLBACK_METHOD config key and ROLLBACK_METHOD="auto" to the default config template, and a Boost unit test covering all detection cases including the subvolid= vs subvol= prefix disambiguation. config.h is included in AppUtil.cc as it now contains the first conditional compilation block in that translation unit.
Reads ROLLBACK_METHOD from the snapper config and resolves the effective rollback method before the ambit switch: - "auto": detect from /proc/mounts via detect_rollback_method() - "set-default": force the existing btrfs default subvolume ioctl - "subvol-rename": use named subvolume swap; errors if root is not mounted with a named subvolume - unknown value: error and exit Adds get_subvol_name() as a thin wrapper around detect_rollback_method() for the explicit "subvol-rename" path, making intent clear without discarding the returned RollbackMethod. Both CLASSIC and TRANSACTIONAL ambit paths switch on the resolved method. The subvol-rename case exits with "not yet implemented" until the rename logic is added in the next commit.
Adds Btrfs::rollbackSubvolRename() which performs rollback on systems using named btrfs subvolumes (subvol=@root in fstab) by atomically swapping subvolumes via renameat2(RENAME_EXCHANGE) rather than changing the btrfs default subvolume id. The sequence: 1. Reject nested subvolume paths (e.g. root/@root) 2. Mount the btrfs top-level (subvolid=5) via TmpMount 3. Rename any stale .incoming from a previous interrupted rollback 4. Create a rw snapshot of the rollback target as <subvol>.incoming 5. Atomically swap <subvol> and <subvol>.incoming 6. Rename old root to <subvol>.rollback.<N> (falls back to <subvol>.rollback.svid.<id> on collision) The old root is never deleted. Strip leading slash from subvol= in /proc/mounts to match kernel vs fstab differences (e.g. Debian). Both CLASSIC and TRANSACTIONAL ambit paths dispatch to rollbackSubvolRename() when rollback_method is SUBVOL_RENAME.
Add ROLLBACK_METHOD to snapper-configs(5) with descriptions of all three values (auto, set-default, subvol-rename). Update the rollback section of snapper(8) to mention auto-detection from /proc/mounts. Add doc/rollback.txt explaining the design rationale, the two rollback methods, and why subvol-rename is needed on systems using named subvolumes (subvol=@root).
Use flat subvolume structure instead of recreating snapper's internal .snapshots/N/snapshot hierarchy. Avoids nested subvol cleanup failures and tests the rename logic directly. Add detect_rollback_method test against real /proc/mounts for @root, root, @rootfs and @ naming conventions, verifying kernel leading-slash normalization is handled correctly.
RollbackMethod, SubvolumeMode, Ambit enums and the detection functions are only used by client code. Move them from snapper/AppUtil to client/snapper/rollback-method to keep the library surface minimal.
Revert the config.h and Exception.h includes added to AppUtil.cc and the sys/syscall.h include in FileUtils.cc (leftover from the removed renameat2 syscall fallback); none of them are used.
Drop the separate RollbackMethod enum and the detect_ambit helper. Add a
SUBVOL_RENAME value to GlobalOptions::Ambit (kept in GlobalOptions.h) and
determine the effective ambit from two small helpers:
use_subvol_rename(rollback_method, subvol_name) - the mechanism, from
ROLLBACK_METHOD and the root mount;
classic_or_transactional(ambit, mode) - the set-default semantic, from
--ambit or the read-only/-write state of the default snapshot.
SUBVOL_RENAME is selected only via ROLLBACK_METHOD, not --ambit, since a
transactional system is always mounted by default subvolume id and never by
name. Both helpers are pure and unit-tested, including the guards for an
unknown ROLLBACK_METHOD and for subvol-rename requested without a top-level
named subvolume.
An explicit --ambit always wins; with auto a top-level named root subvolume selects subvol-rename, otherwise the ambit is derived from the read-only/-write state of the default snapshot. The whole decision lives in one pure function, determine_ambit, so the precedence is unit-tested, including that an explicit --ambit is never overridden by detection. The ROLLBACK_METHOD config key is dropped; the ambit is the single parameter controlling how the rollback is performed.
Warn when an explicit --ambit classic/transactional sets the default
subvolume id while root is mounted by subvol= name - the rollback would
silently have no effect on the next boot (set_default_ineffective, unit
tested). Promote rollbackSubvolRename to Filesystem like setDefault,
removing the dynamic_cast in cmd-rollback.cc. Rename rollback-method.{h,cc}
to ambit.{h,cc} since the rollback-method concept is gone. Compute the
subvolume mode only with --ambit auto. Add subvol-rename to the zsh
completion and fix the nested-subvolume wording in doc/rollback.txt.
A btrfs snapshot does not include nested subvolumes: on layouts where the .snapshots subvolume lives inside the root subvolume (e.g. the default snapper setup on Fedora and Debian) the subvol-rename rollback left only an empty stub directory in the new root, orphaning all snapshots in the renamed-away old root and breaking snapper after the reboot. Move the .snapshots subvolume into the new root after the swap; the running mount stays valid since the kernel tracks it by subvolume id. Adds a cross-directory SDir::rename and unit test, and a real-btrfs test for the stub-replacement semantics. Found by the VM testsuite's second rollback cycle.
Address review feedback: a bool predicate reads better with an "is_" prefix. Pure rename of the declaration, definition, call site and unit test; no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PrmT9CcddjvedN9LoLjSqp
Address review feedback: the cross-directory rename overload hardcoded flags 0, so a cross-directory RENAME_EXCHANGE could not be expressed. Add "int flags = 0" and pass it through to renameat2(), mirroring the same-directory overload. Existing 3-argument callers are unaffected. Covered by a new cross-directory exchange case in rename-exchange.test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PrmT9CcddjvedN9LoLjSqp
Address review feedback: Btrfs declares rollbackSubvolRename
unconditionally (an override), but it was only defined inside the
ENABLE_ROLLBACK branch. Building with --disable-rollback therefore
failed to link:
libsnapper.so: undefined reference to
snapper::Btrfs::rollbackSubvolRename(...)
Add a stub in the #else branch that delegates to Filesystem (which
throws UnsupportedException), mirroring how setDefault/isDefault are
handled there.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PrmT9CcddjvedN9LoLjSqp
The default CI job only builds the full feature set, so the --disable-rollback link failure fixed in the previous commit would not have been caught. Add an isolated Fedora job that bootstraps and compiles with rollback disabled, guarding the feature-reduced build. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PrmT9CcddjvedN9LoLjSqp
On a named-subvolume layout (Fedora/Debian) the .snapshots subvolume is nested inside the root subvolume and reached through the root mount, not by a separate mount. The subvol-rename rollback moves .snapshots into the new root so the snapshot history is not orphaned, but that also unhooks it from the still-mounted old root (the running system, held by subvolume id). /.snapshots then no longer resolved and snapper stopped working until the next reboot: # snapper list IO error (open failed path://.snapshots errno:2 (No such file ...)). After moving .snapshots, mount it back onto the running root by its (unchanged) subvolume id so snapper keeps working until the reboot swaps in the new root, where /.snapshots is a nested subvolume again. The temporary mount does not survive the reboot. Verified end-to-end on a Fedora 43 VM: after rollback, findmnt shows /.snapshots mounted (subvolid unchanged) and `snapper list` succeeds before reboot; the reboot still applies the rollback. A pre-reboot "snapper still works" assertion is added to the VM rollback test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PrmT9CcddjvedN9LoLjSqp
A subvol-rename rollback preserves the previous root as a top-level <subvol>.rollback.<N> subvolume for recovery. These are outside the .snapshots/<N> framework, so no cleanup algorithm ever removes them and they accumulate without bound (reported in review). Add an optional ROLLBACK_BACKUP_LIMIT config key. Consistent with snapper never deleting anything unless configured, it defaults to empty = keep all backups (no behaviour change). When set to a positive number, a rollback deletes the oldest backups beyond that limit; ordering is by btrfs subvolume id, which increases with every rollback, so the newest backup (the one just created) is never deleted. Deletion is recursive because a preserved root contains nested subvolumes (e.g. var/lib/portables). - BtrfsUtils::delete_subvolume gains a recursive flag (libbtrfsutil BTRFS_UTIL_DELETE_SUBVOLUME_RECURSIVE). - prune_rollback_backups() is factored out and unit-tested directly in testsuite-real/rollback-subvol-rename (creates backups with a nested subvolume, verifies only the newest are kept and keep==0 keeps all). - Documented in snapper-configs and the default config template. Verified in a Fedora 43 VM: the testsuite-real suite (incl. the new prune test) passes on a loopback btrfs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PrmT9CcddjvedN9LoLjSqp
The previous commit read ROLLBACK_BACKUP_LIMIT in Btrfs::evalConfigInfo into a member, but the retention never took effect: the rollback client builds its filesystem handler via get_filesystem() -> Filesystem::create(fstype, subvolume, root_prefix), the overload that does NOT call evalConfigInfo. The member (like qgroup on this path) stayed at its default, so prune_rollback_backups() always saw a limit of 0 and kept everything. Read the value in cmd-rollback.cc from the ProxyConfig and pass it into rollbackSubvolRename() as an explicit parameter; drop the dead member and its evalConfigInfo read. A VM cycle with the limit set now prunes as configured (verified: 3 backups -> 2, and the rollback still boots). Found because the testsuite-real unit test exercised prune directly but not the config-to-rollback wiring; the VM integration test caught it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PrmT9CcddjvedN9LoLjSqp
`snapper rollback` with no argument creates a read-only snapshot of the "default subvolume" as the restore point. createSnapshotOfDefault() resolved that via the btrfs *default* subvolume id and mounted it with `subvol=<name>`. On subvol-rename systems (e.g. Fedora, Debian, Ubuntu) the btrfs default subvolume is the top-level (id 5), whose path is empty, so the mount option became `subvol=` and the operation aborted early with a cryptic `mount failed errno:22 (Invalid argument)` before any state was changed. The booted root on these systems is a named subvolume reached via `subvol=` in fstab, not by being the btrfs default. Fall back to the currently mounted subvolume when the default resolves to the top-level (empty path); this is what the system actually boots and the correct restore point. classic and set-default roots are unaffected (their default subvolume has a real path). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PrmT9CcddjvedN9LoLjSqp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This continues #1133 (closed after review). I opted for a fresh PR rather than reopening the old one; it's the same branch, with all review feedback from #1133 addressed — see Changes since the #1133 review below.
On systems where fstab mounts the root filesystem with an explicit
subvol=option (e.g.subvol=root,subvol=@rootfs),snapper rollbackcurrently has no effect on the next boot. TheBTRFS_IOC_DEFAULT_SUBVOLioctl changes the default subvolume ID, but when the kernel mounts by name the default ID is ignored.This affects systems using named-subvolume layouts (Fedora, Ubuntu, Debian, many manual btrfs setups) and is the root cause behind #365 and #1011 (and is discussed in related btrfs-rollback reports #722 and #159).
Changes
The ambit is the single parameter controlling how the rollback is performed.
--ambitaccepts a fourth value:auto(default) — everything is detected: a top-level namedsubvol=root mount selectssubvol-rename; otherwise the read-only/-write state of the default snapshot selectstransactional/classic, exactly as before. No configuration needed on most systems.classic/transactional— existing behavior, unchanged (set-default).subvol-rename— atomically swaps the named root subvolume usingrenameat2(RENAME_EXCHANGE)(Linux 3.17+).An explicit
--ambitalways wins over the detection. The whole decision lives in one pure function (client/snapper/ambit.cc), so the precedence is unit-tested. When an explicitclassic/transactionalsets the default subvolume ID while root is mounted by a top-levelsubvol=name — a rollback that cannot take effect on the next boot — snapper prints a warning.rollbackSubvolRenameis a virtual onFilesystemlikesetDefault, implemented for btrfs.The
subvol-renamesequence:<subvol>.incomingrenameat2(RENAME_EXCHANGE)atomically swaps<subvol>.incoming↔<subvol>— no window where<subvol>does not exist.snapshotssubvolume is moved into the new root (a btrfs snapshot only contains an empty stub directory where a nested subvolume is; without this the default Fedora/Debian layout would lose its snapshot history to the old root)<subvol>.rollback.<N>for recoveryOnly a top-level named subvolume selects
subvol-rename: the kernel shows the resolvedsubvol=path in/proc/mountseven for mounts by default subvolume ID (e.g. the nestedsubvol=/@/.snapshots/N/snapshoton Tumbleweed), so a nested name is no evidence of a by-name mount and such systems keep using set-default.Safety properties
<subvol>.rollback.<N>. If that name already exists, a subvolume-ID-based fallback name<subvol>.rollback.svid.<id>is used — guaranteed unique across the btrfs filesystem..incomingfrom a prior interrupted rollback is renamed (not deleted) before starting. If a previous rollback completed the swap but failed to rename.incoming, that subvolume is the old running root, still mounted by the kernel via subvolume ID. Deleting it risks corruption during the unmount sequence. It is renamed to<subvol>.rollback.svid.<id>instead.rename_exchangefailure deletes only the newly-created.incomingcopy (safe — the swap never occurred) and throws a clear error.Changes since the #1133 review
Addressing @aschnell's feedback:
/.snapshotsunavailable before reboot (Fedora 44). Where.snapshotsis nested inside the root subvolume, the rollback moves it into the new root; that unhooked it from the still-mounted old root (reached by subvolume id, not the exchanged name), so/.snapshotsstopped resolving and snapper stopped working until the reboot. The rollback now re-mounts the.snapshotssubvolume back onto the running root by its unchanged subvolume id, so snapper keeps working immediately. The mount is transient; after reboot the new root provides.snapshotsas a nested subvolume again — no fstab entry required.<subvol>.incoming.<N>never deleted..incomingis only the transient name for the read-write copy during the swap; after the exchange the old root is renamed to<subvol>.rollback.<N>, so nothing named.incomingis left behind. To stop the.rollback.<N>backups from accumulating indefinitely, a new optionalROLLBACK_BACKUP_LIMITconfig key prunes them: empty (the default) keeps all;Nkeeps the N newest and deletes the oldest on each rollback. The backup just created is never pruned.<subvol>.incoming.<N>/var/lib/portables. Expected btrfs behaviour rather than a misconfiguration: a btrfs snapshot does not recurse into nested subvolumes, so the read-write copy gets an empty stub directory where each nested subvolume (systemd'svar/lib/portables, etc.) was. The one nested subvolume that matters,.snapshots, is explicitly moved into the new root; the remaining stubs are empty and are removed when the backup subvolume is pruned/deleted.snapper rollbackon subvol-rename roots (found while validating across distros). It snapshotted the btrfs default subvolume, which on these systems is the top-level (id 5, empty path) → an emptysubvol=mount →EINVAL. It now falls back to the currently-mounted subvolume, the correct restore point.Testing
testsuite/ambit-test.cc);SDir::exchange/RENAME_EXCHANGEand the cross-directorySDir::rename(testsuite/rename-exchange.cc).testsuite-real/rollback-subvol-rename.cc): happy path,.rollback.Ncollision → subvolid fallback, nested-.snapshotsmigration,ROLLBACK_BACKUP_LIMITpruning.--disable-rollbackreduced-build job so the!ENABLE_ROLLBACKpath keeps compiling..snapshotsusable before reboot, the--ambit classicwarning on a named root, the.rollback.Ncollision fallback,ROLLBACK_BACKUP_LIMITpruning across three real rollback+reboot cycles, btrfs quota, SELinux-enforcing, the transactional ambit, and the already-default rejection. Branch: https://github.com/mxsb/snapper/tree/mxsb/testsuite-vm/testsuite-vmI kept the VM test harness out of this PR to avoid scope creep, but it's ready to contribute — happy to open it as a separate PR if you'd find it useful in the repo.
Documentation
snapper(8)—--ambitvalues and the rollback sectiondoc/rollback.txt— design rationale and mechanism descriptionsROLLBACK_BACKUP_LIMITdocumented in the config templatesFixes #365. Related: #722, #159, #1011. Continues #1133.