From f00f9c2a2f99cd3b87cfcf00cfc4f05fd8e74ccb Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 19:10:18 +0200 Subject: [PATCH] Name a filesystem by its kernel id, never its device name (hq ADR 0325 fourth review) An ext4 or xfs library mounted by UUID can come back from a reboot as another device (/dev/sdb1 as /dev/sdc1); naming the filesystem by mountinfo's source read that as a move, stripped the warranted library's entries and asked for a new Approve. A filesystem is now named by statfs's f_fsid (from the UUID, or the ZFS dataset), its type and its mount point. Entries left on the filesystem a folder is on now are merged back or held, never walked away, and a TakenOn naming a mount point is not taken. --- internal/apply/keep.go | 42 ++++++++------ internal/apply/keeper_linux.go | 14 ++++- internal/apply/kept_test.go | 101 +++++++++++++++++++++++++++++---- internal/apply/walk_linux.go | 10 ++++ internal/apply/walk_other.go | 3 + internal/store/kept.go | 9 ++- 6 files changed, 147 insertions(+), 32 deletions(-) diff --git a/internal/apply/keep.go b/internal/apply/keep.go index 7b4231a..5f50759 100644 --- a/internal/apply/keep.go +++ b/internal/apply/keep.go @@ -256,7 +256,7 @@ func assessFolder(ctx context.Context, path string, rec store.KeptFolder, taken } // The controller's word that the folder was taken holds for the filesystem the warrant was carried out on: a // folder moved to another is not taken, and asks again (§8). - takenHere := taken && (takenOnID(rec.TakenOn) == here || (rec.TakenOn == "" && !moved)) + takenHere := taken && (rec.TakenOn == here || (rec.TakenOn == "" && !moved)) switch { case rec.Kept && !moved: return folderVerdict{kind: folderKeep, mount: pl.Mount, how: rec.How} @@ -279,24 +279,16 @@ func assessFolder(ctx context.Context, path string, rec store.KeptFolder, taken func movedRecord(f store.KeptFolder, mount string) store.KeptFolder { left := f.Left if f.How == store.KeptByWarrant && len(f.Users) > 0 && f.Mount != "" { - left = addLeft(left, f.Mount, f.Users) + left = addLeft(left, f.Mount, f.Placed, f.Users) } return store.KeptFolder{Path: f.Path, Mount: mount, Placed: PlacedID(mount), Want: f.Want, IDs: f.IDs, Act: f.Act, TakenOn: f.TakenOn, Left: left} } -// takenOnID is a record's TakenOn as a filesystem's name: one written before names held the mount point itself. -func takenOnID(takenOn string) string { - if strings.HasPrefix(takenOn, "/") { - return PlacedID(takenOn) - } - return takenOn -} - // addLeft adds users' entries left on mount, once per mount and uid. -func addLeft(left []store.KeptLeft, mount string, users []store.KeptUser) []store.KeptLeft { +func addLeft(left []store.KeptLeft, mount, placed string, users []store.KeptUser) []store.KeptLeft { for i := range left { - if left[i].Mount == mount { + if left[i].Mount == mount && left[i].Placed == placed { have := usersOf(left[i].Users) for _, u := range users { if _, had := have[u.UID]; !had { @@ -306,20 +298,37 @@ func addLeft(left []store.KeptLeft, mount string, users []store.KeptUser) []stor return left } } - return append(left, store.KeptLeft{Mount: mount, Users: append([]store.KeptUser(nil), users...)}) + return append(left, store.KeptLeft{Mount: mount, Placed: placed, Users: append([]store.KeptUser(nil), users...)}) } // PlacedID names the filesystem at mount for a wait, with no path in it: the controller asks again when the -// folder's filesystem is not the one a warrant was given for. +// folder's filesystem is not the one a warrant was given for. It is built from what stays the same across reboots +// for one filesystem — the kernel's filesystem id (statfs f_fsid: ext4 and xfs derive it from the filesystem's +// UUID, ZFS from the dataset), the filesystem's type and its mount point — and never from the device name +// mountinfo gives as its source, which a reboot may renumber (/dev/sdb1 becoming /dev/sdc1). func PlacedID(mount string) string { if mount == "" { return "" } - source, fstype := mountSourceOf(mount) - sum := sha256.Sum256([]byte(mount + "\x00" + source + "\x00" + fstype)) + _, fstype := mountSourceOf(mount) + fsid := fsidWithin(mount) + sum := sha256.Sum256([]byte(mount + "\x00" + fsid + "\x00" + fstype)) return "fs-" + hex.EncodeToString(sum[:6]) } +// fsidWithin is MountFsid within statBound: empty when it does not answer, so a mount that is down is named by its +// point and type alone, never held on. +func fsidWithin(mount string) string { + done := make(chan string, 1) + go func() { done <- MountFsid(mount) }() + select { + case id := <-done: + return id + case <-time.After(statBound): + return "" + } +} + // statBound is how long a read of a folder may take before it is said not to answer: a hard network mount whose // server is down blocks a stat for ever, and one access must never hold the apply. var statBound = 5 * time.Second @@ -534,7 +543,6 @@ func applyAccessFolder(ctx context.Context, r *declaration.Access, in inputs) (O // The controller says it was taken, and this engine has no record of where: here, now. f.TakenOn = PlacedID(v.mount) } - f.TakenOn = takenOnID(f.TakenOn) if first { f.How, f.At = v.how, time.Now().UTC() // A folder that held nothing has nothing under it to walk; one the controller says was taken diff --git a/internal/apply/keeper_linux.go b/internal/apply/keeper_linux.go index 67da4ae..5c6e028 100644 --- a/internal/apply/keeper_linux.go +++ b/internal/apply/keeper_linux.go @@ -87,14 +87,24 @@ func dropLeft(path string, f store.KeptFolder) []string { if _, err := lstatWithin(path); err == nil { holding = mountHolding(path, MountPoints()) } + holdingID := "" + if holding != "" { + holdingID = PlacedID(holding) + } var said []string var still []store.KeptLeft var mine []store.KeptUser for _, l := range f.Left { + same := l.Mount == f.Mount && (l.Placed == "" || l.Placed == f.Placed) switch { - case f.Kept && l.Mount == f.Mount: + case f.Kept && same: + // Left on the filesystem the folder is kept on now: the folder's own entries. mine = append(mine, l.Users...) - case l.Mount != holding: + case same: + // On the filesystem the folder is on now, not kept yet: never walked away; they are the folder's + // once it is kept there again. + still = append(still, l) + case l.Mount != holding || (l.Placed != "" && l.Placed != holdingID): still = append(still, l) default: drop := map[uint32]bool{} diff --git a/internal/apply/kept_test.go b/internal/apply/kept_test.go index 3f13c75..53239fa 100644 --- a/internal/apply/kept_test.go +++ b/internal/apply/kept_test.go @@ -1028,24 +1028,31 @@ func TestALeftoverWithNoAccessIdsIsSaidUnderAPathFreeName(t *testing.T) { } } -// Review (third) low: another filesystem mounted at the same point is not the one the warrant was for. -func TestAnotherFilesystemAtTheSameMountPointIsNotTaken(t *testing.T) { +// placedPool is a folder of a library on a pool the test says is mounted, with the pool's device name and filesystem +// id as the test sets them, taken under a warrant. +func placedPool(t *testing.T, source, fsid *string) (pool, lib, taken string) { + t.Helper() dir := aclFolder(t) - pool := filepath.Join(dir, "pool") - lib := filepath.Join(pool, "media") + pool = filepath.Join(dir, "pool") + lib = filepath.Join(pool, "media") if err := os.MkdirAll(filepath.Join(lib, "old"), 0o755); err != nil { t.Fatal(err) } keeping(t, []string{pool}, nil) - source := "tank/a" - was := MountSource + wasSource, wasFsid := MountSource, MountFsid MountSource = func(point string) (string, string) { if point == pool { - return source, "zfs" + return *source, "ext4" } return "", "" } - defer func() { MountSource = was }() + MountFsid = func(point string) string { + if point == pool { + return *fsid + } + return "" + } + t.Cleanup(func() { MountSource, MountFsid = wasSource, wasFsid }) notTaken := accessJSON("radarr.media", lib, "read-write", false, "mesh-radarr:61001:read-write:container") if _, _, err := applyAccesses(t, store.State{}, notTaken); err != nil { t.Fatal(err) @@ -1055,11 +1062,18 @@ func TestAnotherFilesystemAtTheSameMountPointIsNotTaken(t *testing.T) { t.Fatal(err) } <-done - taken := accessJSON("radarr.media", lib, "read-write", true, "mesh-radarr:61001:read-write:container") + taken = accessJSON("radarr.media", lib, "read-write", true, "mesh-radarr:61001:read-write:container") if report, _, _ := applyAccesses(t, store.State{}, taken); outcomeOf(report, "radarr.media").Action == "waiting" { t.Fatal("the folder taken waits on its own filesystem") } - source = "tank/b" // another pool imported at the same mount point + return pool, lib, taken +} + +// Review (third) low: another filesystem mounted at the same point is not the one the warrant was for. +func TestAnotherFilesystemAtTheSameMountPointIsNotTaken(t *testing.T) { + source, fsid := "/dev/sdb1", "0a0b0c0d01020304" + _, lib, taken := placedPool(t, &source, &fsid) + fsid = "1111222233334444" // another filesystem mounted at the same point report, _, err := applyAccesses(t, store.State{}, taken) if err != nil { t.Fatal(err) @@ -1072,6 +1086,28 @@ func TestAnotherFilesystemAtTheSameMountPointIsNotTaken(t *testing.T) { } } +// Review (fourth): the same filesystem whose device the kernel renamed across a reboot (/dev/sdb1 to /dev/sdc1, one +// filesystem id) is the same filesystem: still taken, still kept, and none of its entries walked away. +func TestADeviceRenamedIsTheSameFilesystem(t *testing.T) { + source, fsid := "/dev/sdb1", "0a0b0c0d01020304" + _, lib, taken := placedPool(t, &source, &fsid) + source = "/dev/sdc1" + report, _, err := applyAccesses(t, store.State{}, taken) + if err != nil { + t.Fatal(err) + } + if o := outcomeOf(report, "radarr.media"); o.Action == "waiting" { + t.Fatalf("a device renamed reads as another filesystem: %+v", o) + } + ReconcileKept(context.Background()) + if got := effectiveOn(t, filepath.Join(lib, "old"), radarr); got != 7 { + t.Fatalf("the library's entries were walked away: the identity may %s", permWords(got)) + } + if left := Leftovers(); len(left) != 0 { + t.Fatalf("a device renamed leaves entries said as left: %+v", left) + } +} + // Review L4: the controller's word that a folder was taken holds for the filesystem the warrant was carried out on; // moved to another, it is not taken, the wait names the new filesystem, and nothing is walked. func TestAMovedFolderIsNotTakenOnTheOldWarrant(t *testing.T) { @@ -1148,3 +1184,48 @@ func TestAHungReadIsSharedByEveryoneWhoAsks(t *testing.T) { t.Fatalf("%d reads of one path were started while the first hung", started) } } + +// Review (fourth): entries left on the filesystem the folder is on now are never walked away — merged back into +// the folder's users when it is kept there, held while it is not. +func TestEntriesLeftOnTheFilesystemTheFolderIsOnAreNeverWalkedAway(t *testing.T) { + source, fsid := "/dev/sdb1", "0a0b0c0d01020304" + pool, lib, _ := placedPool(t, &source, &fsid) + rec, _ := keptRecord(lib) + users := rec.Users + for _, kept := range []bool{false, true} { + _ = KeptFolders.Update(lib, func(f *store.KeptFolder, _ bool) bool { + f.Kept, f.Users = kept, nil + f.Left = []store.KeptLeft{{Mount: pool, Placed: f.Placed, Users: users}} + return true + }) + ReconcileKept(context.Background()) + if got := effectiveOn(t, filepath.Join(lib, "old"), radarr); got != 7 { + t.Fatalf("kept=%v: entries on the folder's own filesystem were walked away", kept) + } + r, _ := keptRecord(lib) + if kept && (len(r.Left) != 0 || len(r.Users) != 1) { + t.Errorf("kept: not merged back into the folder's users: %+v", r) + } + if !kept && len(r.Left) != 1 { + t.Errorf("not kept: the entries' record was dropped: %+v", r) + } + } +} + +// Review (fourth): a TakenOn naming a mount point (an unmerged build's record) is not taken on whatever is mounted +// there now. +func TestATakenOnOfTheOldFormIsNotTaken(t *testing.T) { + dir := aclFolder(t) + keeping(t, nil, nil) + lib := filepath.Join(dir, "media") + if err := os.MkdirAll(filepath.Join(lib, "old"), 0o755); err != nil { + t.Fatal(err) + } + _ = KeptFolders.Update(lib, func(f *store.KeptFolder, _ bool) bool { + f.Mount, f.TakenOn = "/", "/" + return true + }) + if v := AccessVerdict(context.Background(), lib, true, true); v.Kind != VerdictWaitKeep { + t.Fatalf("an old-form record is read as taken: %s", v.Kind) + } +} diff --git a/internal/apply/walk_linux.go b/internal/apply/walk_linux.go index 1f334dc..411162c 100644 --- a/internal/apply/walk_linux.go +++ b/internal/apply/walk_linux.go @@ -538,3 +538,13 @@ func createPath(path string, stats walkStats) (int, error) { unix.Close(fd) return made, nil } + +// MountFsid is the kernel's id of the filesystem at point (statfs f_fsid), in hex; empty when it cannot be read. +// A variable so a test can stand in for a filesystem it cannot mount. +var MountFsid = func(point string) string { + var st unix.Statfs_t + if err := unix.Statfs(point, &st); err != nil { + return "" + } + return fmt.Sprintf("%08x%08x", uint32(st.Fsid.Val[0]), uint32(st.Fsid.Val[1])) +} diff --git a/internal/apply/walk_other.go b/internal/apply/walk_other.go index c341303..1a29965 100644 --- a/internal/apply/walk_other.go +++ b/internal/apply/walk_other.go @@ -53,3 +53,6 @@ type Watcher struct{} func NewWatcher() (*Watcher, error) { return nil, errNoKeeping } func (*Watcher) Set(map[string][]store.KeptUser) {} func (*Watcher) Run(context.Context) {} + +// MountFsid reads no filesystem id where a folder is not kept. +var MountFsid = func(string) string { return "" } diff --git a/internal/store/kept.go b/internal/store/kept.go index 34049cc..3dfb6a4 100644 --- a/internal/store/kept.go +++ b/internal/store/kept.go @@ -66,7 +66,7 @@ type KeptFolder struct { Placed string `json:"placed,omitempty"` // TakenOn names, as Placed does, the filesystem the operator's warrant was carried out on (ADR 0325 §8): the // controller's word that the folder was taken holds for that filesystem alone, and a folder moved to another - // asks again. A record written before held the mount point here, and is read as that mount's name. + // asks again. One that names no filesystem this way (a mount point, from an unmerged build) is not taken. TakenOn string `json:"taken_on,omitempty"` // Left is the entries the mesh added on a filesystem no longer at the folder (a disk mounted over it, or // one gone): taken out by a drop walk once that filesystem holds the folder again, said until then. @@ -83,8 +83,11 @@ type KeptFolder struct { // KeptLeft is the mesh's entries left on one filesystem: its mount point and whose entries they are. type KeptLeft struct { - Mount string `json:"mount"` - Users []KeptUser `json:"users"` + Mount string `json:"mount"` + // Placed names that filesystem as KeptFolder.Placed does: entries left on the filesystem the folder is on + // now are the folder's own, never walked away. + Placed string `json:"placed,omitempty"` + Users []KeptUser `json:"users"` } // KeptAct is one walk under a kept folder, as the engine records it. Counts only: never a path.