From 1ccdafe4ea489fe69c90acb21dbbdf94f41a6189 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 26 Aug 2026 09:59:15 +0800 Subject: [PATCH 1/4] review: drop clone-side watchdog leftovers CloneVMConfigFromFlags passes the snapshot policy straight through, so the local only existed for the removed override branch. The clone-path validateBackendFlags call is unreachable by construction: every knob it gates is inherited from the snapshot, and create already rejects them on Firecracker. --- cmd/core/vmconfig.go | 3 +-- cmd/vm/run.go | 5 +---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/cmd/core/vmconfig.go b/cmd/core/vmconfig.go index d8f5f73b..bce4bc73 100644 --- a/cmd/core/vmconfig.go +++ b/cmd/core/vmconfig.go @@ -108,7 +108,6 @@ func CloneVMConfigFromFlags(cmd *cobra.Command, snapCfg types.SnapshotConfig) (* if cmd.Flags().Changed("no-direct-io") { noDirectIO, _ = cmd.Flags().GetBool("no-direct-io") } - noWatchdog := snapCfg.NoWatchdog restoreMode, err := restoreModeFromFlags(cmd) if err != nil { @@ -133,7 +132,7 @@ func CloneVMConfigFromFlags(cmd *cobra.Command, snapCfg types.SnapshotConfig) (* ImageType: snapCfg.ImageType, Network: network, NoDirectIO: noDirectIO, - NoWatchdog: noWatchdog, + NoWatchdog: snapCfg.NoWatchdog, Windows: snapCfg.Windows, SharedMemory: snapCfg.SharedMemory, HugePages: snapCfg.HugePages, diff --git a/cmd/vm/run.go b/cmd/vm/run.go index 6a3a0898..7d2a2883 100644 --- a/cmd/vm/run.go +++ b/cmd/vm/run.go @@ -328,9 +328,6 @@ func (h Handler) prepareClone(ctx context.Context, cmd *cobra.Command, conf *con if err = vmCfg.Validate(); err != nil { return cloneSetup{}, err } - if err = validateBackendFlags(conf, vmCfg); err != nil { - return cloneSetup{}, err - } // Envelope pins share create's digest-lock window; a record-backed clone's source pin already protects these. releasePins, err := cmdcore.PinEnvelopeBlobs(ctx, conf, cfg.ImageBlobIDs) if err != nil { @@ -479,7 +476,7 @@ func (h Handler) createVM(cmd *cobra.Command, image string) (context.Context, *t return ctx, info, hyper, nil } -// validateBackendFlags fast-fails flag combinations the selected backend can never launch; boot-mode-dependent checks live in validateBootCompat. Shared by create, clone, and debug so the capability gate list cannot drift. +// validateBackendFlags fast-fails flag combinations the selected backend can never launch; boot-mode-dependent checks live in validateBootCompat. Shared by create and debug so the capability gate list cannot drift. func validateBackendFlags(conf *config.Config, vmCfg *types.VMConfig) error { if !conf.UseFirecracker { return nil From a3391f244136dd3a61f0fa5d15531f548c6b80ef Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 26 Aug 2026 10:16:39 +0800 Subject: [PATCH 2/4] vm: expose the guest console path in inspect (#201) Consumers driving cocoon over the CLI cannot query the CH API socket (0700, owner-only) to find where a VM's console lives. Inspect now reports console_path for running VMs: the console.sock UDS (UEFI serial, FC relay) resolved by a stat, or the CH-allocated PTY for direct boot, which each boot path (start, clone, restore) queries once via vm.info after the VMM is up and saves to console.pty in the run dir, pidfile-style. Inspect and list stay free of API calls; boot paths pay one vm.info GET plus a small buffered write, direct boot only, after resume. ToVM now also zeroes the runtime socket fields for non-running VMs: clone and restore persist boot-time paths into the record, which previously leaked as stale socket_path/vsock_socket on stopped clones, contradicting the documented State==running contract. --- docs/cli.md | 2 + hypervisor/backend.go | 1 + hypervisor/cloudhypervisor/clone.go | 1 + hypervisor/cloudhypervisor/restore.go | 1 + hypervisor/cloudhypervisor/start.go | 4 ++ hypervisor/cloudhypervisor/utils.go | 16 ++++- hypervisor/cloudhypervisor/utils_test.go | 56 ++++++++++++++++ hypervisor/inspect.go | 21 ++++-- hypervisor/inspect_test.go | 82 ++++++++++++++++++++++++ hypervisor/utils.go | 2 + types/vm.go | 1 + 11 files changed, 181 insertions(+), 6 deletions(-) create mode 100644 hypervisor/inspect_test.go diff --git a/docs/cli.md b/docs/cli.md index db53ccc5..e8ec45b3 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -282,6 +282,8 @@ Applies to `cocoon vm debug`: | ---------------- | -------- | ------------------------------------------------- | | `--escape-char` | `^]` | Escape character (single char or `^X` caret notation) | +For a running VM, `cocoon vm inspect` reports the resolved console as `console_path`: the `console.sock` UDS (UEFI serial, Firecracker relay) or the Cloud Hypervisor-allocated PTY (`/dev/pts/N`, direct-boot OCI, refreshed on every boot). External supervisors can read the console from there without opening the owner-only API socket. + ### Exec Flags `cocoon vm exec` runs a command inside a running VM via the cocoon-agent (vsock, no SSH). Stdin/stdout/stderr stream like `kubectl exec`; the host shell sees the guest command's exit code. diff --git a/hypervisor/backend.go b/hypervisor/backend.go index d5705d3b..01d6b7da 100644 --- a/hypervisor/backend.go +++ b/hypervisor/backend.go @@ -16,6 +16,7 @@ import ( const ( APISocketName = "api.sock" ConsoleSockName = "console.sock" + ConsolePTYName = "console.pty" VsockSockName = "vsock.uds" // VsockGuestCID is constant — per-VM isolation comes from distinct UDS paths. diff --git a/hypervisor/cloudhypervisor/clone.go b/hypervisor/cloudhypervisor/clone.go index bdd79ef5..94d12283 100644 --- a/hypervisor/cloudhypervisor/clone.go +++ b/hypervisor/cloudhypervisor/clone.go @@ -147,6 +147,7 @@ func (ch *CloudHypervisor) cloneAfterExtractParsed(ctx context.Context, vmID str }); err != nil { return nil, err } + saveConsolePTY(ctx, vmID, runDir, sockPath, directBoot) info := &types.VM{ ID: vmID, Hypervisor: typ, State: types.VMStateRunning, diff --git a/hypervisor/cloudhypervisor/restore.go b/hypervisor/cloudhypervisor/restore.go index 5857fd56..7d46d53c 100644 --- a/hypervisor/cloudhypervisor/restore.go +++ b/hypervisor/cloudhypervisor/restore.go @@ -117,6 +117,7 @@ func (ch *CloudHypervisor) restoreAfterExtract(ctx context.Context, vmID string, if err = resumeVM(ctx, hc); err != nil { return nil, fmt.Errorf("vm.resume: %w", err) } + saveConsolePTY(ctx, vmID, rec.RunDir, sockPath, directBoot) logger.Infof(ctx, "VM %s restored from snapshot", vmID) return ch.FinalizeRestore(ctx, vmID, vmCfg, rec, pid) diff --git a/hypervisor/cloudhypervisor/start.go b/hypervisor/cloudhypervisor/start.go index 01f763de..f4c1e3d1 100644 --- a/hypervisor/cloudhypervisor/start.go +++ b/hypervisor/cloudhypervisor/start.go @@ -24,6 +24,10 @@ func (ch *CloudHypervisor) startOne(ctx context.Context, id string) error { ch.saveCmdline(ctx, rec, args) return ch.launchProcess(ctx, rec, args, rec.ResolvedNetnsPath(), false) }, + PostLaunch: func(ctx context.Context, rec *hypervisor.VMRecord, sockPath string, _ int) error { + saveConsolePTY(ctx, rec.ID, rec.RunDir, sockPath, hypervisor.IsDirectBoot(rec.BootConfig)) + return nil + }, }) } diff --git a/hypervisor/cloudhypervisor/utils.go b/hypervisor/cloudhypervisor/utils.go index 8119ae69..1eff6234 100644 --- a/hypervisor/cloudhypervisor/utils.go +++ b/hypervisor/cloudhypervisor/utils.go @@ -40,7 +40,7 @@ const ( chMemoryRestoreMmap chMemoryRestoreMode = "CopyOnWrite" ) -var runtimeFiles = []string{hypervisor.APISocketName, pidFileName, hypervisor.ConsoleSockName, cmdlineFileName, hypervisor.VsockSockName} +var runtimeFiles = []string{hypervisor.APISocketName, pidFileName, hypervisor.ConsoleSockName, hypervisor.ConsolePTYName, cmdlineFileName, hypervisor.VsockSockName} type chMemoryRestoreMode string @@ -288,6 +288,20 @@ func resolveConsole(ctx context.Context, vmID, sockPath, consoleSock string, dir return consoleSock } +// saveConsolePTY records the per-boot PTY path for vm inspect; best-effort — the guest is up either way. +func saveConsolePTY(ctx context.Context, vmID, runDir, sockPath string, directBoot bool) { + if !directBoot { + return + } + pty := resolveConsole(ctx, vmID, sockPath, "", true) + if pty == "" { + return + } + if err := utils.AtomicWriteFileNoSync(hypervisor.ConsolePTYPath(runDir), []byte(pty), 0o600); err != nil { + log.WithFunc("cloudhypervisor.saveConsolePTY").Warnf(ctx, "save console PTY for %s: %v", vmID, err) + } +} + // qemuExpandImage grows a qcow2 disk to targetSize iff its virtual size is smaller. func qemuExpandImage(ctx context.Context, path string, targetSize int64) error { hdr, ok, err := utils.ReadQcow2Header(path) diff --git a/hypervisor/cloudhypervisor/utils_test.go b/hypervisor/cloudhypervisor/utils_test.go index f9195dba..1f2d2d79 100644 --- a/hypervisor/cloudhypervisor/utils_test.go +++ b/hypervisor/cloudhypervisor/utils_test.go @@ -1,11 +1,16 @@ package cloudhypervisor import ( + "encoding/json" "errors" "fmt" + "net" "net/http" + "os" + "path/filepath" "testing" + "github.com/cocoonstack/cocoon/hypervisor" "github.com/cocoonstack/cocoon/utils" ) @@ -37,3 +42,54 @@ func TestIsAlreadyInStateError(t *testing.T) { }) } } + +func TestSaveConsolePTYWritesQueriedPath(t *testing.T) { + runDir := t.TempDir() + sockPath := serveVMInfo(t, "/dev/pts/7") + + saveConsolePTY(t.Context(), "vm1", runDir, sockPath, true) + + got, err := os.ReadFile(hypervisor.ConsolePTYPath(runDir)) + if err != nil { + t.Fatalf("read console.pty: %v", err) + } + if string(got) != "/dev/pts/7" { + t.Errorf("console.pty = %q, want %q", got, "/dev/pts/7") + } +} + +func TestSaveConsolePTYSkipsUEFI(t *testing.T) { + runDir := t.TempDir() + + saveConsolePTY(t.Context(), "vm1", runDir, filepath.Join(runDir, "api.sock"), false) + + if utils.FileExists(hypervisor.ConsolePTYPath(runDir)) { + t.Error("console.pty written for a UEFI boot") + } +} + +func serveVMInfo(t *testing.T, ptyPath string) string { + t.Helper() + // os.MkdirTemp, not t.TempDir: unix socket paths cap at ~104 bytes. + sockDir, err := os.MkdirTemp("", "ch") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(sockDir) }) + sockPath := filepath.Join(sockDir, "api.sock") + ln, err := net.Listen("unix", sockPath) + if err != nil { + t.Fatalf("listen %s: %v", sockPath, err) + } + mux := http.NewServeMux() + mux.HandleFunc("/api/v1/vm.info", func(w http.ResponseWriter, _ *http.Request) { + resp := chVMInfoResponse{Config: chVMInfoConfig{Console: chRuntimeFile{Mode: "Pty", File: ptyPath}}} + if err := json.NewEncoder(w).Encode(resp); err != nil { + t.Errorf("encode vm.info: %v", err) + } + }) + srv := &http.Server{Handler: mux} + go srv.Serve(ln) //nolint:errcheck + t.Cleanup(func() { _ = srv.Close() }) + return sockPath +} diff --git a/hypervisor/inspect.go b/hypervisor/inspect.go index 5aee343c..fd51a41d 100644 --- a/hypervisor/inspect.go +++ b/hypervisor/inspect.go @@ -6,6 +6,7 @@ import ( "maps" "os" "slices" + "strings" "github.com/cocoonstack/cocoon/types" "github.com/cocoonstack/cocoon/utils" @@ -48,6 +49,8 @@ func (b *Backend) List(ctx context.Context) ([]*types.VM, error) { func (b *Backend) ToVM(rec *VMRecord) *types.VM { info := rec.VM info.Hypervisor = b.Typ + // Clear first: clone/restore persist boot-time sockets into the record, which would leak as stale paths once the VM stops. + info.SocketPath, info.VsockSocket, info.ConsolePath = "", "", "" if info.State == types.VMStateRunning { SetRunningSockets(&info, rec.RunDir) info.PID, _ = utils.ReadPIDFile(b.PIDFilePath(rec.RunDir)) @@ -128,15 +131,23 @@ func (b *Backend) UpdateRecord(ctx context.Context, vmID string, mutate func(*VM }) } -// SetRunningSockets fills a running VM's live sockets (API socket, bound vsock UDS) from runDir — for clone/restore records that skip ToVM. +// SetRunningSockets fills a running VM's live sockets (API socket, bound vsock UDS, guest console) from runDir — for clone/restore records that skip ToVM. func SetRunningSockets(info *types.VM, runDir string) { info.SocketPath = SocketPath(runDir) - if p := VsockSockPath(runDir); isVsockBound(p) { + if p := VsockSockPath(runDir); utils.FileExists(p) { info.VsockSocket = p } + info.ConsolePath = consolePathFromRunDir(runDir) } -func isVsockBound(path string) bool { - _, err := os.Stat(path) - return err == nil +// consolePathFromRunDir prefers the console UDS (UEFI serial, FC relay); direct-boot CH VMs instead leave the PTY path saved at boot. +func consolePathFromRunDir(runDir string) string { + if p := ConsoleSockPath(runDir); utils.FileExists(p) { + return p + } + pty, err := os.ReadFile(ConsolePTYPath(runDir)) + if err != nil { + return "" + } + return strings.TrimSpace(string(pty)) } diff --git a/hypervisor/inspect_test.go b/hypervisor/inspect_test.go new file mode 100644 index 00000000..fbbeb275 --- /dev/null +++ b/hypervisor/inspect_test.go @@ -0,0 +1,82 @@ +package hypervisor + +import ( + "os" + "testing" + + "github.com/cocoonstack/cocoon/types" +) + +func TestToVMClearsPersistedRuntimeFieldsWhenStopped(t *testing.T) { + b, _ := newMeteringTestBackend(t) + seedVMRecord(t, b, "vm1", 1, 1<<30, 10<<30, true) + if err := b.dbUpdate(t.Context(), func(idx *VMIndex) error { + idx.VMs["vm1"].State = types.VMStateStopped + idx.VMs["vm1"].SocketPath = "/stale/api.sock" + idx.VMs["vm1"].VsockSocket = "/stale/vsock.uds" + idx.VMs["vm1"].ConsolePath = "/dev/pts/9" + return nil + }); err != nil { + t.Fatalf("seed runtime fields: %v", err) + } + + rec, err := b.LoadRecord(t.Context(), "vm1") + if err != nil { + t.Fatalf("load record: %v", err) + } + info := b.ToVM(&rec) + if info.SocketPath != "" || info.VsockSocket != "" || info.ConsolePath != "" { + t.Errorf("stopped VM leaks runtime paths: socket=%q vsock=%q console=%q", + info.SocketPath, info.VsockSocket, info.ConsolePath) + } +} + +func TestToVMRunningConsolePath(t *testing.T) { + tests := []struct { + name string + setup func(t *testing.T, runDir string) + want func(runDir string) string + }{ + { + name: "console sock", + setup: func(t *testing.T, runDir string) { writeRunFile(t, ConsoleSockPath(runDir), "") }, + want: ConsoleSockPath, + }, + { + name: "pty file", + setup: func(t *testing.T, runDir string) { writeRunFile(t, ConsolePTYPath(runDir), "/dev/pts/5\n") }, + want: func(string) string { return "/dev/pts/5" }, + }, + { + name: "no console", + setup: func(*testing.T, string) {}, + want: func(string) string { return "" }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b, _ := newMeteringTestBackend(t) + seedRunningVM(t, b, "vm1", 1, 1<<30, 10<<30) + rec, err := b.LoadRecord(t.Context(), "vm1") + if err != nil { + t.Fatalf("load record: %v", err) + } + tt.setup(t, rec.RunDir) + + info := b.ToVM(&rec) + if want := tt.want(rec.RunDir); info.ConsolePath != want { + t.Errorf("ConsolePath = %q, want %q", info.ConsolePath, want) + } + if info.SocketPath != SocketPath(rec.RunDir) { + t.Errorf("SocketPath = %q, want %q", info.SocketPath, SocketPath(rec.RunDir)) + } + }) + } +} + +func writeRunFile(t *testing.T, path, content string) { + t.Helper() + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatalf("write %s: %v", path, err) + } +} diff --git a/hypervisor/utils.go b/hypervisor/utils.go index e5b4da8b..6a8b0195 100644 --- a/hypervisor/utils.go +++ b/hypervisor/utils.go @@ -131,6 +131,8 @@ func SocketPath(runDir string) string { return filepath.Join(runDir, APISocketNa func ConsoleSockPath(runDir string) string { return filepath.Join(runDir, ConsoleSockName) } +func ConsolePTYPath(runDir string) string { return filepath.Join(runDir, ConsolePTYName) } + func VsockSockPath(runDir string) string { return filepath.Join(runDir, VsockSockName) } // BalloonSize returns (bytes, enabled); disabled on Windows (virtio-win driver loops on deflation) and below MinBalloonMemory. diff --git a/types/vm.go b/types/vm.go index d9736b3e..38e86615 100644 --- a/types/vm.go +++ b/types/vm.go @@ -102,6 +102,7 @@ type VM struct { PID int `json:"pid"` SocketPath string `json:"socket_path,omitempty"` // CH API Unix socket VsockSocket string `json:"vsock_socket,omitempty"` // hybrid vsock UDS for cocoon-agent + ConsolePath string `json:"console_path,omitempty"` // guest console: console.sock UDS or CH-allocated PTY (direct boot) NetSetup From 527655d716cdb9af6bfc5a194bad4066110fa4af Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 26 Aug 2026 10:38:36 +0800 Subject: [PATCH 3/4] vm: drop runtime paths from stale-running inspect output ReconcileState flips a dead-PID VM to stopped (stale) for display, but ToVM had already populated socket_path/vsock_socket/console_path from the persisted Running state; a reused PTY number could point a supervisor at another process's terminal. Clear the paths at the flip. --- cmd/core/utils.go | 2 ++ cmd/core/utils_test.go | 25 +++++++++++++++++++++++++ docs/cli.md | 2 +- 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/cmd/core/utils.go b/cmd/core/utils.go index 319492f2..0d4bfa28 100644 --- a/cmd/core/utils.go +++ b/cmd/core/utils.go @@ -62,8 +62,10 @@ func RouteRefs(ctx context.Context, hypers []hypervisor.Hypervisor, refs []strin return result, nil } +// ReconcileState returns the effective display state; a stale-running VM also loses its runtime paths — they died with the process, and a reused PTY number must not be advertised. func ReconcileState(vm *types.VM) string { if vm.State == types.VMStateRunning && !utils.IsProcessAlive(vm.PID) { + vm.SocketPath, vm.VsockSocket, vm.ConsolePath = "", "", "" return "stopped (stale)" } return string(vm.State) diff --git a/cmd/core/utils_test.go b/cmd/core/utils_test.go index b58152cc..9293cad4 100644 --- a/cmd/core/utils_test.go +++ b/cmd/core/utils_test.go @@ -200,6 +200,31 @@ func TestPersistSnapshotDirCleansCaptureOnDirectError(t *testing.T) { } } +func TestReconcileStateClearsRuntimePathsOnStaleRunning(t *testing.T) { + vm := &types.VM{ + State: types.VMStateRunning, + PID: 0, + SocketPath: "/run/api.sock", + VsockSocket: "/run/vsock.uds", + ConsolePath: "/dev/pts/3", + } + if got := ReconcileState(vm); got != "stopped (stale)" { + t.Fatalf("ReconcileState = %q, want %q", got, "stopped (stale)") + } + if vm.SocketPath != "" || vm.VsockSocket != "" || vm.ConsolePath != "" { + t.Errorf("stale-running VM keeps runtime paths: socket=%q vsock=%q console=%q", + vm.SocketPath, vm.VsockSocket, vm.ConsolePath) + } + + alive := &types.VM{State: types.VMStateRunning, PID: os.Getpid(), ConsolePath: "/dev/pts/3"} + if got := ReconcileState(alive); got != string(types.VMStateRunning) { + t.Fatalf("ReconcileState = %q, want running", got) + } + if alive.ConsolePath == "" { + t.Error("live VM lost its console path") + } +} + // directErrSnap is a DirectCreator whose CreateFromDir always fails; the embedded interface panics on any other call, so the test proves the failure path alone. type directErrSnap struct { snapshot.Snapshot diff --git a/docs/cli.md b/docs/cli.md index e8ec45b3..211c37a2 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -282,7 +282,7 @@ Applies to `cocoon vm debug`: | ---------------- | -------- | ------------------------------------------------- | | `--escape-char` | `^]` | Escape character (single char or `^X` caret notation) | -For a running VM, `cocoon vm inspect` reports the resolved console as `console_path`: the `console.sock` UDS (UEFI serial, Firecracker relay) or the Cloud Hypervisor-allocated PTY (`/dev/pts/N`, direct-boot OCI, refreshed on every boot). External supervisors can read the console from there without opening the owner-only API socket. +For a running VM, `cocoon vm inspect` reports the console resolved at boot (start, clone, restore) as `console_path`: the `console.sock` UDS (UEFI serial, Firecracker relay) or the Cloud Hypervisor-allocated PTY (`/dev/pts/N`, direct-boot OCI). External supervisors can read the console from there without opening the owner-only API socket. A direct-boot VM booted by an older cocoon reports it from its next start. ### Exec Flags From 10a67ee9efceeb88a23796d2ea81a8d55e11477d Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 26 Aug 2026 10:44:02 +0800 Subject: [PATCH 4/4] vm: reconcile stale-running state in one-shot status JSON statusOnce serialized ToVM output directly, so vm status/list --format json skipped the dead-PID flip and its runtime-path clearing that inspect, table, and event modes already apply. --- cmd/vm/status.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/vm/status.go b/cmd/vm/status.go index a2fcb646..14bf08c8 100644 --- a/cmd/vm/status.go +++ b/cmd/vm/status.go @@ -97,6 +97,10 @@ func statusOnce(ctx context.Context, hypers []hypervisor.Hypervisor, filters []s } vms = applyFilters(vms, filters) sortVMs(vms) + // JSON serializes vms as-is, so stale-running records must reconcile here, not per output row. + for _, vm := range vms { + vm.State = types.VMState(cmdcore.ReconcileState(vm)) + } return renderVMList(vms, format, scopeDir) }