From 5359a1827997697bb56c4b296fe7386c6dbdc9c8 Mon Sep 17 00:00:00 2001 From: chris lee Date: Sun, 11 Oct 2026 17:49:10 -0400 Subject: [PATCH 1/7] macOS guests 4/6: stopped snapshots, same-instance standby and restore, and stopped clones Stopped snapshots after a graceful stop, same-instance restore through a staged disk swap, experimental same-instance standby, and clones from a stopped source with a fresh machine identifier, MAC and SSH host keys. --- cmd/vz-shim/server.go | 16 + docs/macos-snapshots.md | 107 +++++ docs/macos-support.md | 16 +- lib/hypervisor/snapshot_support.go | 8 + lib/hypervisor/snapshot_support_test.go | 34 ++ lib/hypervisor/tracing.go | 12 + lib/hypervisor/vz/client.go | 35 +- lib/hypervisor/vz/client_test.go | 93 +++++ lib/hypervisor/vz/shimconfig/config.go | 2 - lib/instances/fork.go | 16 +- lib/instances/macos.go | 66 +-- lib/instances/macos_admission_test.go | 42 ++ lib/instances/macos_boot_admission_test.go | 84 ++++ lib/instances/macos_identity.go | 246 +++++++++++ lib/instances/macos_identity_buffer_test.go | 20 + lib/instances/macos_identity_darwin.go | 16 + lib/instances/macos_identity_other.go | 9 + lib/instances/macos_identity_test.go | 71 ++++ lib/instances/macos_memory_snapshot_test.go | 425 ++++++++++++++++++++ lib/instances/macos_shim_gone_test.go | 24 ++ lib/instances/macos_snapshot_plan_test.go | 55 +++ lib/instances/macos_test.go | 75 +++- lib/instances/manager.go | 18 + lib/instances/restore.go | 40 +- lib/instances/snapshot.go | 50 ++- lib/instances/snapshot_compression.go | 3 + lib/instances/snapshot_macos.go | 352 ++++++++++++++++ lib/instances/snapshot_macos_darwin_test.go | 73 ++++ lib/instances/snapshot_macos_test.go | 155 +++++++ lib/instances/snapshot_schedule.go | 4 +- lib/instances/snapshot_schedule_test.go | 6 +- lib/instances/standby.go | 26 +- lib/instances/start.go | 12 + lib/instances/stop.go | 5 + lib/instances/types.go | 6 + lib/instances/vm_config_validation.go | 3 + lib/network/allocate.go | 11 + 37 files changed, 2135 insertions(+), 101 deletions(-) create mode 100644 docs/macos-snapshots.md create mode 100644 lib/hypervisor/snapshot_support.go create mode 100644 lib/hypervisor/snapshot_support_test.go create mode 100644 lib/instances/macos_admission_test.go create mode 100644 lib/instances/macos_boot_admission_test.go create mode 100644 lib/instances/macos_identity.go create mode 100644 lib/instances/macos_identity_buffer_test.go create mode 100644 lib/instances/macos_identity_darwin.go create mode 100644 lib/instances/macos_identity_other.go create mode 100644 lib/instances/macos_identity_test.go create mode 100644 lib/instances/macos_memory_snapshot_test.go create mode 100644 lib/instances/macos_snapshot_plan_test.go create mode 100644 lib/instances/snapshot_macos.go create mode 100644 lib/instances/snapshot_macos_darwin_test.go create mode 100644 lib/instances/snapshot_macos_test.go diff --git a/cmd/vz-shim/server.go b/cmd/vz-shim/server.go index 74640b8eb..621c0f3b2 100644 --- a/cmd/vz-shim/server.go +++ b/cmd/vz-shim/server.go @@ -59,6 +59,7 @@ func (s *ShimServer) Handler() http.Handler { mux.HandleFunc("PUT /api/v1/vm.resume", s.handleResume) mux.HandleFunc("PUT /api/v1/vm.shutdown", s.handleShutdown) mux.HandleFunc("PUT /api/v1/vm.snapshot", s.handleSnapshot) + mux.HandleFunc("GET /api/v1/vm.snapshot-support", s.handleSnapshotSupport) mux.HandleFunc("GET /api/v1/vm.balloon", s.handleGetBalloon) mux.HandleFunc("PUT /api/v1/vm.balloon", s.handleSetBalloon) mux.HandleFunc("PUT /api/v1/vm.power-button", s.handlePowerButton) @@ -138,6 +139,21 @@ func (s *ShimServer) handleShutdown(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } +func (s *ShimServer) handleSnapshotSupport(w http.ResponseWriter, r *http.Request) { + s.mu.RLock() + defer s.mu.RUnlock() + response := struct { + Supported bool `json:"supported"` + Reason string `json:"reason,omitempty"` + }{Supported: true} + if err := validateSaveRestoreSupport(s.vmConfig); err != nil { + response.Supported = false + response.Reason = err.Error() + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(response) +} + func (s *ShimServer) handleSnapshot(w http.ResponseWriter, r *http.Request) { s.mu.Lock() defer s.mu.Unlock() diff --git a/docs/macos-snapshots.md b/docs/macos-snapshots.md new file mode 100644 index 000000000..8ed3d1d43 --- /dev/null +++ b/docs/macos-snapshots.md @@ -0,0 +1,107 @@ +# macOS snapshots, schedules and restore (PR 4, draft) + +Supported experimentally: stopped disk snapshots, same-instance memory checkpoints, +snapshot schedules, and cold forks from gracefully stopped instances or snapshots. +Independent warm memory forks remain unsupported. Native validation of the new +memory-snapshot and schedule paths is still required. + +## Stopped snapshots and cold forks + +- `kind: Stopped` requires a stopped source and a graceful shutdown receipt: + acknowledged guest shutdown with confirmed owned VMM exit. A forced stop is + refused with 409. +- Restore targets are Stopped or Running. The source instance, hardware model, + machine identifier and MAC must match the snapshot. +- A stopped-snapshot fork requires a graceful receipt and an enabled guest agent. + Disk and auxiliary storage are copied, the clone gets a fresh machine identifier + and MAC, and SSH host keys are rekeyed on first boot. The default target is + Stopped; Running cold-boots and completes rekeying before returning. +- Forking a stopped instance uses the same identity/rekey contract. Hostnames, + account credentials and application-specific identities are not rekeyed. +- Failed rekeying stops the clone and leaves rekey pending for a later retry. + +## Same-instance memory snapshots + +Use `POST /instances/{id}/snapshots` with `{"kind":"Standby"}`. + +- A Running source goes through pause, VZ machine-state save and confirmed VMM + shutdown. The disk, auxiliary storage and machine state are copied together + while the VMM is down; then the source resumes from its own checkpoint. +- A Standby source is copied without being resumed. Stopped and other source + states are rejected with 409. +- Before pausing, the shim validates the exact VM configuration using Apple's + `validateSaveRestoreSupport`. Host/architecture capability checks alone are + not proof that the guest's configured devices support save/restore. +- Pause waits for raw Paused state, not transitional Pausing state, before save. + Error-path resume and source recovery use bounded cleanup contexts that survive + cancellation of the snapshot request. If recovery fails, the API reports the + failure; the source may need explicit recovery. +- VZ machine-state files stay uncompressed. Explicit enabled compression is + rejected before capture; Linux server compression defaults do not apply. + +Restore using `POST /instances/{id}/snapshots/{snapshot_id}/restore`. + +- This is a same-instance operation, not a clone. Restore requires an inactive + Stopped or Standby source; Running, Paused and other live states are refused. +- The default target for a memory snapshot is Running. Explicit Standby installs + the checkpoint without resuming it. Stopped discards memory and does not create + a graceful-shutdown receipt; a subsequent clean boot/stop is required before + making a stopped snapshot or cold fork. +- Preflight checks saved CPU/RAM, hardware model, machine identifier, NIC MAC, + disk/auxiliary paths and the complete memory/disk/auxiliary bundle before + replacing source storage. Invalid payloads leave current storage untouched. +- Restored metadata is staged with the payload, including the current instance + name and expiration. Directory-swap recovery therefore commits or rolls back + the matching metadata and storage together. Host phase accounting is preserved, + rather than replayed from the checkpoint; memory restore retains its boot time. + An explicit Stopped target suppresses automatic restart. +- Restoring never mutates the persistent snapshot. Framework restore failure + after installation leaves the restored checkpoint available in Standby. +- If the API dies during a restore, the shim it started keeps loading the VM. + Until it opens its socket the instance reads Standby, and start or restore + return 409 while that shim runs. Once loaded it reads Paused: `POST /restore` + resumes it and completes the restore (records the shim, removes the consumed + checkpoint), and DELETE kills it. As on Linux, stop does not accept Paused. +- Saved state is not portable: use the originating Mac and OS account, with a + compatible framework version and VM configuration. API availability on macOS + 14+ does not guarantee support for a particular macOS guest configuration. + +## Schedules + +The existing snapshot-schedule API, interval handling and retention policies are +shared with Linux. macOS schedules require host save/restore support; admission +also probes the exact configuration when a live VM is available. + +- A gracefully Stopped source produces a stopped snapshot. +- A Running source produces a memory snapshot and is resumed afterwards. +- A Standby source produces a memory snapshot and remains in Standby. +- Ineligible state, absent graceful receipt, configuration or capture failure is + recorded in `last_error`; no successful snapshot ID is reported for that run. +- Failed runs consume the interval, matching existing scheduler behavior. The + next run follows the configured cadence rather than tight retries. +- Retention and schedule deletion use the existing controller. Scheduling does + not stop a running guest to manufacture a graceful disk snapshot. + +## Warm memory forks are not supported + +`ForkSnapshot` rejects macOS memory snapshots with 501, and `ForkInstance` +requires a gracefully stopped macOS source. VZ restores saved state only into an +identical configuration: on Apple silicon, changing the MAC, the machine +identifier, the NIC or the auxiliary storage fails with "invalid argument", while +relocating paths works. Apple documents that two VMs running concurrently with +the same machine identifier have undefined guest behavior, so copies of one saved +state cannot run side by side. Repeated same-instance restore is the supported +warm start. + +References: +- [Apple: Mac platform configuration](https://developer.apple.com/documentation/virtualization/vzmacplatformconfiguration) +- [Apple: restoring machine state](https://developer.apple.com/documentation/virtualization/vzvirtualmachine/restoremachinestatefrom(url:completionhandler:)) + +## Validation + +Linux-runnable tests cover the filesystem and metadata paths (capture, restore, +isolation, manifest checks, schedules, retention and crash-swap recovery); mock +HTTP fixtures cover VZ pause, cancellation and configuration probing. Native +Apple-silicon runs have exercised Running capture/resume, same-instance memory +rollback, repeated restore, schedules with retention, cancellation after pause, +API crash during restore, and stopped clones with fresh identities. diff --git a/docs/macos-support.md b/docs/macos-support.md index 18cbe1c61..a57072825 100644 --- a/docs/macos-support.md +++ b/docs/macos-support.md @@ -15,14 +15,18 @@ owns the behavior. | Memory reclaim | Unsupported | Reported per guest as `unsupported`. No balloon change. | PR 1 | | Guest port forwarding | Agent-gated | HTTP and WebSocket to a guest loopback port at `/instances/{id}/ports/{port}/*`. The image starts its own services. See [macos-guest-ports.md](macos-guest-ports.md). | PR 5 | | Builds | Partial | Opt-in `macos_builds`. Restricted recipe. Linux Dockerfile builds unsupported. | PR 6 | -| Stopped snapshots, same-instance restore | Partial | Graceful stop required. Running restore has an open readiness issue. | PR 4 | -| Standby, memory snapshot, memory fork | Unsupported | Rejected with a stable error. | — | -| Stopped clones, fork from snapshot | Unsupported | Needs identity rebinding and guest rekeying. | PR 4 (later) | +| Stopped snapshots, same-instance restore | Experimental | Graceful stop required. Stopped and Running targets; restored metadata and storage are staged together. | PR 4 | +| Standby and restore (same instance) | Experimental | Saves memory, restores the same instance. Restore can fail (about 2% observed, cause unknown); a failed restore leaves the instance in Standby. Recover by deleting, as on Linux. Standby auto-policies rejected: no host-visible connection tracking on NAT. | PR 4 | +| Same-instance memory snapshots | Experimental | Running/Standby capture, with exact VZ configuration validation and an uncompressed disk/aux/memory bundle. Same-instance restore only; new path needs native QA. | PR 4 | +| Independent warm memory forks | Unsupported | VZ restores saved state only into an identical configuration (same machine identifier and MAC), and Apple documents concurrent VMs sharing an identifier as undefined. Repeated same-instance restore is the warm start. | — | +| Stopped clones (fork from stopped) | Experimental | Rebinds machine identifier and MAC; rekeys SSH host keys on first start. Hostname and credentials not yet checked. | PR 4 | +| Fork from stopped snapshot | Experimental | Graceful receipt and enabled agent required. Fresh machine identifier/MAC; SSH rekey on first cold boot. Stopped and Running targets supported. | PR 4 | | Update (PATCH), VM restart policy | Partial | Expiration/TTL and `restart_policy: always` or `never` are supported. Manual stop suppresses automatic restart. A shim that dies without removing its socket (for example, SIGKILL) reads as Stopped, so `always` restarts it. `on_failure` is rejected: no workload exit status is available. | PR 1 | | Exec health checks | Agent-gated | Supported at create and PATCH; `type: none` disables them. HTTP/TCP checks remain unsupported. | PR 3 | | Volumes, env and commands, credentials and auto-standby policies, passthrough, shaping | Unsupported | Rejected at create, start, or update. | — | | Ingress | Unsupported | Refused in macOS-only mode and for macOS targets. | — | -| Snapshot schedules | Unsupported | Refused at admission. | — | +| Snapshot schedules | Experimental | State-based stopped or memory capture, existing cadence/retention, host save/restore gate and live configuration probe. New path needs native QA. | PR 4 | -Limits that apply everywhere: preserved machine identity (one active instance per -identifier, no rekeying), no clean-base sanitation audit, and no external review verdict. +Limits that apply everywhere: one active VM per machine identifier, no clean-base +sanitation audit, and no external review verdict. Same-instance memory restore +preserves identity; stopped clones get fresh machine identifiers and SSH host keys. diff --git a/lib/hypervisor/snapshot_support.go b/lib/hypervisor/snapshot_support.go new file mode 100644 index 000000000..1afcc89dc --- /dev/null +++ b/lib/hypervisor/snapshot_support.go @@ -0,0 +1,8 @@ +package hypervisor + +import "context" + +// SnapshotSupportValidator checks configuration-specific support before pausing. +type SnapshotSupportValidator interface { + ValidateSnapshotSupport(context.Context) error +} diff --git a/lib/hypervisor/snapshot_support_test.go b/lib/hypervisor/snapshot_support_test.go new file mode 100644 index 000000000..f643d1572 --- /dev/null +++ b/lib/hypervisor/snapshot_support_test.go @@ -0,0 +1,34 @@ +package hypervisor + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/require" +) + +type fakeSnapshotSupportValidator struct { + fakeHypervisor + err error + called bool +} + +func (h *fakeSnapshotSupportValidator) ValidateSnapshotSupport(context.Context) error { + h.called = true + return h.err +} + +func TestTracingPreservesSnapshotSupportValidation(t *testing.T) { + probeErr := errors.New("configuration probe failed") + hv := &fakeSnapshotSupportValidator{err: probeErr} + wrapped := WrapHypervisor(TypeVZ, hv) + validator, ok := wrapped.(SnapshotSupportValidator) + require.True(t, ok) + require.ErrorIs(t, validator.ValidateSnapshotSupport(context.Background()), probeErr) + require.True(t, hv.called) + hv.err = nil + require.NoError(t, validator.ValidateSnapshotSupport(context.Background())) + unsupported := WrapHypervisor(TypeCloudHypervisor, fakeHypervisor{}).(SnapshotSupportValidator) + require.ErrorIs(t, unsupported.ValidateSnapshotSupport(context.Background()), ErrNotSupported) +} diff --git a/lib/hypervisor/tracing.go b/lib/hypervisor/tracing.go index 4694c4e21..4abf828e2 100644 --- a/lib/hypervisor/tracing.go +++ b/lib/hypervisor/tracing.go @@ -219,6 +219,18 @@ func (h *tracingHypervisor) Resume(ctx context.Context) (err error) { return h.next.Resume(ctx) } +func (h *tracingHypervisor) ValidateSnapshotSupport(ctx context.Context) (err error) { + validator, ok := h.next.(SnapshotSupportValidator) + if !ok { + return ErrNotSupported + } + ctx, span := startTraceSpan(ctx, h.tracer, "hypervisor.validate_snapshot_support", + h.spanAttrs(attribute.String("operation", "validate_snapshot_support"))..., + ) + defer func() { finishTraceSpan(span, err) }() + return validator.ValidateSnapshotSupport(ctx) +} + func (h *tracingHypervisor) Snapshot(ctx context.Context, destPath string) (err error) { ctx, span := startTraceSpan(ctx, h.tracer, "hypervisor.snapshot", h.spanAttrs( diff --git a/lib/hypervisor/vz/client.go b/lib/hypervisor/vz/client.go index 1f2bc7ae7..984c7edd6 100644 --- a/lib/hypervisor/vz/client.go +++ b/lib/hypervisor/vz/client.go @@ -262,7 +262,28 @@ func (c *Client) GetVMInfo(ctx context.Context) (*hypervisor.VMInfo, error) { } func (c *Client) Pause(ctx context.Context) error { - return c.doPut(ctx, "/api/v1/vm.pause", nil) + if err := c.doPut(ctx, "/api/v1/vm.pause", nil); err != nil { + return err + } + return c.waitForRawState(ctx, "Paused") +} + +func (c *Client) ValidateSnapshotSupport(ctx context.Context) error { + body, err := c.doGet(ctx, "/api/v1/vm.snapshot-support") + if err != nil { + return err + } + var response struct { + Supported bool `json:"supported"` + Reason string `json:"reason"` + } + if err := json.Unmarshal(body, &response); err != nil { + return fmt.Errorf("decode snapshot support: %w", err) + } + if !response.Supported { + return fmt.Errorf("%w: VZ configuration cannot be saved/restored: %s", hypervisor.ErrNotSupported, response.Reason) + } + return nil } func (c *Client) Resume(ctx context.Context) error { @@ -279,16 +300,16 @@ func (c *Client) Resume(ctx context.Context) error { } const ( - resumeReadyTimeout = 10 * time.Second - resumeReadyPollInterval = 25 * time.Millisecond + stateReadyTimeout = 10 * time.Second + stateReadyPollInterval = 25 * time.Millisecond ) // waitForRawState polls vm.info until the shim's raw state string equals want or -// resumeReadyTimeout elapses. It checks the raw state (not the hypervisor.VMState +// stateReadyTimeout elapses. It checks the raw state (not the hypervisor.VMState // mapping, which collapses "Resuming" into Running) so the post-condition is the // guest actually being in the requested state. func (c *Client) waitForRawState(ctx context.Context, want string) error { - deadline := time.Now().Add(resumeReadyTimeout) + deadline := time.Now().Add(stateReadyTimeout) for { state, err := c.rawVMState(ctx) if err == nil && state == want { @@ -298,12 +319,12 @@ func (c *Client) waitForRawState(ctx context.Context, want string) error { if err != nil { return fmt.Errorf("wait for vm state %q: %w", want, err) } - return fmt.Errorf("vm did not reach state %q within %s (last state %q)", want, resumeReadyTimeout, state) + return fmt.Errorf("vm did not reach state %q within %s (last state %q)", want, stateReadyTimeout, state) } select { case <-ctx.Done(): return ctx.Err() - case <-time.After(resumeReadyPollInterval): + case <-time.After(stateReadyPollInterval): } } } diff --git a/lib/hypervisor/vz/client_test.go b/lib/hypervisor/vz/client_test.go index c3090540f..cba9afcdd 100644 --- a/lib/hypervisor/vz/client_test.go +++ b/lib/hypervisor/vz/client_test.go @@ -13,6 +13,8 @@ import ( "sync/atomic" "testing" "time" + + "github.com/kernel/hypeman/lib/hypervisor" ) const connectionCloseTimeout = 2 * time.Second @@ -84,6 +86,97 @@ func (f *unixHTTPFixture) waitForNoConnections(t *testing.T) { } } +func TestPauseWaitsForRawPausedStateBeforeSnapshot(t *testing.T) { + var reads atomic.Int32 + fixture := newUnixHTTPFixture(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v1/vmm.ping": + _, _ = w.Write([]byte("OK")) + case "/api/v1/vm.pause": + w.WriteHeader(http.StatusNoContent) + case "/api/v1/vm.info": + state := "Pausing" + if reads.Add(1) >= 3 { + state = "Paused" + } + _, _ = w.Write([]byte(`{"state":"` + state + `"}`)) + case "/api/v1/vm.snapshot": + if reads.Load() < 3 { + http.Error(w, "not paused", http.StatusBadRequest) + return + } + w.WriteHeader(http.StatusNoContent) + default: + http.NotFound(w, r) + } + })) + client, err := NewClient(fixture.socketPath()) + if err != nil { + t.Fatal(err) + } + if err := client.Pause(context.Background()); err != nil { + t.Fatal(err) + } + if err := client.Snapshot(context.Background(), "/snapshot"); err != nil { + t.Fatal(err) + } +} + +func TestPauseWaitHonorsCancellation(t *testing.T) { + fixture := newUnixHTTPFixture(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v1/vmm.ping": + _, _ = w.Write([]byte("OK")) + case "/api/v1/vm.pause": + w.WriteHeader(http.StatusNoContent) + default: + _, _ = w.Write([]byte(`{"state":"Pausing"}`)) + } + })) + client, err := NewClient(fixture.socketPath()) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + if err := client.Pause(ctx); !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("expected deadline exceeded, got %v", err) + } +} + +func TestValidateSnapshotSupportUsesConfigurationProbe(t *testing.T) { + for _, test := range []struct { + name string + response string + wantErr bool + }{ + {"supported", `{"supported":true}`, false}, + {"unsupported", `{"supported":false,"reason":"unsupported device"}`, true}, + {"missing-result", `{}`, true}, + } { + t.Run(test.name, func(t *testing.T) { + fixture := newUnixHTTPFixture(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/v1/vmm.ping" { + _, _ = w.Write([]byte("OK")) + return + } + _, _ = w.Write([]byte(test.response)) + })) + client, err := NewClient(fixture.socketPath()) + if err != nil { + t.Fatal(err) + } + err = client.ValidateSnapshotSupport(context.Background()) + if test.wantErr && !errors.Is(err, hypervisor.ErrNotSupported) { + t.Fatalf("expected unsupported configuration, got %v", err) + } + if !test.wantErr && err != nil { + t.Fatal(err) + } + }) + } +} + func TestRepeatedClientsReleaseSuccessfulControlConnections(t *testing.T) { fixture := newUnixHTTPFixture(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { diff --git a/lib/hypervisor/vz/shimconfig/config.go b/lib/hypervisor/vz/shimconfig/config.go index 2cd81671a..039114de8 100644 --- a/lib/hypervisor/vz/shimconfig/config.go +++ b/lib/hypervisor/vz/shimconfig/config.go @@ -1,5 +1,3 @@ -//go:build darwin - // Package shimconfig defines the configuration types shared between // the hypeman API server and the vz-shim subprocess. package shimconfig diff --git a/lib/instances/fork.go b/lib/instances/fork.go index 5a5c57847..2c00348e6 100644 --- a/lib/instances/fork.go +++ b/lib/instances/fork.go @@ -46,6 +46,11 @@ func (m *manager) forkInstance(ctx context.Context, id string, req ForkInstanceR return nil, "", false, err } source := m.toInstance(ctx, meta) + // A macOS clone copies the source's disk and aux, so it is taken only from a + // gracefully stopped source, as stopped snapshots are. + if meta.MacOS != nil && (source.State != StateStopped || !meta.StopGraceful) { + return nil, "", false, fmt.Errorf("%w: macOS fork requires a gracefully stopped source (state %s)", ErrInvalidState, source.State) + } targetState, err := resolveForkTargetState(req.TargetState, source.State) if err != nil { return nil, "", false, err @@ -242,7 +247,7 @@ func (m *manager) forkInstanceFromStoppedOrStandby(ctx context.Context, id strin if existsByMetadata { return nil, false, fmt.Errorf("%w: instance name '%s' already exists", ErrAlreadyExists, req.Name) } - if stored.NetworkEnabled { + if stored.usesLinuxNetwork() { exists, err := m.networkManager.NameExists(ctx, req.Name, "") if err != nil { return nil, false, fmt.Errorf("check instance name availability: %w", err) @@ -329,6 +334,9 @@ func (m *manager) forkInstanceFromStoppedOrStandby(ctx context.Context, id strin forkMeta.IP = "" forkMeta.MAC = "" } + if err := prepareMacOSClone(&forkMeta); err != nil { + return nil, false, err + } restoreNeedsSourceLock := false if source.State == StateStandby { @@ -410,7 +418,7 @@ func (m *manager) saveForkMetadata(ctx context.Context, meta *metadata) error { retErr = fmt.Errorf("%w: instance name '%s' already exists", ErrAlreadyExists, name) return retErr } - if meta.NetworkEnabled { + if meta.usesLinuxNetwork() { exists, err := m.networkManager.NameExists(holdCtx, name, "") if err != nil { retErr = fmt.Errorf("check instance name availability: %w", err) @@ -421,6 +429,10 @@ func (m *manager) saveForkMetadata(ctx context.Context, meta *metadata) error { return retErr } } + if err := m.assignMacOSCloneMAC(&meta.StoredMetadata); err != nil { + retErr = err + return retErr + } if err := m.saveMetadata(meta); err != nil { retErr = fmt.Errorf("save fork metadata: %w", err) return retErr diff --git a/lib/instances/macos.go b/lib/instances/macos.go index cc25613a3..a46603083 100644 --- a/lib/instances/macos.go +++ b/lib/instances/macos.go @@ -196,7 +196,7 @@ func applyMacOSNetwork(stored *StoredMetadata) *network.NetworkConfig { } // admitMacOSBoot holds the macOS boot lock and checks socket paths and identity -// admission before a macOS guest boots. The returned release ends the hold; for +// and MAC admission before a macOS guest boots. The returned release ends the hold; for // other guests it does nothing. func (m *manager) admitMacOSBoot(ctx context.Context, stored *StoredMetadata) (func(), error) { if stored.MacOS == nil { @@ -206,7 +206,7 @@ func (m *manager) admitMacOSBoot(ctx context.Context, stored *StoredMetadata) (f return nil, err } m.macOSBootMu.Lock() - if err := m.checkMacOSIdentityAvailable(ctx, stored); err != nil { + if err := m.checkMacOSBootAvailable(ctx, stored); err != nil { m.macOSBootMu.Unlock() return nil, err } @@ -239,41 +239,16 @@ func (m *manager) checkMacOSIdentityAvailable(ctx context.Context, stored *Store if len(pids) > 0 { return fmt.Errorf("%w: a vz-shim (pid %d) is already running for this instance; if an interrupted operation left it, wait for it or delete the instance", ErrInvalidState, pids[0]) } - entries, err := os.ReadDir(m.paths.GuestsDir()) - if err != nil { - return err - } - for _, entry := range entries { - if !entry.IsDir() || entry.Name() == stored.Id { - continue - } - meta, err := m.loadMetadata(entry.Name()) - if errors.Is(err, ErrNotFound) { - // A create in progress or a leftover directory has no instance to admit against. - continue - } - if err != nil { - return fmt.Errorf("check Mac identity admission: %w", err) - } - if meta.MacOS == nil { - continue - } - sameIdentity := bytes.Equal(meta.MacOS.MachineIdentifier, stored.MacOS.MachineIdentifier) - // Two live guests on one MAC share one vmnet DHCP lease. - sameMAC := stored.NetworkEnabled && meta.NetworkEnabled && sameMACAddress(macOSBootMAC(stored), macOSBootMAC(&meta.StoredMetadata)) - if !sameIdentity && !sameMAC { - continue + return m.forEachOtherInstance(stored.Id, "check Mac identity admission", func(meta *metadata) error { + if meta.MacOS == nil || !bytes.Equal(meta.MacOS.MachineIdentifier, stored.MacOS.MachineIdentifier) { + return nil } state := m.deriveStateWithoutHydration(ctx, &meta.StoredMetadata).State - if !state.RequiresVMM() && state != StateUnknown { - continue - } - if sameIdentity { + if state.RequiresVMM() || state == StateUnknown { return fmt.Errorf("%w: macOS template identity is already in use by instance %s; stop it first", ErrInsufficientResources, meta.Id) } - return fmt.Errorf("%w: MAC %s is already in use by instance %s; stop it first", ErrInsufficientResources, macOSBootMAC(stored), meta.Id) - } - return nil + return nil + }) } // maxMacOSSocketPath is the longest Unix socket path macOS accepts: sun_path holds @@ -320,21 +295,16 @@ func guestAgentReadyExecutable(stored *StoredMetadata) string { // macOS guest, so a normal stop can record a graceful receipt and strict export waits. const macOSMinStopTimeout = 30 -// admitGuestOperation refuses an operation this server cannot run for the guest: -// any Linux operation on a macOS-only server, and operations macOS guests do not -// implement. Callers check it right after loading the record under the lock. -func (m *manager) admitGuestOperation(stored *StoredMetadata, operation string) error { - if err := m.requireMacOSGuest(stored); err != nil { - return err - } - if stored.MacOS == nil { - return nil - } - switch operation { - case "fork", "fork snapshot", "restore", "snapshot", "restore snapshot", "snapshot schedules", "standby": - return fmt.Errorf("%w: %s is not implemented for experimental macOS instances", ErrInvalidRequest, operation) - } - return nil +// hypervisorProcessLive reports whether the instance's VMM process is still running. +func hypervisorProcessLive(inst *Instance) bool { + pid, err := resolveLiveHypervisorPID(inst.HypervisorProcessIdentity, inst.SocketPath) + return err == nil && pid != 0 +} + +// admitGuestOperation applies the server's guest-type restriction. Operation-specific +// admission follows at the caller, under the instance lock. +func (m *manager) admitGuestOperation(stored *StoredMetadata, _ string) error { + return m.requireMacOSGuest(stored) } // vmnet stores octets without leading zeroes. A lease is observed addressing, diff --git a/lib/instances/macos_admission_test.go b/lib/instances/macos_admission_test.go new file mode 100644 index 000000000..c69ce0a5b --- /dev/null +++ b/lib/instances/macos_admission_test.go @@ -0,0 +1,42 @@ +package instances + +import ( + "testing" + + "github.com/kernel/hypeman/lib/images" + "github.com/kernel/hypeman/lib/paths" + "github.com/stretchr/testify/require" +) + +func TestClearMacOSStopReceiptPersistsBeforeBoot(t *testing.T) { + p := paths.New(t.TempDir()) + m := &manager{paths: p} + require.NoError(t, m.ensureDirectories("mac")) + stored := StoredMetadata{Id: "mac", DataDir: p.InstanceDir("mac"), MacOS: &images.MacOSImage{}, StopGraceful: true} + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: stored})) + + require.NoError(t, m.clearMacOSStopReceipt(&stored)) + require.False(t, stored.StopGraceful) + meta, err := m.loadMetadata("mac") + require.NoError(t, err) + require.False(t, meta.StopGraceful, "the cleared receipt must be on disk before the guest boots") +} + +func TestAssignMacOSCloneMACAvoidsStoredMACs(t *testing.T) { + p := paths.New(t.TempDir()) + m := &manager{paths: p} + const taken = "02:00:00:00:00:01" + require.NoError(t, m.ensureDirectories("other")) + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: StoredMetadata{Id: "other", DataDir: p.InstanceDir("other"), MAC: taken}})) + + source := &images.MacOSImage{MAC: taken} + clone := StoredMetadata{Id: "clone", NetworkEnabled: true, MAC: taken, MacOS: source} + require.NoError(t, m.assignMacOSCloneMAC(&clone)) + require.NotEqual(t, taken, clone.MAC) + require.Equal(t, clone.MAC, clone.MacOS.MAC) + require.Equal(t, taken, source.MAC, "the clone must not alias its source's macOS struct") + + offline := StoredMetadata{Id: "clone2", MacOS: &images.MacOSImage{}} + require.NoError(t, m.assignMacOSCloneMAC(&offline)) + require.Empty(t, offline.MAC, "a clone without networking gets no MAC") +} diff --git a/lib/instances/macos_boot_admission_test.go b/lib/instances/macos_boot_admission_test.go new file mode 100644 index 000000000..9f7f448cf --- /dev/null +++ b/lib/instances/macos_boot_admission_test.go @@ -0,0 +1,84 @@ +package instances + +import ( + "context" + "errors" + "os" + "path/filepath" + "testing" + "time" + + "github.com/kernel/hypeman/lib/hypervisor" + "github.com/kernel/hypeman/lib/paths" + "github.com/stretchr/testify/require" +) + +type macOSBootAdmissionStarter struct { + concurrentForkPrepareTestStarter + manager *manager + started chan string + release <-chan struct{} +} + +func (s *macOSBootAdmissionStarter) StartVM(_ context.Context, _ *paths.Paths, _ string, socket string, _ hypervisor.VMConfig) (int, hypervisor.Hypervisor, error) { + id := filepath.Base(filepath.Dir(socket)) + s.started <- id + <-s.release + if err := os.WriteFile(socket, nil, 0600); err != nil { + return 0, nil, err + } + s.manager.storeCachedHypervisorState(id, hypervisor.StateRunning) + return os.Getpid(), lifecycleNoopHypervisor{state: hypervisor.StateRunning}, nil +} + +func TestMacOSConcurrentBootsRecheckMACUnderBootLock(t *testing.T) { + p := paths.New(t.TempDir()) + m := &manager{paths: p} + release := make(chan struct{}) + starter := &macOSBootAdmissionStarter{manager: m, started: make(chan string, 2), release: release} + hvType := hypervisor.Type("macos-boot-admission-test") + m.vmStarters = map[hypervisor.Type]hypervisor.VMStarter{hvType: starter} + var guests []*StoredMetadata + for i, id := range []string{"first", "second"} { + require.NoError(t, m.ensureDirectories(id)) + macos := *testMacImage().MacOS + macos.MachineIdentifier = []byte{byte(i + 1)} + stored := &StoredMetadata{Id: id, DataDir: p.InstanceDir(id), SocketPath: p.InstanceSocket(id, starter.SocketName()), HypervisorType: hvType, MacOS: &macos, MAC: macos.MAC, NetworkEnabled: true, Size: int64(macos.Memory), Vcpus: int(macos.CPUs)} + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: *stored})) + guests = append(guests, stored) + } + + // Both advisory checks succeed before either guest enters the boot lock. + for _, stored := range guests { + require.NoError(t, m.checkMacOSMACAvailable(context.Background(), stored)) + require.NoError(t, m.checkMacOSIdentityAvailable(context.Background(), stored)) + } + results := make(chan error, 2) + for _, stored := range guests { + go func() { results <- m.startAndBootVM(context.Background(), stored, nil, nil, false) }() + } + select { + case <-starter.started: + case <-time.After(5 * time.Second): + close(release) + t.Fatal("first boot did not start") + } + close(release) + var succeeded, refused int + for range guests { + select { + case err := <-results: + if err == nil { + succeeded++ + } else { + require.True(t, errors.Is(err, ErrInsufficientResources), "%v", err) + refused++ + } + case <-time.After(5 * time.Second): + t.Fatal("boot admission did not finish") + } + } + require.Equal(t, 1, succeeded) + require.Equal(t, 1, refused) + require.Empty(t, starter.started, "the conflicting guest must never reach StartVM") +} diff --git a/lib/instances/macos_identity.go b/lib/instances/macos_identity.go new file mode 100644 index 000000000..7e97f93cd --- /dev/null +++ b/lib/instances/macos_identity.go @@ -0,0 +1,246 @@ +package instances + +import ( + "context" + "errors" + "fmt" + "io" + "os" + "time" + + "github.com/kernel/hypeman/lib/guest" + "github.com/kernel/hypeman/lib/hypervisor" + "github.com/kernel/hypeman/lib/logger" + "github.com/kernel/hypeman/lib/network" +) + +// macOSIdentityRekeyCommand replaces the guest's SSH host keys. A clone shares its +// source's keys until this runs, so it runs on the clone's first start. +const macOSIdentityRekeyCommand = "rm -f /etc/ssh/ssh_host_* && ssh-keygen -A" + +const ( + macOSRekeyDeadline = 3 * time.Minute + macOSRekeyStderrLimit = 4 << 10 +) + +// boundedBuffer keeps at most limit bytes and reports every write as accepted, so a guest +// cannot grow host memory or stall the exec stream by writing more. +type boundedBuffer struct { + buf []byte + limit int +} + +func (b *boundedBuffer) Write(p []byte) (int, error) { + if room := b.limit - len(b.buf); room > 0 { + if room > len(p) { + room = len(p) + } + b.buf = append(b.buf, p[:room]...) + } + return len(p), nil +} + +func (b *boundedBuffer) String() string { return string(b.buf) } + +// prepareMacOSClone gives a stopped macOS clone its own identity: a fresh Apple machine +// identifier and a pending SSH host key rekey. Its MAC is assigned under the fork +// admission lock. The macOS struct is copied so the clone never aliases its source. +func prepareMacOSClone(clone *StoredMetadata) error { + if clone.MacOS == nil { + return nil + } + if !clone.MacOS.GuestAgent || clone.SkipGuestAgent { + return fmt.Errorf("%w: macOS clone needs the guest agent to rekey its identity", ErrInvalidRequest) + } + macos := *clone.MacOS + macos.HardwareModel = append([]byte(nil), clone.MacOS.HardwareModel...) + id, err := newMacMachineIdentifierData() + if err != nil { + return fmt.Errorf("generate macOS machine identifier: %w", err) + } + macos.MachineIdentifier = id + clone.IP = "" + clone.MacOS = &macos + clone.IdentityRekeyPending = true + return nil +} + +// forEachOtherInstance calls fn with the metadata of every stored instance except +// exclude, stopping at fn's first error. Directories without metadata are skipped; +// any other load error is wrapped with loadErrorContext when it is set. +func (m *manager) forEachOtherInstance(exclude, loadErrorContext string, fn func(*metadata) error) error { + entries, err := os.ReadDir(m.paths.GuestsDir()) + if err != nil { + return err + } + for _, entry := range entries { + if !entry.IsDir() || entry.Name() == exclude { + continue + } + meta, err := m.loadMetadata(entry.Name()) + if errors.Is(err, ErrNotFound) { + // A create in progress or a leftover directory has no instance to admit against. + continue + } + if err != nil { + if loadErrorContext != "" { + return fmt.Errorf("%s: %w", loadErrorContext, err) + } + return err + } + if err := fn(meta); err != nil { + return err + } + } + return nil +} + +// storedMACs returns every MAC recorded by a stored instance, except exclude. +func (m *manager) storedMACs(exclude string) ([]string, error) { + var macs []string + err := m.forEachOtherInstance(exclude, "", func(meta *metadata) error { + if meta.MAC != "" { + macs = append(macs, meta.MAC) + } + return nil + }) + if err != nil { + return nil, err + } + return macs, nil +} + +// checkMacOSMACAvailable refuses a macOS start whose MAC a live instance already holds. +// Two live guests on one MAC share one vmnet DHCP lease, so the API would report one +// address for both. +func (m *manager) checkMacOSMACAvailable(ctx context.Context, stored *StoredMetadata) error { + mac := macOSBootMAC(stored) + if mac == "" { + return nil + } + return m.forEachOtherInstance(stored.Id, "check macOS MAC availability", func(meta *metadata) error { + other := meta.MAC + if meta.MacOS != nil { + other = macOSBootMAC(&meta.StoredMetadata) + } + if !meta.NetworkEnabled || !sameMACAddress(other, mac) { + return nil + } + state := m.deriveStateWithoutHydration(ctx, &meta.StoredMetadata).State + if state.RequiresVMM() || state == StateUnknown { + return fmt.Errorf("%w: MAC %s is already in use by instance %s; stop it first", ErrInsufficientResources, mac, meta.Id) + } + return nil + }) +} + +// completeMacOSIdentityRekey runs on a clone's first start after the guest agent answers. +// It clears the pending flag only after the rekey succeeds and is saved, so a failure +// leaves the flag set and the next start retries. +func (m *manager) completeMacOSIdentityRekey(ctx context.Context, stored *StoredMetadata) error { + log := logger.FromContext(ctx) + dialer, err := hypervisor.NewVsockDialer(stored.HypervisorType, stored.VsockSocket, stored.VsockCID) + if err != nil { + return fmt.Errorf("create vsock dialer for identity rekey: %w", err) + } + // The rekey runs from the clone's own disk, which the source owner controls. Its output is + // untrusted: stdout is discarded and stderr is kept only up to a small cap. The deadline + // bounds the guest's work even if it stalls or streams without end. + var stderr boundedBuffer + stderr.limit = macOSRekeyStderrLimit + rekeyCtx, cancel := context.WithTimeout(ctx, macOSRekeyDeadline) + defer cancel() + exit, err := guest.ExecIntoInstance(rekeyCtx, dialer, guest.ExecOptions{ + Command: []string{"sh", "-c", macOSIdentityRekeyCommand}, + Stdout: io.Discard, + Stderr: &stderr, + Timeout: int32(macOSRekeyDeadline / time.Second), + WaitForAgent: 120 * time.Second, + }) + if err != nil { + return fmt.Errorf("exec macOS identity rekey: %w", err) + } + if exit.Code != 0 { + return fmt.Errorf("macOS identity rekey failed (exit=%d, stderr=%q)", exit.Code, stderr.String()) + } + stored.IdentityRekeyPending = false + if err := m.saveMetadata(&metadata{StoredMetadata: *stored}); err != nil { + return fmt.Errorf("save macOS identity rekey: %w", err) + } + log.InfoContext(ctx, "macOS clone identity rekeyed", "instance_id", stored.Id) + return nil +} + +// rekeyMacOSCloneOnStart rekeys a macOS clone's guest SSH host keys on its first +// start. A failed rekey stops the VM, and the pending flag stays set so the next +// start retries. +func (m *manager) rekeyMacOSCloneOnStart(ctx context.Context, stored *StoredMetadata) error { + if stored.MacOS == nil || !stored.IdentityRekeyPending { + return nil + } + err := m.completeMacOSIdentityRekey(ctx, stored) + if err == nil { + return nil + } + logger.FromContext(ctx).ErrorContext(ctx, "macOS identity rekey failed; stopping clone", "instance_id", stored.Id, "error", err) + // Teardown runs even if saving the pending flag fails. Otherwise the clone would keep + // running, networked, with its source's SSH host keys. + saveErr := m.saveMetadata(&metadata{StoredMetadata: *stored}) + _, stopErr := m.stopInstance(context.WithoutCancel(ctx), stored.Id) + if saveErr != nil || stopErr != nil { + return fmt.Errorf("macOS clone rekey failed and teardown needs attention: %w", errors.Join(err, saveErr, stopErr)) + } + return err +} + +// checkMacOSBootAvailable checks machine identity and MAC under macOSBootMu. +// A stopped or standby guest does not reserve either against another boot. +func (m *manager) checkMacOSBootAvailable(ctx context.Context, stored *StoredMetadata) error { + if err := m.checkMacOSIdentityAvailable(ctx, stored); err != nil { + return err + } + if !stored.NetworkEnabled { + return nil + } + return m.checkMacOSMACAvailable(ctx, stored) +} + +// clearMacOSStopReceipt persists the end of the last graceful stop before the guest +// boots and can write to its disk. Otherwise a start that fails after boot, or a host +// crash, would leave a receipt that admits a stopped snapshot of a changed disk. +func (m *manager) clearMacOSStopReceipt(stored *StoredMetadata) error { + stored.StopGraceful = false + meta, err := m.loadMetadata(stored.Id) + if err != nil { + return err + } + if !meta.StopGraceful { + return nil + } + meta.StopGraceful = false + if err := m.saveMetadata(meta); err != nil { + return fmt.Errorf("clear macOS stop receipt: %w", err) + } + return nil +} + +// assignMacOSCloneMAC gives a networked clone a MAC no stored instance uses. It runs +// under the fork admission lock, so concurrent forks cannot pick the same MAC. +func (m *manager) assignMacOSCloneMAC(clone *StoredMetadata) error { + if clone.MacOS == nil || !clone.NetworkEnabled { + return nil + } + used, err := m.storedMACs(clone.Id) + if err != nil { + return fmt.Errorf("list MACs for macOS clone: %w", err) + } + mac, err := network.NewUniqueMAC(used) + if err != nil { + return fmt.Errorf("allocate macOS clone MAC: %w", err) + } + macos := *clone.MacOS + macos.MAC = mac + clone.MacOS = &macos + clone.MAC = mac + return nil +} diff --git a/lib/instances/macos_identity_buffer_test.go b/lib/instances/macos_identity_buffer_test.go new file mode 100644 index 000000000..b06869930 --- /dev/null +++ b/lib/instances/macos_identity_buffer_test.go @@ -0,0 +1,20 @@ +package instances + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestBoundedBufferCapsAndAcceptsEverything(t *testing.T) { + b := boundedBuffer{limit: 8} + n, err := b.Write([]byte("0123456789")) + require.NoError(t, err) + require.Equal(t, 10, n, "writes report full acceptance so the guest stream is never stalled") + for i := 0; i < 1000; i++ { + _, _ = b.Write([]byte(strings.Repeat("x", 1024))) + } + require.Equal(t, "01234567", b.String()) + require.LessOrEqual(t, len(b.buf), 8) +} diff --git a/lib/instances/macos_identity_darwin.go b/lib/instances/macos_identity_darwin.go new file mode 100644 index 000000000..e8cfa7d28 --- /dev/null +++ b/lib/instances/macos_identity_darwin.go @@ -0,0 +1,16 @@ +//go:build darwin && arm64 + +package instances + +import ( + vz "github.com/Code-Hex/vz/v3" +) + +// newMacMachineIdentifierData returns a fresh Apple machine identifier for a cloned guest. +func newMacMachineIdentifierData() ([]byte, error) { + id, err := vz.NewMacMachineIdentifier() + if err != nil { + return nil, err + } + return id.DataRepresentation(), nil +} diff --git a/lib/instances/macos_identity_other.go b/lib/instances/macos_identity_other.go new file mode 100644 index 000000000..2f17f3a50 --- /dev/null +++ b/lib/instances/macos_identity_other.go @@ -0,0 +1,9 @@ +//go:build !(darwin && arm64) + +package instances + +import "errors" + +func newMacMachineIdentifierData() ([]byte, error) { + return nil, errors.New("macOS machine identifiers require darwin/arm64") +} diff --git a/lib/instances/macos_identity_test.go b/lib/instances/macos_identity_test.go new file mode 100644 index 000000000..d5e950794 --- /dev/null +++ b/lib/instances/macos_identity_test.go @@ -0,0 +1,71 @@ +//go:build darwin && arm64 + +package instances + +import ( + "net" + "testing" + + "github.com/kernel/hypeman/lib/images" + "github.com/kernel/hypeman/lib/network" + "github.com/stretchr/testify/require" +) + +func testMacOSSource() *StoredMetadata { + return &StoredMetadata{ + Id: "source", + NetworkEnabled: true, + MAC: "a6:fd:95:db:a2:62", + MacOS: &images.MacOSImage{ + HardwareModel: []byte("model"), + MachineIdentifier: []byte("source-identifier"), + MAC: "a6:fd:95:db:a2:62", + GuestAgent: true, + }, + } +} + +func TestPrepareMacOSCloneRebindsIdentityWithoutAliasingSource(t *testing.T) { + source := testMacOSSource() + clone := cloneStoredMetadata(*source) + + require.NoError(t, prepareMacOSClone(&clone)) + + require.True(t, clone.IdentityRekeyPending) + require.NotEqual(t, []byte("source-identifier"), clone.MacOS.MachineIdentifier) + require.NotEmpty(t, clone.MacOS.MachineIdentifier) + require.Empty(t, clone.IP) + + // The source keeps its own identity; the clone's struct is independent. + require.Equal(t, []byte("source-identifier"), source.MacOS.MachineIdentifier) + require.Equal(t, "a6:fd:95:db:a2:62", source.MacOS.MAC) + require.False(t, source.IdentityRekeyPending) +} + +func TestPrepareMacOSCloneRefusesGuestWithoutAgent(t *testing.T) { + clone := testMacOSSource() + clone.MacOS.GuestAgent = false + err := prepareMacOSClone(clone) + require.ErrorIs(t, err, ErrInvalidRequest) + require.False(t, clone.IdentityRekeyPending) +} + +func TestPrepareMacOSCloneRefusesSkippedAgent(t *testing.T) { + clone := testMacOSSource() + clone.SkipGuestAgent = true + require.ErrorIs(t, prepareMacOSClone(clone), ErrInvalidRequest) + require.False(t, clone.IdentityRekeyPending) +} + +func TestNewUniqueMACAvoidsUsedAddresses(t *testing.T) { + used := []string{"02:00:00:00:00:01", "02:00:00:00:00:02"} + for i := 0; i < 64; i++ { + mac, err := network.NewUniqueMAC(used) + require.NoError(t, err) + require.NotContains(t, used, mac) + hw, err := net.ParseMAC(mac) + require.NoError(t, err) + require.NotZero(t, hw[0]&0x02, "locally administered bit set") + require.Zero(t, hw[0]&0x01, "unicast") + } +} diff --git a/lib/instances/macos_memory_snapshot_test.go b/lib/instances/macos_memory_snapshot_test.go new file mode 100644 index 000000000..4669b7181 --- /dev/null +++ b/lib/instances/macos_memory_snapshot_test.go @@ -0,0 +1,425 @@ +package instances + +import ( + "context" + "encoding/base64" + "encoding/json" + "os" + "path/filepath" + "testing" + "time" + + "github.com/kernel/hypeman/lib/hypervisor" + "github.com/kernel/hypeman/lib/hypervisor/vz/shimconfig" + "github.com/kernel/hypeman/lib/instances/phasetracking" + "github.com/kernel/hypeman/lib/paths" + restartpolicy "github.com/kernel/hypeman/lib/restart-policy" + snapshotstore "github.com/kernel/hypeman/lib/snapshot" + "github.com/stretchr/testify/require" +) + +const macOSMemoryTestHypervisor hypervisor.Type = "macos-memory-snapshot-test" + +func macOSMemorySnapshotFixture(t *testing.T) (*manager, *metadata) { + t.Helper() + hypervisor.RegisterCapabilities(macOSMemoryTestHypervisor, hypervisor.Capabilities{SupportsMacOSBoot: true, SupportsSnapshot: true}) + m := &manager{paths: paths.New(t.TempDir()), macOSOnly: true, now: time.Now, lifecycleEvents: newLifecycleSubscribers(), vmStarters: map[hypervisor.Type]hypervisor.VMStarter{macOSMemoryTestHypervisor: concurrentForkPrepareTestStarter{}}} + m.deleteSnapshotFn = m.deleteSnapshot + const id = "mac-memory-source" + createStoppedSnapshotSourceFixture(t, m, id, id, macOSMemoryTestHypervisor) + meta, err := m.loadMetadata(id) + require.NoError(t, err) + meta.MacOS = macOSGuestMetadata(2).MacOS + meta.Size, meta.Vcpus = int64(meta.MacOS.Memory), int(meta.MacOS.CPUs) + meta.MAC, meta.NetworkEnabled = meta.MacOS.MAC, true + meta.SkipGuestAgent = true + meta.StopGraceful = true + require.NoError(t, m.saveMetadata(meta)) + require.NoError(t, os.Rename(m.paths.InstanceOverlay(id), m.paths.InstanceMacOSBootDisk(id))) + require.NoError(t, os.WriteFile(filepath.Join(meta.DataDir, macOSAuxStorageFile), []byte("aux-at-checkpoint"), 0600)) + return m, meta +} + +func writeMacOSMemorySnapshotPayload(t *testing.T, m *manager, guestDir string, stored *StoredMetadata) shimconfig.SnapshotManifest { + t.Helper() + snapshotDir := filepath.Join(guestDir, "snapshots", "snapshot-latest") + require.NoError(t, os.MkdirAll(snapshotDir, 0700)) + require.NoError(t, os.WriteFile(filepath.Join(snapshotDir, shimconfig.SnapshotMachineStateFile), []byte("saved-memory"), 0600)) + manifest := shimconfig.SnapshotManifest{Hypervisor: "vz", MachineStateFile: shimconfig.SnapshotMachineStateFile, ShimConfig: shimconfig.ShimConfig{ + VCPUs: stored.Vcpus, MemoryBytes: stored.Size, + MacHardwareModelData: base64.StdEncoding.EncodeToString(stored.MacOS.HardwareModel), MacMachineIdentifierData: base64.StdEncoding.EncodeToString(stored.MacOS.MachineIdentifier), + MacAuxStoragePath: filepath.Join(m.paths.InstanceDir(stored.Id), macOSAuxStorageFile), + Disks: []shimconfig.DiskConfig{{Path: m.paths.InstanceMacOSBootDisk(stored.Id)}}, + }} + if stored.NetworkEnabled { + manifest.ShimConfig.Networks = []shimconfig.NetworkConfig{{MAC: stored.MAC}} + } + writeMacOSSnapshotManifest(t, snapshotDir, manifest) + return manifest +} + +func writeMacOSSnapshotManifest(t *testing.T, snapshotDir string, manifest shimconfig.SnapshotManifest) { + t.Helper() + data, err := json.Marshal(manifest) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(snapshotDir, shimconfig.SnapshotManifestFile), data, 0600)) +} + +func TestMacOSMemorySnapshotCaptureRestoreAndIsolation(t *testing.T) { + m, meta := macOSMemorySnapshotFixture(t) + writeMacOSMemorySnapshotPayload(t, m, meta.DataDir, &meta.StoredMetadata) + meta.StopGraceful = false + bootTime := time.Now().Add(-2 * time.Hour) + checkpointTime := bootTime.Add(time.Hour) + meta.StartedAt = &bootTime + meta.Phases.Record(phasetracking.PhaseRunning, bootTime) + meta.Phases.Record(phasetracking.PhaseStandby, checkpointTime) + require.NoError(t, m.saveMetadata(meta)) + // Linux compression defaults must not start a codec job for VZ machine state. + m.snapshotDefaults.Compression = &snapshotstore.SnapshotCompressionConfig{Enabled: true, Algorithm: snapshotstore.SnapshotCompressionAlgorithmZstd} + ctx := context.Background() + snapshot, err := m.CreateSnapshot(ctx, meta.Id, CreateSnapshotRequest{Kind: SnapshotKindStandby}) + require.NoError(t, err) + require.Equal(t, SnapshotKindStandby, snapshot.Kind) + require.Equal(t, snapshotstore.SnapshotCompressionStateNone, snapshot.CompressionState) + source, err := m.GetInstance(ctx, meta.Id) + require.NoError(t, err) + require.Equal(t, StateStandby, source.State) + _, err = m.ForkSnapshot(ctx, snapshot.Id, ForkSnapshotRequest{Name: "unsafe-warm-fork"}) + require.ErrorIs(t, err, ErrNotSupported) + current, err := m.loadMetadata(meta.Id) + require.NoError(t, err) + current.Phases.Record(phasetracking.PhaseRunning, checkpointTime.Add(10*time.Minute)) + current.Phases.Record(phasetracking.PhaseStandby, checkpointTime.Add(20*time.Minute)) + expiresAt := time.Now().Add(3 * time.Hour) + current.ExpiresAt = &expiresAt + require.NoError(t, m.saveMetadata(current)) + + require.NoError(t, os.WriteFile(m.paths.InstanceMacOSBootDisk(meta.Id), []byte("after-checkpoint"), 0600)) + require.NoError(t, os.WriteFile(filepath.Join(meta.DataDir, macOSAuxStorageFile), []byte("aux-after"), 0600)) + restored, err := m.RestoreSnapshot(ctx, meta.Id, snapshot.Id, RestoreSnapshotRequest{TargetState: StateStandby}) + require.NoError(t, err) + require.Equal(t, StateStandby, restored.State) + require.Equal(t, meta.MacOS.MachineIdentifier, restored.MacOS.MachineIdentifier) + require.Equal(t, meta.MAC, restored.MAC) + require.NotNil(t, restored.StartedAt) + require.True(t, bootTime.Equal(*restored.StartedAt), "a memory restore resumes the checkpoint's boot") + require.Equal(t, current.Phases.Cumulative[phasetracking.PhaseRunning], restored.Phases.Cumulative[phasetracking.PhaseRunning], "same-instance restore must not rewind host phase accounting") + require.NotNil(t, restored.ExpiresAt) + require.True(t, expiresAt.Equal(*restored.ExpiresAt)) + for name, want := range map[string]string{"boot.raw": "overlay", macOSAuxStorageFile: "aux-at-checkpoint", "snapshots/snapshot-latest/" + shimconfig.SnapshotMachineStateFile: "saved-memory"} { + data, err := os.ReadFile(filepath.Join(meta.DataDir, name)) + require.NoError(t, err) + require.Equal(t, want, string(data)) + } + require.NoError(t, os.WriteFile(m.paths.InstanceMacOSBootDisk(meta.Id), []byte("restored-write"), 0600)) + data, err := os.ReadFile(filepath.Join(m.paths.SnapshotGuestDir(snapshot.Id), "boot.raw")) + require.NoError(t, err) + require.Equal(t, "overlay", string(data), "restoring must not alias the persistent snapshot") + + restored, err = m.RestoreSnapshot(ctx, meta.Id, snapshot.Id, RestoreSnapshotRequest{TargetState: StateStopped}) + require.NoError(t, err) + require.Equal(t, StateStopped, restored.State) + require.False(t, restored.StopGraceful, "discarding memory does not establish a clean shutdown receipt") + require.Equal(t, restartpolicy.BlockedReasonManualStop, restored.RestartStatus.BlockedReason, "an explicit stopped target must not be automatically restarted") + _, err = m.CreateSnapshot(ctx, meta.Id, CreateSnapshotRequest{Kind: SnapshotKindStopped}) + require.ErrorIs(t, err, ErrInvalidState) +} + +func TestMacOSMemorySnapshotRejectsInvalidPayloadBeforeStorageSwap(t *testing.T) { + for _, test := range []struct { + name string + mutate func(*shimconfig.SnapshotManifest) + }{ + {"cpu", func(m *shimconfig.SnapshotManifest) { m.ShimConfig.VCPUs++ }}, + {"ram", func(m *shimconfig.SnapshotManifest) { m.ShimConfig.MemoryBytes++ }}, + {"identity", func(m *shimconfig.SnapshotManifest) { m.ShimConfig.MacMachineIdentifierData = "different" }}, + {"hardware", func(m *shimconfig.SnapshotManifest) { m.ShimConfig.MacHardwareModelData = "different" }}, + {"disk", func(m *shimconfig.SnapshotManifest) { m.ShimConfig.Disks[0].Path = "/wrong-disk" }}, + {"aux", func(m *shimconfig.SnapshotManifest) { m.ShimConfig.MacAuxStoragePath = "/wrong-aux" }}, + {"network", func(m *shimconfig.SnapshotManifest) { m.ShimConfig.Networks[0].MAC = "02:00:00:00:00:02" }}, + {"state-path", func(m *shimconfig.SnapshotManifest) { m.MachineStateFile = "../../other-state" }}, + } { + t.Run(test.name, func(t *testing.T) { + m, meta := macOSMemorySnapshotFixture(t) + writeMacOSMemorySnapshotPayload(t, m, meta.DataDir, &meta.StoredMetadata) + snapshot, err := m.CreateSnapshot(context.Background(), meta.Id, CreateSnapshotRequest{Kind: SnapshotKindStandby}) + require.NoError(t, err) + snapshotDir := filepath.Join(m.paths.SnapshotGuestDir(snapshot.Id), "snapshots", "snapshot-latest") + rec, err := m.loadSnapshotRecord(snapshot.Id) + require.NoError(t, err) + manifest := writeMacOSMemorySnapshotPayload(t, m, m.paths.SnapshotGuestDir(snapshot.Id), &rec.StoredMetadata) + test.mutate(&manifest) + writeMacOSSnapshotManifest(t, snapshotDir, manifest) + require.NoError(t, os.WriteFile(m.paths.InstanceMacOSBootDisk(meta.Id), []byte("current-disk"), 0600)) + _, err = m.RestoreSnapshot(context.Background(), meta.Id, snapshot.Id, RestoreSnapshotRequest{}) + require.ErrorIs(t, err, ErrInvalidRequest) + data, err := os.ReadFile(m.paths.InstanceMacOSBootDisk(meta.Id)) + require.NoError(t, err) + require.Equal(t, "current-disk", string(data)) + }) + } +} + +type macOSSnapshotProbeTestHypervisor struct { + lifecycleNoopHypervisor + supportErr error + probeCalls int + pauseCalls int + resumeCalls int +} + +func (h *macOSSnapshotProbeTestHypervisor) ValidateSnapshotSupport(context.Context) error { + h.probeCalls++ + return h.supportErr +} + +func (h *macOSSnapshotProbeTestHypervisor) Pause(context.Context) error { + h.pauseCalls++ + return nil +} + +func (h *macOSSnapshotProbeTestHypervisor) Capabilities() hypervisor.Capabilities { + return hypervisor.Capabilities{SupportsSnapshot: true} +} + +func (h *macOSSnapshotProbeTestHypervisor) Resume(ctx context.Context) error { + h.resumeCalls++ + return ctx.Err() +} + +func TestMacOSSnapshotConfigurationProbeAndPauseRecovery(t *testing.T) { + stored := macOSGuestMetadata(2) + hv := &macOSSnapshotProbeTestHypervisor{supportErr: hypervisor.ErrNotSupported} + require.ErrorIs(t, validateMacOSSaveRestoreSupport(context.Background(), stored, hv), ErrNotSupported) + require.Equal(t, 1, hv.probeCalls) + require.NoError(t, validateMacOSSaveRestoreSupport(context.Background(), &StoredMetadata{}, hv)) + require.Equal(t, 1, hv.probeCalls, "Linux must not call the macOS configuration probe") + require.ErrorIs(t, validateMacOSSaveRestoreSupport(context.Background(), stored, lifecycleNoopHypervisor{}), ErrNotSupported) + hv.supportErr = nil + require.NoError(t, validateMacOSSaveRestoreSupport(context.Background(), stored, hv)) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + require.NoError(t, resumeSnapshotAfterError(ctx, stored, hv)) + require.Equal(t, 1, hv.resumeCalls) +} + +func TestMacOSLiveConfigurationRejectedBeforePauseAndSchedulePersistence(t *testing.T) { + m, meta := macOSMemorySnapshotFixture(t) + const hvType hypervisor.Type = "macos-live-snapshot-probe-test" + hv := &macOSSnapshotProbeTestHypervisor{lifecycleNoopHypervisor: lifecycleNoopHypervisor{state: hypervisor.StateRunning}, supportErr: hypervisor.ErrNotSupported} + hypervisor.RegisterCapabilities(hvType, hypervisor.Capabilities{SupportsMacOSBoot: true, SupportsSnapshot: true}) + hypervisor.RegisterClientFactory(hvType, func(string) (hypervisor.Hypervisor, error) { return hv, nil }) + meta.HypervisorType = hvType + require.NoError(t, m.saveMetadata(meta)) + require.NoError(t, os.WriteFile(meta.SocketPath, nil, 0600)) + ctx := context.Background() + _, err := m.SetSnapshotSchedule(ctx, meta.Id, SetSnapshotScheduleRequest{Interval: time.Hour, NamePrefix: "unsupported", Retention: SnapshotScheduleRetention{MaxCount: 1}}) + require.ErrorIs(t, err, ErrNotSupported) + _, err = m.GetSnapshotSchedule(ctx, meta.Id) + require.ErrorIs(t, err, ErrSnapshotScheduleNotFound) + _, err = m.CreateSnapshot(ctx, meta.Id, CreateSnapshotRequest{Kind: SnapshotKindStandby}) + require.ErrorIs(t, err, ErrNotSupported) + require.Equal(t, 2, hv.probeCalls, "configuration validation must pass through the tracing wrapper") + require.Equal(t, 0, hv.pauseCalls, "unsupported configurations are rejected before pausing") + inst, err := m.GetInstance(ctx, meta.Id) + require.NoError(t, err) + require.Equal(t, StateRunning, inst.State) +} + +type macOSCancelledSnapshotTestHypervisor struct { + macOSSnapshotProbeTestHypervisor + cancel context.CancelFunc + failStage string + writePayload func() +} + +func (h *macOSCancelledSnapshotTestHypervisor) Snapshot(ctx context.Context, _ string) error { + if h.failStage == "snapshot" { + h.cancel() + return ctx.Err() + } + h.writePayload() + return nil +} + +func (h *macOSCancelledSnapshotTestHypervisor) Capabilities() hypervisor.Capabilities { + return hypervisor.Capabilities{SupportsSnapshot: true, SupportsGracefulVMMShutdown: true} +} + +func (h *macOSCancelledSnapshotTestHypervisor) Shutdown(ctx context.Context) error { + h.cancel() + return ctx.Err() +} + +func TestMacOSSnapshotCancellationRecoversSource(t *testing.T) { + for _, stage := range []string{"snapshot", "shutdown"} { + t.Run(stage, func(t *testing.T) { + m, meta := macOSMemorySnapshotFixture(t) + hvType := hypervisor.Type("macos-cancelled-" + stage + "-test") + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + hv := &macOSCancelledSnapshotTestHypervisor{macOSSnapshotProbeTestHypervisor: macOSSnapshotProbeTestHypervisor{lifecycleNoopHypervisor: lifecycleNoopHypervisor{state: hypervisor.StateRunning}}, cancel: cancel, failStage: stage} + hypervisor.RegisterCapabilities(hvType, hypervisor.Capabilities{SupportsMacOSBoot: true, SupportsSnapshot: true}) + hypervisor.RegisterClientFactory(hvType, func(string) (hypervisor.Hypervisor, error) { return hv, nil }) + meta.HypervisorType = hvType + hv.writePayload = func() { writeMacOSMemorySnapshotPayload(t, m, meta.DataDir, &meta.StoredMetadata) } + m.vmStarters[hvType] = macOSSnapshotRestoreContextStarter{} + require.NoError(t, m.saveMetadata(meta)) + require.NoError(t, os.WriteFile(meta.SocketPath, nil, 0600)) + _, err := m.CreateSnapshot(ctx, meta.Id, CreateSnapshotRequest{Kind: SnapshotKindStandby}) + if stage == "snapshot" { + require.ErrorIs(t, err, context.Canceled) + require.Equal(t, 1, hv.resumeCalls) + } else { + require.NoError(t, err, "confirmed shutdown must recover the source despite cancellation") + } + require.Equal(t, 1, hv.pauseCalls) + inst, err := m.GetInstance(context.Background(), meta.Id) + require.NoError(t, err) + require.Equal(t, StateRunning, inst.State) + }) + } +} + +type macOSSnapshotRestoreContextStarter struct { + concurrentForkPrepareTestStarter +} + +func (s macOSSnapshotRestoreContextStarter) RestoreVM(ctx context.Context, p *paths.Paths, version, socket, snapshot string, options hypervisor.RestoreOptions) (int, hypervisor.Hypervisor, error) { + if err := ctx.Err(); err != nil { + return 0, nil, err + } + return s.concurrentForkPrepareTestStarter.RestoreVM(ctx, p, version, socket, snapshot, options) +} + +func TestMacOSSnapshotRestoresSourceAfterRequestCancellation(t *testing.T) { + m, meta := macOSMemorySnapshotFixture(t) + writeMacOSMemorySnapshotPayload(t, m, meta.DataDir, &meta.StoredMetadata) + m.vmStarters[macOSMemoryTestHypervisor] = macOSSnapshotRestoreContextStarter{} + ctx, cancel := context.WithCancel(context.Background()) + cancel() + require.NoError(t, m.restoreSnapshotSource(ctx, &meta.StoredMetadata)) + inst, err := m.GetInstance(context.Background(), meta.Id) + require.NoError(t, err) + require.Equal(t, StateRunning, inst.State) +} + +func TestMacOSMemorySnapshotRequiresPayloadAndRejectsCompression(t *testing.T) { + m, meta := macOSMemorySnapshotFixture(t) + _, err := m.CreateSnapshot(context.Background(), meta.Id, CreateSnapshotRequest{Kind: SnapshotKindStandby, Compression: &snapshotstore.SnapshotCompressionConfig{Enabled: true}}) + require.ErrorIs(t, err, ErrInvalidRequest) + require.NoError(t, os.MkdirAll(m.paths.InstanceSnapshotLatest(meta.Id), 0700)) + require.NoError(t, os.WriteFile(filepath.Join(m.paths.InstanceSnapshotLatest(meta.Id), "incomplete"), []byte("bad-state"), 0600)) + _, err = m.CreateSnapshot(context.Background(), meta.Id, CreateSnapshotRequest{Kind: SnapshotKindStandby}) + require.ErrorIs(t, err, ErrInvalidRequest) +} + +func TestMacOSSnapshotRestoreRefusesLiveStates(t *testing.T) { + for _, state := range []State{StateCreated, StateInitializing, StateRunning, StatePaused, StateShutdown} { + require.ErrorIs(t, validateMacOSSnapshotRestoreState(&Instance{StoredMetadata: *macOSGuestMetadata(2), State: state}), ErrInvalidState) + } + for _, state := range []State{StateStopped, StateStandby} { + require.NoError(t, validateMacOSSnapshotRestoreState(&Instance{StoredMetadata: *macOSGuestMetadata(2), State: state})) + } + require.NoError(t, validateMacOSSnapshotRestoreState(&Instance{State: StatePaused}), "Linux admission is unchanged") +} + +func TestMacOSSnapshotSchedulesCaptureStoppedAndStandbyAndRetain(t *testing.T) { + m, meta := macOSMemorySnapshotFixture(t) + clock := time.Now().UTC() + m.now = func() time.Time { return clock } + ctx := context.Background() + _, err := m.SetSnapshotSchedule(ctx, meta.Id, SetSnapshotScheduleRequest{Interval: time.Hour, NamePrefix: "mac-backup", Retention: SnapshotScheduleRetention{MaxCount: 1}}) + require.NoError(t, err) + markSnapshotScheduleDue(t, m, meta.Id) + require.NoError(t, m.RunSnapshotSchedules(ctx)) + first, err := m.GetSnapshotSchedule(ctx, meta.Id) + require.NoError(t, err) + require.NotNil(t, first.LastSnapshotID) + stopped, err := m.GetSnapshot(ctx, *first.LastSnapshotID) + require.NoError(t, err) + require.Equal(t, SnapshotKindStopped, stopped.Kind) + + writeMacOSMemorySnapshotPayload(t, m, meta.DataDir, &meta.StoredMetadata) + meta.StopGraceful = false + require.NoError(t, m.saveMetadata(meta)) + clock = clock.Add(time.Hour) + markSnapshotScheduleDue(t, m, meta.Id) + require.NoError(t, m.RunSnapshotSchedules(ctx)) + second, err := m.GetSnapshotSchedule(ctx, meta.Id) + require.NoError(t, err) + require.NotNil(t, second.LastSnapshotID) + require.Nil(t, second.LastError) + memory, err := m.GetSnapshot(ctx, *second.LastSnapshotID) + require.NoError(t, err) + require.Equal(t, SnapshotKindStandby, memory.Kind) + _, err = m.GetSnapshot(ctx, stopped.Id) + require.ErrorIs(t, err, ErrSnapshotNotFound) + _, err = m.GetSnapshot(ctx, memory.Id) + require.NoError(t, err) + require.NoError(t, m.DeleteSnapshotSchedule(ctx, meta.Id)) +} + +func TestMacOSSnapshotScheduleRecordsFailureAndAdvancesInterval(t *testing.T) { + m, meta := macOSMemorySnapshotFixture(t) + meta.StopGraceful = false + require.NoError(t, m.saveMetadata(meta)) + ctx := context.Background() + _, err := m.SetSnapshotSchedule(ctx, meta.Id, SetSnapshotScheduleRequest{Interval: time.Hour, NamePrefix: "mac-backup", Retention: SnapshotScheduleRetention{MaxCount: 1}}) + require.NoError(t, err) + markSnapshotScheduleDue(t, m, meta.Id) + require.ErrorIs(t, m.RunSnapshotSchedules(ctx), ErrInvalidState) + schedule, err := m.GetSnapshotSchedule(ctx, meta.Id) + require.NoError(t, err) + require.NotNil(t, schedule.LastError) + require.Nil(t, schedule.LastSnapshotID) + require.True(t, schedule.NextRunAt.After(m.now().UTC())) +} + +// macOS boots its own disk and aux copies, so restore must not need the template image. +func TestMacOSRestoreDoesNotNeedTemplateImage(t *testing.T) { + m, meta := macOSMemorySnapshotFixture(t) + writeMacOSMemorySnapshotPayload(t, m, meta.DataDir, &meta.StoredMetadata) + m.imageManager = readyFixtureImageManager{name: "deleted"} + require.NoError(t, m.restoreSnapshotSource(context.Background(), &meta.StoredMetadata)) + inst, err := m.GetInstance(context.Background(), meta.Id) + require.NoError(t, err) + require.Equal(t, StateRunning, inst.State) + require.NoDirExists(t, m.paths.InstanceSnapshotLatest(meta.Id)) +} + +// A resume that failed before removing its checkpoint leaves a running guest with +// one on disk. A retried restore must remove it rather than return early. +func TestMacOSRestoreRetryRemovesConsumedCheckpoint(t *testing.T) { + m, meta := macOSMemorySnapshotFixture(t) + writeMacOSMemorySnapshotPayload(t, m, meta.DataDir, &meta.StoredMetadata) + require.NoError(t, os.WriteFile(meta.SocketPath, nil, 0600)) + m.storeCachedHypervisorState(meta.Id, hypervisor.StateRunning) + + inst, err := m.RestoreInstance(context.Background(), meta.Id) + require.NoError(t, err) + require.Equal(t, StateRunning, inst.State) + require.False(t, inst.HasSnapshot) + require.NoDirExists(t, m.paths.InstanceSnapshotLatest(meta.Id)) +} + +func TestMacOSRestoreFailsWhenCheckpointCannotBeRemoved(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root ignores directory permissions") + } + m, meta := macOSMemorySnapshotFixture(t) + writeMacOSMemorySnapshotPayload(t, m, meta.DataDir, &meta.StoredMetadata) + snapshots := filepath.Dir(m.paths.InstanceSnapshotLatest(meta.Id)) + require.NoError(t, os.Chmod(snapshots, 0500)) + t.Cleanup(func() { os.Chmod(snapshots, 0700) }) + + _, err := m.restoreInstance(context.Background(), meta.Id) + require.ErrorContains(t, err, "remove consumed snapshot") + + require.NoError(t, os.Chmod(snapshots, 0700)) + m.storeCachedHypervisorState(meta.Id, hypervisor.StateRunning) + inst, err := m.RestoreInstance(context.Background(), meta.Id) + require.NoError(t, err) + require.False(t, inst.HasSnapshot, "the retry removes the consumed checkpoint") +} diff --git a/lib/instances/macos_shim_gone_test.go b/lib/instances/macos_shim_gone_test.go index 0a2e08d92..44f2192ac 100644 --- a/lib/instances/macos_shim_gone_test.go +++ b/lib/instances/macos_shim_gone_test.go @@ -10,9 +10,11 @@ import ( "runtime" "syscall" "testing" + "time" "github.com/kernel/hypeman/lib/hypervisor" "github.com/kernel/hypeman/lib/images" + "github.com/kernel/hypeman/lib/instances/phasetracking" "github.com/kernel/hypeman/lib/paths" "github.com/kernel/hypeman/lib/resources" "github.com/stretchr/testify/require" @@ -134,3 +136,25 @@ func TestMacOSPreListenerShimIsNotStopped(t *testing.T) { require.False(t, ProcessExists(shim.Process.Pid), "the shim is killed") require.Equal(t, StateStopped, m.deriveStateWithoutHydration(context.Background(), stored).State) } + +func TestFinishMacOSResumeRemovesConsumedSnapshot(t *testing.T) { + p := paths.New(t.TempDir()) + m := &manager{paths: p} + id := "resumed" + require.NoError(t, m.ensureDirectories(id)) + image := &images.MacOSImage{GuestAgent: true} + ready := time.Now().UTC().Add(-time.Minute) + meta := &metadata{StoredMetadata: StoredMetadata{Id: id, DataDir: p.InstanceDir(id), MacOS: image, GuestAgentReadyAt: &ready}} + meta.Phases.Record(phasetracking.PhaseStandby, ready) + require.NoError(t, m.saveMetadata(meta)) + snapshot := p.InstanceSnapshotLatest(id) + require.NoError(t, os.MkdirAll(snapshot, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(snapshot, "state"), []byte("x"), 0644)) + + require.NoError(t, m.finishMacOSResume(id)) + + require.NoDirExists(t, snapshot, "a consumed memory snapshot must not survive the resume") + saved, err := m.loadMetadata(id) + require.NoError(t, err) + require.Equal(t, phasetracking.PhaseRunning, saved.Phases.Current) +} diff --git a/lib/instances/macos_snapshot_plan_test.go b/lib/instances/macos_snapshot_plan_test.go new file mode 100644 index 000000000..a181fce74 --- /dev/null +++ b/lib/instances/macos_snapshot_plan_test.go @@ -0,0 +1,55 @@ +package instances + +import ( + "context" + "os" + "testing" + + "github.com/kernel/hypeman/lib/hypervisor" + "github.com/kernel/hypeman/lib/paths" + "github.com/stretchr/testify/require" +) + +func TestMacOSSnapshotTargetUsesMacOSBootConfig(t *testing.T) { + p := paths.New(t.TempDir()) + m := &manager{paths: p} + meta := *macOSGuestMetadata(2) + meta.Id, meta.DataDir = "mac-snapshot", p.InstanceDir("mac-snapshot") + meta.Size, meta.Vcpus = 4<<30, 3 + for _, kind := range []SnapshotKind{SnapshotKindStopped, SnapshotKindStandby} { + config := m.plannedStoredVMConfig(kind, meta) + require.Equal(t, hypervisor.BootModeMacOS, config.BootMode) + require.Equal(t, meta.Size, config.MemoryBytes) + require.Equal(t, meta.Vcpus, config.VCPUs) + require.Len(t, config.Disks, 1) + require.Empty(t, config.KernelPath) + require.False(t, config.GuestMemory.EnableBalloon) + require.NoError(t, hypervisor.ValidateBootConfig(config)) + require.Error(t, hypervisor.ValidateDirectRawConfig("linux-backend", config)) + } +} + +func TestMacOSSnapshotForkRejectsLinuxBackendBeforeCopy(t *testing.T) { + p := paths.New(t.TempDir()) + m := &manager{paths: p, macOSOnly: true, vmStarters: map[hypervisor.Type]hypervisor.VMStarter{hypervisor.TypeFirecracker: macOSSnapshotLinuxStarter{}}} + require.NoError(t, os.MkdirAll(p.GuestsDir(), 0700)) + source := *macOSGuestMetadata(2) + source.Id = "mac-source" + source.HypervisorType = hypervisor.TypeVZ + source.StopGraceful = true + source.MacOS.GuestAgent = true + require.NoError(t, m.saveSnapshotRecord(&snapshotRecord{Snapshot: Snapshot{Id: "mac-snapshot", Kind: SnapshotKindStopped, SourceInstanceID: source.Id, SourceHypervisor: hypervisor.TypeVZ}, StoredMetadata: source})) + _, err := m.ForkSnapshot(context.Background(), "mac-snapshot", ForkSnapshotRequest{Name: "linux-target", TargetHypervisor: hypervisor.TypeFirecracker}) + require.ErrorIs(t, err, ErrInvalidRequest) + entries, err := os.ReadDir(p.GuestsDir()) + require.NoError(t, err) + require.Empty(t, entries, "backend admission rejects the fork before copying files") +} + +type macOSSnapshotLinuxStarter struct { + hypervisor.VMStarter +} + +func (macOSSnapshotLinuxStarter) ValidateConfig(config hypervisor.VMConfig) error { + return hypervisor.ValidateDirectRawConfig("linux-test", config) +} diff --git a/lib/instances/macos_test.go b/lib/instances/macos_test.go index 5c3479871..8d5f97531 100644 --- a/lib/instances/macos_test.go +++ b/lib/instances/macos_test.go @@ -2,6 +2,7 @@ package instances import ( "context" + "encoding/json" "errors" "os" "path/filepath" @@ -174,13 +175,71 @@ func TestMacOSMACAdmission(t *testing.T) { m.storeCachedHypervisorState("active", hypervisor.StateRunning) other := StoredMetadata{Id: "other", DataDir: p.InstanceDir("other"), NetworkEnabled: true, MacOS: &images.MacOSImage{HardwareModel: []byte{1}, MachineIdentifier: []byte{3}, MAC: "02-00-00-00-00-01"}} - require.ErrorIs(t, m.checkMacOSIdentityAvailable(context.Background(), &other), ErrInsufficientResources, "a different identity on the same MAC is refused") + require.ErrorIs(t, m.checkMacOSBootAvailable(context.Background(), &other), ErrInsufficientResources, "a different identity on the same MAC is refused") other.NetworkEnabled = false - require.NoError(t, m.checkMacOSIdentityAvailable(context.Background(), &other), "a guest without a NIC holds no MAC") + require.NoError(t, m.checkMacOSBootAvailable(context.Background(), &other), "a guest without a NIC holds no MAC") other.NetworkEnabled = true other.MacOS.MAC = "02:00:00:00:00:02" - require.NoError(t, m.checkMacOSIdentityAvailable(context.Background(), &other)) + require.NoError(t, m.checkMacOSBootAvailable(context.Background(), &other)) +} + +func TestMacOSIdentityScanErrors(t *testing.T) { + for _, tc := range []struct { + name string + prefix string + check func(*manager, *StoredMetadata) error + }{ + {"stored MACs", "", func(m *manager, stored *StoredMetadata) error { + macs, err := m.storedMACs(stored.Id) + require.Nil(t, macs) + return err + }}, + {"MAC admission", "check macOS MAC availability: ", func(m *manager, stored *StoredMetadata) error { + return m.checkMacOSMACAvailable(context.Background(), stored) + }}, + {"identity admission", "check Mac identity admission: ", func(m *manager, stored *StoredMetadata) error { + return m.checkMacOSIdentityAvailable(context.Background(), stored) + }}, + } { + t.Run(tc.name, func(t *testing.T) { + p := paths.New(t.TempDir()) + m := &manager{paths: p} + stored := StoredMetadata{Id: "self", MAC: "02:00:00:00:00:01", MacOS: testMacImage().MacOS} + _, readErr := os.ReadDir(p.GuestsDir()) + require.EqualError(t, tc.check(m, &stored), readErr.Error()) + + require.NoError(t, m.ensureDirectories("a-valid")) + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: StoredMetadata{Id: "a-valid", MAC: "02:00:00:00:00:02"}})) + require.NoError(t, m.ensureDirectories("b-invalid")) + require.NoError(t, os.WriteFile(p.InstanceMetadata("b-invalid"), []byte("{not json"), 0600)) + _, loadErr := m.loadMetadata("b-invalid") + err := tc.check(m, &stored) + require.EqualError(t, err, tc.prefix+loadErr.Error()) + var syntaxErr *json.SyntaxError + require.ErrorAs(t, err, &syntaxErr) + }) + } +} + +func TestMacOSStoredMACsScan(t *testing.T) { + p := paths.New(t.TempDir()) + m := &manager{paths: p} + for _, id := range []string{"self", "orphan", "a", "b", "empty"} { + require.NoError(t, m.ensureDirectories(id)) + } + require.NoError(t, os.WriteFile(p.InstanceMetadata("self"), []byte("{not json"), 0600)) + require.NoError(t, os.WriteFile(filepath.Join(p.GuestsDir(), "file"), nil, 0600)) + for _, stored := range []StoredMetadata{ + {Id: "a", MAC: "02:00:00:00:00:01"}, + {Id: "b", MAC: "02:00:00:00:00:02", MacOS: testMacImage().MacOS}, + {Id: "empty"}, + } { + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: stored})) + } + macs, err := m.storedMACs("self") + require.NoError(t, err) + require.Equal(t, []string{"02:00:00:00:00:01", "02:00:00:00:00:02"}, macs) } func TestMacOSPreservedIdentityAdmission(t *testing.T) { @@ -196,11 +255,17 @@ func TestMacOSPreservedIdentityAdmission(t *testing.T) { other.Id = "other" err := m.checkMacOSIdentityAvailable(context.Background(), &other) require.True(t, errors.Is(err, ErrInsufficientResources)) + stored.MAC = "02:00:00:00:00:01" + stored.NetworkEnabled = true + other.MAC = stored.MAC + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: stored})) + require.EqualError(t, m.checkMacOSMACAvailable(context.Background(), &other), ErrInsufficientResources.Error()+": MAC "+stored.MAC+" is already in use by instance active; stop it first") + require.NoError(t, m.checkMacOSMACAvailable(context.Background(), &stored)) require.NoError(t, m.checkMacOSIdentityAvailable(context.Background(), &stored)) require.NoError(t, os.Remove(socket)) require.NoError(t, m.checkMacOSIdentityAvailable(context.Background(), &other)) - require.ErrorIs(t, (&manager{}).admitGuestOperation(&stored, "snapshot schedules"), ErrInvalidRequest) - require.ErrorIs(t, (&manager{}).admitGuestOperation(&stored, "fork snapshot"), ErrInvalidRequest) + require.NoError(t, m.checkMacOSMACAvailable(context.Background(), &other)) + require.NoError(t, (&manager{}).admitGuestOperation(&stored, "snapshot schedules")) } func TestMacOSSocketPathLimit(t *testing.T) { diff --git a/lib/instances/manager.go b/lib/instances/manager.go index 7ec4e1040..70515a576 100644 --- a/lib/instances/manager.go +++ b/lib/instances/manager.go @@ -335,6 +335,11 @@ func NewManagerWithConfigE(p *paths.Paths, imageManager images.Manager, systemMa if err := m.recoverPendingStandbyCompressionJobs(context.Background()); err != nil { logger.FromContext(context.Background()).WarnContext(context.Background(), "failed to recover pending standby compression jobs", "error", err) } + // An interrupted macOS restore can leave the previous disk parked in staging. Failure + // is logged here; each restore retries recovery for its own instance before swapping. + if err := m.recoverMacOSRestoreStaging(""); err != nil { + logger.FromContext(context.Background()).ErrorContext(context.Background(), "macOS restore staging recovery failed", "error", err) + } return m, nil } @@ -642,6 +647,14 @@ func (m *manager) RestoreInstance(ctx context.Context, id string) (*Instance, er return nil, err } if current.State == StateRunning || current.State == StateInitializing { + // A running macOS guest has consumed its checkpoint. One still on disk means an + // earlier resume failed before removing it, so finish that resume now. + if current.MacOS != nil && current.HasSnapshot { + if err := m.finishMacOSResume(id); err != nil { + return nil, fmt.Errorf("finish resumed restore: %w", err) + } + return m.currentInstanceWithoutHydration(ctx, id) + } return current, nil } if current.State == StatePaused { @@ -654,6 +667,11 @@ func (m *manager) RestoreInstance(ctx context.Context, id string) (*Instance, er return nil, fmt.Errorf("resume paused vm: %w", err) } m.storeCachedHypervisorState(id, hypervisor.StateRunning) + if current.MacOS != nil { + if err := m.finishMacOSResume(id); err != nil { + return nil, fmt.Errorf("finish resumed restore: %w", err) + } + } current, err = m.currentInstanceWithoutHydration(ctx, id) if err != nil { return nil, err diff --git a/lib/instances/restore.go b/lib/instances/restore.go index bdbdb3b2a..9c93b6608 100644 --- a/lib/instances/restore.go +++ b/lib/instances/restore.go @@ -45,6 +45,11 @@ func (m *manager) restoreInstance( if err := m.admitGuestOperation(&meta.StoredMetadata, "restore"); err != nil { return nil, err } + release, err := m.admitMacOSBoot(ctx, &meta.StoredMetadata) + if err != nil { + return nil, err + } + defer release() inst := m.toInstance(ctx, meta) stored := &meta.StoredMetadata @@ -67,16 +72,19 @@ func (m *manager) restoreInstance( // deep in storage configuration ("disk image not found"), retrying for ~60s // and surfacing as an opaque 500; resolving it up front (mirroring start) // returns images.ErrNotFound fast so the handler maps it to 404. - imageCtx, imageSpanEnd := m.startLifecycleStep(ctx, "resolve_image", - attribute.String("instance_id", id), - attribute.String("hypervisor", string(stored.HypervisorType)), - attribute.String("operation", "resolve_image"), - ) - _, err = m.imageManager.GetImage(imageCtx, bootImageRef(stored)) - imageSpanEnd(err) - if err != nil { - log.ErrorContext(ctx, "failed to resolve image for restore", "instance_id", id, "image", bootImageRef(stored), "error", err) - return nil, fmt.Errorf("get image: %w", err) + // macOS boots the disk and aux storage cloned at create time, as in start. + if stored.MacOS == nil { + imageCtx, imageSpanEnd := m.startLifecycleStep(ctx, "resolve_image", + attribute.String("instance_id", id), + attribute.String("hypervisor", string(stored.HypervisorType)), + attribute.String("operation", "resolve_image"), + ) + _, err = m.imageManager.GetImage(imageCtx, bootImageRef(stored)) + imageSpanEnd(err) + if err != nil { + log.ErrorContext(ctx, "failed to resolve image for restore", "instance_id", id, "image", bootImageRef(stored), "error", err) + return nil, fmt.Errorf("get image: %w", err) + } } // 2b. Validate aggregate resource limits before allocating resources (if configured) @@ -138,8 +146,9 @@ func (m *manager) restoreInstance( } } - // 4. Recreate or allocate network if network enabled - if stored.NetworkEnabled { + // 4. Recreate or allocate network if network enabled. A macOS guest keeps the MAC + // in its saved configuration, and vmnet supplies the address, so no TAP is allocated. + if stored.usesLinuxNetwork() { networkCtx, networkSpanEnd := m.startLifecycleStep(ctx, "restore_network", attribute.String("instance_id", id), attribute.String("hypervisor", string(stored.HypervisorType)), @@ -368,6 +377,13 @@ func (m *manager) restoreInstance( if err := restoreRetainedSnapshotBase(snapshotDir, retainedBaseDir); err != nil { log.WarnContext(ctx, "failed to retain snapshot base after restore", "instance_id", id, "error", err) } + } else if stored.MacOS != nil { + // A checkpoint left behind would read as Standby if the shim later died, and + // restoring it would replay old memory onto a newer disk. The guest is running; + // a retried restore removes it. + if err := os.RemoveAll(snapshotDir); err != nil { + return nil, fmt.Errorf("remove consumed snapshot: %w", err) + } } else { log.InfoContext(ctx, "deleting snapshot after successful restore", "instance_id", id) os.RemoveAll(snapshotDir) // Best effort, ignore errors diff --git a/lib/instances/snapshot.go b/lib/instances/snapshot.go index 6e783ce7e..2063e6431 100644 --- a/lib/instances/snapshot.go +++ b/lib/instances/snapshot.go @@ -66,6 +66,9 @@ func (m *manager) createSnapshot(ctx context.Context, id string, req CreateSnaps if err := m.admitGuestOperation(&meta.StoredMetadata, "snapshot"); err != nil { return nil, err } + if err := validateMacOSSnapshotCreate(meta, req); err != nil { + return nil, err + } inst := m.toInstance(ctx, meta) stored := &meta.StoredMetadata @@ -124,13 +127,16 @@ func (m *manager) createSnapshot(ctx context.Context, id string, req CreateSnaps return nil, fmt.Errorf("%w: standby snapshot requires source in %s or %s, got %s", ErrInvalidState, StateRunning, StateStandby, inst.State) } - copyErr := m.copySnapshotPayload(id, snapshotGuestDir) + copyErr := m.validateMacOSMemorySnapshotPayload(m.paths.InstanceDir(id), stored) + if copyErr == nil { + copyErr = m.copySnapshotPayload(id, snapshotGuestDir) + } if copyErr == nil { meta, copyErr = m.loadMetadata(id) } if restoreSource { - _, restoreErr := m.restoreInstance(ctx, id) + restoreErr := m.restoreSnapshotSource(ctx, stored) if restoreErr != nil { if copyErr != nil { return nil, fmt.Errorf("snapshot copy failed: %v; additionally failed to restore source: %w", copyErr, restoreErr) @@ -262,10 +268,16 @@ func (m *manager) restoreSnapshot(ctx context.Context, id string, snapshotID str if err != nil { return nil, err } + if err := validateMacOSSnapshotRestore(sourceMeta, rec); err != nil { + return nil, err + } if err := m.admitGuestOperation(&sourceMeta.StoredMetadata, "restore snapshot"); err != nil { return nil, err } sourceInst := m.toInstance(ctx, sourceMeta) + if err := validateMacOSSnapshotRestoreState(&sourceInst); err != nil { + return nil, err + } if sourceInst.State == StateRunning { return nil, fmt.Errorf("%w: cannot restore snapshot while source is %s", ErrInvalidState, sourceInst.State) } @@ -297,17 +309,15 @@ func (m *manager) restoreSnapshot(ctx context.Context, id string, snapshotID str m.recordSnapshotCompressionPreemption(ctx, snapshotCompressionPreemptionRestoreSnapshot, target.Target) } - if err := m.replaceInstanceWithSnapshotPayload(snapshotID, id); err != nil { - return nil, err - } - restored := cloneStoredMetadataWithoutPendingStandbyCompression(rec.StoredMetadata) restored.Id = sourceMeta.Id restored.Name = sourceMeta.Name restored.ExpiresAt = sourceMeta.ExpiresAt restored.DataDir = m.paths.InstanceDir(id) restored.HypervisorProcessIdentity.Clear() - restored.StartedAt = nil + if restored.MacOS == nil || rec.Snapshot.Kind == SnapshotKindStopped { + restored.StartedAt = nil + } restored.StoppedAt = nil restored.ExitCode = nil restored.ExitMessage = "" @@ -330,6 +340,20 @@ func (m *manager) restoreSnapshot(ctx context.Context, id string, snapshotID str restored.VsockCID = generateVsockCID(id) } + if restored.MacOS != nil { + if rec.Snapshot.Kind == SnapshotKindStandby { + if err := m.validateMacOSMemorySnapshotPayload(m.paths.SnapshotGuestDir(snapshotID), &rec.StoredMetadata); err != nil { + return nil, err + } + } + prepareMacOSSnapshotRestoreMetadata(&restored, &sourceMeta.StoredMetadata, rec.Snapshot.Kind, targetState) + if err := m.replaceMacOSInstanceWithSnapshotPayload(snapshotID, &metadata{StoredMetadata: restored}); err != nil { + return nil, err + } + } else if err := m.replaceInstanceWithSnapshotPayload(snapshotID, id); err != nil { + return nil, err + } + if err := m.saveMetadata(&metadata{StoredMetadata: restored}); err != nil { return nil, fmt.Errorf("save restored metadata: %w", err) } @@ -387,11 +411,14 @@ func (m *manager) forkSnapshot(ctx context.Context, snapshotID string, req ForkS if err := m.admitGuestOperation(&rec.StoredMetadata, "fork snapshot"); err != nil { return nil, err } + if err := validateMacOSSnapshotFork(rec); err != nil { + return nil, err + } if err := validateForkVolumeSafety(rec.StoredMetadata.Volumes); err != nil { return nil, fmt.Errorf("%w: snapshot requires readonly volume attachments: %v", ErrNotSupported, err) } - if err := m.ensureInstanceNameAvailableForSnapshotFork(ctx, req.Name, rec.StoredMetadata.NetworkEnabled); err != nil { + if err := m.ensureInstanceNameAvailableForSnapshotFork(ctx, req.Name, rec.StoredMetadata.usesLinuxNetwork()); err != nil { return nil, err } @@ -497,7 +524,12 @@ func (m *manager) forkSnapshot(ctx context.Context, snapshotID string, req ForkS } } - if err := m.saveMetadata(&metadata{StoredMetadata: forkMeta}); err != nil { + newMeta := &metadata{StoredMetadata: forkMeta} + if forkMeta.MacOS != nil { + if err := m.saveMacOSSnapshotFork(ctx, newMeta); err != nil { + return nil, err + } + } else if err := m.saveMetadata(newMeta); err != nil { return nil, fmt.Errorf("save fork metadata: %w", err) } diff --git a/lib/instances/snapshot_compression.go b/lib/instances/snapshot_compression.go index 31752aa5e..75eabbfa9 100644 --- a/lib/instances/snapshot_compression.go +++ b/lib/instances/snapshot_compression.go @@ -210,6 +210,9 @@ func normalizeCompressionConfig(cfg *snapshotstore.SnapshotCompressionConfig) (s } func (m *manager) resolveSnapshotCompressionPolicy(stored *StoredMetadata, override *snapshotstore.SnapshotCompressionConfig) (snapshotstore.SnapshotCompressionConfig, error) { + if stored != nil && stored.MacOS != nil { + return snapshotstore.SnapshotCompressionConfig{Enabled: false}, nil + } cfg, err := m.resolveConfiguredCompressionPolicy(stored, override) if err != nil { return snapshotstore.SnapshotCompressionConfig{}, err diff --git a/lib/instances/snapshot_macos.go b/lib/instances/snapshot_macos.go new file mode 100644 index 000000000..7fe34c79e --- /dev/null +++ b/lib/instances/snapshot_macos.go @@ -0,0 +1,352 @@ +package instances + +import ( + "bytes" + "context" + "encoding/base64" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "time" + + "github.com/kernel/hypeman/lib/forkvm" + "github.com/kernel/hypeman/lib/hypervisor" + "github.com/kernel/hypeman/lib/hypervisor/vz/shimconfig" + "github.com/kernel/hypeman/lib/instances/phasetracking" + "github.com/kernel/hypeman/lib/paths" + restartpolicy "github.com/kernel/hypeman/lib/restart-policy" +) + +func validateMacOSSnapshotCreate(meta *metadata, req CreateSnapshotRequest) error { + if meta.MacOS == nil { + return nil + } + if req.Kind == SnapshotKindStandby { + if req.Compression != nil && req.Compression.Enabled { + return fmt.Errorf("%w: macOS memory snapshots do not support compression", ErrInvalidRequest) + } + return validateMacOSMemorySnapshotSupport(&meta.StoredMetadata) + } + if req.Kind != SnapshotKindStopped { + return fmt.Errorf("%w: unsupported macOS snapshot kind %q", ErrInvalidRequest, req.Kind) + } + if !meta.StopGraceful { + return fmt.Errorf("%w: macOS stopped snapshots require a graceful stop", ErrInvalidState) + } + return nil +} + +func validateMacOSMemorySnapshotSupport(stored *StoredMetadata) error { + if stored.MacOS == nil { + return nil + } + caps, ok := hypervisor.CapabilitiesForType(stored.HypervisorType) + if !ok || !caps.SupportsMacOSBoot || !caps.SupportsSnapshot { + return fmt.Errorf("%w: macOS memory snapshots require a host with VZ save/restore support", ErrNotSupported) + } + return nil +} + +func validateMacOSSaveRestoreSupport(ctx context.Context, stored *StoredMetadata, hv hypervisor.Hypervisor) error { + if stored.MacOS == nil { + return nil + } + validator, ok := hv.(hypervisor.SnapshotSupportValidator) + if !ok { + return fmt.Errorf("%w: macOS snapshot configuration validation is unavailable", ErrNotSupported) + } + if err := validator.ValidateSnapshotSupport(ctx); err != nil { + if errors.Is(err, hypervisor.ErrNotSupported) { + return fmt.Errorf("%w: %w", ErrNotSupported, err) + } + return fmt.Errorf("validate macOS snapshot configuration: %w", err) + } + return nil +} + +func (m *manager) restoreSnapshotSource(ctx context.Context, stored *StoredMetadata) error { + if stored.MacOS != nil { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(context.WithoutCancel(ctx), 2*time.Minute) + defer cancel() + } + _, err := m.restoreInstance(ctx, stored.Id) + return err +} + +func resumeSnapshotAfterError(ctx context.Context, stored *StoredMetadata, hv hypervisor.Hypervisor) error { + if stored.MacOS != nil { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(context.WithoutCancel(ctx), 10*time.Second) + defer cancel() + } + return hv.Resume(ctx) +} + +func (m *manager) validateMacOSSnapshotSchedule(ctx context.Context, meta *metadata) error { + if meta.MacOS == nil { + return nil + } + if err := validateMacOSMemorySnapshotSupport(&meta.StoredMetadata); err != nil { + return err + } + inst := m.toInstance(ctx, meta) + switch inst.State { + case StateRunning, StateInitializing, StateCreated, StatePaused: + hv, err := m.getHypervisor(inst.SocketPath, inst.HypervisorType) + if err != nil { + return err + } + return validateMacOSSaveRestoreSupport(ctx, &meta.StoredMetadata, hv) + } + return nil +} + +func prepareMacOSSnapshotRestoreMetadata(restored *StoredMetadata, source *StoredMetadata, kind SnapshotKind, target State) { + restored.Phases = source.Phases + phase := phasetracking.PhaseStopped + if kind == SnapshotKindStandby { + restored.StopGraceful = false + if target != StateStopped { + phase = phasetracking.PhaseStandby + } + } + restored.Phases.Record(phase, time.Now().UTC()) + if target == StateStopped { + restored.RestartStatus = restartpolicy.Status{BlockedReason: restartpolicy.BlockedReasonManualStop} + } +} + +func validateMacOSSnapshotRestoreState(inst *Instance) error { + if inst.MacOS != nil && inst.State != StateStopped && inst.State != StateStandby { + return fmt.Errorf("%w: macOS snapshot restore requires a stopped or standby instance, got %s", ErrInvalidState, inst.State) + } + return nil +} + +// The manifest is used by VZ rather than rebuilt from instance metadata. Validate +// the saved configuration and its disk/aux/memory bundle before replacing storage. +func (m *manager) validateMacOSMemorySnapshotPayload(guestDir string, stored *StoredMetadata) error { + if stored.MacOS == nil { + return nil + } + snapshotDir := filepath.Join(guestDir, "snapshots", "snapshot-latest") + data, err := os.ReadFile(filepath.Join(snapshotDir, shimconfig.SnapshotManifestFile)) + if err != nil { + return fmt.Errorf("%w: read macOS snapshot manifest: %v", ErrInvalidRequest, err) + } + var manifest shimconfig.SnapshotManifest + if err := json.Unmarshal(data, &manifest); err != nil { + return fmt.Errorf("%w: decode macOS snapshot manifest: %v", ErrInvalidRequest, err) + } + cfg := manifest.ShimConfig + origin := m.paths.InstanceDir(stored.Id) + if manifest.Hypervisor != string(hypervisor.TypeVZ) || (manifest.MachineStateFile != "" && manifest.MachineStateFile != shimconfig.SnapshotMachineStateFile) { + return fmt.Errorf("%w: invalid macOS snapshot format", ErrInvalidRequest) + } + if cfg.VCPUs != stored.Vcpus || cfg.MemoryBytes != stored.Size || cfg.MacHardwareModelData != base64.StdEncoding.EncodeToString(stored.MacOS.HardwareModel) || cfg.MacMachineIdentifierData != base64.StdEncoding.EncodeToString(stored.MacOS.MachineIdentifier) { + return fmt.Errorf("%w: macOS snapshot configuration does not match its metadata", ErrInvalidRequest) + } + if len(cfg.Disks) != 1 || cfg.Disks[0].Readonly || cfg.Disks[0].Path != m.paths.InstanceMacOSBootDisk(stored.Id) || cfg.MacAuxStoragePath != filepath.Join(origin, macOSAuxStorageFile) { + return fmt.Errorf("%w: macOS snapshot storage configuration does not match its instance", ErrInvalidRequest) + } + if cfg.KernelPath != "" || cfg.InitrdPath != "" || cfg.EnableMemoryBalloon || cfg.RequireMemoryBalloon || cfg.EnableRosetta { + return fmt.Errorf("%w: unsupported macOS snapshot devices", ErrInvalidRequest) + } + if stored.NetworkEnabled { + if len(cfg.Networks) != 1 || !strings.EqualFold(cfg.Networks[0].MAC, stored.MAC) || !strings.EqualFold(stored.MAC, stored.MacOS.MAC) { + return fmt.Errorf("%w: macOS snapshot network identity does not match its metadata", ErrInvalidRequest) + } + } else if len(cfg.Networks) != 0 { + return fmt.Errorf("%w: macOS snapshot unexpectedly enables networking", ErrInvalidRequest) + } + for _, path := range []string{filepath.Join(guestDir, paths.MacOSBootDiskFile), filepath.Join(guestDir, macOSAuxStorageFile), filepath.Join(snapshotDir, shimconfig.SnapshotMachineStateFile)} { + info, err := os.Stat(path) + if err != nil || !info.Mode().IsRegular() || info.Size() == 0 { + return fmt.Errorf("%w: missing or invalid macOS snapshot payload %s", ErrInvalidRequest, filepath.Base(path)) + } + } + return nil +} + +func validateMacOSSnapshotRestore(source *metadata, rec *snapshotRecord) error { + if source.MacOS == nil && rec.StoredMetadata.MacOS == nil { + return nil + } + if source.MacOS == nil || rec.StoredMetadata.MacOS == nil { + return fmt.Errorf("%w: snapshot and instance guest types differ", ErrInvalidRequest) + } + if rec.Snapshot.Kind != SnapshotKindStopped && rec.Snapshot.Kind != SnapshotKindStandby { + return fmt.Errorf("%w: unsupported macOS snapshot kind %q", ErrInvalidRequest, rec.Snapshot.Kind) + } + if rec.Snapshot.Kind == SnapshotKindStandby { + if err := validateMacOSMemorySnapshotSupport(&rec.StoredMetadata); err != nil { + return err + } + } + want, got := source.MacOS, rec.StoredMetadata.MacOS + if !bytes.Equal(want.MachineIdentifier, got.MachineIdentifier) || !bytes.Equal(want.HardwareModel, got.HardwareModel) || want.MAC != got.MAC { + return fmt.Errorf("%w: snapshot identity does not match this instance", ErrInvalidRequest) + } + return nil +} + +func validateMacOSSnapshotFork(rec *snapshotRecord) error { + if rec.StoredMetadata.MacOS == nil { + return nil + } + if rec.Snapshot.Kind != SnapshotKindStopped { + return fmt.Errorf("%w: macOS forks require stopped snapshots", ErrNotSupported) + } + if !rec.StoredMetadata.StopGraceful { + return fmt.Errorf("%w: macOS snapshot forks require a graceful stop receipt", ErrInvalidState) + } + if !rec.StoredMetadata.GuestAgentEnabled() { + return fmt.Errorf("%w: macOS snapshot forks require an enabled guest agent for identity rekey", ErrInvalidRequest) + } + return nil +} + +func (m *manager) saveMacOSSnapshotFork(ctx context.Context, meta *metadata) error { + if err := prepareMacOSClone(&meta.StoredMetadata); err != nil { + return err + } + meta.ProgramStartedAt = nil + meta.GuestAgentReadyAt = nil + meta.GuestAgent = nil + meta.Phases.Reset() + meta.Phases.Record(phasetracking.PhaseStopped, meta.CreatedAt) + return m.saveForkMetadata(ctx, meta) +} + +// replaceMacOSInstanceWithSnapshotPayload stages the snapshot and restored metadata +// beside the instance, then swaps both in. A failure before the swap leaves the +// previous directory intact; a failed swap is rolled back. The staging root lives outside the guests directory, +// so instance and snapshot listings never see a half-restored copy. +func (m *manager) replaceMacOSInstanceWithSnapshotPayload(snapshotID string, restored *metadata) error { + instanceID := restored.Id + if err := m.recoverMacOSRestoreStaging(instanceID); err != nil { + return fmt.Errorf("recover before restore: %w", err) + } + instanceDir := m.paths.InstanceDir(instanceID) + root := filepath.Join(m.paths.DataDir(), "restore-staging", instanceID) + staged := filepath.Join(root, "staged") + previous := filepath.Join(root, "previous") + fail := func(err error) error { + _ = os.RemoveAll(root) + return err + } + if err := os.RemoveAll(root); err != nil { + return fmt.Errorf("clear restore staging: %w", err) + } + if err := os.MkdirAll(root, 0700); err != nil { + return fmt.Errorf("create restore staging: %w", err) + } + if err := forkvm.CopyGuestDirectory(m.paths.SnapshotGuestDir(snapshotID), staged); err != nil { + if errors.Is(err, forkvm.ErrSparseCopyUnsupported) { + return fail(fmt.Errorf("restore requires sparse-capable filesystem (SEEK_DATA/SEEK_HOLE unsupported): %w", err)) + } + return fail(fmt.Errorf("stage snapshot payload: %w", err)) + } + if restored.Phases.Current == phasetracking.PhaseStopped { + if err := os.RemoveAll(filepath.Join(staged, "snapshots", "snapshot-latest")); err != nil { + return fail(fmt.Errorf("discard staged memory checkpoint: %w", err)) + } + } + data, err := json.MarshalIndent(restored, "", " ") + if err != nil { + return fail(fmt.Errorf("marshal restored metadata: %w", err)) + } + if err := os.WriteFile(filepath.Join(staged, filepath.Base(m.paths.InstanceMetadata(instanceID))), data, 0644); err != nil { + return fail(fmt.Errorf("stage restored metadata: %w", err)) + } + if err := os.Rename(instanceDir, previous); err != nil { + return fail(fmt.Errorf("retire instance directory: %w", err)) + } + if err := os.Rename(staged, instanceDir); err != nil { + if rollbackErr := os.Rename(previous, instanceDir); rollbackErr != nil { + return fmt.Errorf("install snapshot payload: %w (rollback failed: %v; previous state kept at %s)", err, rollbackErr, previous) + } + return fail(fmt.Errorf("install snapshot payload: %w", err)) + } + _ = os.RemoveAll(root) + return nil +} + +// recoverMacOSRestoreStaging resolves a swap interrupted by a crash. With the instance +// directory missing and the previous directory parked, the previous state is put back. +// If the instance directory is present, a completed swap leaves stale staging, which is +// removed. Staging with no previous directory is an unfinished copy and is removed. +// An empty instanceID recovers every instance. +func (m *manager) recoverMacOSRestoreStaging(instanceID string) error { + root := filepath.Join(m.paths.DataDir(), "restore-staging") + ids := []string{instanceID} + if instanceID == "" { + entries, err := os.ReadDir(root) + if errors.Is(err, os.ErrNotExist) { + return nil + } + if err != nil { + return err + } + ids = ids[:0] + for _, entry := range entries { + ids = append(ids, entry.Name()) + } + } + for _, id := range ids { + stagingRoot := filepath.Join(root, id) + previous := filepath.Join(stagingRoot, "previous") + instanceDir := m.paths.InstanceDir(id) + _, previousErr := os.Stat(previous) + if previousErr != nil && !errors.Is(previousErr, os.ErrNotExist) { + return fmt.Errorf("inspect restore staging for %s: %w", id, previousErr) + } + if previousErr == nil { + _, instanceErr := os.Stat(instanceDir) + if instanceErr != nil && !errors.Is(instanceErr, os.ErrNotExist) { + return fmt.Errorf("inspect instance directory for %s: %w", id, instanceErr) + } + if errors.Is(instanceErr, os.ErrNotExist) { + if err := os.MkdirAll(filepath.Dir(instanceDir), 0755); err != nil { + return fmt.Errorf("prepare rollback for %s: %w", id, err) + } + if err := os.Rename(previous, instanceDir); err != nil { + return fmt.Errorf("roll back restore for %s: %w", id, err) + } + } + } + if err := os.RemoveAll(stagingRoot); err != nil { + return fmt.Errorf("clear restore staging for %s: %w", id, err) + } + } + return nil +} + +// finishMacOSResume completes a restore whose API process died after the shim had +// loaded the VM, once resuming the Paused guest succeeds. It records the shim, and +// removes the consumed memory snapshot so a later crash cannot restore stale memory +// onto the guest's newer disk. +func (m *manager) finishMacOSResume(id string) error { + meta, err := m.loadMetadata(id) + if err != nil { + return err + } + if meta.HypervisorPID == nil || !ProcessExists(*meta.HypervisorPID) { + pid, err := vzShimOwner(meta.SocketPath) + if err != nil { + return err + } + if pid > 0 { + meta.HypervisorProcessIdentity.Set(pid) + } + } + if err := os.RemoveAll(m.paths.InstanceSnapshotLatest(id)); err != nil { + return fmt.Errorf("remove consumed snapshot: %w", err) + } + meta.Phases.Record(initialBootPhase(&meta.StoredMetadata), time.Now().UTC()) + return m.saveMetadata(meta) +} diff --git a/lib/instances/snapshot_macos_darwin_test.go b/lib/instances/snapshot_macos_darwin_test.go new file mode 100644 index 000000000..edf1db8a8 --- /dev/null +++ b/lib/instances/snapshot_macos_darwin_test.go @@ -0,0 +1,73 @@ +//go:build darwin && arm64 + +package instances + +import ( + "context" + "os" + "path/filepath" + "testing" + "time" + + "github.com/kernel/hypeman/lib/hypervisor" + "github.com/kernel/hypeman/lib/hypervisor/vz" + "github.com/kernel/hypeman/lib/instances/phasetracking" + "github.com/kernel/hypeman/lib/paths" + "github.com/stretchr/testify/require" +) + +func TestMacOSStoppedSnapshotForkRebindsIdentity(t *testing.T) { + p := paths.New(t.TempDir()) + m := &manager{paths: p, macOSOnly: true, lifecycleEvents: newLifecycleSubscribers(), vmStarters: map[hypervisor.Type]hypervisor.VMStarter{hypervisor.TypeVZ: concurrentForkPrepareTestStarter{}}} + require.NoError(t, os.MkdirAll(p.GuestsDir(), 0700)) + const snapshotID = "mac-fork-snapshot" + source := *macOSGuestMetadata(2) + source.Id, source.Name = "removed-source", "removed-source" + source.DataDir = p.InstanceDir(source.Id) + source.HypervisorType = hypervisor.TypeVZ + source.Size, source.Vcpus = 8<<30, 4 + source.NetworkEnabled = true + source.MAC = source.MacOS.MAC + source.MacOS.GuestAgent = true + source.StopGraceful = true + now := time.Now() + source.GuestAgentReadyAt = &now + source.Phases.Record(phasetracking.PhaseRunning, now.Add(-time.Hour)) + source.Phases.Record(phasetracking.PhaseStopped, now) + require.NoError(t, vz.NewStarter().ValidateConfig(m.plannedStoredVMConfig(SnapshotKindStopped, source))) + require.NoError(t, m.saveSnapshotRecord(&snapshotRecord{Snapshot: Snapshot{Id: snapshotID, Kind: SnapshotKindStopped, SourceInstanceID: source.Id, SourceName: source.Name, SourceHypervisor: source.HypervisorType, CreatedAt: now}, StoredMetadata: source})) + writeGuestFiles(t, p.SnapshotGuestDir(snapshotID), map[string]string{"boot.raw": "snapshot-disk", macOSAuxStorageFile: "snapshot-aux"}) + + first, err := m.ForkSnapshot(context.Background(), snapshotID, ForkSnapshotRequest{Name: "mac-snapshot-clone-1"}) + require.NoError(t, err) + second, err := m.ForkSnapshot(context.Background(), snapshotID, ForkSnapshotRequest{Name: "mac-snapshot-clone-2"}) + require.NoError(t, err) + for _, clone := range []*Instance{first, second} { + require.Equal(t, StateStopped, clone.State) + require.True(t, clone.IdentityRekeyPending) + require.NotEqual(t, source.MacOS.MachineIdentifier, clone.MacOS.MachineIdentifier) + require.NotEqual(t, source.MAC, clone.MAC) + require.Equal(t, clone.MAC, clone.MacOS.MAC) + require.Empty(t, clone.IP) + require.Nil(t, clone.GuestAgentReadyAt) + require.Nil(t, clone.GuestAgent) + require.Nil(t, clone.HealthCheckRuntime) + require.Empty(t, clone.Phases.Cumulative) + require.Equal(t, phasetracking.PhaseStopped, clone.Phases.Current) + require.Equal(t, clone.CreatedAt, clone.Phases.Since) + data, err := os.ReadFile(filepath.Join(clone.DataDir, macOSAuxStorageFile)) + require.NoError(t, err) + require.Equal(t, "snapshot-aux", string(data)) + } + require.NotEqual(t, first.MAC, second.MAC) + require.NotEqual(t, first.MacOS.MachineIdentifier, second.MacOS.MachineIdentifier) + _, err = m.ForkSnapshot(context.Background(), snapshotID, ForkSnapshotRequest{Name: first.Name}) + require.ErrorIs(t, err, ErrAlreadyExists) + persisted, err := m.loadSnapshotRecord(snapshotID) + require.NoError(t, err) + require.Equal(t, source.MacOS, persisted.StoredMetadata.MacOS) + require.NoError(t, os.WriteFile(p.InstanceMacOSBootDisk(first.Id), []byte("clone-write"), 0600)) + data, err := os.ReadFile(filepath.Join(p.SnapshotGuestDir(snapshotID), "boot.raw")) + require.NoError(t, err) + require.Equal(t, "snapshot-disk", string(data)) +} diff --git a/lib/instances/snapshot_macos_test.go b/lib/instances/snapshot_macos_test.go new file mode 100644 index 000000000..dbc8526d9 --- /dev/null +++ b/lib/instances/snapshot_macos_test.go @@ -0,0 +1,155 @@ +package instances + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/kernel/hypeman/lib/images" + "github.com/kernel/hypeman/lib/paths" + "github.com/stretchr/testify/require" +) + +func macOSGuestMetadata(identity byte) *StoredMetadata { + return &StoredMetadata{MacOS: &images.MacOSImage{ + HardwareModel: []byte{1}, MachineIdentifier: []byte{identity}, MAC: "02:00:00:00:00:01", CPUs: 4, Memory: 8 << 30, + }} +} + +func TestMacOSSnapshotCreateGuardsGracefulStopAndHostSupport(t *testing.T) { + require.NoError(t, validateMacOSSnapshotCreate(&metadata{StoredMetadata: StoredMetadata{}}, CreateSnapshotRequest{Kind: SnapshotKindStandby}), "Linux guests keep their rules") + + mac := &metadata{StoredMetadata: *macOSGuestMetadata(2)} + require.ErrorIs(t, validateMacOSSnapshotCreate(mac, CreateSnapshotRequest{Kind: SnapshotKindStandby}), ErrNotSupported) + require.ErrorIs(t, validateMacOSSnapshotCreate(mac, CreateSnapshotRequest{Kind: SnapshotKindStopped}), ErrInvalidState, "a forced stop is not a consistent disk") + + mac.StopGraceful = true + require.NoError(t, validateMacOSSnapshotCreate(mac, CreateSnapshotRequest{Kind: SnapshotKindStopped})) +} + +func TestMacOSSnapshotRestoreRequiresSameIdentityAndSupportedKind(t *testing.T) { + source := &metadata{StoredMetadata: *macOSGuestMetadata(2)} + same := &snapshotRecord{Snapshot: Snapshot{Kind: SnapshotKindStopped}, StoredMetadata: *macOSGuestMetadata(2)} + require.NoError(t, validateMacOSSnapshotRestore(source, same)) + + other := &snapshotRecord{Snapshot: Snapshot{Kind: SnapshotKindStopped}, StoredMetadata: *macOSGuestMetadata(9)} + require.ErrorIs(t, validateMacOSSnapshotRestore(source, other), ErrInvalidRequest, "a different machine identifier is another guest") + + standby := &snapshotRecord{Snapshot: Snapshot{Kind: SnapshotKindStandby}, StoredMetadata: *macOSGuestMetadata(2)} + require.ErrorIs(t, validateMacOSSnapshotRestore(source, standby), ErrNotSupported, "unsupported hosts cannot restore memory snapshots") + + linux := &snapshotRecord{Snapshot: Snapshot{Kind: SnapshotKindStopped}} + require.ErrorIs(t, validateMacOSSnapshotRestore(source, linux), ErrInvalidRequest, "guest types must match") +} + +func TestMacOSSnapshotForkRequiresGracefulStoppedSnapshotAndAgent(t *testing.T) { + rec := &snapshotRecord{Snapshot: Snapshot{Kind: SnapshotKindStopped}, StoredMetadata: *macOSGuestMetadata(2)} + require.ErrorIs(t, validateMacOSSnapshotFork(rec), ErrInvalidState) + rec.StoredMetadata.StopGraceful = true + require.ErrorIs(t, validateMacOSSnapshotFork(rec), ErrInvalidRequest) + rec.StoredMetadata.MacOS.GuestAgent = true + require.NoError(t, validateMacOSSnapshotFork(rec)) + rec.StoredMetadata.SkipGuestAgent = true + require.ErrorIs(t, validateMacOSSnapshotFork(rec), ErrInvalidRequest) + rec.StoredMetadata.SkipGuestAgent = false + rec.Snapshot.Kind = SnapshotKindStandby + require.ErrorIs(t, validateMacOSSnapshotFork(rec), ErrNotSupported) + require.NoError(t, validateMacOSSnapshotFork(&snapshotRecord{}), "Linux admission is unchanged") +} + +func writeGuestFiles(t *testing.T, dir string, files map[string]string) { + t.Helper() + require.NoError(t, os.MkdirAll(dir, 0700)) + for name, body := range files { + require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(body), 0600)) + } +} + +func TestMacOSRestoreSwapsPayloadWithMetadata(t *testing.T) { + m := &manager{paths: paths.New(t.TempDir())} + const id, snap = "mac-restore", "snap-1" + writeGuestFiles(t, m.paths.InstanceDir(id), map[string]string{"boot.raw": "after", macOSAuxStorageFile: "aux-after"}) + writeGuestFiles(t, m.paths.SnapshotGuestDir(snap), map[string]string{"boot.raw": "before", macOSAuxStorageFile: "aux-before", "metadata.json": "stale-metadata"}) + expiresAt := time.Now().Add(time.Hour) + restored := &metadata{StoredMetadata: StoredMetadata{Id: id, Name: "current-name", ExpiresAt: &expiresAt, Vcpus: 4, Size: 8 << 30}} + + require.NoError(t, m.replaceMacOSInstanceWithSnapshotPayload(snap, restored)) + data, err := os.ReadFile(filepath.Join(m.paths.InstanceDir(id), "boot.raw")) + require.NoError(t, err) + require.Equal(t, "before", string(data)) + _, err = os.Stat(filepath.Join(m.paths.DataDir(), "restore-staging", id)) + require.ErrorIs(t, err, os.ErrNotExist, "staging is removed after a successful swap") + // A restart before the caller's own saveMetadata must find the restored metadata. + require.NoError(t, m.recoverMacOSRestoreStaging(id)) + got, err := m.loadMetadata(id) + require.NoError(t, err) + require.Equal(t, restored.Name, got.Name) + require.Equal(t, restored.Vcpus, got.Vcpus) + require.NotNil(t, got.ExpiresAt) + require.True(t, expiresAt.Equal(*got.ExpiresAt)) + + // A missing snapshot payload fails before the instance directory is retired. + writeGuestFiles(t, m.paths.InstanceDir(id), map[string]string{"boot.raw": "current"}) + require.Error(t, m.replaceMacOSInstanceWithSnapshotPayload("missing-snapshot", &metadata{StoredMetadata: StoredMetadata{Id: id}})) + data, err = os.ReadFile(filepath.Join(m.paths.InstanceDir(id), "boot.raw")) + require.NoError(t, err) + require.Equal(t, "current", string(data), "a failed staging leaves the previous disk untouched") +} + +func TestMacOSRestoreRecoveryRollsBackAndCleans(t *testing.T) { + const id = "mac-crash" + newManager := func(t *testing.T) (*manager, string) { + m := &manager{paths: paths.New(t.TempDir())} + return m, filepath.Join(m.paths.DataDir(), "restore-staging", id) + } + + // Crash after retiring the instance directory, before the swap: roll the previous state back. + m, root := newManager(t) + writeGuestFiles(t, filepath.Join(root, "previous"), map[string]string{"boot.raw": "previous"}) + require.NoError(t, m.recoverMacOSRestoreStaging(id)) + data, err := os.ReadFile(filepath.Join(m.paths.InstanceDir(id), "boot.raw")) + require.NoError(t, err) + require.Equal(t, "previous", string(data)) + _, err = os.Stat(root) + require.ErrorIs(t, err, os.ErrNotExist) + + // Crash after a completed swap: the instance directory is current, so stale staging goes. + m, root = newManager(t) + writeGuestFiles(t, m.paths.InstanceDir(id), map[string]string{"boot.raw": "restored"}) + writeGuestFiles(t, filepath.Join(root, "previous"), map[string]string{"boot.raw": "old"}) + require.NoError(t, m.recoverMacOSRestoreStaging(id)) + data, err = os.ReadFile(filepath.Join(m.paths.InstanceDir(id), "boot.raw")) + require.NoError(t, err) + require.Equal(t, "restored", string(data)) + _, err = os.Stat(root) + require.ErrorIs(t, err, os.ErrNotExist) + + // Crash during staging copy: no previous directory, so the partial copy is removed. + m, root = newManager(t) + writeGuestFiles(t, filepath.Join(root, "staged"), map[string]string{"boot.raw": "partial"}) + require.NoError(t, m.recoverMacOSRestoreStaging("")) + _, err = os.Stat(root) + require.ErrorIs(t, err, os.ErrNotExist) +} + +func TestMacOSRestoreRecoveryKeepsBackupWhenInstanceStateIsUnknown(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root ignores directory permissions") + } + const id = "mac-unknown" + m := &manager{paths: paths.New(t.TempDir())} + root := filepath.Join(m.paths.DataDir(), "restore-staging", id) + writeGuestFiles(t, filepath.Join(root, "previous"), map[string]string{"boot.raw": "previous"}) + guests := filepath.Dir(m.paths.InstanceDir(id)) + require.NoError(t, os.MkdirAll(guests, 0755)) + // An unreadable guests directory makes the instance directory's state unknown. + require.NoError(t, os.Chmod(guests, 0)) + t.Cleanup(func() { _ = os.Chmod(guests, 0755) }) + + require.Error(t, m.recoverMacOSRestoreStaging(id)) + require.NoError(t, os.Chmod(guests, 0755)) + data, err := os.ReadFile(filepath.Join(root, "previous", "boot.raw")) + require.NoError(t, err, "the parked backup must survive an inconclusive check") + require.Equal(t, "previous", string(data)) +} diff --git a/lib/instances/snapshot_schedule.go b/lib/instances/snapshot_schedule.go index 4eccc4983..4c01a9037 100644 --- a/lib/instances/snapshot_schedule.go +++ b/lib/instances/snapshot_schedule.go @@ -40,10 +40,12 @@ func (m *manager) SetSnapshotSchedule(ctx context.Context, instanceID string, re if err != nil { return nil, err } - // Schedules run the snapshot path, which macOS guests reject; refuse up front. if err := m.admitGuestOperation(&meta.StoredMetadata, "snapshot schedules"); err != nil { return nil, err } + if err := m.validateMacOSSnapshotSchedule(ctx, meta); err != nil { + return nil, err + } now := m.now().UTC() createdAt := now diff --git a/lib/instances/snapshot_schedule_test.go b/lib/instances/snapshot_schedule_test.go index d208b9abf..1578a4ff0 100644 --- a/lib/instances/snapshot_schedule_test.go +++ b/lib/instances/snapshot_schedule_test.go @@ -658,7 +658,7 @@ func markSnapshotScheduleDue(t *testing.T, mgr *manager, instanceID string) { require.NoError(t, mgr.saveSnapshotScheduleUnlocked(schedule)) } -func TestSnapshotScheduleRejectedForMacOSGuests(t *testing.T) { +func TestSnapshotScheduleRejectsMacOSWithoutSaveRestoreSupport(t *testing.T) { t.Parallel() mgr, _ := setupTestManager(t) ctx := context.Background() @@ -674,6 +674,6 @@ func TestSnapshotScheduleRejectedForMacOSGuests(t *testing.T) { NamePrefix: "nightly", Retention: SnapshotScheduleRetention{MaxCount: 1}, }) - require.ErrorIs(t, err, ErrInvalidRequest) - require.Contains(t, err.Error(), "snapshot schedules") + require.ErrorIs(t, err, ErrNotSupported) + require.Contains(t, err.Error(), "save/restore support") } diff --git a/lib/instances/standby.go b/lib/instances/standby.go index f521b251e..46e761464 100644 --- a/lib/instances/standby.go +++ b/lib/instances/standby.go @@ -45,6 +45,11 @@ func (m *manager) standbyInstance( if err := m.admitGuestOperation(&meta.StoredMetadata, "standby"); err != nil { return nil, err } + if meta.MacOS != nil { + // macOS saved state is not compressed. The framework already omits zero pages, + // and compression would add time inside the paused window. + skipCompression = true + } inst := m.toInstance(ctx, meta) stored := &meta.StoredMetadata @@ -91,7 +96,7 @@ func (m *manager) standbyInstance( // 3. Get network allocation BEFORE killing VMM (while we can still query it) // This is needed to delete the TAP device after VMM shuts down var networkAlloc *network.Allocation - if inst.NetworkEnabled { + if inst.usesLinuxNetwork() { log.DebugContext(ctx, "getting network allocation", "instance_id", id) networkAlloc, err = m.networkManager.GetAllocation(ctx, id) if err != nil { @@ -112,6 +117,10 @@ func (m *manager) standbyInstance( return nil, fmt.Errorf("hypervisor %s does not support standby (snapshots)", stored.HypervisorType) } + if err := validateMacOSSaveRestoreSupport(ctx, stored, hv); err != nil { + return nil, err + } + // 6. Transition: Running → Paused log.DebugContext(ctx, "pausing VM", "instance_id", id) pauseCtx, pauseSpanEnd := m.startLifecycleStep(ctx, "pause_vm", @@ -122,6 +131,11 @@ func (m *manager) standbyInstance( if err := hv.Pause(pauseCtx); err != nil { pauseSpanEnd(err) log.ErrorContext(ctx, "failed to pause VM", "instance_id", id, "error", err) + if stored.MacOS != nil { + if resumeErr := resumeSnapshotAfterError(ctx, stored, hv); resumeErr != nil { + log.ErrorContext(ctx, "failed to resume VM after pause error", "instance_id", id, "error", resumeErr) + } + } return nil, fmt.Errorf("pause vm failed: %w", err) } pauseSpanEnd(nil) @@ -135,7 +149,7 @@ func (m *manager) standbyInstance( var err error promotedExistingBase, err = prepareRetainedSnapshotTarget(snapshotDir, retainedBaseDir) if err != nil { - if resumeErr := hv.Resume(ctx); resumeErr != nil { + if resumeErr := resumeSnapshotAfterError(ctx, stored, hv); resumeErr != nil { log.ErrorContext(ctx, "failed to resume VM after retained snapshot target preparation error", "instance_id", id, "error", resumeErr) } return nil, fmt.Errorf("prepare retained snapshot target: %w", err) @@ -144,7 +158,7 @@ func (m *manager) standbyInstance( // place; if fanout forks still hardlink its inode, replace it with a // private copy first so their memory is never mutated. if err := ensureExclusiveSnapshotMemoryOwnership(ctx, snapshotDir); err != nil { - if resumeErr := hv.Resume(ctx); resumeErr != nil { + if resumeErr := resumeSnapshotAfterError(ctx, stored, hv); resumeErr != nil { log.ErrorContext(ctx, "failed to resume VM after snapshot memory unshare error", "instance_id", id, "error", resumeErr) } if promotedExistingBase { @@ -166,7 +180,7 @@ func (m *manager) standbyInstance( snapshotSpanEnd(err) // Snapshot failed - try to resume VM log.ErrorContext(ctx, "snapshot failed, attempting to resume VM", "instance_id", id, "error", err) - if resumeErr := hv.Resume(ctx); resumeErr != nil { + if resumeErr := resumeSnapshotAfterError(ctx, stored, hv); resumeErr != nil { log.ErrorContext(ctx, "failed to resume VM after snapshot error", "instance_id", id, "error", resumeErr) } if promotedExistingBase { @@ -190,7 +204,7 @@ func (m *manager) standbyInstance( // The hypervisor may still be running: releasing its TAP or clearing // its identity now would orphan a live VMM. The snapshot on disk is // harmless and a retried standby redoes it. - if resumeErr := hv.Resume(ctx); resumeErr != nil { + if resumeErr := resumeSnapshotAfterError(ctx, stored, hv); resumeErr != nil { log.ErrorContext(ctx, "failed to resume VM after shutdown error", "instance_id", id, "error", resumeErr) } return nil, fmt.Errorf("shutdown hypervisor: %w", err) @@ -215,7 +229,7 @@ func (m *manager) standbyInstance( // 9. Release network allocation (delete TAP device) // TAP devices with explicit Owner/Group fields do NOT auto-delete when VMM exits // They must be explicitly deleted - if inst.NetworkEnabled { + if inst.usesLinuxNetwork() { m.unregisterEgressProxyInstance(ctx, id) log.DebugContext(ctx, "releasing network", "instance_id", id, "network", "default") releaseNetworkCtx, releaseNetworkSpanEnd := m.startLifecycleStep(ctx, "release_network", diff --git a/lib/instances/start.go b/lib/instances/start.go index ea1b7e75d..2a34d625c 100644 --- a/lib/instances/start.go +++ b/lib/instances/start.go @@ -151,6 +151,14 @@ func (m *manager) startInstance( }) } } + if stored.MacOS != nil { + if err := m.checkMacOSMACAvailable(ctx, stored); err != nil { + return nil, err + } + if err := m.clearMacOSStopReceipt(stored); err != nil { + return nil, err + } + } // 4b. Recreate the vGPU if this instance had a GPU profile // Note: GPU availability was already validated in step 2b @@ -226,6 +234,10 @@ func (m *manager) startInstance( // Success - release cleanup stack (prevent cleanup) cu.Release() + if err := m.rekeyMacOSCloneOnStart(ctx, stored); err != nil { + return nil, err + } + // 7. Update metadata (set PID, StartedAt). Boot markers were cleared at // the top of this function, so we are in Initializing until they hydrate. stored.Phases.Record(initialBootPhase(stored), time.Now().UTC()) diff --git a/lib/instances/stop.go b/lib/instances/stop.go index 8cc602f07..3f096e88a 100644 --- a/lib/instances/stop.go +++ b/lib/instances/stop.go @@ -169,6 +169,7 @@ func (m *manager) stopInstance( attribute.String("operation", "graceful_guest_shutdown"), attribute.Int("stop_timeout_seconds", stopTimeout), ) + macOSVMMLive := inst.MacOS != nil && hypervisorProcessLive(&inst) gracefulShutdown := m.tryGracefulGuestShutdown(gracefulCtx, &inst, stopTimeout) if gracefulShutdown { gracefulSpanEnd(nil) @@ -285,6 +286,10 @@ func (m *manager) stopInstance( // 10. Update metadata (clear PID, set StoppedAt) now := time.Now().UTC() stored.StoppedAt = &now + if stored.MacOS != nil { + // A VMM that was already gone exits "gracefully" without a guest shutdown. + stored.StopGraceful = gracefulShutdown && macOSVMMLive + } stored.HypervisorProcessIdentity.Clear() // Boot markers are per-boot-run and must not carry across stop/restore/start. stored.ProgramStartedAt = nil diff --git a/lib/instances/types.go b/lib/instances/types.go index d280e3e1c..49c1b0446 100644 --- a/lib/instances/types.go +++ b/lib/instances/types.go @@ -94,6 +94,9 @@ type StoredMetadata struct { // Read-only; echoed on the instance API. Platform string MacOS *images.MacOSImage `json:"MacOS,omitempty"` // Non-nil for experimental local macOS disk guests. + // IdentityRekeyPending marks a macOS clone whose SSH host keys still come from its + // source. The next start rekeys them and clears the flag. + IdentityRekeyPending bool `json:"identity_rekey_pending,omitempty"` // Resources (matching Cloud Hypervisor terminology) Size int64 // Base memory in bytes @@ -121,6 +124,9 @@ type StoredMetadata struct { ExpiresAt *time.Time // Absolute expiration time; nil disables expiration StartedAt *time.Time // Boot epoch start time (set on create/start; preserved across standby restore) StoppedAt *time.Time // Last time VM was stopped + // StopGraceful records whether the last stop was an acknowledged guest shutdown with + // owned VMM exit. macOS stopped snapshots require it; a forced stop is not consistent. + StopGraceful bool `json:"stop_graceful,omitempty"` // Boot progress markers (derived from guest serial log sentinels and persisted) ProgramStartedAt *time.Time // Set when guest program handoff/start boundary is reached diff --git a/lib/instances/vm_config_validation.go b/lib/instances/vm_config_validation.go index 61639e9b4..8a04b7f41 100644 --- a/lib/instances/vm_config_validation.go +++ b/lib/instances/vm_config_validation.go @@ -42,6 +42,9 @@ func (m *manager) validateStoredVMConfig(starter hypervisor.VMStarter, snapshotK } func (m *manager) plannedStoredVMConfig(snapshotKind SnapshotKind, meta StoredMetadata) hypervisor.VMConfig { + if meta.MacOS != nil { + return m.macOSVMConfig(&Instance{StoredMetadata: meta}) + } hasVGPU := storedVGPUDevicePath(&meta) != "" || meta.GPUProfile != "" config := m.plannedVMConfig( meta.HotplugSize, diff --git a/lib/network/allocate.go b/lib/network/allocate.go index 112ee151b..02d9005bb 100644 --- a/lib/network/allocate.go +++ b/lib/network/allocate.go @@ -690,3 +690,14 @@ func GenerateTAPName(instanceID string) string { } return TAPPrefix + strings.ToLower(shortID) } + +// NewUniqueMAC returns a locally administered MAC that is not in used. Callers that +// assign MACs outside the network allocator (macOS vmnet) use it so both paths share +// one uniqueness rule. +func NewUniqueMAC(used []string) (string, error) { + usedMACs := make(map[string]bool, len(used)) + for _, mac := range used { + usedMACs[strings.ToLower(strings.TrimSpace(mac))] = true + } + return allocateUniqueMACFromSet(usedMACs, generateMAC) +} From 8e04e575269c1e9a98c633fdd9e93c26f9702625 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:01:26 +0000 Subject: [PATCH 2/7] Tighten macOS stop receipts and restored state - Grant the macOS stop receipt only from the shutdown attempt that saw the VMM exit. Drop the separate pre-stop liveness probe, and do not count a VMM that was already gone as a graceful stop. - Clear the receipt right before the VMM starts instead of at admission, so a start that fails earlier keeps it. - Clear the source's restart block when a stopped snapshot restores to Running. - Reject explicit compression on macOS standby instead of silently ignoring it, matching snapshot create. - Stop re-saving restored metadata after the macOS directory swap, which already committed it. - Drop the unused operation argument from admitGuestOperation. --- lib/instances/create.go | 8 +++++++ lib/instances/fork.go | 2 +- lib/instances/macos.go | 8 +------ lib/instances/macos_admission_test.go | 25 ++++++++++++++++++++ lib/instances/macos_test.go | 2 +- lib/instances/process_identity_linux_test.go | 18 ++++++++++++++ lib/instances/restore.go | 2 +- lib/instances/snapshot.go | 21 +++++++++------- lib/instances/snapshot_macos.go | 16 +++++++++++-- lib/instances/snapshot_macos_test.go | 18 ++++++++++++++ lib/instances/snapshot_schedule.go | 2 +- lib/instances/standby.go | 5 +++- lib/instances/start.go | 3 --- lib/instances/stop.go | 8 +++---- lib/instances/update.go | 2 +- 15 files changed, 109 insertions(+), 31 deletions(-) diff --git a/lib/instances/create.go b/lib/instances/create.go index 3b7e9b150..4b70b7e00 100644 --- a/lib/instances/create.go +++ b/lib/instances/create.go @@ -839,6 +839,14 @@ func (m *manager) startAndBootVM( return fmt.Errorf("build vm config: %w", err) } + if stored.MacOS != nil { + // Clear the stop receipt right before the guest can write to its disk. A start + // that fails earlier leaves the disk untouched and keeps the receipt. + if err := m.clearMacOSStopReceipt(stored); err != nil { + return err + } + } + // Start VM (handles process start, configuration, and boot) log.DebugContext(ctx, "starting VM", "instance_id", stored.Id, "hypervisor", stored.HypervisorType, "version", stored.HypervisorVersion) pid, hv, err := starter.StartVM(ctx, m.paths, stored.HypervisorVersion, stored.SocketPath, vmConfig) diff --git a/lib/instances/fork.go b/lib/instances/fork.go index 2c00348e6..f82666183 100644 --- a/lib/instances/fork.go +++ b/lib/instances/fork.go @@ -42,7 +42,7 @@ func (m *manager) forkInstance(ctx context.Context, id string, req ForkInstanceR if err != nil { return nil, "", false, err } - if err := m.admitGuestOperation(&meta.StoredMetadata, "fork"); err != nil { + if err := m.admitGuestOperation(&meta.StoredMetadata); err != nil { return nil, "", false, err } source := m.toInstance(ctx, meta) diff --git a/lib/instances/macos.go b/lib/instances/macos.go index a46603083..c61e451af 100644 --- a/lib/instances/macos.go +++ b/lib/instances/macos.go @@ -295,15 +295,9 @@ func guestAgentReadyExecutable(stored *StoredMetadata) string { // macOS guest, so a normal stop can record a graceful receipt and strict export waits. const macOSMinStopTimeout = 30 -// hypervisorProcessLive reports whether the instance's VMM process is still running. -func hypervisorProcessLive(inst *Instance) bool { - pid, err := resolveLiveHypervisorPID(inst.HypervisorProcessIdentity, inst.SocketPath) - return err == nil && pid != 0 -} - // admitGuestOperation applies the server's guest-type restriction. Operation-specific // admission follows at the caller, under the instance lock. -func (m *manager) admitGuestOperation(stored *StoredMetadata, _ string) error { +func (m *manager) admitGuestOperation(stored *StoredMetadata) error { return m.requireMacOSGuest(stored) } diff --git a/lib/instances/macos_admission_test.go b/lib/instances/macos_admission_test.go index c69ce0a5b..0ed718ad7 100644 --- a/lib/instances/macos_admission_test.go +++ b/lib/instances/macos_admission_test.go @@ -1,8 +1,12 @@ package instances import ( + "context" + "path/filepath" + "strings" "testing" + "github.com/kernel/hypeman/lib/hypervisor" "github.com/kernel/hypeman/lib/images" "github.com/kernel/hypeman/lib/paths" "github.com/stretchr/testify/require" @@ -40,3 +44,24 @@ func TestAssignMacOSCloneMACAvoidsStoredMACs(t *testing.T) { require.NoError(t, m.assignMacOSCloneMAC(&offline)) require.Empty(t, offline.MAC, "a clone without networking gets no MAC") } + +func TestRejectedMacOSStartKeepsStopReceipt(t *testing.T) { + p := paths.New(t.TempDir()) + m := &manager{paths: p} + require.NoError(t, m.ensureDirectories("mac")) + stored := StoredMetadata{ + Id: "mac", + DataDir: p.InstanceDir("mac"), + HypervisorType: hypervisor.TypeVZ, + SocketPath: filepath.Join(t.TempDir(), strings.Repeat("s", maxMacOSSocketPath)), + MacOS: &images.MacOSImage{}, + StopGraceful: true, + } + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: stored})) + + err := m.startAndBootVM(context.Background(), &stored, nil, nil, false) + require.ErrorIs(t, err, ErrInvalidRequest) + meta, err := m.loadMetadata("mac") + require.NoError(t, err) + require.True(t, meta.StopGraceful, "a start rejected at boot admission must keep the stop receipt") +} diff --git a/lib/instances/macos_test.go b/lib/instances/macos_test.go index 8d5f97531..6bf99636d 100644 --- a/lib/instances/macos_test.go +++ b/lib/instances/macos_test.go @@ -265,7 +265,7 @@ func TestMacOSPreservedIdentityAdmission(t *testing.T) { require.NoError(t, os.Remove(socket)) require.NoError(t, m.checkMacOSIdentityAvailable(context.Background(), &other)) require.NoError(t, m.checkMacOSMACAvailable(context.Background(), &other)) - require.NoError(t, (&manager{}).admitGuestOperation(&stored, "snapshot schedules")) + require.NoError(t, (&manager{}).admitGuestOperation(&stored)) } func TestMacOSSocketPathLimit(t *testing.T) { diff --git a/lib/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index 35c70822d..de33190bd 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -19,6 +19,7 @@ import ( "github.com/kernel/hypeman/lib/devices" "github.com/kernel/hypeman/lib/hypervisor" + "github.com/kernel/hypeman/lib/images" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -738,3 +739,20 @@ func TestKillHypervisorHardKillsNonVGPUHypervisor(t *testing.T) { assert.ErrorIs(t, syscall.Kill(pid, 0), syscall.ESRCH) assert.NoFileExists(t, markerPath, "non-vGPU hypervisors keep the direct SIGKILL path") } + +func TestMacOSGracefulShutdownWithoutLiveVMMIsNotGraceful(t *testing.T) { + inst := &Instance{StoredMetadata: StoredMetadata{ + Id: "graceful-macos-no-vmm", + HypervisorType: hypervisor.TypeCloudHypervisor, + MacOS: &images.MacOSImage{GuestAgent: true}, + VsockSocket: filepath.Join(t.TempDir(), "missing-vsock.sock"), + }} + sent := false + m := &manager{shutdownGuestFn: func(context.Context, hypervisor.VsockDialer, int32) error { + sent = true + return nil + }} + assert.False(t, m.tryGracefulGuestShutdown(context.Background(), inst, 1), + "a macOS stop with no VMM to signal must not produce a graceful receipt") + assert.False(t, sent) +} diff --git a/lib/instances/restore.go b/lib/instances/restore.go index 9c93b6608..effb8d17c 100644 --- a/lib/instances/restore.go +++ b/lib/instances/restore.go @@ -42,7 +42,7 @@ func (m *manager) restoreInstance( log.ErrorContext(ctx, "failed to load instance metadata", "instance_id", id, "error", err) return nil, err } - if err := m.admitGuestOperation(&meta.StoredMetadata, "restore"); err != nil { + if err := m.admitGuestOperation(&meta.StoredMetadata); err != nil { return nil, err } release, err := m.admitMacOSBoot(ctx, &meta.StoredMetadata) diff --git a/lib/instances/snapshot.go b/lib/instances/snapshot.go index 2063e6431..a7f13bd57 100644 --- a/lib/instances/snapshot.go +++ b/lib/instances/snapshot.go @@ -63,7 +63,7 @@ func (m *manager) createSnapshot(ctx context.Context, id string, req CreateSnaps if err != nil { return nil, err } - if err := m.admitGuestOperation(&meta.StoredMetadata, "snapshot"); err != nil { + if err := m.admitGuestOperation(&meta.StoredMetadata); err != nil { return nil, err } if err := validateMacOSSnapshotCreate(meta, req); err != nil { @@ -271,7 +271,7 @@ func (m *manager) restoreSnapshot(ctx context.Context, id string, snapshotID str if err := validateMacOSSnapshotRestore(sourceMeta, rec); err != nil { return nil, err } - if err := m.admitGuestOperation(&sourceMeta.StoredMetadata, "restore snapshot"); err != nil { + if err := m.admitGuestOperation(&sourceMeta.StoredMetadata); err != nil { return nil, err } sourceInst := m.toInstance(ctx, sourceMeta) @@ -347,15 +347,18 @@ func (m *manager) restoreSnapshot(ctx context.Context, id string, snapshotID str } } prepareMacOSSnapshotRestoreMetadata(&restored, &sourceMeta.StoredMetadata, rec.Snapshot.Kind, targetState) + // The staged metadata is the committed record. Writing it again after the + // swap could fail a restore that already happened. if err := m.replaceMacOSInstanceWithSnapshotPayload(snapshotID, &metadata{StoredMetadata: restored}); err != nil { return nil, err } - } else if err := m.replaceInstanceWithSnapshotPayload(snapshotID, id); err != nil { - return nil, err - } - - if err := m.saveMetadata(&metadata{StoredMetadata: restored}); err != nil { - return nil, fmt.Errorf("save restored metadata: %w", err) + } else { + if err := m.replaceInstanceWithSnapshotPayload(snapshotID, id); err != nil { + return nil, err + } + if err := m.saveMetadata(&metadata{StoredMetadata: restored}); err != nil { + return nil, fmt.Errorf("save restored metadata: %w", err) + } } switch rec.Snapshot.Kind { @@ -408,7 +411,7 @@ func (m *manager) forkSnapshot(ctx context.Context, snapshotID string, req ForkS if err != nil { return nil, err } - if err := m.admitGuestOperation(&rec.StoredMetadata, "fork snapshot"); err != nil { + if err := m.admitGuestOperation(&rec.StoredMetadata); err != nil { return nil, err } if err := validateMacOSSnapshotFork(rec); err != nil { diff --git a/lib/instances/snapshot_macos.go b/lib/instances/snapshot_macos.go index 7fe34c79e..db66cf4de 100644 --- a/lib/instances/snapshot_macos.go +++ b/lib/instances/snapshot_macos.go @@ -18,6 +18,7 @@ import ( "github.com/kernel/hypeman/lib/instances/phasetracking" "github.com/kernel/hypeman/lib/paths" restartpolicy "github.com/kernel/hypeman/lib/restart-policy" + "github.com/kernel/hypeman/lib/snapshot" ) func validateMacOSSnapshotCreate(meta *metadata, req CreateSnapshotRequest) error { @@ -25,8 +26,8 @@ func validateMacOSSnapshotCreate(meta *metadata, req CreateSnapshotRequest) erro return nil } if req.Kind == SnapshotKindStandby { - if req.Compression != nil && req.Compression.Enabled { - return fmt.Errorf("%w: macOS memory snapshots do not support compression", ErrInvalidRequest) + if err := validateMacOSMemoryCompression(req.Compression); err != nil { + return err } return validateMacOSMemorySnapshotSupport(&meta.StoredMetadata) } @@ -39,6 +40,15 @@ func validateMacOSSnapshotCreate(meta *metadata, req CreateSnapshotRequest) erro return nil } +// validateMacOSMemoryCompression rejects an explicit compression request. macOS saved +// state is never compressed, so accepting one would silently drop it. +func validateMacOSMemoryCompression(compression *snapshot.SnapshotCompressionConfig) error { + if compression != nil && compression.Enabled { + return fmt.Errorf("%w: macOS memory snapshots do not support compression", ErrInvalidRequest) + } + return nil +} + func validateMacOSMemorySnapshotSupport(stored *StoredMetadata) error { if stored.MacOS == nil { return nil @@ -115,6 +125,8 @@ func prepareMacOSSnapshotRestoreMetadata(restored *StoredMetadata, source *Store } } restored.Phases.Record(phase, time.Now().UTC()) + // A restored VM is a new run, so it must not inherit the source's restart block. + restored.RestartStatus = restartpolicy.Status{} if target == StateStopped { restored.RestartStatus = restartpolicy.Status{BlockedReason: restartpolicy.BlockedReasonManualStop} } diff --git a/lib/instances/snapshot_macos_test.go b/lib/instances/snapshot_macos_test.go index dbc8526d9..7fc480ae7 100644 --- a/lib/instances/snapshot_macos_test.go +++ b/lib/instances/snapshot_macos_test.go @@ -8,6 +8,8 @@ import ( "github.com/kernel/hypeman/lib/images" "github.com/kernel/hypeman/lib/paths" + restartpolicy "github.com/kernel/hypeman/lib/restart-policy" + "github.com/kernel/hypeman/lib/snapshot" "github.com/stretchr/testify/require" ) @@ -153,3 +155,19 @@ func TestMacOSRestoreRecoveryKeepsBackupWhenInstanceStateIsUnknown(t *testing.T) require.NoError(t, err, "the parked backup must survive an inconclusive check") require.Equal(t, "previous", string(data)) } + +func TestMacOSRestoreClearsSourceRestartBlock(t *testing.T) { + restored := StoredMetadata{RestartStatus: restartpolicy.Status{BlockedReason: restartpolicy.BlockedReasonManualStop}} + source := StoredMetadata{} + prepareMacOSSnapshotRestoreMetadata(&restored, &source, SnapshotKindStopped, StateRunning) + require.True(t, restored.RestartStatus.IsZero(), "a running restore must not inherit the source's manual-stop block") + + prepareMacOSSnapshotRestoreMetadata(&restored, &source, SnapshotKindStopped, StateStopped) + require.Equal(t, restartpolicy.BlockedReasonManualStop, restored.RestartStatus.BlockedReason) +} + +func TestMacOSMemoryCompressionRejectsEnabledOnly(t *testing.T) { + require.NoError(t, validateMacOSMemoryCompression(nil)) + require.NoError(t, validateMacOSMemoryCompression(&snapshot.SnapshotCompressionConfig{Enabled: false})) + require.ErrorIs(t, validateMacOSMemoryCompression(&snapshot.SnapshotCompressionConfig{Enabled: true}), ErrInvalidRequest) +} diff --git a/lib/instances/snapshot_schedule.go b/lib/instances/snapshot_schedule.go index 4c01a9037..7aa240ea4 100644 --- a/lib/instances/snapshot_schedule.go +++ b/lib/instances/snapshot_schedule.go @@ -40,7 +40,7 @@ func (m *manager) SetSnapshotSchedule(ctx context.Context, instanceID string, re if err != nil { return nil, err } - if err := m.admitGuestOperation(&meta.StoredMetadata, "snapshot schedules"); err != nil { + if err := m.admitGuestOperation(&meta.StoredMetadata); err != nil { return nil, err } if err := m.validateMacOSSnapshotSchedule(ctx, meta); err != nil { diff --git a/lib/instances/standby.go b/lib/instances/standby.go index 46e761464..a674baf3c 100644 --- a/lib/instances/standby.go +++ b/lib/instances/standby.go @@ -42,10 +42,13 @@ func (m *manager) standbyInstance( log.ErrorContext(ctx, "failed to load instance metadata", "instance_id", id, "error", err) return nil, err } - if err := m.admitGuestOperation(&meta.StoredMetadata, "standby"); err != nil { + if err := m.admitGuestOperation(&meta.StoredMetadata); err != nil { return nil, err } if meta.MacOS != nil { + if err := validateMacOSMemoryCompression(req.Compression); err != nil { + return nil, err + } // macOS saved state is not compressed. The framework already omits zero pages, // and compression would add time inside the paused window. skipCompression = true diff --git a/lib/instances/start.go b/lib/instances/start.go index 2a34d625c..2fd3fd236 100644 --- a/lib/instances/start.go +++ b/lib/instances/start.go @@ -155,9 +155,6 @@ func (m *manager) startInstance( if err := m.checkMacOSMACAvailable(ctx, stored); err != nil { return nil, err } - if err := m.clearMacOSStopReceipt(stored); err != nil { - return nil, err - } } // 4b. Recreate the vGPU if this instance had a GPU profile diff --git a/lib/instances/stop.go b/lib/instances/stop.go index 3f096e88a..f99753e97 100644 --- a/lib/instances/stop.go +++ b/lib/instances/stop.go @@ -65,7 +65,9 @@ func (m *manager) tryGracefulGuestShutdown(ctx context.Context, inst *Instance, return false } if pid == 0 { - return true + // Nothing was signalled. Linux treats an already-gone VMM as stopped, but + // macOS only counts a stop as graceful after a shutdown attempt. + return inst.MacOS == nil } inst.HypervisorProcessIdentity.Set(pid) @@ -169,7 +171,6 @@ func (m *manager) stopInstance( attribute.String("operation", "graceful_guest_shutdown"), attribute.Int("stop_timeout_seconds", stopTimeout), ) - macOSVMMLive := inst.MacOS != nil && hypervisorProcessLive(&inst) gracefulShutdown := m.tryGracefulGuestShutdown(gracefulCtx, &inst, stopTimeout) if gracefulShutdown { gracefulSpanEnd(nil) @@ -287,8 +288,7 @@ func (m *manager) stopInstance( now := time.Now().UTC() stored.StoppedAt = &now if stored.MacOS != nil { - // A VMM that was already gone exits "gracefully" without a guest shutdown. - stored.StopGraceful = gracefulShutdown && macOSVMMLive + stored.StopGraceful = gracefulShutdown } stored.HypervisorProcessIdentity.Clear() // Boot markers are per-boot-run and must not carry across stop/restore/start. diff --git a/lib/instances/update.go b/lib/instances/update.go index 55249fa38..ca15bfd00 100644 --- a/lib/instances/update.go +++ b/lib/instances/update.go @@ -27,7 +27,7 @@ func (m *manager) updateInstance(ctx context.Context, id string, req UpdateInsta log.ErrorContext(ctx, "failed to load instance metadata", "instance_id", id, "error", err) return nil, err } - if err := m.admitGuestOperation(&meta.StoredMetadata, "update"); err != nil { + if err := m.admitGuestOperation(&meta.StoredMetadata); err != nil { return nil, err } if err := checkMacOSUpdate(&meta.StoredMetadata, req); err != nil { From 27035766e30a6db7b85a53060b7879f8f5fbbfd7 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:11:47 +0000 Subject: [PATCH 3/7] Trim macOS snapshot docs and merge identity refusal tests Restate the snapshot guard as a single save call so the Linux restore branch keeps its original indentation. --- docs/macos-snapshots.md | 149 +++++++++++++-------------- lib/instances/macos_identity_test.go | 23 ++--- lib/instances/snapshot.go | 13 +-- 3 files changed, 90 insertions(+), 95 deletions(-) diff --git a/docs/macos-snapshots.md b/docs/macos-snapshots.md index 8ed3d1d43..28d7c4e10 100644 --- a/docs/macos-snapshots.md +++ b/docs/macos-snapshots.md @@ -1,9 +1,9 @@ # macOS snapshots, schedules and restore (PR 4, draft) -Supported experimentally: stopped disk snapshots, same-instance memory checkpoints, -snapshot schedules, and cold forks from gracefully stopped instances or snapshots. -Independent warm memory forks remain unsupported. Native validation of the new -memory-snapshot and schedule paths is still required. +Experimental: stopped disk snapshots, same-instance memory checkpoints, snapshot +schedules, and cold forks from gracefully stopped instances or snapshots. +Independent warm memory forks are unsupported. Native validation of the memory +snapshot and schedule paths is still required. ## Stopped snapshots and cold forks @@ -16,82 +16,79 @@ memory-snapshot and schedule paths is still required. Disk and auxiliary storage are copied, the clone gets a fresh machine identifier and MAC, and SSH host keys are rekeyed on first boot. The default target is Stopped; Running cold-boots and completes rekeying before returning. -- Forking a stopped instance uses the same identity/rekey contract. Hostnames, - account credentials and application-specific identities are not rekeyed. -- Failed rekeying stops the clone and leaves rekey pending for a later retry. +- Forking a stopped instance follows the same identity and rekey contract. Hostnames, + account credentials and application identities are not rekeyed. +- A failed rekey stops the clone and leaves rekey pending for a later retry. ## Same-instance memory snapshots -Use `POST /instances/{id}/snapshots` with `{"kind":"Standby"}`. - -- A Running source goes through pause, VZ machine-state save and confirmed VMM - shutdown. The disk, auxiliary storage and machine state are copied together - while the VMM is down; then the source resumes from its own checkpoint. -- A Standby source is copied without being resumed. Stopped and other source - states are rejected with 409. -- Before pausing, the shim validates the exact VM configuration using Apple's - `validateSaveRestoreSupport`. Host/architecture capability checks alone are - not proof that the guest's configured devices support save/restore. -- Pause waits for raw Paused state, not transitional Pausing state, before save. - Error-path resume and source recovery use bounded cleanup contexts that survive - cancellation of the snapshot request. If recovery fails, the API reports the - failure; the source may need explicit recovery. -- VZ machine-state files stay uncompressed. Explicit enabled compression is - rejected before capture; Linux server compression defaults do not apply. - -Restore using `POST /instances/{id}/snapshots/{snapshot_id}/restore`. - -- This is a same-instance operation, not a clone. Restore requires an inactive - Stopped or Standby source; Running, Paused and other live states are refused. -- The default target for a memory snapshot is Running. Explicit Standby installs - the checkpoint without resuming it. Stopped discards memory and does not create - a graceful-shutdown receipt; a subsequent clean boot/stop is required before - making a stopped snapshot or cold fork. -- Preflight checks saved CPU/RAM, hardware model, machine identifier, NIC MAC, - disk/auxiliary paths and the complete memory/disk/auxiliary bundle before - replacing source storage. Invalid payloads leave current storage untouched. -- Restored metadata is staged with the payload, including the current instance - name and expiration. Directory-swap recovery therefore commits or rolls back - the matching metadata and storage together. Host phase accounting is preserved, - rather than replayed from the checkpoint; memory restore retains its boot time. - An explicit Stopped target suppresses automatic restart. -- Restoring never mutates the persistent snapshot. Framework restore failure - after installation leaves the restored checkpoint available in Standby. -- If the API dies during a restore, the shim it started keeps loading the VM. - Until it opens its socket the instance reads Standby, and start or restore - return 409 while that shim runs. Once loaded it reads Paused: `POST /restore` - resumes it and completes the restore (records the shim, removes the consumed - checkpoint), and DELETE kills it. As on Linux, stop does not accept Paused. -- Saved state is not portable: use the originating Mac and OS account, with a - compatible framework version and VM configuration. API availability on macOS - 14+ does not guarantee support for a particular macOS guest configuration. +`POST /instances/{id}/snapshots` with `{"kind":"Standby"}`. + +- A Running source is paused, its VZ machine state saved, and the VMM shut down + with confirmed exit. The disk, auxiliary storage and machine state are copied + while the VMM is down, then the source resumes from its own checkpoint. +- A Standby source is copied without being resumed. Stopped and other states are + rejected with 409. +- Before pausing, the shim validates the exact VM configuration with Apple's + `validateSaveRestoreSupport`. Host and architecture capability checks alone do + not prove that the guest's devices support save and restore. +- Pause waits for raw Paused state, not transitional Pausing, before save. Error-path + resume and source recovery use bounded cleanup contexts that survive cancellation + of the request. If recovery fails, the API reports it and the source may need + explicit recovery. +- VZ machine-state files are uncompressed. Explicit enabled compression is rejected + before capture; Linux compression defaults do not apply. + +Restore with `POST /instances/{id}/snapshots/{snapshot_id}/restore`. + +- Same-instance only, not a clone. The source must be inactive (Stopped or Standby); + Running, Paused and other live states are refused. +- The default target for a memory snapshot is Running. Explicit Standby installs the + checkpoint without resuming it. Stopped discards memory and creates no graceful + receipt, so a clean boot and stop are needed before a stopped snapshot or cold fork. +- Preflight checks CPU/RAM, hardware model, machine identifier, NIC MAC, disk and + auxiliary paths, and the full memory/disk/auxiliary bundle before storage is + replaced. An invalid payload leaves current storage untouched. +- Restored metadata is staged with the payload, including the current name and + expiration, so a directory-swap recovery commits or rolls back metadata and + storage together. Host phase accounting is preserved rather than replayed from the + checkpoint; a memory restore keeps its boot time. An explicit Stopped target + suppresses automatic restart. +- A restore never mutates the persistent snapshot. If the framework restore fails + after installation, the restored checkpoint remains available in Standby. +- If the API dies during a restore, the shim it started keeps loading the VM. Until + it opens its socket the instance reads Standby, and start or restore return 409 + while that shim runs. Once loaded it reads Paused: `POST /restore` resumes it and + completes the restore (records the shim, removes the consumed checkpoint), and + DELETE kills it. As on Linux, stop does not accept Paused. +- Saved state is not portable. Use the originating Mac and OS account, a compatible + framework version and the same VM configuration. API availability on macOS 14+ + does not guarantee support for a particular guest configuration. ## Schedules -The existing snapshot-schedule API, interval handling and retention policies are -shared with Linux. macOS schedules require host save/restore support; admission -also probes the exact configuration when a live VM is available. +Schedules reuse the existing snapshot-schedule API, interval handling and retention. +They require host save/restore support, and admission probes the exact configuration +when a live VM is available. -- A gracefully Stopped source produces a stopped snapshot. -- A Running source produces a memory snapshot and is resumed afterwards. -- A Standby source produces a memory snapshot and remains in Standby. -- Ineligible state, absent graceful receipt, configuration or capture failure is - recorded in `last_error`; no successful snapshot ID is reported for that run. -- Failed runs consume the interval, matching existing scheduler behavior. The - next run follows the configured cadence rather than tight retries. -- Retention and schedule deletion use the existing controller. Scheduling does - not stop a running guest to manufacture a graceful disk snapshot. +- A gracefully Stopped source produces a stopped snapshot. A Running source produces + a memory snapshot and is resumed afterwards. A Standby source produces a memory + snapshot and stays in Standby. +- An ineligible state, a missing graceful receipt, or a configuration or capture + failure is recorded in `last_error`, and no snapshot ID is reported for that run. +- Failed runs consume the interval, matching the existing scheduler. The next run + follows the configured cadence rather than retrying tightly. +- Scheduling never stops a running guest to manufacture a graceful disk snapshot. ## Warm memory forks are not supported -`ForkSnapshot` rejects macOS memory snapshots with 501, and `ForkInstance` -requires a gracefully stopped macOS source. VZ restores saved state only into an -identical configuration: on Apple silicon, changing the MAC, the machine -identifier, the NIC or the auxiliary storage fails with "invalid argument", while -relocating paths works. Apple documents that two VMs running concurrently with -the same machine identifier have undefined guest behavior, so copies of one saved -state cannot run side by side. Repeated same-instance restore is the supported -warm start. +`ForkSnapshot` rejects macOS memory snapshots with 501, and `ForkInstance` requires a +gracefully stopped macOS source. VZ restores saved state only into an identical +configuration: on Apple silicon, changing the MAC, machine identifier, NIC or +auxiliary storage fails with "invalid argument", while relocating paths works. +Apple documents that concurrent VMs sharing a machine identifier have undefined guest +behavior, so copies of one saved state cannot run side by side. Repeated same-instance +restore is the supported warm start. References: - [Apple: Mac platform configuration](https://developer.apple.com/documentation/virtualization/vzmacplatformconfiguration) @@ -99,9 +96,9 @@ References: ## Validation -Linux-runnable tests cover the filesystem and metadata paths (capture, restore, -isolation, manifest checks, schedules, retention and crash-swap recovery); mock -HTTP fixtures cover VZ pause, cancellation and configuration probing. Native -Apple-silicon runs have exercised Running capture/resume, same-instance memory -rollback, repeated restore, schedules with retention, cancellation after pause, -API crash during restore, and stopped clones with fresh identities. +Linux-runnable tests cover capture, restore, isolation, manifest checks, schedules, +retention and crash-swap recovery. Mock HTTP fixtures cover VZ pause, cancellation +and configuration probing. Native Apple-silicon runs have exercised Running capture +and resume, same-instance rollback, repeated restore, schedules with retention, +cancellation after pause, API crash during restore, and stopped clones with fresh +identities. diff --git a/lib/instances/macos_identity_test.go b/lib/instances/macos_identity_test.go index d5e950794..2978e7da1 100644 --- a/lib/instances/macos_identity_test.go +++ b/lib/instances/macos_identity_test.go @@ -42,19 +42,16 @@ func TestPrepareMacOSCloneRebindsIdentityWithoutAliasingSource(t *testing.T) { require.False(t, source.IdentityRekeyPending) } -func TestPrepareMacOSCloneRefusesGuestWithoutAgent(t *testing.T) { - clone := testMacOSSource() - clone.MacOS.GuestAgent = false - err := prepareMacOSClone(clone) - require.ErrorIs(t, err, ErrInvalidRequest) - require.False(t, clone.IdentityRekeyPending) -} - -func TestPrepareMacOSCloneRefusesSkippedAgent(t *testing.T) { - clone := testMacOSSource() - clone.SkipGuestAgent = true - require.ErrorIs(t, prepareMacOSClone(clone), ErrInvalidRequest) - require.False(t, clone.IdentityRekeyPending) +func TestPrepareMacOSCloneRefusesWithoutGuestAgent(t *testing.T) { + for _, disable := range []func(*StoredMetadata){ + func(s *StoredMetadata) { s.MacOS.GuestAgent = false }, + func(s *StoredMetadata) { s.SkipGuestAgent = true }, + } { + clone := testMacOSSource() + disable(clone) + require.ErrorIs(t, prepareMacOSClone(clone), ErrInvalidRequest) + require.False(t, clone.IdentityRekeyPending) + } } func TestNewUniqueMACAvoidsUsedAddresses(t *testing.T) { diff --git a/lib/instances/snapshot.go b/lib/instances/snapshot.go index a7f13bd57..5440da40c 100644 --- a/lib/instances/snapshot.go +++ b/lib/instances/snapshot.go @@ -347,15 +347,16 @@ func (m *manager) restoreSnapshot(ctx context.Context, id string, snapshotID str } } prepareMacOSSnapshotRestoreMetadata(&restored, &sourceMeta.StoredMetadata, rec.Snapshot.Kind, targetState) - // The staged metadata is the committed record. Writing it again after the - // swap could fail a restore that already happened. if err := m.replaceMacOSInstanceWithSnapshotPayload(snapshotID, &metadata{StoredMetadata: restored}); err != nil { return nil, err } - } else { - if err := m.replaceInstanceWithSnapshotPayload(snapshotID, id); err != nil { - return nil, err - } + } else if err := m.replaceInstanceWithSnapshotPayload(snapshotID, id); err != nil { + return nil, err + } + + // The macOS staged metadata is already the committed record. Writing it again + // after the swap could fail a restore that already happened. + if restored.MacOS == nil { if err := m.saveMetadata(&metadata{StoredMetadata: restored}); err != nil { return nil, fmt.Errorf("save restored metadata: %w", err) } From 2e843eace19afee9caedd76b478282a075d3e8e8 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Thu, 22 Oct 2026 18:33:47 +0000 Subject: [PATCH 4/7] Fix restore, clone rekey and standby recovery review findings - A restore retried while the shim is still loading the VM returns 409 and keeps the checkpoint. The checkpoint is consumed only once the VM answers on its control socket. - Start finishes a pending clone rekey before it returns for a Running or Initializing clone, so a start retried after an API exit cannot report success with the source's SSH host keys still in place. - A snapshot whose standby fails after the VMM stopped restores the source before the error is returned, instead of leaving it in Standby. This applies to Linux as well. - Correct the docs: a shim still loading reads Initializing, and start returns the current instance rather than 409. --- docs/macos-snapshots.md | 5 ++- lib/instances/macos.go | 11 ++++++ lib/instances/manager.go | 13 ++++++ lib/instances/snapshot.go | 17 +++++++- lib/instances/snapshot_macos_test.go | 59 ++++++++++++++++++++++++++++ 5 files changed, 102 insertions(+), 3 deletions(-) diff --git a/docs/macos-snapshots.md b/docs/macos-snapshots.md index 28d7c4e10..00b082524 100644 --- a/docs/macos-snapshots.md +++ b/docs/macos-snapshots.md @@ -57,8 +57,9 @@ Restore with `POST /instances/{id}/snapshots/{snapshot_id}/restore`. - A restore never mutates the persistent snapshot. If the framework restore fails after installation, the restored checkpoint remains available in Standby. - If the API dies during a restore, the shim it started keeps loading the VM. Until - it opens its socket the instance reads Standby, and start or restore return 409 - while that shim runs. Once loaded it reads Paused: `POST /restore` resumes it and + it opens its socket the instance reads Initializing. Restore returns 409 while + that shim has not resumed the VM, and keeps the checkpoint. Once loaded it reads + Paused: `POST /restore` resumes it and completes the restore (records the shim, removes the consumed checkpoint), and DELETE kills it. As on Linux, stop does not accept Paused. - Saved state is not portable. Use the originating Mac and OS account, a compatible diff --git a/lib/instances/macos.go b/lib/instances/macos.go index c61e451af..4c6d9dd1f 100644 --- a/lib/instances/macos.go +++ b/lib/instances/macos.go @@ -396,6 +396,17 @@ func (m *manager) macOSStateWithoutShim(stored *StoredMetadata) stateResult { return stateResult{State: StateStopped} } +// macOSSocketAnswers reports whether a macOS guest's control socket answers a query. +// A shim still loading the VM has not opened it yet. +func (m *manager) macOSSocketAnswers(ctx context.Context, inst *Instance) bool { + hv, err := m.getHypervisor(inst.SocketPath, inst.HypervisorType) + if err != nil { + return false + } + _, err = hv.GetVMInfo(ctx) + return err == nil +} + // findVZShimPIDs lists vz-shim processes by control socket. Tests replace it. var findVZShimPIDs = hypervisor.VZShimPIDs diff --git a/lib/instances/manager.go b/lib/instances/manager.go index 70515a576..8dfa302f8 100644 --- a/lib/instances/manager.go +++ b/lib/instances/manager.go @@ -650,6 +650,11 @@ func (m *manager) RestoreInstance(ctx context.Context, id string) (*Instance, er // A running macOS guest has consumed its checkpoint. One still on disk means an // earlier resume failed before removing it, so finish that resume now. if current.MacOS != nil && current.HasSnapshot { + // Initializing without a control socket is a shim still loading the VM. It has + // not resumed, so the checkpoint it will consume must stay. + if current.State == StateInitializing && !m.macOSSocketAnswers(ctx, current) { + return nil, fmt.Errorf("%w: macOS restore is still loading the VM; retry shortly", ErrInvalidState) + } if err := m.finishMacOSResume(id); err != nil { return nil, fmt.Errorf("finish resumed restore: %w", err) } @@ -740,6 +745,14 @@ func (m *manager) StartInstance(ctx context.Context, id string, req StartInstanc return nil, err } if current.State == StateRunning || current.State == StateInitializing { + // The API can exit after a clone boots and before its rekey finishes. Finish it + // here, or the clone keeps its source's SSH host keys. + if current.IdentityRekeyPending { + if err := m.rekeyMacOSCloneOnStart(ctx, ¤t.StoredMetadata); err != nil { + return nil, err + } + return m.currentInstanceWithoutHydration(ctx, id) + } return current, nil } } diff --git a/lib/instances/snapshot.go b/lib/instances/snapshot.go index 5440da40c..f395dccdc 100644 --- a/lib/instances/snapshot.go +++ b/lib/instances/snapshot.go @@ -45,6 +45,21 @@ func (m *manager) getSnapshot(ctx context.Context, snapshotID string) (*Snapshot return snapshot, nil } +// recoverSourceAfterFailedStandby returns standbyErr after putting the source back if +// standby failed after its VMM was saved and stopped. Otherwise the failed snapshot +// would leave a running guest in Standby. A failure before the VMM stops leaves the +// source Running and needs no recovery. +func (m *manager) recoverSourceAfterFailedStandby(ctx context.Context, stored *StoredMetadata, standbyErr error) error { + inst, err := m.currentInstanceWithoutHydration(ctx, stored.Id) + if err != nil || inst.State != StateStandby { + return standbyErr + } + if restoreErr := m.restoreSnapshotSource(ctx, stored); restoreErr != nil { + return fmt.Errorf("%w; additionally failed to restore source: %v", standbyErr, restoreErr) + } + return standbyErr +} + func (m *manager) createSnapshot(ctx context.Context, id string, req CreateSnapshotRequest) (_ *Snapshot, retErr error) { log := logger.FromContext(ctx) log.InfoContext(ctx, "creating snapshot", "instance_id", id, "kind", req.Kind, "name", req.Name) @@ -106,7 +121,7 @@ func (m *manager) createSnapshot(ctx context.Context, id string, req CreateSnaps return nil, err } if _, err := m.standbyInstance(ctx, id, StandbyInstanceRequest{}, true); err != nil { - return nil, fmt.Errorf("standby source instance: %w", err) + return nil, m.recoverSourceAfterFailedStandby(ctx, stored, fmt.Errorf("standby source instance: %w", err)) } restoreSource = true case StateStandby: diff --git a/lib/instances/snapshot_macos_test.go b/lib/instances/snapshot_macos_test.go index 7fc480ae7..2ff862b14 100644 --- a/lib/instances/snapshot_macos_test.go +++ b/lib/instances/snapshot_macos_test.go @@ -1,11 +1,15 @@ package instances import ( + "context" + "errors" "os" + "os/exec" "path/filepath" "testing" "time" + "github.com/kernel/hypeman/lib/hypervisor" "github.com/kernel/hypeman/lib/images" "github.com/kernel/hypeman/lib/paths" restartpolicy "github.com/kernel/hypeman/lib/restart-policy" @@ -171,3 +175,58 @@ func TestMacOSMemoryCompressionRejectsEnabledOnly(t *testing.T) { require.NoError(t, validateMacOSMemoryCompression(&snapshot.SnapshotCompressionConfig{Enabled: false})) require.ErrorIs(t, validateMacOSMemoryCompression(&snapshot.SnapshotCompressionConfig{Enabled: true}), ErrInvalidRequest) } + +func TestRestoreKeepsCheckpointWhileShimLoads(t *testing.T) { + m, meta := macOSMemorySnapshotFixture(t) + writeMacOSMemorySnapshotPayload(t, m, meta.DataDir, &meta.StoredMetadata) + // A live shim that has not opened its control socket yet. + shim := exec.Command("sleep", "60") + require.NoError(t, shim.Start()) + t.Cleanup(func() { shim.Process.Kill(); shim.Wait() }) + prev := findVZShimPIDs + findVZShimPIDs = func(string) ([]int, error) { return []int{shim.Process.Pid}, nil } + t.Cleanup(func() { findVZShimPIDs = prev }) + + _, err := m.RestoreInstance(context.Background(), meta.Id) + require.ErrorIs(t, err, ErrInvalidState) + require.DirExists(t, m.paths.InstanceSnapshotLatest(meta.Id), "the checkpoint is consumed only after the VM resumes") +} + +func TestStartRetryFinishesCloneRekey(t *testing.T) { + m, meta := macOSMemorySnapshotFixture(t) + meta.HypervisorType = lifecycleNoopHypervisorType + meta.SkipGuestAgent = false + meta.MacOS.GuestAgent = true + meta.IdentityRekeyPending = true + require.NoError(t, os.WriteFile(meta.SocketPath, nil, 0600)) + lifecycleNoopHypervisorStates.Store(meta.SocketPath, hypervisor.StateRunning) + t.Cleanup(func() { lifecycleNoopHypervisorStates.Delete(meta.SocketPath) }) + require.NoError(t, m.saveMetadata(meta)) + + // The fixture has no guest agent, so the rekey retries until its context ends. The + // retry must report that failure rather than return a clone still carrying its + // source's SSH host keys. + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + _, err := m.StartInstance(ctx, meta.Id, StartInstanceRequest{}) + require.Error(t, err) + reloaded, err := m.loadMetadata(meta.Id) + require.NoError(t, err) + require.True(t, reloaded.IdentityRekeyPending, "a failed rekey keeps the flag for the next start") +} + +func TestRecoverSourceAfterFailedStandby(t *testing.T) { + standbyErr := errors.New("save metadata") + + // A checkpoint with the VM down is the state a late standby failure leaves behind. + m, meta := macOSMemorySnapshotFixture(t) + writeMacOSMemorySnapshotPayload(t, m, meta.DataDir, &meta.StoredMetadata) + require.ErrorIs(t, m.recoverSourceAfterFailedStandby(context.Background(), &meta.StoredMetadata, standbyErr), standbyErr) + inst, err := m.GetInstance(context.Background(), meta.Id) + require.NoError(t, err) + require.Equal(t, StateRunning, inst.State, "the source must not stay in Standby after a failed snapshot") + + // Without a checkpoint the VM never stopped, so there is nothing to recover. + m2, meta2 := macOSMemorySnapshotFixture(t) + require.Equal(t, standbyErr, m2.recoverSourceAfterFailedStandby(context.Background(), &meta2.StoredMetadata, standbyErr)) +} From edb1be83c4a0c390c7901d7e1e951ec49ea36c84 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Thu, 22 Oct 2026 18:42:34 +0000 Subject: [PATCH 5/7] Recover standby source from the record standby reached A standby whose final metadata save fails has already stopped the VMM, but the disk copy still predates standby. Recovery restored the source from that stale copy: a Firecracker restore could reuse the previous cycle's cache key, and the standby phase was never recorded, so Running time kept accruing across the failed standby. - standbyInstance returns the record it reached when the save fails after the VMM has stopped. - recoverSourceAfterFailedStandby saves that record before restoring the source. If the save fails, the error reports that the source is left in Standby. - Apply the same recovery to the public standby request and to fork from a running source, which had the same late-failure gap. --- lib/instances/fork.go | 4 +-- lib/instances/manager.go | 7 ++-- lib/instances/snapshot.go | 22 +++++++----- lib/instances/snapshot_macos_test.go | 50 +++++++++++++++++++++++++--- lib/instances/standby.go | 4 ++- 5 files changed, 67 insertions(+), 20 deletions(-) diff --git a/lib/instances/fork.go b/lib/instances/fork.go index f82666183..b7a10a72e 100644 --- a/lib/instances/fork.go +++ b/lib/instances/fork.go @@ -71,8 +71,8 @@ func (m *manager) forkInstance(ctx context.Context, id string, req ForkInstanceR log.InfoContext(ctx, "fork from running requested; transitioning source to standby", "source_instance_id", id, "hypervisor", source.HypervisorType) - if _, err := m.standbyInstance(ctx, id, StandbyInstanceRequest{}, true); err != nil { - return nil, "", false, fmt.Errorf("standby source instance: %w", err) + if inst, err := m.standbyInstance(ctx, id, StandbyInstanceRequest{}, true); err != nil { + return nil, "", false, m.recoverSourceAfterFailedStandby(ctx, inst, fmt.Errorf("standby source instance: %w", err)) } forked, _, forkErr := m.forkInstanceFromStoppedOrStandby(ctx, id, req, true) diff --git a/lib/instances/manager.go b/lib/instances/manager.go index 8dfa302f8..768be372c 100644 --- a/lib/instances/manager.go +++ b/lib/instances/manager.go @@ -631,10 +631,11 @@ func (m *manager) StandbyInstance(ctx context.Context, id string, req StandbyIns } } inst, err := m.standbyInstance(ctx, id, req, false) - if err == nil { - m.notifyLifecycleEvent(ctx, LifecycleEventStandby, inst) + if err != nil { + return nil, m.recoverSourceAfterFailedStandby(ctx, inst, err) } - return inst, err + m.notifyLifecycleEvent(ctx, LifecycleEventStandby, inst) + return inst, nil } // RestoreInstance restores an instance from standby diff --git a/lib/instances/snapshot.go b/lib/instances/snapshot.go index f395dccdc..cf61c5c22 100644 --- a/lib/instances/snapshot.go +++ b/lib/instances/snapshot.go @@ -46,15 +46,18 @@ func (m *manager) getSnapshot(ctx context.Context, snapshotID string) (*Snapshot } // recoverSourceAfterFailedStandby returns standbyErr after putting the source back if -// standby failed after its VMM was saved and stopped. Otherwise the failed snapshot -// would leave a running guest in Standby. A failure before the VMM stops leaves the -// source Running and needs no recovery. -func (m *manager) recoverSourceAfterFailedStandby(ctx context.Context, stored *StoredMetadata, standbyErr error) error { - inst, err := m.currentInstanceWithoutHydration(ctx, stored.Id) - if err != nil || inst.State != StateStandby { +// standby failed after its VMM was stopped. standby is the record standby reached at +// that point, and it is nil for failures before the VMM stopped, which leave the +// source Running. The disk copy can predate standby, so the record is saved before +// the restore reads it. Otherwise the restore would use the pre-standby record. +func (m *manager) recoverSourceAfterFailedStandby(ctx context.Context, standby *Instance, standbyErr error) error { + if standby == nil { return standbyErr } - if restoreErr := m.restoreSnapshotSource(ctx, stored); restoreErr != nil { + if err := m.saveMetadata(&metadata{StoredMetadata: standby.StoredMetadata}); err != nil { + return fmt.Errorf("%w; additionally failed to record standby, source left in Standby: %v", standbyErr, err) + } + if restoreErr := m.restoreSnapshotSource(ctx, &standby.StoredMetadata); restoreErr != nil { return fmt.Errorf("%w; additionally failed to restore source: %v", standbyErr, restoreErr) } return standbyErr @@ -120,8 +123,9 @@ func (m *manager) createSnapshot(ctx context.Context, id string, req CreateSnaps if err := ensureGuestAgentReadyForForkPhase(ctx, &inst.StoredMetadata, "before running snapshot"); err != nil { return nil, err } - if _, err := m.standbyInstance(ctx, id, StandbyInstanceRequest{}, true); err != nil { - return nil, m.recoverSourceAfterFailedStandby(ctx, stored, fmt.Errorf("standby source instance: %w", err)) + standby, err := m.standbyInstance(ctx, id, StandbyInstanceRequest{}, true) + if err != nil { + return nil, m.recoverSourceAfterFailedStandby(ctx, standby, fmt.Errorf("standby source instance: %w", err)) } restoreSource = true case StateStandby: diff --git a/lib/instances/snapshot_macos_test.go b/lib/instances/snapshot_macos_test.go index 2ff862b14..c782d7d23 100644 --- a/lib/instances/snapshot_macos_test.go +++ b/lib/instances/snapshot_macos_test.go @@ -11,6 +11,7 @@ import ( "github.com/kernel/hypeman/lib/hypervisor" "github.com/kernel/hypeman/lib/images" + "github.com/kernel/hypeman/lib/instances/phasetracking" "github.com/kernel/hypeman/lib/paths" restartpolicy "github.com/kernel/hypeman/lib/restart-policy" "github.com/kernel/hypeman/lib/snapshot" @@ -216,17 +217,56 @@ func TestStartRetryFinishesCloneRekey(t *testing.T) { } func TestRecoverSourceAfterFailedStandby(t *testing.T) { + ctx := context.Background() standbyErr := errors.New("save metadata") - // A checkpoint with the VM down is the state a late standby failure leaves behind. + // The disk copy predates standby: still Running, with no Standby phase. m, meta := macOSMemorySnapshotFixture(t) writeMacOSMemorySnapshotPayload(t, m, meta.DataDir, &meta.StoredMetadata) - require.ErrorIs(t, m.recoverSourceAfterFailedStandby(context.Background(), &meta.StoredMetadata, standbyErr), standbyErr) - inst, err := m.GetInstance(context.Background(), meta.Id) + started := time.Now().Add(-time.Hour) + meta.StartedAt = &started + meta.Phases.Record(phasetracking.PhaseRunning, started) + require.NoError(t, m.saveMetadata(meta)) + + // The record standby reached after stopping the VMM. + stoppedAt := time.Now() + standby := cloneStoredMetadata(meta.StoredMetadata) + standby.StoppedAt = &stoppedAt + standby.HypervisorProcessIdentity.Clear() + standby.Phases.Record(phasetracking.PhaseStandby, stoppedAt) + + require.ErrorIs(t, m.recoverSourceAfterFailedStandby(ctx, &Instance{StoredMetadata: standby}, standbyErr), standbyErr) + inst, err := m.GetInstance(ctx, meta.Id) require.NoError(t, err) require.Equal(t, StateRunning, inst.State, "the source must not stay in Standby after a failed snapshot") + disk, err := m.loadMetadata(meta.Id) + require.NoError(t, err) + require.Contains(t, disk.Phases.Cumulative, phasetracking.PhaseStandby, "the standby record must reach the disk before the restore reads it") - // Without a checkpoint the VM never stopped, so there is nothing to recover. + // Nil means standby failed before the VMM stopped, so there is nothing to recover. m2, meta2 := macOSMemorySnapshotFixture(t) - require.Equal(t, standbyErr, m2.recoverSourceAfterFailedStandby(context.Background(), &meta2.StoredMetadata, standbyErr)) + require.Equal(t, standbyErr, m2.recoverSourceAfterFailedStandby(ctx, nil, standbyErr)) + _ = meta2 +} + +func TestRecoverSourceKeepsStandbyWhenRecordCannotBeSaved(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root ignores directory permissions") + } + ctx := context.Background() + m, meta := macOSMemorySnapshotFixture(t) + writeMacOSMemorySnapshotPayload(t, m, meta.DataDir, &meta.StoredMetadata) + require.NoError(t, os.Chmod(meta.DataDir, 0500)) + t.Cleanup(func() { os.Chmod(meta.DataDir, 0700) }) + + standbyErr := errors.New("save metadata") + stoppedAt := time.Now() + standby := cloneStoredMetadata(meta.StoredMetadata) + standby.StoppedAt = &stoppedAt + err := m.recoverSourceAfterFailedStandby(ctx, &Instance{StoredMetadata: standby}, standbyErr) + require.ErrorIs(t, err, standbyErr) + require.ErrorContains(t, err, "source left in Standby") + inst, gerr := m.GetInstance(ctx, meta.Id) + require.NoError(t, gerr) + require.Equal(t, StateStandby, inst.State) } diff --git a/lib/instances/standby.go b/lib/instances/standby.go index a674baf3c..5ec9041d4 100644 --- a/lib/instances/standby.go +++ b/lib/instances/standby.go @@ -269,7 +269,9 @@ func (m *manager) standbyInstance( meta = &metadata{StoredMetadata: *stored} if err := m.saveMetadata(meta); err != nil { log.ErrorContext(ctx, "failed to save metadata", "instance_id", id, "error", err) - return nil, fmt.Errorf("save metadata: %w", err) + // The VMM is down and the checkpoint is saved, but the disk copy predates standby. + // Return the record standby reached so the caller can persist it before restoring. + return &Instance{StoredMetadata: meta.StoredMetadata}, fmt.Errorf("save metadata: %w", err) } // Record metrics From 234278be54d0fcf2ba050b7cf0cf87ce1802ffeb Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Thu, 22 Oct 2026 19:48:13 +0000 Subject: [PATCH 6/7] Make standby-record recovery test independent of timing Tracker drops sub-millisecond phase time, so a standby record stamped at time.Now() can accrue no standby time when the restore runs in the same millisecond. CI caught this. Backdate the standby start so the assertion holds deterministically. --- lib/instances/snapshot_macos_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/instances/snapshot_macos_test.go b/lib/instances/snapshot_macos_test.go index c782d7d23..0302c1830 100644 --- a/lib/instances/snapshot_macos_test.go +++ b/lib/instances/snapshot_macos_test.go @@ -228,8 +228,9 @@ func TestRecoverSourceAfterFailedStandby(t *testing.T) { meta.Phases.Record(phasetracking.PhaseRunning, started) require.NoError(t, m.saveMetadata(meta)) - // The record standby reached after stopping the VMM. - stoppedAt := time.Now() + // The record standby reached after stopping the VMM. It is backdated so the standby + // phase accrues measurable time; Tracker drops sub-millisecond elapsed time. + stoppedAt := time.Now().Add(-time.Minute) standby := cloneStoredMetadata(meta.StoredMetadata) standby.StoppedAt = &stoppedAt standby.HypervisorProcessIdentity.Clear() @@ -241,7 +242,7 @@ func TestRecoverSourceAfterFailedStandby(t *testing.T) { require.Equal(t, StateRunning, inst.State, "the source must not stay in Standby after a failed snapshot") disk, err := m.loadMetadata(meta.Id) require.NoError(t, err) - require.Contains(t, disk.Phases.Cumulative, phasetracking.PhaseStandby, "the standby record must reach the disk before the restore reads it") + require.GreaterOrEqual(t, disk.Phases.Cumulative[phasetracking.PhaseStandby], int64(time.Minute/time.Millisecond), "the standby record must reach the disk before the restore reads it") // Nil means standby failed before the VMM stopped, so there is nothing to recover. m2, meta2 := macOSMemorySnapshotFixture(t) From b21ccd55e531299be8ef5d1d3cea123349f5f614 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:58:55 +0000 Subject: [PATCH 7/7] Keep restore staging in the guest directory, retire checkpoints before resume, and confirm guest power-off Restore staging moves under guests//restore-staging and the swap moves files within the instance directory, so deleting an instance always deletes its staging and recovery can never recreate a deleted instance. A loaded macOS checkpoint is retired before the guest resumes, so a crash after resume cannot leave it to be restored onto a newer disk. The vz-shim writes a stop receipt only when the guest stops the VM itself. A macOS stop is graceful only with that receipt; VMM exit alone no longer counts. --- cmd/vz-shim/main.go | 15 +++ cmd/vz-shim/server.go | 5 + docs/macos-snapshots.md | 25 ++-- lib/forkvm/copy.go | 4 +- lib/hypervisor/vz/macos_config_test.go | 2 + lib/hypervisor/vz/shimconfig/config.go | 7 + lib/hypervisor/vz/starter.go | 4 + lib/instances/macos.go | 6 + lib/instances/macos_memory_snapshot_test.go | 60 ++++++++- lib/instances/manager.go | 5 + lib/instances/process_identity_linux_test.go | 49 +++++++ lib/instances/restore.go | 16 +-- lib/instances/snapshot_macos.go | 127 +++++++++++++------ lib/instances/snapshot_macos_test.go | 73 ++++++----- lib/instances/stop.go | 9 ++ lib/instances/types.go | 5 +- lib/paths/paths.go | 9 ++ 17 files changed, 327 insertions(+), 94 deletions(-) diff --git a/cmd/vz-shim/main.go b/cmd/vz-shim/main.go index 896e9a71d..ed333b7a8 100644 --- a/cmd/vz-shim/main.go +++ b/cmd/vz-shim/main.go @@ -141,6 +141,9 @@ func main() { case newState := <-stateChanged: slog.Info("VM state changed", "state", newState) if newState == vz.VirtualMachineStateStopped || newState == vz.VirtualMachineStateError { + if newState == vz.VirtualMachineStateStopped && !server.forcedStop.Load() { + writeStopReceipt(config.StopReceiptPath) + } slog.Info("VM stopped, shutting down shim") cancel() return @@ -165,6 +168,7 @@ func main() { if vm.State() == vz.VirtualMachineStateRunning { slog.Info("stopping VM") if vm.CanStop() { + server.forcedStop.Store(true) vm.Stop() } } @@ -172,6 +176,17 @@ func main() { slog.Info("vz-shim shutdown complete") } +// writeStopReceipt records that the guest stopped the VM itself. The host counts a +// macOS stop as graceful only with this receipt. +func writeStopReceipt(path string) { + if path == "" { + return + } + if err := os.WriteFile(path, []byte(time.Now().UTC().Format(time.RFC3339Nano)+"\n"), 0600); err != nil { + slog.Error("failed to write stop receipt", "error", err, "path", path) + } +} + func setupLogging(logPath string) error { if logPath == "" { // Log to stderr if no path specified diff --git a/cmd/vz-shim/server.go b/cmd/vz-shim/server.go index 621c0f3b2..94b53103e 100644 --- a/cmd/vz-shim/server.go +++ b/cmd/vz-shim/server.go @@ -13,6 +13,7 @@ import ( "os" "path/filepath" "sync" + "sync/atomic" "github.com/Code-Hex/vz/v3" "github.com/kernel/hypeman/lib/hypervisor/vz/shimconfig" @@ -24,6 +25,9 @@ type ShimServer struct { vmConfig *vz.VirtualMachineConfiguration shimConfig shimconfig.ShimConfig mu sync.RWMutex + // forcedStop is set before the shim stops the VM itself, so that stop is not + // mistaken for the guest powering off. + forcedStop atomic.Bool } // NewShimServer creates a new shim server. @@ -305,6 +309,7 @@ func (s *ShimServer) handleVMMShutdown(w http.ResponseWriter, r *http.Request) { defer s.mu.Unlock() if s.vm.CanStop() { + s.forcedStop.Store(true) s.vm.Stop() } // Process will exit when VM stops (monitored in main) diff --git a/docs/macos-snapshots.md b/docs/macos-snapshots.md index 00b082524..8e1636ba1 100644 --- a/docs/macos-snapshots.md +++ b/docs/macos-snapshots.md @@ -7,9 +7,11 @@ snapshot and schedule paths is still required. ## Stopped snapshots and cold forks -- `kind: Stopped` requires a stopped source and a graceful shutdown receipt: - acknowledged guest shutdown with confirmed owned VMM exit. A forced stop is - refused with 409. +- `kind: Stopped` requires a stopped source and a graceful shutdown receipt: after + the shutdown request, the vz-shim recorded that the guest powered the VM off + itself, and the owned VMM exited. VMM exit alone does not count, and a forced + stop is refused with 409. A shim started before this receipt existed never + writes one, so its guest needs a fresh boot and stop. - Restore targets are Stopped or Running. The source instance, hardware model, machine identifier and MAC must match the snapshot. - A stopped-snapshot fork requires a graceful receipt and an enabled guest agent. @@ -49,19 +51,24 @@ Restore with `POST /instances/{id}/snapshots/{snapshot_id}/restore`. - Preflight checks CPU/RAM, hardware model, machine identifier, NIC MAC, disk and auxiliary paths, and the full memory/disk/auxiliary bundle before storage is replaced. An invalid payload leaves current storage untouched. -- Restored metadata is staged with the payload, including the current name and - expiration, so a directory-swap recovery commits or rolls back metadata and - storage together. Host phase accounting is preserved rather than replayed from the +- Restore stages the payload and restored metadata (including the current name and + expiration) in `restore-staging/` inside the instance directory, then swaps the + instance's files in place. Deleting the instance always removes its staging. A + crash before the swap commits rolls back to the previous files; after it, server + start or the next restore finishes the install. Host phase accounting is preserved rather than replayed from the checkpoint; a memory restore keeps its boot time. An explicit Stopped target suppresses automatic restart. - A restore never mutates the persistent snapshot. If the framework restore fails after installation, the restored checkpoint remains available in Standby. +- Once the shim has loaded the VM, the checkpoint is retired before the guest + resumes, so a later crash cannot restore old memory onto a newer disk. If retiring + it fails, the VM is shut down and the checkpoint is kept. A resume that fails after + it was retired leaves the instance Stopped with no graceful receipt. - If the API dies during a restore, the shim it started keeps loading the VM. Until it opens its socket the instance reads Initializing. Restore returns 409 while that shim has not resumed the VM, and keeps the checkpoint. Once loaded it reads - Paused: `POST /restore` resumes it and - completes the restore (records the shim, removes the consumed checkpoint), and - DELETE kills it. As on Linux, stop does not accept Paused. + Paused: `POST /restore` retires the checkpoint, resumes the VM and records the + shim, and DELETE kills it. As on Linux, stop does not accept Paused. - Saved state is not portable. Use the originating Mac and OS account, a compatible framework version and the same VM configuration. API availability on macOS 14+ does not guarantee support for a particular guest configuration. diff --git a/lib/forkvm/copy.go b/lib/forkvm/copy.go index 606ef2e6a..8908f5593 100644 --- a/lib/forkvm/copy.go +++ b/lib/forkvm/copy.go @@ -8,6 +8,8 @@ import ( "path/filepath" "strings" "sync/atomic" + + "github.com/kernel/hypeman/lib/paths" ) var ( @@ -172,7 +174,7 @@ func copyRegularFile(state *copyState, srcPath, dstPath string, perms fs.FileMod } func shouldSkipDirectory(relPath string) bool { - return relPath == "logs" + return relPath == "logs" || relPath == paths.RestoreStagingDir } func shouldSkipRegularFile(relPath string) bool { diff --git a/lib/hypervisor/vz/macos_config_test.go b/lib/hypervisor/vz/macos_config_test.go index 995fefabe..8efb65c2c 100644 --- a/lib/hypervisor/vz/macos_config_test.go +++ b/lib/hypervisor/vz/macos_config_test.go @@ -15,6 +15,8 @@ func TestMacOSStarterConfig(t *testing.T) { shim := buildShimConfigFromVMConfig(cfg, "/vm/vz.sock") require.Equal(t, base64.StdEncoding.EncodeToString([]byte{1}), shim.MacHardwareModelData) require.Equal(t, "/vm/aux.img", shim.MacAuxStoragePath) + require.Equal(t, "/vm/vz-guest-stopped", shim.StopReceiptPath) + require.Empty(t, buildShimConfigFromVMConfig(hypervisor.VMConfig{}, "/vm/vz.sock").StopReceiptPath, "Linux guests write no receipt") for _, resources := range []struct { cpus int memory int64 diff --git a/lib/hypervisor/vz/shimconfig/config.go b/lib/hypervisor/vz/shimconfig/config.go index 039114de8..897b0fb36 100644 --- a/lib/hypervisor/vz/shimconfig/config.go +++ b/lib/hypervisor/vz/shimconfig/config.go @@ -8,6 +8,9 @@ const ( SnapshotManifestFile = "config.json" // SnapshotMachineStateFile is the serialized VM machine state filename. SnapshotMachineStateFile = "machine-state.vzm" + // StopReceiptFile is written in the instance directory when a macOS guest stops + // its VM itself. + StopReceiptFile = "vz-guest-stopped" ) // ShimConfig is the configuration passed from hypeman to the shim. @@ -51,6 +54,10 @@ type ShimConfig struct { // Logging LogPath string `json:"log_path"` + // StopReceiptPath, when set, is written once the guest stops the VM itself, as + // after an orderly power-off. A stop the shim forces does not write it. + StopReceiptPath string `json:"stop_receipt_path,omitempty"` + // Generic machine identifier data representation (base64), used to keep // platform identity stable across save/restore. MachineIdentifierData string `json:"machine_identifier_data,omitempty"` diff --git a/lib/hypervisor/vz/starter.go b/lib/hypervisor/vz/starter.go index 3ed50f481..26346eac6 100644 --- a/lib/hypervisor/vz/starter.go +++ b/lib/hypervisor/vz/starter.go @@ -197,6 +197,9 @@ func (s *Starter) RestoreVM(ctx context.Context, p *paths.Paths, version string, shimConfig.VsockSocket = filepath.Join(instanceDir, "vz.vsock") shimConfig.LogPath = filepath.Join(instanceDir, "logs", "vz-shim.log") shimConfig.RestoreMachineStatePath = restorePath + if shimConfig.MacAuxStoragePath != "" { + shimConfig.StopReceiptPath = filepath.Join(instanceDir, shimconfig.StopReceiptFile) + } return s.startShim(ctx, p, version, shimConfig, 90*time.Second) } @@ -221,6 +224,7 @@ func buildShimConfigFromVMConfig(config hypervisor.VMConfig, socketPath string) cfg.MacHardwareModelData = base64.StdEncoding.EncodeToString(config.MacOS.HardwareModelData) cfg.MacMachineIdentifierData = base64.StdEncoding.EncodeToString(config.MacOS.MachineIdentifierData) cfg.MacAuxStoragePath = config.MacOS.AuxStoragePath + cfg.StopReceiptPath = filepath.Join(instanceDir, shimconfig.StopReceiptFile) } for _, disk := range config.Disks { cfg.Disks = append(cfg.Disks, shimconfig.DiskConfig{ diff --git a/lib/instances/macos.go b/lib/instances/macos.go index 4c6d9dd1f..9afe3a268 100644 --- a/lib/instances/macos.go +++ b/lib/instances/macos.go @@ -15,6 +15,7 @@ import ( "github.com/kernel/hypeman/lib/healthcheck" "github.com/kernel/hypeman/lib/hypervisor" + "github.com/kernel/hypeman/lib/hypervisor/vz/shimconfig" "github.com/kernel/hypeman/lib/images" "github.com/kernel/hypeman/lib/instances/phasetracking" "github.com/kernel/hypeman/lib/network" @@ -356,6 +357,11 @@ func (s *StoredMetadata) VMStoragePath() string { return filepath.Join(s.DataDir, paths.MacOSBootDiskFile) } +// macOSStopReceiptPath is where the vz-shim records that the guest stopped the VM. +func macOSStopReceiptPath(stored *StoredMetadata) string { + return filepath.Join(stored.DataDir, shimconfig.StopReceiptFile) +} + // requireMacOSGuest refuses Linux guests on a macOS-only server. func (m *manager) requireMacOSGuest(stored *StoredMetadata) error { if m.macOSOnly && stored.MacOS == nil { diff --git a/lib/instances/macos_memory_snapshot_test.go b/lib/instances/macos_memory_snapshot_test.go index 4669b7181..a478b4216 100644 --- a/lib/instances/macos_memory_snapshot_test.go +++ b/lib/instances/macos_memory_snapshot_test.go @@ -404,22 +404,72 @@ func TestMacOSRestoreRetryRemovesConsumedCheckpoint(t *testing.T) { require.NoDirExists(t, m.paths.InstanceSnapshotLatest(meta.Id)) } -func TestMacOSRestoreFailsWhenCheckpointCannotBeRemoved(t *testing.T) { +func TestMacOSRestoreKeepsCheckpointWhenItCannotBeRetired(t *testing.T) { if os.Geteuid() == 0 { t.Skip("root ignores directory permissions") } m, meta := macOSMemorySnapshotFixture(t) writeMacOSMemorySnapshotPayload(t, m, meta.DataDir, &meta.StoredMetadata) + resumed := false + m.vmStarters[macOSMemoryTestHypervisor] = macOSResumeProbeStarter{onResume: func() { resumed = true }} snapshots := filepath.Dir(m.paths.InstanceSnapshotLatest(meta.Id)) require.NoError(t, os.Chmod(snapshots, 0500)) t.Cleanup(func() { os.Chmod(snapshots, 0700) }) _, err := m.restoreInstance(context.Background(), meta.Id) - require.ErrorContains(t, err, "remove consumed snapshot") + require.ErrorContains(t, err, "invalidate checkpoint before resume") + require.False(t, resumed, "the guest must not run while its checkpoint is still restorable") + require.FileExists(t, filepath.Join(m.paths.InstanceSnapshotLatest(meta.Id), shimconfig.SnapshotMachineStateFile)) require.NoError(t, os.Chmod(snapshots, 0700)) - m.storeCachedHypervisorState(meta.Id, hypervisor.StateRunning) - inst, err := m.RestoreInstance(context.Background(), meta.Id) + // The shut-down shim removes its control socket on exit; the fake leaves it. + require.NoError(t, os.Remove(meta.SocketPath)) + _, err = m.restoreInstance(context.Background(), meta.Id) + require.NoError(t, err) + require.True(t, resumed) + require.NoDirExists(t, m.paths.InstanceSnapshotLatest(meta.Id)) +} + +// The checkpoint is retired once the VM is loaded and before the guest runs, so a crash +// after resume cannot leave it to be restored onto the newer disk. +func TestMacOSRestoreRetiresCheckpointBeforeResume(t *testing.T) { + m, meta := macOSMemorySnapshotFixture(t) + writeMacOSMemorySnapshotPayload(t, m, meta.DataDir, &meta.StoredMetadata) + checkpoint := m.paths.InstanceSnapshotLatest(meta.Id) + resumed, presentAtResume := false, false + m.vmStarters[macOSMemoryTestHypervisor] = macOSResumeProbeStarter{onResume: func() { + resumed = true + _, err := os.Stat(checkpoint) + presentAtResume = err == nil + }} + + _, err := m.restoreInstance(context.Background(), meta.Id) require.NoError(t, err) - require.False(t, inst.HasSnapshot, "the retry removes the consumed checkpoint") + require.True(t, resumed) + require.False(t, presentAtResume) + require.NoDirExists(t, checkpoint) + require.NoDirExists(t, filepath.Join(filepath.Dir(checkpoint), "snapshot-consumed")) +} + +type macOSResumeProbeStarter struct { + concurrentForkPrepareTestStarter + onResume func() +} + +func (s macOSResumeProbeStarter) RestoreVM(ctx context.Context, p *paths.Paths, version, socket, snapshot string, options hypervisor.RestoreOptions) (int, hypervisor.Hypervisor, error) { + pid, hv, err := s.concurrentForkPrepareTestStarter.RestoreVM(ctx, p, version, socket, snapshot, options) + if err != nil { + return 0, nil, err + } + return pid, resumeProbeHypervisor{Hypervisor: hv, onResume: s.onResume}, nil +} + +type resumeProbeHypervisor struct { + hypervisor.Hypervisor + onResume func() +} + +func (h resumeProbeHypervisor) Resume(ctx context.Context) error { + h.onResume() + return h.Hypervisor.Resume(ctx) } diff --git a/lib/instances/manager.go b/lib/instances/manager.go index 768be372c..14b23e147 100644 --- a/lib/instances/manager.go +++ b/lib/instances/manager.go @@ -668,6 +668,11 @@ func (m *manager) RestoreInstance(ctx context.Context, id string) (*Instance, er if err != nil { return nil, fmt.Errorf("create hypervisor client: %w", err) } + if current.MacOS != nil { + if err := m.invalidateMacOSCheckpoint(id); err != nil { + return nil, err + } + } if err := hv.Resume(ctx); err != nil { m.invalidateCachedHypervisorState(id) return nil, fmt.Errorf("resume paused vm: %w", err) diff --git a/lib/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index de33190bd..0bf65d7af 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -756,3 +756,52 @@ func TestMacOSGracefulShutdownWithoutLiveVMMIsNotGraceful(t *testing.T) { "a macOS stop with no VMM to signal must not produce a graceful receipt") assert.False(t, sent) } + +// A macOS VMM exits after a forced stop as well, so a stop is graceful only when the +// shim recorded that the guest powered off. A receipt from an earlier run never counts. +func TestMacOSGracefulShutdownRequiresGuestStopReceipt(t *testing.T) { + for _, guestStopped := range []bool{true, false} { + t.Run(fmt.Sprintf("receipt=%v", guestStopped), func(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + owner := exec.Command(os.Args[0], "-test.run=^TestSocketListenerHelper$") + owner.Env = append(os.Environ(), "HYPERVISOR_SOCKET_HELPER=1", "HYPERVISOR_SOCKET_PATH="+socketPath) + stdin, err := owner.StdinPipe() + require.NoError(t, err) + stdout, err := owner.StdoutPipe() + require.NoError(t, err) + require.NoError(t, owner.Start()) + exited := make(chan struct{}) + go func() { _ = owner.Wait(); close(exited) }() + t.Cleanup(func() { + _ = stdin.Close() + _ = owner.Process.Kill() + <-exited + }) + _, err = bufio.NewReader(stdout).ReadString('\n') + require.NoError(t, err) + + inst := &Instance{StoredMetadata: StoredMetadata{ + Id: "graceful-macos-receipt", + HypervisorType: hypervisor.TypeCloudHypervisor, + HypervisorProcessIdentity: HypervisorProcessIdentity{HypervisorPID: &owner.Process.Pid}, + MacOS: &images.MacOSImage{GuestAgent: true}, + DataDir: t.TempDir(), + SocketPath: socketPath, + VsockSocket: filepath.Join(t.TempDir(), "missing-vsock.sock"), + }} + receipt := macOSStopReceiptPath(&inst.StoredMetadata) + require.NoError(t, os.WriteFile(receipt, []byte("earlier run"), 0600)) + m := &manager{shutdownGuestFn: func(context.Context, hypervisor.VsockDialer, int32) error { + assert.NoFileExists(t, receipt, "a receipt from an earlier run is cleared before shutdown") + if guestStopped { + require.NoError(t, os.WriteFile(receipt, []byte("now"), 0600)) + } + _, err := stdin.Write([]byte{1}) + return err + }} + + assert.Equal(t, guestStopped, m.tryGracefulGuestShutdown(context.Background(), inst, 5)) + assert.NoFileExists(t, receipt, "the receipt is consumed") + }) + } +} diff --git a/lib/instances/restore.go b/lib/instances/restore.go index effb8d17c..7a02741b8 100644 --- a/lib/instances/restore.go +++ b/lib/instances/restore.go @@ -320,6 +320,13 @@ func (m *manager) restoreInstance( releaseNetwork() return nil, err } + // A macOS guest allocates no host network here, so there is none to release. + if stored.MacOS != nil { + if err := m.invalidateMacOSCheckpoint(id); err != nil { + hv.Shutdown(ctx) + return nil, err + } + } // 6. Transition: Paused → Running (resume) resumeCtx, resumeSpanEnd := m.startLifecycleStep(ctx, "resume_vm", @@ -377,14 +384,7 @@ func (m *manager) restoreInstance( if err := restoreRetainedSnapshotBase(snapshotDir, retainedBaseDir); err != nil { log.WarnContext(ctx, "failed to retain snapshot base after restore", "instance_id", id, "error", err) } - } else if stored.MacOS != nil { - // A checkpoint left behind would read as Standby if the shim later died, and - // restoring it would replay old memory onto a newer disk. The guest is running; - // a retried restore removes it. - if err := os.RemoveAll(snapshotDir); err != nil { - return nil, fmt.Errorf("remove consumed snapshot: %w", err) - } - } else { + } else if stored.MacOS == nil { log.InfoContext(ctx, "deleting snapshot after successful restore", "instance_id", id) os.RemoveAll(snapshotDir) // Best effort, ignore errors } diff --git a/lib/instances/snapshot_macos.go b/lib/instances/snapshot_macos.go index db66cf4de..50d3ff42b 100644 --- a/lib/instances/snapshot_macos.go +++ b/lib/instances/snapshot_macos.go @@ -234,26 +234,25 @@ func (m *manager) saveMacOSSnapshotFork(ctx context.Context, meta *metadata) err return m.saveForkMetadata(ctx, meta) } -// replaceMacOSInstanceWithSnapshotPayload stages the snapshot and restored metadata -// beside the instance, then swaps both in. A failure before the swap leaves the -// previous directory intact; a failed swap is rolled back. The staging root lives outside the guests directory, -// so instance and snapshot listings never see a half-restored copy. +// replaceMacOSInstanceWithSnapshotPayload stages the snapshot and restored metadata in +// the instance's restore staging directory, then swaps the instance's files for the +// staged ones. The instance directory itself stays in place, so deleting it always +// removes any staging. Renaming staged to installing commits the swap: a crash before +// that rolls back to the previous files, and a crash after it finishes the install. func (m *manager) replaceMacOSInstanceWithSnapshotPayload(snapshotID string, restored *metadata) error { instanceID := restored.Id if err := m.recoverMacOSRestoreStaging(instanceID); err != nil { return fmt.Errorf("recover before restore: %w", err) } instanceDir := m.paths.InstanceDir(instanceID) - root := filepath.Join(m.paths.DataDir(), "restore-staging", instanceID) + root := m.paths.InstanceRestoreStaging(instanceID) staged := filepath.Join(root, "staged") previous := filepath.Join(root, "previous") + installing := filepath.Join(root, "installing") fail := func(err error) error { _ = os.RemoveAll(root) return err } - if err := os.RemoveAll(root); err != nil { - return fmt.Errorf("clear restore staging: %w", err) - } if err := os.MkdirAll(root, 0700); err != nil { return fmt.Errorf("create restore staging: %w", err) } @@ -275,29 +274,58 @@ func (m *manager) replaceMacOSInstanceWithSnapshotPayload(snapshotID string, res if err := os.WriteFile(filepath.Join(staged, filepath.Base(m.paths.InstanceMetadata(instanceID))), data, 0644); err != nil { return fail(fmt.Errorf("stage restored metadata: %w", err)) } - if err := os.Rename(instanceDir, previous); err != nil { - return fail(fmt.Errorf("retire instance directory: %w", err)) + if err := os.Mkdir(previous, 0700); err != nil { + return fail(fmt.Errorf("create restore backup: %w", err)) + } + retired := moveDirEntries(instanceDir, previous) + if retired == nil { + retired = os.Rename(staged, installing) } - if err := os.Rename(staged, instanceDir); err != nil { - if rollbackErr := os.Rename(previous, instanceDir); rollbackErr != nil { - return fmt.Errorf("install snapshot payload: %w (rollback failed: %v; previous state kept at %s)", err, rollbackErr, previous) + if retired != nil { + if rollbackErr := moveDirEntries(previous, instanceDir); rollbackErr != nil { + return fmt.Errorf("retire instance files: %w (rollback failed: %v; previous files kept at %s)", retired, rollbackErr, previous) } - return fail(fmt.Errorf("install snapshot payload: %w", err)) + return fail(fmt.Errorf("retire instance files: %w", retired)) + } + if err := moveDirEntries(installing, instanceDir); err != nil { + return fmt.Errorf("install snapshot payload: %w (the next restore or server start finishes it)", err) } _ = os.RemoveAll(root) return nil } -// recoverMacOSRestoreStaging resolves a swap interrupted by a crash. With the instance -// directory missing and the previous directory parked, the previous state is put back. -// If the instance directory is present, a completed swap leaves stale staging, which is -// removed. Staging with no previous directory is an unfinished copy and is removed. -// An empty instanceID recovers every instance. +// moveDirEntries renames every entry of src into dst, except restore staging. +func moveDirEntries(src, dst string) error { + entries, err := os.ReadDir(src) + if err != nil { + return err + } + for _, entry := range entries { + if entry.Name() == paths.RestoreStagingDir { + continue + } + if err := os.Rename(filepath.Join(src, entry.Name()), filepath.Join(dst, entry.Name())); err != nil { + return err + } + } + return nil +} + +func pathExists(path string) (bool, error) { + _, err := os.Stat(path) + if errors.Is(err, os.ErrNotExist) { + return false, nil + } + return err == nil, err +} + +// recoverMacOSRestoreStaging resolves a swap interrupted by a crash. A committed swap +// (installing present) is finished; otherwise any retired files are moved back. The +// staging directory is then removed. An empty instanceID recovers every instance. func (m *manager) recoverMacOSRestoreStaging(instanceID string) error { - root := filepath.Join(m.paths.DataDir(), "restore-staging") ids := []string{instanceID} if instanceID == "" { - entries, err := os.ReadDir(root) + entries, err := os.ReadDir(m.paths.GuestsDir()) if errors.Is(err, os.ErrNotExist) { return nil } @@ -310,37 +338,56 @@ func (m *manager) recoverMacOSRestoreStaging(instanceID string) error { } } for _, id := range ids { - stagingRoot := filepath.Join(root, id) - previous := filepath.Join(stagingRoot, "previous") + root := m.paths.InstanceRestoreStaging(id) + if ok, err := pathExists(root); err != nil { + return fmt.Errorf("inspect restore staging for %s: %w", id, err) + } else if !ok { + continue + } instanceDir := m.paths.InstanceDir(id) - _, previousErr := os.Stat(previous) - if previousErr != nil && !errors.Is(previousErr, os.ErrNotExist) { - return fmt.Errorf("inspect restore staging for %s: %w", id, previousErr) + installing, err := pathExists(filepath.Join(root, "installing")) + if err != nil { + return fmt.Errorf("inspect restore staging for %s: %w", id, err) } - if previousErr == nil { - _, instanceErr := os.Stat(instanceDir) - if instanceErr != nil && !errors.Is(instanceErr, os.ErrNotExist) { - return fmt.Errorf("inspect instance directory for %s: %w", id, instanceErr) + previous, err := pathExists(filepath.Join(root, "previous")) + if err != nil { + return fmt.Errorf("inspect restore staging for %s: %w", id, err) + } + if installing { + if err := moveDirEntries(filepath.Join(root, "installing"), instanceDir); err != nil { + return fmt.Errorf("finish restore for %s: %w", id, err) } - if errors.Is(instanceErr, os.ErrNotExist) { - if err := os.MkdirAll(filepath.Dir(instanceDir), 0755); err != nil { - return fmt.Errorf("prepare rollback for %s: %w", id, err) - } - if err := os.Rename(previous, instanceDir); err != nil { - return fmt.Errorf("roll back restore for %s: %w", id, err) - } + } else if previous { + if err := moveDirEntries(filepath.Join(root, "previous"), instanceDir); err != nil { + return fmt.Errorf("roll back restore for %s: %w", id, err) } } - if err := os.RemoveAll(stagingRoot); err != nil { + if err := os.RemoveAll(root); err != nil { return fmt.Errorf("clear restore staging for %s: %w", id, err) } } return nil } +// invalidateMacOSCheckpoint retires the checkpoint of a loaded, still paused macOS VM +// before it resumes. Once the guest runs its disk moves past the checkpoint, so a later +// crash must not find it and restore old memory onto the newer disk. The rename is +// atomic: if it fails, the checkpoint is whole and the restore can be retried. +func (m *manager) invalidateMacOSCheckpoint(id string) error { + consumed := filepath.Join(m.paths.InstanceSnapshots(id), "snapshot-consumed") + if err := os.RemoveAll(consumed); err != nil { + return fmt.Errorf("clear consumed checkpoint: %w", err) + } + if err := os.Rename(m.paths.InstanceSnapshotLatest(id), consumed); err != nil && !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("invalidate checkpoint before resume: %w", err) + } + _ = os.RemoveAll(consumed) + return nil +} + // finishMacOSResume completes a restore whose API process died after the shim had -// loaded the VM, once resuming the Paused guest succeeds. It records the shim, and -// removes the consumed memory snapshot so a later crash cannot restore stale memory +// loaded the VM, once resuming the Paused guest succeeds. It records the shim and +// removes any checkpoint still on disk, so a later crash cannot restore stale memory // onto the guest's newer disk. func (m *manager) finishMacOSResume(id string) error { meta, err := m.loadMetadata(id) diff --git a/lib/instances/snapshot_macos_test.go b/lib/instances/snapshot_macos_test.go index 0302c1830..47cad876d 100644 --- a/lib/instances/snapshot_macos_test.go +++ b/lib/instances/snapshot_macos_test.go @@ -85,8 +85,7 @@ func TestMacOSRestoreSwapsPayloadWithMetadata(t *testing.T) { data, err := os.ReadFile(filepath.Join(m.paths.InstanceDir(id), "boot.raw")) require.NoError(t, err) require.Equal(t, "before", string(data)) - _, err = os.Stat(filepath.Join(m.paths.DataDir(), "restore-staging", id)) - require.ErrorIs(t, err, os.ErrNotExist, "staging is removed after a successful swap") + require.NoDirExists(t, m.paths.InstanceRestoreStaging(id), "staging is removed after a successful swap") // A restart before the caller's own saveMetadata must find the restored metadata. require.NoError(t, m.recoverMacOSRestoreStaging(id)) got, err := m.loadMetadata(id) @@ -104,58 +103,74 @@ func TestMacOSRestoreSwapsPayloadWithMetadata(t *testing.T) { require.Equal(t, "current", string(data), "a failed staging leaves the previous disk untouched") } -func TestMacOSRestoreRecoveryRollsBackAndCleans(t *testing.T) { +func TestMacOSRestoreRecoveryRollsBackOrFinishes(t *testing.T) { const id = "mac-crash" newManager := func(t *testing.T) (*manager, string) { m := &manager{paths: paths.New(t.TempDir())} - return m, filepath.Join(m.paths.DataDir(), "restore-staging", id) + return m, m.paths.InstanceRestoreStaging(id) + } + readBoot := func(t *testing.T, m *manager) string { + data, err := os.ReadFile(filepath.Join(m.paths.InstanceDir(id), "boot.raw")) + require.NoError(t, err) + return string(data) } - // Crash after retiring the instance directory, before the swap: roll the previous state back. + // Crash while retiring files, before the swap commits: the retired files come back. m, root := newManager(t) + writeGuestFiles(t, m.paths.InstanceDir(id), map[string]string{macOSAuxStorageFile: "aux-previous"}) writeGuestFiles(t, filepath.Join(root, "previous"), map[string]string{"boot.raw": "previous"}) + writeGuestFiles(t, filepath.Join(root, "staged"), map[string]string{"boot.raw": "restored"}) require.NoError(t, m.recoverMacOSRestoreStaging(id)) - data, err := os.ReadFile(filepath.Join(m.paths.InstanceDir(id), "boot.raw")) - require.NoError(t, err) - require.Equal(t, "previous", string(data)) - _, err = os.Stat(root) - require.ErrorIs(t, err, os.ErrNotExist) + require.Equal(t, "previous", readBoot(t, m)) + require.FileExists(t, filepath.Join(m.paths.InstanceDir(id), macOSAuxStorageFile)) + require.NoDirExists(t, root) - // Crash after a completed swap: the instance directory is current, so stale staging goes. + // Crash while installing a committed swap: the rest of the payload is installed. m, root = newManager(t) - writeGuestFiles(t, m.paths.InstanceDir(id), map[string]string{"boot.raw": "restored"}) - writeGuestFiles(t, filepath.Join(root, "previous"), map[string]string{"boot.raw": "old"}) + writeGuestFiles(t, m.paths.InstanceDir(id), map[string]string{macOSAuxStorageFile: "aux-restored"}) + writeGuestFiles(t, filepath.Join(root, "installing"), map[string]string{"boot.raw": "restored"}) + writeGuestFiles(t, filepath.Join(root, "previous"), map[string]string{"boot.raw": "old", macOSAuxStorageFile: "aux-old"}) require.NoError(t, m.recoverMacOSRestoreStaging(id)) - data, err = os.ReadFile(filepath.Join(m.paths.InstanceDir(id), "boot.raw")) - require.NoError(t, err) - require.Equal(t, "restored", string(data)) - _, err = os.Stat(root) - require.ErrorIs(t, err, os.ErrNotExist) + require.Equal(t, "restored", readBoot(t, m)) + require.NoDirExists(t, root) - // Crash during staging copy: no previous directory, so the partial copy is removed. + // Crash during the staging copy: nothing was retired, so the partial copy is removed. m, root = newManager(t) + writeGuestFiles(t, m.paths.InstanceDir(id), map[string]string{"boot.raw": "current"}) writeGuestFiles(t, filepath.Join(root, "staged"), map[string]string{"boot.raw": "partial"}) + writeGuestFiles(t, m.paths.InstanceDir("other"), map[string]string{"boot.raw": "untouched"}) + require.NoError(t, m.recoverMacOSRestoreStaging("")) + require.Equal(t, "current", readBoot(t, m)) + require.NoDirExists(t, root) + require.FileExists(t, filepath.Join(m.paths.InstanceDir("other"), "boot.raw")) +} + +func TestMacOSRestoreStagingIsDeletedWithInstance(t *testing.T) { + m := &manager{paths: paths.New(t.TempDir())} + const id = "mac-deleted" + root := m.paths.InstanceRestoreStaging(id) + writeGuestFiles(t, filepath.Join(root, "previous"), map[string]string{"boot.raw": "previous"}) + require.Equal(t, m.paths.InstanceDir(id), filepath.Dir(root), "staging lives inside the instance directory") + + require.NoError(t, os.RemoveAll(m.paths.InstanceDir(id))) require.NoError(t, m.recoverMacOSRestoreStaging("")) - _, err = os.Stat(root) - require.ErrorIs(t, err, os.ErrNotExist) + require.NoDirExists(t, m.paths.InstanceDir(id), "recovery must not resurrect a deleted instance") } -func TestMacOSRestoreRecoveryKeepsBackupWhenInstanceStateIsUnknown(t *testing.T) { +func TestMacOSRestoreRecoveryKeepsBackupWhenStagingIsUnreadable(t *testing.T) { if os.Geteuid() == 0 { t.Skip("root ignores directory permissions") } const id = "mac-unknown" m := &manager{paths: paths.New(t.TempDir())} - root := filepath.Join(m.paths.DataDir(), "restore-staging", id) + root := m.paths.InstanceRestoreStaging(id) writeGuestFiles(t, filepath.Join(root, "previous"), map[string]string{"boot.raw": "previous"}) - guests := filepath.Dir(m.paths.InstanceDir(id)) - require.NoError(t, os.MkdirAll(guests, 0755)) - // An unreadable guests directory makes the instance directory's state unknown. - require.NoError(t, os.Chmod(guests, 0)) - t.Cleanup(func() { _ = os.Chmod(guests, 0755) }) + // An unreadable staging directory makes the swap's progress unknown. + require.NoError(t, os.Chmod(root, 0)) + t.Cleanup(func() { _ = os.Chmod(root, 0755) }) require.Error(t, m.recoverMacOSRestoreStaging(id)) - require.NoError(t, os.Chmod(guests, 0755)) + require.NoError(t, os.Chmod(root, 0755)) data, err := os.ReadFile(filepath.Join(root, "previous", "boot.raw")) require.NoError(t, err, "the parked backup must survive an inconclusive check") require.Equal(t, "previous", string(data)) diff --git a/lib/instances/stop.go b/lib/instances/stop.go index f99753e97..2d8ee7a39 100644 --- a/lib/instances/stop.go +++ b/lib/instances/stop.go @@ -70,6 +70,9 @@ func (m *manager) tryGracefulGuestShutdown(ctx context.Context, inst *Instance, return inst.MacOS == nil } inst.HypervisorProcessIdentity.Set(pid) + if inst.MacOS != nil { + _ = os.Remove(macOSStopReceiptPath(&inst.StoredMetadata)) + } shutdownGuest := guest.ShutdownInstance if m.shutdownGuestFn != nil { @@ -103,6 +106,12 @@ func (m *manager) tryGracefulGuestShutdown(ctx context.Context, inst *Instance, } if WaitForProcessExit(pid, waitTimeout) { + // The shim exits after a forced stop too, so macOS also needs its receipt that + // the guest powered off. + if inst.MacOS != nil && os.Remove(macOSStopReceiptPath(&inst.StoredMetadata)) != nil { + log.WarnContext(ctx, "VM exited without confirming a guest power-off", "instance_id", inst.Id) + return false + } log.DebugContext(ctx, "VM shut down gracefully", "instance_id", inst.Id) return true } diff --git a/lib/instances/types.go b/lib/instances/types.go index 49c1b0446..bd8989885 100644 --- a/lib/instances/types.go +++ b/lib/instances/types.go @@ -124,8 +124,9 @@ type StoredMetadata struct { ExpiresAt *time.Time // Absolute expiration time; nil disables expiration StartedAt *time.Time // Boot epoch start time (set on create/start; preserved across standby restore) StoppedAt *time.Time // Last time VM was stopped - // StopGraceful records whether the last stop was an acknowledged guest shutdown with - // owned VMM exit. macOS stopped snapshots require it; a forced stop is not consistent. + // StopGraceful records whether the last stop was a guest power-off, confirmed by the + // vz-shim's stop receipt, with owned VMM exit. macOS stopped snapshots require it; a + // forced stop is not consistent. StopGraceful bool `json:"stop_graceful,omitempty"` // Boot progress markers (derived from guest serial log sentinels and persisted) diff --git a/lib/paths/paths.go b/lib/paths/paths.go index ae615c61d..f8fba4cb3 100644 --- a/lib/paths/paths.go +++ b/lib/paths/paths.go @@ -259,6 +259,15 @@ func (p *Paths) InstanceMacOSBootDisk(id string) string { return filepath.Join(p.InstanceDir(id), MacOSBootDiskFile) } +// RestoreStagingDir holds a macOS restore in progress inside its instance directory, +// so deleting the instance also deletes any staging it left behind. +const RestoreStagingDir = "restore-staging" + +// InstanceRestoreStaging returns the path to an instance's restore staging directory. +func (p *Paths) InstanceRestoreStaging(id string) string { + return filepath.Join(p.InstanceDir(id), RestoreStagingDir) +} + // InstanceConfigDisk returns the path to instance config disk. func (p *Paths) InstanceConfigDisk(id string) string { return filepath.Join(p.InstanceDir(id), "config.ext4")