diff --git a/modules/zfs/cmd/zfs-tools/apply_devices.go b/modules/zfs/cmd/zfs-tools/apply_devices.go new file mode 100644 index 000000000..191d0d2df --- /dev/null +++ b/modules/zfs/cmd/zfs-tools/apply_devices.go @@ -0,0 +1,516 @@ +package main + +import ( + "context" + "errors" + "fmt" + "regexp" + "strings" +) + +// The apply's cache and log devices (design 42 zfs-owns-zfs §17). +// +// From `zpool status -P -L ` the apply knows each entry's kernel path, from `zpool status -P ` the +// name the pool holds it by, and from the link under /dev/disk/by-id each declared name's kernel path. Then, in +// this order: an entry named by its kernel name that is a declared device is removed and added again by its +// stable name; a declared device not in the pool is added after its checks, never with -f; only while the list +// is set and not empty is a cache entry it does not name removed; and a log vdev it does not name, one to +// rename, or the log when `log-mirror` changes is removed only while the pool is healthy and idle, else it is +// a wait. A declared device that is not present is a wait. + +// ByIDDir is where a declared device's stable name is a link. +const ByIDDir = "/dev/disk/by-id/" + +// DeviceStatusArgv reads one pool's status with the paths its vdevs are held by (real false) or the kernel +// paths they resolve to (real true). +func DeviceStatusArgv(pool string, real bool) []string { + a := []string{"status", "-P"} + if real { + a = append(a, "-L") + } + return append(a, "--", pool) +} + +// AllRealStatusArgv reads every imported pool's vdevs by kernel path. +func AllRealStatusArgv() []string { return []string{"status", "-P", "-L"} } + +// RemoveArgv removes one cache or log entry of the pool. +func RemoveArgv(pool, entry string) []string { return []string{"remove", "--", pool, entry} } + +// AddArgv adds cache or log devices by their paths; the log as one mirror when mirror is true. Never -f. +func AddArgv(pool, role string, mirror bool, paths ...string) []string { + a := []string{"add", "--", pool, role} + if mirror { + a = append(a, "mirror") + } + return append(a, paths...) +} + +// WipefsArgv lists a device's signatures and writes nothing. +func WipefsArgv(device string) []string { return []string{"--no-act", device} } + +// ZdbLabelArgv reads a device's ZFS label. +func ZdbLabelArgv(device string) []string { return []string{"-l", device} } + +// devEntry is one cache or log device of the pool. +type devEntry struct { + Held string // the path the pool holds it by + Real string // its kernel path + Role string // cache or log + Vdev string // the top-level vdev it is in ("" for a device of its own) +} + +// logVdev is one top-level vdev of the log: a device of its own, or a mirror. +type logVdev struct { + Name string + Mirror bool + Members []devEntry +} + +// poolDevices is what the apply knows of the pool's cache and log. +type poolDevices struct { + Cache []devEntry + Log []logVdev + Healthy bool // ONLINE, no error on any device, no scrub or resilver running + Why string // what holds it from being healthy +} + +func (p *poolDevices) members() []devEntry { + out := append([]devEntry{}, p.Cache...) + for _, v := range p.Log { + out = append(out, v.Members...) + } + return out +} + +func (p *poolDevices) inPool(real string) *devEntry { + for _, e := range p.members() { + if e.Real == real { + return &e + } + } + return nil +} + +func findPool(out, pool string) (*PoolStatus, error) { + ps, err := ParseStatus(out) + if err != nil { + return nil, err + } + for i := range ps { + if ps[i].Name == pool { + return &ps[i], nil + } + } + return nil, fmt.Errorf("zpool status does not show the pool %s", pool) +} + +func sectionOf(p *PoolStatus, role string) *Device { + if p.Root == nil { + return nil + } + for _, c := range p.Root.Children { + if c.Kind == "section" && c.Role == role { + return c + } + } + return nil +} + +func deviceErrors(d *Device) uint64 { + n := d.Errors() + for _, c := range d.Children { + n += deviceErrors(c) + } + return n +} + +// devicesOf reads the pool's cache and log, held-by and real read side by side. +func (m *Machine) devicesOf(ctx context.Context, pool string) (*poolDevices, error) { + var st [2]*PoolStatus + for i, real := range []bool{false, true} { + out, err := m.ask(ctx, "zpool", DeviceStatusArgv(pool, real)...) + if err != nil { + return nil, err + } + if st[i], err = findPool(out, pool); err != nil { + return nil, err + } + } + held, real := st[0], st[1] + pd := &poolDevices{Healthy: true} + switch { + case real.State != "ONLINE": + pd.Healthy, pd.Why = false, "the pool is "+real.State + case real.Root != nil && deviceErrors(real.Root) > 0: + pd.Healthy, pd.Why = false, "a device of the pool has read, write or checksum errors" + } + for _, s := range real.Scans { + if (s.Kind == "scrub" || s.Kind == "resilver") && (s.State == "running" || s.State == "paused") { + pd.Healthy, pd.Why = false, "a "+s.Kind+" is running on the pool" + } + } + for _, role := range []string{"cache", "log"} { + hs, rs := sectionOf(held, role), sectionOf(real, role) + if hs == nil || rs == nil { + continue + } + if len(hs.Children) != len(rs.Children) { + return nil, fmt.Errorf("zpool status -P and -P -L list a different %s", role) + } + for i, hc := range hs.Children { + rc := rs.Children[i] + if len(hc.Children) != len(rc.Children) { + return nil, fmt.Errorf("zpool status -P and -P -L list a different %s", role) + } + if len(hc.Children) == 0 { + e := devEntry{Held: hc.Name, Real: rc.Name, Role: role} + if role == "cache" { + pd.Cache = append(pd.Cache, e) + } else { + pd.Log = append(pd.Log, logVdev{Name: hc.Name, Members: []devEntry{e}}) + } + continue + } + v := logVdev{Name: hc.Name, Mirror: true} + for j, hm := range hc.Children { + v.Members = append(v.Members, devEntry{Held: hm.Name, Real: rc.Children[j].Name, Role: role, Vdev: hc.Name}) + } + if role == "log" { + pd.Log = append(pd.Log, v) + } else { + return nil, fmt.Errorf("the cache holds a vdev %s, which a cache never is", hc.Name) + } + } + } + return pd, nil +} + +// declared is one device the config names. +type declared struct { + Name, Path, Real, Role string + Present bool +} + +func (m *Machine) resolveDeclared(names []string, role string) []declared { + var out []declared + for _, n := range names { + d := declared{Name: n, Path: ByIDDir + n, Role: role} + if m.Resolve != nil { + if r, err := m.Resolve(d.Path); err == nil { + d.Real, d.Present = r, true + } + } + out = append(out, d) + } + return out +} + +func byID(held string) bool { return strings.HasPrefix(held, ByIDDir) } + +var labelName = regexp.MustCompile(`(?m)^\s*name:\s*'([^']*)'`) + +// partOf says whether entry is device itself or one of its partitions. +func partOf(entry, device string) bool { + rest, ok := strings.CutPrefix(entry, device) + if !ok { + return false + } + if rest == "" { + return true + } + rest = strings.TrimPrefix(rest, "p") + return rest != "" && strings.Trim(rest, "0123456789") == "" +} + +// checkDevice is the refusal's reason for a device to add, "" when it may be added (§17): a partition table or +// a file-system signature refuses it, but for a zfs_member signature judged by its label (this pool's is +// allowed, another's refused), and a member of another imported pool is refused. +func (m *Machine) checkDevice(ctx context.Context, pool string, d declared, others []string) (string, error) { + for _, o := range others { + if partOf(o, d.Real) || partOf(d.Real, o) { + return d.Name + " is a member of another imported pool", nil + } + } + r, err := m.AsRoot(ctx, "wipefs", WipefsArgv(d.Path)...) + if err != nil { + return "", err + } + zfsMember := false + for _, l := range lines(r.Out) { + f := strings.Fields(l) + if len(f) == 0 || f[0] == "DEVICE" { + continue + } + if len(f) < 3 { + return d.Name + " shows a signature wipefs names no type for: " + l, nil + } + if f[2] != "zfs_member" { + return fmt.Sprintf("%s holds a %s signature or partition table", d.Name, f[2]), nil + } + zfsMember = true + } + if !zfsMember { + return "", nil + } + z, err := m.AsRoot(ctx, "zdb", ZdbLabelArgv(d.Path)...) + if err != nil { + return d.Name + " holds a ZFS label that cannot be read: " + err.Error(), nil + } + lm := labelName.FindStringSubmatch(z.Out) + switch { + case lm == nil: + return d.Name + " holds a ZFS label that names no pool", nil + case lm[1] != pool: + return fmt.Sprintf("%s holds the label of the pool %s, not %s", d.Name, lm[1], pool), nil + } + return "", nil +} + +// otherPoolDevices are the kernel paths of every imported pool's vdevs but the pool's own. +func (m *Machine) otherPoolDevices(ctx context.Context, pool string) ([]string, error) { + out, err := m.ask(ctx, "zpool", AllRealStatusArgv()...) + if err != nil { + return nil, err + } + ps, err := ParseStatus(out) + if err != nil { + return nil, err + } + var paths []string + var walk func(d *Device) + walk = func(d *Device) { + for _, c := range d.Children { + if strings.HasPrefix(c.Name, "/dev/") { + paths = append(paths, c.Name) + } + walk(c) + } + } + for _, p := range ps { + if p.Name != pool && p.Root != nil { + walk(p.Root) + } + } + return paths, nil +} + +// ApplyDevices converges the cache and log devices the config names; it answers every failed command as one +// error. An empty list is not managed, so it removes nothing. +func (m *Machine) ApplyDevices(ctx context.Context, c *Config) error { + if c.Pool == "" || (len(c.CacheDevices) == 0 && len(c.LogDevices) == 0) { + return nil + } + var failed []error + fail := func(key string, err error) { + failed = append(failed, fmt.Errorf("%s: %v", key, err)) + _ = m.CommandFailed(key, err) + } + others, err := m.otherPoolDevices(ctx, c.Pool) + if err != nil { + fail("cache-devices", err) + return errors.Join(failed...) + } + for _, role := range []string{"cache", "log"} { + names := c.CacheDevices + if role == "log" { + names = c.LogDevices + } + if len(names) == 0 { + continue + } + key := role + "-devices" + if err := m.applyRole(ctx, c, role, key, m.resolveDeclared(names, role), others); err != nil { + fail(key, err) + } + } + return errors.Join(failed...) +} + +func (m *Machine) applyRole(ctx context.Context, c *Config, role, key string, want []declared, others []string) error { + pd, err := m.devicesOf(ctx, c.Pool) + if err != nil { + return err + } + before := m.rolesSaid(pd, role) + for _, d := range want { + if !d.Present { + return m.Waiting(key, "the device "+d.Name+" to appear under "+ByIDDir) + } + } + if role == "log" { + if bad, _ := badLog(c, want, pd); len(bad) > 0 && !pd.Healthy { + return m.Waiting(key, "the pool to be ONLINE without errors and with no scrub or resilver ("+pd.Why+")") + } + } + // Checks first, so nothing is removed that cannot be put back. + for _, d := range want { + if e := pd.inPool(d.Real); e != nil && byID(e.Held) { + continue + } + var out []string + if pd.inPool(d.Real) == nil { + out = others + } + why, err := m.checkDevice(ctx, c.Pool, d, out) + if err != nil { + return err + } + if why != "" { + return m.Refused(Refusal{Key: key, Value: d.Name, Reason: why}) + } + } + var changes []string + if role == "cache" { + changes, err = m.applyCache(ctx, c, want, pd) + } else { + changes, err = m.applyLog(ctx, c, want, pd) + } + if err != nil { + return err + } + after, err := m.devicesOf(ctx, c.Pool) + if err != nil { + return err + } + for _, d := range want { + if e := after.inPool(d.Real); e == nil || !byID(e.Held) { + if len(changes) == 0 { + continue + } + return fmt.Errorf("read back: %s is not in the pool by its stable name after the change", d.Name) + } + } + if len(changes) == 0 { + return m.keepAsSet(key, before) + } + return m.Changed("pool", key, before, strings.Join(changes, "; "), m.rolesSaid(after, role)) +} + +func (m *Machine) rolesSaid(pd *poolDevices, role string) string { + var held []string + for _, e := range pd.members() { + if e.Role == role { + held = append(held, e.Held) + } + } + return strings.Join(held, " ") +} + +func (m *Machine) applyCache(ctx context.Context, c *Config, want []declared, pd *poolDevices) ([]string, error) { + var changes []string + for _, d := range want { + if e := pd.inPool(d.Real); e != nil && e.Role == "cache" && !byID(e.Held) { + if _, err := m.AsRoot(ctx, "zpool", RemoveArgv(c.Pool, e.Held)...); err != nil { + return changes, err + } + if _, err := m.AsRoot(ctx, "zpool", AddArgv(c.Pool, "cache", false, d.Path)...); err != nil { + return changes, err + } + changes = append(changes, fmt.Sprintf("%s re-added by its stable name %s", e.Held, d.Name)) + } + } + var adds []string + var names []string + for _, d := range want { + if pd.inPool(d.Real) == nil { + adds, names = append(adds, d.Path), append(names, d.Name) + } + } + if len(adds) > 0 { + if _, err := m.AsRoot(ctx, "zpool", AddArgv(c.Pool, "cache", false, adds...)...); err != nil { + return changes, err + } + changes = append(changes, "added "+strings.Join(names, " ")) + } + for _, e := range pd.Cache { + if !declaredReal(want, e.Real) { + if _, err := m.AsRoot(ctx, "zpool", RemoveArgv(c.Pool, e.Held)...); err != nil { + return changes, err + } + changes = append(changes, "removed "+e.Held) + } + } + return changes, nil +} + +func declaredReal(want []declared, real string) bool { + for _, d := range want { + if d.Real == real { + return true + } + } + return false +} + +// badLog is the log vdevs that must go: one holding a device the config does not name or one held by its +// kernel name, any when log-mirror asks the other form, and a mirror whose members are not the declared pair. +func badLog(c *Config, want []declared, pd *poolDevices) (bad []logVdev, mirrorChange bool) { + for _, v := range pd.Log { + b := false + for _, e := range v.Members { + if !declaredReal(want, e.Real) || !byID(e.Held) { + b = true + } + } + if c.LogMirror != nil { + if *c.LogMirror && (!v.Mirror || len(v.Members) != len(want)) { + b, mirrorChange = true, true + } + if !*c.LogMirror && v.Mirror { + b, mirrorChange = true, true + } + } + if b { + bad = append(bad, v) + } + } + return bad, mirrorChange +} + +func (m *Machine) applyLog(ctx context.Context, c *Config, want []declared, pd *poolDevices) ([]string, error) { + var changes []string + bad, mirrorChange := badLog(c, want, pd) + if len(bad) > 0 { + for _, v := range bad { + if _, err := m.AsRoot(ctx, "zpool", RemoveArgv(c.Pool, v.Name)...); err != nil { + return changes, err + } + note := "removed " + v.Name + if mirrorChange { + note += " (a log-mirror change: a removal and a re-add)" + } + changes = append(changes, note) + } + var err error + if pd, err = m.devicesOf(ctx, c.Pool); err != nil { + return changes, err + } + } + var missing []declared + for _, d := range want { + if pd.inPool(d.Real) == nil { + missing = append(missing, d) + } + } + if len(missing) == 0 { + return changes, nil + } + var paths, names []string + for _, d := range missing { + paths, names = append(paths, d.Path), append(names, d.Name) + } + mirror := c.LogMirror != nil && *c.LogMirror + if mirror && len(missing) == len(want) { + if _, err := m.AsRoot(ctx, "zpool", AddArgv(c.Pool, "log", true, paths...)...); err != nil { + return changes, err + } + } else { + for _, p := range paths { + if _, err := m.AsRoot(ctx, "zpool", AddArgv(c.Pool, "log", false, p)...); err != nil { + return changes, err + } + } + } + return append(changes, "added "+strings.Join(names, " ")), nil +} diff --git a/modules/zfs/cmd/zfs-tools/apply_devices_test.go b/modules/zfs/cmd/zfs-tools/apply_devices_test.go new file mode 100644 index 000000000..7fc66d444 --- /dev/null +++ b/modules/zfs/cmd/zfs-tools/apply_devices_test.go @@ -0,0 +1,240 @@ +package main + +import ( + "context" + "fmt" + "strings" + "testing" +) + +type devSpec struct { + state, scan string + errs bool + cache [][2]string // held, real + logs [][][2]string // top-level log vdevs; more than one member is a mirror +} + +func (s devSpec) text(real bool) string { + state, scan := s.state, s.scan + if state == "" { + state = "ONLINE" + } + if scan == "" { + scan = "scrub repaired 0B in 00:04:11 with 0 errors on Sun Jul 12 00:28:12 2026" + } + cksum := "0" + if s.errs { + cksum = "3" + } + name := func(p [2]string) string { + if real { + return p[1] + } + return p[0] + } + var b strings.Builder + fmt.Fprintf(&b, " pool: tank\n state: %s\n scan: %s\nconfig:\n\n\tNAME STATE READ WRITE CKSUM\n\ttank %s 0 0 %s\n", state, scan, state, cksum) + b.WriteString("\t /dev/disk/by-id/data-1 ONLINE 0 0 0\n") + if len(s.logs) > 0 { + b.WriteString("\tlogs\t\n") + for i, v := range s.logs { + if len(v) == 1 { + fmt.Fprintf(&b, "\t %s ONLINE 0 0 0\n", name(v[0])) + continue + } + fmt.Fprintf(&b, "\t mirror-%d ONLINE 0 0 0\n", i) + for _, m := range v { + fmt.Fprintf(&b, "\t %s ONLINE 0 0 0\n", name(m)) + } + } + } + if len(s.cache) > 0 { + b.WriteString("\tcache\t\n") + for _, c := range s.cache { + fmt.Fprintf(&b, "\t %s ONLINE 0 0 0\n", name(c)) + } + } + b.WriteString("\nerrors: No known data errors\n") + return b.String() +} + +func (s devSpec) set(f *fake) { + f.on("zpool status -P -- tank", s.text(false)) + f.on("zpool status -P -L -- tank", s.text(true)) +} + +func dev(n string) [2]string { return [2]string{"/dev/disk/by-id/" + n, "/dev/" + n + "k"} } +func kern(n string) [2]string { return [2]string{"/dev/" + n + "k", "/dev/" + n + "k"} } + +// §17, §31 convergence table: a kernel-named entry is re-added by its stable name; an add is after its +// checks, never -f; an empty list removes nothing; a log removal while not healthy is a wait; a log-mirror +// change is a removal and a re-add, said so. +func TestDevicesConvergeFromWhatZFSSays(t *testing.T) { + const wipeGPT = "DEVICE OFFSET TYPE UUID LABEL\nnvme1k 0x200 gpt\n" + const wipeZFS = "DEVICE OFFSET TYPE UUID LABEL\nnvme1k 0x4000 zfs_member 123 tank\n" + cases := []struct { + name string + conf string + before, mid devSpec + hasMid bool + after devSpec + answers map[string]string + others string + ran []string + notRan []string + event, state string + wantErr bool + waitsOrRefuse string + }{ + {name: "kernel name re-added by its stable name", conf: `{"pool":"tank","cache-devices":["c1"]}`, + before: devSpec{cache: [][2]string{kern("c1")}}, mid: devSpec{}, hasMid: true, after: devSpec{cache: [][2]string{dev("c1")}}, + answers: map[string]string{"zdb -l /dev/disk/by-id/c1": "name: 'tank'\n", "wipefs --no-act /dev/disk/by-id/c1": wipeZFS}, + ran: []string{"zpool remove -- tank /dev/c1k", "zpool add -- tank cache /dev/disk/by-id/c1"}, event: "config.applied", state: "applied"}, + {name: "declared device not in the pool is added", conf: `{"pool":"tank","cache-devices":["c2"]}`, + before: devSpec{}, after: devSpec{cache: [][2]string{dev("c2")}}, + answers: map[string]string{"wipefs --no-act /dev/disk/by-id/c2": ""}, + ran: []string{"zpool add -- tank cache /dev/disk/by-id/c2"}, event: "config.applied", state: "applied"}, + {name: "a partition table refuses it", conf: `{"pool":"tank","cache-devices":["c2"]}`, + before: devSpec{}, + answers: map[string]string{"wipefs --no-act /dev/disk/by-id/c2": wipeGPT}, + notRan: []string{"zpool add"}, event: "config.refused", state: "refused"}, + {name: "another pool's label refuses it", conf: `{"pool":"tank","cache-devices":["c2"]}`, + before: devSpec{}, + answers: map[string]string{"wipefs --no-act /dev/disk/by-id/c2": wipeZFS, "zdb -l /dev/disk/by-id/c2": "name: 'other'\n"}, + notRan: []string{"zpool add"}, event: "config.refused", state: "refused"}, + {name: "this pool's label allows it", conf: `{"pool":"tank","cache-devices":["c2"]}`, + before: devSpec{}, after: devSpec{cache: [][2]string{dev("c2")}}, + answers: map[string]string{"wipefs --no-act /dev/disk/by-id/c2": wipeZFS, "zdb -l /dev/disk/by-id/c2": "name: 'tank'\n"}, + ran: []string{"zpool add -- tank cache /dev/disk/by-id/c2"}, event: "config.applied", state: "applied"}, + {name: "a member of another imported pool is refused", conf: `{"pool":"tank","cache-devices":["c2"]}`, + before: devSpec{}, others: "/dev/c2kp1", notRan: []string{"wipefs", "zpool add"}, event: "config.refused", state: "refused"}, + {name: "a device not present is a wait", conf: `{"pool":"tank","cache-devices":["c9"]}`, + before: devSpec{}, notRan: []string{"zpool add", "wipefs"}, event: "config.waiting", state: "waiting"}, + {name: "an empty cache list removes nothing", conf: `{"pool":"tank","cache-devices":[],"log-devices":["l1"]}`, + before: devSpec{cache: [][2]string{dev("old")}, logs: [][][2]string{{dev("l1")}}}, notRan: []string{"zpool remove", "zpool add"}, state: "applied"}, + {name: "a cache entry not named is removed", conf: `{"pool":"tank","cache-devices":["c1"]}`, + before: devSpec{cache: [][2]string{dev("c1"), dev("old")}}, after: devSpec{cache: [][2]string{dev("c1")}}, + ran: []string{"zpool remove -- tank /dev/disk/by-id/old"}, event: "config.applied", state: "applied"}, + {name: "a log removal while a scrub runs is a wait", conf: `{"pool":"tank","log-devices":["l1"]}`, + before: devSpec{scan: "scrub in progress since Sun Jul 12 00:28:12 2026", logs: [][][2]string{{kern("l1")}}}, + notRan: []string{"zpool remove", "zpool add"}, event: "config.waiting", state: "waiting"}, + {name: "a log removal with errors is a wait", conf: `{"pool":"tank","log-devices":["l1"]}`, + before: devSpec{errs: true, logs: [][][2]string{{dev("l1"), dev("x")}}}, notRan: []string{"zpool remove"}, event: "config.waiting", state: "waiting"}, + {name: "a log-mirror change is a removal and a re-add", conf: `{"pool":"tank","log-devices":["l1","l2"],"log-mirror":true}`, + before: devSpec{logs: [][][2]string{{dev("l1")}, {dev("l2")}}}, mid: devSpec{}, hasMid: true, + after: devSpec{logs: [][][2]string{{dev("l1"), dev("l2")}}}, + answers: map[string]string{"wipefs --no-act /dev/disk/by-id/l1": "", "wipefs --no-act /dev/disk/by-id/l2": ""}, + ran: []string{"zpool remove -- tank /dev/disk/by-id/l1", "zpool remove -- tank /dev/disk/by-id/l2", + "zpool add -- tank log mirror /dev/disk/by-id/l1 /dev/disk/by-id/l2"}, event: "config.applied", state: "applied"}, + {name: "everything as set runs nothing", conf: `{"pool":"tank","cache-devices":["c1"],"log-devices":["l1","l2"],"log-mirror":true}`, + before: devSpec{cache: [][2]string{dev("c1")}, logs: [][][2]string{{dev("l1"), dev("l2")}}}, + notRan: []string{"zpool remove", "zpool add"}, state: "applied"}, + } + for _, c := range cases { + f := newFake(t) + f.uid = 0 + for _, n := range []string{"c1", "c2", "l1", "l2", "old", "x"} { + f.links["/dev/disk/by-id/"+n] = "/dev/" + n + "k" + } + c.before.set(f) + f.on("zpool status -P -L", " pool: other\n state: ONLINE\nconfig:\n\n\tNAME STATE READ WRITE CKSUM\n\tother ONLINE 0 0 0\n\t "+ + orDefault(c.others, "/dev/zzz")+" ONLINE 0 0 0\n\nerrors: No known data errors\n") + for k, v := range c.answers { + f.on(k, v) + } + mid := c.mid + if !c.hasMid { + mid = c.after + } + // A removal moves the pool to mid, an add to after. + hook := func(spec devSpec) func(*fake) { return func(f *fake) { spec.set(f) } } + for _, r := range c.ran { + f.on(r, "") + if strings.HasPrefix(r, "zpool remove") { + f.after[r] = hook(mid) + } else { + f.after[r] = hook(c.after) + } + } + m := f.machine() + st := memStates{} + m.States = st + cfg, ref := ParseConfig([]byte(c.conf), 16000) + if len(ref) != 0 { + t.Fatal(c.name, ref) + } + err := m.ApplyDevices(context.Background(), cfg) + if (err != nil) != c.wantErr { + t.Errorf("%s: error %v", c.name, err) + } + for _, r := range c.ran { + if len(ran(f, r)) != 1 { + t.Errorf("%s: %q not run once; ran %v", c.name, r, f.calls) + } + } + for _, n := range c.notRan { + if len(ran(f, n)) != 0 { + t.Errorf("%s: ran %v", c.name, ran(f, n)) + } + } + for _, call := range f.calls { + if strings.Contains(call, " -f") { + t.Errorf("%s: forced: %s", c.name, call) + } + } + if c.event == "" && len(f.emitted) != 0 { + t.Errorf("%s: events %v", c.name, f.emitted) + } + if c.event != "" && (len(f.emitted) != 1 || f.emitted[0]["_type"] != c.event) { + t.Errorf("%s: events %v, want %s", c.name, f.emitted, c.event) + } + if c.state != "" { + found := false + for k, v := range st { + if strings.Contains(string(v), `"outcome":"`+c.state+`"`) { + found = true + _ = k + } + } + if !found { + t.Errorf("%s: state %v, want outcome %s", c.name, st, c.state) + } + } + } +} + +func orDefault(s, d string) string { + if s == "" { + return d + } + return s +} + +// §17: a log-mirror change is said as a removal and a re-add. +func TestALogMirrorChangeIsSaidAsARemovalAndAReAdd(t *testing.T) { + f := newFake(t) + f.uid = 0 + for _, n := range []string{"l1", "l2"} { + f.links["/dev/disk/by-id/"+n] = "/dev/" + n + "k" + } + before := devSpec{logs: [][][2]string{{dev("l1")}, {dev("l2")}}} + after := devSpec{logs: [][][2]string{{dev("l1"), dev("l2")}}} + before.set(f) + f.on("zpool status -P -L", "") + f.on("wipefs --no-act /dev/disk/by-id/l1", "") + f.on("wipefs --no-act /dev/disk/by-id/l2", "") + f.on("zpool remove -- tank /dev/disk/by-id/l1", "") + f.on("zpool remove -- tank /dev/disk/by-id/l2", "") + f.after["zpool remove -- tank /dev/disk/by-id/l2"] = func(f *fake) { devSpec{}.set(f) } + f.on("zpool add -- tank log mirror /dev/disk/by-id/l1 /dev/disk/by-id/l2", "") + f.after["zpool add -- tank log mirror /dev/disk/by-id/l1 /dev/disk/by-id/l2"] = func(f *fake) { after.set(f) } + m := f.machine() + m.States = memStates{} + cfg, _ := ParseConfig([]byte(`{"pool":"tank","log-devices":["l1","l2"],"log-mirror":true}`), 16000) + if err := m.ApplyDevices(context.Background(), cfg); err != nil { + t.Fatal(err) + } + if a, _ := f.emitted[0]["after"].(string); !strings.Contains(a, "a log-mirror change: a removal and a re-add") { + t.Errorf("event %v", f.emitted[0]) + } +} diff --git a/modules/zfs/cmd/zfs-tools/helpers_test.go b/modules/zfs/cmd/zfs-tools/helpers_test.go index 3cc1be180..9a245c587 100644 --- a/modules/zfs/cmd/zfs-tools/helpers_test.go +++ b/modules/zfs/cmd/zfs-tools/helpers_test.go @@ -32,13 +32,14 @@ type fake struct { emitted []map[string]any emitErr error files map[string]string + links map[string]string now time.Time uid int errors *ErrorMemory } func newFake(t *testing.T) *fake { - return &fake{t: t, answers: map[string]Ran{}, after: map[string]func(*fake){}, files: map[string]string{}, + return &fake{t: t, answers: map[string]Ran{}, after: map[string]func(*fake){}, files: map[string]string{}, links: map[string]string{}, now: time.Date(2026, 10, 9, 12, 0, 0, 0, time.Local), uid: 1000, errors: NewErrorMemory()} } @@ -74,6 +75,12 @@ func (f *fake) machine() *Machine { } return []byte(s), nil }, + Resolve: func(p string) (string, error) { + if r, ok := f.links[p]; ok { + return r, nil + } + return "", os.ErrNotExist + }, Errors: f.errors, Emit: func(eventType string, body any) error { if f.emitErr != nil { diff --git a/modules/zfs/cmd/zfs-tools/machine.go b/modules/zfs/cmd/zfs-tools/machine.go index b70fa3f4a..3164bfacc 100644 --- a/modules/zfs/cmd/zfs-tools/machine.go +++ b/modules/zfs/cmd/zfs-tools/machine.go @@ -8,6 +8,7 @@ import ( "fmt" "os" "os/exec" + "path/filepath" "strings" "sync" "syscall" @@ -111,6 +112,8 @@ type Machine struct { Now func() time.Time // ReadFile reads a kernel file (the ARC's counters). ReadFile func(path string) ([]byte, error) + // Resolve follows a link to its kernel path (a declared device's name under /dev/disk/by-id). + Resolve func(path string) (string, error) // Emit publishes an event as this module (the record of an act); nil where nothing can be published. Emit func(eventType string, body any) error // States is the module's states on the bus (§26); nil where this process reaches none. @@ -122,7 +125,7 @@ type Machine struct { // Here is the machine this process runs on. func Here() *Machine { - return &Machine{Run: ExecRunner, UID: os.Getuid(), Now: time.Now, ReadFile: os.ReadFile, Errors: LoadErrorMemory(ErrorsFile)} + return &Machine{Run: ExecRunner, UID: os.Getuid(), Now: time.Now, ReadFile: os.ReadFile, Resolve: filepath.EvalSymlinks, Errors: LoadErrorMemory(ErrorsFile)} } // NotApplicable is the answer of a machine without ZFS: the module does not apply here. diff --git a/modules/zfs/cmd/zfs-tools/manifest_test.go b/modules/zfs/cmd/zfs-tools/manifest_test.go index 3814ce6ae..eca905d50 100644 --- a/modules/zfs/cmd/zfs-tools/manifest_test.go +++ b/modules/zfs/cmd/zfs-tools/manifest_test.go @@ -220,6 +220,8 @@ func TestTheBundleRunsOnlyReadingVerbsSnapshotAndScrub(t *testing.T) { "TimerShowArgv": {"systemctl", "show"}, "ServiceShowArgv": {"systemctl", "show"}, "PoolGetArgv": {"zpool", "get"}, "PoolSetArgv": {"zpool", "set"}, "DatasetGetArgv": {"zfs", "get"}, "DatasetSetArgv": {"zfs", "set"}, "DatasetInheritArgv": {"zfs", "inherit"}, + "DeviceStatusArgv": {"zpool", "status"}, "AllRealStatusArgv": {"zpool", "status"}, "RemoveArgv": {"zpool", "remove"}, + "AddArgv": {"zpool", "add"}, "WipefsArgv": {"wipefs", "--no-act"}, "ZdbLabelArgv": {"zdb", "-l"}, } verbs := map[string][]string{"DatasetsArgv": DatasetsArgv(), "SnapshotsArgv": SnapshotsArgv(), "PoolsArgv": PoolsArgv(), "StatusArgv": StatusArgv(), "IOArgv": IOArgv(3), "EventsArgv": EventsArgv(), "HistoryArgv": HistoryArgv("tank"), @@ -227,7 +229,10 @@ func TestTheBundleRunsOnlyReadingVerbsSnapshotAndScrub(t *testing.T) { "TimerShowArgv": TimerShowArgv("x.timer"), "ServiceShowArgv": ServiceShowArgv("x.service"), "WaitArgv": WaitArgv("tank"), "PoolGetArgv": PoolGetArgv("tank", "autotrim"), "PoolSetArgv": PoolSetArgv("tank", "autotrim", "on"), "DatasetGetArgv": DatasetGetArgv("tank", "atime"), "DatasetSetArgv": DatasetSetArgv("tank", "atime", "on"), - "DatasetInheritArgv": DatasetInheritArgv("tank", "atime")} + "DatasetInheritArgv": DatasetInheritArgv("tank", "atime"), + "DeviceStatusArgv": DeviceStatusArgv("tank", true), "AllRealStatusArgv": AllRealStatusArgv(), + "RemoveArgv": RemoveArgv("tank", "x"), "AddArgv": AddArgv("tank", "cache", false, "/dev/x"), + "WipefsArgv": WipefsArgv("/dev/x"), "ZdbLabelArgv": ZdbLabelArgv("/dev/x")} for fn, argv := range verbs { for _, a := range argv { if fn != "FollowArgv" && (a == "-f" || a == "-r" || a == "-R" || a == "-d") { @@ -248,7 +253,7 @@ func TestTheBundleRunsOnlyReadingVerbsSnapshotAndScrub(t *testing.T) { } } call := regexp.MustCompile(`(\.(ask|AsRoot|Escalated|Run|Start|Output|CombinedOutput)\(|exec\.Command(Context)?\(|os\.StartProcess\(|syscall\.(Exec|ForkExec)\()`) - site := regexp.MustCompile(`(?:\.(?:ask|AsRoot|Escalated)|exec\.Command)\((?:\w+, )?"(zfs|zpool|systemctl)", (\w+)\(`) + site := regexp.MustCompile(`(?:\.(?:ask|AsRoot|Escalated)|exec\.Command)\((?:\w+, )?"(zfs|zpool|systemctl|wipefs|zdb)", (\w+)\(`) // The bodies that run what they are given, each once. bodies := map[string]bool{ "machine.go: r := m.Run(ctx, name, args...)": true,