btrfs tools: clearer aside names, warnings and errors - #145
Draft
Yury-MonZon wants to merge 7 commits into
Draft
Conversation
This was referenced Jul 29, 2026
Yury-MonZon
force-pushed
the
btrfs-tools-messages
branch
from
July 29, 2026 17:32
baa2c55 to
eeb79fe
Compare
Yury-MonZon
force-pushed
the
btrfs-tools-messages
branch
from
July 31, 2026 08:53
eeb79fe to
f0fefcb
Compare
Yury-MonZon
force-pushed
the
btrfs-tools-messages
branch
2 times, most recently
from
July 31, 2026 10:47
2960440 to
d5d4486
Compare
Yury-MonZon
force-pushed
the
btrfs-tools-messages
branch
from
July 31, 2026 12:59
d5d4486 to
bacbaa3
Compare
validate_profile_name rejects '.', so the aside name create-profile generated for a replaced profile was one its own validator would refuse, and it could not be passed back to rename-profile or create-profile. Use _old_<ts> instead. The list-profiles classifier anchors on the timestamp so a user-chosen name merely containing "_old_" is still a profile, and still recognizes the legacy .old_ spelling.
Replacing the profile you are booted from is deliberate, since it is how a profile is reset to its _stock, and unlike delete-profile and rename-profile there is no reason to refuse it. What was missing was the consequence: the running system keeps using the subvolume moved aside as _old_<ts>, so everything written after the replace lands in that copy rather than in the new profile, and only a reboot switches over. The confirmation now states this before the fact, and the summary ends with a REBOOT REQUIRED warning on stderr, with the delete-profile hint deferred until after the reboot. Detection compares the target's subvolume id with the booted one, so it fires on an explicit name as well as on 'current'.
…tools These two answered an unknown option with "unexpected argument", where the other ten say "unknown option", and they could not tell one from the other: a typo'd flag and a stray positional argument produced the same message. They now report an unknown option in the same words as everywhere else, and say plainly that they take no arguments when given one.
Two failures reported someone else's error instead of the cause. Dedup keeps duperemove's hash database in @var-cache, so it survives between runs and is not itself deduped. That directory only exists on filesystems our image built, and -d/--device exists precisely for the ones it did not, where duperemove died with "Error opening db ... unable to open database file" and rc=14. It now falls back to a per-run temp hashfile, registered for cleanup, and says where the database went. Balance on a nearly full filesystem failed with btrfs's own "There may be more info in syslog - try dmesg | tail", while the kernel logged "1 enospc errors during balance". It now checks for ENOSPC in the output and in the recent kernel log, and explains that a balance needs free space to move chunks into. Both reproduced and verified on the device against loop filesystems built for the purpose.
[ "$do_scrub" = 1 ] && btrfs scrub ... ends the script under set -e when the scrub reports errors, so 'all' printed the scrub summary and stopped there, with no dedup, no balance, and nothing said about either. Stopping is right, since both rewrite extents and that is not what a filesystem with unfixed errors needs, but it has to be stated. Measured on a sandbox with one corrupted extent (scrub rc=3, csum=1): before, exit 3 and no mention of the two verbs that did not run; after, a message naming both and pointing at 'btrfs-maintenance fix'. The exit status becomes 1, die's, rather than btrfs's 3. A healthy filesystem still runs check and all to completion, dedup and balance included.
replacing_booted compared the target's subvolume id with the booted profile's, and ids are per-filesystem: 265 on a target reached through -d and 265 here are unrelated. That made the tool announce REBOOT REQUIRED for a profile the running system never used, and the confirmation claim that the running system would keep using the copy moved aside. Measured on the device: the sandbox's @desktop and the booted @desktop both had id 265 and the warning fired. Gated on top_is_booted_fs, the predicate five other tools already use for exactly this question. Replacing the profile you did boot from still warns.
duperemove exits 22 with "No dedupe candidates found" when there is nothing to do, and that became "Error: Duperemove failed (rc=22)". It is the normal answer on a filesystem that was just deduped or has little in common, and it made a successful pass look broken. Measured both ways: with duplicate data above duperemove's block size it exits 0 and that path is unchanged; with nothing to dedup the tool now says so plainly. A genuine failure still dies, as an unwritable hashfile shows (rc=14). Output stays live during the run, since a real pass takes minutes, so duperemove records its own status for the check that follows the pipeline.
Yury-MonZon
force-pushed
the
btrfs-tools-messages
branch
from
July 31, 2026 15:15
bacbaa3 to
9fc2e49
Compare
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
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.
Four changes to what the tools tell you. Nothing about what they do to the filesystem changes.
_old_<ts>instead of.old_<ts>, becausevalidate_profile_namerejectsa leading dot, so the old name could not be passed back to the very tools meant to consume it.
list-profilesclassifies them with a pattern anchored on the timestamp shape, so a user-chosen namethat merely contains
_old_stays a profile.create-profilewarns that replacing the booted profile needs a reboot. Doing so is deliberate,since it is how a profile is reset to its
_stock, and unlikedelete-profileandrename-profilethere is no reason to refuse it. What was missing was the consequence: the running system keeps using
the subvolume moved aside as
_old_<ts>, so everything written afterwards lands in that copy. With-ythis happened with no prompt at all. Verified end to end on hardware, including the reboot.list-profilesandlist-snapshotssayunknown optionlike the other ten tools rather thanunexpected argument, and now tell a typo'd flag from a stray positional.btrfs-maintenanceexplains its own failures instead of relaying someone else's. A missing@var-cache, which is exactly the-dcase, no longer surfaces duperemove's opaqueError opening db ... rc=14but falls back to a per-run temp hashfile; a balance on a full filesystemsays it needs free space rather than leaving you with
try dmesg | tail.Split out of the closed #141. Only the combined final state was run on hardware; each
commit parses and defines the helpers it uses, and the stack tip is byte-identical to what
was tested.