Files
egress-proxies-operator/docs/reviews/2026-09-17-kvm2-provider-review/REPORT.md
T
kacerrandClaude Opus 5 8fa9e8aad9 Housekeeping: review docs into docs/, refreshed agent guide and settings
Four unrelated working-tree changes, none of them touching operator
behaviour:

- review/*.md -> docs/reviews/2026-09-17-kvm2-provider-review/, byte for
  byte, so the kvm2 review sits with the rest of the docs under a name
  that says which review it was.
- AGENTS.md: replaced the generic kubebuilder scaffold guide with what is
  actually true of this repo — the real component layout, the lint/test
  commands that work here, and the behavioural gotchas (replace-on-change
  hashing, the cloud-init Secret label, in-memory leases).
- .claude/settings.json: the allow list still carried /Users/jan.novak
  paths from the macOS checkout; re-pointed at /home/novakj and collapsed
  the one-off entries into prefixes.
- config/samples/proxy_kvm2_crawl_only_one.yaml: a one-VM cut of the crawl
  pool, the rest of the members left commented out.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-18 08:12:27 +00:00

184 lines
28 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# kvm2 provider review: final report
Branch `feat/kvm2-provider` at `2510842`. Scope in `scope.md`; phase 1 detail in `sweep.md`;
phase 2 detail, measurements and unrun probes in `deep.md`.
**What this contains.** Confirmed findings only: the sweep findings (each reported by its
sweeper as verified in code) plus the escalations that came back CONFIRMED. Where a deep dive
refined a sweep finding, the refined version is given. **52 findings: 4 high, 20 medium, 28 low.**
**How far to trust it.** Nothing here was fixed or re-verified by a second reader except where
two or three reviewers reported the same thing independently (marked ×2/×3). Hypervisor access
was read-only, so every claim about what libvirt does on a *write* is from libvirt source or
memory, with the reviewer's stated confidence; the scripts that would settle them are in
`deep.md`. The kvm2 integration test has **not yet run on beelink** (Step 11 is unchecked in
the plan execution file; every `base-*` volume there predates kvm2), so no kvm2 behaviour in
this report has been observed against real libvirt.
Themes: **T1** unknown errors default to Transient and retry forever with no condition on the
Proxy · **T2** the ownership mark is enforced on Create/ListByTag but not Get/Delete ·
**T3** base image cache identity, lifetime and fencing · **T4** any caller can kill the shared
pooled link, and several callers misread the resulting error.
---
## High
### H1 — `markDead` blocks on a silent host and pins every reconcile worker (S1.1 + E1)
`internal/provider/kvm2/backend.go:138-148`, `ssh.go:109-114`
`markDead` runs `kill()` inside `once.Do` and only then closes `l.closed`. `kill` is `sshSocket.Close`, which closes the SSH channel first; that needs `ch.writeMu` and `t.mu`, both held by a writer stuck in a TCP write. Reproduced against x/crypto v0.54.0: with a 256 K send buffer the close was still blocked after 8 s, and a plain `client.Close()` released it instantly. Precondition: a bulk write in flight and a send buffer under the 2 MiB SSH window.
**Scenario.** Three Proxies placed on one host with an uncached image. Worker 1 uploads (`image.go:270`, 20 min budget); workers 2 and 3 park on the image lock (`kvm2.go:224-233`) with no deadline; `MaxConcurrentReconciles` is 3. Host is power-cut. The keepalive's `markDead` (`hosts.go:158`) blocks in the channel close; worker 1's `call` cannot see `done`, `l.closed`, or even a ctx cancel (it queues in `once.Do`). For ~15 min no Proxy on any provider reconciles. No `ReconciliationTimeout` is set, and one alone would not release worker 1.
**Fix.** Close the TCP connection first in `sshSocket.Close` (`s.client.Close()` before, or instead of, `s.Conn.Close()`); in `markDead`, `close(l.closed)` before `kill()` or run `kill` in a goroutine. Regression test with a frozen proxy and small `SO_SNDBUF`. Optionally set `ReconciliationTimeout`.
### H2 — An error during `StorageVolUpload` wedges go-libvirt's stream sender forever (S4.1)
`internal/provider/kvm2/image.go:273` (upstream `rpc.go:290-308`, `socket/socket.go:347-376`)
`abort` and `outErr` are unbuffered. On a reader error `SendStream` blocks forever on `outErr <-`; on a server-side error `requestStream` blocks forever on `abort <- true`. A socket kill does not unblock them (E6 found why: `listen` blocks in `callback`'s send under `cmux`), contrary to the comment at `backend.go:152-156`.
**Scenario.** A mirror truncates the body, or libvirtd hits ENOSPC mid-upload. The caller is rescued only after the 20 min transfer budget; each failed upload leaks 1-2 goroutines plus a 4 MiB buffer, accumulating under reconcile retries; `discardVolume` (`image.go:292`) deletes the base before the deferred `fresh.Close()` (`:265`), so the file is unlinked while the server stream holds the fd. `TestEnsureImage_shortReadRemovesVolumeAndIsTransient` cannot catch it: the fake returns the reader error immediately.
**Fix.** Have `countingReader` convert a read error to `io.EOF` and report it out of band; the existing `readErr` / `read != size` checks (`image.go:281-290`) still catch truncation. Close the dedicated connection before `discardVolume`. Consider an upstream patch for the unbuffered channels.
### H3 — `address()` swallows every error, demoting healthy Running proxies (S3.1)
`internal/provider/kvm2/get.go:104-119`
Every non-`context` error from both address sources becomes `("", nil)`, so `mapState(running, "")` = Provisioning.
**Scenario.** (a) On beelink the ARP source never answers (`kvm2-provider-spec.md:146`), so a `qemu-guest-agent` restart in the guest (`ErrAgentUnresponsive`) empties the address. (b) Another reconcile's cancel kills the shared pooled link between `DomainGetState` and the address read; the "connection is closed" error carries no ctx sentinel, so `Get` *succeeds* with `IP=""`. Then `proxy_controller.go:208-211` clears `status.IP` and sets `Provisioned=False`, and `internal/health/engine.go:200-207` prunes the proxy's health state. One RPC hiccup pulls a working proxy out of rotation. `TestGet_addressErrorIsProvisioning` pins this.
**Fix.** Return Transient from `Get` when the address read fails with a connection-class error (`isDead(be)` or the closed-link error) and return NotFound on `ErrNoDomain`; keep "no address yet" only for agent-unavailable/ARP-empty on a domain that has never reported an address. Consider having the controller keep the last known IP while an instance is Running.
### H4 — The base image cache is shared and unfenced across provider entries (S7.1 ×2, E3-3a/3b, E4)
`internal/provider/kvm2/volume.go:17-25`, `image.go:113-146`, `:235-245`, `kvm2.go:111`; `internal/provider/kvm/image.go:44-57`; `config/manager/providers_config.yaml`, `config/samples/providers-config.yaml:41,73`
The base is `base-<key>.<format>` with a `.sha256` marker, byte-identical in the kvm provider, in the same default pool, under **separate, per-entry locks**. The shipped config puts `kvm-home` and `kvm2-home` on `beelink`, key `debian-13`, pool `default`, and the docs say both types may share a host. On the host both live (kvm-made) overlays back onto that very file. Neither the name nor the marker records the URL.
**Scenarios.**
1. *Wrong image, silently.* The two entries' URLs drift and neither sets `sha256`: whoever fetched first owns the file. Likewise, editing `images.<key>.url` alone is a silent no-op.
2. *Digest mismatch (3a).* One entry pins `sha256: D2` while the cached base is D1: `cachedImage` (`image.go:129` → `:145`) deletes the base under the other provider's live overlays. libvirt refuses a delete only on `in_use`/`building` (`storage_driver.c:1844-1856`, read at v11.6.0). With two different pins, every Create on either side deletes and refetches.
3. *Concurrent cold fetch (3b, by source, unexecuted).* B finds A's base-without-marker and deletes it (`image.go:117-118`), recreates it and uploads. libvirt holds `in_use` only during stream setup (`storage_driver.c:2421-2427`). A's upload completes into the unlinked inode, A writes the marker, and `StorageVolGetPath` resolves to B's partial file: **A boots a guest from a half-written base.** If B is kvm2, its `writeMarker` then fails `ErrStorageVolExist` and `image.go:241-243` discards the complete base under A's running domain. If the process dies during B's upload, a truncated base stays under a valid marker forever, since nothing re-hashes the base.
The rollout/leader-handover variant of this was **refuted** for the shipped manifests (lease held until the old pod dies; SIGKILL at 10 s).
**Fix.** kvm2-specific prefix including the entry name, or one process-wide image lock table; reject two kvm/kvm2 entries with the same address+socket+pool at config load. Upload to `<base>.part-<random>`, verify, then publish the final name; never delete a name this call did not create, in particular drop `discardVolume(base)` on a marker `ErrStorageVolExist`. Record size and URL next to the digest and check the base's allocation before trusting a marker.
---
## Medium
### M1 — Paused domains are destroyed (S2.2 ×2)
`create.go:230-231`, `get.go:56`, `state.go:16-27`. `get.go:56` discards the `DomainGetState` reason; paused/pmsuspended map to Stopped; `reconcileManaged` (`proxy_controller.go:217-225`) then deletes and recreates. The "virsh resume it" guard in `ensureRunning` only fires before an ID is persisted. **Scenario:** the pool fills and QEMU pauses the guest with `PAUSED_IOERROR` (or `virsh suspend`/save/snapshot/migrate); at the next drift poll the VM and its disks are gone and the recreate refills the pool. **Fix:** keep the reason; map paused to a non-destructive state (Provisioning or a new Degraded) except for reasons that are terminal.
### M2 — `Get` and `Delete` do not enforce the kvm2 mark (S2.3 ×2, E3-2) [T2]
`get.go:52-73`, `delete.go:55-98` vs `create.go:80-82`. IDs are byte-identical to the kvm provider's (`kvm/id.go:11-13`). **Scenario:** an entry is re-typed `kvm` → `kvm2` under the same name without first deleting its Proxies (the documented migration, out of order): `status.ProviderID` survives, `Get` returns the v1 domain with `UID: ""`, and anything but Running leads to kvm2 destroying a domain the docs say it can never touch. A tampered `status.providerID` such as `hosts/beelink/domains/docker-29` destroys an unrelated VM. Nothing prevents a re-type (no webhook, `config.go:93-135`). The reverse direction (kvm provider against kvm2 domains) has no guard at all (`kvm/kvm.go:177-181`). **Fix:** `removeDomain` and `get` require `meta.managed()`; pass the expected UID to Delete or compare `inst.UID` with `p.UID` in the controller.
### M3 — Volumes created before `DomainDefineXML` are unreachable when Create returns no ID (S2.1)
`create.go:107-114`. **Scenario:** overlay and seed succeed, define fails Permanent (or the manager dies). No `ProviderID` is set; on Proxy deletion `reconcileDelete` takes the empty-ID shortcut (`proxy_controller.go:270-274`); `ListByTag` lists domains only. `proxy-<x>.qcow2` and `-seed.iso` leak permanently. Five such leftovers already sit in beelink's `default` pool (Appendix C). **Fix:** on a failed define, best-effort remove the two volumes before returning; or have `Delete`/GC derive volume names from the Proxy UID.
### M4 — The pool-space precheck reads a cached, reserve-inflated number (E8)
`create.go:92-100`, `image.go:450-458`, `check.go:106-112`. Measured on beelink: libvirt's `available` is cached per pool (three pools on one filesystem report two values, 1.68 GB apart; frozen over 80 s while `f_bfree` moved) and is `f_bfree`, **24.7 GB above** what unprivileged QEMU can use on the kvm2 filesystem. The only refresh is after an image upload. **Scenario:** with `f_bavail` between 0 and ~10.7 GB the precheck passes for a 10 GiB disk, `ErrQuotaExceeded` never fires, and Create, the upload and the guests hit ENOSPC (feeding H2 and M1). `poolAvailableGiB` in ProviderHealth over-reports the same way. **Fix:** `StoragePoolRefresh` right before `checkPoolSpace` (not in Check); add headroom to `neededBytes` from the discarded `capacity` or a `poolReserveGiB` setting; make the fake's pool info cached-until-refresh.
### M5 — An overlay smaller than the base's virtual size is accepted; the Proxy hangs in Provisioning (S5.6 + E10)
`volume.go:72`, `create.go:107,158`, `internal/provider/config_kvm.go:133`. Measured locally: `qemu-img create` with a capacity below the backing file exits 0; reads past the capacity fail; a GPT base under a short overlay shows only the protective MBR. Bases on the host are 3 GiB virtual. **Scenario:** `diskSizeGb: 2` validates; Create succeeds; the guest has no root device and never reports an address; `Get` says Provisioning forever (no provisioning deadline exists), nothing names `diskSizeGb`, and raising it later does not repair that Proxy. libvirt's pass-through is ~90% from source; the 200 KiB probe is in `deep.md`. **Fix:** add `StorageVolGetInfo` to the backend seam and fail Permanent when the base's capacity exceeds the overlay's; same check on a reused overlay.
### M6 — A peer-close right after ConnectOpen wedges a pooled link forever (E6a)
`backend.go:177-178`, `:229-236`, `hosts.go:75`, `errors.go:55` (upstream race `libvirt.go:297/305/821`, reported by `-race`). **Scenario:** libvirtd or the forwarded socket goes away within ~1 ms of a successful open (daemon restart). `Connect()` returns nil with a `Disconnected()` channel nothing will close; every later RPC fails `EINVAL`, classified Transient; no result marks the link dead; the SSH keepalive keeps succeeding. The host answers "invalid argument" until the operator restarts. Wedged in 546/1000 cycles over real SSH in the reviewer's harness. The hypothesised trigger (kvm2's own kill) is harmless. **Fix:** in `call`, `markDead` on `syscall.EINVAL` / `libvirt.ErrInterrupted`; or wrap the `net.Conn` so a failed `Read` marks the link dead.
### M7 — Permanent host/config rejections retry forever with nothing in status (S5.2 ×2, S3.5, E9b) [T1]
`errors.go:58-71`, `xml.go:26-27,194-206`, `proxy_controller.go:344-345`. `x86_64`/`q35`/`kvm`/host-passthrough/efi are unconditional with no capability gate; code 67 (`ErrConfigUnsupported`) and the bare `libvirt.ErrUnsupported` sentinel are unmapped; `providerFailure`'s Transient arm sets no condition. **Scenario:** an aarch64 or KVM-less host is added; Check says Ready; every Create retries an impossible define forever, rerunning `writeSeed` each time; a missing bridge (38, at start) or missing OVMF (9) behaves the same and Check looks at neither. "Unknown means retry" is a recorded spec decision (`kvm2-provider-spec.md:317`), but it never considered 67. **Fix:** 67 → Permanent on create; `ErrUnsupported` → Permanent; leave 9/38/1/84. Surface a non-latching condition for Transient create failures, or check domain capabilities and the bridge in the health probe.
### M8 — Seed ISO I/O failures latch as Permanent with no requeue (S5.1) [T1, inverted]
`create.go:199-201`, `seed.go:66-74`. `permanentf("building seed ISO …: %v")` covers a missing `/tmp`, ENOSPC, and even a `RemoveAll` failure on an otherwise successful build. **Scenario:** node disk pressure for a minute leaves the Proxy unprovisioned until a spec edit or the ~10 h resync. **Fix:** Permanent only for the empty-name guard; wrap the rest Transient with `%w`.
### M9 — `HEAD` 2xx without a usable Content-Length becomes a latching Permanent (S4.2) [T1]
`image.go:434-439`, `:391-396`. The GET fallback fires only on non-2xx. **Scenario:** a CDN (or Go's own `net/http`) answers HEAD with `Content-Length: 0`; the Proxy never provisions although GET works. **Fix:** fall back to GET whenever HEAD yields no positive length.
### M10 — A completed upload is discarded when the pooled link died meanwhile (S4.3) [T4]
`image.go:235-245`. Refresh and `writeMarker` run on the pooled `be` after a transfer on a dedicated link. **Scenario:** any other reconcile's cancel during a 20 min download kills the pooled link; the verified base is deleted and refetched. **Fix:** run the refresh and marker on the still-open dedicated connection.
### M11 — Precheck uses HEAD's length, the volume uses GET's, and there is no size cap (S4.4)
`image.go:419-443`, `:226-230`. **Scenario:** HEAD says 1 MB, GET says 4 TiB: a sparse 4 TiB volume is created and the stream fills the host pool. **Fix:** re-check space against the GET length; add `maxImageBytes`.
### M12 — `placement.image` is an unvalidated user-supplied URL fetched by the operator (S4.5)
`image.go:59-65`, `internal/provider/config_kvm.go:101-104`. No CRD validation, no host allowlist, `http.DefaultClient` follows redirects, and a URL image can never carry a digest. **Scenario:** anyone who can create a Proxy makes the operator issue requests to cluster-internal addresses and boots arbitrary bytes on the egress bridge. **Fix:** a config switch to allow URL images at all (default off) or a host allowlist; `CheckRedirect` policy; block link-local/private ranges.
### M13 — A `sha256` change replaces the base in place under live overlays (S4.6 ×3)
`image.go:118,129,145`. No in-use check, `V(1)` log; a mistyped digest deletes the base before anything is verified. The kvm2 sample omits the warning the kvm sample carries. Whether guests actually corrupt is Appendix A (E7). **Fix:** on a digest mismatch return Permanent "add the image under a new key"; or put the digest in the volume name.
### M14 — IP detection takes the first IPv4 from any guest interface (S3.2)
`state.go:35-49`. `Hwaddr` is available but unused. **Scenario:** `docker0`/`tailscale0` comes up before the DHCP lease; the Proxy latches `Provisioned=True` with 172.17.0.1 and is never corrected. **Fix:** match the interface by the domain NIC's MAC.
### M15 — A link that dies mid-list yields a silently partial `ListByTag` (S3.3) [T4]
`list.go:38-45`. The guard tests `ctx.Err()`, but a killed link has no ctx sentinel. GC only under-deletes, so nothing wrong is removed, but the sweep covers a fraction of the fleet and says so nowhere. **Fix:** fail the list on any connection-class error (`isDead(be)`).
### M16 — `KVMHost.Address` is not validated as `host:port` (S7.2) [T1]
`internal/provider/config_kvm.go:180-182`. **Scenario:** `address: beelink` loads; every dial fails "missing port in address", classified Transient, and Check reports `Unreachable`. **Fix:** `net.SplitHostPort` at load, or default `:22`.
### M17 — Plain `http://` images with no digest are accepted silently (S7.3)
`internal/provider/config_kvm.go:101-104,142-147`. The root disk of every VM can come from an unauthenticated channel and the marker then pins it. **Fix:** require `sha256` for `http://` catalogue entries, or warn loudly at load.
### M18 — The fake ignores the metadata namespace (S7.4) [T2]
`fake_test.go:362-376`, `:259-282`. No unit test can build a kvm-v1-marked domain and assert `ListByTag` skips it; if that filter regressed, kvm2 would hand kvm's VMs to orphan GC with the suite green. E3/E9 observed that real libvirt 11.6 does scope by namespace. **Fix:** key the fake's metadata on the element's namespace; add the test.
### M19 — The integration target's timeout can skip VM cleanup on the real host (S7.5)
`Makefile:75`. `-timeout 40m` is fixed while two tests each take up to `KVM2_TEST_TIMEOUT` (20 m) plus 5 m cleanup. **Scenario:** a cold download or a raised env timeout trips go test's own timeout before `t.Cleanup` (`kvm2_integration_test.go:373-382`); a domain and two volumes stay on beelink under a random name. **Fix:** derive `-timeout` from the env var with margin; print the name up front; add a `review`-style sweep-by-prefix cleanup target.
### M20 — `sshSocket.keepalive` has no test (S1.2)
`ssh.go:119-147`. It is the only detector of a silently dead pooled link between calls, and `testSSHServer` already services global requests. **Fix:** an end-to-end test that stops the server's request handling and asserts `markDead`.
---
## Low
| ID | Where | Defect → scenario | Fix sketch |
|---|---|---|---|
| L1 (S1.3) | `ssh.go:120` | `NewTicker` panics on interval ≤ 0 in a goroutine → a future `libvirtDialer{}` literal kills the operator on first dial | default the interval when ≤ 0 |
| L2 (S1.4) [T1] | `errors.go:45-51` | `*knownhosts.RevokedError` unrecognised → a revoked host key retries forever | classify it Permanent |
| L3 (S1.5) | `ssh.go:167-172` | `HostKeyAlgorithms` empty → a host pinned by RSA only but serving ed25519 reports a Permanent "mismatch" | on `KeyError` with `Want`, retry with those key types; or document `ssh-keyscan` without `-t` |
| L4 (S2.4) | `create.go:87` | Adoption path starts a defined domain without checking its disks → missing overlay/seed retries forever (E5's EIO residual lands here) | on a storage error in the adoption branch, undefine and fall through to a full create |
| L5 (S2.5) [T1] | `delete.go:15` | `undefineFlags` lacks `CheckpointsMetadata` → an admin/backup checkpoint wedges the finalizer | add the flag |
| L6 (S3.4) | `get.go:104-119` | `ErrNoDomain` swallowed → a vanished domain reads as Provisioning for one poll; comment about ctx-only errors is wrong (a budget expiry never falls back to ARP) | return NotFound; fix the comment |
| L7 (S4.7) | `image.go:62` | URL images assumed qcow2/uefi → a raw URL image fails to boot with a misleading error | sniff the qcow2 magic from the first bytes |
| L8 (S4.8) [T3] | `volume.go:22-25` | 64-bit URL key, namespace shared with catalogue keys → colliding URLs or a 16-hex catalogue key share a base | longer key and a distinct `url-` prefix |
| L9 (S5.3, narrowed by E2) | `metadata.go:44-50` | future-dated mark → a true orphan is never collected. (The zero-time half is benign: `gc.go:118` checks the UID first.) | clamp future timestamps to now |
| L10 (S5.4) | `seed.go:48-53` | `rc-update` fallback unguarded under `set -e` → agent installed but not started on non-systemd, non-OpenRC images | `elif command -v rc-update`, as the kvm provider does |
| L11 (S5.5) | `xml.go:178-193` | `VCPUs`/`MemoryMiB` not validated in the builder (latent) | reject ≤ 0 |
| L12 (S5.7) | `internal/provider/config_kvm.go:186-188` | `bridge`/`pool` only non-empty → opaque libvirt error at Create | apply `kvmIdentifier`-style validation |
| L13 (S5.8) | `seed.go:107-108` | multipart or headerless configured `vendorData` silently dropped with `installGuestAgent` | reject at load what cloud-init will not recognise |
| L14 (S6.1) | `check.go:90`, `backend.go:162,351` | `Close()` can send `ConnectClose` for up to 5 s after the round deadline | `markDead` when `call` refuses on a done ctx, or skip the RPC in Close then |
| L15 (S6.2) | `check.go:57-58` | Latency includes the teardown RPC | stop the clock before Close |
| L16 (S6.3) | `check.go:66` | round-wide `ctx.Err()` masks a per-host reason landing on the deadline | classify the error first; Timeout only if it carries the ctx error |
| L17 (S6.4) | `check.go:135-148`, `internal/providerhealth/monitor.go:113` | a 10 s round timeout collapses Unreachable into Timeout | validate round timeout > dial budget, or shrink the dial budget to fit |
| L18 (S6.5) | `check.go:101` | unbounded remote `hostname` in status `details` → status patch "too large" | truncate; add `maxLength` to the CRD |
| L19 (S6.6) | `check.go:41-45` | unbounded per-host fan-out, key re-read every dial | cap concurrency; cache credentials |
| L20 (S7.6) [T1] | `internal/provider/config_kvm.go:133`, `image.go:466` | no upper bound on `diskSizeGb`; `neededBytes` wraps at 2^34 → precheck passes; huge `memoryMiB` retries forever | bound both at load |
| L21 (S7.7) | `fake_test.go:190-208` | fake accepts calls on a done ctx → `discardVolume`'s `WithoutCancel` is untested | make `enter()` refuse like `call()` |
| L22 (S7.8) | `fake_test.go:579-584` | fake `Close()` leaves the backend live → use-after-close passes in tests | mark dead on Close |
| L23 (S7.9) | `Makefile:60-75`, `.gitea/workflows/build.yaml:27-32` | nothing compiles `-tags=integration` | add `go vet -tags=integration ./...` to the `check` job |
| L24 (S7.10) | `image.go:228,283-289,385-395` | full image URL, including any userinfo, in logs and wrapped errors | use `urlHost`/redacted URL everywhere; reject userinfo at load |
| L25 (S7.11) | `kvm2_integration_test.go:89,307` | `KVM2_TEST_KEEP=0` enables keep; 100 ms wall-clock assertion on `New()` | parse the bool; drop or loosen the timing check |
| L26 (S7.12) | `config/manager/manager.yaml:163-178`, `seed.go:56-74` | cloud-init payloads staged on node-local disk; comment names kvm only | `medium: Memory`; fix the comment |
| L27 (E6 side) | go-libvirt `socket/socket.go:233-234` | packet length < 28 panics the listen goroutine and the operator; a huge length allocates up to 4 GiB → `host.Socket` pointed at a non-libvirt socket (reproduced) | upstream bounds check; until then validate the socket in Check before pooling |
| L28 (E6 side) | go-libvirt `rpc.go:76-84` | duplicate reply deadlocks `callback` under `cmux`, leaking 3 goroutines that survive a socket kill (measured +420/200 cycles); same mechanism keeps H2's leak alive | upstream: send outside the lock |
---
## Appendix A — PLAUSIBLE, with what would settle it
- **E2 — a padded or case-changed `<uid>` in the GC mark makes GC delete a live Proxy's VM.** Reproduced on a verbatim copy of the parser and the `gc.go` switch; there is no second guard before `Delete`. No trigger libvirt produces on its own was found (it re-indents between children only), so it needs a hand edit or a reformatting tool. The kvm provider has the same field, and the two live VMs are kvm v1. *Settle:* the `virsh metadata --set` round trip in `deep.md` on a scratch libvirt. *The fix is three `TrimSpace` lines and worth making regardless.*
- **E7 — in-place base replacement corrupts existing overlays.** The delete is confirmed (M13). Corruption needs a new QEMU process on an old overlay; the controller never does that, but `libvirt-guests` is enabled on beelink and restarts guests after a host reboot. *Settle:* `grep -Ev '^\s*(#|$)' /etc/default/libvirt-guests` on beelink, and the `review-e7-*` inode probe in `deep.md`.
- **Unexecuted links inside confirmed findings.** H4-3b: libvirt allows `vol-delete` during an upload (read in 11.6.0 source; the `probe-race`/`e4probe` scripts). M5: libvirt passes a short capacity through (~90%; `review-e10-short`). H1: the real path's send buffer is under ~2.2 MB (`ss -tmi` during an upload). E9a: duplicate volume → code 90 (~95%; `review-e9-dup`).
## Appendix B — REFUTED
- **E5** overlay left corrupt by an abandoned `StorageVolCreateXML`: libvirtd finishes or unlinks the build regardless of the client (~85%, source memory); creation is 2-11 ms on the host; the residual path fails loudly.
- **E3-1** GC reconciling one provider's list against another's Proxies: the live set is global and the list is namespace-filtered (observed on the host).
- **E3-2** re-type destroying *another* Proxy's instance: the ID always names the Proxy's own UID-derived domain (the remaining hole is M2).
- **E4 rollout/leader handover** deleting a base mid-upload: the lease is held until the old pod dies; SIGKILL at 10 s; the residual window is theoretical and self-healing.
- **E6b** goroutine/fd accumulation from per-round check dials: +0/+0 over 1000 cycles on three transports.
- **E9a** idempotency codes differing from the fake: 42, 49, 50, 80 observed on libvirt 11.6; 90 and 55 sourced at high confidence.
- **E2 (half)** an unparsable `createdAt` deleting a live VM: `gc.go:118` checks the UID first.
- **E7 (half)** marker-missing trigger without an admin edit: marker errors are returned, not treated as unreadable.
- **No XML injection**: three sweepers independently found every value goes through `encoding/xml`.
## Appendix C — Out of scope but noticed
- **Orphan volumes on beelink, pool `default`:** `proxy-7hjzuf3d3w2mjnye.qcow2` + `-seed.iso`, `proxy-qxge3dbm2mnpc3ih.qcow2` + `-seed.iso`, `proxy-sbs36ruo73sxxtqc.qcow2`. 196 KiB each, root-owned, no domain. Reported independently by four deep reviewers; they are M3 in the wild (likely from kvm v1 integration runs). Nothing was deleted.
- **The kvm2 integration test has not run on beelink** (plan execution Step 11 unchecked).
- **The kvm provider shares most of these defects:** untrimmed mark UID (E2), no mark check on Get/Delete and adoption of any same-named domain (`kvm/kvm.go:177-181`), short-overlay hole (`kvm/volume.go:24-32`), same base names.
- **No provisioning deadline in the controller:** any VM that boots but never reports an address stays Provisioning forever with no signal (what makes M5 silent).
- **`providerFailure` sets no condition for Transient errors** (`proxy_controller.go:344-345`), which is what makes all of T1 invisible.
- **Manager wiring:** no `ReconciliationTimeout`, no `LeaderElectionReleaseOnCancel`, default RollingUpdate with one replica, 128Mi memory limit (an OOM mid-upload is one way to H4's truncated base).
- **Host key naming:** `~/.ssh/kvm_known_hosts` pins beelink under its Tailscale FQDN only, while the sample config says `address: beelink:22`; an operator using that file would fail host key verification (seen from this workstation; not verified from inside the operator).
- **Dropped escalations not investigated:** D1 delete-during-open-stream disk-space effect, D2 cloud-init `text/plain` vendor-data dispatch, D3 ProviderHealth status churn, D4 nested `kvm-ssh` volumeMount under the read-only config mount, D5 whether error strings with image URLs reach Proxy status.