Storage/btrfs ioctl - #1
Open
Yasirunet wants to merge 11 commits into
Open
Conversation
The btrfs CLI's human-readable output is not a stable interface; parsing "UUID:", "parent_uuid " and filesystem-usage fields broke across btrfs-progs versions. Migrate all subvolume call sites to the pure-Go github.com/dennwc/btrfs library (kernel ioctls underneath): - CreateRoot/Delete -> CreateSubVolume/DeleteSubVolume - Fork -> SnapshotSubVolume, rollback on PrepareClone failure preserved - subvolUUID/Children -> one FS handle per Children call: SubvolumeByPath for the parent UUID, ListSubvolumes(nil) filtered on ParentUUID (single open instead of two) - Usage -> FS.Usage(), TotalUsed/Total mapped to UsedBytes/TotalBytes - Init pool image -> os.Create + os.Truncate(PoolSizeGB<<30) instead of the truncate(1) exec - Available probe now checks mkfs.btrfs: it is the only btrfs-progs binary still required mkfs.btrfs, mount/umount/losetup and modprobe stay as shell-outs; the kernel exposes no ioctl for formatting or mounting. Verified on this machine: go build/vet/fmt clean, unit tests pass, and the conformance suite passes as root via sudo -E with PATH preserved (all 7 subtests, including deleting_a_branch_with_children_is_refused, which exercises the rewritten ioctl-based Children). Usage output was cross-checked against `btrfs filesystem usage -b`: identical byte counts.
Switching the btrfs availability probe from the `btrfs` binary to mkfs.btrfs made it PATH-dependent: mkfs.btrfs lives in /usr/sbin, which Debian/Ubuntu keep off a normal non-root PATH while /usr/bin/btrfs is always reachable. doctor, an explicitly no-root command, false-negatived with "mkfs.btrfs is not installed" on machines where it was in fact installed. Fall back to statting /usr/sbin/mkfs.btrfs and /sbin/mkfs.btrfs when LookPath misses. Shared helper used by both Btrfs.Available and detectBtrfs so the two probes cannot drift. Verified: sudo -u coder env PATH=/usr/local/bin:/usr/bin:/bin ./bin/forklift doctor now reports "btrfs available".
…orks sudo resets PATH to secure_path, which excludes /usr/local/go/bin, so "sudo make test-integration" died in the build prerequisite with "go: not found" before the test recipe (which already re-adds PATH) could run. Fall back to /usr/local/go/bin/go when `go` is not on PATH. Verified: "sudo make test-integration" now passes from a clean shell with no manual PATH export; all 7 conformance subtests pass.
Both changes fix the same root cause: sudo replaces PATH with secure_path, so anything assuming PATH survives privilege elevation breaks. test-integration drops the build prerequisite (where "go: not found" died under plain sudo) and the absolute-go fallback. The recipe now elevates itself with sudo -E env PATH=..., and runs go directly when already root (CI containers). Invoking it as "sudo make" fails fast with a message saying to drop the sudo instead of the cryptic "go: not found". install previously ran `go install`, landing the binary in ~/go/bin, which is not on secure_path, so `sudo forklift` failed with "command not found". It now builds as the invoking user and copies to $(PREFIX)/bin (/usr/local/bin), elevating only for the copy. Observed on this machine: - `make test-integration` as a normal user: all 7 conformance subtests PASS, no manual PATH export. - Uncached run (`go test -count=1 -run Conformance` as root): ok 0.620s. - `sudo make test-integration`: exits with the actionable message, not "go: not found". - `make install`: installs /usr/local/bin/forklift; plain `sudo forklift doctor` then reports btrfs available.
`sudo forklift` fails with "command not found" whenever the binary is off sudo's secure_path (e.g. go install puts it in ~/go/bin), so the old "Re-run with sudo" hint suggested a command that could not work and made it look like forklift was not installed. requireRoot now resolves the binary via os.Executable and prints a copy-pasteable command including the current flags, falling back to the bare name if resolution fails. Observed on this machine: running ~/go/bin/forklift create demo unprivileged prints "Re-run: sudo /home/coder/go/bin/forklift create demo"; pasting that line literally succeeded (branch created in 8.15s, then deleted).
mkfsBtrfsPresent fixed the PATH hazard once, by hand, for one binary. The same hazard applies to every shell-out: losetup and dmsetup live in /usr/sbin, which is off a normal non-root PATH and off sudo's secure_path alike, so any call site that trusts exec.LookPath breaks depending on who runs it. Generalise into internal/tool.Resolve(name): LookPath first, then /usr/local/sbin, /usr/sbin and /sbin (executable check included). Delete the one-off helper and route every exec call site through it: cp, dmsetup, docker, losetup, mkfs.btrfs, modprobe, mount, truncate, umount. Docker resolves once per process and reuses the path. Observed on this machine: gofmt clean, vet clean, go test -count=1 ./... passes, and unprivileged `doctor` with PATH=/usr/local/bin:/usr/ bin:/bin (no /usr/sbin) still reports btrfs available — mkfs.btrfs is resolved via the sbin fallback.
Both Btrfs.Available and detectBtrfs discarded modprobe's error and declared "btrfs not supported by this kernel" whenever the module was not loaded — an assertion we had not established. On hosts without kmod or /lib/modules (minimal containers) the module may exist but simply be unloaded. btrfsKernelSupport now distinguishes the four cases: - present in /proc/filesystems: supported (built in or already loaded) - absent, modprobe missing: "not loaded and modprobe is unavailable to load it" - absent, /lib/modules/$(uname -r) missing (read from /proc/sys/kernel/osrelease): "this kernel has no loadable modules" - absent, modprobe present but failing: includes modprobe's stderr A missing modprobe stays tolerated, not fatal. Observed on this machine: gofmt/vet clean, go test -count=1 passes, doctor reports btrfs available as both root and unprivileged user with /usr/sbin off PATH (the present-in-/proc/filesystems path). The three absent paths are not reproducible here — btrfs is loaded and in use by the pool — so their messages are exercised only by inspection.
…"unavailable"
Detection conflated three states: tool absent, probe ran and failed,
and cannot tell without root. All rendered as "unavailable", so doctor
lied: as a normal user it claimed dm-thin was unavailable when the
truth was that dmsetup needs root to query (/dev/mapper/control:
Permission denied), and that loop devices were unavailable when
attaching one simply requires root.
Mechanism.Available becomes Mechanism.State (unknown / unavailable /
available). StateUnknown is reserved for probes that could not run —
dm-thin when dmsetup is missing or refuses to answer unprivileged,
loop devices when not root. Probes that ran to completion report a
definite answer either way. Best() never selects an unknown mechanism.
doctor renders unknown distinctly and tells the user to re-run with
sudo.
Observed on this machine:
- `sudo -u coder ./bin/forklift doctor`: dm-thin and loop devices read
"unknown — re-run with sudo to determine"; nbd and reflink still give
definite answers because their probes need no privilege.
- `sudo ./bin/forklift doctor`: dm-thin reports a definite negative
("absent; dm targets present: multipath, striped, linear, error"),
loop devices report available.
- gofmt/vet clean, go test -count=1 ./... passes.
…not see
Under sudo we run docker as root. If the invoking user runs rootless
Docker or a non-default context, root's docker is a different daemon:
branches get created where the user cannot see them, or fail
confusingly mid-create.
- InspectDaemon queries context, rootless flag and the comparable
daemon ID (`docker info -f {{.ID}}`).
- EnsureSameDaemonAsUser, called from buildManager, runs when SUDO_USER
is set: it compares the daemon ID root sees with the one that user
sees (via sudo -u with DOCKER_HOST, DOCKER_CONTEXT and
XDG_RUNTIME_DIR forwarded — sudo -u does not inherit them) and fails,
naming both daemons, on a mismatch. When the comparison is impossible
(no sudo binary, or the user's query fails) it stays silent rather
than block on something unverifiable.
- doctor now shows the daemon: context, rootless yes/no, ID.
Observed on this machine: doctor reports "context default, rootless no,
id a64d6a4e-46ea-4099-80a1-5786e3a1f575" identically as root and as an
unprivileged user; `sudo forklift list` passes because the IDs match.
The mismatch path is not reproducible here (single daemon), so its
error message is exercised only by inspection.
github.com/dennwc/btrfs is pinned to an untagged pseudo-version (v0.0.0-20260222081608-edfb8b9e4f55) of a single-maintainer repo with no tagged releases, and since the ioctl migration it is load-bearing for every storage operation. If upstream is deleted, force-pushed or retagged, builds break with nothing to fall back to. Vendor the whole tree so the source ships in-repo: dennwc/btrfs, dennwc/ioctl, inconshreveable/mousetrap, lib/pq, spf13/cobra, spf13/pflag. go.mod and go.sum are unchanged; no import paths changed. Observed on this machine: - go build -mod=vendor ./... succeeds. - go test -mod=vendor -count=1 ./... passes. - GOFLAGS=-mod=vendor GOPROXY=off go build ./... succeeds — with the proxy off nothing can be fetched, so this only passes because the source is genuinely in-repo. - make test-integration (run without sudo; the recipe self-elevates): all 7 conformance subtests PASS. - gofmt and vet clean for cmd/ and internal/ (two pre-existing files in vendored pflag are not gofmt-clean upstream; vendor/ is left exactly as the toolchain wrote it).
The last nine commits changed observable behaviour that the README still contradicted — anyone comparing their output to it would think something was broken. README: - doctor example now shows the tri-state (available / unavailable / unknown) and the docker row, verified byte-identical to real unprivileged output. Explains why "unknown" is distinct: reporting a probe we could not run as "unavailable" tells people a mechanism is broken on their machine when it is not. - Testing section no longer says test-integration "needs root". It elevates itself and must NOT be prefixed with sudo, which strips the Go toolchain from PATH — the exact failure that was once misdiagnosed as "this machine cannot run the suite". - Usage documents `make install` targeting /usr/local/bin rather than GOBIN, and why: go install lands the binary in ~/go/bin, off sudo's secure_path, so `sudo forklift` would fail with "command not found" while every pool command needs root. CLAUDE.md added (was untracked in the main checkout), covering the architecture plus the invariants that are expensive to rediscover: snapshot atomicity across PGDATA including pg_wal, the clone fixups, ValidateName at the provider boundary, registry outside the branchable data, and the three new ones from this branch — resolve binaries through internal/tool, detection is tri-state, never use a Docker daemon the invoking user cannot see. Verified: `make install PREFIX=/tmp/pfxtest` installs there; the README doctor block diffs clean against actual `forklift doctor` output.
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.
No description provided.