From 8f2f67bee553b9e16e6c23c2f8f61240ecdbf467 Mon Sep 17 00:00:00 2001 From: T-Crypt <132922589+T-Crypt@users.noreply.github.com> Date: Fri, 30 Oct 2026 21:16:13 -0600 Subject: [PATCH] feat(cli): name the card for berth hold and berth vfio run On a node with several cards, berth hold and berth vfio run always took the first one. Both now take --gpu, the same card names and PCI addresses berth run takes. hold accepts a comma list for one hold on several cards. vfio run takes one card, refuses a list, and reads the named card's nvidia-smi index from berthd's status narrowed to that card. Without --gpu, both take the first card as before. --- CHANGELOG.md | 3 ++ docs/leasing.md | 3 +- docs/vfio.md | 5 ++- internal/cli/commands.go | 10 +++-- internal/cli/events_test.go | 35 ++++++++++++++-- internal/cli/helpers_test.go | 13 ++++++ internal/cli/hold.go | 14 ++++++- internal/cli/hold_test.go | 76 +++++++++++++++++++++++++++++++++ internal/cli/vfio.go | 14 +++++-- internal/cli/vfio_run.go | 14 ++++++- internal/cli/vfio_test.go | 81 ++++++++++++++++++++++++++++++------ internal/skill/SKILL.md | 2 +- 12 files changed, 240 insertions(+), 30 deletions(-) create mode 100644 internal/cli/hold_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index a0d3cb7..d956ae1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ has a matching GitHub release whose body comes from its section below. ### Added +- `berth hold --gpu CARD` and `berth vfio run --gpu CARD` name the card on a node with several. `hold` takes a + comma list for one hold on several cards, and `vfio run` takes one card. Before, both always took the node's + first card. Without `--gpu` they still do. - `slice = "NAME.slice"` on an engine entry, or `berth run --slice NAME.slice` (which wins), starts the engine's transient scope under that systemd slice on a Linux user manager: the slice unit's resource policy, CPU, memory and swap limits, applies to the engine and everything it forks, not only to the wrapper that asked for the card. diff --git a/docs/leasing.md b/docs/leasing.md index 7a96734..ac174fd 100644 --- a/docs/leasing.md +++ b/docs/leasing.md @@ -117,7 +117,8 @@ `reaper SteamLaunch AppId=...`, so the REAPER DAW, also named `reaper`, holds nothing. - **`berth hold [--for 2h] [--reason game]`** takes the card for you. It is a pinned lease at the top priority, so nothing drains it, and it ends on Ctrl-C, after `--for`, or when the - process dies. + process dies. On a node with several cards it takes the first one. `--gpu gpu1` holds another + card, and `--gpu gpu0,gpu1` holds both under one lease. - **`berth run --pinned`** gives an app a lease that is never drained (it keeps the card until it exits). - A hold cannot free memory held by something outside Berth: that memory is still reported, diff --git a/docs/vfio.md b/docs/vfio.md index 79ce4e1..d9060ec 100644 --- a/docs/vfio.md +++ b/docs/vfio.md @@ -47,8 +47,9 @@ group and the driver bound to each function, the processes holding the card's de berthd reports. While the card is bound to `vfio-pci`, `nvidia-smi` cannot see it, so pass the card's address yourself (`berth vfio status 0000:01:00.0`). -`berth vfio run [--reason NAME] [--before CMD] [--after CMD] -- CMD` holds the card and runs your -script: +`berth vfio run [--gpu CARD] [--reason NAME] [--before CMD] [--after CMD] -- CMD` holds the card and +runs your script. On a node with several cards it takes the first one. `--gpu gpu1` (or the card's PCI +address) passes another card through, and only that card's engines drain: ```sh berth vfio run --reason galaxy \ diff --git a/internal/cli/commands.go b/internal/cli/commands.go index 2f0acb5..b2e26c2 100644 --- a/internal/cli/commands.go +++ b/internal/cli/commands.go @@ -240,11 +240,13 @@ device for the actions.`, { Name: "hold", Summary: "keep the card free for a game or a call; engines wait", - Args: "[--for D] [--reason R] [--needs-mib N] [--wait D]", + Args: "[--gpu CARD] [--for D] [--reason R] [--needs-mib N] [--wait D]", Help: `Hold the card for yourself. Every engine on it is drained the usual way, none starts while the hold lasts, and nothing drains the hold: it is a pinned lease at the top priority. It ends on Ctrl-C, after --for, or when this process dies, so a forgotten terminal never strands the card for long. +--gpu names the card on a node with several (gpu1, its PCI address, or a comma +list); without it the hold takes the node's first card. Games you launch from Steam, Lutris or gamescope are held for automatically by the built-in claims (see berth claims); berth hold is for everything else.`, @@ -605,11 +607,13 @@ docs/vfio.md has the requirements and an example script.`, { Name: "run", Summary: "hold the card, run the switch script, wait for the card back", - Args: "[--reason NAME] [--wait D] [--before CMD] [--after CMD] -- CMD [ARGS...]", + Args: "[--gpu CARD] [--reason NAME] [--wait D] [--before CMD] [--after CMD] -- CMD [ARGS...]", Help: `Acquire the VFIO hold the way berth hold does: engines drain at their boundaries and none starts. Then the script runs with the terminal attached, so its sudo prompt reaches you, and with BERTH_VFIO_DEVICES set to the card's PCI -functions, so it hardcodes no address. +functions, so it hardcodes no address. --gpu names the card on a node with +several (gpu1 or its PCI address); without it the run takes the node's first +card. --before runs once the engines are off the card: stop your own services that hold it there. --after runs once the hold has ended, whenever --before ran, diff --git a/internal/cli/events_test.go b/internal/cli/events_test.go index ae61c1c..509e114 100644 --- a/internal/cli/events_test.go +++ b/internal/cli/events_test.go @@ -15,6 +15,7 @@ import ( "github.com/T-Crypt/berth/internal/broker" "github.com/T-Crypt/berth/internal/eventlog" + "github.com/T-Crypt/berth/internal/gpu" "github.com/T-Crypt/berth/internal/node" "github.com/T-Crypt/berth/internal/procs" "github.com/T-Crypt/berth/internal/proto" @@ -24,6 +25,15 @@ import ( // startNode runs a real berthd over one broker per named card, sharing one // event log as berthd's cards do, and notes one event on the last card. func startNode(t *testing.T, names ...string) string { + t.Helper() + sock, brokers := serveNode(t, names...) + brokers[len(brokers)-1].Note(proto.Event{Kind: proto.EvAlertRaised, Detail: "too hot"}) + return sock +} + +// serveNode runs a real berthd over one broker per named card, the card at +// position i reading as nvidia-smi index i, and returns the brokers in order. +func serveNode(t *testing.T, names ...string) (string, []*broker.Broker) { t.Helper() sock := filepath.Join(t.TempDir(), "berth.sock") log := slog.New(slog.NewTextHandler(io.Discard, nil)) @@ -31,27 +41,44 @@ func startNode(t *testing.T, names ...string) string { t.Cleanup(cancel) events, ids := eventlog.New(), &atomic.Uint64{} var cards []*node.Card - for _, name := range names { + var brokers []*broker.Broker + for i, name := range names { b := broker.New(broker.Config{ Card: name, Events: events, IDs: ids, Poll: 20 * time.Millisecond, DefaultWait: 5 * time.Second, JuggleWindow: -1, Version: "test", Log: log, - }, emptyCard{}, procs.System{}) + }, indexCard(i), procs.System{}) b.Tick(ctx) go b.Run(ctx) cards = append(cards, &node.Card{Name: name, B: b}) + brokers = append(brokers, b) } n, err := node.New(events, ids, cards...) if err != nil { t.Fatal(err) } - cards[len(cards)-1].B.Note(proto.Event{Kind: proto.EvAlertRaised, Detail: "too hot"}) ln, err := server.Listen(sock) if err != nil { t.Fatal(err) } go server.Serve(ctx, ln, n, server.Extras{}, log) - return sock + return sock, brokers +} + +// indexCard is an empty card that nvidia-smi numbers with its value. +type indexCard int + +func (i indexCard) Read(context.Context) (gpu.Snapshot, error) { + return gpu.Snapshot{Index: int(i), Total: 24 << 30, Free: 24 << 30, At: time.Now()}, nil +} + +// leaseNames is the engines holding a lease on b. +func leaseNames(b *broker.Broker) []string { + var out []string + for _, l := range b.Status().Leases { + out = append(out, l.Engine) + } + return out } // An event line names its card once berthd runs more than one, as the CARD diff --git a/internal/cli/helpers_test.go b/internal/cli/helpers_test.go index a322ff5..5444d58 100644 --- a/internal/cli/helpers_test.go +++ b/internal/cli/helpers_test.go @@ -10,6 +10,7 @@ import ( "path/filepath" "strings" "testing" + "time" ) // isolate points every path the CLI reads at a temp dir, so a test never @@ -113,3 +114,15 @@ func boolInt(b bool) int { } return 0 } + +// waitUntil polls until f is true, so async daemon-side releases settle. +func waitUntil(t *testing.T, f func() bool) { + t.Helper() + deadline := time.Now().Add(5 * time.Second) + for !f() { + if time.Now().After(deadline) { + t.Fatal("the condition never became true") + } + time.Sleep(5 * time.Millisecond) + } +} diff --git a/internal/cli/hold.go b/internal/cli/hold.go index 0ed314d..c4e3c9c 100644 --- a/internal/cli/hold.go +++ b/internal/cli/hold.go @@ -28,6 +28,7 @@ func runHold(c *Context) int { reason := fs.String("reason", "", "shown on the lease, e.g. game or call (default: hold)") dur := fs.Duration("for", 0, "end the hold after this long; 0 = until Ctrl-C") needs := fs.Uint64("needs-mib", 0, "hold only this much memory instead of the whole card") + card := fs.String("gpu", "", "card to hold: its name on the node (gpu1) or its PCI address, or a comma list of them for one hold on several cards; default the node's first card") wait := fs.Duration("wait", 10*time.Minute, "how long to wait for engines to drain; 0 = berthd's default; negative = no deadline") if code, done := c.Parse(fs); done { return code @@ -35,6 +36,11 @@ func runHold(c *Context) int { if code, done := c.noArgs(); done { return code } + if *card != "" { + if err := gpu.CheckCard(*card); err != nil { + return c.fail(Systemf("%v", err)) + } + } name := "hold" if r := strings.TrimSpace(*reason); r != "" { name = "hold:" + strings.ReplaceAll(r, " ", "-") @@ -49,7 +55,7 @@ func runHold(c *Context) int { } defer raw.Close() go func() { <-ctx.Done(); raw.Close() }() - if err := conn.Send(proto.Msg{Op: proto.OpAcquire, Engine: name, Group: "hold", Priority: "hold", + if err := conn.Send(proto.Msg{Op: proto.OpAcquire, Engine: name, Card: *card, Group: "hold", Priority: "hold", Needs: *needs * gpu.MiB, Wait: wait.Seconds(), Pinned: true}); err != nil { return c.fail(Systemf("berthd on %s: %v", *sock, err)) } @@ -78,7 +84,11 @@ func runHold(c *Context) int { until = "for " + dur.String() timer = time.After(*dur) } - fmt.Fprintf(c.Err, "berth: card held as %s (%s); engines wait until it ends\n", m.Lease, until) + held := "card" + if *card != "" { + held = *card + } + fmt.Fprintf(c.Err, "berth: %s held as %s (%s); engines wait until it ends\n", held, m.Lease, until) if m.Reason != "" { fmt.Fprintf(c.Err, "berth: note: %s\n", m.Reason) } diff --git a/internal/cli/hold_test.go b/internal/cli/hold_test.go new file mode 100644 index 0000000..2aa1d78 --- /dev/null +++ b/internal/cli/hold_test.go @@ -0,0 +1,76 @@ +// Copyright 2026 Trevin Tindall (T-Crypt) +// SPDX-License-Identifier: Apache-2.0 + +package cli + +import ( + "slices" + "strings" + "testing" +) + +// berth hold --gpu holds the card it names on a node of several, and leaves +// the others to their engines. +func TestHoldTakesTheCardItNames(t *testing.T) { + isolate(t) + for _, tc := range []struct { + gpu string + want []bool // held, per card + }{ + {"gpu1", []bool{false, true, false}}, + {"gpu0,gpu2", []bool{true, false, true}}, + {"", []bool{true, false, false}}, + } { + sock, brokers := serveNode(t, "gpu0", "gpu1", "gpu2") + args := []string{"hold", "--socket", sock, "--for", "1s", "--reason", "game"} + if tc.gpu != "" { + args = append(args, "--gpu", tc.gpu) + } + done := make(chan string, 1) + go func() { _, _, errOut := runCLI(t, args...); done <- errOut }() + held := func(b int) bool { return slices.Contains(leaseNames(brokers[b]), "hold:game") } + waitUntil(t, func() bool { + for i, want := range tc.want { + if want && !held(i) { + return false + } + } + return true + }) + for i, want := range tc.want { + if !want && held(i) { + t.Errorf("--gpu %q: gpu%d is held too", tc.gpu, i) + } + } + if errOut := <-done; !strings.Contains(errOut, "hold released") { + t.Errorf("--gpu %q: the hold did not end cleanly: %q", tc.gpu, errOut) + } + } +} + +// berth hold refuses a --gpu no card can be, before berthd is asked. +func TestHoldRefusesABadGPU(t *testing.T) { + isolate(t) + sock, brokers := serveNode(t, "gpu0", "gpu1") + for _, bad := range []string{"card one", "gpu1,gpu1"} { + code, _, errOut := runCLI(t, "hold", "--socket", sock, "--gpu", bad) + if code == ExitOK || !strings.Contains(errOut, "gpu") { + t.Errorf("--gpu %q: exit = %d, stderr %q; want a refusal naming the card", bad, code, errOut) + } + } + for i, b := range brokers { + if n := leaseNames(b); len(n) > 0 { + t.Errorf("gpu%d holds %v after a refused hold", i, n) + } + } +} + +// berth hold --gpu names a card berthd does not have: refused, nothing held. +func TestHoldRefusesACardTheNodeLacks(t *testing.T) { + isolate(t) + sock, _ := serveNode(t, "gpu0", "gpu1") + code, _, errOut := runCLI(t, "hold", "--socket", sock, "--gpu", "gpu7", "--wait", "1s", "--for", "1s") + if code == ExitOK || !strings.Contains(errOut, "no card gpu7") { + t.Errorf("exit = %d, stderr %q; want a refusal that names gpu7", code, errOut) + } +} diff --git a/internal/cli/vfio.go b/internal/cli/vfio.go index 709d70a..3e2a7fc 100644 --- a/internal/cli/vfio.go +++ b/internal/cli/vfio.go @@ -12,6 +12,7 @@ import ( "text/tabwriter" "time" + "github.com/T-Crypt/berth/internal/node" "github.com/T-Crypt/berth/internal/proto" "github.com/T-Crypt/berth/internal/server" "github.com/T-Crypt/berth/internal/vfio" @@ -32,10 +33,17 @@ type vfioCard struct { // readable while the card is bound to vfio-pci and nvidia-smi cannot see it; // berthd is asked either way. Otherwise the status berthd reports names the scheduled // index, and nvidia-smi names that card's bus id; when berthd is down or -// has no reading yet, index 0. -func vfioCardAt(sock, explicit, smiBin string) (vfioCard, *Error) { +// has no reading yet, index 0. key is the card a run names (vfio run +// --gpu), "" for berthd's first card; berthd's status is narrowed to it. +func vfioCardAt(sock, explicit, key, smiBin string) (vfioCard, *Error) { var card vfioCard st, err := vfioStatusOf(sock) + if err == nil && key != "" { + if st = node.View(st, key); st == nil { + return card, Failf("berthd has no card %s", key). + WithHint("berth status names the cards berthd schedules") + } + } card.Daemon, card.DialErr = st, err if explicit != "" { card.Addr = vfio.SysAddr(explicit) @@ -301,7 +309,7 @@ func vfioStatus(c *Context, r vfio.Reader, smiBin string) int { if len(c.Args) == 1 { explicit = c.Args[0] } - card, e := vfioCardAt(*sock, explicit, smiBin) + card, e := vfioCardAt(*sock, explicit, "", smiBin) if e != nil { return c.fail(e) } diff --git a/internal/cli/vfio_run.go b/internal/cli/vfio_run.go index a696b4c..90b3c3f 100644 --- a/internal/cli/vfio_run.go +++ b/internal/cli/vfio_run.go @@ -12,6 +12,7 @@ import ( "syscall" "time" + "github.com/T-Crypt/berth/internal/gpu" "github.com/T-Crypt/berth/internal/proto" "github.com/T-Crypt/berth/internal/runner" "github.com/T-Crypt/berth/internal/server" @@ -114,6 +115,7 @@ func vfioRun(c *Context, o vfioRunOpts) int { wait := fs.Duration("wait", 10*time.Minute, "how long to wait for engines to drain; 0 = berthd's default; negative = no deadline") before := fs.String("before", "", "shell command run once the engines are off the card, before the check that nothing holds it (stop your own services here)") after := fs.String("after", "", "shell command run once the hold has ended, whenever --before ran (start those services again)") + gpuKey := fs.String("gpu", "", "card to pass through: its name on the node (gpu1) or its PCI address; default the node's first card") if code, done := c.Parse(fs); done { return code } @@ -122,9 +124,17 @@ func vfioRun(c *Context, o vfioRunOpts) int { WithHint("usage: berth vfio run [--reason NAME] -- CMD [ARGS...]")) } argv := c.Args + if *gpuKey != "" { + if err := gpu.CheckCard(*gpuKey); err != nil { + return c.fail(Systemf("%v", err)) + } + if strings.Contains(*gpuKey, ",") { + return c.fail(Systemf("--gpu %s names several cards; a passthrough moves one card", *gpuKey)) + } + } // The card, by way of berthd's scheduled index and nvidia-smi. - card, e := vfioCardAt(*sock, "", o.smiBin) + card, e := vfioCardAt(*sock, "", *gpuKey, o.smiBin) if e != nil { return c.fail(e) } @@ -176,7 +186,7 @@ func vfioRun(c *Context, o vfioRunOpts) int { raw.Close() } }() - if err := conn.Send(proto.Msg{Op: proto.OpAcquire, Engine: name, Group: proto.GroupVFIO, Priority: "hold", + if err := conn.Send(proto.Msg{Op: proto.OpAcquire, Engine: name, Card: *gpuKey, Group: proto.GroupVFIO, Priority: "hold", Wait: wait.Seconds(), Pinned: true}); err != nil { return c.fail(Systemf("berthd on %s: %v", *sock, err)) } diff --git a/internal/cli/vfio_test.go b/internal/cli/vfio_test.go index 2a11ea6..9d049a2 100644 --- a/internal/cli/vfio_test.go +++ b/internal/cli/vfio_test.go @@ -215,18 +215,6 @@ func startVfioDaemon(t *testing.T) (string, *broker.Broker) { return sock, b } -// waitUntil polls until f is true, so async daemon-side releases settle. -func waitUntil(t *testing.T, f func() bool) { - t.Helper() - deadline := time.Now().Add(5 * time.Second) - for !f() { - if time.Now().After(deadline) { - t.Fatal("the condition never became true") - } - time.Sleep(5 * time.Millisecond) - } -} - // none is true when the daemon holds no lease. func none(b *broker.Broker) func() bool { return func() bool { return len(b.Status().Leases) == 0 } @@ -670,3 +658,72 @@ func TestVfioStatusNamesTheVMNotAStaleReading(t *testing.T) { t.Errorf("berthd's line = %q, want the VM state and no current-looking memory figure", got) } } + +// TestVfioRunTakesTheCardItNames: on a node of two cards, --gpu gpu1 holds +// gpu1 alone and passes through the card at nvidia-smi index 1. Index 0 is +// another card with no functions in the tree, so a run that resolved the +// first card would fail its preflight. +func TestVfioRunTakesTheCardItNames(t *testing.T) { + isolate(t) + sock, brokers := serveNode(t, "gpu0", "gpu1") + tr := newVfioTree(t) + tr.function(vfioAddr, "nvidia") + tr.group() + smi := tr.script("nvidia-smi", "case \" $* \" in *\" -i 0 \"*) echo 00000000:02:00.0; exit 0;; esac\nexec "+tr.smi()+" \"$@\"\n") + held, resume := filepath.Join(t.TempDir(), "held"), filepath.Join(t.TempDir(), "resume") + cmd := tr.script("switch", fmt.Sprintf("touch %s\nwhile [ ! -e %s ]; do sleep 0.01; done\n%s%sexit 0\n", + held, resume, tr.flip("vfio-pci"), tr.flip("nvidia"))) + done := make(chan string, 1) + go func() { + code, _, errOut := runVfioRunIn(t, tr, smi, vfioRunOpts{settle: time.Millisecond, poll: 5 * time.Millisecond}, + "--socket", sock, "--gpu", "gpu1", "--", cmd) + done <- fmt.Sprintf("exit %d: %s", code, errOut) + }() + waitUntil(t, func() bool { + select { + case out := <-done: + t.Fatalf("the run ended before its script held the card: %s", out) + default: + } + _, err := os.Stat(held) + return err == nil + }) + if got := leaseNames(brokers[1]); len(got) != 1 || got[0] != "vfio" { + t.Errorf("gpu1 leases = %v, want the vfio hold", got) + } + if got := leaseNames(brokers[0]); len(got) != 0 { + t.Errorf("gpu0 leases = %v, want none", got) + } + if err := os.WriteFile(resume, nil, 0o644); err != nil { + t.Fatal(err) + } + if out := <-done; !strings.HasPrefix(out, "exit 0:") || !strings.Contains(out, "the card is back; hold released") { + t.Errorf("run = %q, want exit 0 with the card back", out) + } + waitUntil(t, none(brokers[1])) +} + +// TestVfioRunRefusesSeveralCardsOrOneTheNodeLacks: a passthrough moves one +// card berthd schedules; anything else is refused before a hold is asked. +func TestVfioRunRefusesSeveralCardsOrOneTheNodeLacks(t *testing.T) { + isolate(t) + sock, brokers := serveNode(t, "gpu0", "gpu1") + tr := newVfioTree(t) + tr.function(vfioAddr, "nvidia") + tr.group() + for _, tc := range []struct{ gpu, want string }{ + {"gpu0,gpu1", "a passthrough moves one card"}, + {"gpu7", "berthd has no card gpu7"}, + } { + code, _, errOut := runVfioRunIn(t, tr, tr.smi(), vfioRunOpts{settle: time.Millisecond}, + "--socket", sock, "--gpu", tc.gpu, "--", "true") + if code == ExitOK || !strings.Contains(errOut, tc.want) { + t.Errorf("--gpu %s: exit = %d, stderr %q; want %q", tc.gpu, code, errOut, tc.want) + } + } + for i, b := range brokers { + if n := leaseNames(b); len(n) > 0 { + t.Errorf("gpu%d holds %v after a refused run", i, n) + } + } +} diff --git a/internal/skill/SKILL.md b/internal/skill/SKILL.md index d004e0e..f291b55 100644 --- a/internal/skill/SKILL.md +++ b/internal/skill/SKILL.md @@ -45,7 +45,7 @@ do not assume any engine is protected. | Run a configured engine without retyping its command | `berth run --as NAME` with no command runs the `start` of NAME's `[[engine]]` entry | | Run an app that must not be drained (Blender, a training job) | `berth run --as NAME --pinned -- CMD` | | Give an engine a minimum turn before a request can drain it | `berth run --as NAME --min-turn 5m -- CMD`, or `min_turn = "5m"` in its engine entry (a hold still wins; `berth status` shows the turn left in the TURN column) | -| Free the card for the user (game, call) | `berth hold [--for 2h] [--reason game]` (ends on Ctrl-C) | +| Free the card for the user (game, call) | `berth hold [--for 2h] [--reason game] [--gpu gpu1]` (ends on Ctrl-C; default the first card) | | Call a model and let Berth start its engine | the gateway: `http://127.0.0.1:9799/v1`, bearer key from `berth key` | | Wrap every llama-swap model at once | `berth wire llama-swap --config CFG` (plan), `--write` (apply, keeps a backup), `--write --update` (rewrite wire's flags on lines it wrote, keeping flags added by hand), `--wait 10m` (every model waits that long for the card) | | Run an engine that lives in Docker or Podman | `berth run --as NAME --container CNAME -- docker run --rm --name CNAME ...` |