Harden vGPU release ordering in the instance lifecycle

lib/instances/delete.go

@@ -85,6 +85,21 @@ func (m *manager) deleteInstanceWithOptions(
 		guest.CloseConn(dialer.Key())
 	}
 
+	// 3b. Block the restart policy before any teardown. If the delete fails
+	// partway (e.g. the hypervisor cannot be confirmed dead) the metadata is
+	// retained with the VMM already stopped, and without this marker the
+	// restart policy controller would start the instance again.
+	if err := m.markRestartManualStopLocked(ctx, id); err != nil {
+		return fmt.Errorf("block restart policy before delete: %w", err)
+	}
+	// markRestartManualStopLocked persists through a separate metadata load.
+	// Reload it so later saves in this delete do not overwrite the block.
+	meta, err = m.loadMetadata(id)
+	if err != nil {
+		return fmt.Errorf("reload metadata after blocking restart policy: %w", err)
+	}
+	stored = &meta.StoredMetadata
+
 	// 4. If active, try graceful guest shutdown before force kill.
 	gracefulShutdown := false
 	if !options.skipGracefulShutdown && (inst.State == StateRunning || inst.State == StateInitializing) {
@@ -125,6 +140,25 @@ func (m *manager) deleteInstanceWithOptions(
 	}
 	m.closeFirecrackerUFFDSession(ctx, stored)
 
+	// 5b. Release the vGPU assignment if present, before any network, device,
+	// or volume teardown. Release failure is logged and the delete continues,
+	// matching the pre-refactor contract: the VMM is already confirmed dead,
+	// the guards inside the release never destroy a device they cannot prove
+	// is unowned, and a skipped release is recovered by startup
+	// reconciliation.
+	hadVGPUAssignment := storedVGPUDevicePath(stored) != ""
+	if hadVGPUAssignment {
+		log.InfoContext(ctx, "destroying vGPU", "instance_id", id, "uuid", stored.GPUMdevUUID)
+	}
+	if err := releaseStoredVGPU(ctx, stored); err != nil {
+		// Log error but continue with cleanup.
+		log.WarnContext(ctx, "failed to destroy vGPU, continuing with cleanup", "instance_id", id, "uuid", stored.GPUMdevUUID, "error", err)
+	} else if hadVGPUAssignment {
+		if err := m.saveMetadata(meta); err != nil {
+			log.WarnContext(ctx, "failed to save metadata after vGPU release", "instance_id", id, "error", err)
+		}
+	}
+
 	// 6. Release network allocation
 	if inst.NetworkEnabled {
 		m.unregisterEgressProxyInstance(ctx, id)
@@ -170,15 +204,6 @@ func (m *manager) deleteInstanceWithOptions(
 		}
 	}
 
-	// 7c. Release the vGPU assignment if present.
-	if path := storedVGPUDevicePath(stored); path != "" {
-		log.InfoContext(ctx, "destroying vGPU", "instance_id", id, "device_path", path)
-		if err := releaseStoredVGPU(ctx, stored); err != nil {
-			// Log error but continue with cleanup
-			log.WarnContext(ctx, "failed to destroy vGPU, continuing with cleanup", "instance_id", id, "device_path", path, "error", err)
-		}
-	}
-
 	// 8. Delete all instance data
 	log.DebugContext(ctx, "deleting instance data", "instance_id", id)
 	_, dataSpanEnd := m.startLifecycleStep(ctx, "delete_instance_data",

lib/instances/fork.go

@@ -298,6 +298,11 @@ func (m *manager) forkInstanceFromStoppedOrStandby(ctx context.Context, id strin
 	// phase (Standby for snapshot forks, Stopped for stopped forks) will be
 	// recorded by the appropriate operation when the fork is acted on.
 	forkMeta.Phases.Reset()
+	// A vGPU assignment is never shared with a fork: normally stop already
+	// released it, and an assignment retained by a failed release must stay
+	// with the source so only one instance retries it. The fork acquires its
+	// own vGPU on start from GPUProfile.
+	clearStoredVGPUDevice(&forkMeta)
 	switch source.State {
 	case StateStandby:
 		forkMeta.Phases.Record(phasetracking.PhaseStandby, now)

lib/instances/fork_test.go

@@ -17,6 +17,7 @@ import (
 	"time"
 
 	"github.com/kernel/hypeman/lib/autostandby"
+	"github.com/kernel/hypeman/lib/devices"
 	"github.com/kernel/hypeman/lib/guest"
 	"github.com/kernel/hypeman/lib/healthcheck"
 	"github.com/kernel/hypeman/lib/hypervisor"
@@ -29,6 +30,39 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestForkInstanceClearsVGPUAssignment(t *testing.T) {
+	manager, _ := setupTestManager(t)
+	ctx := context.Background()
+	hvType := hypervisor.Type("fork-vgpu-test")
+	hypervisor.RegisterCapabilities(hvType, hypervisor.Capabilities{SupportsConcurrentForkPrepare: true})
+	manager.vmStarters[hvType] = concurrentForkPrepareTestStarter{}
+
+	sourceID := "fork-vgpu-source"
+	createStoppedSnapshotSourceFixture(t, manager, sourceID, sourceID, hvType)
+
+	// A retained assignment (release failed during stop) must stay with the
+	// source; the fork keeps only the profile and acquires its own vGPU on
+	// start.
+	meta, err := manager.loadMetadata(sourceID)
+	require.NoError(t, err)
+	meta.GPUProfile = "NVIDIA L40S-2Q"
+	meta.GPUFramework = devices.VGPUFramework("future-framework")
+	meta.GPUDevicePath = "/sys/bus/pci/devices/0000:82:00.4"
+	meta.GPUMdevUUID = "retained-uuid"
+	require.NoError(t, manager.saveMetadata(meta))
+
+	forked, err := manager.ForkInstance(ctx, sourceID, ForkInstanceRequest{Name: "fork-vgpu-copy"})
+	require.NoError(t, err)
+	assert.Equal(t, "NVIDIA L40S-2Q", forked.GPUProfile)
+	assert.Equal(t, devices.VGPUFrameworkNone, forked.GPUFramework)
+	assert.Empty(t, forked.GPUDevicePath)
+	assert.Empty(t, forked.GPUMdevUUID)
+
+	source, err := manager.loadMetadata(sourceID)
+	require.NoError(t, err)
+	assert.Equal(t, "/sys/bus/pci/devices/0000:82:00.4", source.GPUDevicePath)
+}
+
 func TestForkInstance_VZStoppedSourceSupported(t *testing.T) {
 	t.Parallel()
 	manager, _ := setupTestManager(t)

lib/instances/lifecycle_noop_test.go

@@ -9,8 +9,10 @@ import (
 	"testing"
 	"time"
 
+	"github.com/kernel/hypeman/lib/devices"
 	"github.com/kernel/hypeman/lib/hypervisor"
 	"github.com/kernel/hypeman/lib/paths"
+	restartpolicy "github.com/kernel/hypeman/lib/restart-policy"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -147,6 +149,157 @@ func TestLifecycleNoopStandbyWithOptionsStillRejectsStandbyInstance(t *testing.T
 	assertNoLifecycleEvent(t, events)
 }
 
+func TestDeleteContinuesWhenVGPUReleaseFails(t *testing.T) {
+	m, id := newLifecycleNoopManagerWithInstance(t, StateStopped, time.Now().UTC())
+	meta, err := m.loadMetadata(id)
+	require.NoError(t, err)
+	meta.GPUProfile = "NVIDIA L40S-2Q"
+	meta.GPUFramework = devices.VGPUFramework("future-framework")
+	meta.GPUDevicePath = "/sys/bus/pci/devices/0000:82:00.4"
+	require.NoError(t, m.saveMetadata(meta))
+
+	// A failed release is logged and the delete continues, matching the
+	// pre-refactor contract; the leaked assignment is recovered by startup
+	// reconciliation.
+	require.NoError(t, m.DeleteInstance(context.Background(), id))
+
+	_, err = m.loadMetadata(id)
+	require.Error(t, err, "instance data must be deleted despite the failed release")
+}
+
+func TestDeletePersistsVGPUReleaseBeforeTeardown(t *testing.T) {
+	m, id := newLifecycleNoopManagerWithInstance(t, StateStopped, time.Now().UTC())
+	var persisted *metadata
+	deviceManager := &recordingDeviceManager{
+		onMarkDetached: func() {
+			var err error
+			persisted, err = m.loadMetadata(id)
+			require.NoError(t, err)
+		},
+	}
+	m.deviceManager = deviceManager
+	meta, err := m.loadMetadata(id)
+	require.NoError(t, err)
+	meta.RestartPolicy = &restartpolicy.Policy{Policy: restartpolicy.PolicyAlways}
+	meta.GPUProfile = "NVIDIA L40S-2Q"
+	meta.GPUDevicePath = "/sys/bus/mdev/devices/test-mdev"
+	meta.GPUMdevUUID = "test-mdev"
+	meta.Devices = []string{"dev-1"}
+	require.NoError(t, m.saveMetadata(meta))
+
+	require.NoError(t, m.DeleteInstance(context.Background(), id))
+	require.NotNil(t, persisted)
+	assert.Empty(t, persisted.GPUDevicePath)
+	assert.Empty(t, persisted.GPUMdevUUID)
+	assert.Equal(t, "NVIDIA L40S-2Q", persisted.GPUProfile)
+	assert.Equal(t, restartpolicy.BlockedReasonManualStop, persisted.RestartStatus.BlockedReason)
+}
+
+func TestDeleteContinuesTeardownAfterFailedVGPURelease(t *testing.T) {
+	m, id := newLifecycleNoopManagerWithInstance(t, StateStopped, time.Now().UTC())
+	deviceManager := &recordingDeviceManager{}
+	m.deviceManager = deviceManager
+	meta, err := m.loadMetadata(id)
+	require.NoError(t, err)
+	meta.GPUProfile = "NVIDIA L40S-2Q"
+	meta.GPUFramework = devices.VGPUFramework("future-framework")
+	meta.GPUDevicePath = "/sys/bus/pci/devices/0000:82:00.4"
+	meta.Devices = []string{"dev-1"}
+	require.NoError(t, m.saveMetadata(meta))
+
+	// The failed release must not block the rest of the teardown: devices
+	// are detached and the instance is fully deleted.
+	require.NoError(t, m.DeleteInstance(context.Background(), id))
+	assert.Equal(t, []string{"dev-1"}, deviceManager.detached)
+
+	_, err = m.loadMetadata(id)
+	require.Error(t, err, "instance data must be deleted despite the failed release")
+}
+
+// A stale release during start must be persisted immediately: if start fails
+// later (here at vGPU recreation on a host without VFs), the on-disk metadata
+// must no longer point at the already-released device.
+func TestStartPersistsStaleVGPUReleaseImmediately(t *testing.T) {
+	m, id := newLifecycleNoopManagerWithInstance(t, StateStopped, time.Now().UTC())
+	m.imageManager = readyFixtureImageManager{name: "test-image"}
+	meta, err := m.loadMetadata(id)
+	require.NoError(t, err)
+	meta.GPUProfile = "NVIDIA L40S-2Q"
+	meta.HypervisorType = hypervisor.TypeQEMU
+	meta.GPUFramework = devices.VGPUFrameworkNone
+	meta.GPUDevicePath = "/sys/bus/pci/devices/0000:82:00.4"
+	require.NoError(t, m.saveMetadata(meta))
+
+	_, err = m.StartInstance(context.Background(), id, StartInstanceRequest{})
+	require.Error(t, err)
+
+	stored, err := m.loadMetadata(id)
+	require.NoError(t, err)
+	assert.Empty(t, stored.GPUDevicePath, "released assignment should be persisted despite the failed start")
+	assert.Equal(t, "NVIDIA L40S-2Q", stored.GPUProfile, "profile is kept for the next start")
+}
+
+func TestStopStoppedInstanceReleasesRetainedVGPU(t *testing.T) {
+	m, id := newLifecycleNoopManagerWithInstance(t, StateStopped, time.Now().UTC())
+	meta, err := m.loadMetadata(id)
+	require.NoError(t, err)
+	meta.GPUProfile = "NVIDIA L40S-2Q"
+	meta.GPUFramework = devices.VGPUFrameworkNone
+	meta.GPUDevicePath = "/sys/bus/pci/devices/0000:82:00.4"
+	require.NoError(t, m.saveMetadata(meta))
+
+	inst, err := m.StopInstance(context.Background(), id)
+	require.NoError(t, err)
+	require.NotNil(t, inst)
+	assert.Equal(t, StateStopped, inst.State)
+
+	stored, err := m.loadMetadata(id)
+	require.NoError(t, err)
+	assert.Empty(t, stored.GPUDevicePath)
+}
+
+func TestStopStoppedInstanceVGPUReleaseFailureRemainsNoop(t *testing.T) {
+	m, id := newLifecycleNoopManagerWithInstance(t, StateStopped, time.Now().UTC())
+	meta, err := m.loadMetadata(id)
+	require.NoError(t, err)
+	meta.GPUProfile = "NVIDIA L40S-2Q"
+	meta.GPUFramework = devices.VGPUFramework("future-framework")
+	meta.GPUDevicePath = "/sys/bus/pci/devices/0000:82:00.4"
+	require.NoError(t, m.saveMetadata(meta))
+
+	inst, err := m.StopInstance(context.Background(), id)
+	require.NoError(t, err)
+	require.NotNil(t, inst)
+	assert.Equal(t, StateStopped, inst.State)
+
+	stored, err := m.loadMetadata(id)
+	require.NoError(t, err)
+	assert.Equal(t, devices.VGPUFramework("future-framework"), stored.GPUFramework)
+	assert.Equal(t, "/sys/bus/pci/devices/0000:82:00.4", stored.GPUDevicePath)
+}
+
+// recordingDeviceManager is a devices.Manager stub that records passthrough
+// teardown calls. Only the methods delete exercises are implemented.
+type recordingDeviceManager struct {
+	devices.Manager
+	detached       []string
+	unbound        []string
+	onMarkDetached func()
+}
+
+func (m *recordingDeviceManager) MarkDetached(ctx context.Context, deviceID string) error {
+	m.detached = append(m.detached, deviceID)
+	if m.onMarkDetached != nil {
+		m.onMarkDetached()
+	}
+	return nil
+}
+
+func (m *recordingDeviceManager) UnbindFromVFIO(ctx context.Context, id string) error {
+	m.unbound = append(m.unbound, id)
+	return nil
+}
+
 func newLifecycleNoopManagerWithInstance(t *testing.T, state State, now time.Time) (*manager, string) {
 	t.Helper()
 

lib/instances/manager.go

@@ -621,6 +621,12 @@ func (m *manager) StopInstance(ctx context.Context, id string) (*Instance, error
 		if err := m.markRestartManualStopLocked(ctx, id); err != nil {
 			return nil, err
 		}
+		// A stopped instance can retain a vGPU assignment when the release
+		// failed during the original stop. Retry it here so the vGPU slot is
+		// not held until the next start, delete, or hypeman restart. A failed
+		// retry only logs, keeping stop's no-op contract for already-stopped
+		// instances.
+		m.releaseRetainedVGPULocked(ctx, id)
 		updated, err := m.currentInstanceWithoutHydration(ctx, id)
 		if err != nil {
 			return nil, err

lib/instances/snapshot.go

@@ -305,6 +305,12 @@ func (m *manager) restoreSnapshot(ctx context.Context, id string, snapshotID str
 	restored.StoppedAt = nil
 	restored.ExitCode = nil
 	restored.ExitMessage = ""
+	// vGPU assignments are live host state, not snapshot payload: keep the
+	// instance's current assignment (possibly retained from a failed release)
+	// instead of resurrecting the one embedded in the snapshot.
+	restored.GPUFramework = sourceMeta.GPUFramework
+	restored.GPUDevicePath = sourceMeta.GPUDevicePath
+	restored.GPUMdevUUID = sourceMeta.GPUMdevUUID
 	restored.HypervisorType = targetHypervisor
 	restored.HypervisorVersion = targetHypervisorVersion
 	restored.SocketPath = m.paths.InstanceSocket(id, starter.SocketName())
@@ -433,6 +439,7 @@ func (m *manager) forkSnapshot(ctx context.Context, snapshotID string, req ForkS
 	forkMeta.ExitCode = nil
 	forkMeta.ExitMessage = ""
 	forkMeta.RestartStatus = restartpolicy.Status{}
+	clearStoredVGPUDevice(&forkMeta)
 	forkMeta.FirecrackerUFFDSessionID = ""
 	forkMeta.FirecrackerUFFDPagerVersion = ""
 	forkMeta.FirecrackerUseUFFDOnNextRestore = useFirecrackerUFFDOnNextRestore(targetHypervisor, rec.Snapshot.Kind == SnapshotKindStandby, targetState)