From 1450eee4c20f012bc0c19dbede7b9d2ffb518a4a Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Thu, 23 May 2024 16:00:48 -0700 Subject: [PATCH] review feedback --- nexus/db-queries/src/db/datastore/vmm.rs | 10 +++++----- nexus/src/app/background/abandoned_vmm_reaper.rs | 11 +++++------ 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/vmm.rs b/nexus/db-queries/src/db/datastore/vmm.rs index d25815731d..10c368d573 100644 --- 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 index 2ab3a83da7..5641580c17 100644 --- 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") }); } }