diff --git a/docs/concepts/access-and-auth.md b/docs/concepts/access-and-auth.md index 7107883..914f601 100644 --- a/docs/concepts/access-and-auth.md +++ b/docs/concepts/access-and-auth.md @@ -130,7 +130,7 @@ the detail screen: ``` ssh stoat@127.0.0.1:2202 - console stoat / stoat (qemu window only) + console stoat / stoat (over the qemu window, or vnc on a headless host) ``` and in the log line written when the VM starts, which you can read with diff --git a/docs/getting-started/installation.md b/docs/getting-started/installation.md index a0ceed3..5bef534 100644 --- a/docs/getting-started/installation.md +++ b/docs/getting-started/installation.md @@ -50,9 +50,9 @@ sudo usermod -aG kvm "$USER" # then log out and back in ### GPU/display -One kind of VM opens a real QEMU window: a disk-mode VM that has not been installed yet, whose OS installer has to be driven by a human. That window is `-display gtk,gl=on`, so your QEMU build needs GTK and OpenGL support (the `qemu-full`/`qemu-desktop` Arch packages and the Debian/Ubuntu packages above provide this). Every other VM is headless with its screen on a VNC socket, and needs none of it. +A VM opens a real QEMU window by default. That window is `-display gtk,gl=on`, so your QEMU build needs GTK and OpenGL support (the `qemu-full`/`qemu-desktop` Arch packages and the Debian/Ubuntu packages above provide this). Set `display = "vnc"` on a VM to keep it headless with its screen on a VNC socket instead. -**On a host with no graphical session** (a server, an ssh session with no forwarding) stoat does not ask for that window at all: it puts the install console on the VNC socket too and prints how to attach, so a disk VM can be installed from another machine. It detects this from `DISPLAY`, `WAYLAND_DISPLAY` and `$XDG_RUNTIME_DIR/wayland-0`. +**On a host with no graphical session** (a server, an ssh session with no forwarding) stoat does not ask for a window at all: every VM's screen goes to a VNC socket and stoat prints how to attach, so a disk VM can still be installed from another machine. It detects this from `DISPLAY`, `WAYLAND_DISPLAY` and `$XDG_RUNTIME_DIR/wayland-0`. If a VM still fails to start with a display or GL error, your host has a session QEMU cannot draw on. Set `STOAT_GRAPHICAL=0` to take the window out of play; see [troubleshooting](../troubleshooting.md). No source edit and no rebuild. diff --git a/docs/reference/cli.md b/docs/reference/cli.md index bc34f1d..6089408 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -85,8 +85,7 @@ ssh port: 2222 ssh user: root recipes: xfce forwards: 8080:80 -display: no qemu window; the screen is on /home/user/.stoat/work/vnc.sock - attach with: gvncviewer /home/user/.stoat/work/vnc.sock +display: a qemu window ``` `display` is the only line here that is not a `vm.toml` field. See [`stoat up`](#stoat-up-name) for what it means and why the answer changes. It is omitted entirely for a broken VM, whose `vm.toml` supplies neither of the facts the answer depends on. @@ -153,34 +152,32 @@ display: no qemu window; the screen is on /home/user/.stoat/work/vnc.sock ### Where the screen is -Exactly one kind of VM gets a real QEMU window: a **disk-mode VM that is not yet installed**, on a host with a graphical session. Its OS installer draws to VGA and a human has to drive it, so `up` says so: +A VM gets a real QEMU window by default, on a host with a graphical session. Set `display = "vnc"` in `vm.toml` (or cycle it with the `d` key in the TUI) to keep a VM headless instead. QEMU then starts with `-display none` and a VNC server bound to a unix socket in the VM's directory; `-display none` cannot be undone on a running QEMU, so binding VNC at launch keeps a misbehaving guest recoverable. ``` -display: a qemu window, for the OS installer's console +$ stoat up work +starting work... +work started (ssh :2222) +display: no qemu window; the screen is on /home/user/.stoat/work/vnc.sock + attach with: gvncviewer /home/user/.stoat/work/vnc.sock ``` -Every other VM is headless. QEMU is started with `-display none` and a VNC server bound to a unix socket in the VM's directory, because `-display none` cannot be undone on a running QEMU and binding VNC at launch keeps a misbehaving guest recoverable. - -**This includes a disk VM the moment its install finishes.** `setup-alpine` completes, stoat records `installed = true`, and the next start has no window. That is not a failure; the screen moved to the socket. It surprises people who provisioned a desktop onto a disk VM and expected the window to keep coming back. - The attach command names a viewer that is actually installed on your machine: - `gvncviewer ` opens the socket directly, when `gvncviewer` is present. - Otherwise `socat TCP-LISTEN:5900,bind=127.0.0.1,reuseaddr,fork UNIX-CONNECT:` republishes it on loopback, and any VNC client connects to `127.0.0.1:5900`. - If neither is installed, `up` says so and names them rather than printing a command that would fail. -There is currently no way to ask for a QEMU window on an installed disk VM. `-display gtk` needs a graphical session on the host, so granting one by default would make `stoat up` fail outright over SSH or from a script rather than merely come up headless. - ### On a host with no graphical session -`-display gtk` does not degrade when there is no display server: QEMU exits 1. So the install console goes to VNC there too, and `up` says why before it says where: +`-display gtk` does not degrade when there is no display server: QEMU exits 1. So every VM's screen goes to VNC there instead, and `up` says why before it says where: ``` $ stoat up alpinedisk starting alpinedisk... alpinedisk started (ssh :2200) -display: no usable graphical session on this host, so the OS installer's - console is on VNC instead; drive it from a machine with a screen +display: no usable graphical session on this host, so the screen + is on VNC instead; attach to watch it display: no qemu window; the screen is on /home/user/.stoat/alpinedisk/vnc.sock attach with: gvncviewer /home/user/.stoat/alpinedisk/vnc.sock ``` diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 850f569..34a0416 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -2,19 +2,12 @@ Symptom-first. Find the error text you're seeing and jump to it. -## No QEMU window appears any more +## No QEMU window appears -You created a disk VM, let it install itself, maybe provisioned a desktop onto -it, and now no window opens on start. There is no error because nothing failed. - -Exactly one kind of VM gets a real QEMU window: a disk-mode VM that is **not -yet installed**, on a host with a graphical session. That window shows the -unattended `setup-alpine` install as it runs, which draws to VGA. The moment -stoat records `installed = true`, -the next start uses `-display none` with a VNC server bound to a unix socket in -the VM's directory (`internal/qemu/args.go`). `-display none` cannot be undone -on a running QEMU, so binding VNC at launch is what keeps a guest that has -locked up or lost its network still reachable. +A VM with `display = "vnc"` in its `vm.toml`, or one started on a host with no +graphical session, puts its screen on a VNC socket instead of a window. Every +other VM opens a real QEMU window by default (`internal/qemu/args.go`), whether +it is live, cloud, or an installed or uninstalled disk VM. **Fix:** ask stoat where the screen went. Both `stoat up` and `stoat get` print it, with a command for a VNC viewer that is actually installed on your machine: @@ -31,10 +24,9 @@ installed, stoat names what to install rather than printing a command that would fail. With only `socat`, the command bridges the socket to loopback and any VNC client connects to `127.0.0.1:5900`. -There is no way to ask for a QEMU window on an installed disk VM. `-display -gtk` needs a graphical session on the host, so granting one by default would -make `stoat up` fail outright over SSH or from a script instead of merely -coming up headless. +Set `display = "vnc"` (or cycle it with the `d` key in the TUI) to keep a VM +headless on purpose: `-display none` binds a VNC server at launch instead of a +window, which stays reachable even if the guest locks up or loses its network. ## `OpenGL is not supported by display backend 'gtk'` @@ -43,10 +35,10 @@ stoat: up: qemu failed to start: qemu-system-x86_64: OpenGL is not supported by ``` This one is not about OpenGL, and mesa and your GPU drivers are not the place -to look. stoat starts an uninstalled disk VM with `-display gtk,gl=on`, and -`gl=on` is simply the first option QEMU rejects when it cannot open a window at -all. With the same window and no `gl=on`, the same host says `gtk -initialization failed` instead. +to look. stoat starts a VM with `-display gtk,gl=on` by default, and `gl=on` +is simply the first option QEMU rejects when it cannot open a window at all. +With the same window and no `gl=on`, the same host says `gtk initialization +failed` instead. If stoat prints this, it found a graphical session on the host and QEMU still could not use it: usually a QEMU or GTK build without working GL. The message @@ -92,7 +84,7 @@ directions, for every command and for the TUI: | value | effect | | --- | --- | -| `STOAT_GRAPHICAL=0` | never open a window; the install console goes to VNC | +| `STOAT_GRAPHICAL=0` | never open a window; every VM's screen goes to VNC | | `STOAT_GRAPHICAL=1` | open the window; use this if stoat did not recognize your session | | unset | detect (the default) | diff --git a/internal/cli/display_test.go b/internal/cli/display_test.go index ef4011c..e1e71de 100644 --- a/internal/cli/display_test.go +++ b/internal/cli/display_test.go @@ -45,12 +45,12 @@ func getText(t *testing.T, name string) string { return out.String() } -// The reported bug: a disk VM whose window vanishes once setup-alpine marks -// it installed, with nothing anywhere saying where the screen went. +// A VM pinned to display="vnc" always has a socket to attach to, whether or +// not it is installed. func TestGetTellsAnInstalledDiskVMWhereItsScreenWent(t *testing.T) { root := cliRoot(t) saveVM(t, &config.VM{Name: "alpinedisk", OS: "alpine", Mode: "disk", Disk: "8G", - Installed: true, RAM: 2048, CPUs: 2, SSHPort: 2200}) + Installed: true, Display: "vnc", RAM: 2048, CPUs: 2, SSHPort: 2200}) fakeViewers(t, "gvncviewer") out := getText(t, "alpinedisk") @@ -62,8 +62,8 @@ func TestGetTellsAnInstalledDiskVMWhereItsScreenWent(t *testing.T) { } } -// Before the install finishes there IS a window, and saying "no qemu window" -// then would send a user hunting for a socket qemu never bound. +// A disk VM on a graphical host gets a window whether or not the install has +// finished. // // The override is pinned rather than left to detection: this assertion is // about a host with a session, and the machine running the test may not be @@ -83,11 +83,11 @@ func TestGetSaysAFreshDiskVMHasAWindow(t *testing.T) { } } -// The same VM on a host with no graphical session. qemu cannot open a window -// there, so the install console is on VNC, and the output has to say that -// before it says "no qemu window": mid-install, "no qemu window" on its own -// reads as the thing that went wrong. -func TestGetExplainsTheInstallConsoleOnAHeadlessHost(t *testing.T) { +// A host with no graphical session. qemu cannot open a window there, so +// every VM's console is on VNC, and the output has to say that before it +// says "no qemu window": on its own, "no qemu window" reads as the thing +// that went wrong. +func TestGetExplainsTheVNCFallbackOnAHeadlessHost(t *testing.T) { root := cliRoot(t) saveVM(t, &config.VM{Name: "alpinedisk", OS: "alpine", Mode: "disk", Disk: "8G", Installed: false, RAM: 2048, CPUs: 2, SSHPort: 2200}) @@ -98,19 +98,18 @@ func TestGetExplainsTheInstallConsoleOnAHeadlessHost(t *testing.T) { sock := filepath.Join(root, "alpinedisk", "vnc.sock") for _, want := range []string{ "no usable graphical session on this host", - "console is on VNC instead", "attach with: gvncviewer " + sock, } { if !strings.Contains(out, want) { t.Errorf("get output missing %q:\n%s", want, out) } } - // An installed VM on the same host is on VNC for its own reasons and must - // not be given the host's excuse. + // A VM pinned to display="vnc" was already on VNC for its own reasons and + // must not be given the host's excuse. saveVM(t, &config.VM{Name: "done", OS: "alpine", Mode: "disk", Disk: "8G", - Installed: true, RAM: 2048, CPUs: 2, SSHPort: 2201}) + Installed: true, Display: "vnc", RAM: 2048, CPUs: 2, SSHPort: 2201}) if out := getText(t, "done"); strings.Contains(out, "graphical session") { - t.Errorf("an installed VM was never getting a window:\n%s", out) + t.Errorf("a VM pinned to vnc was never getting a window:\n%s", out) } } @@ -118,6 +117,7 @@ func TestGetExplainsTheInstallConsoleOnAHeadlessHost(t *testing.T) { // nothing: it reads as an instruction and fails as one. func TestGetNamesWhatToInstallWhenNoViewerExists(t *testing.T) { cliRoot(t) + t.Setenv(core.GraphicalEnv, "0") saveVM(t, &config.VM{Name: "cloudy", OS: "alpine", Mode: "cloud", RAM: 2048, CPUs: 2, SSHPort: 2201}) fakeViewers(t) @@ -135,6 +135,7 @@ func TestGetNamesWhatToInstallWhenNoViewerExists(t *testing.T) { // anybody anything. func TestGetPrintsTheBridgeFollowUpStep(t *testing.T) { cliRoot(t) + t.Setenv(core.GraphicalEnv, "0") saveVM(t, &config.VM{Name: "cloudy", OS: "alpine", Mode: "cloud", RAM: 2048, CPUs: 2, SSHPort: 2201}) fakeViewers(t, "socat") @@ -162,7 +163,7 @@ func TestGetOnABrokenVMSaysNothingAboutTheDisplay(t *testing.T) { func TestJSONCarriesTheDisplayKindButNeverTheSocket(t *testing.T) { root := cliRoot(t) saveVM(t, &config.VM{Name: "alpinedisk", OS: "alpine", Mode: "disk", Disk: "8G", - Installed: true, RAM: 2048, CPUs: 2, SSHPort: 2200}) + Installed: true, Display: "vnc", RAM: 2048, CPUs: 2, SSHPort: 2200}) fakeViewers(t, "gvncviewer") code, objs := runJSON(t, "get", "alpinedisk") diff --git a/internal/cli/run_vm.go b/internal/cli/run_vm.go index ac94271..3713810 100644 --- a/internal/cli/run_vm.go +++ b/internal/cli/run_vm.go @@ -94,24 +94,23 @@ func runUp(a *Args, stdout, stderr io.Writer) int { // printDisplay says where a VM's screen is and how to reach it. // // This prints on every start, not only the surprising one, because the -// surprising one is not detectable from here: a disk VM shows a window until -// setup-alpine marks it installed, and the start after that silently moves -// the screen to a VNC socket. A user who was never told the socket exists has -// no error to search for and no path to guess. +// surprising one is not detectable from here: a VM with display="vnc", or one +// on a host with no graphical session, has its screen on a socket instead of +// the window a user expects by default. A user who was never told the socket +// exists has no error to search for and no path to guess. func printDisplay(w io.Writer, d core.Display) { switch d.Kind { case core.DisplayWindow: - fmt.Fprintln(w, "display: a qemu window, for the OS installer's console") + fmt.Fprintln(w, "display: a qemu window") case core.DisplayVNC: if d.NoSession { - // The install console, on a host that cannot open a window. Said - // before the socket line, because without it "no qemu window" for a - // VM that is mid-install reads as the thing that went wrong. + // Said before the socket line, because without it "no qemu window" + // on its own reads as the thing that went wrong. // "no usable session" rather than "no session": the same line // prints when the user set STOAT_GRAPHICAL=0 on a host that plainly // has one, because its GTK cannot draw on it. - fmt.Fprintln(w, "display: no usable graphical session on this host, so the OS installer's") - fmt.Fprintln(w, " console is on VNC instead; drive it from a machine with a screen") + fmt.Fprintln(w, "display: no usable graphical session on this host, so the screen") + fmt.Fprintln(w, " is on VNC instead; attach to watch it") } fmt.Fprintf(w, "display: no qemu window; the screen is on %s\n", d.Socket) if d.Attach.Command == "" { diff --git a/internal/cli/wire/dto_test.go b/internal/cli/wire/dto_test.go index 9a2c319..a599a14 100644 --- a/internal/cli/wire/dto_test.go +++ b/internal/cli/wire/dto_test.go @@ -12,7 +12,7 @@ import ( func TestVMGolden(t *testing.T) { got := marshal(t, FromVM(sampleVM(), true)) - want := `{"name":"work","os":"alpine","mode":"live","backend":"apkovl","state":"running","cpus":4,"ram_mb":4096,"disk":"8G","share":"/home/u/src","recipes":["xfce"],"ssh_port":2222,"ssh_user":"root","installed":false,"forwards":[{"host_port":8080,"guest_port":80}],"allow_exec":true,"display":"vnc"}` + want := `{"name":"work","os":"alpine","mode":"live","backend":"apkovl","state":"running","cpus":4,"ram_mb":4096,"disk":"8G","share":"/home/u/src","recipes":["xfce"],"ssh_port":2222,"ssh_user":"root","installed":false,"forwards":[{"host_port":8080,"guest_port":80}],"allow_exec":true,"display":"window"}` if got != want { t.Errorf("got %s\nwant %s", got, want) } @@ -28,11 +28,13 @@ func TestVMDisplayNamesTheSurfaceNotTheSocket(t *testing.T) { if !strings.Contains(got, `"display":"window"`) { t.Errorf("a VM mid-install has a real window: %s", got) } - installed := fresh - installed.Installed = true - got = marshal(t, FromVM(installed, true)) + + pinned := fresh + pinned.Installed = true + pinned.Display = "vnc" + got = marshal(t, FromVM(pinned, true)) if !strings.Contains(got, `"display":"vnc"`) { - t.Errorf("an installed disk VM is headless: %s", got) + t.Errorf("a VM pinned to vnc is headless: %s", got) } if strings.Contains(got, "vnc.sock") || strings.Contains(got, "/home/u") { t.Errorf("the socket path reached the wire: %s", got) diff --git a/internal/cloudinit/cloudinit.go b/internal/cloudinit/cloudinit.go index 1849fe2..c137b8a 100644 --- a/internal/cloudinit/cloudinit.go +++ b/internal/cloudinit/cloudinit.go @@ -35,9 +35,9 @@ const User = "stoat" // does not exist in the image. The shell must match the guest; see // guestShell. consolePasswordBlock below fills in the password block. // -// ssh_pwauth stays false on purpose. The password exists only for the VNC -// console: a cloud VM never gets a qemu window (qemu.NeedsWindow), so the -// console socket is the only place a password login happens. The network +// ssh_pwauth stays false on purpose. The password exists only for the +// console, a qemu window on a graphical host or the VNC socket otherwise +// (qemu.NeedsWindow), the only place a password login happens. The network // stays key-only. const userDataTemplate = `#cloud-config users: diff --git a/internal/config/config.go b/internal/config/config.go index 25f3296..d064df7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -46,10 +46,13 @@ type VM struct { Recipes []string `toml:"recipes"` // Display is the user's screen preference: "" or "auto" (default), - // "window", or "vnc". core.validateDisplay is the single place that - // checks the value; empty means an old vm.toml predates this field, so - // it must read the same as "auto". qemu.DisplayKind is the rule that - // turns this into DisplayWindow or DisplayVNC. + // "window", or "vnc". "auto" opens a real qemu window on a graphical + // host, and falls back to VNC only when the host has no display server; + // set "vnc" to opt out of a window on a graphical host. core.validateDisplay + // is the single place that checks the value; empty means an old vm.toml + // predates this field, so it must read the same as "auto". + // qemu.DisplayKind is the rule that turns this into DisplayWindow or + // DisplayVNC. Display string `toml:"display"` // Forwards are user-declared TCP ports forwarded from host to guest, in diff --git a/internal/config/password.go b/internal/config/password.go index 9b40be1..b3c5535 100644 --- a/internal/config/password.go +++ b/internal/config/password.go @@ -7,10 +7,10 @@ import ( // DefaultConsolePassword is what a new cloud VM gets unless a random one is // asked for. The value is fixed and documented on purpose. You see this -// login prompt at the VM's VNC console on your own machine (a cloud VM -// never gets a qemu window; see qemu.NeedsWindow), usually because ssh -// isn't working. Looking the password up at that moment is the failure -// this prevents. +// login prompt at the VM's console (a qemu window, or the VNC socket on a +// headless host or with display="vnc"; see qemu.NeedsWindow), usually +// because ssh isn't working. Looking the password up at that moment is the +// failure this prevents. // // This value is safe here in a way it would not be on a server. The seed // sets ssh_pwauth: false, so this password is refused over the forwarded diff --git a/internal/core/core.go b/internal/core/core.go index 243bce6..b16194d 100644 --- a/internal/core/core.go +++ b/internal/core/core.go @@ -247,8 +247,8 @@ func plan(s Spec) (*config.VM, error) { if img.backend == "cloudinit" { v.Base = img.abs // Only a cloud image needs a console password. cloud-init locks - // every account by default, so its VNC console (a cloud VM never - // gets a qemu window; see qemu.NeedsWindow) shows a login prompt + // every account by default, so its console (a qemu window, or VNC + // on a headless host; see qemu.NeedsWindow) shows a login prompt // with no valid answer otherwise. A live Alpine VM logs root in at // the console with no password. A disk VM's password is whatever // the user set in the guest's own installer. diff --git a/internal/core/display.go b/internal/core/display.go index 995dfdd..14cabb4 100644 --- a/internal/core/display.go +++ b/internal/core/display.go @@ -60,11 +60,11 @@ func DisplayKind(v VM, graphical bool) string { if v.State == StateBroken { return "" } - return qemu.DisplayKind(v.Display, v.Mode, v.Installed, graphical) + return qemu.DisplayKind(v.Display, graphical) } // validateDisplay checks a config.VM.Display candidate. "" and "auto" both -// mean the installer-console default; anything but those two and "window"/ +// mean a window on a graphical host; anything but those two and "window"/ // "vnc" is rejected so a typo in vm.toml or an MCP call fails loudly instead // of silently falling back to auto. func validateDisplay(pref string) error { diff --git a/internal/core/display_test.go b/internal/core/display_test.go index 86a7886..a87c733 100644 --- a/internal/core/display_test.go +++ b/internal/core/display_test.go @@ -17,16 +17,31 @@ func fakeBins(t *testing.T, names ...string) { t.Setenv("PATH", dir) } -// The reported case: a disk VM that showed a window through setup-alpine and -// is headless on every start after it. DisplayFor must be able to say where -// the screen went and what opens it. +// An installed disk VM on a graphical host still gets a window: "auto" no +// longer singles out the installer console. func TestDisplayForAnInstalledDiskVM(t *testing.T) { - fakeBins(t, "gvncviewer") v := VM{ Name: "alpinedisk", Mode: "disk", Installed: true, State: StateStopped, Paths: Paths{VNCSocket: "/home/u/.stoat/alpinedisk/vnc.sock"}, } d := DisplayFor(v, true) + if d.Kind != DisplayWindow { + t.Errorf("Kind = %q, want %q", d.Kind, DisplayWindow) + } + if d.Socket != "" || d.Attach.Command != "" { + t.Errorf("a windowed VM must offer no VNC attach: %+v", d) + } +} + +// The same VM with display="vnc" set explicitly. That is the only way an +// installed disk VM stays headless now. +func TestDisplayForAnInstalledDiskVMPinnedToVNC(t *testing.T) { + fakeBins(t, "gvncviewer") + v := VM{ + Name: "alpinedisk", Mode: "disk", Installed: true, State: StateStopped, + Display: "vnc", Paths: Paths{VNCSocket: "/home/u/.stoat/alpinedisk/vnc.sock"}, + } + d := DisplayFor(v, true) if d.Kind != DisplayVNC { t.Errorf("Kind = %q, want %q", d.Kind, DisplayVNC) } @@ -78,14 +93,14 @@ func TestDisplayForAFreshDiskVMWithNoSessionFallsBackToVNC(t *testing.T) { } } -// An installed disk VM was already on VNC for its own reasons. A headless host -// must not make it claim it was demoted, or the explanation gets printed on -// every VM stoat has. +// A VM pinned to display="vnc" was already on VNC for its own reasons. A +// headless host must not make it claim it was demoted, or the explanation +// gets printed on every VM stoat has. func TestDisplayForAnAlreadyHeadlessVMIsNotBlamedOnTheHost(t *testing.T) { fakeBins(t) - v := VM{Name: "c", Mode: "cloud", State: StateStopped, Paths: Paths{VNCSocket: "/x/vnc.sock"}} + v := VM{Name: "c", Mode: "cloud", Display: "vnc", State: StateStopped, Paths: Paths{VNCSocket: "/x/vnc.sock"}} if d := DisplayFor(v, false); d.NoSession { - t.Error("a cloud VM never wanted a window; NoSession must be false") + t.Error("this VM never wanted a window; NoSession must be false") } } @@ -108,8 +123,8 @@ func TestDisplayKindIsPure(t *testing.T) { t.Setenv("DISPLAY", "") t.Setenv("WAYLAND_DISPLAY", "") t.Setenv("XDG_RUNTIME_DIR", "") - if got := DisplayKind(VM{Mode: "cloud"}, true); got != DisplayVNC { - t.Errorf("DisplayKind = %q, want %q", got, DisplayVNC) + if got := DisplayKind(VM{Mode: "cloud"}, true); got != DisplayWindow { + t.Errorf("DisplayKind = %q, want %q", got, DisplayWindow) } if got := DisplayKind(VM{Mode: "disk"}, true); got != DisplayWindow { t.Errorf("DisplayKind = %q, want %q", got, DisplayWindow) @@ -120,8 +135,7 @@ func TestDisplayKindIsPure(t *testing.T) { } } -// A per-VM preference overrides the mode/installed default, but not the -// host's veto. +// A per-VM preference overrides the auto default, but not the host's veto. func TestDisplayKindHonoursPreference(t *testing.T) { if got := DisplayKind(VM{Mode: "cloud", Display: "window"}, true); got != DisplayWindow { t.Errorf("Display=window: got %q, want %q", got, DisplayWindow) diff --git a/internal/qemu/args.go b/internal/qemu/args.go index 38ebb87..489d008 100644 --- a/internal/qemu/args.go +++ b/internal/qemu/args.go @@ -19,16 +19,11 @@ const ( DisplayVNC = "vnc" ) -// DisplayKind is the rule, stated over the four facts it actually depends on -// rather than over a *config.VM: core.VM is a different type that carries -// pref/mode/installed, and it must ask this question rather than restate it. +// DisplayKind is the rule, stated over the two facts it actually depends on. // -// pref is config.VM.Display: "window" or "vnc" pins the surface outright, -// "" or "auto" runs the installer-console default below. Only an uninstalled -// disk-mode VM wants a window under that default, because its OS installer -// draws to VGA rather than the serial console and a human has to drive it. -// live, cloud and installed disk VMs reach ssh with no console interaction, -// so their screen goes to the VNC socket unless pref overrides it. +// pref is config.VM.Display: "window" pins a window, "vnc" pins the socket, +// "" or "auto" defaults to a window. Every mode and installed state defaults +// the same way; a VM opts into VNC by setting display="vnc", not by its mode. // // graphical is the host's veto, checked before pref and unconditional: even // a "window" preference falls back to VNC on a host with no display server. @@ -41,21 +36,14 @@ const ( // same VGA framebuffer over a socket the user can reach from a machine that // does have a screen. Whoever answers this must answer it for the host, not // for the VM: see GraphicalSession. -func DisplayKind(pref, mode string, installed, graphical bool) string { +func DisplayKind(pref string, graphical bool) string { if !graphical { return DisplayVNC } - switch pref { - case DisplayWindow: - return DisplayWindow - case DisplayVNC: - return DisplayVNC - default: - if mode == "disk" && !installed { - return DisplayWindow - } + if pref == DisplayVNC { return DisplayVNC } + return DisplayWindow } // NeedsWindow reports whether this VM gets a real qemu window on a host whose @@ -64,7 +52,7 @@ func DisplayKind(pref, mode string, installed, graphical bool) string { // Exported so the TUI can describe the right escape hatch (a GTK window vs. // the VNC socket) without duplicating this rule. func NeedsWindow(v *config.VM, graphical bool) bool { - return DisplayKind(v.Display, v.Mode, v.Installed, graphical) == DisplayWindow + return DisplayKind(v.Display, graphical) == DisplayWindow } // WantsWindow reports whether this VM would use a window if the host could diff --git a/internal/qemu/args_test.go b/internal/qemu/args_test.go index 102e6e3..9140d53 100644 --- a/internal/qemu/args_test.go +++ b/internal/qemu/args_test.go @@ -171,10 +171,11 @@ func TestOnlyManualInstallsGetAWindow(t *testing.T) { vm config.VM wantsWindow bool }{ - {"live is automated", config.VM{Mode: "live"}, false}, - {"cloud is automated", config.VM{Mode: "cloud"}, false}, - {"installed disk boots like the others", config.VM{Mode: "disk", Installed: true}, false}, + {"live gets a window on a graphical host", config.VM{Mode: "live"}, true}, + {"cloud gets a window on a graphical host", config.VM{Mode: "cloud"}, true}, + {"installed disk boots like the others", config.VM{Mode: "disk", Installed: true}, true}, {"uninstalled disk is a manual install", config.VM{Mode: "disk"}, true}, + {"vnc opts a mode out of the window", config.VM{Mode: "live", Display: "vnc"}, false}, } { t.Run(tc.name, func(t *testing.T) { v := tc.vm diff --git a/internal/qemu/display_test.go b/internal/qemu/display_test.go index 29694bd..c735f0e 100644 --- a/internal/qemu/display_test.go +++ b/internal/qemu/display_test.go @@ -31,18 +31,20 @@ func TestDisplayKind(t *testing.T) { graphical bool want string }{ - // pref "" and "auto" both run the installer-console default. + // pref "" and "auto" both resolve to a window on a graphical host, + // for every mode and installed state. {"", "disk", false, true, DisplayWindow}, {"auto", "disk", false, true, DisplayWindow}, - // The same VM on a host with no display server. qemu would exit 1 on - // the window rather than fall back, so stoat falls back for it. + {"", "disk", true, true, DisplayWindow}, + {"", "live", false, true, DisplayWindow}, + {"", "live", true, true, DisplayWindow}, + {"", "cloud", false, true, DisplayWindow}, + {"", "cloud", true, true, DisplayWindow}, + + // The same VMs on a host with no display server. qemu would exit 1 + // on the window rather than fall back, so stoat falls back for them. {"", "disk", false, false, DisplayVNC}, {"auto", "disk", false, false, DisplayVNC}, - {"", "disk", true, true, DisplayVNC}, - {"", "live", false, true, DisplayVNC}, - {"", "live", true, true, DisplayVNC}, - {"", "cloud", false, true, DisplayVNC}, - {"", "cloud", true, true, DisplayVNC}, {"", "cloud", false, false, DisplayVNC}, // "window" pins the surface, on every mode/installed combination, @@ -50,8 +52,8 @@ func TestDisplayKind(t *testing.T) { {"window", "live", false, true, DisplayWindow}, {"window", "disk", true, true, DisplayWindow}, {"window", "cloud", false, true, DisplayWindow}, - // "vnc" pins the other way, including the installer-console case - // the auto default would have given a window. + // "vnc" pins the other way, including the disk-mode case the auto + // default would have given a window. {"vnc", "disk", false, true, DisplayVNC}, {"vnc", "live", true, true, DisplayVNC}, @@ -60,8 +62,8 @@ func TestDisplayKind(t *testing.T) { {"window", "disk", false, false, DisplayVNC}, {"window", "live", true, false, DisplayVNC}, } { - if got := DisplayKind(c.pref, c.mode, c.installed, c.graphical); got != c.want { - t.Errorf("DisplayKind(%q, %q, %v, graphical=%v) = %q, want %q", c.pref, c.mode, c.installed, c.graphical, got, c.want) + if got := DisplayKind(c.pref, c.graphical); got != c.want { + t.Errorf("DisplayKind(%q, graphical=%v) = %q, want %q", c.pref, c.graphical, got, c.want) } } } @@ -75,27 +77,26 @@ func TestDisplayKindReadsNoEnvironment(t *testing.T) { t.Setenv("WAYLAND_DISPLAY", "") t.Setenv("XDG_RUNTIME_DIR", "") t.Setenv(GraphicalEnv, "0") - if got := DisplayKind("", "disk", false, true); got != DisplayWindow { + if got := DisplayKind("", true); got != DisplayWindow { t.Errorf("DisplayKind = %q, want %q: the argument decides, not the environment", got, DisplayWindow) } } -// NeedsWindow is a wrapper over DisplayKind. It must still answer exactly what -// it answered before on a host with a session, since qemu.Args and the TUI -// both branch on it. +// NeedsWindow is a wrapper over DisplayKind. Every mode and installed state +// now wants a window on a graphical host, since qemu.Args and the TUI both +// branch on it. func TestNeedsWindowMatchesDisplayKind(t *testing.T) { for _, mode := range []string{"live", "disk", "cloud"} { for _, installed := range []bool{false, true} { v := &config.VM{Mode: mode, Installed: installed} - want := mode == "disk" && !installed - if got := NeedsWindow(v, true); got != want { - t.Errorf("NeedsWindow(mode=%q installed=%v) = %v, want %v", mode, installed, got, want) + if got := NeedsWindow(v, true); !got { + t.Errorf("NeedsWindow(mode=%q installed=%v, graphical=true) = %v, want true", mode, installed, got) } // WantsWindow is the same question with the host taken out of it, // which is what separates "this VM never gets a window" from "this // host could not give it one". - if got := WantsWindow(v); got != want { - t.Errorf("WantsWindow(mode=%q installed=%v) = %v, want %v", mode, installed, got, want) + if got := WantsWindow(v); !got { + t.Errorf("WantsWindow(mode=%q installed=%v) = %v, want true", mode, installed, got) } if NeedsWindow(v, false) { t.Errorf("no host session, yet NeedsWindow(mode=%q installed=%v) said yes", mode, installed) @@ -136,11 +137,12 @@ func TestFreshDiskVMStillGetsARealWindow(t *testing.T) { t.Errorf("the install console must not be headless:\n%s", got) } - // ...and the moment it is installed, the same VM goes to the socket. + // ...and after install, the same VM still opens a window: "auto" no + // longer singles out the installer console. v.Installed = true got = joined(Args(v, GraphicalSession())) - if !strings.Contains(got, "-display none -vnc unix:/data/alpinedisk/vnc.sock") { - t.Errorf("an installed disk VM must bind VNC at launch:\n%s", got) + if !strings.Contains(got, "-display gtk,gl=on") { + t.Errorf("an installed disk VM on a graphical host must still open a window:\n%s", got) } } diff --git a/internal/tui/creds_test.go b/internal/tui/creds_test.go index e82a3d4..9f6c4c4 100644 --- a/internal/tui/creds_test.go +++ b/internal/tui/creds_test.go @@ -7,6 +7,7 @@ import ( "github.com/novusedge/stoat/internal/config" "github.com/novusedge/stoat/internal/core" "github.com/novusedge/stoat/internal/iso" + "github.com/novusedge/stoat/internal/qemu" ) // TestDetailShowsConsoleCredentials covers the reported problem: an Ubuntu @@ -17,6 +18,7 @@ import ( // The seed sets ssh_pwauth: false. Without a password there is no valid // answer to the prompt. func TestDetailShowsConsoleCredentials(t *testing.T) { + t.Setenv(qemu.GraphicalEnv, "0") withPassword := core.VM{ Name: "ubuntu-1", Mode: "cloud", OS: "ubuntu", Backend: "cloudinit", RAM: 4096, CPUs: 4, SSHPort: 2202, Paths: core.Paths{Dir: t.TempDir()}, @@ -34,13 +36,14 @@ func TestDetailShowsConsoleCredentials(t *testing.T) { } // The password is not an ssh credential: the seed refuses password auth // over the forwarded port. - // Cloud VMs never get a qemu window (qemu.NeedsWindow). The row must - // point at the vnc socket the detail screen also surfaces. + // This test runs with no graphical session, so qemu.DisplayKind falls + // back to vnc regardless of mode. The row must point at the socket the + // detail screen also surfaces. if !strings.Contains(out, "vnc") { t.Error("detail pane does not say the password is reached over vnc") } if strings.Contains(out, "qemu window only") { - t.Error("cloud VMs never get a qemu window; the console row must not claim one") + t.Error("the console row must not claim a surface no code ever prints") } // A cloud VM without one (created before this existed) must say so diff --git a/internal/tui/detail.go b/internal/tui/detail.go index bcdd001..d81ff8f 100644 --- a/internal/tui/detail.go +++ b/internal/tui/detail.go @@ -391,19 +391,20 @@ func (m model) viewDetail() string { facts.row("", "", effect) } line("display", displayPrefLabel(v.Display)) - // qemu.DisplayKind takes pref, mode, installed, and host graphical - // directly, not a *config.VM, so a core.VM caller can call it without - // going through qemu.NeedsWindow/WantsWindow, which need a *config.VM. + // qemu.DisplayKind takes pref and host graphical directly, not a + // *config.VM, so a core.VM caller can call it without going through + // qemu.NeedsWindow/WantsWindow, which need a *config.VM. // // A bare socket path is not enough: the user needs the actual command // that opens it, so this prints one for a viewer installed on this host. // - // With no graphical session, the install console also lands on this - // socket. That is the case where the user needs the explanation most, - // since the VM would otherwise look like it refused to start. - if graphical := qemu.GraphicalSession(); qemu.DisplayKind(v.Display, v.Mode, v.Installed, graphical) != qemu.DisplayWindow { - if !graphical && qemu.DisplayKind(v.Display, v.Mode, v.Installed, true) == qemu.DisplayWindow { - facts.row("", "", warnStyle.Render("no usable graphical session on this host: the installer console is on vnc")) + // With no graphical session, a VM that would otherwise get a window also + // lands on this socket. That is the case where the user needs the + // explanation most, since the VM would otherwise look like it refused + // to start. + if graphical := qemu.GraphicalSession(); qemu.DisplayKind(v.Display, graphical) != qemu.DisplayWindow { + if !graphical && qemu.DisplayKind(v.Display, true) == qemu.DisplayWindow { + facts.row("", "", warnStyle.Render("no usable graphical session on this host: falling back to vnc")) } line("vnc", v.Paths.VNCSocket) att := qemu.AttachVNC(v.Paths.VNCSocket) @@ -420,10 +421,14 @@ func (m model) viewDetail() string { // every account, so without a password the console shows a login prompt // with no valid answer. The moment a user needs it is the moment ssh has // failed. This password is set only by the cloudinit backend (form.go), - // which is always cloud mode, so it is always reached over VNC, never a - // qemu window. + // which is always cloud mode; the surface it is typed at is whichever one + // is shown above, a qemu window or the VNC socket. if v.ConsolePassword != "" { - line("console", sshUser+" / "+v.ConsolePassword+dimStyle.Render(" (over vnc, above)")) + surface := "the qemu window" + if qemu.DisplayKind(v.Display, qemu.GraphicalSession()) == qemu.DisplayVNC { + surface = "vnc, above" + } + line("console", sshUser+" / "+v.ConsolePassword+dimStyle.Render(" (over "+surface+")")) } else if v.Mode == "cloud" { line("console", warnStyle.Render("no password set: console login is not possible")) } diff --git a/internal/tui/detail_test.go b/internal/tui/detail_test.go index 523a2a9..3c28324 100644 --- a/internal/tui/detail_test.go +++ b/internal/tui/detail_test.go @@ -232,13 +232,13 @@ func TestTypeConsolePasswordKeyRefusesWhenUnavailable(t *testing.T) { } } -// A cloud VM never gets a qemu window (qemu.NeedsWindow), so the detail -// screen must surface the VNC socket as the actual way to get a display. -// Before this fix, nothing told the user that socket exists. The -// console-password row also claimed a "(qemu window only)" that never -// applies: the password is set only by the cloudinit backend, which is -// always cloud mode. +// This VM runs with no graphical session (the test sets no Display and no +// WAYLAND_DISPLAY/DISPLAY), so qemu.DisplayKind falls back to vnc regardless +// of mode. The detail screen must surface the VNC socket as the actual way +// to get a display. Before this fix, nothing told the user that socket +// exists. func TestDetailSurfacesVNCForAHeadlessVM(t *testing.T) { + t.Setenv(qemu.GraphicalEnv, "0") dir := t.TempDir() sock := filepath.Join(dir, "vnc.sock") v := core.VM{Name: "cloudy", Mode: "cloud", ConsolePassword: "stoat", Paths: core.Paths{Dir: dir, VNCSocket: sock}} @@ -300,6 +300,7 @@ func TestDetailExplainsTheVNCFallbackOnAHeadlessHost(t *testing.T) { // no way to open it. The detail pane instead names a viewer actually // installed on this host. func TestDetailShowsHowToAttachToTheVNCSocket(t *testing.T) { + t.Setenv(qemu.GraphicalEnv, "0") fakeViewerPath(t, "gvncviewer") v := core.VM{Name: "alpinedisk", Mode: "disk", Installed: true, Paths: core.Paths{Dir: t.TempDir()}} m := model{screen: screenDetail, width: 120, height: 40} @@ -314,6 +315,7 @@ func TestDetailShowsHowToAttachToTheVNCSocket(t *testing.T) { // Naming a viewer the user does not have is worse than naming none: it reads // as an instruction and fails as one. func TestDetailSaysWhatToInstallWhenNoViewerExists(t *testing.T) { + t.Setenv(qemu.GraphicalEnv, "0") fakeViewerPath(t) v := core.VM{Name: "alpinedisk", Mode: "disk", Installed: true, Paths: core.Paths{Dir: t.TempDir()}} m := model{screen: screenDetail, width: 120, height: 40} diff --git a/internal/tui/form.go b/internal/tui/form.go index 67d6b05..125d99a 100644 --- a/internal/tui/form.go +++ b/internal/tui/form.go @@ -914,10 +914,10 @@ func (m model) viewForm() string { marker = selStyle.Render(glyphCursor) } b.row(marker, "console", radio("stoat", !f.randomPassword)+" "+radio("random", f.randomPassword)) - // cloudinit is always cloud mode (effectiveMode above), and a cloud VM - // never gets a QEMU window (qemu.NeedsWindow), so this password is only - // ever typed at the VNC socket the detail screen surfaces. - b.hint("stoat user's login over VNC, cloud VMs have no qemu window") + // This password is typed at whichever console the VM opens: a qemu + // window by default on a graphical host, or the VNC socket the + // detail screen surfaces on a headless host or display="vnc". + b.hint("stoat user's login at the console (window, or VNC on a headless host)") } // The download block and the error are full-width blocks, not field rows,