From d6cfd22ddb39e314a0b352da3b0dbe9f9b008c3c Mon Sep 17 00:00:00 2001
From: Josh Siviour <mrk@mulgadc.com>
Date: Mon, 29 Jun 2026 21:05:06 +1000
Subject: [PATCH] fix: re-assert boot-volume in-use state on daemon-restart
 reconnect (#441)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

reconnectInstance re-asserted the instance running state but never the boot
volume's in-use state, so after a daemon restart describe-instances showed the
VM running while describe-volumes showed its root volume available — a
split-brain that makes start-instances attempt to double-launch a live QEMU.

Extract the launch path's boot-volume promotion into markBootVolumesInUse and
call it from reconnectInstance so launch and recovery keep volume state
consistent. Add a reconnect unit test asserting the boot volume is marked
in-use and non-boot volumes are left to their own attach flow.
---
 spinifex/vm/lifecycle.go    | 32 +++++++++++++++++++++-----------
 spinifex/vm/restore.go      |  4 ++++
 spinifex/vm/restore_test.go | 29 +++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+), 11 deletions(-)

diff --git a/spinifex/vm/lifecycle.go b/spinifex/vm/lifecycle.go
index 7dd4e97b0..78f840f1b 100644
--- a/spinifex/vm/lifecycle.go
+++ b/spinifex/vm/lifecycle.go
@@ -160,17 +160,7 @@ func (m *Manager) launch(instance *VM) error {
 	}
 
 	// Mark boot volumes as "in-use" now that instance is confirmed running.
-	if m.deps.VolumeStateUpdater != nil {
-		instance.EBSRequests.Mu.Lock()
-		for _, ebsReq := range instance.EBSRequests.Requests {
-			if ebsReq.Boot {
-				if err := m.deps.VolumeStateUpdater.UpdateVolumeState(ebsReq.Name, "in-use", instance.ID, ""); err != nil {
-					slog.Error("Failed to update volume state to in-use", "volumeId", ebsReq.Name, "err", err)
-				}
-			}
-		}
-		instance.EBSRequests.Mu.Unlock()
-	}
+	m.markBootVolumesInUse(instance)
 
 	if m.deps.Hooks.OnInstanceUp != nil {
 		// Launch path: per-instance subscribe failures are logged and the
@@ -186,6 +176,26 @@ func (m *Manager) launch(instance *VM) error {
 	return nil
 }
 
+// markBootVolumesInUse re-asserts "in-use" status for an instance's boot
+// volumes once it is confirmed running. Used by the launch path and the
+// daemon-restart reconnect path so both keep volume state consistent with a
+// running instance. Errors are logged, not fatal.
+func (m *Manager) markBootVolumesInUse(instance *VM) {
+	if m.deps.VolumeStateUpdater == nil {
+		return
+	}
+	instance.EBSRequests.Mu.Lock()
+	defer instance.EBSRequests.Mu.Unlock()
+	for _, ebsReq := range instance.EBSRequests.Requests {
+		if !ebsReq.Boot {
+			continue
+		}
+		if err := m.deps.VolumeStateUpdater.UpdateVolumeState(ebsReq.Name, "in-use", instance.ID, ""); err != nil {
+			slog.Error("Failed to update volume state to in-use", "volumeId", ebsReq.Name, "err", err)
+		}
+	}
+}
+
 const (
 	nbdReadyTimeout    = 5 * time.Second
 	qemuStartupTimeout = 5 * time.Second
diff --git a/spinifex/vm/restore.go b/spinifex/vm/restore.go
index c9cd9ada0..192b2bcb6 100644
--- a/spinifex/vm/restore.go
+++ b/spinifex/vm/restore.go
@@ -318,6 +318,10 @@ func (m *Manager) reconnectInstance(instance *VM) error {
 
 	instance.Status = StateRunning
 
+	// Re-assert boot-volume in-use state; a daemon restart otherwise leaves the
+	// root volume "available" while the instance runs (split-brain, siv-464).
+	m.markBootVolumesInUse(instance)
+
 	if err := m.writeRunningState(); err != nil {
 		return fmt.Errorf("failed to persist reconnected instance state: %w", err)
 	}
diff --git a/spinifex/vm/restore_test.go b/spinifex/vm/restore_test.go
index 8262a825e..09a051cbc 100644
--- a/spinifex/vm/restore_test.go
+++ b/spinifex/vm/restore_test.go
@@ -885,6 +885,35 @@ func TestReconnectInstance(t *testing.T) {
 		assert.NotNil(t, saved["i-no-hook"], "the reconnected instance must appear in the persisted snapshot")
 	})
 
+	t.Run("reconnect re-asserts boot-volume in-use, skips non-boot", func(t *testing.T) {
+		stateUpdater := &fakeVolumeStateUpdater{}
+		m := NewManager()
+		m.SetDeps(Deps{
+			NodeID:             "test-node",
+			StateStore:         newFakeStateStore(),
+			VolumeStateUpdater: stateUpdater,
+		})
+
+		orig := attachQMPForReconnect
+		t.Cleanup(func() { attachQMPForReconnect = orig })
+		attachQMPForReconnect = func(_ *Manager, v *VM) error { fakeAttachQMP(t, v); return nil }
+
+		instance := &VM{ID: "i-reconnect-vol", Status: StatePending}
+		instance.EBSRequests.Requests = []types.EBSRequest{
+			{Name: "vol-root", Boot: true},
+			{Name: "vol-data", Boot: false},
+		}
+		m.Insert(instance)
+
+		require.NoError(t, m.reconnectInstance(instance))
+
+		calls := stateUpdater.snapshot()
+		require.Len(t, calls, 1,
+			"reconnect must re-assert exactly the boot volume — a daemon restart otherwise leaves the root volume 'available' while the VM runs (siv-464)")
+		assert.Equal(t, volumeStateUpdate{VolumeID: "vol-root", State: "in-use", InstanceID: "i-reconnect-vol"}, calls[0],
+			"the boot volume must be marked in-use under the running instance; non-boot volumes are owned by their own attach flow")
+	})
+
 	t.Run("AttachQMP succeeds, OnInstanceUp errors: QMP closed and nil'd", func(t *testing.T) {
 		m := NewManager()
 		hookErr := errors.New("nats subscribe failed")