okay FINE (i really hope this is safe)

nexus/db-model/src/vmm_state.rs

@@ -48,7 +48,7 @@ impl VmmState {
     /// States in which it is safe to deallocate a VMM's sled resources and mark
     /// it as deleted.
     pub const DESTROYABLE_STATES: &'static [Self] =
-        &[Self::Destroyed, Self::SagaUnwound];
+        &[Self::Destroyed, Self::Failed, Self::SagaUnwound];
 
     pub const TERMINAL_STATES: &'static [Self] =
         &[Self::Destroyed, Self::Failed];

nexus/db-queries/src/db/datastore/vmm.rs

@@ -82,11 +82,9 @@ impl DataStore {
         opctx: &OpContext,
         vmm_id: &PropolisUuid,
     ) -> UpdateResult<bool> {
-        let valid_states = vec![DbVmmState::Destroyed, DbVmmState::Failed];
-
         let updated = diesel::update(dsl::vmm)
             .filter(dsl::id.eq(vmm_id.into_untyped_uuid()))
-            .filter(dsl::state.eq_any(valid_states))
+            .filter(dsl::state.eq_any(DbVmmState::DESTROYABLE_STATES))
             .filter(dsl::time_deleted.is_null())
             .set(dsl::time_deleted.eq(Utc::now()))
             .check_if_exists::<Vmm>(vmm_id.into_untyped_uuid())
@@ -307,6 +305,40 @@ impl DataStore {
             })
     }
 
+    /// Transitions a VMM to the `SagaUnwound` state.
+    ///
+    /// This may *only* be called by the saga that created a VMM record.
+    pub async fn vmm_mark_saga_unwound(
+        &self,
+        opctx: &OpContext,
+        vmm_id: &PropolisUuid,
+    ) -> Result<bool, Error> {
+        diesel::update(dsl::vmm)
+            .filter(dsl::time_deleted.is_null())
+            .filter(dsl::id.eq(vmm_id.into_untyped_uuid()))
+            .set((
+                dsl::state.eq(DbVmmState::SagaUnwound),
+                dsl::time_state_updated.eq(chrono::Utc::now()),
+                dsl::state_generation.eq(dsl::state_generation + 1),
+            ))
+            .check_if_exists::<Vmm>(vmm_id.into_untyped_uuid())
+            .execute_and_check(&*self.pool_connection_authorized(opctx).await?)
+            .await
+            .map(|r| match r.status {
+                UpdateStatus::Updated => true,
+                UpdateStatus::NotUpdatedButExists => false,
+            })
+            .map_err(|e| {
+                public_error_from_diesel(
+                    e,
+                    ErrorHandler::NotFoundByLookup(
+                        ResourceType::Vmm,
+                        LookupType::ById(vmm_id.into_untyped_uuid()),
+                    ),
+                )
+            })
+    }
+
     /// Forcibly overwrites the Propolis IP/Port in the supplied VMM's record with
     /// the supplied Propolis IP.
     ///
@@ -357,7 +389,7 @@ impl DataStore {
 
         paginated(dsl::vmm, dsl::id, pagparams)
             // In order to be considered "abandoned", a VMM must be:
-            // - in the `Destroyed` or `SagaUnwound` state
+            // - in the `Destroyed`, `SagaUnwound`, or `Failed` states
             .filter(dsl::state.eq_any(DbVmmState::DESTROYABLE_STATES))
             // - not deleted yet
             .filter(dsl::time_deleted.is_null())

nexus/src/app/sagas/instance_common.rs

@@ -7,7 +7,6 @@
 use std::net::{IpAddr, Ipv6Addr};
 
 use crate::Nexus;
-use chrono::Utc;
 use nexus_db_model::{
     ByteCount, ExternalIp, InstanceState, IpAttachState, Ipv4NatEntry,
     SledReservationConstraints, SledResource, VmmState,
@@ -115,32 +114,6 @@ pub async fn create_and_insert_vmm_record(
     Ok(vmm)
 }
 
-/// Given a previously-inserted VMM record, set its state to `SagaUnwound` and
-/// then delete it.
-///
-/// This function succeeds idempotently if called with the same parameters,
-/// provided that the VMM record was not changed by some other actor after the
-/// calling saga inserted it.
-///
-/// This function is intended to be called when a saga which created a VMM
-/// record unwinds.
-pub async fn unwind_vmm_record(
-    datastore: &DataStore,
-    opctx: &OpContext,
-    prev_record: &db::model::Vmm,
-) -> Result<(), anyhow::Error> {
-    let new_runtime = db::model::VmmRuntimeState {
-        state: db::model::VmmState::SagaUnwound,
-        time_state_updated: Utc::now(),
-        gen: prev_record.runtime.gen.next().into(),
-    };
-
-    let prev_id = PropolisUuid::from_untyped_uuid(prev_record.id);
-    datastore.vmm_update_runtime(&prev_id, &new_runtime).await?;
-    datastore.vmm_mark_deleted(&opctx, &prev_id).await?;
-    Ok(())
-}
-
 /// Allocates a new IPv6 address for a propolis instance that will run on the
 /// supplied sled.
 pub(super) async fn allocate_vmm_ipv6(

nexus/src/app/sagas/instance_migrate.rs

@@ -342,18 +342,15 @@ async fn sim_destroy_vmm_record(
         &params.serialized_authn,
     );
 
-    let vmm = sagactx
-        .lookup::<db::model::Vmm>(REGISTERED_VMM_RECORD)
-        .or_else(|_| sagactx.lookup::<db::model::Vmm>(INITIAL_VMM_RECORD))?;
-    info!(osagactx.log(), "destroying vmm record for migration unwind";
-          "propolis_id" => %vmm.id);
+    let propolis_id = sagactx.lookup::<PropolisUuid>("dst_propolis_id")?;
+    info!(
+        osagactx.log(),
+        "destroying vmm record for migration unwind";
+        "propolis_id" => %propolis_id,
+    );
 
-    super::instance_common::unwind_vmm_record(
-        osagactx.datastore(),
-        &opctx,
-        &vmm,
-    )
-    .await
+    osagactx.datastore().vmm_mark_saga_unwound(&opctx, &propolis_id).await?;
+    Ok(())
 }
 
 async fn sim_set_migration_ids(

nexus/src/app/sagas/instance_start.rs

@@ -217,15 +217,15 @@ async fn sis_destroy_vmm_record(
         &params.serialized_authn,
     );
 
-    let vmm = sagactx
-        .lookup::<db::model::Vmm>(REGISTERED_VMM_RECORD)
-        .or_else(|_| sagactx.lookup::<db::model::Vmm>(INITIAL_VMM_RECORD))?;
-    super::instance_common::unwind_vmm_record(
-        osagactx.datastore(),
-        &opctx,
-        &vmm,
-    )
-    .await
+    let propolis_id = sagactx.lookup::<PropolisUuid>("propolis_id")?;
+    info!(
+        osagactx.log(),
+        "destroying vmm record for start saga unwind";
+        "propolis_id" => %propolis_id,
+    );
+
+    osagactx.datastore().vmm_mark_saga_unwound(&opctx, &propolis_id).await?;
+    Ok(())
 }
 
 async fn sis_move_to_starting(