diff --git a/modules/zfs/cmd/zfs-tools/config.go b/modules/zfs/cmd/zfs-tools/config.go index b608a09cd..54462242c 100644 --- a/modules/zfs/cmd/zfs-tools/config.go +++ b/modules/zfs/cmd/zfs-tools/config.go @@ -263,6 +263,18 @@ func ParseConfig(text []byte, memMiB uint64) (*Config, []Refusal) { return oneOf(s, "Mon", "Tue", "Wed", "Thu", "Fri", "Sat", "Sun") }) c.TrimTime, _ = p.str("trim-time", clockForm) + // A-33: a trim schedule of weekly or monthly with no day or no time runs no trim and is said, as one refused value. + if c.TrimSchedule == "weekly" || c.TrimSchedule == "monthly" { + var missing []string + for _, need := range []struct{ key, have string }{{"trim-day", c.TrimDay}, {"trim-time", c.TrimTime}} { + if need.have == "" { + missing = append(missing, need.key) + } + } + if len(missing) > 0 { + p.refuse("trim-schedule", p.raw["trim-schedule"], "needs "+strings.Join(missing, " and ")+" set: without it no trim runs") + } + } c.Zed, _ = p.str("zed", func(s string) string { return oneOf(s, "on", "off") }) if h, ok := p.str("hostid", func(s string) string { switch { diff --git a/modules/zfs/cmd/zfs-tools/config_test.go b/modules/zfs/cmd/zfs-tools/config_test.go index af1298096..52b498129 100644 --- a/modules/zfs/cmd/zfs-tools/config_test.go +++ b/modules/zfs/cmd/zfs-tools/config_test.go @@ -31,6 +31,8 @@ func TestEachValueHasItsFormAndABlankOneIsNotManaged(t *testing.T) { {"trim schedule", bad("trim-schedule", `"daily"`), "none, weekly, monthly"}, {"trim day", bad("trim-day", `"Monday"`), "Mon"}, {"trim time", bad("trim-time", `"24:00"`), "HH:MM"}, + {"trim schedule without day or time", `{"trim-schedule":"weekly","trim-time":"03:00"}`, "needs trim-day"}, + {"trim schedule without time", `{"trim-schedule":"monthly","trim-day":"Sun"}`, "needs trim-time"}, {"cache device with a slash", bad("cache-devices", `["a/b"]`), "under /dev/disk/by-id"}, {"cache device with a dollar", bad("cache-devices", `["a$b"]`), "under /dev/disk/by-id"}, {"cache device with a quote", bad("cache-devices", `["a\"b"]`), "under /dev/disk/by-id"}, diff --git a/modules/zfs/cmd/zfs-tools/snapshots_prune.go b/modules/zfs/cmd/zfs-tools/snapshots_prune.go index ab2f9dc47..e053a8234 100644 --- a/modules/zfs/cmd/zfs-tools/snapshots_prune.go +++ b/modules/zfs/cmd/zfs-tools/snapshots_prune.go @@ -120,6 +120,16 @@ func (m *Machine) PruneScheduled(ctx context.Context, c *Config, person bool, by } for _, k := range sortedKeys(groups) { g := groups[k] + if c.Snapshots.Of(g[0].Kind) == 0 { + // A-35: a keep count of 0 is that kind's none: its scheduled snapshots are kept and a warning names them. + names := make([]string, 0, len(g)) + for _, e := range g { + names = append(names, e.Dataset+"@"+e.Name) + } + sort.Strings(names) + m.Warnings = append(m.Warnings, fmt.Sprintf("the keep count of %s is 0, which removes nothing: kept %s", g[0].Kind, strings.Join(names, ", "))) + continue + } for _, e := range surplus(g, c.Snapshots.Of(g[0].Kind)) { full := e.Dataset + "@" + e.Name why, err := m.refuseRemoval(ctx, c.Pool, e) diff --git a/modules/zfs/cmd/zfs-tools/snapshots_prune_test.go b/modules/zfs/cmd/zfs-tools/snapshots_prune_test.go index d1ef491a7..d09388b66 100644 --- a/modules/zfs/cmd/zfs-tools/snapshots_prune_test.go +++ b/modules/zfs/cmd/zfs-tools/snapshots_prune_test.go @@ -27,6 +27,7 @@ func TestPruningRemovesEverySurplusByItsRecordAndNothingThatDoesNotFit(t *testin setup func(st memStates) removed []string left int + warns string // a warning the run must have kept wantErr bool }{ {name: "every surplus in one run, oldest first", keeps: `{"hourly":2}`, l: &ledgerCalls{entries: four}, @@ -42,6 +43,8 @@ func TestPruningRemovesEverySurplusByItsRecordAndNothingThatDoesNotFit(t *testin {name: "a name that does not fit is left", keeps: `{"hourly":1}`, l: &ledgerCalls{entries: []LedgerEntry{ {"tank", "before-move", "hourly", at(1)}, entry("tank", 2)}}, left: 1}, {name: "none removes nothing", keeps: `{"none":true}`, l: &ledgerCalls{entries: four}}, + {name: "a keep count of 0 is none: nothing removed, a warning names them (A-35)", keeps: `{"hourly":0,"daily":1}`, + l: &ledgerCalls{entries: four}, warns: "keep count of hourly is 0"}, {name: "paused removes nothing", keeps: `{"hourly":1}`, l: &ledgerCalls{entries: four}, setup: func(st memStates) { st.Put(StateSchedule, KeyPaused, map[string]string{}) }}, {name: "no read, no removal", keeps: `{"hourly":1}`, l: &ledgerCalls{read: errors.New("no bus")}}, @@ -81,6 +84,9 @@ func TestPruningRemovesEverySurplusByItsRecordAndNothingThatDoesNotFit(t *testin } f.on(line("zfs", SnapshotsArgv()), list.String()+"tank@by-hand\t1791400000\t1\t2\n") said, err := m.PruneScheduled(context.Background(), keepsOfJSON(t, c.keeps), false, "") + if c.warns != "" && !strings.Contains(strings.Join(m.Warnings, "|"), c.warns) { + t.Errorf("%s: warnings %v, want %q", c.name, m.Warnings, c.warns) + } if (err != nil) != c.wantErr { t.Errorf("%s: error %v", c.name, err) }