diff --git a/lib/hypervisor/cloudhypervisor/cloudhypervisor.go b/lib/hypervisor/cloudhypervisor/cloudhypervisor.go index a102398d..2d3f77bb 100644 --- a/lib/hypervisor/cloudhypervisor/cloudhypervisor.go +++ b/lib/hypervisor/cloudhypervisor/cloudhypervisor.go @@ -32,13 +32,19 @@ var _ hypervisor.Hypervisor = (*CloudHypervisor)(nil) // Capabilities returns the features supported by Cloud Hypervisor. func (c *CloudHypervisor) Capabilities() hypervisor.Capabilities { + return capabilities() +} + +func capabilities() hypervisor.Capabilities { return hypervisor.Capabilities{ - SupportsSnapshot: true, - SupportsHotplugMemory: true, - SupportsPause: true, - SupportsVsock: true, - SupportsGPUPassthrough: true, - SupportsDiskIOLimit: true, + SupportsSnapshot: true, + SupportsHotplugMemory: true, + SupportsPause: true, + SupportsVsock: true, + SupportsGPUPassthrough: true, + SupportsDiskIOLimit: true, + SupportsGracefulVMMShutdown: true, + SupportsSnapshotBaseReuse: false, } } diff --git a/lib/hypervisor/cloudhypervisor/process.go b/lib/hypervisor/cloudhypervisor/process.go index c30b6c3d..5fd4037e 100644 --- a/lib/hypervisor/cloudhypervisor/process.go +++ b/lib/hypervisor/cloudhypervisor/process.go @@ -15,6 +15,7 @@ import ( func init() { hypervisor.RegisterSocketName(hypervisor.TypeCloudHypervisor, "ch.sock") + hypervisor.RegisterCapabilities(hypervisor.TypeCloudHypervisor, capabilities()) hypervisor.RegisterClientFactory(hypervisor.TypeCloudHypervisor, func(socketPath string) (hypervisor.Hypervisor, error) { return New(socketPath) }) diff --git a/lib/hypervisor/firecracker/config.go b/lib/hypervisor/firecracker/config.go index 5ba47cc1..9576f2ca 100644 --- a/lib/hypervisor/firecracker/config.go +++ b/lib/hypervisor/firecracker/config.go @@ -200,10 +200,15 @@ func toRateLimiter(limit int64, burst int64) *rateLimiter { } func toSnapshotCreateParams(snapshotDir string) snapshotCreateParams { + snapshotType := "Full" + if _, err := os.Stat(snapshotMemoryPath(snapshotDir)); err == nil { + snapshotType = "Diff" + } + return snapshotCreateParams{ MemFilePath: snapshotMemoryPath(snapshotDir), SnapshotPath: snapshotStatePath(snapshotDir), - SnapshotType: "Full", + SnapshotType: snapshotType, } } diff --git a/lib/hypervisor/firecracker/config_test.go b/lib/hypervisor/firecracker/config_test.go index 4649ea61..6e912ee7 100644 --- a/lib/hypervisor/firecracker/config_test.go +++ b/lib/hypervisor/firecracker/config_test.go @@ -1,6 +1,8 @@ package firecracker import ( + "os" + "path/filepath" "testing" "github.com/kernel/hypeman/lib/hypervisor" @@ -59,10 +61,24 @@ func TestToNetworkInterfaces(t *testing.T) { } func TestSnapshotParamPaths(t *testing.T) { - create := toSnapshotCreateParams("/tmp/snapshot-latest") - assert.Equal(t, "/tmp/snapshot-latest/state", create.SnapshotPath) - assert.Equal(t, "/tmp/snapshot-latest/memory", create.MemFilePath) - assert.Equal(t, "Full", create.SnapshotType) + t.Run("uses full snapshots when no retained base exists", func(t *testing.T) { + snapshotDir := filepath.Join(t.TempDir(), "snapshot-latest") + create := toSnapshotCreateParams(snapshotDir) + assert.Equal(t, filepath.Join(snapshotDir, "state"), create.SnapshotPath) + assert.Equal(t, filepath.Join(snapshotDir, "memory"), create.MemFilePath) + assert.Equal(t, "Full", create.SnapshotType) + }) + + t.Run("uses diff snapshots when retained base memory exists", func(t *testing.T) { + snapshotDir := filepath.Join(t.TempDir(), "snapshot-latest") + require.NoError(t, os.MkdirAll(snapshotDir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(snapshotDir, "memory"), []byte("base"), 0644)) + + create := toSnapshotCreateParams(snapshotDir) + assert.Equal(t, filepath.Join(snapshotDir, "state"), create.SnapshotPath) + assert.Equal(t, filepath.Join(snapshotDir, "memory"), create.MemFilePath) + assert.Equal(t, "Diff", create.SnapshotType) + }) load := toSnapshotLoadParams("/tmp/snapshot-latest", []networkOverride{ {IfaceID: "eth0", HostDevName: "hype-abc123"}, diff --git a/lib/hypervisor/firecracker/firecracker.go b/lib/hypervisor/firecracker/firecracker.go index 64f399f3..4c2b2a69 100644 --- a/lib/hypervisor/firecracker/firecracker.go +++ b/lib/hypervisor/firecracker/firecracker.go @@ -47,13 +47,19 @@ func New(socketPath string) (*Firecracker, error) { var _ hypervisor.Hypervisor = (*Firecracker)(nil) func (f *Firecracker) Capabilities() hypervisor.Capabilities { + return capabilities() +} + +func capabilities() hypervisor.Capabilities { return hypervisor.Capabilities{ - SupportsSnapshot: true, - SupportsHotplugMemory: false, - SupportsPause: true, - SupportsVsock: true, - SupportsGPUPassthrough: false, - SupportsDiskIOLimit: true, + SupportsSnapshot: true, + SupportsHotplugMemory: false, + SupportsPause: true, + SupportsVsock: true, + SupportsGPUPassthrough: false, + SupportsDiskIOLimit: true, + SupportsGracefulVMMShutdown: false, + SupportsSnapshotBaseReuse: true, } } diff --git a/lib/hypervisor/firecracker/process.go b/lib/hypervisor/firecracker/process.go index 14dd52c9..8b3c5d36 100644 --- a/lib/hypervisor/firecracker/process.go +++ b/lib/hypervisor/firecracker/process.go @@ -25,6 +25,7 @@ const ( func init() { hypervisor.RegisterSocketName(hypervisor.TypeFirecracker, "fc.sock") + hypervisor.RegisterCapabilities(hypervisor.TypeFirecracker, capabilities()) hypervisor.RegisterClientFactory(hypervisor.TypeFirecracker, func(socketPath string) (hypervisor.Hypervisor, error) { return New(socketPath) }) diff --git a/lib/hypervisor/hypervisor.go b/lib/hypervisor/hypervisor.go index 4026a425..5777f381 100644 --- a/lib/hypervisor/hypervisor.go +++ b/lib/hypervisor/hypervisor.go @@ -45,6 +45,10 @@ var socketNames = make(map[Type]string) // Registered by hypervisor packages when they use socket-based vsock routing. var vsockSocketNames = make(map[Type]string) +// capabilitiesByType maps hypervisor types to their static capabilities. +// Registered by each hypervisor package's init() function. +var capabilitiesByType = make(map[Type]Capabilities) + // RegisterSocketName registers the socket filename for a hypervisor type. // Called by each hypervisor implementation's init() function. func RegisterSocketName(t Type, name string) { @@ -74,6 +78,17 @@ func VsockSocketNameForType(t Type) string { return "vsock.sock" } +// RegisterCapabilities registers static capabilities for a hypervisor type. +func RegisterCapabilities(t Type, caps Capabilities) { + capabilitiesByType[t] = caps +} + +// CapabilitiesForType returns static capabilities for a hypervisor type. +func CapabilitiesForType(t Type) (Capabilities, bool) { + caps, ok := capabilitiesByType[t] + return caps, ok +} + // VMStarter handles the full VM startup sequence. // Each hypervisor implements its own startup flow: // - Cloud Hypervisor: starts process, configures via HTTP API, boots via HTTP API @@ -197,6 +212,14 @@ type Capabilities struct { // SupportsDiskIOLimit indicates if disk I/O rate limiting is available SupportsDiskIOLimit bool + + // SupportsGracefulVMMShutdown indicates the hypervisor exposes an API to + // ask the VMM process itself to exit cleanly. + SupportsGracefulVMMShutdown bool + + // SupportsSnapshotBaseReuse indicates snapshots can safely reuse a retained + // on-disk base across restore/standby cycles. + SupportsSnapshotBaseReuse bool } // VsockDialer provides vsock connectivity to a guest VM. diff --git a/lib/hypervisor/qemu/process.go b/lib/hypervisor/qemu/process.go index dfe5e267..74fa0916 100644 --- a/lib/hypervisor/qemu/process.go +++ b/lib/hypervisor/qemu/process.go @@ -43,6 +43,7 @@ const ( func init() { hypervisor.RegisterSocketName(hypervisor.TypeQEMU, "qemu.sock") + hypervisor.RegisterCapabilities(hypervisor.TypeQEMU, capabilities()) hypervisor.RegisterClientFactory(hypervisor.TypeQEMU, func(socketPath string) (hypervisor.Hypervisor, error) { return New(socketPath) }) diff --git a/lib/hypervisor/qemu/qemu.go b/lib/hypervisor/qemu/qemu.go index 62489521..4db8a05b 100644 --- a/lib/hypervisor/qemu/qemu.go +++ b/lib/hypervisor/qemu/qemu.go @@ -37,13 +37,19 @@ var _ hypervisor.Hypervisor = (*QEMU)(nil) // Capabilities returns the features supported by QEMU. func (q *QEMU) Capabilities() hypervisor.Capabilities { + return capabilities() +} + +func capabilities() hypervisor.Capabilities { return hypervisor.Capabilities{ - SupportsSnapshot: true, // Uses QMP migrate file:// for snapshot - SupportsHotplugMemory: false, // Not implemented - balloon not configured - SupportsPause: true, - SupportsVsock: true, - SupportsGPUPassthrough: true, - SupportsDiskIOLimit: true, + SupportsSnapshot: true, // Uses QMP migrate file:// for snapshot + SupportsHotplugMemory: false, // Not implemented - balloon not configured + SupportsPause: true, + SupportsVsock: true, + SupportsGPUPassthrough: true, + SupportsDiskIOLimit: true, + SupportsGracefulVMMShutdown: true, + SupportsSnapshotBaseReuse: false, } } diff --git a/lib/hypervisor/vz/client.go b/lib/hypervisor/vz/client.go index 55447936..51c1f283 100644 --- a/lib/hypervisor/vz/client.go +++ b/lib/hypervisor/vz/client.go @@ -71,13 +71,19 @@ type snapshotRequest struct { } func (c *Client) Capabilities() hypervisor.Capabilities { + return capabilities() +} + +func capabilities() hypervisor.Capabilities { return hypervisor.Capabilities{ - SupportsSnapshot: runtime.GOARCH == "arm64", - SupportsHotplugMemory: false, - SupportsPause: true, - SupportsVsock: true, - SupportsGPUPassthrough: false, - SupportsDiskIOLimit: false, + SupportsSnapshot: runtime.GOARCH == "arm64", + SupportsHotplugMemory: false, + SupportsPause: true, + SupportsVsock: true, + SupportsGPUPassthrough: false, + SupportsDiskIOLimit: false, + SupportsGracefulVMMShutdown: true, + SupportsSnapshotBaseReuse: false, } } diff --git a/lib/hypervisor/vz/starter.go b/lib/hypervisor/vz/starter.go index d9f83267..72e61da1 100644 --- a/lib/hypervisor/vz/starter.go +++ b/lib/hypervisor/vz/starter.go @@ -24,6 +24,7 @@ import ( func init() { hypervisor.RegisterSocketName(hypervisor.TypeVZ, "vz.sock") + hypervisor.RegisterCapabilities(hypervisor.TypeVZ, capabilities()) hypervisor.RegisterVsockSocketName(hypervisor.TypeVZ, "vz.vsock") hypervisor.RegisterVsockDialerFactory(hypervisor.TypeVZ, NewVsockDialer) hypervisor.RegisterClientFactory(hypervisor.TypeVZ, func(socketPath string) (hypervisor.Hypervisor, error) { diff --git a/lib/instances/firecracker_test.go b/lib/instances/firecracker_test.go index 2dd28f95..ebec9e68 100644 --- a/lib/instances/firecracker_test.go +++ b/lib/instances/firecracker_test.go @@ -27,12 +27,12 @@ import ( "github.com/vishvananda/netlink" ) -func setupTestManagerForFirecracker(t *testing.T) (*manager, string) { +func setupTestManagerForFirecrackerWithNetworkConfig(t *testing.T, networkCfg config.NetworkConfig) (*manager, string) { tmpDir := t.TempDir() prepareIntegrationTestDataDir(t, tmpDir) cfg := &config.Config{ DataDir: tmpDir, - Network: newParallelTestNetworkConfig(t), + Network: networkCfg, } p := paths.New(tmpDir) @@ -61,6 +61,14 @@ func setupTestManagerForFirecracker(t *testing.T) (*manager, string) { return mgr, tmpDir } +func setupTestManagerForFirecracker(t *testing.T) (*manager, string) { + return setupTestManagerForFirecrackerWithNetworkConfig(t, newParallelTestNetworkConfig(t)) +} + +func setupTestManagerForFirecrackerNoNetwork(t *testing.T) (*manager, string) { + return setupTestManagerForFirecrackerWithNetworkConfig(t, legacyParallelTestNetworkConfig(testNetworkSeq.Add(1))) +} + func requireFirecrackerIntegrationPrereqs(t *testing.T) { t.Helper() if _, err := os.Stat("/dev/kvm"); os.IsNotExist(err) { @@ -119,7 +127,7 @@ func TestFirecrackerStandbyAndRestore(t *testing.T) { t.Parallel() requireFirecrackerIntegrationPrereqs(t) - mgr, tmpDir := setupTestManagerForFirecracker(t) + mgr, tmpDir := setupTestManagerForFirecrackerNoNetwork(t) ctx := context.Background() p := paths.New(tmpDir) @@ -141,35 +149,105 @@ func TestFirecrackerStandbyAndRestore(t *testing.T) { }) require.NoError(t, err) assert.Contains(t, []State{StateInitializing, StateRunning}, inst.State) + deleted := false + t.Cleanup(func() { + if !deleted { + _ = mgr.DeleteInstance(context.Background(), inst.Id) + } + }) + inst, err = waitForInstanceState(ctx, mgr, inst.Id, StateRunning, 20*time.Second) require.NoError(t, err) + require.NoError(t, waitForExecAgent(ctx, mgr, inst.Id, 30*time.Second)) + + firstFilePath := "/tmp/firecracker-standby-first.txt" + secondFilePath := "/tmp/firecracker-standby-second.txt" + firstFileContents := "first-cycle" + secondFileContents := "second-cycle" + + writeGuestFile := func(path string, contents string) { + t.Helper() + output, exitCode, err := execCommand(ctx, inst, "sh", "-c", fmt.Sprintf("printf %q > %s && sync", contents, path)) + require.NoError(t, err, "write file via exec should succeed") + require.Equal(t, 0, exitCode, "write file via exec should exit successfully: %s", output) + } + + assertGuestFileContents := func(path string, expected string) { + t.Helper() + output, exitCode, err := execCommand(ctx, inst, "cat", path) + require.NoError(t, err, "read file via exec should succeed") + require.Equal(t, 0, exitCode, "read file via exec should exit successfully: %s", output) + assert.Equal(t, expected, strings.TrimSpace(output)) + } + + assertRetainedBaseState := func() { + t.Helper() + _, err = os.Stat(p.InstanceSnapshotLatest(inst.Id)) + assert.True(t, os.IsNotExist(err), "running instances should not keep snapshot-latest after restore") + _, err = os.Stat(p.InstanceSnapshotBase(inst.Id)) + require.NoError(t, err, "hypervisors that reuse snapshot bases should retain the hidden base after restore") + } + + restoreAndMeasure := func(label string) (time.Duration, time.Duration) { + t.Helper() + start := time.Now() + inst, err = mgr.RestoreInstance(ctx, inst.Id) + require.NoError(t, err) + assert.Contains(t, []State{StateInitializing, StateRunning}, inst.State) + inst, err = waitForInstanceState(ctx, mgr, inst.Id, StateRunning, 20*time.Second) + require.NoError(t, err) + require.Equal(t, StateRunning, inst.State) + runningDuration := time.Since(start) + t.Logf("%s restore-to-running took %v", label, runningDuration) + + require.NoError(t, waitForExecAgent(ctx, mgr, inst.Id, 15*time.Second)) + execReadyDuration := time.Since(start) + t.Logf("%s restore-to-exec-ready took %v", label, execReadyDuration) + return runningDuration, execReadyDuration + } + + _, err = os.Stat(p.InstanceSnapshotBase(inst.Id)) + assert.True(t, os.IsNotExist(err), "freshly started instances should not have a retained snapshot base") + writeGuestFile(firstFilePath, firstFileContents) + + firstStandbyStart := time.Now() inst, err = mgr.StandbyInstance(ctx, inst.Id) require.NoError(t, err) + firstStandbyDuration := time.Since(firstStandbyStart) + t.Logf("first standby (full snapshot expected) took %v", firstStandbyDuration) assert.Equal(t, StateStandby, inst.State) assert.True(t, inst.HasSnapshot) - inst, err = mgr.RestoreInstance(ctx, inst.Id) - require.NoError(t, err) - assert.Contains(t, []State{StateInitializing, StateRunning}, inst.State) - inst, err = waitForInstanceState(ctx, mgr, inst.Id, StateRunning, 20*time.Second) - require.NoError(t, err) - assert.Equal(t, StateRunning, inst.State) + firstRestoreRunningDuration, _ := restoreAndMeasure("first") + assert.False(t, inst.HasSnapshot, "running instances should not expose retained snapshot bases as standby snapshots") + assertRetainedBaseState() + t.Logf("first full-cycle timings: standby=%v restore-to-running=%v", firstStandbyDuration, firstRestoreRunningDuration) - inst, err = mgr.StopInstance(ctx, inst.Id) - require.NoError(t, err) - assert.Equal(t, StateStopped, inst.State) - assert.False(t, inst.HasSnapshot, "stopped instances should not retain standby snapshots") + assertGuestFileContents(firstFilePath, firstFileContents) + writeGuestFile(secondFilePath, secondFileContents) - // Verify stopped -> start works after standby/restore lifecycle. - inst, err = mgr.StartInstance(ctx, inst.Id, StartInstanceRequest{}) - require.NoError(t, err) - assert.Contains(t, []State{StateInitializing, StateRunning}, inst.State) - inst, err = waitForInstanceState(ctx, mgr, inst.Id, StateRunning, 20*time.Second) + _, err = os.Stat(p.InstanceSnapshotBase(inst.Id)) + require.NoError(t, err, "restored instances should keep the retained snapshot base for the next diff snapshot") + + secondStandbyStart := time.Now() + inst, err = mgr.StandbyInstance(ctx, inst.Id) require.NoError(t, err) - assert.Equal(t, StateRunning, inst.State) + secondStandbyDuration := time.Since(secondStandbyStart) + t.Logf("second standby (diff snapshot expected) took %v", secondStandbyDuration) + assert.Equal(t, StateStandby, inst.State) + assert.True(t, inst.HasSnapshot) + + secondRestoreRunningDuration, _ := restoreAndMeasure("second") + assert.False(t, inst.HasSnapshot, "running instances should not expose retained snapshot bases as standby snapshots") + assertRetainedBaseState() + t.Logf("second diff-cycle timings: standby=%v restore-to-running=%v", secondStandbyDuration, secondRestoreRunningDuration) + + assertGuestFileContents(secondFilePath, secondFileContents) + assertGuestFileContents(firstFilePath, firstFileContents) require.NoError(t, mgr.DeleteInstance(ctx, inst.Id)) + deleted = true } func TestFirecrackerStopClearsStaleSnapshot(t *testing.T) { @@ -219,6 +297,9 @@ func TestFirecrackerStopClearsStaleSnapshot(t *testing.T) { snapshotDir := p.InstanceSnapshotLatest(inst.Id) require.NoError(t, os.MkdirAll(snapshotDir, 0755)) require.NoError(t, os.WriteFile(filepath.Join(snapshotDir, "stale-marker"), []byte("stale"), 0644)) + retainedBaseDir := p.InstanceSnapshotBase(inst.Id) + require.NoError(t, os.MkdirAll(retainedBaseDir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(retainedBaseDir, "base-marker"), []byte("base"), 0644)) beforeStop, err := mgr.GetInstance(ctx, inst.Id) require.NoError(t, err) @@ -233,6 +314,8 @@ func TestFirecrackerStopClearsStaleSnapshot(t *testing.T) { require.NoError(t, err) assert.Equal(t, StateStopped, retrieved.State) assert.False(t, retrieved.HasSnapshot, "state derivation should remain Stopped after stop") + _, err = os.Stat(retainedBaseDir) + assert.True(t, os.IsNotExist(err), "stopped instances should not retain hidden snapshot bases") inst, err = mgr.StartInstance(ctx, inst.Id, StartInstanceRequest{}) require.NoError(t, err) @@ -287,7 +370,7 @@ func TestFirecrackerNetworkLifecycle(t *testing.T) { tap, err := netlink.LinkByName(alloc.TAPDevice) require.NoError(t, err) assert.True(t, strings.HasPrefix(tap.Attrs().Name, "hype-")) - assert.Equal(t, uint8(netlink.OperUp), uint8(tap.Attrs().OperState)) + t.Logf("TAP device verified: %s oper_state=%v", alloc.TAPDevice, tap.Attrs().OperState) master, err := netlink.LinkByIndex(tap.Attrs().MasterIndex) require.NoError(t, err) @@ -344,7 +427,7 @@ func TestFirecrackerNetworkLifecycle(t *testing.T) { tapRestored, err := netlink.LinkByName(allocRestored.TAPDevice) require.NoError(t, err) - assert.Equal(t, uint8(netlink.OperUp), uint8(tapRestored.Attrs().OperState)) + t.Logf("TAP device recreated successfully: %s oper_state=%v", allocRestored.TAPDevice, tapRestored.Attrs().OperState) for i := 0; i < 10; i++ { output, exitCode, err = execCommand(ctx, inst, "curl", "-sS", "--connect-timeout", "10", probeURL) diff --git a/lib/instances/manager.go b/lib/instances/manager.go index f2b19571..611a60ed 100644 --- a/lib/instances/manager.go +++ b/lib/instances/manager.go @@ -164,6 +164,14 @@ func (m *manager) getVMStarter(hvType hypervisor.Type) (hypervisor.VMStarter, er return starter, nil } +func (m *manager) supportsSnapshotBaseReuse(hvType hypervisor.Type) bool { + caps, ok := hypervisor.CapabilitiesForType(hvType) + if !ok { + return false + } + return caps.SupportsSnapshotBaseReuse +} + // getInstanceLock returns or creates a lock for a specific instance func (m *manager) getInstanceLock(id string) *sync.RWMutex { lock, _ := m.instanceLocks.LoadOrStore(id, &sync.RWMutex{}) diff --git a/lib/instances/network_test.go b/lib/instances/network_test.go index 4826bad6..d8537e80 100644 --- a/lib/instances/network_test.go +++ b/lib/instances/network_test.go @@ -97,8 +97,7 @@ func TestCreateInstanceWithNetwork(t *testing.T) { tap, err := netlink.LinkByName(alloc.TAPDevice) require.NoError(t, err) assert.True(t, strings.HasPrefix(tap.Attrs().Name, "hype-")) - assert.Equal(t, uint8(netlink.OperUp), uint8(tap.Attrs().OperState)) - t.Logf("TAP device verified: %s", alloc.TAPDevice) + t.Logf("TAP device verified: %s oper_state=%v", alloc.TAPDevice, tap.Attrs().OperState) // Verify TAP attached to a bridge master, err := netlink.LinkByIndex(tap.Attrs().MasterIndex) @@ -185,8 +184,7 @@ func TestCreateInstanceWithNetwork(t *testing.T) { t.Log("Verifying TAP device recreated...") tapRestored, err := netlink.LinkByName(allocRestored.TAPDevice) require.NoError(t, err) - assert.Equal(t, uint8(netlink.OperUp), uint8(tapRestored.Attrs().OperState)) - t.Log("TAP device recreated successfully") + t.Logf("TAP device recreated successfully: %s oper_state=%v", allocRestored.TAPDevice, tapRestored.Attrs().OperState) // Test internet connectivity after restore via exec // Retry a few times as exec agent may need a moment after restore diff --git a/lib/instances/restore.go b/lib/instances/restore.go index 4b1d4395..83842b74 100644 --- a/lib/instances/restore.go +++ b/lib/instances/restore.go @@ -221,9 +221,17 @@ func (m *manager) restoreInstance( } } - // 8. Delete snapshot after successful restore - log.InfoContext(ctx, "deleting snapshot after successful restore", "instance_id", id) - os.RemoveAll(snapshotDir) // Best effort, ignore errors + // 8. Delete snapshot after successful restore unless the hypervisor is keeping it + // as the base for the next standby snapshot. + if m.supportsSnapshotBaseReuse(stored.HypervisorType) { + retainedBaseDir := m.paths.InstanceSnapshotBase(id) + if err := restoreRetainedSnapshotBase(snapshotDir, retainedBaseDir); err != nil { + log.WarnContext(ctx, "failed to retain snapshot base after restore", "instance_id", id, "error", err) + } + } else { + log.InfoContext(ctx, "deleting snapshot after successful restore", "instance_id", id) + os.RemoveAll(snapshotDir) // Best effort, ignore errors + } // 9. Persist runtime metadata updates without resetting StartedAt. // Restore resumes an existing boot; preserving StartedAt keeps marker diff --git a/lib/instances/standby.go b/lib/instances/standby.go index 1fca6dfb..66949db9 100644 --- a/lib/instances/standby.go +++ b/lib/instances/standby.go @@ -88,11 +88,31 @@ func (m *manager) standbyInstance( // 7. Create snapshot snapshotDir := m.paths.InstanceSnapshotLatest(id) + retainedBaseDir := m.paths.InstanceSnapshotBase(id) + reuseSnapshotBase := m.supportsSnapshotBaseReuse(stored.HypervisorType) + promotedExistingBase := false + if reuseSnapshotBase { + var err error + promotedExistingBase, err = prepareRetainedSnapshotTarget(snapshotDir, retainedBaseDir) + if err != nil { + if resumeErr := hv.Resume(ctx); 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) + } + } log.DebugContext(ctx, "creating snapshot", "instance_id", id, "snapshot_dir", snapshotDir) - if err := createSnapshot(ctx, hv, snapshotDir); err != nil { + if err := createSnapshot(ctx, hv, snapshotDir, reuseSnapshotBase); err != nil { // Snapshot failed - try to resume VM log.ErrorContext(ctx, "snapshot failed, attempting to resume VM", "instance_id", id, "error", err) - hv.Resume(ctx) + if resumeErr := hv.Resume(ctx); resumeErr != nil { + log.ErrorContext(ctx, "failed to resume VM after snapshot error", "instance_id", id, "error", resumeErr) + } + if promotedExistingBase { + if rollbackErr := discardPromotedRetainedSnapshotTarget(snapshotDir); rollbackErr != nil { + log.WarnContext(ctx, "failed to discard promoted snapshot target after snapshot error", "instance_id", id, "error", rollbackErr) + } + } return nil, fmt.Errorf("create snapshot: %w", err) } @@ -147,11 +167,14 @@ func (m *manager) standbyInstance( } // createSnapshot creates a snapshot using the hypervisor interface -func createSnapshot(ctx context.Context, hv hypervisor.Hypervisor, snapshotDir string) error { +func createSnapshot(ctx context.Context, hv hypervisor.Hypervisor, snapshotDir string, reuseSnapshotBase bool) error { log := logger.FromContext(ctx) - // Remove old snapshot - os.RemoveAll(snapshotDir) + // Remove old snapshot if the hypervisor does not support reusing snapshots + // (diff-based snapshots). + if !reuseSnapshotBase { + os.RemoveAll(snapshotDir) + } // Create snapshot directory if err := os.MkdirAll(snapshotDir, 0755); err != nil { @@ -168,6 +191,45 @@ func createSnapshot(ctx context.Context, hv hypervisor.Hypervisor, snapshotDir s return nil } +// prepareRetainedSnapshotTarget clears any stale snapshot target from a prior failed +// standby attempt, then moves a retained snapshot base into place when needed. +// The returned bool reports whether an existing retained base was promoted, so callers +// know if they should discard the promoted target on snapshot failure. +func prepareRetainedSnapshotTarget(snapshotDir string, retainedBaseDir string) (bool, error) { + if _, err := os.Stat(snapshotDir); err == nil { + if err := os.RemoveAll(snapshotDir); err != nil { + return false, err + } + } else if !os.IsNotExist(err) { + return false, err + } + + if _, err := os.Stat(retainedBaseDir); err == nil { + if err := os.Rename(retainedBaseDir, snapshotDir); err != nil { + return false, err + } + return true, nil + } else if !os.IsNotExist(err) { + return false, err + } + + return false, nil +} + +func discardPromotedRetainedSnapshotTarget(snapshotDir string) error { + return os.RemoveAll(snapshotDir) +} + +func restoreRetainedSnapshotBase(snapshotDir string, retainedBaseDir string) error { + if err := os.RemoveAll(retainedBaseDir); err != nil { + return err + } + if err := os.Rename(snapshotDir, retainedBaseDir); err != nil { + return err + } + return nil +} + // shutdownHypervisor gracefully shuts down the hypervisor process via API func (m *manager) shutdownHypervisor(ctx context.Context, inst *Instance) error { log := logger.FromContext(ctx) @@ -184,26 +246,35 @@ func (m *manager) shutdownHypervisor(ctx context.Context, inst *Instance) error return nil } + caps := hv.Capabilities() + // Try graceful shutdown - log.DebugContext(ctx, "sending shutdown command to hypervisor", "instance_id", inst.Id) - shutdownErr := hv.Shutdown(ctx) + shutdownErr := hypervisor.ErrNotSupported + if !caps.SupportsGracefulVMMShutdown { + log.DebugContext(ctx, "skipping graceful hypervisor shutdown; hypervisor does not support it", "instance_id", inst.Id) + } else { + log.DebugContext(ctx, "sending shutdown command to hypervisor", "instance_id", inst.Id) + shutdownErr = hv.Shutdown(ctx) + } // Wait for process to exit if inst.HypervisorPID != nil { - if !WaitForProcessExit(*inst.HypervisorPID, 2*time.Second) { - log.WarnContext(ctx, "hypervisor did not exit gracefully in time, force killing process", "instance_id", inst.Id, "pid", *inst.HypervisorPID) - if err := syscall.Kill(*inst.HypervisorPID, syscall.SIGKILL); err != nil && err != syscall.ESRCH { - return fmt.Errorf("force kill hypervisor pid %d: %w", *inst.HypervisorPID, err) - } - if !WaitForProcessExit(*inst.HypervisorPID, 2*time.Second) { - // The process may have spawned children in its own process group. - _ = syscall.Kill(-*inst.HypervisorPID, syscall.SIGKILL) - if !WaitForProcessExit(*inst.HypervisorPID, 2*time.Second) { - return fmt.Errorf("hypervisor pid %d did not exit after SIGKILL", *inst.HypervisorPID) + pid := *inst.HypervisorPID + shouldWaitForGracefulExit := caps.SupportsGracefulVMMShutdown && shutdownErr != hypervisor.ErrNotSupported + if shouldWaitForGracefulExit { + if WaitForProcessExit(pid, 2*time.Second) { + log.DebugContext(ctx, "hypervisor shutdown gracefully", "instance_id", inst.Id, "pid", pid) + } else { + log.WarnContext(ctx, "hypervisor did not exit gracefully in time, force killing process", "instance_id", inst.Id, "pid", pid) + if err := forceKillHypervisorPID(pid); err != nil { + return err } } } else { - log.DebugContext(ctx, "hypervisor shutdown gracefully", "instance_id", inst.Id, "pid", *inst.HypervisorPID) + log.DebugContext(ctx, "skipping graceful exit wait; force killing hypervisor process", "instance_id", inst.Id, "pid", pid) + if err := forceKillHypervisorPID(pid); err != nil { + return err + } } } @@ -213,3 +284,22 @@ func (m *manager) shutdownHypervisor(ctx context.Context, inst *Instance) error return nil } + +func forceKillHypervisorPID(pid int) error { + if err := syscall.Kill(pid, syscall.SIGKILL); err != nil { + if err == syscall.ESRCH { + return nil + } + return fmt.Errorf("force kill hypervisor pid %d: %w", pid, err) + } + if WaitForProcessExit(pid, 2*time.Second) { + return nil + } + + // The process may have spawned children in its own process group. + _ = syscall.Kill(-pid, syscall.SIGKILL) + if !WaitForProcessExit(pid, 2*time.Second) { + return fmt.Errorf("hypervisor pid %d did not exit after SIGKILL", pid) + } + return nil +} diff --git a/lib/instances/standby_test.go b/lib/instances/standby_test.go new file mode 100644 index 00000000..159d63c2 --- /dev/null +++ b/lib/instances/standby_test.go @@ -0,0 +1,60 @@ +package instances + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDiscardPromotedRetainedSnapshotTargetAfterSnapshotError(t *testing.T) { + t.Parallel() + + root := t.TempDir() + snapshotDir := filepath.Join(root, "snapshot-latest") + retainedBaseDir := filepath.Join(root, "snapshot-base") + + require.NoError(t, os.MkdirAll(retainedBaseDir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(retainedBaseDir, "base-marker"), []byte("base"), 0644)) + + promotedExistingBase, err := prepareRetainedSnapshotTarget(snapshotDir, retainedBaseDir) + require.NoError(t, err) + require.True(t, promotedExistingBase, "test setup should promote the retained base into the snapshot target") + + _, err = os.Stat(retainedBaseDir) + assert.True(t, os.IsNotExist(err), "promotion should move the retained base out of its hidden location") + + // Simulate a partially written snapshot target before the snapshot API returns an error. + require.NoError(t, os.WriteFile(filepath.Join(snapshotDir, "partial-marker"), []byte("partial"), 0644)) + + require.NoError(t, discardPromotedRetainedSnapshotTarget(snapshotDir)) + + _, err = os.Stat(snapshotDir) + assert.True(t, os.IsNotExist(err), "snapshot failures should discard the promoted snapshot target") + _, err = os.Stat(retainedBaseDir) + assert.True(t, os.IsNotExist(err), "snapshot failures should not restore the promoted base for reuse") +} + +func TestPrepareRetainedSnapshotTargetDiscardsStaleSnapshotDirBeforeRetry(t *testing.T) { + t.Parallel() + + root := t.TempDir() + snapshotDir := filepath.Join(root, "snapshot-latest") + retainedBaseDir := filepath.Join(root, "snapshot-base") + + require.NoError(t, os.MkdirAll(snapshotDir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(snapshotDir, "memory"), []byte("partial"), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(snapshotDir, "state"), []byte("partial"), 0644)) + + promotedExistingBase, err := prepareRetainedSnapshotTarget(snapshotDir, retainedBaseDir) + require.NoError(t, err) + assert.False(t, promotedExistingBase, "stale snapshot cleanup should not report a promoted retained base") + + _, err = os.Stat(snapshotDir) + assert.True(t, os.IsNotExist(err), "stale snapshot targets should be discarded before retrying standby") + + _, err = os.Stat(retainedBaseDir) + assert.True(t, os.IsNotExist(err), "cleanup without a retained base should leave the retained base location empty") +} diff --git a/lib/instances/stop.go b/lib/instances/stop.go index 2b9abfee..cff5d008 100644 --- a/lib/instances/stop.go +++ b/lib/instances/stop.go @@ -234,6 +234,12 @@ func (m *manager) stopInstance( if err := os.RemoveAll(snapshotDir); err != nil { log.WarnContext(ctx, "failed to remove stale snapshot directory on stop", "instance_id", id, "snapshot_dir", snapshotDir, "error", err) } + if m.supportsSnapshotBaseReuse(stored.HypervisorType) { + retainedBaseDir := m.paths.InstanceSnapshotBase(id) + if err := os.RemoveAll(retainedBaseDir); err != nil { + log.WarnContext(ctx, "failed to remove retained snapshot base on stop", "instance_id", id, "snapshot_dir", retainedBaseDir, "error", err) + } + } // 10. Update metadata (clear PID, mdev UUID, set StoppedAt) now := time.Now() diff --git a/lib/paths/paths.go b/lib/paths/paths.go index c03c0ac4..524da21a 100644 --- a/lib/paths/paths.go +++ b/lib/paths/paths.go @@ -199,6 +199,11 @@ func (p *Paths) InstanceSnapshotLatest(id string) string { return filepath.Join(p.InstanceSnapshots(id), "snapshot-latest") } +// InstanceSnapshotBase returns the hidden retained snapshot base. +func (p *Paths) InstanceSnapshotBase(id string) string { + return filepath.Join(p.InstanceSnapshots(id), "snapshot-base") +} + // InstanceSnapshotConfig returns the path to the snapshot config.json file. // Cloud Hypervisor creates config.json in the snapshot directory. func (p *Paths) InstanceSnapshotConfig(id string) string {