fix(vmpool): correct the stale --cgroup comment at the jailer arg site

remote-worker/internal/vmpool/cgroup.go

@@ -16,15 +16,16 @@ import (
 // group. On start, sweep the slice for orphans from a previous incarnation."
 //
 // ARM-AGNOSTIC BY DESIGN (hardware-corrections D3): the cgroup-CREATION mechanism is
-// per-arm — Firecracker's jailer makes one via --cgroup/--parent-cgroup/--cgroup-version
-// (see firecrackerCgroupArgs in launcher_firecracker.go); Cloud Hypervisor has no jailer
-// equivalent, so its launcher places the VMM in a `systemd-run --scope` instead (see
-// launcher_chv.go). But SweepOrphans and vmCgroupPath below only ever walk directories
-// and read/write the two files (cgroup.procs, memory.max) that both mechanisms produce
-// under the SAME parent slice (spec §5.3). Neither function contains one line that knows
-// which VMM made a given subdirectory — that is the whole point: one sweep, at worker
-// start, covers whatever either arm left behind, including a mix of both across restarts
-// where SH_VMM was changed.
+// per-arm — on the Firecracker arm cgroupPool creates the cgroup and jailer is merely
+// relocated into it by --parent-cgroup (see cgroup_pool.go, and firecrackerCgroupArgs in
+// launcher_firecracker.go; jailer's own --cgroup created one per VM until #319); Cloud
+// Hypervisor has no jailer equivalent, so its launcher places the VMM in a `systemd-run
+// --scope` instead (see launcher_chv.go). But SweepOrphans and vmCgroupPath below only ever
+// walk directories and read/write the two files (cgroup.procs, memory.max) that both
+// mechanisms produce under the SAME parent slice (spec §5.3). Neither function contains one
+// line that knows which VMM made a given subdirectory — that is the whole point: one sweep,
+// at worker start, covers whatever either arm left behind, including a mix of both across
+// restarts where SH_VMM was changed.
 //
 // D8 (comm-truncation trap): this sweep kills by reading pids out of cgroup.procs, never
 // by matching a process name. That is not just simpler — it is the only form of this that
@@ -41,7 +42,8 @@ import (
 //
 //	/sys/fs/cgroup/microvm-vms.slice/
 //	├── microvm-worker.service/   <-- cgroup.procs holds the SWEEPING PROCESS's own pid
-//	├── vm-9/                     <-- Firecracker: jailer --id/--parent-cgroup/--cgroup
+//	├── pool-9/                   <-- Firecracker: cgroupPool's, jailer relocated in
+//	├── vm-9/                     <-- Firecracker pre-#319: jailer --id + its own --cgroup
 //	└── vm-vm-9.scope/            <-- Cloud Hypervisor: systemd-run --scope --slice=
 //
 // The original sweep's only filter was entry.IsDir(), so it read its own pid out of
@@ -97,11 +99,15 @@ import (
 // returning 0 swept while VMs leaked.
 const (
 	// vmIDPrefix prefixes every pool-generated VM id ("vm-" + a decimal sequence
-	// number). The Firecracker arm's cgroup directory IS that id: jailer, given
+	// number). That id is jailer's --id, so it names the VM's JAIL directory.
+	//
+	// It named the Firecracker arm's CGROUP directory too, until #319: jailer, given
 	// --cgroup, "will create a new cgroup named <id> for the microvm in the
-	// <cgroup_base>/<parent_cgroup> subfolder" (firecracker docs/jailer.md), so
-	// --parent-cgroup replaces the exec-file-name default entirely and the VM cgroup is
-	// an immediate child of the slice.
+	// <cgroup_base>/<parent_cgroup> subfolder" (firecracker docs/jailer.md), which made the
+	// VM cgroup an immediate child of the slice. That flag is gone, so a live VM now sits in
+	// a pooledCgroupPrefix directory (below) and a vm-<n> cgroup can only be an orphan left
+	// by an incarnation that predates #319. The name stays in this list for exactly that
+	// reason — the sweep must still recognise one.
 	vmIDPrefix = "vm-"
 
 	// chvScopeUnitPrefix prefixes the Cloud Hypervisor arm's `systemd-run --scope

remote-worker/internal/vmpool/launcher_chv.go

@@ -382,8 +382,13 @@ func chvCgroupSliceName(parentCgroup string) string {
 // Firecracker's jailer --parent-cgroup targets (spec §5.3), and
 // -p MemoryMax=<bytes> — the value that must equal vmpool.PerVMBytes(cfg), never a
 // second constant (D1's argument, mirrored here). Split out of Restore's argv
-// construction, like firecrackerCgroupArgs, so a test can assert the memory bound
-// agrees with PerVMBytes(cfg) without spawning systemd-run.
+// construction so a test can assert the memory bound agrees with PerVMBytes(cfg)
+// without spawning systemd-run.
+//
+// The arms are NOT symmetric about that bound any more, so do not read one off the
+// other: since #319 firecrackerCgroupArgs carries no bound at all — cgroupPool writes
+// memory.max and jailer gets only --parent-cgroup — whereas this argv still carries
+// its own, because systemd-run creates the scope's cgroup and nothing pools it.
 //
 // UNVERIFIED END TO END (see this file's package comment, D4 disposition): this task
 // has no host with systemd + KVM to run this argv for real. What IS verified is the

remote-worker/internal/vmpool/launcher_firecracker.go

@@ -144,14 +144,19 @@ func (o FirecrackerOptions) validate() error {
 	return nil
 }
 
-// firecrackerCgroupArgs returns the jailer flags that create and bound this VM's own
-// cgroup: --cgroup-version 2 (D2: jailer's own default is version "1", which this
-// cgroup2-only host does not have — always pass 2 explicitly), --parent-cgroup (so the
-// VM's cgroup lands under Task 17's systemd slice, spec §5.3), and --cgroup
-// memory.max=<bytes> (D1: the flag that actually creates the per-VM cgroup at all;
-// --parent-cgroup alone only relocates the process into the shared parent). Split out
-// of Restore's argv construction so a test can assert the memory bound agrees with
-// PerVMBytes(cfg) without spawning jailer.
+// firecrackerCgroupArgs returns the jailer cgroup flags: --cgroup-version 2 (D2: jailer's
+// own default is version "1", which this cgroup2-only host does not have — always pass 2
+// explicitly) and --parent-cgroup, which relocates the jailed process into the pooled
+// cgroup cgroupPool already created under Task 17's systemd slice (spec §5.3).
+//
+// It creates nothing and bounds nothing itself, and deliberately returns no --cgroup — #319
+// removed that flag, and the body says why. D1's per-VM memory.max is not lost, only moved:
+// cgroupPool writes it on that pooled cgroup, from the same PerVMBytes admission control
+// charges.
+//
+// Split out of Restore's argv construction so a test can assert those flags — and the
+// absence of --cgroup — without spawning jailer. The bound itself is asserted off the pool,
+// in the same test (cgroup_test.go).
 func firecrackerCgroupArgs(parentRel string) []string {
 	if parentRel == "" {
 		return nil
@@ -400,10 +405,8 @@ func (l *firecrackerLauncher) Restore(ctx context.Context, req RestoreRequest) (
 		"--gid", strconv.Itoa(l.opts.GID),
 		"--chroot-base-dir", l.opts.ChrootBase,
 	}
-	// firecrackerCgroupArgs appends --cgroup-version/--parent-cgroup/--cgroup: the last
-	// of the three is load-bearing, not decorative — without it jailer relocates this
-	// process into the shared parent cgroup but creates no cgroup of its own, so no
-	// per-VM memory.max is ever set (Task 17, hardware-corrections D1).
+	// firecrackerCgroupArgs appends --cgroup-version and --parent-cgroup, and deliberately
+	// not --cgroup: see its doc comment.
 	args = append(args, firecrackerCgroupArgs(cgroupRel)...)
 	// Coordinator finding #5: this launcher never issues PUT /network-interfaces —
 	// standbys are headless by construction, not by omission. Nothing below adds one.