Skip to content

feat: reachable VNC for --net cni, headless picker fix, parallel image pull#15

Merged
CMGS merged 8 commits into
masterfrom
feat/vnc-cni-native-pull
Jul 3, 2026
Merged

feat: reachable VNC for --net cni, headless picker fix, parallel image pull#15
CMGS merged 8 commits into
masterfrom
feat/vnc-cni-native-pull

Conversation

@CMGS

@CMGS CMGS commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Three independent improvements, each its own commit.

--net cni VNC reachable off-box

QEMU runs inside the CNI netns, so -vnc 127.0.0.1:590n only listens there. Bind VNC to a unix socket (visible across namespaces on the host FS) and front it with a small detached proxy (vm _vnc-proxy) that listens on 0.0.0.0:590n in the host netns; the proxy is started alongside qemu and torn down in stop/rm/launch-failure. Also always pass -display none — without --vnc, qemu fell back to GTK and aborted headless.

Verified: 10.5.17.79:5901RFB 003.008 from the internal network (no socat, no macOS Screen Sharing / TCC involved).

Headless OpenCore boots without the picker

Timeout alone doesn't prevent a stall: OpenCanopy cancels its countdown on the stray input from USB device enumeration and then waits at the picker forever. Set ShowPicker=false so OpenCore boots the default entry immediately with no picker UI.

Verified: guest boots and DHCPs in ~12s with no keypress nudge.

Parallel Range image pull

ghcr throttles and resets a single HTTP/2 stream on multi-GB blobs. Download the qcow2 layer in 8 concurrent Range chunks to a temp file (single-stream fallback when the registry doesn't support Range), verify sha256 against the layer digest, then import.

Checks

  • make lint clean on GOOS=darwin and GOOS=linux
  • go test -race passes
  • /simplify reuse/simplification/efficiency/altitude review applied (adopted utils.WritePIDFile/ReadPIDFile, io.NewOffsetWriter; the altitude finding is what turned the picker fix from a timed keypress into ShowPicker=false)

CMGS added 4 commits July 3, 2026 21:32
QEMU runs inside the CNI netns, so its -vnc 127.0.0.1 bind is unreachable
off-box. Bind it to a unix socket instead and front that on a host TCP port
with a small detached proxy, torn down with the VM. Also always pass
-display none: without --vnc, qemu fell back to GTK and aborted headless.
Timeout alone isn't enough: OpenCanopy cancels its countdown on the stray
input from USB device enumeration and then waits forever. ShowPicker=false
boots the default entry immediately with no picker UI.
ghcr throttles and resets a single HTTP/2 stream on multi-GB blobs. Fetch
the qcow2 layer in 8 concurrent Range chunks to a temp file (single-stream
fallback when Range is unsupported), verify the sha256 against the layer
digest, then import.
A persistent VNC port widens the host's exposure for the VM's whole
lifetime. Make it launch-scoped: vm start only enables VNC when --vnc is
given, and stop clears the recorded display/password with the qemu they
belonged to.
@CMGS

CMGS commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Added in 0d9608f — VNC is now decided per start, off by default:

  • vm start only enables VNC when --vnc N [--vnc-password ...] is given for that start; omitting it leaves no listener at all. vm stop clears the recorded display/password along with the qemu they belonged to, and kills the proxy.
  • Rationale: a persistent VNC port widens the host's exposure for the VM's whole lifetime; binding VNC to the guest's own IP isn't cleanly possible (the IP lives in the guest's stack; in-guest Screen Sharing is TCC-blocked headless), so the mitigation is to only open the host port while actually wanted.
  • Also fixes proxy teardown: the root command traps SIGTERM via signal.NotifyContext, so a TERMed proxy survived and held the port, shadowing the next start's proxy. stop now SIGKILLs it (stateless pipe — nothing to lose) and start reaps any stale proxy first.

Verified on the testbed: start without --vnc → 0 proxies, port closed; start with --vnc 1 → RFB reachable; stop → proxy gone.

CMGS added 2 commits July 3, 2026 22:11
…ch-scoped --vnc

TerminateProcess verifies the pidfile's PID still runs our _vnc-proxy
before signaling (a reused PID must not be killed) and waits for death;
zero grace escalates straight to SIGKILL since the proxy traps SIGTERM.
Also note on create/run/clone --vnc that the setting is cleared on stop.
…lure

Review follow-ups: the CNI proxy listens on all host interfaces, so
--vnc there now requires --vnc-password; the parent binds the proxy's
TCP port and passes the listener fd to the detached child, so a taken
port fails the launch (and kills qemu) instead of silently reporting a
VNC entry point that doesn't exist. Also stop tearing down the
persisted TAP/netns when a restart fails — only the create path cleans
up its own fresh network. README documents the per-mode exposure.
@CMGS

CMGS commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Both review findings addressed in c948fa9.

P1 — off-box exposure contract. --vnc with --net cni now hard-requires --vnc-password (checked at launch, covering run/start/clone/restore); the error spells out that the proxy listens on all host interfaces. README's loopback-only claim replaced with the per-mode reality: user/tap/bridge bind 127.0.0.1, cni fronts a proxy on 0.0.0.0:590n, password mandatory, launch-scoped. Kept the 0.0.0.0 bind rather than adding a --vnc-listen knob — off-box reach is the feature's purpose and password + per-start opt-in bound the exposure window; a listen-address flag can come later if a use case shows up.

P2 — no silent VNC gaps. The detached child's bind error was invisible to the parent, so the launch reported a VNC entry point that didn't exist. The parent now binds the TCP port itself and passes the listener fd to the child (ExtraFiles); a taken port fails the launch synchronously and the already-started qemu is killed. Verified: occupied 5901 → start fails, zero qemu left; freed → RFB served through the inherited fd.

Testing P2 also exposed a pre-existing bug this PR now fixes: the failed-qemu-launch path unconditionally tore down the VM's TAP/netns, which is correct for run (fresh net) but destroyed the persisted netns a stopped VM reuses — one failed restart bricked every later vm start with "Cannot open network namespace". Teardown-on-failure now lives only in Run's create path; verified stop → start keeps the netns.

… scaffolding

A failed set_password leaves qemu at password=on with no password, so
every VNC auth fails — kill qemu and error instead of warning. The cni
--vnc-password check now also runs before create scaffolds anything,
and a failed run removes the VM dir it just made: a leftover record
pointing at a torn-down netns bricked same-name retries and vm start.
@CMGS

CMGS commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Both round-3 findings fixed in the latest commit.

Password-set failure now fails the launch. setVNCPassword's own failure mode is qemu left at password=on with no password set — every VNC auth fails — so warning-and-continuing handed out a dead entry point. It now terminates qemu and returns the error, same policy as the proxy-bind failure.

cni --vnc-without-password is rejected before any state exists. The check runs in create ahead of scaffolding (shared by create/run), so nothing is written; the launch-time check stays for vm start/restore. On top of that, run is now atomic create+boot: any mid-launch failure removes the VM dir it just made along with its network, because the leftover record pointed at a torn-down netns and bricked both same-name retries ("already exists") and later vm start.

Verified on the testbed: no-password run → immediate error, no dir, no netns; occupied port + valid run → bind error, no dir/netns/qemu left, same-name retry then succeeds with RFB served; existing VM untouched.

@CMGS

CMGS commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Clone now runs the same pre-scaffold check as create: vm clone --net cni --vnc N without --vnc-password is rejected before any state is written (verified on the testbed — error, no VM dir). The stopped-record display nit (a never-started create/clone showing its recorded VNC column) is left as-is for now: it applies equally to vm create, exposes nothing (no listener until start, which re-decides VNC anyway), and making non-launching paths drop VNC fields would silently ignore user-passed flags — happy to revisit if the launch-scoped model should go further.

@CMGS CMGS merged commit fc2f7f6 into master Jul 3, 2026
3 checks passed
@CMGS CMGS deleted the feat/vnc-cni-native-pull branch July 3, 2026 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant