Skip to content

fix: make macOS start idempotent and repair VNC proxy - #30

Open
czmDeRepository wants to merge 4 commits into
cocoonstack:masterfrom
czmDeRepository:fix/macos-start-recovery
Open

fix: make macOS start idempotent and repair VNC proxy#30
czmDeRepository wants to merge 4 commits into
cocoonstack:masterfrom
czmDeRepository:fix/macos-start-recovery

Conversation

@czmDeRepository

Copy link
Copy Markdown
Contributor

Problem

Lifecycle operations can overlap: a queued vm start may acquire the VM lock after another operation has already restarted QEMU. Launching a second QEMU then fails and can disturb an otherwise healthy guest.

Separately, the host-side VNC proxy may be missing while QEMU and its Unix VNC socket are still healthy, leaving the VM running but unreachable over VNC.

Fix

  • treat vm start as idempotent when the recorded QEMU process identity is still valid
  • preserve the running VM's current launch state instead of applying start flags to an already-running process
  • recreate the VNC proxy when QEMU is healthy but the proxy is missing
  • verify the proxy using the executable name and VM-specific Unix socket path

This intentionally does not add compatibility for deployment-specific wrapper process names.

Validation

  • go test ./...

@CMGS CMGS left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

solid fix — the per-vm sock match in stopVNCProxy closes a real pid-reuse mis-kill, and the adopt path stops a queued start from disturbing a healthy guest (whose launch-failure path would have killed the live vnc proxy). small items inline: comment budget + two test helpers that already exist in cocoon utils.

gates on 9a648ea: make lint (linux+darwin) 0 issues, asl ./... clean on both GOOS, go test ./... green.

Comment thread cmd/vm/lifecycle.go Outdated
Comment thread cmd/vm/lifecycle.go Outdated
Comment thread cmd/vm/handler_test.go Outdated
Comment thread cmd/vm/vnc_test.go Outdated
Comment thread cmd/vm/vnc_test.go Outdated

@CMGS CMGS left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all five items landed, thanks. one new gate failure though: the structural layout check (asl ./..., both GOOS) rejects handler_test.go — a helper now sits between two test funcs. see inline; everything else (lint linux+darwin 0 issues, go test, vet) is green on 595ae88.

Comment thread cmd/vm/handler_test.go
}
}

func captureStdout(t *testing.T, fn func() error) (string, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

layout rule for _test.go: every Test* first, helpers after the last one — this helper between two tests fails the structural gate on both GOOS.

but i'd drop it altogether: 25 lines of os.Stdout swapping to assert a hint string is brittle and past the test's job — the contract is 'no error + record unchanged', which the test already proves. if you want to keep the assertion anyway, move the helper below TestPrepareNetNoProvision (last test in the file).

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.

2 participants