From 6f7c8169f396b86dd17f8ac8e72db253934e5449 Mon Sep 17 00:00:00 2001 From: John Baublitz Date: Thu, 16 Jun 2022 13:30:40 -0400 Subject: [PATCH] PR revisions --- src/dbus_api/api/manager_3_2/methods.rs | 10 +++++++-- src/dbus_api/tree.rs | 2 +- src/engine/sim_engine/engine.rs | 22 +++++++++++++++---- src/engine/strat_engine/backstore/devices.rs | 6 ++++- src/engine/strat_engine/engine.rs | 2 +- .../strat_engine/liminal/device_info.rs | 4 ++-- src/engine/types/mod.rs | 4 ++-- 7 files changed, 37 insertions(+), 13 deletions(-) diff --git a/src/dbus_api/api/manager_3_2/methods.rs b/src/dbus_api/api/manager_3_2/methods.rs index be081b6f10e..86465b1d4e2 100644 --- a/src/dbus_api/api/manager_3_2/methods.rs +++ b/src/dbus_api/api/manager_3_2/methods.rs @@ -148,7 +148,13 @@ where } }; - let is_encrypted = block_on(dbus_context.engine.get_pool(LockKey::Uuid(pool_uuid))) + // If Some(_), send a locked pool property change signal only if the pool is + // encrypted. If None, the pool may already be stopped or not exist at all. + // Both of these cases are handled by stop_pool and the value we provide + // for send_locked_signal does not matter as send_locked_signal is only + // used when a pool is newly stopped which can only occur if the pool is found + // here. + let send_locked_signal = block_on(dbus_context.engine.get_pool(LockKey::Uuid(pool_uuid))) .map(|g| { let (_, _, p) = g.as_tuple(); p.is_encrypted() @@ -158,7 +164,7 @@ where let msg = match handle_action!(block_on(dbus_context.engine.stop_pool(pool_uuid))) { Ok(StopAction::Stopped(_)) => { dbus_context.push_remove(&pool_path, consts::pool_interface_list()); - if is_encrypted { + if send_locked_signal { dbus_context.push_locked_pools(block_on(dbus_context.engine.locked_pools())); } dbus_context.push_stopped_pools(block_on(dbus_context.engine.stopped_pools())); diff --git a/src/dbus_api/tree.rs b/src/dbus_api/tree.rs index 8afb3d93029..b2574f31d33 100644 --- a/src/dbus_api/tree.rs +++ b/src/dbus_api/tree.rs @@ -398,7 +398,7 @@ where .property_changed_invalidated_signal( &Path::new(consts::STRATIS_BASE_PATH).expect("Valid path"), prop_hashmap! { - consts::MANAGER_INTERFACE_NAME_3_1 => { + consts::MANAGER_INTERFACE_NAME_3_2 => { Vec::new(), consts::STOPPED_POOLS_PROP.to_string() => box_variant!(stopped_pools_to_prop(&stopped_pools)) diff --git a/src/engine/sim_engine/engine.rs b/src/engine/sim_engine/engine.rs index fc437978157..f28ef67fff4 100644 --- a/src/engine/sim_engine/engine.rs +++ b/src/engine/sim_engine/engine.rs @@ -98,10 +98,24 @@ impl Report for SimEngine { fn get_report(&self, report_type: ReportType) -> Value { match report_type { - ReportType::ErroredPoolDevices => json!({ - "errored_pools": json!([]), - "hopeless_devices": json!([]), - }), + ReportType::StoppedPools => { + json!({ + "stopped_pools": (&*block_on(self.stopped_pools.read())).iter().map(|(name, uuid, pool)| { + let json = json!({ + "pool_uuid": uuid.to_string(), + "name": name.to_string(), + }); + let pool_json = pool.into(); + if let (Value::Object(mut map), Value::Object(submap)) = (json, pool_json) { + map.extend(submap.into_iter()); + Value::Object(map) + } else { + unreachable!("json!() output is always JSON object"); + } + }) + .collect::>() + }) + } } } } diff --git a/src/engine/strat_engine/backstore/devices.rs b/src/engine/strat_engine/backstore/devices.rs index a6d7a969a11..6cff7b6f689 100644 --- a/src/engine/strat_engine/backstore/devices.rs +++ b/src/engine/strat_engine/backstore/devices.rs @@ -106,7 +106,11 @@ fn get_devno_from_path(path: &Path) -> StratisResult { Ok(Device::from_kdev_t(convert_int!(info.st_rdev, u64, u32)?)) } -/// Find all devices that match the given +/// Find all devices that match the given pool and device UUIDs using libblkid. +/// +/// This method is specifically a work around for cases where due to locking +/// internally in stratisd, udev events cannot be used for device identification +/// because they will not have the opportunity to be processed. pub fn find_stratis_devs_by_uuid( pool_uuid: PoolUuid, uuids: Vec, diff --git a/src/engine/strat_engine/engine.rs b/src/engine/strat_engine/engine.rs index f961b35846d..a0ae92d7e0b 100644 --- a/src/engine/strat_engine/engine.rs +++ b/src/engine/strat_engine/engine.rs @@ -291,7 +291,7 @@ impl Report for StratEngine { fn get_report(&self, report_type: ReportType) -> Value { match report_type { - ReportType::ErroredPoolDevices => (&*self.liminal_devices.blocking_read()).into(), + ReportType::StoppedPools => (&*self.liminal_devices.blocking_read()).into(), } } } diff --git a/src/engine/strat_engine/liminal/device_info.rs b/src/engine/strat_engine/liminal/device_info.rs index 8e17c02f43b..afaaa9923be 100644 --- a/src/engine/strat_engine/liminal/device_info.rs +++ b/src/engine/strat_engine/liminal/device_info.rs @@ -517,8 +517,8 @@ impl DeviceSet { } /// Process the data from an add udev event. If the added data is - /// incompatible with the existing, drain the set and return all the - /// drained elements, including the incompatible ones. + /// incompatible with the existing, warn the user and ignore the new + /// entry. pub fn process_info_add(&mut self, info: DeviceInfo) { let stratis_identifiers = info.stratis_identifiers(); let device_uuid = stratis_identifiers.device_uuid; diff --git a/src/engine/types/mod.rs b/src/engine/types/mod.rs index db0efae7931..924b0e930d0 100644 --- a/src/engine/types/mod.rs +++ b/src/engine/types/mod.rs @@ -203,7 +203,7 @@ impl fmt::Display for Name { /// * `ErroredPoolDevices` returns the state of devices that caused an error while /// attempting to reconstruct a pool. pub enum ReportType { - ErroredPoolDevices, + StoppedPools, } impl<'a> TryFrom<&'a str> for ReportType { @@ -211,7 +211,7 @@ impl<'a> TryFrom<&'a str> for ReportType { fn try_from(name: &str) -> StratisResult { match name { - "errored_pool_report" => Ok(ReportType::ErroredPoolDevices), + "stopped_pools" => Ok(ReportType::StoppedPools), _ => Err(StratisError::Msg(format!( "Report name {} not understood", name