fix(sshx): mount the 9p share on a disk VM

internal/apkovl/apkovl.go

@@ -193,11 +193,11 @@ func Build(v *config.VM) error {
 	//
 	// `nofail`: some guest kernels have no 9p module at all (Debian's cloud
 	// kernel doesn't). Without it an unmountable share holds up boot.
-	fstab += "work /mnt/work 9p trans=virtio,version=9p2000.L,rw,_netdev,nofail 0 0\n"
+	fstab += WorkMount9p.FstabLine()
 	b.dir("mnt", 0o755)
 	b.dir("mnt/work", 0o755)
 	if v.Share != "" {
-		fstab += "host /mnt/host 9p trans=virtio,version=9p2000.L,ro,_netdev,nofail 0 0\n"
+		fstab += HostMount9p.FstabLine()
 		b.dir("mnt/host", 0o755)
 	}
 	// The initramfs's default-boot-services set doesn't include localmount,

internal/apkovl/mount9p.go

@@ -0,0 +1,36 @@
+package apkovl
+
+import "fmt"
+
+// Mount9p describes one 9p share qemu attaches to a VM: its mount tag (qemu's
+// mount_tag / virtfs id), the guest mountpoint, and the fstab options.
+// internal/qemu/args.go's -virtfs flags and this package's fstab must agree
+// on the tag, or the guest mounts nothing. internal/sshx's disk-VM mount step
+// reads these same values, so a disk-installed guest gets the identical
+// fstab line the live overlay would have written.
+type Mount9p struct {
+	Tag     string
+	Dir     string
+	Options string
+}
+
+// HostMount9p is the user's share directory, read-only. It applies only when
+// the VM has Share configured; see qemu.Args.
+var HostMount9p = Mount9p{
+	Tag:     "host",
+	Dir:     "/mnt/host",
+	Options: "trans=virtio,version=9p2000.L,ro,_netdev,nofail",
+}
+
+// WorkMount9p is stoat's per-VM scratch export, writable. qemu attaches it
+// unconditionally, so every VM mode gets a fstab line for it.
+var WorkMount9p = Mount9p{
+	Tag:     "work",
+	Dir:     "/mnt/work",
+	Options: "trans=virtio,version=9p2000.L,rw,_netdev,nofail",
+}
+
+// FstabLine renders m as one /etc/fstab entry, newline included.
+func (m Mount9p) FstabLine() string {
+	return fmt.Sprintf("%s %s 9p %s 0 0\n", m.Tag, m.Dir, m.Options)
+}

internal/sshx/sharemount.go

@@ -0,0 +1,105 @@
+package sshx
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"os/exec"
+	"strings"
+	"syscall"
+
+	"github.com/novusedge/stoat/internal/apkovl"
+	"github.com/novusedge/stoat/internal/config"
+)
+
+// guestFstabPath is the guest's real fstab. shareMountScript takes it as a
+// parameter so a test can point at a scratch file instead.
+const guestFstabPath = "/etc/fstab"
+
+// shareMountTags returns the 9p shares a disk-installed guest must mount for
+// v, or nil if none apply.
+//
+// A live VM already mounts through the apkovl overlay's own fstab (see
+// apkovl.Build). A cloud VM seeds its mounts through cloud-init instead. So
+// this only fires for Mode "disk", and only when Share is configured: with
+// no Share there is no host mount to add, by this feature's design.
+//
+// qemu.Args attaches the work virtfs to every VM unconditionally, so a disk
+// VM gets both tags once Share makes this step run at all.
+func shareMountTags(v *config.VM) []apkovl.Mount9p {
+	if v.Mode != "disk" || v.Share == "" {
+		return nil
+	}
+	return []apkovl.Mount9p{apkovl.HostMount9p, apkovl.WorkMount9p}
+}
+
+// fstabEnsureLine returns the sh fragment that appends m's fstab line to
+// fstabPath, only if a line for m.Dir is not already there. grep's pattern
+// anchors on "<tag> " so a substring match (e.g. "work" inside "network")
+// can't produce a false positive.
+func fstabEnsureLine(m apkovl.Mount9p, fstabPath string) string {
+	return fmt.Sprintf(
+		"grep -q '^%s %s ' %s 2>/dev/null || printf '%%s' %s >> %s\n",
+		m.Tag, m.Dir, fstabPath, shQuote(m.FstabLine()), fstabPath,
+	)
+}
+
+// mountIfNeeded returns the sh fragment that creates m's mountpoint and
+// mounts it if not already mounted, echoing the outcome for the provision
+// log. It does not use `set -e`: a failed mount must not stop the script,
+// since the share is best-effort, matching the overlay's own nofail option.
+func mountIfNeeded(m apkovl.Mount9p) string {
+	dir := shQuote(m.Dir)
+	return fmt.Sprintf(
+		"mkdir -p %s\n"+
+			"if mountpoint -q %s; then\n"+
+			"  echo \"stoat: %s share already mounted\"\n"+
+			"elif mount %s; then\n"+
+			"  echo \"stoat: mounted %s share\"\n"+
+			"else\n"+
+			"  echo \"stoat: could not mount %s share (continuing)\"\n"+
+			"fi\n",
+		dir, dir, m.Tag, dir, m.Tag, m.Tag,
+	)
+}
+
+// shQuote wraps s in single quotes for sh, escaping any single quote it
+// contains. m.Dir and m.Tag come from this package's own Mount9p values, not
+// user input, but the recipe body is piped to `sh -s` the same way, so this
+// keeps the same discipline.
+func shQuote(s string) string {
+	return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'"
+}
+
+// shareMountScript returns the full sh script that idempotently mounts every
+// tag in tags, best-effort. fstabPath lets a test point at a scratch fstab;
+// production always passes guestFstabPath.
+func shareMountScript(tags []apkovl.Mount9p, fstabPath string) string {
+	var b strings.Builder
+	for _, m := range tags {
+		b.WriteString(fstabEnsureLine(m, fstabPath))
+		b.WriteString(mountIfNeeded(m))
+	}
+	return b.String()
+}
+
+// mountShares runs shareMountScript over ssh for v, if shareMountTags finds
+// any tags. It writes to log the same way a recipe does, and never returns
+// an error: the share is best-effort, so a failure here must not stop
+// Provision from running the recipes that follow.
+func mountShares(ctx context.Context, v *config.VM, log io.Writer) {
+	tags := shareMountTags(v)
+	if len(tags) == 0 {
+		return
+	}
+	fmt.Fprintln(log, "\n=== mounting 9p shares ===")
+	cmd := exec.CommandContext(ctx, "ssh", Args(v, "sh", "-s")...)
+	cmd.Cancel = func() error { return cmd.Process.Signal(syscall.SIGTERM) }
+	cmd.WaitDelay = recipeShutdownGrace
+	cmd.Stdin = strings.NewReader(shareMountScript(tags, guestFstabPath))
+	cmd.Stdout = log
+	cmd.Stderr = log
+	if err := cmd.Run(); err != nil {
+		fmt.Fprintf(log, "stoat: share mount step failed, continuing: %v\n", err)
+	}
+}

internal/sshx/sharemount_test.go

@@ -0,0 +1,182 @@
+package sshx
+
+import (
+	"context"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/novusedge/stoat/internal/apkovl"
+	"github.com/novusedge/stoat/internal/config"
+)
+
+func TestShareMountTagsDiskWithShare(t *testing.T) {
+	v := &config.VM{Mode: "disk", Share: "/home/u/vms"}
+	tags := shareMountTags(v)
+	if len(tags) != 2 {
+		t.Fatalf("tags = %v, want host and work", tags)
+	}
+	if tags[0].Tag != "host" || tags[1].Tag != "work" {
+		t.Errorf("tags = %v, want [host work]", tags)
+	}
+}
+
+func TestShareMountTagsSkipsLiveVM(t *testing.T) {
+	v := &config.VM{Mode: "live", Share: "/home/u/vms"}
+	if tags := shareMountTags(v); tags != nil {
+		t.Errorf("live VM: tags = %v, want nil", tags)
+	}
+}
+
+func TestShareMountTagsSkipsCloudVM(t *testing.T) {
+	v := &config.VM{Mode: "cloud", Share: "/home/u/vms"}
+	if tags := shareMountTags(v); tags != nil {
+		t.Errorf("cloud VM: tags = %v, want nil", tags)
+	}
+}
+
+func TestShareMountTagsSkipsDiskWithNoShare(t *testing.T) {
+	v := &config.VM{Mode: "disk"}
+	if tags := shareMountTags(v); tags != nil {
+		t.Errorf("disk VM with no share: tags = %v, want nil", tags)
+	}
+}
+
+// runFstabEnsure executes fstabEnsureLine's sh fragment against a real
+// temp file, proving the idempotency logic (not just its string shape).
+func runFstabEnsure(t *testing.T, m apkovl.Mount9p, fstabPath string) {
+	t.Helper()
+	cmd := exec.Command("sh", "-c", fstabEnsureLine(m, fstabPath))
+	if out, err := cmd.CombinedOutput(); err != nil {
+		t.Fatalf("fstabEnsureLine script failed: %v\n%s", err, out)
+	}
+}
+
+func TestFstabEnsureLineAppendsWhenAbsent(t *testing.T) {
+	fstab := filepath.Join(t.TempDir(), "fstab")
+	if err := os.WriteFile(fstab, []byte("/dev/cdrom /media/cdrom iso9660 noauto,ro 0 0\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	runFstabEnsure(t, apkovl.HostMount9p, fstab)
+
+	got, err := os.ReadFile(fstab)
+	if err != nil {
+		t.Fatal(err)
+	}
+	want := "host /mnt/host 9p trans=virtio,version=9p2000.L,ro,_netdev,nofail 0 0"
+	if !strings.Contains(string(got), want) {
+		t.Errorf("fstab = %q, missing %q", got, want)
+	}
+}
+
+func TestFstabEnsureLineNoopsWhenPresent(t *testing.T) {
+	fstab := filepath.Join(t.TempDir(), "fstab")
+	initial := "work /mnt/work 9p trans=virtio,version=9p2000.L,rw,_netdev,nofail 0 0\n"
+	if err := os.WriteFile(fstab, []byte(initial), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	runFstabEnsure(t, apkovl.WorkMount9p, fstab)
+
+	got, err := os.ReadFile(fstab)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(got) != initial {
+		t.Errorf("fstab changed when the line already existed:\ngot:  %q\nwant: %q", got, initial)
+	}
+}
+
+func TestFstabEnsureLineRunTwiceAppendsOnce(t *testing.T) {
+	fstab := filepath.Join(t.TempDir(), "fstab")
+	if err := os.WriteFile(fstab, []byte(""), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	runFstabEnsure(t, apkovl.HostMount9p, fstab)
+	runFstabEnsure(t, apkovl.HostMount9p, fstab)
+
+	got, err := os.ReadFile(fstab)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if n := strings.Count(string(got), "host /mnt/host"); n != 1 {
+		t.Errorf("host line appears %d times after running twice, want 1:\n%s", n, got)
+	}
+}
+
+func TestShareMountScriptCoversEveryTag(t *testing.T) {
+	tags := []apkovl.Mount9p{apkovl.HostMount9p, apkovl.WorkMount9p}
+	script := shareMountScript(tags, guestFstabPath)
+	for _, m := range tags {
+		if !strings.Contains(script, m.Dir) {
+			t.Errorf("script missing mountpoint %q:\n%s", m.Dir, script)
+		}
+		if !strings.Contains(script, "mount "+shQuote(m.Dir)) {
+			t.Errorf("script missing mount call for %q:\n%s", m.Dir, script)
+		}
+	}
+}
+
+func TestShareMountScriptEmptyForNoTags(t *testing.T) {
+	if got := shareMountScript(nil, guestFstabPath); got != "" {
+		t.Errorf("shareMountScript(nil, ...) = %q, want empty", got)
+	}
+}
+
+// installFakeSSHEcho puts a stand-in "ssh" on PATH that runs its stdin
+// through the real sh and exits, standing in for a real guest connection.
+func installFakeSSHEcho(t *testing.T) {
+	t.Helper()
+	bin := t.TempDir()
+	if err := os.WriteFile(filepath.Join(bin, "ssh"), []byte("#!/bin/sh\nexec sh\n"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	t.Setenv("PATH", bin+":"+os.Getenv("PATH"))
+}
+
+func TestProvisionMountsSharesForDiskVMWithShare(t *testing.T) {
+	root := t.TempDir()
+	t.Setenv("STOAT_HOME", root)
+	if err := os.MkdirAll(filepath.Join(root, "recipes"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	installFakeSSHEcho(t)
+	port := acceptOnly(t, "SSH-2.0-OpenSSH_9.6\r\n")
+
+	v := &config.VM{Name: "x", SSHPort: port, Dir: t.TempDir(), Mode: "disk", Share: "/home/u/vms"}
+	if err := Provision(context.Background(), v); err != nil {
+		t.Fatalf("Provision failed: %v", err)
+	}
+
+	log, err := os.ReadFile(v.ProvisionLogPath())
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(string(log), "mounting 9p shares") {
+		t.Errorf("provision log has no share-mount step:\n%s", log)
+	}
+}
+
+func TestProvisionSkipsShareMountForLiveVM(t *testing.T) {
+	root := t.TempDir()
+	t.Setenv("STOAT_HOME", root)
+	if err := os.MkdirAll(filepath.Join(root, "recipes"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	installFakeSSHEcho(t)
+	port := acceptOnly(t, "SSH-2.0-OpenSSH_9.6\r\n")
+
+	v := &config.VM{Name: "x", SSHPort: port, Dir: t.TempDir(), Mode: "live", Share: "/home/u/vms"}
+	if err := Provision(context.Background(), v); err != nil {
+		t.Fatalf("Provision failed: %v", err)
+	}
+
+	log, err := os.ReadFile(v.ProvisionLogPath())
+	if err != nil {
+		t.Fatal(err)
+	}
+	if strings.Contains(string(log), "mounting 9p shares") {
+		t.Errorf("provision log ran the share-mount step for a live VM:\n%s", log)
+	}
+}

internal/sshx/sshx.go

@@ -231,6 +231,8 @@ func Provision(ctx context.Context, v *config.VM) (err error) {
 		return err
 	}
 
+	mountShares(ctx, v, log)
+
 	for _, name := range v.Recipes {
 		// Checked before each recipe, not left to cmd.Run below alone: a ctx
 		// cancelled between recipes must stop here rather than start one more