review feedback

oxidecomputer · May 23, 2024 · 1450eee · 1450eee
1 parent 66fe92d
commit 1450eee
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 11 deletions.
diff --git a/nexus/db-queries/src/db/datastore/vmm.rs b/nexus/db-queries/src/db/datastore/vmm.rs
@@ -179,10 +179,9 @@ impl DataStore {
     /// A VMM is considered "abandoned" if (and only if):
     ///
     /// - It is in the `Destroyed` state.
-    /// - It has previously been assigned to an instance.
-    /// - It is not currently running the instance, and it is also not the
-    ///   migration target of that instance (i.e. it is no longer pointed to by
-    ///   the instance record's `active_propolis_id` and `target_propolis_id`
+    /// - It is not currently running an instance, and it is also not the
+    ///   migration target of any instance (i.e. it is not pointed to by
+    ///   any instance record's `active_propolis_id` and `target_propolis_id`
     ///   fields).
     /// - It has not been deleted yet.
     pub async fn vmm_list_abandoned(
@@ -198,7 +197,8 @@ impl DataStore {
             .filter(dsl::state.eq(destroyed))
             // - not deleted yet
             .filter(dsl::time_deleted.is_null())
-            // - not pointed to by their corresponding instances
+            // - not pointed to by any instance's `active_propolis_id` or
+            //   `target_propolis_id`.
             .left_join(
                 instance_dsl::instance
                     .on(instance_dsl::id.eq(dsl::instance_id)),

diff --git a/nexus/src/app/background/abandoned_vmm_reaper.rs b/nexus/src/app/background/abandoned_vmm_reaper.rs
@@ -7,10 +7,9 @@
 //! A VMM is considered "abandoned" if (and only if):
 //!
 //! - It is in the `Destroyed` state.
-//! - It has previously been assigned to an instance.
-//! - It is not currently running the instance, and it is also not the
-//!   migration target of that instance (i.e. it is no longer pointed to by
-//!   the instance record's `active_propolis_id` and `target_propolis_id`
+//! - It is not currently running an instance, and it is also not the
+//!   migration target of any instance (i.e. it is not pointed to by
+//!   any instance record's `active_propolis_id` and `target_propolis_id`
 //!   fields).
 //! - It has not been deleted yet.
 
@@ -113,7 +112,7 @@ impl AbandonedVmmReaper {
                     results.error_count += 1;
                     *last_err = Err(e).with_context(|| {
                         format!(
-                            "failed to delete sled reservation for {vmm_id}"
+                            "failed to delete sled reservation for VMM {vmm_id}"
                         )
                     });
                 }
@@ -146,7 +145,7 @@ impl AbandonedVmmReaper {
                     );
                     results.error_count += 1;
                     *last_err = Err(e).with_context(|| {
-                        format!("failed to mark {vmm_id} as deleted")
+                        format!("failed to mark VMM {vmm_id} as deleted")
                     });
                 }
             }