diff --git a/src/engine/strat_engine/backstore/crypt/shared.rs b/src/engine/strat_engine/backstore/crypt/shared.rs index 1932b927481..1e947dd8942 100644 --- a/src/engine/strat_engine/backstore/crypt/shared.rs +++ b/src/engine/strat_engine/backstore/crypt/shared.rs @@ -44,7 +44,7 @@ use crate::{ STRATIS_TOKEN_ID, STRATIS_TOKEN_POOLNAME_KEY, STRATIS_TOKEN_POOL_UUID_KEY, STRATIS_TOKEN_TYPE, TOKEN_KEYSLOTS_KEY, TOKEN_TYPE_KEY, }, - handle::CryptHandle, + handle::{CryptHandle, CryptMetadata}, }, devices::get_devno_from_path, }, @@ -62,8 +62,6 @@ use crate::{ stratis::{StratisError, StratisResult}, }; -use super::handle::CryptMetadata; - /// Set up crypt logging to log cryptsetup debug information at the trace level. pub fn set_up_crypt_logging() { fn logging_callback(level: CryptLogLevel, msg: &str, _: Option<&mut ()>) { diff --git a/src/engine/strat_engine/liminal/liminal.rs b/src/engine/strat_engine/liminal/liminal.rs index f93a6bdae13..af1a1abd2c1 100644 --- a/src/engine/strat_engine/liminal/liminal.rs +++ b/src/engine/strat_engine/liminal/liminal.rs @@ -18,10 +18,7 @@ use crate::{ engine::{DumpState, Pool, StateDiff}, strat_engine::{ backstore::{find_stratis_devs_by_uuid, CryptHandle, StratBlockDev}, - dm::{ - has_leftover_devices, list_of_crypt_devices, remove_optional_devices, - stop_partially_constructed_pool, - }, + dm::{has_leftover_devices, stop_partially_constructed_pool}, liminal::{ device_info::{ reconstruct_stratis_infos, split_stratis_infos, stratis_infos_ref, DeviceSet, @@ -92,13 +89,10 @@ impl LiminalDevices { pools: &Table, pool_uuid: PoolUuid, unlock_method: UnlockMethod, - ) -> StratisResult> { - fn handle_luks( - luks_info: &LLuksInfo, - unlock_method: UnlockMethod, - ) -> StratisResult { - if let Some(h) = CryptHandle::setup(&luks_info.dev_info.devnode, Some(unlock_method))? { - Ok(h) + ) -> StratisResult> { + fn handle_luks(luks_info: &LLuksInfo, unlock_method: UnlockMethod) -> StratisResult<()> { + if CryptHandle::setup(&luks_info.dev_info.devnode, Some(unlock_method))?.is_some() { + Ok(()) } else { Err(StratisError::Msg(format!( "Block device {} does not appear to be formatted with @@ -135,16 +129,8 @@ impl LiminalDevices { match info { LInfo::Stratis(_) => (), LInfo::Luks(ref luks_info) => match handle_luks(luks_info, unlock_method) { - Ok(handle) => unlocked.push((*dev_uuid, handle)), - Err(e) => { - return Err(handle_unlock_rollback( - e, - unlocked - .into_iter() - .map(|(dev_uuid, _)| dev_uuid) - .collect::>(), - )); - } + Ok(()) => unlocked.push(*dev_uuid), + Err(e) => return Err(e), }, } } @@ -216,24 +202,13 @@ impl LiminalDevices { (Err(e), _) => return Err(e), }; - let (uuids, handles) = unlocked_devices.into_iter().fold( - (Vec::new(), HashMap::new()), - |(mut uuids, mut handles), (uuid, handle)| { - uuids.push(uuid); - handles.insert(uuid, handle); - (uuids, handles) - }, - ); + let uuids = unlocked_devices.into_iter().collect::>(); let mut stopped_pool = self .stopped_pools .remove(&pool_uuid) .or_else(|| self.partially_constructed_pools.remove(&pool_uuid)) .expect("Checked above"); - let all_uuids = stopped_pool - .iter() - .map(|(dev_uuid, _)| *dev_uuid) - .collect::>(); match find_stratis_devs_by_uuid(pool_uuid, &uuids) { Ok(infos) => infos.into_iter().for_each(|(dev_uuid, (path, devno))| { if let Ok(Ok(Some(bda))) = bda_wrapper(&path) { @@ -256,15 +231,12 @@ impl LiminalDevices { }), Err(e) => { warn!("Failed to scan for newly unlocked Stratis devices: {}", e); - let err = handle_unlock_rollback(e, all_uuids); - return Err(err); + return Err(e); } }; - match self.try_setup_pool(pools, pool_uuid, stopped_pool, handles) { - Ok((name, pool)) => Ok((name, pool_uuid, pool, uuids)), - Err(e) => Err(handle_unlock_rollback(e, all_uuids)), - } + self.try_setup_pool(pools, pool_uuid, stopped_pool) + .map(|(name, pool)| (name, pool_uuid, pool, uuids)) } /// Stop a pool, tear down the devicemapper devices, and store the pool information @@ -554,7 +526,6 @@ impl LiminalDevices { pools: &Table, pool_uuid: PoolUuid, device_set: DeviceSet, - handles: HashMap, ) -> StratisResult<(Name, StratPool)> { fn try_setup_pool_failure( pools: &Table, @@ -562,7 +533,6 @@ impl LiminalDevices { luks_info: StratisResult<(Option, MaybeInconsistent>)>, infos: &HashMap, bdas: HashMap, - handles: HashMap, meta_res: StratisResult<(DateTime, PoolSave)>, ) -> BDARecordResult<(Name, StratPool)> { let (timestamp, metadata) = match meta_res { @@ -571,7 +541,7 @@ impl LiminalDevices { }; setup_pool( - pools, pool_uuid, luks_info, infos, bdas, handles, timestamp, metadata, + pools, pool_uuid, luks_info, infos, bdas, timestamp, metadata, ) } @@ -595,7 +565,7 @@ impl LiminalDevices { let res = load_stratis_metadata(pool_uuid, stratis_infos_ref(&infos)); let (infos, bdas) = split_stratis_infos(infos); - match try_setup_pool_failure(pools, pool_uuid, luks_info, &infos, bdas, handles, res) { + match try_setup_pool_failure(pools, pool_uuid, luks_info, &infos, bdas, res) { Ok((name, pool)) => { self.uuid_lookup = self .uuid_lookup @@ -652,22 +622,8 @@ impl LiminalDevices { Err(e) => return Err((e, bdas)), }; if let Some(true) | None = metadata.started { - let mut handles = HashMap::default(); - for (dev_uuid, info) in infos { - if let Some((dev_uuid, handle)) = match info.luks.as_ref() { - Some(l) => match CryptHandle::setup(&l.dev_info.devnode, None) { - Ok(Some(handle)) => Some((dev_uuid, handle)), - Ok(None) => None, - Err(e) => return Err((e, bdas)), - }, - None => None, - } { - handles.insert(*dev_uuid, handle); - } - } - setup_pool( - pools, pool_uuid, luks_info, infos, bdas, handles, timestamp, metadata, + pools, pool_uuid, luks_info, infos, bdas, timestamp, metadata, ) .map(Either::Left) } else { @@ -824,6 +780,7 @@ impl LiminalDevices { let mut devices = self .stopped_pools .remove(&pool_uuid) + .or_else(|| self.partially_constructed_pools.remove(&pool_uuid)) .unwrap_or_else(DeviceSet::new); self.uuid_lookup @@ -914,44 +871,8 @@ impl LiminalDevices { .map(|(dev_uuid, _)| *dev_uuid) .collect::>(); if has_leftover_devices(pool_uuid, &device_uuids) { - let dev_uuids = device_set - .iter() - .map(|(dev_uuid, _)| *dev_uuid) - .collect::>(); - let res = stop_partially_constructed_pool(pool_uuid, &dev_uuids); - let device_set = device_set - .into_iter() - .map(|(dev_uuid, info)| { - ( - dev_uuid, - match info { - LInfo::Luks(l) => LInfo::Luks(l), - LInfo::Stratis(mut s) => { - if let Some(l) = s.luks { - if !s.dev_info.devnode.exists() { - LInfo::Luks(l) - } else { - s.luks = Some(l); - LInfo::Stratis(s) - } - } else { - LInfo::Stratis(s) - } - } - }, - ) - }) - .collect::(); - match res { - Ok(_) => { - assert!(!has_leftover_devices(pool_uuid, &device_uuids)); - self.stopped_pools.insert(pool_uuid, device_set); - } - Err(_) => { - self.partially_constructed_pools - .insert(pool_uuid, device_set); - } - } + self.partially_constructed_pools + .insert(pool_uuid, device_set); } else { self.stopped_pools.insert(pool_uuid, device_set); } @@ -1056,14 +977,12 @@ fn load_stratis_metadata( /// /// If there is a name conflict between the set of devices in devices /// and some existing pool, return an error. -#[allow(clippy::too_many_arguments)] fn setup_pool( pools: &Table, pool_uuid: PoolUuid, luks_info: StratisResult<(Option, MaybeInconsistent>)>, infos: &HashMap, bdas: HashMap, - handles: HashMap, timestamp: DateTime, metadata: PoolSave, ) -> BDARecordResult<(Name, StratPool)> { @@ -1077,7 +996,7 @@ fn setup_pool( )), bdas)); } - let (datadevs, cachedevs) = match get_blockdevs(&metadata.backstore, infos, bdas, handles) { + let (datadevs, cachedevs) = match get_blockdevs(&metadata.backstore, infos, bdas) { Err((err, bdas)) => return Err( (StratisError::Chained( format!( @@ -1148,21 +1067,3 @@ fn setup_pool( ), bdas) }) } - -/// Rollback an unlock operation for some or all devices of a pool that have been -/// unlocked prior to the failure occurring. -fn handle_unlock_rollback(causal_error: StratisError, uuids: Vec) -> StratisError { - let devices = list_of_crypt_devices(&uuids); - if let Err(e) = remove_optional_devices(devices) { - warn!("Failed to roll back encrypted pool unlock; some previously locked encrypted devices may be left in an unlocked state"); - return StratisError::NoActionRollbackError { - causal_error: Box::new(causal_error), - rollback_error: Box::new(StratisError::Chained( - "Failed to roll back encrypted pool unlock; some previously locked encrypted devices may be left in an unlocked state".to_string(), - Box::new(e), - )), - }; - } - - causal_error -} diff --git a/src/engine/strat_engine/liminal/setup.rs b/src/engine/strat_engine/liminal/setup.rs index ac14d181f98..e92790537b0 100644 --- a/src/engine/strat_engine/liminal/setup.rs +++ b/src/engine/strat_engine/liminal/setup.rs @@ -137,7 +137,6 @@ pub fn get_blockdevs( backstore_save: &BackstoreSave, infos: &HashMap, mut bdas: HashMap, - mut handles: HashMap, ) -> BDARecordResult<(Vec, Vec)> { let recorded_data_map: HashMap = backstore_save .data_tier @@ -185,7 +184,6 @@ pub fn get_blockdevs( fn get_blockdev( info: &LStratisDevInfo, bda: BDA, - handle: Option, data_map: &HashMap, cache_map: &HashMap, segment_table: &HashMap>, @@ -249,6 +247,10 @@ pub fn get_blockdevs( Some(luks) => &luks.dev_info.devnode, None => &info.dev_info.devnode, }; + let handle = match CryptHandle::setup(physical_path, None) { + Ok(h) => h, + Err(e) => return Err((e, bda)), + }; let underlying_device = match handle { Some(handle) => UnderlyingDevice::Encrypted(handle), None => UnderlyingDevice::Unencrypted(match DevicePath::new(physical_path) { @@ -275,7 +277,6 @@ pub fn get_blockdevs( match get_blockdev( infos.get(dev_uuid).expect("bdas.keys() == infos.keys()"), bdas.remove(dev_uuid).expect("bdas.keys() == infos.keys()"), - handles.remove(dev_uuid), &recorded_data_map, &recorded_cache_map, &segment_table,