From 7455c6d0793eb2d5d2f75857e7db0b392b35465c Mon Sep 17 00:00:00 2001 From: Sylvain Zimmer Date: Mon, 19 Oct 2026 18:37:29 +0200 Subject: [PATCH 1/5] Release 0.3.1: Packaging & QEMU fix --- CHANGELOG.md | 22 +++- RELEASING.md | 9 ++ cmd/agent-vm/main.go | 9 ++ flake.lock | 27 ++++ flake.nix | 122 ++++++++++++++++++ internal/cli/cli.go | 20 ++- internal/cli/cli_test.go | 17 +++ internal/cli/doctor.go | 4 + internal/cli/doctor_test.go | 11 ++ internal/cli/e2e_test.go | 59 +++++++++ internal/cli/start.go | 13 ++ internal/cli/start_test.go | 23 ++++ internal/cli/vms.go | 2 +- internal/cli/vms_test.go | 12 ++ internal/host/host_test.go | 19 +++ internal/host/prereqs.go | 27 ++-- internal/vm/lima.go | 19 ++- internal/vm/lima_test.go | 60 +++++++++ internal/vm/vm.go | 7 +- internal/vm/vmtest/fake.go | 4 +- packaging/nfpm.yaml | 48 +++++++ patches/README.md | 2 + ...nce-hostagent-outlives-start-context.patch | 76 +++++++++++ release.sh | 64 ++++++++- tests/release.sh | 38 +++++- third_party/lima/pkg/instance/start.go | 4 +- 26 files changed, 691 insertions(+), 27 deletions(-) create mode 100644 flake.lock create mode 100644 flake.nix create mode 100644 packaging/nfpm.yaml create mode 100644 patches/lima/0006-instance-hostagent-outlives-start-context.patch diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d4faae5..ec47e852 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,26 @@ # Changelog -## Unreleased +## 0.3.1 + +### Added + +- Releases have a `.deb` and an `.rpm` for Linux (amd64, arm64): the binary in + `/usr/bin` and the docs, with QEMU and ssh recommended. Not in a package + repository yet: install the file with `apt install ./agent-vm_X.Y.Z_arm64.deb` + or `dnf install ./agent-vm-X.Y.Z-1.aarch64.rpm`. +- A Nix flake (Linux): `nix run github:sylvinus/agent-vm`. QEMU comes with it, + ssh is the system's. + +### Fixed + +- Linux: VMs turned Broken seconds to minutes after a start. The end of the + agent-vm command that started a VM (closing its terminal, a kill, and at + random a plain exit) killed Lima's host agent, which left QEMU running + without it, cut off, and failing every later start. The host agent now + outlives that command. On macOS, the same killed the VM. +- A VM left in that state is stopped by force, then started again, on the + next run; `agent-vm stop` stops it too. `stop` no longer says "VM stopped." + for a Broken VM it did not stop. ## 0.3.0 diff --git a/RELEASING.md b/RELEASING.md index d2e679ba..9769056e 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -23,6 +23,9 @@ binaries `osslsigncode` (`brew install osslsigncode`). `AGENT_VM_WINDOWS_PFX_PASS=`. Without, they go out unsigned, with a warning. - Homebrew: `AGENT_VM_TAP=` updates `Formula/agent-vm.rb` there (each platform's url and sha256) and prints the commands to commit it. + - The Linux `.deb` and `.rpm` (`packaging/nfpm.yaml`) are made with nfpm, built with Go at the + version pinned in `release.sh` (`NFPM_VERSION`; `AGENT_VM_NFPM=` uses another). They + are release assets and in `SHA256SUMS`, after the tarballs. - A run that tagged and then failed is resumed by running it again. 3. In a commit after it: `NEXT_VERSION` to the next version, and a new `## Unreleased` section. @@ -34,5 +37,11 @@ binaries `osslsigncode` (`brew install osslsigncode`). `on_arm`/`on_intel`), installing the tarball's `agent-vm`: that is the shape `AGENT_VM_TAP` updates. Then, in the tap: `brew audit --strict sylvinus/tap/agent-vm`, `brew reinstall` and `brew test` before pushing. +- `flake.nix` builds from source, with its `vendorHash` for the Go dependencies. When go.mod or + go.sum change, `release.sh` stops until it is updated: on a machine with Nix, set + `pkgs.lib.fakeHash`, run `nix build .#agent-vm` (Linux), put the hash it prints back, and update + the `go.mod+go.sum:` checksum line next to it (the error gives the value). The hash is of the + downloaded modules only (`proxyVendor`), so patches to `third_party/` leave it as it is. Do the + same after `nix flake update`: `flake.lock` pins nixpkgs, and with it the Go that downloads them. - `--not-latest` publishes a release without making it GitHub's latest: the installer's default stays the previous release, and the Homebrew formula is left alone, `AGENT_VM_TAP` or not. diff --git a/cmd/agent-vm/main.go b/cmd/agent-vm/main.go index 08b74ddc..a03f84ea 100644 --- a/cmd/agent-vm/main.go +++ b/cmd/agent-vm/main.go @@ -4,12 +4,21 @@ package main import ( "os" + "strings" "github.com/sylvinus/agent-vm/internal/cli" "github.com/sylvinus/agent-vm/internal/limaembed" ) +// extraPath goes first on PATH, for this process and what it starts (the +// hostagent, QEMU): set at build time (-ldflags -X) by a build that ships +// QEMU with agent-vm (flake.nix). Empty otherwise. +var extraPath string + func main() { + if p := os.Getenv("PATH"); extraPath != "" && !strings.HasPrefix(p, extraPath+string(os.PathListSeparator)) { + os.Setenv("PATH", extraPath+string(os.PathListSeparator)+p) + } // Lima's libraries start this executable again for its hostagent. if limaembed.Run(os.Args[1:]) { return diff --git a/flake.lock b/flake.lock new file mode 100644 index 00000000..2899d811 --- /dev/null +++ b/flake.lock @@ -0,0 +1,27 @@ +{ + "nodes": { + "nixpkgs": { + "locked": { + "lastModified": 1792094642, + "narHash": "sha256-zp8ToDMnqZcu4o4IH9pdwXFXGp7yLm/M0dEnNWnyd7I=", + "owner": "NixOS", + "repo": "nixpkgs", + "rev": "fe13242281a67641c0102cdb1087f0ebe586c7df", + "type": "github" + }, + "original": { + "owner": "NixOS", + "ref": "nixos-unstable", + "repo": "nixpkgs", + "type": "github" + } + }, + "root": { + "inputs": { + "nixpkgs": "nixpkgs" + } + } + }, + "root": "root", + "version": 7 +} diff --git a/flake.nix b/flake.nix new file mode 100644 index 00000000..ba5319d7 --- /dev/null +++ b/flake.nix @@ -0,0 +1,122 @@ +{ + description = "agent-vm: AI coding agents in a disposable Linux VM per project"; + + inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable"; + + outputs = + { self, nixpkgs }: + let + # Linux only for now: on macOS the binary needs cgo, the SDK and the + # virtualization entitlement (nixpkgs' lima signs it with + # darwin.sigtool), none of which this build sets up. + systems = [ + "x86_64-linux" + "aarch64-linux" + ]; + forAllSystems = f: nixpkgs.lib.genAttrs systems (system: f nixpkgs.legacyPackages.${system}); + + # The version, the build tags and Lima's version are the Makefile's and + # third_party/SOURCES's: there is one place to change each. + linesOf = file: builtins.filter builtins.isString (builtins.split "\n" (builtins.readFile file)); + lineWith = + file: prefix: + builtins.head ( + builtins.filter (l: builtins.substring 0 (builtins.stringLength prefix) l == prefix) (linesOf file) + ); + makeVar = + name: + let + prefix = "${name} := "; + in + builtins.substring (builtins.stringLength prefix) (-1) (lineWith ./Makefile prefix); + # lima + limaVersion = builtins.elemAt (builtins.filter (s: builtins.isString s && s != "") ( + builtins.split " +" (lineWith ./third_party/SOURCES "lima ") + )) 3; + in + { + packages = forAllSystems (pkgs: rec { + agent-vm = pkgs.buildGoModule { + pname = "agent-vm"; + version = makeVar "NEXT_VERSION"; + src = self; + + # The downloaded modules only: a vendor folder would hold + # third_party/ too (go.mod's replaces), and change with each patch. + proxyVendor = true; + # Changes with go.mod and go.sum: set pkgs.lib.fakeHash here, build, + # and nix prints the new one. release.sh refuses a release when + # this checksum of `cat go.mod go.sum` is not the current one: + # update it with the hash. + # go.mod+go.sum: 0d831936e2c268c65229802f0d5026b4aba48507157f9b6a034225b64408a24e + vendorHash = "sha256-zfAasYQEKqcrGi4OI3UKJyq3VoUCmNE5lU4JKPtJDmc="; + + subPackages = [ "cmd/agent-vm" ]; + tags = pkgs.lib.splitString "," (makeVar "TAGS"); + env.CGO_ENABLED = "0"; + ldflags = [ + "-s" + "-w" + "-X github.com/sylvinus/agent-vm/internal/version.Version=${makeVar "NEXT_VERSION"}" + "-X github.com/lima-vm/lima/v2/pkg/version.Version=${limaVersion}" + # QEMU comes with it, first on its PATH (cmd/agent-vm/main.go), as + # nixpkgs' lima does with a wrapper: a wrapped binary would make + # `agent-vm install` link past the wrapper. qemu_kvm, this host's + # architecture only, is a third smaller than qemu. ssh is the + # system's. + "-X main.extraPath=${pkgs.lib.makeBinPath [ pkgs.qemu_kvm ]}" + ]; + + nativeBuildInputs = [ pkgs.gzip ]; + + # go.mod names the Go the project is developed with, and nixpkgs + # has it a few days after a release: the build uses nixpkgs' (the + # sandbox cannot download a toolchain). + postPatch = '' + sed -i 's/^go [0-9.]*$/go ${pkgs.go.version}/' go.mod + ''; + + # What the Makefile's `assets` target does: the guest agent runs in + # the Linux guest, of this machine's architecture, and is embedded + # in the binary with Lima's templates. + preBuild = + let + guestArch = if pkgs.stdenv.hostPlatform.isAarch64 then "aarch64" else "x86_64"; + assets = "internal/limaembed/assets"; + in + '' + go build -trimpath -ldflags "-s -w -X github.com/lima-vm/lima/v2/pkg/version.Version=${limaVersion}" \ + -o ${assets}/lima-guestagent.Linux-${guestArch} github.com/lima-vm/lima/v2/cmd/lima-guestagent + gzip -9 -n -f ${assets}/lima-guestagent.Linux-${guestArch} + rm -rf ${assets}/templates + cp -R third_party/lima/templates ${assets}/templates + ''; + + # The module download shares preBuild with the build, and has no + # modules to build the guest agent from yet. + overrideModAttrs = _: { + preBuild = ""; + }; + + # The tests run in CI, on the three platforms. + doCheck = false; + + meta = { + description = "Run AI coding agents in a disposable Linux VM per project"; + homepage = "https://www.agent-vm.org/"; + license = pkgs.lib.licenses.mit; + mainProgram = "agent-vm"; + platforms = systems; + }; + }; + default = agent-vm; + }); + + apps = forAllSystems (pkgs: { + default = { + type = "app"; + program = "${self.packages.${pkgs.stdenv.hostPlatform.system}.agent-vm}/bin/agent-vm"; + }; + }); + }; +} diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 03f7867d..c43ac794 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -171,8 +171,26 @@ func versionCmd(args []string, io IO) int { return 1 } -// updateCommand is how this copy of agent-vm is updated. +// updateCommand is how this copy of agent-vm is updated: by what installed +// it. The installer's copy would otherwise land in ~/.local/bin, ahead of a +// package's on the PATH, and hide its updates. func updateCommand() string { + exe, err := self() + if err != nil { + exe = "" + } + return updateCommandFor(exe) +} + +func updateCommandFor(exe string) string { + switch { + case strings.HasPrefix(exe, "/nix/store/"): + return "with Nix, as installed (nix profile upgrade agent-vm, or the flake input)" + case strings.Contains(exe, "/Cellar/agent-vm/"): + return "brew upgrade agent-vm" + case exe == "/usr/bin/agent-vm": + return "install the .deb or .rpm of https://github.com/sylvinus/agent-vm/releases/latest" + } return "curl -fsSL https://www.agent-vm.org/install.sh | sh" } diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index b1ab42cc..7f1f6969 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -114,6 +114,23 @@ func TestHelp(t *testing.T) { } } +// A copy a package manager installed is updated by it: the installer's +// would land ahead of it on the PATH. +func TestUpdateCommand(t *testing.T) { + for exe, want := range map[string]string{ + "/nix/store/abc-agent-vm-0.3.1/bin/agent-vm": "nix profile upgrade", + "/opt/homebrew/Cellar/agent-vm/0.3.1/bin/agent-vm": "brew upgrade agent-vm", + "/usr/bin/agent-vm": ".deb or .rpm", + "/home/u/.local/share/agent-vm/0.3.1/agent-vm": "install.sh | sh", + "/usr/local/bin/agent-vm": "install.sh | sh", + "": "install.sh | sh", + } { + if got := updateCommandFor(exe); !strings.Contains(got, want) { + t.Errorf("%q: %q", exe, got) + } + } +} + func TestSplit(t *testing.T) { for _, c := range []struct { in []string diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index 177aa129..0abf73fd 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -301,6 +301,10 @@ func (d *doctor) directory(ctx context.Context, dir string, protects bool, getVM case inst.Status == vm.Running: d.line("warn", "running with a share the host does not confine: "+confined.Error(), "The next start (after 'agent-vm stop') sets its shares back.") + case inst.Orphaned: + d.line("warn", "running without Lima's host agent, cut off", "'agent-vm stop' stops it, and so does the next agent-vm command here.") + case inst.Status == vm.Broken: + d.line("warn", "Broken: Lima cannot say whether it runs") default: d.line("info", "stopped") } diff --git a/internal/cli/doctor_test.go b/internal/cli/doctor_test.go index 94648b60..91327314 100644 --- a/internal/cli/doctor_test.go +++ b/internal/cli/doctor_test.go @@ -11,6 +11,7 @@ import ( "github.com/sylvinus/agent-vm/internal/paths" "github.com/sylvinus/agent-vm/internal/state" + "github.com/sylvinus/agent-vm/internal/vm" "github.com/sylvinus/agent-vm/internal/vmname" ) @@ -83,6 +84,16 @@ func TestDoctor(t *testing.T) { if o = out(); !strings.Contains(o, " warn running with a share the host does not confine: ") || !strings.Contains(o, `the mount type is "9p"`) { t.Errorf("9p:\n%s", o) } + // Broken, orphaned or not: never "stopped". + v.Status = vm.Broken + if o = out(); !strings.Contains(o, " warn Broken: Lima cannot say whether it runs") { + t.Errorf("Broken:\n%s", o) + } + v.Orphaned = true + if o = out(); !strings.Contains(o, " warn running without Lima's host agent, cut off") { + t.Errorf("orphaned:\n%s", o) + } + v.Status, v.Orphaned = vm.Running, false if strings.Contains(se.fake.CallLog(), "edit") || strings.Contains(se.fake.CallLog(), "start") || strings.Contains(se.fake.CallLog(), "stop") { t.Errorf("doctor changed the VM:\n%s", se.fake.CallLog()) } diff --git a/internal/cli/e2e_test.go b/internal/cli/e2e_test.go index 4b7e55ae..0e244410 100644 --- a/internal/cli/e2e_test.go +++ b/internal/cli/e2e_test.go @@ -9,11 +9,17 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "strings" + "syscall" "testing" "time" "github.com/lima-vm/lima/v2/pkg/instance" + + "github.com/sylvinus/agent-vm/internal/state" + "github.com/sylvinus/agent-vm/internal/vm" + "github.com/sylvinus/agent-vm/internal/vmname" ) // agent-vm as a user runs it, on real VMs: setup, then a command in a @@ -93,6 +99,9 @@ func TestE2E(t *testing.T) { if b, _ := os.ReadFile(filepath.Join(proj, "from-vm")); string(b) != "hi\na\n" { t.Errorf("on the host: %q", b) } + // The run that started the VM is over and its context cancelled: the VM + // runs on (Lima once killed its hostagent then). + time.Sleep(2 * time.Second) if code, out := run(proj, "info"); code != 0 || !strings.Contains(out, "vm_running=1") || !strings.Contains(out, "git_protected=1") { t.Errorf("info: %s", out) } @@ -113,7 +122,57 @@ func TestE2E(t *testing.T) { if code != 0 || !strings.Contains(out, "the project share is not mounted") || !strings.Contains(out, "v2, longer") { t.Errorf("lost share: %d %s", code, out) } + // The hostagent killed under the VM: with QEMU, the VM runs on, cut off. + // The next run stops it and starts it again; so does stop. With vz, the + // VM goes with the hostagent. + st := state.Dir(filepath.Join(home, ".agent-vm")) + b, err := vm.NewLima(LimaHome(st), string(st)) + if err != nil { + t.Fatal(err) + } + name := vmname.Name(proj) + orphan := func() (driver int) { + t.Helper() + inst, err := b.Get(ctx, name) + if err != nil || inst.Status != vm.Running { + t.Fatalf("before the kill: %+v %v", inst, err) + } + pid := func(f string) int { + b, _ := os.ReadFile(filepath.Join(inst.Dir, f)) + n, _ := strconv.Atoi(strings.TrimSpace(string(b))) + return n + } + driver = pid(inst.VMType + ".pid") + if err := syscall.Kill(pid("ha.pid"), syscall.SIGKILL); err != nil { + t.Fatal(err) + } + // Broken without more until the hostagent is reaped. + for range 100 { + if inst, err = b.Get(ctx, name); err == nil && (inst.Orphaned || inst.Status == vm.Stopped) { + break + } + time.Sleep(100 * time.Millisecond) + } + if !inst.Orphaned { + t.Logf("not orphaned by the kill: %s", inst.Status) + return 0 + } + return driver + } + orphaned := orphan() != 0 + if code, out := run(proj, "run", "true"); code != 0 || orphaned != strings.Contains(out, "left running without Lima's host agent") { + t.Errorf("run on an orphaned VM: %d %s", code, out) + } + driver := orphan() if code, out := run(proj, "stop"); code != 0 || !strings.Contains(out, "VM stopped.") { t.Errorf("stop: %s", out) } + // Killed, then reaped by init. + for i := 0; driver != 0 && syscall.Kill(driver, 0) == nil; i++ { + if i == 50 { + t.Errorf("the orphaned driver %d still runs", driver) + break + } + time.Sleep(100 * time.Millisecond) + } } diff --git a/internal/cli/start.go b/internal/cli/start.go index a2b9e264..095ae5bd 100644 --- a/internal/cli/start.go +++ b/internal/cli/start.go @@ -416,6 +416,19 @@ func (s *start) ensureRunningLocked() error { if !o.Reset && s.configErr(inst) { return errStop } + // Lima's hostagent died under it: the VM is cut off, and blocks its next + // start. + if inst != nil && inst.Orphaned { + s.warnf("Warning: VM '%s' was left running without Lima's host agent, cut off: stopping it.", s.name) + if !s.stopVM(s.ctx, s.name) { + s.warnf("Error: could not stop VM '%s'. See 'agent-vm list'.", s.name) + return errStop + } + if inst, err = s.get(s.name); err != nil { + s.warnf("Error: could not query Lima: %v", err) + return errStop + } + } if o.Reset && inst != nil { fmt.Fprintf(s.io.Stdout, "Resetting VM '%s'...\n", s.name) if !s.deleteVM(s.ctx, s.name) { diff --git a/internal/cli/start_test.go b/internal/cli/start_test.go index d0f5c8fc..b65a8eb0 100644 --- a/internal/cli/start_test.go +++ b/internal/cli/start_test.go @@ -143,6 +143,29 @@ func TestStartNewVM(t *testing.T) { } } +// A VM left running without Lima's hostagent is stopped by force, then +// started again. +func TestStartOrphaned(t *testing.T) { + se := newStartEnv(t) + se.base() + name := vmname.Name(se.proj) + if code := se.run("run", "true"); code != 0 { + t.Fatalf("run: %d %s", code, se.out()) + } + v := se.fake.VMs[name] + v.Status, v.Orphaned = vm.Broken, true + if se.run("run", "true") != 0 || !strings.Contains(se.out(), "left running without Lima's host agent") || + !strings.HasPrefix(se.fake.CallLog(), "stop "+name) || !strings.Contains(se.fake.CallLog(), "start "+name) || v.Status != vm.Running { + t.Errorf("orphaned: %s\n%s", se.out(), se.fake.CallLog()) + } + // One that does not stop: no start. + v.Status, v.Orphaned = vm.Broken, true + se.fake.Sticky[name] = true + if se.run("run", "true") != 1 || !strings.Contains(se.out(), "could not stop VM") || strings.Contains(se.fake.CallLog(), "start") { + t.Errorf("orphaned, stop failed: %s\n%s", se.out(), se.fake.CallLog()) + } +} + // The resources asked against the VM's: in bytes, the disk one way (it // only grows), a VM that cannot be read counts as a change. func TestResourcesDiffer(t *testing.T) { diff --git a/internal/cli/vms.go b/internal/cli/vms.go index f4af937f..2e811c78 100644 --- a/internal/cli/vms.go +++ b/internal/cli/vms.go @@ -75,7 +75,7 @@ func (e *app) stopVM(ctx context.Context, name string) bool { } _ = b.Stop(ctx, name) inst, err := b.Get(ctx, name) - return err == nil && inst.Status != vm.Running + return err == nil && inst.Status == vm.Stopped } func (e *app) rmCmd(ctx context.Context, args []string) int { diff --git a/internal/cli/vms_test.go b/internal/cli/vms_test.go index 1c078ab7..b97358f3 100644 --- a/internal/cli/vms_test.go +++ b/internal/cli/vms_test.go @@ -92,6 +92,18 @@ func TestStopRm(t *testing.T) { if te.run("stop", "proj-deadbeef") != 1 || !strings.Contains(te.out(), "is still running") { t.Errorf("stop that did not take: %s", te.out()) } + // A Broken VM is not stopped: said so. An orphaned one is stopped by force. + te.fake.Sticky = map[string]bool{} + te.fake.VMs["proj-deadbeef"].Status = vm.Broken + if te.run("stop", "proj-deadbeef") != 1 || !strings.Contains(te.out(), "is still running") { + t.Errorf("stop of a Broken VM: %s", te.out()) + } + te.fake.VMs["proj-deadbeef"].Orphaned = true + if te.run("stop", "proj-deadbeef") != 0 || !strings.HasSuffix(te.out(), "VM stopped.\n") || te.fake.VMs["proj-deadbeef"].Status != vm.Stopped { + t.Errorf("stop of an orphaned VM: %s", te.out()) + } + te.fake.VMs["proj-deadbeef"].Status = vm.Running + te.fake.Sticky["proj-deadbeef"] = true // Nor a delete that did not take. os.WriteFile(te.state.Marker(state.VersionOf, "proj-deadbeef"), []byte("1\n"), 0o644) if te.run("rm", "proj-deadbeef") != 1 || !strings.Contains(te.out(), "could not delete VM 'proj-deadbeef'") || diff --git a/internal/host/host_test.go b/internal/host/host_test.go index 544f07fa..dc75fd9c 100644 --- a/internal/host/host_test.go +++ b/internal/host/host_test.go @@ -47,3 +47,22 @@ func TestCapBash(t *testing.T) { t.Error("an unknown host clamped") } } + +// QEMU's package by the host's package manager, as the .deb and .rpm +// recommend it. +func TestQemuBinaryFor(t *testing.T) { + for _, c := range []struct { + goarch string + dnf bool + bin, cmd string + }{ + {"amd64", false, "qemu-system-x86_64", "sudo apt-get install qemu-system-x86"}, + {"arm64", false, "qemu-system-aarch64", "sudo apt-get install qemu-system-arm"}, + {"amd64", true, "qemu-system-x86_64", "sudo dnf install qemu-system-x86-core"}, + {"arm64", true, "qemu-system-aarch64", "sudo dnf install qemu-system-aarch64-core"}, + } { + if bin, cmd := qemuBinaryFor(c.goarch, c.dnf); bin != c.bin || cmd != c.cmd { + t.Errorf("%s dnf=%v: %s %q", c.goarch, c.dnf, bin, cmd) + } + } +} diff --git a/internal/host/prereqs.go b/internal/host/prereqs.go index 131a9c32..2c43cd7a 100644 --- a/internal/host/prereqs.go +++ b/internal/host/prereqs.go @@ -13,15 +13,26 @@ import ( ) // qemuBinary is QEMU's emulator for this host's architecture, and the -// Debian package that has it. -func qemuBinary() (bin, pkg string) { - switch runtime.GOARCH { +// command that installs it: dnf's package where there is dnf and no apt-get, +// else Debian's. +func qemuBinary() (bin, install string) { + _, aptErr := exec.LookPath("apt-get") + _, dnfErr := exec.LookPath("dnf") + return qemuBinaryFor(runtime.GOARCH, aptErr != nil && dnfErr == nil) +} + +func qemuBinaryFor(goarch string, dnf bool) (bin, install string) { + bin, deb, rpm := "qemu-system-"+goarch, "qemu-system", "qemu-system-"+goarch+"-core" + switch goarch { case "amd64": - return "qemu-system-x86_64", "qemu-system-x86" + bin, deb, rpm = "qemu-system-x86_64", "qemu-system-x86", "qemu-system-x86-core" case "arm64": - return "qemu-system-aarch64", "qemu-system-arm" + bin, deb, rpm = "qemu-system-aarch64", "qemu-system-arm", "qemu-system-aarch64-core" + } + if dnf { + return bin, "sudo dnf install " + rpm } - return "qemu-system-" + runtime.GOARCH, "qemu-system" + return bin, "sudo apt-get install " + deb } // WSLVersion is 2 or 1 under WSL, from the kernel release, 0 elsewhere. @@ -47,9 +58,9 @@ func LinuxPrereqs(w io.Writer) bool { return true } ok := true - bin, pkg := qemuBinary() + bin, install := qemuBinary() if _, err := exec.LookPath(bin); err != nil { - fmt.Fprintf(w, "Error: Lima needs '%s' on PATH (not found).\n Install with: sudo apt-get install %s\n", bin, pkg) + fmt.Fprintf(w, "Error: Lima needs '%s' on PATH (not found).\n Install with: %s\n", bin, install) ok = false } _, err := os.Stat("/dev/kvm") diff --git a/internal/vm/lima.go b/internal/vm/lima.go index 6e0bc61c..d60c5387 100644 --- a/internal/vm/lima.go +++ b/internal/vm/lima.go @@ -222,6 +222,9 @@ func fromLima(inst *limatype.Instance) *Instance { out.Status = Broken out.ConfigErr = errors.Join(inst.Errors...) } + // "driver is running but host agent is not". Not the other way around: a + // hostagent without its driver is also one booting the VM. + out.Orphaned = out.Status == Broken && inst.Config != nil && inst.HostAgentPID == 0 && inst.DriverPID > 0 if c := inst.Config; c != nil { if c.SSH.LocalPort != nil { out.SSHPortConfig = *c.SSH.LocalPort @@ -505,12 +508,18 @@ func (l *Lima) Stop(ctx context.Context, name string) error { if err != nil { return err } - if inst.Status != limatype.StatusRunning { - return nil + switch { + case inst.Status == limatype.StatusRunning: + return withLog(io.Discard, func() error { + return errors.Join(instance.StopGracefully(ctx, inst, false), reconcile.Reconcile(ctx, "")) + }) + case fromLima(inst).Orphaned: + return withLog(io.Discard, func() error { + instance.StopForcibly(inst) + return reconcile.Reconcile(ctx, "") + }) } - return withLog(io.Discard, func() error { - return errors.Join(instance.StopGracefully(ctx, inst, false), reconcile.Reconcile(ctx, "")) - }) + return nil } func (l *Lima) Delete(ctx context.Context, name string) error { diff --git a/internal/vm/lima_test.go b/internal/vm/lima_test.go index 87283e13..3a0779b9 100644 --- a/internal/vm/lima_test.go +++ b/internal/vm/lima_test.go @@ -5,15 +5,20 @@ import ( "errors" "io" "os" + "os/exec" "path/filepath" "reflect" + "runtime" + "strconv" "strings" "sync" "testing" + "time" "github.com/lima-vm/lima/v2/pkg/guestagent/api" "github.com/lima-vm/lima/v2/pkg/hostagent/events" "github.com/lima-vm/lima/v2/pkg/limatype" + "github.com/lima-vm/lima/v2/pkg/limatype/filenames" "github.com/lima-vm/lima/v2/pkg/limayaml" "github.com/lima-vm/lima/v2/pkg/portfwd" @@ -291,6 +296,61 @@ func TestSharesKeptWindows(t *testing.T) { } } +// A driver running without its hostagent: orphaned, and Stop kills it. A +// hostagent without its driver is not: it may be booting the VM. +func TestLimaStopOrphaned(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("no sleep to stand for the driver") + } + ctx := context.Background() + l := newTestLima(t) + none := []Mount{} + if err := l.Create(ctx, "o", "template:debian-13", Settings{Mounts: &none}, io.Discard); err != nil { + t.Fatal(err) + } + inst, err := l.Get(ctx, "o") + if err != nil { + t.Fatal(err) + } + driver := exec.Command("sleep", "60") + if err := driver.Start(); err != nil { + t.Fatal(err) + } + exited := make(chan struct{}) + go func() { driver.Wait(); close(exited) }() + t.Cleanup(func() { driver.Process.Kill(); <-exited }) + pid := []byte(strconv.Itoa(driver.Process.Pid)) + + ha := filepath.Join(inst.Dir, filenames.HostAgentPID) + os.WriteFile(ha, pid, 0o644) + if got, _ := l.Get(ctx, "o"); got.Status != Broken || got.Orphaned { + t.Fatalf("a hostagent without its driver: %+v", got) + } + l.Stop(ctx, "o") + select { + case <-exited: + t.Fatal("Stop killed a hostagent without its driver") + case <-time.After(200 * time.Millisecond): + } + os.Remove(ha) + + os.WriteFile(filepath.Join(inst.Dir, filenames.PIDFile(inst.VMType)), pid, 0o644) + if got, _ := l.Get(ctx, "o"); got.Status != Broken || !got.Orphaned { + t.Fatalf("a driver without its hostagent: %+v", got) + } + if err := l.Stop(ctx, "o"); err != nil { + t.Fatal(err) + } + select { + case <-exited: + case <-time.After(10 * time.Second): + t.Fatal("the driver still runs") + } + if got, _ := l.Get(ctx, "o"); got.Status != Stopped || got.Orphaned { + t.Errorf("after Stop: %+v", got) + } +} + func TestConfined(t *testing.T) { b := &SSHFS{SFTPDriver: "builtin"} for _, c := range []struct { diff --git a/internal/vm/vm.go b/internal/vm/vm.go index 8104ac7b..03342ae8 100644 --- a/internal/vm/vm.go +++ b/internal/vm/vm.go @@ -61,6 +61,10 @@ type Instance struct { Mounts []Mount // ConfigErr is why the backend cannot read the VM's config (Broken). ConfigErr error + // Orphaned: Broken, the VM still running without the backend's process + // that serves it (Lima's hostagent). Nothing reaches it; Stop ends it by + // force. + Orphaned bool } // Mount is a host folder shared with the guest. @@ -140,7 +144,8 @@ type Backend interface { // gets the backend's progress. It refuses a VM whose shares are not // confined to their folders (see Confined). Start(ctx context.Context, name string, log io.Writer) error - // Stop shuts down the running VM name. + // Stop shuts down the running VM name, and kills an orphaned one. Any + // other VM is left as it is. Stop(ctx context.Context, name string) error // Delete removes the VM name, running or not. Delete(ctx context.Context, name string) error diff --git a/internal/vm/vmtest/fake.go b/internal/vm/vmtest/fake.go index 92f21f02..36ec3427 100644 --- a/internal/vm/vmtest/fake.go +++ b/internal/vm/vmtest/fake.go @@ -177,8 +177,8 @@ func (f *Fake) Stop(_ context.Context, name string) error { if err := f.call("stop %s", name); err != nil { return err } - if v, ok := f.VMs[name]; ok && !f.Sticky[name] { - v.Status = vm.Stopped + if v, ok := f.VMs[name]; ok && !f.Sticky[name] && (v.Status == vm.Running || v.Orphaned) { + v.Status, v.Orphaned = vm.Stopped, false } return nil } diff --git a/packaging/nfpm.yaml b/packaging/nfpm.yaml new file mode 100644 index 00000000..71c74548 --- /dev/null +++ b/packaging/nfpm.yaml @@ -0,0 +1,48 @@ +# The .deb and .rpm of a Linux build, made by release.sh (linux_packages) with +# nfpm, run in the folder of that build's tarball. The variables come from +# release.sh. QEMU and ssh are recommended, not required: apt and dnf install +# recommends by default; RHEL and its rebuilds name QEMU qemu-kvm, and a QEMU +# from elsewhere on the PATH does too. `agent-vm setup` says what is missing. +name: agent-vm +arch: ${GOARCH} +platform: linux +version: ${VERSION} +release: "1" +section: utils +priority: optional +maintainer: Sylvain Zimmer +description: | + Run AI coding agents in a disposable Linux VM per project. + The VM gets the project directory and nothing else of the host's. +homepage: https://www.agent-vm.org/ +license: MIT + +contents: + - src: agent-vm + dst: /usr/bin/agent-vm + file_info: + mode: 0755 + - src: README.md + dst: /usr/share/doc/agent-vm/README.md + - src: CHANGELOG.md + dst: /usr/share/doc/agent-vm/CHANGELOG.md + - src: LICENSE + dst: /usr/share/doc/agent-vm/LICENSE + - src: NOTICE + dst: /usr/share/doc/agent-vm/NOTICE + - src: THIRD_PARTY_LICENSES + dst: /usr/share/doc/agent-vm/THIRD_PARTY_LICENSES + - src: runtime.example.sh + dst: /usr/share/doc/agent-vm/runtime.example.sh + +overrides: + deb: + recommends: + - ${QEMU_DEB} + - qemu-utils + - openssh-client + rpm: + recommends: + - ${QEMU_RPM} + - qemu-img + - openssh-clients diff --git a/patches/README.md b/patches/README.md index 5b7abe01..3401c8e8 100644 --- a/patches/README.md +++ b/patches/README.md @@ -34,6 +34,7 @@ It runs inside the Lima hostagent, which with vz also runs the VM: a panic there | 18 | When the first sshfs mount fails, Lima retries with `-o nonempty` in place of the share's options. | The share is mounted again without `cache=no` (sshfs's attribute cache back on). | lima 0003 | | 19 | The host's event for a removal the guest asked for is relayed back as a host deletion. | The guest's next removal of a file it created again is taken for the relayed one and not done: the file stays on the host. | sshocker 0009 | | 20 | The client sets any mode bit, setuid, setgid and sticky included. | A file the host runs carries the privileges of its owner. | sshocker 0006 | +| 21 | `instance.StartWithPaths` starts the hostagent with `exec.CommandContext`: os/exec kills it when the caller's context ends, also after Start returned. | The end of the agent-vm command that started a VM kills its hostagent: QEMU runs on without it (Broken, every later start fails), vz VMs die. | lima 0006 | ## The patches @@ -44,6 +45,7 @@ It runs inside the Lima hostagent, which with vz also runs the VM: a panic there | `lima/0003-hostagent-keep-sshfs-options-on-retry` | lima-vm/lima | not sent | Bug 18: the retry adds `nonempty` to the options. No test: it needs a guest whose first mount fails. | | `lima/0004-portfwd-not-next-to-a-host-program` | lima-vm/lima | not sent | A guest port is forwarded only when no host program uses it: with `SO_REUSEADDR`, a listener on 127.0.0.1:P is allowed next to a host program's 0.0.0.0:P on macOS and BSD, and takes its local connections. The port is tried on 127.0.0.1, ::1, 0.0.0.0 and :: first (with `SO_EXCLUSIVEADDRUSE` on Windows); one in use gives a `failed` port-forward event. Tests `TestForwardNotNextToHostProgram`, `TestForwardFreePort`. | | `lima/0005-network-isolation-hooks` | lima-vm/lima | not sent | Hooks for agent-vm's network policy: `usernet.Outbound` and `usernet.AllowHostService` (the hostagent's DNS ports), `dns.Names`/`dns.Answered` (a name refused is NXDOMAIN, never looked up). QEMU gets its own netstack per instance, as vz has, so the policy applies to both, and its guest's DNS is the gateway's. QEMU is handed the connected socket, or on Windows, which cannot pass it, connects itself (`stream` netdev, QEMU 7.2 or later). `usernet.Names` and `usernet.Answered` reach the gateway's DNS server through gvisor-tap-vsock 0002, and a host service is allowed for its network and port on 127.0.0.1 only. Tests `TestNamesAnswered`, `TestOutboundHostServices`; the rest is covered by agent-vm's e2e (`internal/vm/lima_e2e_test.go`). | +| `lima/0006-instance-hostagent-outlives-start-context` | lima-vm/lima | not sent; the issue to open is the patch's header | Bug 21: `exec.Command`. No Lima test (its unit tests start no hostagent): agent-vm's e2e (`internal/cli/e2e_test.go`) fails without it. The header also reports the QEMU driver's "send on closed channel" panic that follows, not fixed here. | | `sshocker/0001-rooted-server-readonly-names` | lima-vm/sshocker | fork branch `readonly-names` | The builtin server serves only the mounted directory, and `ReadonlyNames`. | | `sshocker/0002-expect-remove` | lima-vm/sshocker | fork branch `expect-remove` | `ExpectRemove`, for 0002 of Lima. | | `sshocker/0003-append-fix` | lima-vm/sshocker | not sent | Bug 1. Tests `TestRootedOpenAppend`, `TestRootedOpenMode`. No pkg/sftp dependency: send first. | diff --git a/patches/lima/0006-instance-hostagent-outlives-start-context.patch b/patches/lima/0006-instance-hostagent-outlives-start-context.patch new file mode 100644 index 00000000..a62fd053 --- /dev/null +++ b/patches/lima/0006-instance-hostagent-outlives-start-context.patch @@ -0,0 +1,76 @@ +Subject: [PATCH] instance: do not kill the hostagent when Start's context ends + +StartWithPaths starts the hostagent with exec.CommandContext(ctx, ...). +The hostagent is meant to outlive the call ("leave the hostagent process +running" when Start returns), but os/exec kills it as soon as ctx is +done, also long after Start returned. + +limactl does not show it: its process exits right after Start, before +anything cancels the context, and a Ctrl-C kills limactl without +cancelling it. A Go program that uses pkg/instance does: it cancels the +context it passed (a deferred cancel, a signal handler, a request scope) +while it keeps running, and the hostagent is killed then. When the +cancel comes just before the program exits, os/exec's watcher races the +exit: the instance breaks sometimes, depending on the machine. + +What follows depends on the driver: + +- qemu: QEMU is a child of the hostagent, not killed with it. It keeps + running, reparented to init, unreachable (no hostagent, so no SSH + forward or usernet), and holds the lock on its PID file. The instance + is Broken: "qemu driver is running but host agent is not". The next + start fails: QEMU exits at once ("cannot create PID file: Cannot lock + pid file"), the QEMU driver panics (below), and limactl reports "host + agent process has exited: exit status 2". `limactl stop -f` recovers. +- vz: the VM runs in the hostagent, so it is killed with it, without a + shutdown (not tested). + +Reproduce, from a Go program, on Linux with the qemu driver: + + ctx, cancel := context.WithCancel(context.Background()) + err := instance.Start(ctx, inst, false, false) // nil, instance Running + cancel() + time.Sleep(time.Second) + // store.Inspect: Broken; `ps -o ppid= -C qemu-system-x86_64`: 1 + +Found in agent-vm, which embeds Lima: on Linux hosts its VMs became +Broken a few seconds to minutes after a start, when the command that +started them ended (closed terminal, SIGTERM, or a plain exit losing the +race). Tested there: with this patch the VM stays Running after the +starting program ends, cancelled context included, and the next start +works. + +The fix: exec.Command. ctx still bounds the wait for the hostagent to +come up (waitHostAgentStart, watchHostAgentEvents). A start cancelled +during that wait leaves the hostagent running, as an interrupted limactl +start does (the hostagent has its own process group, +BackgroundSysProcAttr, and the terminal's Ctrl-C does not reach it). + +No unit test: Lima's unit tests never start a hostagent. agent-vm's e2e +test (internal/cli/e2e_test.go) fails without it. + +Separate bug, to report on its own: the panic of the QEMU driver, +"panic: send on closed channel" in LimaQemuDriver.Start.func5 +(pkg/driver/qemu/qemu_driver.go). When QEMU exits early, the goroutine +waiting on it closes qWaitCh and cancels the driver's context; the +ConfigureDriver goroutine then fails on that context and sends its +error on the closed qWaitCh. Already seen in #4645 (closed on its +Colima-specific part, the panic itself left unfixed). Sending with a +select on a done channel, or one closer for both goroutines, fixes it. + +Assisted-by: Claude Opus 5.5 +--- +diff --git a/pkg/instance/start.go b/pkg/instance/start.go +--- a/pkg/instance/start.go ++++ b/pkg/instance/start.go +@@ -291,7 +291,9 @@ + args = append(args, "--progress") + } + args = append(args, inst.Name) +- haCmd = exec.CommandContext(ctx, limactl, args...) ++ // Not exec.CommandContext: the hostagent outlives this call, and os/exec ++ // would kill it whenever ctx ends, even after Start returned. ++ haCmd = exec.Command(limactl, args...) + + haCmd.SysProcAttr = executil.BackgroundSysProcAttr + diff --git a/release.sh b/release.sh index 42764026..8951affb 100755 --- a/release.sh +++ b/release.sh @@ -30,7 +30,11 @@ # SHA256SUMS. Each tarball holds that platform's binary, agent-vm, built # here from the tag, agent-vm.sh, and LICENSE, NOTICE and # THIRD_PARTY_LICENSES: the curl installer takes the one of the -# machine, unpacks it and runs `agent-vm.sh install`. The release is +# machine, unpacks it and runs `agent-vm.sh install`. The Linux builds +# also come as agent-vm_X.Y.Z_.deb and agent-vm-X.Y.Z-1..rpm +# (packaging/nfpm.yaml; nfpm is built with Go at a pinned version, or +# AGENT_VM_NFPM names a binary), listed in SHA256SUMS after the tarballs. +# The release is # created as a draft, its assets are downloaded back and compared with # the local ones, and only then is it published; then the published # assets are downloaded once more and checked against SHA256SUMS (and @@ -153,9 +157,45 @@ package() { "$src/README.md" "$src/CHANGELOG.md" "$src/runtime.example.sh" "$stage/" [ "$os" != windows ] || [ -z "${AGENT_VM_WINDOWS_PFX:-}" ] || sign_windows "$stage/$bin" "$out" make_tarball "$out/agent-vm-$version-$p.tar.gz" "$out/stage-$p" "agent-vm-$version" + [ "$os" != linux ] || linux_packages "$ref" "$version" "$out" "$arch" "$stage" done } +# nfpm, which makes the .deb and .rpm: AGENT_VM_NFPM names a binary, else the +# pinned version is built with Go (once, into $WORK/bin). +NFPM_VERSION=v2.48.0 +nfpm_bin() { + if [ -n "${AGENT_VM_NFPM:-}" ]; then echo "$AGENT_VM_NFPM"; return; fi + local bin="$WORK/bin/nfpm" + if [ ! -x "$bin" ]; then + ( cd "$WORK" && GOWORK=off GOBIN="$WORK/bin" go install "github.com/goreleaser/nfpm/v2/cmd/nfpm@$NFPM_VERSION" ) >"$WORK/nfpm-build.log" 2>&1 \ + || { tail -n 20 "$WORK/nfpm-build.log" >&2; die "could not build nfpm $NFPM_VERSION (or set AGENT_VM_NFPM)"; } + fi + echo "$bin" +} + +# linux_packages : in outdir, +# agent-vm__.deb and agent-vm--1..rpm, from +# packaging/nfpm.yaml and the files of the tarball's folder. The dates inside +# are the commit's, so the same commit gives the same packages. +linux_packages() { + local ref="$1" version="$2" out="$3" goarch="$4" stage="$5" nfpm rpmarch qdeb qrpm + case "$goarch" in + amd64) rpmarch=x86_64; qdeb=qemu-system-x86; qrpm=qemu-system-x86-core ;; + arm64) rpmarch=aarch64; qdeb=qemu-system-arm; qrpm=qemu-system-aarch64-core ;; + *) die "no .deb or .rpm for $goarch" ;; + esac + nfpm="$(nfpm_bin)" + echo " packaging linux-$goarch (.deb, .rpm)" + local vars=(GOARCH="$goarch" VERSION="$version" QEMU_DEB="$qdeb" QEMU_RPM="$qrpm" + SOURCE_DATE_EPOCH="$(git -C "$REPO_DIR" log -1 --format=%ct "$ref")") + ( cd "$stage" \ + && env "${vars[@]}" "$nfpm" package -f "$out/src/packaging/nfpm.yaml" -p deb -t "$out/agent-vm_${version}_${goarch}.deb" \ + && env "${vars[@]}" "$nfpm" package -f "$out/src/packaging/nfpm.yaml" -p rpm -t "$out/agent-vm-${version}-1.${rpmarch}.rpm" ) \ + >"$out/nfpm-$goarch.log" 2>&1 || { tail -n 20 "$out/nfpm-$goarch.log" >&2; die "packaging linux-$goarch failed: $out/nfpm-$goarch.log"; } + printf '%s\n' "agent-vm_${version}_${goarch}.deb" "agent-vm-${version}-1.${rpmarch}.rpm" >> "$out/packages.list" +} + # make_tarball : of as a tarball owned by # root (numeric ids), with no extended attributes and no AppleDouble files # (macOS's bsdtar adds both), and a gzip header without a date. GNU tar and @@ -412,6 +452,15 @@ for f in LICENSE NOTICE THIRD_PARTY_LICENSES; do done ok "LICENSE, NOTICE and THIRD_PARTY_LICENSES are there" +# flake.nix's vendorHash is of the Go dependencies: it notes the checksum of +# go.mod and go.sum it was made for. +if [ -f flake.nix ]; then + mods="$(cat go.mod go.sum | sha256_of /dev/stdin)" + grep -q "go.mod+go.sum: $mods\$" flake.nix \ + || die "flake.nix's vendorHash is not for this go.mod and go.sum: update it and its 'go.mod+go.sum: $mods' line (see the comment there)" + ok "flake.nix's vendorHash is for this go.mod and go.sum" +fi + # --- 3. tests ------------------------------------------------------------------ # CI is the authority: it runs on Linux, macOS and Windows. # --bypass-checks skips the workflow only: the checks above decide what the @@ -490,6 +539,16 @@ for p in $PLATFORMS; do assets+=("$WORK/$f") ok "$f: $(du -h "$WORK/$f" | cut -f1), sha256 $sha" done +# The Linux packages, after the tarballs: the installer reads only the +# tarballs' lines of SHA256SUMS. +if [ -f "$WORK/packages.list" ]; then + while read -r f; do + sha="$(sha256_of "$WORK/$f")" + printf '%s %s\n' "$sha" "$f" >> "$WORK/SHA256SUMS" + assets+=("$WORK/$f") + ok "$f: $(du -h "$WORK/$f" | cut -f1), sha256 $sha" + done < "$WORK/packages.list" +fi if [ -n "${AGENT_VM_MINISIGN_KEY:-}" ]; then # Prompts for the key's password, on the terminal. if [ -z "$DRY_RUN" ]; then @@ -529,6 +588,7 @@ SMOKE_FAILED="" echo echo "Homebrew formula, per platform (each tarball's agent-vm is the binary):" while read -r sha f; do + case "$f" in *.tar.gz) ;; *) continue ;; esac echo " url \"$BASE/$f\"" echo " sha256 \"$sha\"" done < "$WORK/SHA256SUMS" @@ -545,7 +605,7 @@ if [ -n "${AGENT_VM_TAP:-}" ]; then # sha256 after it that tarball's. A `bottle do` block has sha256 lines # of its own, after no url: left alone. awk -v base="$BASE" -v version="$VERSION" -v sums="$WORK/SHA256SUMS" ' - BEGIN { while ((getline l < sums) > 0) { split(l, a, " "); p = a[2]; sub(/^agent-vm-[0-9.]+-/, "", p); sub(/\.tar\.gz$/, "", p); sha[p] = a[1] } } + BEGIN { while ((getline l < sums) > 0) { split(l, a, " "); p = a[2]; if (p !~ /\.tar\.gz$/) continue; sub(/^agent-vm-[0-9.]+-/, "", p); sub(/\.tar\.gz$/, "", p); sha[p] = a[1] } } /^[[:space:]]*url / { for (p in sha) if (index($0, "-" p ".tar.gz")) { sub(/url .*/, "url \"" base "/agent-vm-" version "-" p ".tar.gz\""); want = p diff --git a/tests/release.sh b/tests/release.sh index 3841e536..e2979226 100755 --- a/tests/release.sh +++ b/tests/release.sh @@ -75,7 +75,7 @@ RR="$SB/repo"; RO="$SB/origin.git"; RB="$SB/bin" mkdir -p "$RR/www/public" "$RB" cp -R "$SELF_DIR/release.sh" "$SELF_DIR/agent-vm.sh" "$SELF_DIR/agent-vm.setup.sh" "$SELF_DIR/runtime.example.sh" \ "$SELF_DIR/LICENSE" "$SELF_DIR/NOTICE" "$SELF_DIR/THIRD_PARTY_LICENSES" "$SELF_DIR/README.md" \ - "$SELF_DIR/tests" "$SELF_DIR/scripts" "$RR/" + "$SELF_DIR/tests" "$SELF_DIR/scripts" "$SELF_DIR/packaging" "$RR/" cp "$SELF_DIR/www/public/install.sh" "$RR/www/public/" printf 'NEXT_VERSION := %s\n' "$V" > "$RR/Makefile" printf '# Changelog\n\n## %s\n\n- notes\n' "$V" > "$RR/CHANGELOG.md" @@ -95,7 +95,7 @@ case "\$1 \$2" in "release create") echo "\$*" > "$SB/gh-create.log" mkdir -p "$SB/assets" - for a in "\$@"; do case "\$a" in *.tar.gz|*SHA256SUMS|*.minisig) cp "\$a" "$SB/assets/" ;; esac; done + for a in "\$@"; do case "\$a" in *.tar.gz|*.deb|*.rpm|*SHA256SUMS|*.minisig) cp "\$a" "$SB/assets/" ;; esac; done touch "$SB/draft" ;; "release download") dir="" @@ -156,7 +156,17 @@ case "\$*" in else echo "\$os-\$arch" > "\$2/_output/bin/agent-vm.exe"; fi ;; esac STUB -chmod +x "$RB/gh" "$RB/make" "$RB/curl" +# nfpm writes what it was told to (-t), holding the variables release.sh sets +# and the files it was run next to. +cat > "$RB/nfpm" <&2; exit 1; } +[ -f agent-vm ] || { echo "nfpm stub: no agent-vm in \$(pwd)" >&2; exit 1; } +echo "\$fmt \$GOARCH \$VERSION \$QEMU_DEB \$QEMU_RPM \$SOURCE_DATE_EPOCH" > "\$out" +STUB +chmod +x "$RB/gh" "$RB/make" "$RB/curl" "$RB/nfpm" +export AGENT_VM_NFPM="$RB/nfpm" ( cd "$RR" && git init -q -b main && git add -A && git commit -qm r \ && git init -q --bare "$RO" && git remote add origin "$RO" && git push -q origin main ) >/dev/null 2>&1 @@ -188,6 +198,18 @@ out="$(relrun)"; rc=$? case "$rc:$out" in 1:*"NEXT_VERSION is '9.9.8'"*) pass "a Makefile on another version is refused" ;; *) fail "other version: $out" ;; esac ( cd "$RR" && printf 'NEXT_VERSION := %s\n' "$V" > Makefile && git commit -qam v && git push -q origin main ) >/dev/null 2>&1 +# flake.nix names the checksum of go.mod and go.sum its vendorHash is for. +mods="$(cat "$SELF_DIR/go.mod" "$SELF_DIR/go.sum" | sha256_of /dev/stdin)" +check "flake.nix: its note is the checksum release.sh computes" "$(grep -c "go.mod+go.sum: $mods\$" "$SELF_DIR/flake.nix")" "1" +( cd "$RR" && cp "$SELF_DIR/go.mod" "$SELF_DIR/go.sum" . && printf '# go.mod+go.sum: %s\n' "$mods" > flake.nix \ + && git add -A && git commit -qm n && git push -q origin main ) >/dev/null 2>&1 +out="$(relrun)"; rc=$? +case "$rc:$out" in 0:*"vendorHash is for this go.mod and go.sum"*) pass "a flake.nix for these Go dependencies passes" ;; *) fail "flake.nix: $out" ;; esac +( cd "$RR" && printf '\n// changed\n' >> go.mod && git commit -qam d && git push -q origin main ) >/dev/null 2>&1 +out="$(relrun)"; rc=$? +case "$rc:$out" in 1:*"flake.nix's vendorHash is not for this go.mod and go.sum"*) pass "a flake.nix for other Go dependencies is refused" ;; *) fail "stale flake.nix: $out" ;; esac +( cd "$RR" && cp "$SELF_DIR/go.mod" go.mod && git commit -qam r && git push -q origin main ) >/dev/null 2>&1 + # A missing formula stops the run before it tags or publishes anything. out="$(AGENT_VM_TAP="$SB/no-tap" relrun)"; rc=$? case "$rc:$out" in @@ -284,8 +306,14 @@ check "real run: the tag is on origin" "$(git -C "$RO" tag)" "v$V" check "real run: the latest release (no --latest=false)" "$(grep -c -- '--latest=false' "$SB/gh-edit.log")" "0" check "real run: published as the latest release" "$(grep -c -- '--draft=false --latest$' "$SB/gh-edit.log")" "1" # Byte order (LC_ALL=C): macOS sorts these differently in its locale. -check "real run: one tarball per platform, and their sums" "$(LC_ALL=C ls "$SB/assets" | tr '\n' ' ')" \ - "SHA256SUMS agent-vm-$V-linux-amd64.tar.gz agent-vm-$V-linux-arm64.tar.gz agent-vm-$V-windows-amd64.tar.gz agent-vm-$V-windows-arm64.tar.gz " +check "real run: one tarball per platform, a .deb and .rpm per Linux one, and their sums" "$(LC_ALL=C ls "$SB/assets" | tr '\n' ' ')" \ + "SHA256SUMS agent-vm-$V-1.aarch64.rpm agent-vm-$V-1.x86_64.rpm agent-vm-$V-linux-amd64.tar.gz agent-vm-$V-linux-arm64.tar.gz agent-vm-$V-windows-amd64.tar.gz agent-vm-$V-windows-arm64.tar.gz agent-vm_${V}_amd64.deb agent-vm_${V}_arm64.deb " +check "real run: the tarballs come first in SHA256SUMS" "$(awk '{ print $2 }' "$SB/assets/SHA256SUMS" | tr '\n' ' ' | sed 's/agent-vm-[^ ]*-[a-z]*-[a-z0-9]*\.tar\.gz/T/g')" \ + "T T T T agent-vm_${V}_arm64.deb agent-vm-$V-1.aarch64.rpm agent-vm_${V}_amd64.deb agent-vm-$V-1.x86_64.rpm " +check "real run: the arm64 .deb is made from that build" "$(cat "$SB/assets/agent-vm_${V}_arm64.deb")" \ + "deb arm64 $V qemu-system-arm qemu-system-aarch64-core $(git -C "$RR" log -1 --format=%ct)" +check "real run: the x86_64 .rpm is made from that build" "$(cat "$SB/assets/agent-vm-$V-1.x86_64.rpm")" \ + "rpm amd64 $V qemu-system-x86 qemu-system-x86-core $(git -C "$RR" log -1 --format=%ct)" check "real run: Windows's is agent-vm.exe" \ "$(tar -tzf "$SB/assets/agent-vm-$V-windows-arm64.tar.gz" | grep -c "^agent-vm-$V/agent-vm.exe$")" "1" if [ -n "$SIGN" ]; then diff --git a/third_party/lima/pkg/instance/start.go b/third_party/lima/pkg/instance/start.go index 41caa5be..0c70232a 100644 --- a/third_party/lima/pkg/instance/start.go +++ b/third_party/lima/pkg/instance/start.go @@ -291,7 +291,9 @@ func StartWithPaths(ctx context.Context, inst *limatype.Instance, launchHostAgen args = append(args, "--progress") } args = append(args, inst.Name) - haCmd = exec.CommandContext(ctx, limactl, args...) + // Not exec.CommandContext: the hostagent outlives this call, and os/exec + // would kill it whenever ctx ends, even after Start returned. + haCmd = exec.Command(limactl, args...) haCmd.SysProcAttr = executil.BackgroundSysProcAttr From b9dc5419576c74d19e05be2c65a1b2ca6e908daf Mon Sep 17 00:00:00 2001 From: Sylvain Zimmer Date: Tue, 20 Oct 2026 15:46:46 +0200 Subject: [PATCH 2/5] Workaround QEMU networking issue on Linux --- CHANGELOG.md | 12 + patches/README.md | 2 + patches/lima/0007-qemu-dgram-netdev.patch | 465 ++++++++++++++++++ .../lima/pkg/driver/qemu/dgram_unix.go | 45 ++ .../lima/pkg/driver/qemu/dgram_unix_test.go | 56 +++ .../lima/pkg/driver/qemu/dgram_windows.go | 16 + third_party/lima/pkg/driver/qemu/qemu.go | 17 +- .../lima/pkg/driver/qemu/qemu_driver.go | 52 +- .../lima/pkg/networks/usernet/dgram_test.go | 82 +++ .../lima/pkg/networks/usernet/gvproxy.go | 24 + 10 files changed, 756 insertions(+), 15 deletions(-) create mode 100644 patches/lima/0007-qemu-dgram-netdev.patch create mode 100644 third_party/lima/pkg/driver/qemu/dgram_unix.go create mode 100644 third_party/lima/pkg/driver/qemu/dgram_unix_test.go create mode 100644 third_party/lima/pkg/driver/qemu/dgram_windows.go create mode 100644 third_party/lima/pkg/networks/usernet/dgram_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index ec47e852..09a76e03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Changelog +## Unreleased + +### Fixed + +- Linux: a VM could lose its network and stop in the middle of `agent-vm + setup` or any download (`exit status 255`, the VM Stopped), with QEMU + 10.2 and a KVM host. QEMU aborted on `net_fill_rstate: Assertion 'size == + 0' failed`, a bug of its `socket` network backend, which handled a failed + read of the stream badly. The VMs' network now goes through QEMU's `dgram` + backend (QEMU 7.2 or later): no stream, nothing to lose. An older QEMU keeps + the former backend, and so does Windows (it has no unix datagram sockets). + ## 0.3.1 ### Added diff --git a/patches/README.md b/patches/README.md index 3401c8e8..e0a717f7 100644 --- a/patches/README.md +++ b/patches/README.md @@ -35,6 +35,7 @@ It runs inside the Lima hostagent, which with vz also runs the VM: a panic there | 19 | The host's event for a removal the guest asked for is relayed back as a host deletion. | The guest's next removal of a file it created again is taken for the relayed one and not done: the file stays on the host. | sshocker 0009 | | 20 | The client sets any mode bit, setuid, setgid and sticky included. | A file the host runs carries the privileges of its owner. | sshocker 0006 | | 21 | `instance.StartWithPaths` starts the hostagent with `exec.CommandContext`: os/exec kills it when the caller's context ends, also after Start returned. | The end of the agent-vm command that started a VM kills its hostagent: QEMU runs on without it (Broken, every later start fails), vz VMs die. | lima 0006 | +| 22 | QEMU's `socket` and `stream` netdevs fall through to `net_fill_rstate` with `size == -1` when `recv()` returns EAGAIN (QEMU 10.2.4 at least). | QEMU aborts (`Assertion 'size == 0' failed`) under sustained traffic on Linux with KVM: the instance stops, mid-setup. Windows keeps the `stream` netdev and so the risk. | lima 0007 | ## The patches @@ -46,6 +47,7 @@ It runs inside the Lima hostagent, which with vz also runs the VM: a panic there | `lima/0004-portfwd-not-next-to-a-host-program` | lima-vm/lima | not sent | A guest port is forwarded only when no host program uses it: with `SO_REUSEADDR`, a listener on 127.0.0.1:P is allowed next to a host program's 0.0.0.0:P on macOS and BSD, and takes its local connections. The port is tried on 127.0.0.1, ::1, 0.0.0.0 and :: first (with `SO_EXCLUSIVEADDRUSE` on Windows); one in use gives a `failed` port-forward event. Tests `TestForwardNotNextToHostProgram`, `TestForwardFreePort`. | | `lima/0005-network-isolation-hooks` | lima-vm/lima | not sent | Hooks for agent-vm's network policy: `usernet.Outbound` and `usernet.AllowHostService` (the hostagent's DNS ports), `dns.Names`/`dns.Answered` (a name refused is NXDOMAIN, never looked up). QEMU gets its own netstack per instance, as vz has, so the policy applies to both, and its guest's DNS is the gateway's. QEMU is handed the connected socket, or on Windows, which cannot pass it, connects itself (`stream` netdev, QEMU 7.2 or later). `usernet.Names` and `usernet.Answered` reach the gateway's DNS server through gvisor-tap-vsock 0002, and a host service is allowed for its network and port on 127.0.0.1 only. Tests `TestNamesAnswered`, `TestOutboundHostServices`; the rest is covered by agent-vm's e2e (`internal/vm/lima_e2e_test.go`). | | `lima/0006-instance-hostagent-outlives-start-context` | lima-vm/lima | not sent; the issue to open is the patch's header | Bug 21: `exec.Command`. No Lima test (its unit tests start no hostagent): agent-vm's e2e (`internal/cli/e2e_test.go`) fails without it. The header also reports the QEMU driver's "send on closed channel" panic that follows, not fixed here. | +| `lima/0007-qemu-dgram-netdev` | lima-vm/lima | not sent; the issue to open at QEMU is the patch's header | Bug 22: QEMU gets the netstack through a datagram socketpair (`-netdev dgram`, `GVisorNetstackOpts.DgramConn`), as vz does; the `socket` netdev stays for a QEMU older than 7.2, and Windows (the `stream` netdev has the bug too). Tests `TestDgramConn`, `TestDgramPair`, `TestFdDgramTemplate`; the netdev choice is covered by agent-vm's e2e. | | `sshocker/0001-rooted-server-readonly-names` | lima-vm/sshocker | fork branch `readonly-names` | The builtin server serves only the mounted directory, and `ReadonlyNames`. | | `sshocker/0002-expect-remove` | lima-vm/sshocker | fork branch `expect-remove` | `ExpectRemove`, for 0002 of Lima. | | `sshocker/0003-append-fix` | lima-vm/sshocker | not sent | Bug 1. Tests `TestRootedOpenAppend`, `TestRootedOpenMode`. No pkg/sftp dependency: send first. | diff --git a/patches/lima/0007-qemu-dgram-netdev.patch b/patches/lima/0007-qemu-dgram-netdev.patch new file mode 100644 index 00000000..4a9c233a --- /dev/null +++ b/patches/lima/0007-qemu-dgram-netdev.patch @@ -0,0 +1,465 @@ +Subject: [PATCH] qemu: connect the instance's netstack through the dgram netdev + +startUsernet gives QEMU its own gvisor-tap-vsock netstack through the +`socket` netdev (a length-prefixed stream on a unix socket). That netdev, +like `stream`, aborts QEMU in this case: net_socket_send() and +net_stream_data_send() ignore a recv() that fails with EWOULDBLOCK, but +then fall through to net_fill_rstate(rs, buf, size) with size == -1, +which skips its loop and fails `assert(size == 0)`: + + qemu-system-x86_64: ../net/net.c:2172: net_fill_rstate: Assertion + `size == 0' failed. + +QEMU exits with SIGABRT, the hostagent stops the instance, and the guest +loses its network in the middle of what it was doing (an apt install, in +agent-vm). It happens on Linux hosts with KVM, under sustained traffic, +with QEMU 10.2.x, to everyone whose guest goes through that netstack +(Lima's user-v2 network, Colima: qemu-project/qemu#3285, +estuary/flow#3267). What makes recv() return EAGAIN after poll() said +readable is not known. The fault is QEMU's: a unix stream socket whose +only pending data is an out-of-band byte does the same, with no netstack +(`-netdev socket,fd=N` or `-netdev stream`, the peer sends one MSG_OOB +byte). + +The dgram netdev (QEMU 7.2 or later) returns on any failed recv() and has +no framing to lose. With it, the netstack is served on a datagram +socketpair, as vz does for its network (createSockPair, AcceptBess): one +end is passed to QEMU (-netdev dgram,local.type=fd), the other is +served by usernet.GVisorNetstackOpts.DgramConn. A unix datagram socket +keeps frame boundaries and loses nothing: a full buffer blocks the +sender, and QEMU stops reading until it can write. + +The socket netdev stays for a QEMU without dgram. Windows has no unix +datagram sockets and no file descriptor passing: it keeps the stream +netdev, so the abort remains possible there. + +Tests: TestDgramConn (a frame sent as a datagram is answered by the +netstack; it times out without DgramConn being served), TestDgramPair, +TestFdDgramTemplate. The netdev choice in Cmdline needs a QEMU binary: +checked by agent-vm's e2e (internal/vm/lima_e2e_test.go), whose guest +boots and installs packages through it. + +--- +diff --git a/pkg/driver/qemu/dgram_unix.go b/pkg/driver/qemu/dgram_unix.go +new file mode 100644 +index 00000000..e1be774c +--- /dev/null ++++ b/pkg/driver/qemu/dgram_unix.go +@@ -0,0 +1,45 @@ ++//go:build !windows ++ ++// SPDX-FileCopyrightText: Copyright The Lima Authors ++// SPDX-License-Identifier: Apache-2.0 ++ ++package qemu ++ ++import ( ++ "net" ++ "os" ++ "syscall" ++) ++ ++// dgramPair returns the two ends of a connected datagram socket, as vz does ++// for its network: ours as a net.Conn for the netstack, and QEMU's as a file ++// to pass to it (-netdev dgram,local.type=fd). A datagram is a frame, and ++// unix datagram sockets neither lose nor reorder them: a full buffer blocks ++// the sender. ++func dgramPair() (ours net.Conn, qemu *os.File, err error) { ++ // The descriptors must not leak into the processes started meanwhile: ++ // QEMU gets its end through ExtraFiles. ++ syscall.ForkLock.RLock() ++ fds, err := syscall.Socketpair(syscall.AF_UNIX, syscall.SOCK_DGRAM, 0) ++ if err == nil { ++ syscall.CloseOnExec(fds[0]) ++ syscall.CloseOnExec(fds[1]) ++ } ++ syscall.ForkLock.RUnlock() ++ if err != nil { ++ return nil, nil, err ++ } ++ for _, fd := range fds { ++ // Best effort: the kernel caps these (net.core.wmem_max, rmem_max). ++ _ = syscall.SetsockoptInt(fd, syscall.SOL_SOCKET, syscall.SO_SNDBUF, 1<<20) ++ _ = syscall.SetsockoptInt(fd, syscall.SOL_SOCKET, syscall.SO_RCVBUF, 4<<20) ++ } ++ f := os.NewFile(uintptr(fds[0]), "usernet-dgram") ++ defer f.Close() ++ ours, err = net.FileConn(f) ++ if err != nil { ++ syscall.Close(fds[1]) ++ return nil, nil, err ++ } ++ return ours, os.NewFile(uintptr(fds[1]), "qemu-dgram"), nil ++} +diff --git a/pkg/driver/qemu/dgram_unix_test.go b/pkg/driver/qemu/dgram_unix_test.go +new file mode 100644 +index 00000000..c6b1fb04 +--- /dev/null ++++ b/pkg/driver/qemu/dgram_unix_test.go +@@ -0,0 +1,56 @@ ++//go:build !windows ++ ++// SPDX-FileCopyrightText: Copyright The Lima Authors ++// SPDX-License-Identifier: Apache-2.0 ++ ++package qemu ++ ++import ( ++ "net" ++ "testing" ++ ++ "gotest.tools/v3/assert" ++) ++ ++// Each write is one datagram, both ways, and QEMU's end is a file that stays ++// usable once ours is a net.Conn. ++func TestDgramPair(t *testing.T) { ++ ours, qemu, err := dgramPair() ++ assert.NilError(t, err) ++ t.Cleanup(func() { ours.Close(); qemu.Close() }) ++ theirs, err := net.FileConn(qemu) ++ assert.NilError(t, err) ++ t.Cleanup(func() { theirs.Close() }) ++ ++ for _, c := range []struct{ from, to net.Conn }{{ours, theirs}, {theirs, ours}} { ++ _, err = c.from.Write([]byte("abc")) ++ assert.NilError(t, err) ++ _, err = c.from.Write([]byte("defgh")) ++ assert.NilError(t, err) ++ buf := make([]byte, 64) ++ n, err := c.to.Read(buf) ++ assert.NilError(t, err) ++ assert.Equal(t, string(buf[:n]), "abc") ++ n, err = c.to.Read(buf) ++ assert.NilError(t, err) ++ assert.Equal(t, string(buf[:n]), "defgh") ++ } ++} ++ ++// fd_dgram hands QEMU the datagram socket as the next file descriptor, after ++// those fd_connect made. ++func TestFdDgramTemplate(t *testing.T) { ++ _, qemu, err := dgramPair() ++ assert.NilError(t, err) ++ t.Cleanup(func() { qemu.Close() }) ++ ++ a := &qArgTemplateApplier{dgram: qemu} ++ got, err := a.applyTemplate("dgram,id=net0,local.type=fd,local.str={{ fd_dgram }}") ++ assert.NilError(t, err) ++ assert.Equal(t, got, "dgram,id=net0,local.type=fd,local.str=3") ++ assert.DeepEqual(t, len(a.files), 1) ++ assert.Equal(t, a.files[0], qemu) ++ ++ _, err = (&qArgTemplateApplier{}).applyTemplate("{{ fd_dgram }}") ++ assert.ErrorContains(t, err, "no datagram socket") ++} +diff --git a/pkg/driver/qemu/dgram_windows.go b/pkg/driver/qemu/dgram_windows.go +new file mode 100644 +index 00000000..2f11d288 +--- /dev/null ++++ b/pkg/driver/qemu/dgram_windows.go +@@ -0,0 +1,16 @@ ++// SPDX-FileCopyrightText: Copyright The Lima Authors ++// SPDX-License-Identifier: Apache-2.0 ++ ++package qemu ++ ++import ( ++ "errors" ++ "net" ++ "os" ++) ++ ++// dgramPair: Windows has no unix datagram sockets, and cannot pass a file ++// descriptor to QEMU. ++func dgramPair() (net.Conn, *os.File, error) { ++ return nil, nil, errors.ErrUnsupported ++} +diff --git a/pkg/driver/qemu/qemu.go b/pkg/driver/qemu/qemu.go +index 1bb7bccf..2501aba6 100644 +--- a/pkg/driver/qemu/qemu.go ++++ b/pkg/driver/qemu/qemu.go +@@ -50,6 +50,10 @@ type Config struct { + // UsernetQEMUSock is the QEMU socket of the instance's own gvisor + // netstack, used instead of slirp when set (see startUsernet). + UsernetQEMUSock string ++ // UsernetDgram is QEMU's end of a datagram socketpair whose other end the ++ // netstack serves, preferred to UsernetQEMUSock when QEMU has the dgram ++ // netdev (see dgramPair). Nil on Windows. ++ UsernetDgram *os.File + } + + // minimumQemuVersion returns hardMin and softMin. +@@ -809,11 +813,22 @@ func Cmdline(ctx context.Context, cfg Config) (exe string, args []string, err er + case firstUsernetIndex == -1 && cfg.UsernetQEMUSock != "" && runtime.GOOS == "windows": + // No file descriptor passing there (fd_connect): QEMU connects to the + // socket itself, which QEMU 7.2 or later can on Windows. The same +- // stream as the socket netdev's. ++ // stream as the socket netdev's, so the same abort on EAGAIN (see the ++ // dgram netdev below): Windows has no datagram unix sockets. + if !strings.Contains(string(features.NetdevHelp), "stream") { + return "", nil, errors.New("QEMU 7.2 or later is needed (its stream netdev)") + } + args = append(args, "-netdev", fmt.Sprintf("stream,id=net0,server=off,addr.type=unix,addr.path=%s", cfg.UsernetQEMUSock)) ++ case firstUsernetIndex == -1 && cfg.UsernetDgram != nil && strings.Contains(string(features.NetdevHelp), "dgram"): ++ // Not the socket netdev below: it reads a length-prefixed stream, and ++ // QEMU (10.2.4 at least) aborts with `net_fill_rstate: Assertion ++ // 'size == 0' failed` when its recv() returns EAGAIN, which it ++ // meant to ignore (net_socket_send, and net_stream_data_send for the ++ // stream netdev, fall through to net_fill_rstate with size -1). ++ // Seen on Linux hosts, under sustained guest traffic. The dgram ++ // netdev ignores a failed recv() and has no framing to lose. It ++ // needs QEMU 7.2 or later. ++ args = append(args, "-netdev", "dgram,id=net0,local.type=fd,local.str={{ fd_dgram }}") + case firstUsernetIndex == -1 && cfg.UsernetQEMUSock != "": + args = append(args, "-netdev", fmt.Sprintf("socket,id=net0,fd={{ fd_connect %q }}", cfg.UsernetQEMUSock)) + case firstUsernetIndex == -1: +diff --git a/pkg/driver/qemu/qemu_driver.go b/pkg/driver/qemu/qemu_driver.go +index 5a68db80..d675c763 100644 +--- a/pkg/driver/qemu/qemu_driver.go ++++ b/pkg/driver/qemu/qemu_driver.go +@@ -69,38 +69,49 @@ type LimaQemuDriver struct { + + // startUsernet starts the instance's own gvisor netstack, as vz does: one + // network per instance, in the hostagent, where usernet.Outbound applies. +-func startUsernet(ctx context.Context, inst *limatype.Instance) (client *usernet.Client, qemuSock string, stop context.CancelFunc, err error) { ++// ++// QEMU reaches it through qemuDgram (its dgram netdev) when it has one, and ++// qemuSock otherwise (see Cmdline). qemuDgram is nil on Windows. ++func startUsernet(ctx context.Context, inst *limatype.Instance) (client *usernet.Client, qemuSock string, qemuDgram *os.File, stop context.CancelFunc, err error) { + endpointSock, err := usernet.SockWithDirectory(inst.Dir, "", usernet.EndpointSock) + if err != nil { +- return nil, "", nil, err ++ return nil, "", nil, nil, err + } + qemuSock, err = usernet.SockWithDirectory(inst.Dir, "", usernet.QEMUSock) + if err != nil { +- return nil, "", nil, err ++ return nil, "", nil, nil, err + } + os.RemoveAll(endpointSock) + os.RemoveAll(qemuSock) ++ dgramConn, qemuDgram, err := dgramPair() ++ if err != nil && !errors.Is(err, errors.ErrUnsupported) { ++ return nil, "", nil, nil, err ++ } + ctx, stop = context.WithCancel(ctx) + err = usernet.StartGVisorNetstack(ctx, &usernet.GVisorNetstackOpts{ + MTU: 1500, + Endpoint: endpointSock, + QemuSocket: qemuSock, ++ DgramConn: dgramConn, + Async: true, + DefaultLeases: map[string]string{ + networks.SlirpIPAddress: limayaml.MACAddress(inst.Dir), + }, + Subnet: networks.SlirpNetwork, + }) +- if err != nil { +- stop() +- return nil, "", nil, err ++ if err == nil { ++ var subnetIP net.IP ++ subnetIP, _, err = net.ParseCIDR(networks.SlirpNetwork) ++ if err == nil { ++ return usernet.NewClient(endpointSock, subnetIP), qemuSock, qemuDgram, stop, nil ++ } + } +- subnetIP, _, err := net.ParseCIDR(networks.SlirpNetwork) +- if err != nil { +- stop() +- return nil, "", nil, err ++ stop() ++ if dgramConn != nil { ++ dgramConn.Close() ++ qemuDgram.Close() + } +- return usernet.NewClient(endpointSock, subnetIP), qemuSock, stop, nil ++ return nil, "", nil, nil, err + } + + var _ driver.Driver = (*LimaQemuDriver)(nil) +@@ -366,12 +377,16 @@ func (l *LimaQemuDriver) Start(ctx context.Context) (chan error, error) { + VirtioGA: l.virtioPort != "", + } + if limayaml.FirstUsernetIndex(l.Instance.Config) == -1 { +- client, sock, stop, err := startUsernet(ctx, l.Instance) ++ client, sock, dgram, stop, err := startUsernet(ctx, l.Instance) + if err != nil { + return nil, err + } +- l.usernetClient, l.usernetStop, qCfg.UsernetQEMUSock = client, stop, sock ++ l.usernetClient, l.usernetStop, qCfg.UsernetQEMUSock, qCfg.UsernetDgram = client, stop, sock, dgram + defer func() { ++ // QEMU has its own copy once started. ++ if dgram != nil { ++ dgram.Close() ++ } + if l.qCmd == nil { + stop() + } +@@ -400,7 +415,7 @@ func (l *LimaQemuDriver) Start(ctx context.Context) (chan error, error) { + } + + var qArgsFinal []string +- applier := &qArgTemplateApplier{} ++ applier := &qArgTemplateApplier{dgram: qCfg.UsernetDgram} + for _, unapplied := range qArgs { + applied, err := applier.applyTemplate(unapplied) + if err != nil { +@@ -900,6 +915,8 @@ func (l *LimaQemuDriver) GuestAgentConn(ctx context.Context) (net.Conn, string, + + type qArgTemplateApplier struct { + files []*os.File ++ // dgram is the file fd_dgram passes to QEMU. ++ dgram *os.File + } + + func (a *qArgTemplateApplier) applyTemplate(qArg string) (string, error) { +@@ -938,6 +955,13 @@ func (a *qArgTemplateApplier) applyTemplate(qArg string) (string, error) { + } + return res + }, ++ "fd_dgram": func() (string, error) { ++ if a.dgram == nil { ++ return "", errors.New("fd_dgram: no datagram socket") ++ } ++ a.files = append(a.files, a.dgram) ++ return strconv.Itoa(len(a.files) + 2), nil ++ }, + } + tmpl, err := template.New("").Funcs(funcMap).Parse(qArg) + if err != nil { +diff --git a/pkg/networks/usernet/dgram_test.go b/pkg/networks/usernet/dgram_test.go +new file mode 100644 +index 00000000..1fe698df +--- /dev/null ++++ b/pkg/networks/usernet/dgram_test.go +@@ -0,0 +1,82 @@ ++//go:build !windows ++ ++// SPDX-FileCopyrightText: Copyright The Lima Authors ++// SPDX-License-Identifier: Apache-2.0 ++ ++package usernet ++ ++import ( ++ "bytes" ++ "context" ++ "net" ++ "os" ++ "path/filepath" ++ "syscall" ++ "testing" ++ "time" ++ ++ "gotest.tools/v3/assert" ++) ++ ++// A frame sent as a datagram is served by the netstack, and the answer comes ++// back as one: an ARP request for the gateway gets its reply. ++func TestDgramConn(t *testing.T) { ++ dir, err := os.MkdirTemp("", "usernet") ++ assert.NilError(t, err) ++ t.Cleanup(func() { os.RemoveAll(dir) }) ++ ++ fds, err := syscall.Socketpair(syscall.AF_UNIX, syscall.SOCK_DGRAM, 0) ++ assert.NilError(t, err) ++ serverFile, peerFile := os.NewFile(uintptr(fds[0]), "server"), os.NewFile(uintptr(fds[1]), "peer") ++ server, err := net.FileConn(serverFile) ++ assert.NilError(t, err) ++ peer, err := net.FileConn(peerFile) ++ assert.NilError(t, err) ++ serverFile.Close() ++ peerFile.Close() ++ t.Cleanup(func() { peer.Close() }) ++ ++ ctx, cancel := context.WithCancel(context.Background()) ++ t.Cleanup(cancel) ++ err = StartGVisorNetstack(ctx, &GVisorNetstackOpts{ ++ MTU: 1500, ++ Endpoint: filepath.Join(dir, "ep.sock"), ++ DgramConn: server, ++ Async: true, ++ Subnet: "192.168.5.0/24", ++ }) ++ assert.NilError(t, err) ++ ++ guestMAC := net.HardwareAddr{0x52, 0x54, 0x00, 0x00, 0x00, 0x01} ++ gatewayMAC, err := net.ParseMAC(gatewayMacAddr) ++ assert.NilError(t, err) ++ arp := bytes.Join([][]byte{ ++ {0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, guestMAC, {0x08, 0x06}, // Ethernet: broadcast, ARP ++ {0x00, 0x01, 0x08, 0x00, 0x06, 0x04, 0x00, 0x01}, // Ethernet, IPv4, request ++ guestMAC, {192, 168, 5, 15}, ++ make([]byte, 6), {192, 168, 5, 2}, // who has the gateway ++ }, nil) ++ _, err = peer.Write(arp) ++ assert.NilError(t, err) ++ ++ buf := make([]byte, 2048) ++ assert.NilError(t, peer.SetReadDeadline(time.Now().Add(10*time.Second))) ++ n, err := peer.Read(buf) ++ assert.NilError(t, err) ++ reply := buf[:n] ++ assert.Assert(t, n >= 42, "frame of %d bytes", n) ++ assert.DeepEqual(t, reply[0:6], []byte(guestMAC)) ++ assert.DeepEqual(t, reply[6:12], []byte(gatewayMAC)) ++ assert.DeepEqual(t, reply[12:14], []byte{0x08, 0x06}) ++ assert.DeepEqual(t, reply[20:22], []byte{0x00, 0x02}) // ARP reply ++ ++ // The connection ends with the netstack's context. ++ cancel() ++ assert.NilError(t, peer.SetReadDeadline(time.Now().Add(10*time.Second))) ++ _, err = peer.Write(arp) ++ for i := 0; err == nil && i < 100; i++ { ++ time.Sleep(50 * time.Millisecond) ++ _, err = peer.Write(arp) ++ } ++ assert.ErrorContains(t, err, "") ++} +diff --git a/pkg/networks/usernet/gvproxy.go b/pkg/networks/usernet/gvproxy.go +index 05c30cad..1ce63e30 100644 +--- a/pkg/networks/usernet/gvproxy.go ++++ b/pkg/networks/usernet/gvproxy.go +@@ -33,6 +33,11 @@ type GVisorNetstackOpts struct { + FdSocket string + Endpoint string + ++ // DgramConn is one end of a datagram socketpair, whose other end is ++ // QEMU's dgram netdev: one raw Ethernet frame per datagram. The netstack ++ // closes it when its context ends. ++ DgramConn net.Conn ++ + Subnet string + + Async bool +@@ -160,9 +165,28 @@ func run(ctx context.Context, g *errgroup.Group, configuration *types.Configurat + return err + } + } ++ if opts.DgramConn != nil { ++ serveDgram(ctx, vn, opts.DgramConn) ++ } + return nil + } + ++// serveDgram serves the netstack on conn, until ctx ends. Frames are the ++// datagrams: there is no length prefix to lose track of, as with QEMU's ++// socket and stream netdevs. ++func serveDgram(ctx context.Context, vn *virtualnetwork.VirtualNetwork, conn net.Conn) { ++ go func() { ++ <-ctx.Done() ++ conn.Close() ++ }() ++ go func() { ++ defer conn.Close() ++ if err := vn.AcceptBess(ctx, &UDPFileConn{Conn: conn}); err != nil && ctx.Err() == nil { ++ logrus.Error("QEMU datagram connection closed with error: ", err) ++ } ++ }() ++} ++ + func listenQEMU(ctx context.Context, vn *virtualnetwork.VirtualNetwork) error { + var lc net.ListenConfig + listener, err := lc.Listen(ctx, "unix", opts.QemuSocket) diff --git a/third_party/lima/pkg/driver/qemu/dgram_unix.go b/third_party/lima/pkg/driver/qemu/dgram_unix.go new file mode 100644 index 00000000..e1be774c --- /dev/null +++ b/third_party/lima/pkg/driver/qemu/dgram_unix.go @@ -0,0 +1,45 @@ +//go:build !windows + +// SPDX-FileCopyrightText: Copyright The Lima Authors +// SPDX-License-Identifier: Apache-2.0 + +package qemu + +import ( + "net" + "os" + "syscall" +) + +// dgramPair returns the two ends of a connected datagram socket, as vz does +// for its network: ours as a net.Conn for the netstack, and QEMU's as a file +// to pass to it (-netdev dgram,local.type=fd). A datagram is a frame, and +// unix datagram sockets neither lose nor reorder them: a full buffer blocks +// the sender. +func dgramPair() (ours net.Conn, qemu *os.File, err error) { + // The descriptors must not leak into the processes started meanwhile: + // QEMU gets its end through ExtraFiles. + syscall.ForkLock.RLock() + fds, err := syscall.Socketpair(syscall.AF_UNIX, syscall.SOCK_DGRAM, 0) + if err == nil { + syscall.CloseOnExec(fds[0]) + syscall.CloseOnExec(fds[1]) + } + syscall.ForkLock.RUnlock() + if err != nil { + return nil, nil, err + } + for _, fd := range fds { + // Best effort: the kernel caps these (net.core.wmem_max, rmem_max). + _ = syscall.SetsockoptInt(fd, syscall.SOL_SOCKET, syscall.SO_SNDBUF, 1<<20) + _ = syscall.SetsockoptInt(fd, syscall.SOL_SOCKET, syscall.SO_RCVBUF, 4<<20) + } + f := os.NewFile(uintptr(fds[0]), "usernet-dgram") + defer f.Close() + ours, err = net.FileConn(f) + if err != nil { + syscall.Close(fds[1]) + return nil, nil, err + } + return ours, os.NewFile(uintptr(fds[1]), "qemu-dgram"), nil +} diff --git a/third_party/lima/pkg/driver/qemu/dgram_unix_test.go b/third_party/lima/pkg/driver/qemu/dgram_unix_test.go new file mode 100644 index 00000000..c6b1fb04 --- /dev/null +++ b/third_party/lima/pkg/driver/qemu/dgram_unix_test.go @@ -0,0 +1,56 @@ +//go:build !windows + +// SPDX-FileCopyrightText: Copyright The Lima Authors +// SPDX-License-Identifier: Apache-2.0 + +package qemu + +import ( + "net" + "testing" + + "gotest.tools/v3/assert" +) + +// Each write is one datagram, both ways, and QEMU's end is a file that stays +// usable once ours is a net.Conn. +func TestDgramPair(t *testing.T) { + ours, qemu, err := dgramPair() + assert.NilError(t, err) + t.Cleanup(func() { ours.Close(); qemu.Close() }) + theirs, err := net.FileConn(qemu) + assert.NilError(t, err) + t.Cleanup(func() { theirs.Close() }) + + for _, c := range []struct{ from, to net.Conn }{{ours, theirs}, {theirs, ours}} { + _, err = c.from.Write([]byte("abc")) + assert.NilError(t, err) + _, err = c.from.Write([]byte("defgh")) + assert.NilError(t, err) + buf := make([]byte, 64) + n, err := c.to.Read(buf) + assert.NilError(t, err) + assert.Equal(t, string(buf[:n]), "abc") + n, err = c.to.Read(buf) + assert.NilError(t, err) + assert.Equal(t, string(buf[:n]), "defgh") + } +} + +// fd_dgram hands QEMU the datagram socket as the next file descriptor, after +// those fd_connect made. +func TestFdDgramTemplate(t *testing.T) { + _, qemu, err := dgramPair() + assert.NilError(t, err) + t.Cleanup(func() { qemu.Close() }) + + a := &qArgTemplateApplier{dgram: qemu} + got, err := a.applyTemplate("dgram,id=net0,local.type=fd,local.str={{ fd_dgram }}") + assert.NilError(t, err) + assert.Equal(t, got, "dgram,id=net0,local.type=fd,local.str=3") + assert.DeepEqual(t, len(a.files), 1) + assert.Equal(t, a.files[0], qemu) + + _, err = (&qArgTemplateApplier{}).applyTemplate("{{ fd_dgram }}") + assert.ErrorContains(t, err, "no datagram socket") +} diff --git a/third_party/lima/pkg/driver/qemu/dgram_windows.go b/third_party/lima/pkg/driver/qemu/dgram_windows.go new file mode 100644 index 00000000..2f11d288 --- /dev/null +++ b/third_party/lima/pkg/driver/qemu/dgram_windows.go @@ -0,0 +1,16 @@ +// SPDX-FileCopyrightText: Copyright The Lima Authors +// SPDX-License-Identifier: Apache-2.0 + +package qemu + +import ( + "errors" + "net" + "os" +) + +// dgramPair: Windows has no unix datagram sockets, and cannot pass a file +// descriptor to QEMU. +func dgramPair() (net.Conn, *os.File, error) { + return nil, nil, errors.ErrUnsupported +} diff --git a/third_party/lima/pkg/driver/qemu/qemu.go b/third_party/lima/pkg/driver/qemu/qemu.go index 1bb7bccf..2501aba6 100644 --- a/third_party/lima/pkg/driver/qemu/qemu.go +++ b/third_party/lima/pkg/driver/qemu/qemu.go @@ -50,6 +50,10 @@ type Config struct { // UsernetQEMUSock is the QEMU socket of the instance's own gvisor // netstack, used instead of slirp when set (see startUsernet). UsernetQEMUSock string + // UsernetDgram is QEMU's end of a datagram socketpair whose other end the + // netstack serves, preferred to UsernetQEMUSock when QEMU has the dgram + // netdev (see dgramPair). Nil on Windows. + UsernetDgram *os.File } // minimumQemuVersion returns hardMin and softMin. @@ -809,11 +813,22 @@ func Cmdline(ctx context.Context, cfg Config) (exe string, args []string, err er case firstUsernetIndex == -1 && cfg.UsernetQEMUSock != "" && runtime.GOOS == "windows": // No file descriptor passing there (fd_connect): QEMU connects to the // socket itself, which QEMU 7.2 or later can on Windows. The same - // stream as the socket netdev's. + // stream as the socket netdev's, so the same abort on EAGAIN (see the + // dgram netdev below): Windows has no datagram unix sockets. if !strings.Contains(string(features.NetdevHelp), "stream") { return "", nil, errors.New("QEMU 7.2 or later is needed (its stream netdev)") } args = append(args, "-netdev", fmt.Sprintf("stream,id=net0,server=off,addr.type=unix,addr.path=%s", cfg.UsernetQEMUSock)) + case firstUsernetIndex == -1 && cfg.UsernetDgram != nil && strings.Contains(string(features.NetdevHelp), "dgram"): + // Not the socket netdev below: it reads a length-prefixed stream, and + // QEMU (10.2.4 at least) aborts with `net_fill_rstate: Assertion + // 'size == 0' failed` when its recv() returns EAGAIN, which it + // meant to ignore (net_socket_send, and net_stream_data_send for the + // stream netdev, fall through to net_fill_rstate with size -1). + // Seen on Linux hosts, under sustained guest traffic. The dgram + // netdev ignores a failed recv() and has no framing to lose. It + // needs QEMU 7.2 or later. + args = append(args, "-netdev", "dgram,id=net0,local.type=fd,local.str={{ fd_dgram }}") case firstUsernetIndex == -1 && cfg.UsernetQEMUSock != "": args = append(args, "-netdev", fmt.Sprintf("socket,id=net0,fd={{ fd_connect %q }}", cfg.UsernetQEMUSock)) case firstUsernetIndex == -1: diff --git a/third_party/lima/pkg/driver/qemu/qemu_driver.go b/third_party/lima/pkg/driver/qemu/qemu_driver.go index 5a68db80..d675c763 100644 --- a/third_party/lima/pkg/driver/qemu/qemu_driver.go +++ b/third_party/lima/pkg/driver/qemu/qemu_driver.go @@ -69,38 +69,49 @@ type LimaQemuDriver struct { // startUsernet starts the instance's own gvisor netstack, as vz does: one // network per instance, in the hostagent, where usernet.Outbound applies. -func startUsernet(ctx context.Context, inst *limatype.Instance) (client *usernet.Client, qemuSock string, stop context.CancelFunc, err error) { +// +// QEMU reaches it through qemuDgram (its dgram netdev) when it has one, and +// qemuSock otherwise (see Cmdline). qemuDgram is nil on Windows. +func startUsernet(ctx context.Context, inst *limatype.Instance) (client *usernet.Client, qemuSock string, qemuDgram *os.File, stop context.CancelFunc, err error) { endpointSock, err := usernet.SockWithDirectory(inst.Dir, "", usernet.EndpointSock) if err != nil { - return nil, "", nil, err + return nil, "", nil, nil, err } qemuSock, err = usernet.SockWithDirectory(inst.Dir, "", usernet.QEMUSock) if err != nil { - return nil, "", nil, err + return nil, "", nil, nil, err } os.RemoveAll(endpointSock) os.RemoveAll(qemuSock) + dgramConn, qemuDgram, err := dgramPair() + if err != nil && !errors.Is(err, errors.ErrUnsupported) { + return nil, "", nil, nil, err + } ctx, stop = context.WithCancel(ctx) err = usernet.StartGVisorNetstack(ctx, &usernet.GVisorNetstackOpts{ MTU: 1500, Endpoint: endpointSock, QemuSocket: qemuSock, + DgramConn: dgramConn, Async: true, DefaultLeases: map[string]string{ networks.SlirpIPAddress: limayaml.MACAddress(inst.Dir), }, Subnet: networks.SlirpNetwork, }) - if err != nil { - stop() - return nil, "", nil, err + if err == nil { + var subnetIP net.IP + subnetIP, _, err = net.ParseCIDR(networks.SlirpNetwork) + if err == nil { + return usernet.NewClient(endpointSock, subnetIP), qemuSock, qemuDgram, stop, nil + } } - subnetIP, _, err := net.ParseCIDR(networks.SlirpNetwork) - if err != nil { - stop() - return nil, "", nil, err + stop() + if dgramConn != nil { + dgramConn.Close() + qemuDgram.Close() } - return usernet.NewClient(endpointSock, subnetIP), qemuSock, stop, nil + return nil, "", nil, nil, err } var _ driver.Driver = (*LimaQemuDriver)(nil) @@ -366,12 +377,16 @@ func (l *LimaQemuDriver) Start(ctx context.Context) (chan error, error) { VirtioGA: l.virtioPort != "", } if limayaml.FirstUsernetIndex(l.Instance.Config) == -1 { - client, sock, stop, err := startUsernet(ctx, l.Instance) + client, sock, dgram, stop, err := startUsernet(ctx, l.Instance) if err != nil { return nil, err } - l.usernetClient, l.usernetStop, qCfg.UsernetQEMUSock = client, stop, sock + l.usernetClient, l.usernetStop, qCfg.UsernetQEMUSock, qCfg.UsernetDgram = client, stop, sock, dgram defer func() { + // QEMU has its own copy once started. + if dgram != nil { + dgram.Close() + } if l.qCmd == nil { stop() } @@ -400,7 +415,7 @@ func (l *LimaQemuDriver) Start(ctx context.Context) (chan error, error) { } var qArgsFinal []string - applier := &qArgTemplateApplier{} + applier := &qArgTemplateApplier{dgram: qCfg.UsernetDgram} for _, unapplied := range qArgs { applied, err := applier.applyTemplate(unapplied) if err != nil { @@ -900,6 +915,8 @@ func (l *LimaQemuDriver) GuestAgentConn(ctx context.Context) (net.Conn, string, type qArgTemplateApplier struct { files []*os.File + // dgram is the file fd_dgram passes to QEMU. + dgram *os.File } func (a *qArgTemplateApplier) applyTemplate(qArg string) (string, error) { @@ -938,6 +955,13 @@ func (a *qArgTemplateApplier) applyTemplate(qArg string) (string, error) { } return res }, + "fd_dgram": func() (string, error) { + if a.dgram == nil { + return "", errors.New("fd_dgram: no datagram socket") + } + a.files = append(a.files, a.dgram) + return strconv.Itoa(len(a.files) + 2), nil + }, } tmpl, err := template.New("").Funcs(funcMap).Parse(qArg) if err != nil { diff --git a/third_party/lima/pkg/networks/usernet/dgram_test.go b/third_party/lima/pkg/networks/usernet/dgram_test.go new file mode 100644 index 00000000..1fe698df --- /dev/null +++ b/third_party/lima/pkg/networks/usernet/dgram_test.go @@ -0,0 +1,82 @@ +//go:build !windows + +// SPDX-FileCopyrightText: Copyright The Lima Authors +// SPDX-License-Identifier: Apache-2.0 + +package usernet + +import ( + "bytes" + "context" + "net" + "os" + "path/filepath" + "syscall" + "testing" + "time" + + "gotest.tools/v3/assert" +) + +// A frame sent as a datagram is served by the netstack, and the answer comes +// back as one: an ARP request for the gateway gets its reply. +func TestDgramConn(t *testing.T) { + dir, err := os.MkdirTemp("", "usernet") + assert.NilError(t, err) + t.Cleanup(func() { os.RemoveAll(dir) }) + + fds, err := syscall.Socketpair(syscall.AF_UNIX, syscall.SOCK_DGRAM, 0) + assert.NilError(t, err) + serverFile, peerFile := os.NewFile(uintptr(fds[0]), "server"), os.NewFile(uintptr(fds[1]), "peer") + server, err := net.FileConn(serverFile) + assert.NilError(t, err) + peer, err := net.FileConn(peerFile) + assert.NilError(t, err) + serverFile.Close() + peerFile.Close() + t.Cleanup(func() { peer.Close() }) + + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + err = StartGVisorNetstack(ctx, &GVisorNetstackOpts{ + MTU: 1500, + Endpoint: filepath.Join(dir, "ep.sock"), + DgramConn: server, + Async: true, + Subnet: "192.168.5.0/24", + }) + assert.NilError(t, err) + + guestMAC := net.HardwareAddr{0x52, 0x54, 0x00, 0x00, 0x00, 0x01} + gatewayMAC, err := net.ParseMAC(gatewayMacAddr) + assert.NilError(t, err) + arp := bytes.Join([][]byte{ + {0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, guestMAC, {0x08, 0x06}, // Ethernet: broadcast, ARP + {0x00, 0x01, 0x08, 0x00, 0x06, 0x04, 0x00, 0x01}, // Ethernet, IPv4, request + guestMAC, {192, 168, 5, 15}, + make([]byte, 6), {192, 168, 5, 2}, // who has the gateway + }, nil) + _, err = peer.Write(arp) + assert.NilError(t, err) + + buf := make([]byte, 2048) + assert.NilError(t, peer.SetReadDeadline(time.Now().Add(10*time.Second))) + n, err := peer.Read(buf) + assert.NilError(t, err) + reply := buf[:n] + assert.Assert(t, n >= 42, "frame of %d bytes", n) + assert.DeepEqual(t, reply[0:6], []byte(guestMAC)) + assert.DeepEqual(t, reply[6:12], []byte(gatewayMAC)) + assert.DeepEqual(t, reply[12:14], []byte{0x08, 0x06}) + assert.DeepEqual(t, reply[20:22], []byte{0x00, 0x02}) // ARP reply + + // The connection ends with the netstack's context. + cancel() + assert.NilError(t, peer.SetReadDeadline(time.Now().Add(10*time.Second))) + _, err = peer.Write(arp) + for i := 0; err == nil && i < 100; i++ { + time.Sleep(50 * time.Millisecond) + _, err = peer.Write(arp) + } + assert.ErrorContains(t, err, "") +} diff --git a/third_party/lima/pkg/networks/usernet/gvproxy.go b/third_party/lima/pkg/networks/usernet/gvproxy.go index 05c30cad..1ce63e30 100644 --- a/third_party/lima/pkg/networks/usernet/gvproxy.go +++ b/third_party/lima/pkg/networks/usernet/gvproxy.go @@ -33,6 +33,11 @@ type GVisorNetstackOpts struct { FdSocket string Endpoint string + // DgramConn is one end of a datagram socketpair, whose other end is + // QEMU's dgram netdev: one raw Ethernet frame per datagram. The netstack + // closes it when its context ends. + DgramConn net.Conn + Subnet string Async bool @@ -160,9 +165,28 @@ func run(ctx context.Context, g *errgroup.Group, configuration *types.Configurat return err } } + if opts.DgramConn != nil { + serveDgram(ctx, vn, opts.DgramConn) + } return nil } +// serveDgram serves the netstack on conn, until ctx ends. Frames are the +// datagrams: there is no length prefix to lose track of, as with QEMU's +// socket and stream netdevs. +func serveDgram(ctx context.Context, vn *virtualnetwork.VirtualNetwork, conn net.Conn) { + go func() { + <-ctx.Done() + conn.Close() + }() + go func() { + defer conn.Close() + if err := vn.AcceptBess(ctx, &UDPFileConn{Conn: conn}); err != nil && ctx.Err() == nil { + logrus.Error("QEMU datagram connection closed with error: ", err) + } + }() +} + func listenQEMU(ctx context.Context, vn *virtualnetwork.VirtualNetwork) error { var lc net.ListenConfig listener, err := lc.Listen(ctx, "unix", opts.QemuSocket) From 59e1165944429c93b5fba728f1edc87cd9180f15 Mon Sep 17 00:00:00 2001 From: Sylvain Zimmer Date: Tue, 20 Oct 2026 16:25:09 +0200 Subject: [PATCH 3/5] fix coderabbits --- internal/cli/doctor.go | 2 +- internal/cli/e2e_test.go | 2 +- packaging/nfpm.yaml | 5 +++++ patches/lima/0007-qemu-dgram-netdev.patch | 4 ++-- third_party/lima/pkg/networks/usernet/dgram_test.go | 2 +- 5 files changed, 10 insertions(+), 5 deletions(-) diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index 0abf73fd..6677db7b 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -302,7 +302,7 @@ func (d *doctor) directory(ctx context.Context, dir string, protects bool, getVM d.line("warn", "running with a share the host does not confine: "+confined.Error(), "The next start (after 'agent-vm stop') sets its shares back.") case inst.Orphaned: - d.line("warn", "running without Lima's host agent, cut off", "'agent-vm stop' stops it, and so does the next agent-vm command here.") + d.line("warn", "running without Lima's host agent, cut off", "'agent-vm stop' stops it, and so does the next agent-vm command here that starts a VM.") case inst.Status == vm.Broken: d.line("warn", "Broken: Lima cannot say whether it runs") default: diff --git a/internal/cli/e2e_test.go b/internal/cli/e2e_test.go index 0e244410..e768b0f7 100644 --- a/internal/cli/e2e_test.go +++ b/internal/cli/e2e_test.go @@ -1,4 +1,4 @@ -//go:build e2e +//go:build e2e && !windows package cli diff --git a/packaging/nfpm.yaml b/packaging/nfpm.yaml index 71c74548..93bb8066 100644 --- a/packaging/nfpm.yaml +++ b/packaging/nfpm.yaml @@ -35,6 +35,11 @@ contents: - src: runtime.example.sh dst: /usr/share/doc/agent-vm/runtime.example.sh +# The RPM records the build host's name: fixed, so that the same commit gives +# the same RPM on any machine. +rpm: + buildhost: agent-vm + overrides: deb: recommends: diff --git a/patches/lima/0007-qemu-dgram-netdev.patch b/patches/lima/0007-qemu-dgram-netdev.patch index 4a9c233a..463880a6 100644 --- a/patches/lima/0007-qemu-dgram-netdev.patch +++ b/patches/lima/0007-qemu-dgram-netdev.patch @@ -332,7 +332,7 @@ index 5a68db80..d675c763 100644 if err != nil { diff --git a/pkg/networks/usernet/dgram_test.go b/pkg/networks/usernet/dgram_test.go new file mode 100644 -index 00000000..1fe698df +index 00000000..01782a14 --- /dev/null +++ b/pkg/networks/usernet/dgram_test.go @@ -0,0 +1,82 @@ @@ -410,7 +410,7 @@ index 00000000..1fe698df + + // The connection ends with the netstack's context. + cancel() -+ assert.NilError(t, peer.SetReadDeadline(time.Now().Add(10*time.Second))) ++ assert.NilError(t, peer.SetWriteDeadline(time.Now().Add(10*time.Second))) + _, err = peer.Write(arp) + for i := 0; err == nil && i < 100; i++ { + time.Sleep(50 * time.Millisecond) diff --git a/third_party/lima/pkg/networks/usernet/dgram_test.go b/third_party/lima/pkg/networks/usernet/dgram_test.go index 1fe698df..01782a14 100644 --- a/third_party/lima/pkg/networks/usernet/dgram_test.go +++ b/third_party/lima/pkg/networks/usernet/dgram_test.go @@ -72,7 +72,7 @@ func TestDgramConn(t *testing.T) { // The connection ends with the netstack's context. cancel() - assert.NilError(t, peer.SetReadDeadline(time.Now().Add(10*time.Second))) + assert.NilError(t, peer.SetWriteDeadline(time.Now().Add(10*time.Second))) _, err = peer.Write(arp) for i := 0; err == nil && i < 100; i++ { time.Sleep(50 * time.Millisecond) From 62ebccfb5bc4e5fdb216f858e26577997f53b020 Mon Sep 17 00:00:00 2001 From: Sylvain Zimmer Date: Tue, 20 Oct 2026 20:35:41 +0200 Subject: [PATCH 4/5] more review fixes --- CHANGELOG.md | 25 +-- README.md | 2 +- internal/cli/setup.go | 2 +- internal/cli/start.go | 4 +- internal/cli/start_test.go | 22 +++ internal/cli/vms.go | 13 ++ internal/host/host_test.go | 18 +- internal/host/prereqs.go | 23 ++- internal/vm/lima.go | 8 +- internal/vm/lima_test.go | 43 ++++- internal/vm/procname.go | 39 ++++ patches/README.md | 7 +- .../0003-accept-ended-by-context.patch | 80 ++++++++ .../lima/0007-qemu-features-own-outputs.patch | 147 +++++++++++++++ ...dev.patch => 0008-qemu-dgram-netdev.patch} | 175 +++++++++++++----- .../lima/0009-start-channels-buffered.patch | 34 ++++ .../gvisor-tap-vsock/pkg/tap/switch.go | 4 + .../gvisor-tap-vsock/pkg/tap/switch_test.go | 48 +++++ .../lima/pkg/driver/qemu/features_test.go | 36 ++++ third_party/lima/pkg/driver/qemu/qemu.go | 71 +++---- .../lima/pkg/driver/qemu/qemu_driver.go | 118 ++++++++---- third_party/lima/pkg/instance/start.go | 7 +- www/src/i18n/en.ts | 14 +- www/src/i18n/fr.ts | 14 +- 24 files changed, 772 insertions(+), 182 deletions(-) create mode 100644 internal/vm/procname.go create mode 100644 patches/gvisor-tap-vsock/0003-accept-ended-by-context.patch create mode 100644 patches/lima/0007-qemu-features-own-outputs.patch rename patches/lima/{0007-qemu-dgram-netdev.patch => 0008-qemu-dgram-netdev.patch} (77%) create mode 100644 patches/lima/0009-start-channels-buffered.patch create mode 100644 third_party/gvisor-tap-vsock/pkg/tap/switch_test.go create mode 100644 third_party/lima/pkg/driver/qemu/features_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 09a76e03..8ca94920 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,17 +1,5 @@ # Changelog -## Unreleased - -### Fixed - -- Linux: a VM could lose its network and stop in the middle of `agent-vm - setup` or any download (`exit status 255`, the VM Stopped), with QEMU - 10.2 and a KVM host. QEMU aborted on `net_fill_rstate: Assertion 'size == - 0' failed`, a bug of its `socket` network backend, which handled a failed - read of the stream badly. The VMs' network now goes through QEMU's `dgram` - backend (QEMU 7.2 or later): no stream, nothing to lose. An older QEMU keeps - the former backend, and so does Windows (it has no unix datagram sockets). - ## 0.3.1 ### Added @@ -33,6 +21,19 @@ - A VM left in that state is stopped by force, then started again, on the next run; `agent-vm stop` stops it too. `stop` no longer says "VM stopped." for a Broken VM it did not stop. +- A start that fails (the VM never came up) now stops the VM, which Lima's + host agent would otherwise keep running. The force-stop of a VM left without + its host agent checks that the process is QEMU first: the PID file of a QEMU + that aborted may name another program's process by now. +- The missing-QEMU hint no longer says `apt-get` where there is neither apt + nor dnf (NixOS): it points to the distribution's package, or the Nix flake. +- Linux: a VM could lose its network and stop in the middle of `agent-vm + setup` or any download (`exit status 255`, the VM Stopped), with QEMU + 10.2 and a KVM host. QEMU aborted on `net_fill_rstate: Assertion 'size == + 0' failed`, a bug of its `socket` network backend, which handled a failed + read of the stream badly. The VMs' network now goes through QEMU's `dgram` + backend (QEMU 7.2 or later): no stream, nothing to lose. An older QEMU keeps + the former backend, and so does Windows (it has no unix datagram sockets). ## 0.3.0 diff --git a/README.md b/README.md index 8acb59a5..9a46cfd3 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ One binary with [Lima](https://lima-vm.io/) built in. Ships dev tools, Docker, h curl -fsSL https://www.agent-vm.org/install.sh | sh ``` -Or `brew install sylvinus/tap/agent-vm`. From a clone, with Go (and on macOS the Xcode command line tools): `./agent-vm.sh install` builds it and links it onto your `PATH`; after a `git pull`, run it again. See [Install](https://www.agent-vm.org/#install) for prerequisites. +Or `brew install sylvinus/tap/agent-vm`; on Linux, the `.deb` or `.rpm` of the [latest release](https://github.com/sylvinus/agent-vm/releases/latest), or `nix run github:sylvinus/agent-vm` (QEMU included). From a clone, with Go (and on macOS the Xcode command line tools): `./agent-vm.sh install` builds it and links it onto your `PATH`; after a `git pull`, run it again. See [Install](https://www.agent-vm.org/#install) for prerequisites. ## Use diff --git a/internal/cli/setup.go b/internal/cli/setup.go index 0709e07f..c53b463a 100644 --- a/internal/cli/setup.go +++ b/internal/cli/setup.go @@ -446,7 +446,7 @@ func (e *app) buildBase(ctx context.Context, in install, disk, memory, cpus int) (&start{app: e, ctx: ctx, name: building}).printResources() fmt.Fprintln(e.io.Stdout, "Starting base VM (the first run downloads a Debian image)...") - if !windowed(func(w io.Writer) error { return b.Start(ctx, building, w) }) { + if !windowed(func(w io.Writer) error { return e.startVM(ctx, b, building, w) }) { ha := filepath.Join(LimaHome(e.state), building, "ha.stderr.log") fmt.Fprintf(e.io.Stderr, "Error: Failed to start base VM. Full log: %s\nLima's own log: %s\n", logPath, ha) host.WindowsStartHint(e.io.Stderr, logPath, ha) diff --git a/internal/cli/start.go b/internal/cli/start.go index 095ae5bd..5dc435f6 100644 --- a/internal/cli/start.go +++ b/internal/cli/start.go @@ -631,7 +631,7 @@ func (s *start) boot() error { fmt.Fprintf(s.io.Stdout, "Starting VM '%s'...\n", s.name) b, _ := s.backend() var log bytes.Buffer - if err := b.Start(s.ctx, s.name, &log); err != nil { + if err := s.startVM(s.ctx, b, s.name, &log); err != nil { ha := paths.Host(filepath.Join(LimaHome(s.state), s.name, "ha.stderr.log")) s.warnf("Error: Failed to start VM '%s'.\n--- Lima's output ---\n%s%v\nFull log: %s", s.name, log.String(), err, ha) host.WindowsStartHint(s.io.Stderr, ha) @@ -711,7 +711,7 @@ func (s *start) afterBoot(isNew, wasRunning, wantWritable bool) error { return err } b, _ := s.backend() - if err := b.Start(s.ctx, s.name, io.Discard); err != nil { + if err := s.startVM(s.ctx, b, s.name, io.Discard); err != nil { s.warnf("Error: VM '%s' did not come back up after changing the project mount.", s.name) return errStop } diff --git a/internal/cli/start_test.go b/internal/cli/start_test.go index b65a8eb0..9f87e1c7 100644 --- a/internal/cli/start_test.go +++ b/internal/cli/start_test.go @@ -234,6 +234,28 @@ func TestStartSSHPort(t *testing.T) { } } +// A start that fails stops the VM: the hostagent would outlive the command. +// An interrupted one is left to finish in the background. +func TestStartFailedStops(t *testing.T) { + se := newStartEnv(t) + se.base() + name := vmname.Name(se.proj) + se.fake.FailOn = map[string]error{"start": errors.New("boom")} + if se.run("run", "true") != 1 || !strings.HasSuffix(se.fake.CallLog(), "start "+name+"\nstop "+name) { + t.Errorf("start fails: %s\n%s", se.out(), se.fake.CallLog()) + } + e, err := newApp(IO{Stdin: strings.NewReader(""), Stdout: io.Discard, Stderr: io.Discard}) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + n := len(se.fake.Calls) + if err := e.startVM(ctx, se.fake, name, io.Discard); err == nil || strings.Contains(strings.Join(se.fake.Calls[n:], "\n"), "stop") { + t.Errorf("interrupted: %v\n%s", err, se.fake.CallLog()) + } +} + // A new VM that cannot be configured is deleted, never started, nothing // recorded; --reset that cannot delete changes nothing. func TestStartFailures(t *testing.T) { diff --git a/internal/cli/vms.go b/internal/cli/vms.go index 2e811c78..319b87d7 100644 --- a/internal/cli/vms.go +++ b/internal/cli/vms.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "io" "strings" "text/tabwriter" @@ -67,6 +68,18 @@ func (e *app) stopCmd(ctx context.Context, args []string) int { return 0 } +// startVM starts name on b. Lima's hostagent outlives the command, so a start +// that fails (the VM never came up) would leave it running, cut off: it is +// stopped. Not after an interrupt: that start goes on in the background, as +// limactl's does, and the next command finds the VM up. +func (e *app) startVM(ctx context.Context, b vm.Backend, name string, log io.Writer) error { + err := b.Start(ctx, name, log) + if err != nil && ctx.Err() == nil { + _ = b.Stop(ctx, name) + } + return err +} + // stopVM stops name, and reports whether it is known to be stopped. func (e *app) stopVM(ctx context.Context, name string) bool { b, err := e.backend() diff --git a/internal/host/host_test.go b/internal/host/host_test.go index dc75fd9c..d6e35470 100644 --- a/internal/host/host_test.go +++ b/internal/host/host_test.go @@ -52,17 +52,17 @@ func TestCapBash(t *testing.T) { // recommend it. func TestQemuBinaryFor(t *testing.T) { for _, c := range []struct { - goarch string - dnf bool - bin, cmd string + goarch, manager string + bin, cmd string }{ - {"amd64", false, "qemu-system-x86_64", "sudo apt-get install qemu-system-x86"}, - {"arm64", false, "qemu-system-aarch64", "sudo apt-get install qemu-system-arm"}, - {"amd64", true, "qemu-system-x86_64", "sudo dnf install qemu-system-x86-core"}, - {"arm64", true, "qemu-system-aarch64", "sudo dnf install qemu-system-aarch64-core"}, + {"amd64", "apt-get", "qemu-system-x86_64", "sudo apt-get install qemu-system-x86"}, + {"arm64", "apt-get", "qemu-system-aarch64", "sudo apt-get install qemu-system-arm"}, + {"amd64", "dnf", "qemu-system-x86_64", "sudo dnf install qemu-system-x86-core"}, + {"arm64", "dnf", "qemu-system-aarch64", "sudo dnf install qemu-system-aarch64-core"}, + {"amd64", "", "qemu-system-x86_64", "your distribution's QEMU package (with Nix, 'nix run github:sylvinus/agent-vm' brings its own)"}, } { - if bin, cmd := qemuBinaryFor(c.goarch, c.dnf); bin != c.bin || cmd != c.cmd { - t.Errorf("%s dnf=%v: %s %q", c.goarch, c.dnf, bin, cmd) + if bin, cmd := qemuBinaryFor(c.goarch, c.manager); bin != c.bin || cmd != c.cmd { + t.Errorf("%s %q: %s %q", c.goarch, c.manager, bin, cmd) } } } diff --git a/internal/host/prereqs.go b/internal/host/prereqs.go index 2c43cd7a..462d6e27 100644 --- a/internal/host/prereqs.go +++ b/internal/host/prereqs.go @@ -13,15 +13,19 @@ import ( ) // qemuBinary is QEMU's emulator for this host's architecture, and the -// command that installs it: dnf's package where there is dnf and no apt-get, -// else Debian's. +// way to install it: the command with apt-get's package, or dnf's where there +// is dnf and no apt-get; a general hint where there is neither (NixOS, Arch). func qemuBinary() (bin, install string) { - _, aptErr := exec.LookPath("apt-get") - _, dnfErr := exec.LookPath("dnf") - return qemuBinaryFor(runtime.GOARCH, aptErr != nil && dnfErr == nil) + manager := "" + if _, err := exec.LookPath("apt-get"); err == nil { + manager = "apt-get" + } else if _, err := exec.LookPath("dnf"); err == nil { + manager = "dnf" + } + return qemuBinaryFor(runtime.GOARCH, manager) } -func qemuBinaryFor(goarch string, dnf bool) (bin, install string) { +func qemuBinaryFor(goarch, manager string) (bin, install string) { bin, deb, rpm := "qemu-system-"+goarch, "qemu-system", "qemu-system-"+goarch+"-core" switch goarch { case "amd64": @@ -29,10 +33,13 @@ func qemuBinaryFor(goarch string, dnf bool) (bin, install string) { case "arm64": bin, deb, rpm = "qemu-system-aarch64", "qemu-system-arm", "qemu-system-aarch64-core" } - if dnf { + switch manager { + case "dnf": return bin, "sudo dnf install " + rpm + case "apt-get": + return bin, "sudo apt-get install " + deb } - return bin, "sudo apt-get install " + deb + return bin, "your distribution's QEMU package (with Nix, 'nix run github:sylvinus/agent-vm' brings its own)" } // WSLVersion is 2 or 1 under WSL, from the kernel release, 0 elsewhere. diff --git a/internal/vm/lima.go b/internal/vm/lima.go index d60c5387..3376347e 100644 --- a/internal/vm/lima.go +++ b/internal/vm/lima.go @@ -223,8 +223,12 @@ func fromLima(inst *limatype.Instance) *Instance { out.ConfigErr = errors.Join(inst.Errors...) } // "driver is running but host agent is not". Not the other way around: a - // hostagent without its driver is also one booting the VM. - out.Orphaned = out.Status == Broken && inst.Config != nil && inst.HostAgentPID == 0 && inst.DriverPID > 0 + // hostagent without its driver is also one booting the VM. Only QEMU + // outlives the hostagent (vz and the others run in it), and the PID file + // of one that aborted may name another program's process now: Stop kills + // this one. + out.Orphaned = out.Status == Broken && inst.Config != nil && inst.HostAgentPID == 0 && inst.DriverPID > 0 && + inst.VMType == limatype.QEMU && isQEMU(inst.DriverPID) if c := inst.Config; c != nil { if c.SSH.LocalPort != nil { out.SSHPortConfig = *c.SSH.LocalPort diff --git a/internal/vm/lima_test.go b/internal/vm/lima_test.go index 3a0779b9..22450a01 100644 --- a/internal/vm/lima_test.go +++ b/internal/vm/lima_test.go @@ -312,14 +312,26 @@ func TestLimaStopOrphaned(t *testing.T) { if err != nil { t.Fatal(err) } - driver := exec.Command("sleep", "60") - if err := driver.Start(); err != nil { - t.Fatal(err) + // sleep, under the name of a program: QEMU's, or any other. + spawn := func(name string) ([]byte, chan struct{}) { + sleep, err := exec.LookPath("sleep") + if err != nil { + t.Skip(err) + } + exe := filepath.Join(t.TempDir(), name) + if err := os.Symlink(sleep, exe); err != nil { + t.Fatal(err) + } + cmd := exec.Command(exe, "60") + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + done := make(chan struct{}) + go func() { cmd.Wait(); close(done) }() + t.Cleanup(func() { cmd.Process.Kill(); <-done }) + return []byte(strconv.Itoa(cmd.Process.Pid)), done } - exited := make(chan struct{}) - go func() { driver.Wait(); close(exited) }() - t.Cleanup(func() { driver.Process.Kill(); <-exited }) - pid := []byte(strconv.Itoa(driver.Process.Pid)) + pid, exited := spawn("qemu-system-fake") ha := filepath.Join(inst.Dir, filenames.HostAgentPID) os.WriteFile(ha, pid, 0o644) @@ -334,7 +346,22 @@ func TestLimaStopOrphaned(t *testing.T) { } os.Remove(ha) - os.WriteFile(filepath.Join(inst.Dir, filenames.PIDFile(inst.VMType)), pid, 0o644) + // A PID file left by a QEMU that aborted, its number another program's + // now: not orphaned, and Stop leaves it alone. + other, otherExited := spawn("not-a-vm") + driverPID := filepath.Join(inst.Dir, filenames.PIDFile(inst.VMType)) + os.WriteFile(driverPID, other, 0o644) + if got, _ := l.Get(ctx, "o"); got.Orphaned { + t.Fatalf("another program's process: %+v", got) + } + l.Stop(ctx, "o") + select { + case <-otherExited: + t.Fatal("Stop killed a process that is not QEMU") + case <-time.After(200 * time.Millisecond): + } + + os.WriteFile(driverPID, pid, 0o644) if got, _ := l.Get(ctx, "o"); got.Status != Broken || !got.Orphaned { t.Fatalf("a driver without its hostagent: %+v", got) } diff --git a/internal/vm/procname.go b/internal/vm/procname.go new file mode 100644 index 00000000..cdd57d49 --- /dev/null +++ b/internal/vm/procname.go @@ -0,0 +1,39 @@ +package vm + +import ( + "os" + "os/exec" + "path/filepath" + "runtime" + "strconv" + "strings" +) + +// isQEMU reports whether pid is a QEMU process. A PID file outlives a QEMU +// that aborted, and its number may belong to another program by now: that one +// must not be killed. +func isQEMU(pid int) bool { + var name string + switch runtime.GOOS { + case "linux": + // The kernel's name for it, cut to 15 characters: "qemu-system-x86". + b, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/comm") + if err != nil { + return false + } + name = string(b) + case "windows": + out, err := exec.Command("tasklist", "/FI", "PID eq "+strconv.Itoa(pid), "/FO", "CSV", "/NH").Output() + if err != nil { + return false + } + name = strings.Trim(strings.SplitN(string(out), ",", 2)[0], `"`) + default: + out, err := exec.Command("ps", "-p", strconv.Itoa(pid), "-o", "comm=").Output() + if err != nil { + return false + } + name = string(out) + } + return strings.HasPrefix(strings.ToLower(filepath.Base(strings.TrimSpace(name))), "qemu") +} diff --git a/patches/README.md b/patches/README.md index e0a717f7..42b562cb 100644 --- a/patches/README.md +++ b/patches/README.md @@ -35,7 +35,7 @@ It runs inside the Lima hostagent, which with vz also runs the VM: a panic there | 19 | The host's event for a removal the guest asked for is relayed back as a host deletion. | The guest's next removal of a file it created again is taken for the relayed one and not done: the file stays on the host. | sshocker 0009 | | 20 | The client sets any mode bit, setuid, setgid and sticky included. | A file the host runs carries the privileges of its owner. | sshocker 0006 | | 21 | `instance.StartWithPaths` starts the hostagent with `exec.CommandContext`: os/exec kills it when the caller's context ends, also after Start returned. | The end of the agent-vm command that started a VM kills its hostagent: QEMU runs on without it (Broken, every later start fails), vz VMs die. | lima 0006 | -| 22 | QEMU's `socket` and `stream` netdevs fall through to `net_fill_rstate` with `size == -1` when `recv()` returns EAGAIN (QEMU 10.2.4 at least). | QEMU aborts (`Assertion 'size == 0' failed`) under sustained traffic on Linux with KVM: the instance stops, mid-setup. Windows keeps the `stream` netdev and so the risk. | lima 0007 | +| 22 | QEMU's `socket` and `stream` netdevs fall through to `net_fill_rstate` with `size == -1` when `recv()` returns EAGAIN (QEMU 10.2.4 at least). | QEMU aborts (`Assertion 'size == 0' failed`) under sustained traffic on Linux with KVM: the instance stops, mid-setup. Windows keeps the `stream` netdev and so the risk. | lima 0008 | ## The patches @@ -47,7 +47,9 @@ It runs inside the Lima hostagent, which with vz also runs the VM: a panic there | `lima/0004-portfwd-not-next-to-a-host-program` | lima-vm/lima | not sent | A guest port is forwarded only when no host program uses it: with `SO_REUSEADDR`, a listener on 127.0.0.1:P is allowed next to a host program's 0.0.0.0:P on macOS and BSD, and takes its local connections. The port is tried on 127.0.0.1, ::1, 0.0.0.0 and :: first (with `SO_EXCLUSIVEADDRUSE` on Windows); one in use gives a `failed` port-forward event. Tests `TestForwardNotNextToHostProgram`, `TestForwardFreePort`. | | `lima/0005-network-isolation-hooks` | lima-vm/lima | not sent | Hooks for agent-vm's network policy: `usernet.Outbound` and `usernet.AllowHostService` (the hostagent's DNS ports), `dns.Names`/`dns.Answered` (a name refused is NXDOMAIN, never looked up). QEMU gets its own netstack per instance, as vz has, so the policy applies to both, and its guest's DNS is the gateway's. QEMU is handed the connected socket, or on Windows, which cannot pass it, connects itself (`stream` netdev, QEMU 7.2 or later). `usernet.Names` and `usernet.Answered` reach the gateway's DNS server through gvisor-tap-vsock 0002, and a host service is allowed for its network and port on 127.0.0.1 only. Tests `TestNamesAnswered`, `TestOutboundHostServices`; the rest is covered by agent-vm's e2e (`internal/vm/lima_e2e_test.go`). | | `lima/0006-instance-hostagent-outlives-start-context` | lima-vm/lima | not sent; the issue to open is the patch's header | Bug 21: `exec.Command`. No Lima test (its unit tests start no hostagent): agent-vm's e2e (`internal/cli/e2e_test.go`) fails without it. The header also reports the QEMU driver's "send on closed channel" panic that follows, not fixed here. | -| `lima/0007-qemu-dgram-netdev` | lima-vm/lima | not sent; the issue to open at QEMU is the patch's header | Bug 22: QEMU gets the netstack through a datagram socketpair (`-netdev dgram`, `GVisorNetstackOpts.DgramConn`), as vz does; the `socket` netdev stays for a QEMU older than 7.2, and Windows (the `stream` netdev has the bug too). Tests `TestDgramConn`, `TestDgramPair`, `TestFdDgramTemplate`; the netdev choice is covered by agent-vm's e2e. | +| `lima/0007-qemu-features-own-outputs` | lima-vm/lima | not sent | `inspectFeatures` ran its four help requests into the same buffers: each feature text held the earlier ones'. One buffer per request (`qemuHelp`). Test `TestInspectFeaturesOwnOutputs`. | +| `lima/0008-qemu-dgram-netdev` | lima-vm/lima | not sent; the issue to open at QEMU is the patch's header | Bug 22: QEMU gets the netstack through a datagram socketpair (`-netdev dgram`, `GVisorNetstackOpts.DgramConn`), as vz does; the `socket` netdev, and its unix socket listener, only for a QEMU older than 7.2, and Windows (the `stream` netdev has the bug too). Tests `TestDgramConn`, `TestDgramPair`, `TestFdDgramTemplate`; the netdev choice is covered by agent-vm's e2e. | +| `lima/0009-start-channels-buffered` | lima-vm/lima | not sent | `StartWithPaths` left the losing goroutine of its race blocked on an unbuffered send, for as long as the program lives. No test: it starts a hostagent. | | `sshocker/0001-rooted-server-readonly-names` | lima-vm/sshocker | fork branch `readonly-names` | The builtin server serves only the mounted directory, and `ReadonlyNames`. | | `sshocker/0002-expect-remove` | lima-vm/sshocker | fork branch `expect-remove` | `ExpectRemove`, for 0002 of Lima. | | `sshocker/0003-append-fix` | lima-vm/sshocker | not sent | Bug 1. Tests `TestRootedOpenAppend`, `TestRootedOpenMode`. No pkg/sftp dependency: send first. | @@ -64,6 +66,7 @@ It runs inside the Lima hostagent, which with vz also runs the VM: a panic there | `sftp/0005-fuzz-request-server` | pkg/sftp | not sent | `FuzzRequestServer`: any bytes after INIT, to a server set as sshocker sets it; it never panics nor hangs. Its seed is a WRITE at a huge offset, which made `InMemHandler` allocate it. | | `gvisor-tap-vsock/0001-outbound-policy` | containers/gvisor-tap-vsock | not sent | `Configuration.Outbound`: the TCP and UDP forwarders refuse a destination it denies, checked after NAT (the gateway's address is the host's loopback by then). No test of its own: agent-vm's e2e connects to denied and allowed addresses. | | `gvisor-tap-vsock/0002-dns-policy` | containers/gvisor-tap-vsock | not sent | `Configuration.DNSNames` and `DNSAnswered`: the gateway's DNS server answers NXDOMAIN, over UDP and TCP, for a name `DNSNames` refuses, without asking the host's resolver, and tells `DNSAnswered` the IPv4 addresses it gave, so that `Outbound` can allow those only. Test `TestPolicy`. | +| `gvisor-tap-vsock/0003-accept-ended-by-context` | containers/gvisor-tap-vsock | not sent | `Switch.Accept` returns nil, and logs nothing, when its connection ends after the program cancelled the context. Test `TestAcceptEndedByContext`. | Lima's `0b63ae0` of the fork (`replace` of sshocker) is gone: agent-vm's `go.mod` replaces all three modules. FSETSTAT through a read handle keeps the path checks (0004): a read handle can be diff --git a/patches/gvisor-tap-vsock/0003-accept-ended-by-context.patch b/patches/gvisor-tap-vsock/0003-accept-ended-by-context.patch new file mode 100644 index 00000000..688a4e64 --- /dev/null +++ b/patches/gvisor-tap-vsock/0003-accept-ended-by-context.patch @@ -0,0 +1,80 @@ +Subject: [PATCH] tap: a connection closed by the program's cancel is not an error + +Switch.Accept logs "cannot receive packets from X, disconnecting" at error +level, and returns the error, when the connection's read fails. A program +that stops its switch by cancelling the context and closing the connection +gets that error each time, though it asked for it. Accept returns nil then, +and says nothing. + +Test: TestAcceptEndedByContext (a read failing by itself is still an error). + +--- +diff --git a/pkg/tap/switch.go b/pkg/tap/switch.go +index 944b618f..e96a0fd9 100644 +--- a/pkg/tap/switch.go ++++ b/pkg/tap/switch.go +@@ -96,6 +96,10 @@ func (e *Switch) Accept(ctx context.Context, rawConn net.Conn, protocol types.Pr + e.disconnect(id, conn) + }() + if err := e.rx(ctx, id, conn); err != nil { ++ if ctx.Err() != nil { ++ // Closing the connection is how a program ends the switch. ++ return nil ++ } + err := fmt.Errorf("cannot receive packets from %s, disconnecting: %w", conn.RemoteAddr().String(), err) + log.Error(err) + return err +diff --git a/pkg/tap/switch_test.go b/pkg/tap/switch_test.go +new file mode 100644 +index 00000000..4039fe9b +--- /dev/null ++++ b/pkg/tap/switch_test.go +@@ -0,0 +1,48 @@ ++package tap ++ ++import ( ++ "context" ++ "net" ++ "sync" ++ "testing" ++ ++ "github.com/containers/gvisor-tap-vsock/pkg/types" ++ "github.com/stretchr/testify/require" ++) ++ ++// readingConn says when it is first read. ++type readingConn struct { ++ net.Conn ++ reading chan struct{} ++ once sync.Once ++} ++ ++func (c *readingConn) Read(b []byte) (int, error) { ++ c.once.Do(func() { close(c.reading) }) ++ return c.Conn.Read(b) ++} ++ ++// A connection that ends because the program cancelled the context is not an ++// error; one that ends by itself is. ++func TestAcceptEndedByContext(t *testing.T) { ++ for _, cancelled := range []bool{true, false} { ++ s := NewSwitch(false) ++ ours, theirs := net.Pipe() ++ conn := &readingConn{Conn: ours, reading: make(chan struct{})} ++ ctx, cancel := context.WithCancel(context.Background()) ++ done := make(chan error, 1) ++ go func() { done <- s.Accept(ctx, conn, types.VfkitProtocol) }() ++ <-conn.reading ++ if cancelled { ++ cancel() ++ } ++ theirs.Close() ++ err := <-done ++ cancel() ++ if cancelled { ++ require.NoError(t, err) ++ } else { ++ require.Error(t, err) ++ } ++ } ++} diff --git a/patches/lima/0007-qemu-features-own-outputs.patch b/patches/lima/0007-qemu-features-own-outputs.patch new file mode 100644 index 00000000..856abf5b --- /dev/null +++ b/patches/lima/0007-qemu-features-own-outputs.patch @@ -0,0 +1,147 @@ +Subject: [PATCH] qemu: give each help request its own output + +inspectFeatures runs `qemu -accel help`, `-netdev help`, `-machine help` and +`-cpu help` into the same two buffers, and keeps each output as the buffer's +Bytes() after its run. NetdevHelp is the accelerators' text followed by the +netdevs', MachineHelp has both and its own, CPUHelp all four. The features +are searched as text (strings.Contains): a word of an earlier help can match +a later one's feature. + +Each request has its own buffers now (qemuHelp). + +Test: TestInspectFeaturesOwnOutputs. + +--- +diff --git a/pkg/driver/qemu/features_test.go b/pkg/driver/qemu/features_test.go +new file mode 100644 +index 00000000..16228077 +--- /dev/null ++++ b/pkg/driver/qemu/features_test.go +@@ -0,0 +1,36 @@ ++//go:build !windows ++ ++// SPDX-FileCopyrightText: Copyright The Lima Authors ++// SPDX-License-Identifier: Apache-2.0 ++ ++package qemu ++ ++import ( ++ "context" ++ "os" ++ "path/filepath" ++ "testing" ++ ++ "gotest.tools/v3/assert" ++) ++ ++// Each help text is its own: the features are searched as text, and a netdev ++// list holding the accelerators' words would match them. ++func TestInspectFeaturesOwnOutputs(t *testing.T) { ++ exe := filepath.Join(t.TempDir(), "qemu-fake") ++ script := `#!/bin/sh ++case "$*" in ++ *-cpu*) echo "cpus: max" ;; ++ *-accel*) echo "accels: tcg" ;; ++ *-netdev*) echo "netdevs: dgram" ;; ++ *-machine*) echo "machines: virt" ;; ++esac ++` ++ assert.NilError(t, os.WriteFile(exe, []byte(script), 0o755)) ++ f, err := inspectFeatures(context.Background(), exe, "virt") ++ assert.NilError(t, err) ++ assert.Equal(t, string(f.AccelHelp), "accels: tcg\n") ++ assert.Equal(t, string(f.NetdevHelp), "netdevs: dgram\n") ++ assert.Equal(t, string(f.MachineHelp), "machines: virt\n") ++ assert.Equal(t, string(f.CPUHelp), "cpus: max\n") ++} +diff --git a/pkg/driver/qemu/qemu.go b/pkg/driver/qemu/qemu.go +index 1bb7bccf..79318857 100644 +--- a/pkg/driver/qemu/qemu.go ++++ b/pkg/driver/qemu/qemu.go +@@ -310,61 +310,40 @@ type features struct { + } + + func inspectFeatures(ctx context.Context, exe, machine string) (*features, error) { +- var ( +- f features +- stdout bytes.Buffer +- stderr bytes.Buffer +- ) +- cmd := exec.CommandContext(ctx, exe, "-M", "none", "-accel", "help") +- cmd.Stdout = &stdout +- cmd.Stderr = &stderr +- if err := cmd.Run(); err != nil { +- return nil, fmt.Errorf("failed to run %v: stdout=%#q, stderr=%#q", cmd.Args, stdout.String(), stderr.String()) ++ var f features ++ var err error ++ if f.AccelHelp, err = qemuHelp(ctx, exe, "-M", "none", "-accel", "help"); err != nil { ++ return nil, err + } +- f.AccelHelp = stdout.Bytes() +- // on older versions qemu will write "help" output to stderr +- if len(f.AccelHelp) == 0 { +- f.AccelHelp = stderr.Bytes() ++ if f.NetdevHelp, err = qemuHelp(ctx, exe, "-M", "none", "-netdev", "help"); err != nil { ++ logrus.Warn(err) + } +- +- cmd = exec.CommandContext(ctx, exe, "-M", "none", "-netdev", "help") +- cmd.Stdout = &stdout +- cmd.Stderr = &stderr +- if err := cmd.Run(); err != nil { +- logrus.Warnf("failed to run %v: stdout=%#q, stderr=%#q", cmd.Args, stdout.String(), stderr.String()) +- } else { +- f.NetdevHelp = stdout.Bytes() +- if len(f.NetdevHelp) == 0 { +- f.NetdevHelp = stderr.Bytes() +- } ++ if f.MachineHelp, err = qemuHelp(ctx, exe, "-machine", "help"); err != nil { ++ logrus.Warn(err) + } +- +- cmd = exec.CommandContext(ctx, exe, "-machine", "help") +- cmd.Stdout = &stdout +- cmd.Stderr = &stderr +- if err := cmd.Run(); err != nil { +- logrus.Warnf("failed to run %v: stdout=%#q, stderr=%#q", cmd.Args, stdout.String(), stderr.String()) +- } else { +- f.MachineHelp = stdout.Bytes() +- if len(f.MachineHelp) == 0 { +- f.MachineHelp = stderr.Bytes() +- } ++ // Avoid error: "No machine specified, and there is no default" ++ if f.CPUHelp, err = qemuHelp(ctx, exe, "-cpu", "help", "-machine", machine); err != nil { ++ logrus.Warn(err) + } ++ return &f, nil ++} + +- // Avoid error: "No machine specified, and there is no default" +- cmd = exec.CommandContext(ctx, exe, "-cpu", "help", "-machine", machine) ++// qemuHelp is what exe prints for args, a help request. Each one has its own ++// buffers: the features are searched as text, and one holding another's ++// output would match words that are not its own. ++func qemuHelp(ctx context.Context, exe string, args ...string) ([]byte, error) { ++ var stdout, stderr bytes.Buffer ++ cmd := exec.CommandContext(ctx, exe, args...) + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { +- logrus.Warnf("failed to run %v: stdout=%#q, stderr=%#q", cmd.Args, stdout.String(), stderr.String()) +- } else { +- f.CPUHelp = stdout.Bytes() +- if len(f.CPUHelp) == 0 { +- f.CPUHelp = stderr.Bytes() +- } ++ return nil, fmt.Errorf("failed to run %v: stdout=%#q, stderr=%#q", cmd.Args, stdout.String(), stderr.String()) + } +- +- return &f, nil ++ // on older versions qemu will write "help" output to stderr ++ if stdout.Len() == 0 { ++ return stderr.Bytes(), nil ++ } ++ return stdout.Bytes(), nil + } + + // adjustMemBytesDarwinARM64HVF adjusts the memory to be <= 3 GiB, only when the following conditions are met: diff --git a/patches/lima/0007-qemu-dgram-netdev.patch b/patches/lima/0008-qemu-dgram-netdev.patch similarity index 77% rename from patches/lima/0007-qemu-dgram-netdev.patch rename to patches/lima/0008-qemu-dgram-netdev.patch index 463880a6..e163f867 100644 --- a/patches/lima/0007-qemu-dgram-netdev.patch +++ b/patches/lima/0008-qemu-dgram-netdev.patch @@ -29,9 +29,11 @@ served by usernet.GVisorNetstackOpts.DgramConn. A unix datagram socket keeps frame boundaries and loses nothing: a full buffer blocks the sender, and QEMU stops reading until it can write. -The socket netdev stays for a QEMU without dgram. Windows has no unix -datagram sockets and no file descriptor passing: it keeps the stream -netdev, so the abort remains possible there. +Which one QEMU gets is decided by Cmdline, from the netdevs it lists, and +the netstack starts after it: it listens on the unix socket only for the +socket netdev, of a QEMU without dgram. Windows has no unix datagram +sockets and no file descriptor passing: it keeps the stream netdev, so the +abort remains possible there. Tests: TestDgramConn (a frame sent as a datagram is answered by the netstack; it times out without DgramConn being served), TestDgramPair, @@ -176,7 +178,7 @@ index 00000000..2f11d288 + return nil, nil, errors.ErrUnsupported +} diff --git a/pkg/driver/qemu/qemu.go b/pkg/driver/qemu/qemu.go -index 1bb7bccf..2501aba6 100644 +index 79318857..bf55ddb3 100644 --- a/pkg/driver/qemu/qemu.go +++ b/pkg/driver/qemu/qemu.go @@ -50,6 +50,10 @@ type Config struct { @@ -190,7 +192,7 @@ index 1bb7bccf..2501aba6 100644 } // minimumQemuVersion returns hardMin and softMin. -@@ -809,11 +813,22 @@ func Cmdline(ctx context.Context, cfg Config) (exe string, args []string, err er +@@ -788,11 +792,22 @@ func Cmdline(ctx context.Context, cfg Config) (exe string, args []string, err er case firstUsernetIndex == -1 && cfg.UsernetQEMUSock != "" && runtime.GOOS == "windows": // No file descriptor passing there (fd_connect): QEMU connects to the // socket itself, which QEMU 7.2 or later can on Windows. The same @@ -215,90 +217,163 @@ index 1bb7bccf..2501aba6 100644 args = append(args, "-netdev", fmt.Sprintf("socket,id=net0,fd={{ fd_connect %q }}", cfg.UsernetQEMUSock)) case firstUsernetIndex == -1: diff --git a/pkg/driver/qemu/qemu_driver.go b/pkg/driver/qemu/qemu_driver.go -index 5a68db80..d675c763 100644 +index 5a68db80..dba82218 100644 --- a/pkg/driver/qemu/qemu_driver.go +++ b/pkg/driver/qemu/qemu_driver.go -@@ -69,38 +69,49 @@ type LimaQemuDriver struct { +@@ -67,40 +67,81 @@ type LimaQemuDriver struct { + usernetStop context.CancelFunc + } - // startUsernet starts the instance's own gvisor netstack, as vz does: one - // network per instance, in the hostagent, where usernet.Outbound applies. +-// startUsernet starts the instance's own gvisor netstack, as vz does: one +-// network per instance, in the hostagent, where usernet.Outbound applies. -func startUsernet(ctx context.Context, inst *limatype.Instance) (client *usernet.Client, qemuSock string, stop context.CancelFunc, err error) { -+// -+// QEMU reaches it through qemuDgram (its dgram netdev) when it has one, and -+// qemuSock otherwise (see Cmdline). qemuDgram is nil on Windows. -+func startUsernet(ctx context.Context, inst *limatype.Instance) (client *usernet.Client, qemuSock string, qemuDgram *os.File, stop context.CancelFunc, err error) { - endpointSock, err := usernet.SockWithDirectory(inst.Dir, "", usernet.EndpointSock) +- endpointSock, err := usernet.SockWithDirectory(inst.Dir, "", usernet.EndpointSock) ++// usernetSockets are what connects QEMU to the instance's own gvisor netstack ++// (see startUsernet). QEMU gets the dgram end, or the socket path when its ++// netdevs have no dgram (see Cmdline): the netstack listens on the path only ++// then, as nothing else should be able to connect to it. ++type usernetSockets struct { ++ endpoint string ++ // qemu is the path of the socket netdev's listener. ++ qemu string ++ // dgramConn is the netstack's end of a datagram socketpair, qemuDgram ++ // QEMU's. Both are nil on Windows. ++ dgramConn net.Conn ++ qemuDgram *os.File ++} ++ ++func newUsernetSockets(inst *limatype.Instance) (*usernetSockets, error) { ++ endpoint, err := usernet.SockWithDirectory(inst.Dir, "", usernet.EndpointSock) if err != nil { - return nil, "", nil, err -+ return nil, "", nil, nil, err ++ return nil, err } - qemuSock, err = usernet.SockWithDirectory(inst.Dir, "", usernet.QEMUSock) +- qemuSock, err = usernet.SockWithDirectory(inst.Dir, "", usernet.QEMUSock) ++ qemu, err := usernet.SockWithDirectory(inst.Dir, "", usernet.QEMUSock) if err != nil { - return nil, "", nil, err -+ return nil, "", nil, nil, err ++ return nil, err } - os.RemoveAll(endpointSock) - os.RemoveAll(qemuSock) -+ dgramConn, qemuDgram, err := dgramPair() +- os.RemoveAll(endpointSock) +- os.RemoveAll(qemuSock) +- ctx, stop = context.WithCancel(ctx) +- err = usernet.StartGVisorNetstack(ctx, &usernet.GVisorNetstackOpts{ +- MTU: 1500, +- Endpoint: endpointSock, +- QemuSocket: qemuSock, +- Async: true, ++ os.RemoveAll(endpoint) ++ os.RemoveAll(qemu) ++ s := &usernetSockets{endpoint: endpoint, qemu: qemu} ++ s.dgramConn, s.qemuDgram, err = dgramPair() + if err != nil && !errors.Is(err, errors.ErrUnsupported) { -+ return nil, "", nil, nil, err ++ return nil, err + } - ctx, stop = context.WithCancel(ctx) - err = usernet.StartGVisorNetstack(ctx, &usernet.GVisorNetstackOpts{ - MTU: 1500, - Endpoint: endpointSock, - QemuSocket: qemuSock, -+ DgramConn: dgramConn, - Async: true, ++ return s, nil ++} ++ ++// closeQEMUEnd closes the file given to QEMU: it has its own copy once started. ++func (s *usernetSockets) closeQEMUEnd() { ++ if s.qemuDgram != nil { ++ s.qemuDgram.Close() ++ } ++} ++ ++// startUsernet starts the instance's own gvisor netstack, as vz does: one ++// network per instance, in the hostagent, where usernet.Outbound applies. ++// viaDgram says which end QEMU was given. ++func startUsernet(ctx context.Context, inst *limatype.Instance, s *usernetSockets, viaDgram bool) (client *usernet.Client, stop context.CancelFunc, err error) { ++ opts := &usernet.GVisorNetstackOpts{ ++ MTU: 1500, ++ Endpoint: s.endpoint, ++ Async: true, DefaultLeases: map[string]string{ networks.SlirpIPAddress: limayaml.MACAddress(inst.Dir), }, Subnet: networks.SlirpNetwork, - }) +- }) - if err != nil { - stop() - return nil, "", nil, err -+ if err == nil { -+ var subnetIP net.IP -+ subnetIP, _, err = net.ParseCIDR(networks.SlirpNetwork) -+ if err == nil { -+ return usernet.NewClient(endpointSock, subnetIP), qemuSock, qemuDgram, stop, nil -+ } } - subnetIP, _, err := net.ParseCIDR(networks.SlirpNetwork) - if err != nil { - stop() - return nil, "", nil, err -+ stop() -+ if dgramConn != nil { -+ dgramConn.Close() -+ qemuDgram.Close() ++ if viaDgram { ++ opts.DgramConn = s.dgramConn ++ } else { ++ opts.QemuSocket = s.qemu ++ if s.dgramConn != nil { ++ s.dgramConn.Close() ++ } } - return usernet.NewClient(endpointSock, subnetIP), qemuSock, stop, nil -+ return nil, "", nil, nil, err ++ ctx, stop = context.WithCancel(ctx) ++ err = usernet.StartGVisorNetstack(ctx, opts) ++ if err == nil { ++ var subnetIP net.IP ++ subnetIP, _, err = net.ParseCIDR(networks.SlirpNetwork) ++ if err == nil { ++ return usernet.NewClient(s.endpoint, subnetIP), stop, nil ++ } ++ } ++ stop() ++ if viaDgram { ++ s.dgramConn.Close() ++ } ++ return nil, nil, err } var _ driver.Driver = (*LimaQemuDriver)(nil) -@@ -366,12 +377,16 @@ func (l *LimaQemuDriver) Start(ctx context.Context) (chan error, error) { +@@ -365,22 +406,40 @@ func (l *LimaQemuDriver) Start(ctx context.Context) (chan error, error) { + SSHAddress: l.Instance.SSHAddress, VirtioGA: l.virtioPort != "", } ++ // The netstack starts once Cmdline has chosen how QEMU reaches it. ++ var socks *usernetSockets if limayaml.FirstUsernetIndex(l.Instance.Config) == -1 { - client, sock, stop, err := startUsernet(ctx, l.Instance) -+ client, sock, dgram, stop, err := startUsernet(ctx, l.Instance) ++ var err error ++ if socks, err = newUsernetSockets(l.Instance); err != nil { ++ return nil, err ++ } ++ qCfg.UsernetQEMUSock, qCfg.UsernetDgram = socks.qemu, socks.qemuDgram ++ defer socks.closeQEMUEnd() ++ } ++ netstackStarted := false ++ defer func() { ++ if socks != nil && socks.dgramConn != nil && !netstackStarted { ++ socks.dgramConn.Close() ++ } ++ }() ++ qExe, qArgs, err := Cmdline(ctx, qCfg) ++ if err != nil { ++ return nil, err ++ } ++ if socks != nil { ++ viaDgram := slices.ContainsFunc(qArgs, func(a string) bool { return strings.HasPrefix(a, "dgram,id=net0,") }) ++ client, stop, err := startUsernet(ctx, l.Instance, socks, viaDgram) if err != nil { return nil, err } - l.usernetClient, l.usernetStop, qCfg.UsernetQEMUSock = client, stop, sock -+ l.usernetClient, l.usernetStop, qCfg.UsernetQEMUSock, qCfg.UsernetDgram = client, stop, sock, dgram ++ netstackStarted = true ++ l.usernetClient, l.usernetStop = client, stop defer func() { -+ // QEMU has its own copy once started. -+ if dgram != nil { -+ dgram.Close() -+ } if l.qCmd == nil { stop() } -@@ -400,7 +415,7 @@ func (l *LimaQemuDriver) Start(ctx context.Context) (chan error, error) { + }() + } +- qExe, qArgs, err := Cmdline(ctx, qCfg) +- if err != nil { +- return nil, err +- } + + var vhostCmds []*exec.Cmd + if *l.Instance.Config.MountType == limatype.VIRTIOFS { +@@ -400,7 +459,7 @@ func (l *LimaQemuDriver) Start(ctx context.Context) (chan error, error) { } var qArgsFinal []string @@ -307,7 +382,7 @@ index 5a68db80..d675c763 100644 for _, unapplied := range qArgs { applied, err := applier.applyTemplate(unapplied) if err != nil { -@@ -900,6 +915,8 @@ func (l *LimaQemuDriver) GuestAgentConn(ctx context.Context) (net.Conn, string, +@@ -900,6 +959,8 @@ func (l *LimaQemuDriver) GuestAgentConn(ctx context.Context) (net.Conn, string, type qArgTemplateApplier struct { files []*os.File @@ -316,7 +391,7 @@ index 5a68db80..d675c763 100644 } func (a *qArgTemplateApplier) applyTemplate(qArg string) (string, error) { -@@ -938,6 +955,13 @@ func (a *qArgTemplateApplier) applyTemplate(qArg string) (string, error) { +@@ -938,6 +999,13 @@ func (a *qArgTemplateApplier) applyTemplate(qArg string) (string, error) { } return res }, diff --git a/patches/lima/0009-start-channels-buffered.patch b/patches/lima/0009-start-channels-buffered.patch new file mode 100644 index 00000000..214564f3 --- /dev/null +++ b/patches/lima/0009-start-channels-buffered.patch @@ -0,0 +1,34 @@ +Subject: [PATCH] instance: do not leave goroutines blocked after Start returns + +StartWithPaths races two goroutines (the hostagent's events and its exit) +on unbuffered channels and returns on the first. The loser blocks on its +send for as long as the program lives: one goroutine per start in a program +that outlives the call (limactl exits, so it does not show there). The +channels hold one value, so the sends complete; the exit goroutine also +finishes its Wait, which reaps the hostagent when it ends. + +No test: StartWithPaths starts a hostagent, which Lima's unit tests do not. + +--- +diff --git a/pkg/instance/start.go b/pkg/instance/start.go +index 0c70232a..489df5f9 100644 +--- a/pkg/instance/start.go ++++ b/pkg/instance/start.go +@@ -319,12 +319,15 @@ func StartWithPaths(ctx context.Context, inst *limatype.Instance, launchHostAgen + return err + } + +- watchErrCh := make(chan error) ++ // Buffered: the goroutine that loses the race below must not block on its ++ // send for as long as the program lives (it would, once StartWithPaths ++ // has returned). ++ watchErrCh := make(chan error, 1) + go func() { + watchErrCh <- watchHostAgentEvents(ctx, inst, haStdoutPath, haStderrPath, begin, showProgress) + close(watchErrCh) + }() +- waitErrCh := make(chan error) ++ waitErrCh := make(chan error, 1) + if haCmd != nil { + go func() { + waitErrCh <- haCmd.Wait() diff --git a/third_party/gvisor-tap-vsock/pkg/tap/switch.go b/third_party/gvisor-tap-vsock/pkg/tap/switch.go index 944b618f..e96a0fd9 100644 --- a/third_party/gvisor-tap-vsock/pkg/tap/switch.go +++ b/third_party/gvisor-tap-vsock/pkg/tap/switch.go @@ -96,6 +96,10 @@ func (e *Switch) Accept(ctx context.Context, rawConn net.Conn, protocol types.Pr e.disconnect(id, conn) }() if err := e.rx(ctx, id, conn); err != nil { + if ctx.Err() != nil { + // Closing the connection is how a program ends the switch. + return nil + } err := fmt.Errorf("cannot receive packets from %s, disconnecting: %w", conn.RemoteAddr().String(), err) log.Error(err) return err diff --git a/third_party/gvisor-tap-vsock/pkg/tap/switch_test.go b/third_party/gvisor-tap-vsock/pkg/tap/switch_test.go new file mode 100644 index 00000000..4039fe9b --- /dev/null +++ b/third_party/gvisor-tap-vsock/pkg/tap/switch_test.go @@ -0,0 +1,48 @@ +package tap + +import ( + "context" + "net" + "sync" + "testing" + + "github.com/containers/gvisor-tap-vsock/pkg/types" + "github.com/stretchr/testify/require" +) + +// readingConn says when it is first read. +type readingConn struct { + net.Conn + reading chan struct{} + once sync.Once +} + +func (c *readingConn) Read(b []byte) (int, error) { + c.once.Do(func() { close(c.reading) }) + return c.Conn.Read(b) +} + +// A connection that ends because the program cancelled the context is not an +// error; one that ends by itself is. +func TestAcceptEndedByContext(t *testing.T) { + for _, cancelled := range []bool{true, false} { + s := NewSwitch(false) + ours, theirs := net.Pipe() + conn := &readingConn{Conn: ours, reading: make(chan struct{})} + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { done <- s.Accept(ctx, conn, types.VfkitProtocol) }() + <-conn.reading + if cancelled { + cancel() + } + theirs.Close() + err := <-done + cancel() + if cancelled { + require.NoError(t, err) + } else { + require.Error(t, err) + } + } +} diff --git a/third_party/lima/pkg/driver/qemu/features_test.go b/third_party/lima/pkg/driver/qemu/features_test.go new file mode 100644 index 00000000..16228077 --- /dev/null +++ b/third_party/lima/pkg/driver/qemu/features_test.go @@ -0,0 +1,36 @@ +//go:build !windows + +// SPDX-FileCopyrightText: Copyright The Lima Authors +// SPDX-License-Identifier: Apache-2.0 + +package qemu + +import ( + "context" + "os" + "path/filepath" + "testing" + + "gotest.tools/v3/assert" +) + +// Each help text is its own: the features are searched as text, and a netdev +// list holding the accelerators' words would match them. +func TestInspectFeaturesOwnOutputs(t *testing.T) { + exe := filepath.Join(t.TempDir(), "qemu-fake") + script := `#!/bin/sh +case "$*" in + *-cpu*) echo "cpus: max" ;; + *-accel*) echo "accels: tcg" ;; + *-netdev*) echo "netdevs: dgram" ;; + *-machine*) echo "machines: virt" ;; +esac +` + assert.NilError(t, os.WriteFile(exe, []byte(script), 0o755)) + f, err := inspectFeatures(context.Background(), exe, "virt") + assert.NilError(t, err) + assert.Equal(t, string(f.AccelHelp), "accels: tcg\n") + assert.Equal(t, string(f.NetdevHelp), "netdevs: dgram\n") + assert.Equal(t, string(f.MachineHelp), "machines: virt\n") + assert.Equal(t, string(f.CPUHelp), "cpus: max\n") +} diff --git a/third_party/lima/pkg/driver/qemu/qemu.go b/third_party/lima/pkg/driver/qemu/qemu.go index 2501aba6..bf55ddb3 100644 --- a/third_party/lima/pkg/driver/qemu/qemu.go +++ b/third_party/lima/pkg/driver/qemu/qemu.go @@ -314,61 +314,40 @@ type features struct { } func inspectFeatures(ctx context.Context, exe, machine string) (*features, error) { - var ( - f features - stdout bytes.Buffer - stderr bytes.Buffer - ) - cmd := exec.CommandContext(ctx, exe, "-M", "none", "-accel", "help") - cmd.Stdout = &stdout - cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { - return nil, fmt.Errorf("failed to run %v: stdout=%#q, stderr=%#q", cmd.Args, stdout.String(), stderr.String()) + var f features + var err error + if f.AccelHelp, err = qemuHelp(ctx, exe, "-M", "none", "-accel", "help"); err != nil { + return nil, err } - f.AccelHelp = stdout.Bytes() - // on older versions qemu will write "help" output to stderr - if len(f.AccelHelp) == 0 { - f.AccelHelp = stderr.Bytes() + if f.NetdevHelp, err = qemuHelp(ctx, exe, "-M", "none", "-netdev", "help"); err != nil { + logrus.Warn(err) } - - cmd = exec.CommandContext(ctx, exe, "-M", "none", "-netdev", "help") - cmd.Stdout = &stdout - cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { - logrus.Warnf("failed to run %v: stdout=%#q, stderr=%#q", cmd.Args, stdout.String(), stderr.String()) - } else { - f.NetdevHelp = stdout.Bytes() - if len(f.NetdevHelp) == 0 { - f.NetdevHelp = stderr.Bytes() - } + if f.MachineHelp, err = qemuHelp(ctx, exe, "-machine", "help"); err != nil { + logrus.Warn(err) } - - cmd = exec.CommandContext(ctx, exe, "-machine", "help") - cmd.Stdout = &stdout - cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { - logrus.Warnf("failed to run %v: stdout=%#q, stderr=%#q", cmd.Args, stdout.String(), stderr.String()) - } else { - f.MachineHelp = stdout.Bytes() - if len(f.MachineHelp) == 0 { - f.MachineHelp = stderr.Bytes() - } + // Avoid error: "No machine specified, and there is no default" + if f.CPUHelp, err = qemuHelp(ctx, exe, "-cpu", "help", "-machine", machine); err != nil { + logrus.Warn(err) } + return &f, nil +} - // Avoid error: "No machine specified, and there is no default" - cmd = exec.CommandContext(ctx, exe, "-cpu", "help", "-machine", machine) +// qemuHelp is what exe prints for args, a help request. Each one has its own +// buffers: the features are searched as text, and one holding another's +// output would match words that are not its own. +func qemuHelp(ctx context.Context, exe string, args ...string) ([]byte, error) { + var stdout, stderr bytes.Buffer + cmd := exec.CommandContext(ctx, exe, args...) cmd.Stdout = &stdout cmd.Stderr = &stderr if err := cmd.Run(); err != nil { - logrus.Warnf("failed to run %v: stdout=%#q, stderr=%#q", cmd.Args, stdout.String(), stderr.String()) - } else { - f.CPUHelp = stdout.Bytes() - if len(f.CPUHelp) == 0 { - f.CPUHelp = stderr.Bytes() - } + return nil, fmt.Errorf("failed to run %v: stdout=%#q, stderr=%#q", cmd.Args, stdout.String(), stderr.String()) } - - return &f, nil + // on older versions qemu will write "help" output to stderr + if stdout.Len() == 0 { + return stderr.Bytes(), nil + } + return stdout.Bytes(), nil } // adjustMemBytesDarwinARM64HVF adjusts the memory to be <= 3 GiB, only when the following conditions are met: diff --git a/third_party/lima/pkg/driver/qemu/qemu_driver.go b/third_party/lima/pkg/driver/qemu/qemu_driver.go index d675c763..dba82218 100644 --- a/third_party/lima/pkg/driver/qemu/qemu_driver.go +++ b/third_party/lima/pkg/driver/qemu/qemu_driver.go @@ -67,51 +67,81 @@ type LimaQemuDriver struct { usernetStop context.CancelFunc } -// startUsernet starts the instance's own gvisor netstack, as vz does: one -// network per instance, in the hostagent, where usernet.Outbound applies. -// -// QEMU reaches it through qemuDgram (its dgram netdev) when it has one, and -// qemuSock otherwise (see Cmdline). qemuDgram is nil on Windows. -func startUsernet(ctx context.Context, inst *limatype.Instance) (client *usernet.Client, qemuSock string, qemuDgram *os.File, stop context.CancelFunc, err error) { - endpointSock, err := usernet.SockWithDirectory(inst.Dir, "", usernet.EndpointSock) +// usernetSockets are what connects QEMU to the instance's own gvisor netstack +// (see startUsernet). QEMU gets the dgram end, or the socket path when its +// netdevs have no dgram (see Cmdline): the netstack listens on the path only +// then, as nothing else should be able to connect to it. +type usernetSockets struct { + endpoint string + // qemu is the path of the socket netdev's listener. + qemu string + // dgramConn is the netstack's end of a datagram socketpair, qemuDgram + // QEMU's. Both are nil on Windows. + dgramConn net.Conn + qemuDgram *os.File +} + +func newUsernetSockets(inst *limatype.Instance) (*usernetSockets, error) { + endpoint, err := usernet.SockWithDirectory(inst.Dir, "", usernet.EndpointSock) if err != nil { - return nil, "", nil, nil, err + return nil, err } - qemuSock, err = usernet.SockWithDirectory(inst.Dir, "", usernet.QEMUSock) + qemu, err := usernet.SockWithDirectory(inst.Dir, "", usernet.QEMUSock) if err != nil { - return nil, "", nil, nil, err + return nil, err } - os.RemoveAll(endpointSock) - os.RemoveAll(qemuSock) - dgramConn, qemuDgram, err := dgramPair() + os.RemoveAll(endpoint) + os.RemoveAll(qemu) + s := &usernetSockets{endpoint: endpoint, qemu: qemu} + s.dgramConn, s.qemuDgram, err = dgramPair() if err != nil && !errors.Is(err, errors.ErrUnsupported) { - return nil, "", nil, nil, err + return nil, err } - ctx, stop = context.WithCancel(ctx) - err = usernet.StartGVisorNetstack(ctx, &usernet.GVisorNetstackOpts{ - MTU: 1500, - Endpoint: endpointSock, - QemuSocket: qemuSock, - DgramConn: dgramConn, - Async: true, + return s, nil +} + +// closeQEMUEnd closes the file given to QEMU: it has its own copy once started. +func (s *usernetSockets) closeQEMUEnd() { + if s.qemuDgram != nil { + s.qemuDgram.Close() + } +} + +// startUsernet starts the instance's own gvisor netstack, as vz does: one +// network per instance, in the hostagent, where usernet.Outbound applies. +// viaDgram says which end QEMU was given. +func startUsernet(ctx context.Context, inst *limatype.Instance, s *usernetSockets, viaDgram bool) (client *usernet.Client, stop context.CancelFunc, err error) { + opts := &usernet.GVisorNetstackOpts{ + MTU: 1500, + Endpoint: s.endpoint, + Async: true, DefaultLeases: map[string]string{ networks.SlirpIPAddress: limayaml.MACAddress(inst.Dir), }, Subnet: networks.SlirpNetwork, - }) + } + if viaDgram { + opts.DgramConn = s.dgramConn + } else { + opts.QemuSocket = s.qemu + if s.dgramConn != nil { + s.dgramConn.Close() + } + } + ctx, stop = context.WithCancel(ctx) + err = usernet.StartGVisorNetstack(ctx, opts) if err == nil { var subnetIP net.IP subnetIP, _, err = net.ParseCIDR(networks.SlirpNetwork) if err == nil { - return usernet.NewClient(endpointSock, subnetIP), qemuSock, qemuDgram, stop, nil + return usernet.NewClient(s.endpoint, subnetIP), stop, nil } } stop() - if dgramConn != nil { - dgramConn.Close() - qemuDgram.Close() + if viaDgram { + s.dgramConn.Close() } - return nil, "", nil, nil, err + return nil, nil, err } var _ driver.Driver = (*LimaQemuDriver)(nil) @@ -376,26 +406,40 @@ func (l *LimaQemuDriver) Start(ctx context.Context) (chan error, error) { SSHAddress: l.Instance.SSHAddress, VirtioGA: l.virtioPort != "", } + // The netstack starts once Cmdline has chosen how QEMU reaches it. + var socks *usernetSockets if limayaml.FirstUsernetIndex(l.Instance.Config) == -1 { - client, sock, dgram, stop, err := startUsernet(ctx, l.Instance) + var err error + if socks, err = newUsernetSockets(l.Instance); err != nil { + return nil, err + } + qCfg.UsernetQEMUSock, qCfg.UsernetDgram = socks.qemu, socks.qemuDgram + defer socks.closeQEMUEnd() + } + netstackStarted := false + defer func() { + if socks != nil && socks.dgramConn != nil && !netstackStarted { + socks.dgramConn.Close() + } + }() + qExe, qArgs, err := Cmdline(ctx, qCfg) + if err != nil { + return nil, err + } + if socks != nil { + viaDgram := slices.ContainsFunc(qArgs, func(a string) bool { return strings.HasPrefix(a, "dgram,id=net0,") }) + client, stop, err := startUsernet(ctx, l.Instance, socks, viaDgram) if err != nil { return nil, err } - l.usernetClient, l.usernetStop, qCfg.UsernetQEMUSock, qCfg.UsernetDgram = client, stop, sock, dgram + netstackStarted = true + l.usernetClient, l.usernetStop = client, stop defer func() { - // QEMU has its own copy once started. - if dgram != nil { - dgram.Close() - } if l.qCmd == nil { stop() } }() } - qExe, qArgs, err := Cmdline(ctx, qCfg) - if err != nil { - return nil, err - } var vhostCmds []*exec.Cmd if *l.Instance.Config.MountType == limatype.VIRTIOFS { diff --git a/third_party/lima/pkg/instance/start.go b/third_party/lima/pkg/instance/start.go index 0c70232a..489df5f9 100644 --- a/third_party/lima/pkg/instance/start.go +++ b/third_party/lima/pkg/instance/start.go @@ -319,12 +319,15 @@ func StartWithPaths(ctx context.Context, inst *limatype.Instance, launchHostAgen return err } - watchErrCh := make(chan error) + // Buffered: the goroutine that loses the race below must not block on its + // send for as long as the program lives (it would, once StartWithPaths + // has returned). + watchErrCh := make(chan error, 1) go func() { watchErrCh <- watchHostAgentEvents(ctx, inst, haStdoutPath, haStderrPath, begin, showProgress) close(watchErrCh) }() - waitErrCh := make(chan error) + waitErrCh := make(chan error, 1) if haCmd != nil { go func() { waitErrCh <- haCmd.Wait() diff --git a/www/src/i18n/en.ts b/www/src/i18n/en.ts index cd7f2dc7..1bda375f 100644 --- a/www/src/i18n/en.ts +++ b/www/src/i18n/en.ts @@ -107,7 +107,7 @@ export const en = { }, ], methodsTitle: 'Install the command', - methodsBody: 'Pick one. Each puts `agent-vm` on your `PATH` without root.', + methodsBody: 'Pick one. Each puts `agent-vm` on your `PATH`; only the Linux packages need root.', methodsLabel: 'Install method', methods: [ { @@ -122,6 +122,18 @@ export const en = { code: 'brew install sylvinus/tap/agent-vm', note: 'macOS, or Linux with Homebrew. `brew upgrade agent-vm` updates it.', }, + { + id: 'linux', + label: 'Linux package', + code: 'sudo apt install ./agent-vm_X.Y.Z_amd64.deb\nsudo dnf install ./agent-vm-X.Y.Z-1.x86_64.rpm', + note: 'Download the `.deb` or `.rpm` of your architecture (`amd64`, `arm64`; `x86_64`, `aarch64`) from the GitHub releases. It installs `agent-vm` in `/usr/bin` and recommends QEMU and ssh. Not in a package repository yet: install the new file to update.', + }, + { + id: 'nix', + label: 'Nix', + code: 'nix run github:sylvinus/agent-vm\nnix profile install github:sylvinus/agent-vm', + note: 'Linux, NixOS included. QEMU comes with it; ssh is the system’s. `nix profile upgrade agent-vm` updates it.', + }, { id: 'git', label: 'git', diff --git a/www/src/i18n/fr.ts b/www/src/i18n/fr.ts index 3c46f794..7b171787 100644 --- a/www/src/i18n/fr.ts +++ b/www/src/i18n/fr.ts @@ -109,7 +109,7 @@ export const fr: Dictionary = { }, ], methodsTitle: 'Installer la commande', - methodsBody: 'Au choix. Chacune met `agent-vm` dans ton `PATH`, sans root.', + methodsBody: 'Au choix. Chacune met `agent-vm` dans ton `PATH` ; seuls les paquets Linux demandent root.', methodsLabel: 'Méthode d\'installation', methods: [ { @@ -124,6 +124,18 @@ export const fr: Dictionary = { code: 'brew install sylvinus/tap/agent-vm', note: 'macOS, ou Linux avec Homebrew. `brew upgrade agent-vm` le met à jour.', }, + { + id: 'linux', + label: 'Paquet Linux', + code: 'sudo apt install ./agent-vm_X.Y.Z_amd64.deb\nsudo dnf install ./agent-vm-X.Y.Z-1.x86_64.rpm', + note: 'Télécharge le `.deb` ou le `.rpm` de ton architecture (`amd64`, `arm64` ; `x86_64`, `aarch64`) depuis les versions publiées sur GitHub. Il installe `agent-vm` dans `/usr/bin` et recommande QEMU et ssh. Pas encore dans un dépôt de paquets : installe le nouveau fichier pour mettre à jour.', + }, + { + id: 'nix', + label: 'Nix', + code: 'nix run github:sylvinus/agent-vm\nnix profile install github:sylvinus/agent-vm', + note: 'Linux, NixOS compris. QEMU est fourni ; ssh est celui du système. `nix profile upgrade agent-vm` le met à jour.', + }, { id: 'git', label: 'git', From a2ca74d51850551608a91bf5c9391d6969730e0b Mon Sep 17 00:00:00 2001 From: Sylvain Zimmer Date: Tue, 20 Oct 2026 22:51:06 +0200 Subject: [PATCH 5/5] coderabbits --- internal/cli/vms.go | 4 +++- internal/host/host_test.go | 4 ++-- internal/host/prereqs.go | 12 ++++++++---- internal/vm/lima_test.go | 3 +++ 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/internal/cli/vms.go b/internal/cli/vms.go index 319b87d7..9cebbd29 100644 --- a/internal/cli/vms.go +++ b/internal/cli/vms.go @@ -75,7 +75,9 @@ func (e *app) stopCmd(ctx context.Context, args []string) int { func (e *app) startVM(ctx context.Context, b vm.Backend, name string, log io.Writer) error { err := b.Start(ctx, name, log) if err != nil && ctx.Err() == nil { - _ = b.Stop(ctx, name) + if stopErr := b.Stop(ctx, name); stopErr != nil { + err = errors.Join(err, fmt.Errorf("stopping VM '%s' failed, it may still be running: %w", name, stopErr)) + } } return err } diff --git a/internal/host/host_test.go b/internal/host/host_test.go index d6e35470..9b5d3132 100644 --- a/internal/host/host_test.go +++ b/internal/host/host_test.go @@ -57,8 +57,8 @@ func TestQemuBinaryFor(t *testing.T) { }{ {"amd64", "apt-get", "qemu-system-x86_64", "sudo apt-get install qemu-system-x86"}, {"arm64", "apt-get", "qemu-system-aarch64", "sudo apt-get install qemu-system-arm"}, - {"amd64", "dnf", "qemu-system-x86_64", "sudo dnf install qemu-system-x86-core"}, - {"arm64", "dnf", "qemu-system-aarch64", "sudo dnf install qemu-system-aarch64-core"}, + {"amd64", "dnf", "qemu-system-x86_64", "sudo dnf install qemu-system-x86-core qemu-img"}, + {"arm64", "dnf", "qemu-system-aarch64", "sudo dnf install qemu-system-aarch64-core qemu-img"}, {"amd64", "", "qemu-system-x86_64", "your distribution's QEMU package (with Nix, 'nix run github:sylvinus/agent-vm' brings its own)"}, } { if bin, cmd := qemuBinaryFor(c.goarch, c.manager); bin != c.bin || cmd != c.cmd { diff --git a/internal/host/prereqs.go b/internal/host/prereqs.go index 462d6e27..efdd37e7 100644 --- a/internal/host/prereqs.go +++ b/internal/host/prereqs.go @@ -35,7 +35,7 @@ func qemuBinaryFor(goarch, manager string) (bin, install string) { } switch manager { case "dnf": - return bin, "sudo dnf install " + rpm + return bin, "sudo dnf install " + rpm + " qemu-img" case "apt-get": return bin, "sudo apt-get install " + deb } @@ -66,9 +66,13 @@ func LinuxPrereqs(w io.Writer) bool { } ok := true bin, install := qemuBinary() - if _, err := exec.LookPath(bin); err != nil { - fmt.Fprintf(w, "Error: Lima needs '%s' on PATH (not found).\n Install with: %s\n", bin, install) - ok = false + // Fedora packages qemu-img apart from the emulator, and Lima runs it on + // the disk image. + for _, b := range []string{bin, "qemu-img"} { + if _, err := exec.LookPath(b); err != nil { + fmt.Fprintf(w, "Error: Lima needs '%s' on PATH (not found).\n Install with: %s\n", b, install) + ok = false + } } _, err := os.Stat("/dev/kvm") switch { diff --git a/internal/vm/lima_test.go b/internal/vm/lima_test.go index 22450a01..2083d6df 100644 --- a/internal/vm/lima_test.go +++ b/internal/vm/lima_test.go @@ -312,6 +312,9 @@ func TestLimaStopOrphaned(t *testing.T) { if err != nil { t.Fatal(err) } + if inst.VMType != "qemu" { + t.Skip("only QEMU outlives the hostagent") + } // sleep, under the name of a program: QEMU's, or any other. spawn := func(name string) ([]byte, chan struct{}) { sleep, err := exec.LookPath("sleep")