diff --git a/src/dbus_api/api/prop_conv.rs b/src/dbus_api/api/prop_conv.rs index 3d2332ede77..e53051fac1e 100644 --- a/src/dbus_api/api/prop_conv.rs +++ b/src/dbus_api/api/prop_conv.rs @@ -70,6 +70,7 @@ pub fn stopped_pools_to_prop(pools: &StoppedPoolsInfo) -> StoppedOrLockedPools { pools .stopped .iter() + .chain(pools.partially_constructed.iter()) .map(|(u, stopped)| { let mut map = HashMap::new(); if let Some(enc_info) = stopped.info.as_ref() { diff --git a/src/engine/sim_engine/engine.rs b/src/engine/sim_engine/engine.rs index 5c84b56cf58..4426d2c2015 100644 --- a/src/engine/sim_engine/engine.rs +++ b/src/engine/sim_engine/engine.rs @@ -84,8 +84,9 @@ impl<'a> Into for &'a SimEngine { unreachable!("json!() output is always JSON object"); } }) - .collect() + .collect(), ), + "partially_constructed_pools": Value::Array(Vec::new()) }) } } diff --git a/src/engine/strat_engine/backstore/backstore.rs b/src/engine/strat_engine/backstore/backstore.rs index 5823b25ba93..72cf1ddbb43 100644 --- a/src/engine/strat_engine/backstore/backstore.rs +++ b/src/engine/strat_engine/backstore/backstore.rs @@ -569,25 +569,24 @@ impl Backstore { } /// Teardown the DM devices in the backstore. - pub fn teardown(&mut self) -> StratisResult> { - let mut devs = match self + pub fn teardown(&mut self) -> StratisResult<()> { + match self .cache .as_mut() .and_then(|c| self.cache_tier.as_mut().map(|ct| (c, ct))) { Some((cache, cache_tier)) => { cache.teardown(get_dm())?; - cache_tier.block_mgr.teardown()? + cache_tier.block_mgr.teardown()?; } None => { if let Some(ref mut linear) = self.linear { linear.teardown(get_dm())?; } - Vec::new() } }; - devs.extend(self.data_tier.block_mgr.teardown()?); - Ok(devs) + self.data_tier.block_mgr.teardown()?; + Ok(()) } /// Consume the backstore and convert it into a set of BDAs representing @@ -605,6 +604,18 @@ impl Backstore { .collect::>() } + /// Drain the backstore devices into a set of all data and cache devices. + pub fn drain_bds(&mut self) -> Vec { + let mut bds = self.data_tier.block_mgr.drain_bds(); + bds.extend( + self.cache_tier + .as_mut() + .map(|ct| ct.block_mgr.drain_bds()) + .unwrap_or_default(), + ); + bds + } + /// Return the device that this tier is currently using. /// This changes, depending on whether the backstore is supporting a cache /// or not. There may be no device if no data has yet been allocated from diff --git a/src/engine/strat_engine/backstore/blockdevmgr.rs b/src/engine/strat_engine/backstore/blockdevmgr.rs index c2b48a6bc09..3146e37bbe4 100644 --- a/src/engine/strat_engine/backstore/blockdevmgr.rs +++ b/src/engine/strat_engine/backstore/blockdevmgr.rs @@ -81,6 +81,11 @@ impl BlockDevMgr { bds_to_bdas(self.block_devs) } + /// Drain the BlockDevMgr block devices into a collection of block devices. + pub fn drain_bds(&mut self) -> Vec { + self.block_devs.drain(..).collect::>() + } + /// Get a hashmap that maps UUIDs to Devices. pub fn uuid_to_devno(&self) -> HashMap { self.block_devs @@ -399,27 +404,16 @@ impl BlockDevMgr { } /// Tear down devicemapper devices for the block devices in this BlockDevMgr. - pub fn teardown(&mut self) -> StratisResult> { - let (oks, errs) = self - .block_devs - .drain(..) - .map(|mut bd| { - bd.teardown()?; - Ok(bd) - }) - .partition::, _>(|res| res.is_ok()); - let (oks, errs) = ( - oks.into_iter().filter_map(|ok| ok.ok()).collect::>(), - errs.into_iter() - .filter_map(|err| match err { - Ok(_) => None, - Err(e) => Some(e), - }) - .collect::>(), - ); + pub fn teardown(&mut self) -> StratisResult<()> { + let errs = self.block_devs.iter_mut().fold(Vec::new(), |mut errs, bd| { + if let Err(e) = bd.teardown() { + errs.push(e); + } + errs + }); if errs.is_empty() { - Ok(oks) + Ok(()) } else { Err(StratisError::BestEffortError("Failed to remove devicemapper devices for some or all physical devices in the pool".to_string(), errs)) } diff --git a/src/engine/strat_engine/backstore/crypt/initialize.rs b/src/engine/strat_engine/backstore/crypt/initialize.rs index 138c8fe5259..7a4a84257e8 100644 --- a/src/engine/strat_engine/backstore/crypt/initialize.rs +++ b/src/engine/strat_engine/backstore/crypt/initialize.rs @@ -52,9 +52,10 @@ impl CryptInitializer { pool_uuid: PoolUuid, dev_uuid: DevUuid, ) -> CryptInitializer { + let dm_name = format_crypt_name(&dev_uuid).to_string(); CryptInitializer { physical_path, - activation_name: format_crypt_name(&dev_uuid), + activation_name: dm_name, identifiers: StratisIdentifiers::new(pool_uuid, dev_uuid), } } diff --git a/src/engine/strat_engine/dm.rs b/src/engine/strat_engine/dm.rs index 0c7135e6276..6358db10644 100644 --- a/src/engine/strat_engine/dm.rs +++ b/src/engine/strat_engine/dm.rs @@ -4,11 +4,23 @@ // Get ability to instantiate a devicemapper context. -use std::sync::Once; +use std::{path::Path, sync::Once}; -use devicemapper::{DmResult, DM}; +use devicemapper::{DevId, DmNameBuf, DmOptions, DmResult, DM}; -use crate::stratis::{StratisError, StratisResult}; +use crate::{ + engine::{ + strat_engine::{ + liminal::DeviceSet, + names::{ + format_backstore_ids, format_crypt_name, format_flex_ids, format_thinpool_ids, + CacheRole, FlexRole, ThinPoolRole, + }, + }, + types::{DevUuid, PoolUuid}, + }, + stratis::{StratisError, StratisResult}, +}; static INIT: Once = Once::new(); static mut DM_CONTEXT: Option> = None; @@ -35,3 +47,86 @@ pub fn get_dm() -> &'static DM { "the engine has already called get_dm_init() and exited if get_dm_init() returned an error", ) } + +pub fn stop_partially_constructed_pool( + pool_uuid: PoolUuid, + device_set: &DeviceSet, +) -> StratisResult { + let mut did_something = false; + let devices = get_dm() + .list_devices()? + .into_iter() + .map(|(name, _, _)| name) + .collect::>(); + let dev_uuids = device_set + .iter() + .map(|(dev_uuid, _)| *dev_uuid) + .collect::>(); + for device in list_of_devices(pool_uuid, &dev_uuids) { + if devices.contains(&device) { + did_something = true; + get_dm().device_remove(&DevId::Name(&device), DmOptions::default())?; + } + } + Ok(did_something) +} + +/// List of device names for removal on partially constructed pool stop. Does not have +/// filesystem names because partially constructed pools are guaranteed not to have any +/// active filesystems. +fn list_of_devices(pool_uuid: PoolUuid, dev_uuids: &[DevUuid]) -> Vec { + let mut devs = Vec::new(); + let (thin_pool, _) = format_thinpool_ids(pool_uuid, ThinPoolRole::Pool); + devs.push(thin_pool); + let (thin_data, _) = format_flex_ids(pool_uuid, FlexRole::ThinData); + devs.push(thin_data); + let (thin_meta, _) = format_flex_ids(pool_uuid, FlexRole::ThinMeta); + devs.push(thin_meta); + let (thin_meta_spare, _) = format_flex_ids(pool_uuid, FlexRole::ThinMetaSpare); + devs.push(thin_meta_spare); + let (thin_mdv, _) = format_flex_ids(pool_uuid, FlexRole::MetadataVolume); + devs.push(thin_mdv); + let (cache, _) = format_backstore_ids(pool_uuid, CacheRole::Cache); + devs.push(cache); + let (cache_sub, _) = format_backstore_ids(pool_uuid, CacheRole::CacheSub); + devs.push(cache_sub); + let (cache_meta, _) = format_backstore_ids(pool_uuid, CacheRole::MetaSub); + devs.push(cache_meta); + let (origin, _) = format_backstore_ids(pool_uuid, CacheRole::OriginSub); + devs.push(origin); + + for dev_uuid in dev_uuids.iter() { + let crypt = format_crypt_name(dev_uuid); + devs.push(crypt); + } + + devs +} + +/// Check whether there are any leftover devicemapper devices from the pool. +pub fn has_leftover_devices(pool_uuid: PoolUuid, dev_uuids: &[DevUuid]) -> bool { + let mut has_leftover = false; + let devices = list_of_devices(pool_uuid, dev_uuids); + match get_dm().list_devices() { + Ok(l) => { + let listed_devices = l + .into_iter() + .map(|(dm_name, _, _)| dm_name) + .collect::>(); + for device in devices { + if listed_devices.contains(&device) { + has_leftover |= true; + } + } + } + Err(_) => { + for device in devices { + if Path::new(&format!("/dev/mapper/{}", &*device)).exists() { + has_leftover |= true; + } + } + } + } + + has_leftover +} diff --git a/src/engine/strat_engine/engine.rs b/src/engine/strat_engine/engine.rs index 8de6a6fc656..859d3069322 100644 --- a/src/engine/strat_engine/engine.rs +++ b/src/engine/strat_engine/engine.rs @@ -27,7 +27,7 @@ use crate::{ cmd::verify_executables, dm::get_dm, keys::StratKeyActions, - liminal::{find_all, LiminalDevices}, + liminal::{find_all, DeviceSet, LiminalDevices}, ns::MemoryFilesystem, pool::StratPool, }, @@ -453,10 +453,21 @@ impl Engine for StratEngine { .remove_by_uuid(uuid) .expect("Must succeed since self.pools.get_by_uuid() returned a value"); - let (res, pool) = spawn_blocking!((pool.destroy(), pool))?; - if let Err(err) = res { + let (res, mut pool) = spawn_blocking!((pool.destroy(), pool))?; + if let Err((err, true)) = res { guard.insert(pool_name, uuid, pool); Err(err) + } else if let Err((err, false)) = res { + // We use blkid to scan for existing devices with this pool UUID and device UUIDs + // because some of the block devices could have been destroyed above. Using the + // cached data structures alone could result in phantom devices that have already + // been destroyed but are still recorded in the stopped pool. + let device_set = DeviceSet::from(pool.drain_bds()); + self.liminal_devices + .write() + .await + .handle_stopped_pool(uuid, device_set); + Err(err) } else { Ok(DeleteAction::Deleted(uuid)) } @@ -638,30 +649,22 @@ impl Engine for StratEngine { async fn stop_pool(&self, pool_uuid: PoolUuid) -> StratisResult> { let mut pools = self.pools.write_all().await; - if let Some((name, mut pool)) = pools.remove_by_uuid(pool_uuid) { - if let Err(e) = self - .liminal_devices + if let Some((name, pool)) = pools.remove_by_uuid(pool_uuid) { + self.liminal_devices .write() .await - .stop_pool(&name, pool_uuid, &mut pool) - { - pools.insert(name, pool_uuid, pool); - return Err(e); - } else { - return Ok(StopAction::Stopped(pool_uuid)); - } + .stop_pool(&mut pools, name, pool_uuid, pool)?; + return Ok(StopAction::Stopped(pool_uuid)); } drop(pools); - if self - .liminal_devices - .read() - .await - .stopped_pools() - .stopped - .get(&pool_uuid) - .is_some() + let stopped_pools = self.liminal_devices.read().await.stopped_pools(); + if stopped_pools.stopped.get(&pool_uuid).is_some() + || stopped_pools + .partially_constructed + .get(&pool_uuid) + .is_some() { Ok(StopAction::Identity) } else { diff --git a/src/engine/strat_engine/liminal/device_info.rs b/src/engine/strat_engine/liminal/device_info.rs index ef73ddbc672..d8df7340c97 100644 --- a/src/engine/strat_engine/liminal/device_info.rs +++ b/src/engine/strat_engine/liminal/device_info.rs @@ -18,6 +18,7 @@ use crate::{ engine::{ shared::{gather_encryption_info, gather_pool_name}, strat_engine::{ + backstore::StratBlockDev, liminal::{ identify::{DeviceInfo, LuksInfo, StratisDevInfo, StratisInfo}, setup::get_name, @@ -501,6 +502,19 @@ impl FromIterator<(DevUuid, LInfo)> for DeviceSet { } } +impl From> for DeviceSet { + fn from(vec: Vec) -> Self { + vec.into_iter() + .flat_map(|bd| { + let dev_uuid = bd.uuid(); + Vec::::from(bd) + .into_iter() + .map(move |info| (dev_uuid, LInfo::from(info))) + }) + .collect::() + } +} + impl DeviceSet { /// Create a new, empty DeviceSet pub fn new() -> DeviceSet { diff --git a/src/engine/strat_engine/liminal/identify.rs b/src/engine/strat_engine/liminal/identify.rs index 5dc25324508..0e3847c9bc1 100644 --- a/src/engine/strat_engine/liminal/identify.rs +++ b/src/engine/strat_engine/liminal/identify.rs @@ -49,17 +49,22 @@ use std::{ use serde_json::Value; use devicemapper::Device; - -use crate::engine::{ - strat_engine::{ - backstore::{CryptMetadataHandle, StratBlockDev}, - metadata::{static_header, StratisIdentifiers, BDA}, - udev::{ - block_enumerator, decide_ownership, UdevOwnership, CRYPTO_FS_TYPE, FS_TYPE_KEY, - STRATIS_FS_TYPE, +use libblkid_rs::BlkidProbe; + +use crate::{ + engine::{ + engine::BlockDev, + strat_engine::{ + backstore::{CryptMetadataHandle, StratBlockDev}, + metadata::{static_header, StratisIdentifiers, BDA}, + udev::{ + block_enumerator, decide_ownership, UdevOwnership, CRYPTO_FS_TYPE, FS_TYPE_KEY, + STRATIS_FS_TYPE, + }, }, + types::{EncryptionInfo, Name, PoolUuid, UdevEngineDevice, UdevEngineEvent}, }, - types::{EncryptionInfo, Name, PoolUuid, UdevEngineDevice, UdevEngineEvent}, + stratis::StratisResult, }; /// Information related to device number and path for either a Stratis device @@ -171,30 +176,81 @@ impl DeviceInfo { } } -impl From for DeviceInfo { +impl From for Vec { fn from(bd: StratBlockDev) -> Self { + fn is_valid(blockdev: &StratBlockDev) -> StratisResult<(bool, bool)> { + let mut valid = true; + let mut unlocked = true; + if blockdev.is_encrypted() { + valid &= blockdev.physical_path().exists(); + unlocked &= blockdev.metadata_path().exists(); + + if valid { + let mut probe = BlkidProbe::new_from_filename(blockdev.physical_path())?; + probe.enable_superblocks(true)?; + probe.do_safeprobe()?; + valid &= probe.lookup_value("TYPE").ok() == Some(CRYPTO_FS_TYPE.to_string()); + } + + if unlocked { + let mut probe = BlkidProbe::new_from_filename(blockdev.physical_path())?; + probe.enable_superblocks(true)?; + probe.do_safeprobe()?; + unlocked = probe.lookup_value("TYPE").ok() == Some(STRATIS_FS_TYPE.to_string()); + } + } else { + let mut probe = BlkidProbe::new_from_filename(blockdev.physical_path())?; + probe.enable_superblocks(true)?; + probe.do_safeprobe()?; + valid &= probe.lookup_value("TYPE").ok() == Some(STRATIS_FS_TYPE.to_string()); + } + Ok((valid, unlocked)) + } + + let (valid, unlocked) = match is_valid(&bd) { + Ok(o) => o, + Err(e) => { + warn!("Failed to probe devices with blkid: {}; ignoring", e); + (false, false) + } + }; + let mut device_infos = Vec::new(); match (bd.encryption_info(), bd.pool_name(), bd.luks_device()) { - (Some(ei), Some(pname), Some(dev)) => DeviceInfo::Luks(LuksInfo { - encryption_info: ei.clone(), - dev_info: StratisDevInfo { - device_number: *dev, - devnode: bd.physical_path().to_owned(), - }, - identifiers: StratisIdentifiers { - pool_uuid: bd.pool_uuid(), - device_uuid: bd.uuid(), - }, - pool_name: pname.cloned(), - }), - (None, None, None) => DeviceInfo::Stratis(StratisInfo { + (Some(ei), Some(pname), Some(dev)) => { + if valid { + device_infos.push(DeviceInfo::Luks(LuksInfo { + encryption_info: ei.clone(), + dev_info: StratisDevInfo { + device_number: *dev, + devnode: bd.physical_path().to_owned(), + }, + identifiers: StratisIdentifiers { + pool_uuid: bd.pool_uuid(), + device_uuid: bd.uuid(), + }, + pool_name: pname.cloned(), + })); + if unlocked { + device_infos.push(DeviceInfo::Stratis(StratisInfo { + dev_info: StratisDevInfo { + device_number: *bd.device(), + devnode: bd.metadata_path().to_owned(), + }, + bda: bd.bda, + })); + } + } + } + (None, None, None) => device_infos.push(DeviceInfo::Stratis(StratisInfo { dev_info: StratisDevInfo { device_number: *bd.device(), devnode: bd.physical_path().to_owned(), }, bda: bd.bda, - }), + })), (_, _, _) => unreachable!("If bd.is_encrypted(), all are Some(_)"), } + device_infos } } diff --git a/src/engine/strat_engine/liminal/liminal.rs b/src/engine/strat_engine/liminal/liminal.rs index 8b5b18e60a2..a389753db28 100644 --- a/src/engine/strat_engine/liminal/liminal.rs +++ b/src/engine/strat_engine/liminal/liminal.rs @@ -20,6 +20,7 @@ use crate::{ backstore::{ find_stratis_devs_by_uuid, CryptActivationHandle, CryptHandle, StratBlockDev, }, + dm::{has_leftover_devices, stop_partially_constructed_pool}, liminal::{ device_info::{ reconstruct_stratis_infos, split_stratis_infos, stratis_infos_ref, DeviceSet, @@ -57,6 +58,9 @@ pub struct LiminalDevices { uuid_lookup: HashMap, /// Devices that have not yet been set up or have been stopped. stopped_pools: HashMap, + /// Devices that have been left in a partially constructed state either during start + /// or stop. + partially_constructed_pools: HashMap, /// Lookup data structure for name to UUID mapping for starting pools by name. name_to_uuid: HashMap, } @@ -105,7 +109,11 @@ impl LiminalDevices { } } - let unlocked = match self.stopped_pools.get(&pool_uuid) { + let unlocked = match self + .stopped_pools + .get(&pool_uuid) + .or_else(|| self.partially_constructed_pools.get(&pool_uuid)) + { Some(map) => { let encryption_info = map.encryption_info(); if let Ok(None) = encryption_info { @@ -180,12 +188,14 @@ impl LiminalDevices { .ok_or_else(|| StratisError::Msg(format!("Could not find a pool with name {n}"))) .and_then(|uc| uc.to_result())?, }; + // Must leave stopped pool in record let encryption_info = self .stopped_pools .get(&pool_uuid) + .or_else(|| self.partially_constructed_pools.get(&pool_uuid)) .ok_or_else(|| { StratisError::Msg(format!( - "Requested pool with UUID {pool_uuid} was not found in stopped pools" + "Requested pool with UUID {pool_uuid} was not found in stopped or partially constructed pools" )) })? .encryption_info(); @@ -217,6 +227,7 @@ impl LiminalDevices { let mut stopped_pool = self .stopped_pools .remove(&pool_uuid) + .or_else(|| self.partially_constructed_pools.remove(&pool_uuid)) .expect("Checked above"); match find_stratis_devs_by_uuid(pool_uuid, uuids) { Ok(infos) => infos.into_iter().for_each(|(dev_uuid, (path, devno))| { @@ -240,8 +251,8 @@ impl LiminalDevices { }), Err(e) => { warn!("Failed to scan for newly unlocked Stratis devices: {}", e); - self.stopped_pools.insert(pool_uuid, stopped_pool); - return Err(handle_unlock_rollback(e, handles)); + let err = handle_unlock_rollback(e, handles); + return Err(err); } }; @@ -255,11 +266,22 @@ impl LiminalDevices { /// in an internal data structure for later starting. pub fn stop_pool( &mut self, - pool_name: &Name, + pools: &mut Table, + pool_name: Name, pool_uuid: PoolUuid, - pool: &mut StratPool, + mut pool: StratPool, ) -> StratisResult<()> { - let devices = pool.stop(pool_name)?; + let (devices, err) = match pool.stop(&pool_name) { + Ok(devs) => (devs, None), + Err((e, true)) => { + pools.insert(pool_name, pool_uuid, pool); + return Err(e); + } + Err((e, false)) => { + warn!("Failed to stop pool; placing in partially constructed pools"); + (DeviceSet::from(pool.drain_bds()), Some(e)) + } + }; for (_, device) in devices.iter() { match device { LInfo::Luks(l) => { @@ -279,8 +301,12 @@ impl LiminalDevices { } } } - self.stopped_pools.insert(pool_uuid, devices); - if let Some(maybe_conflict) = self.name_to_uuid.get_mut(pool_name) { + if err.is_some() { + self.partially_constructed_pools.insert(pool_uuid, devices); + } else { + self.stopped_pools.insert(pool_uuid, devices); + } + if let Some(maybe_conflict) = self.name_to_uuid.get_mut(&pool_name) { maybe_conflict.add(pool_uuid); if let UuidOrConflict::Conflict(set) = maybe_conflict { warn!("Found conflicting names for stopped pools; UUID will be required to start pools with UUIDs {}", set.iter().map(|u| u.to_string()).collect::>().join(", ")); @@ -289,7 +315,11 @@ impl LiminalDevices { self.name_to_uuid .insert(pool_name.clone(), UuidOrConflict::Uuid(pool_uuid)); } - Ok(()) + if let Some(e) = err { + Err(e) + } else { + Ok(()) + } } /// Get a mapping of pool UUIDs from all of the LUKS2 devices that are currently @@ -356,6 +386,13 @@ impl LiminalDevices { map.stopped_pool_info().map(|info| (*pool_uuid, info)) }) .collect(), + partially_constructed: self + .partially_constructed_pools + .iter() + .filter_map(|(pool_uuid, map)| { + map.stopped_pool_info().map(|info| (*pool_uuid, info)) + }) + .collect(), } } @@ -530,9 +567,7 @@ impl LiminalDevices { "Some of the devices in pool with UUID {pool_uuid} are unopened" )); info!("Attempt to set up pool failed, but it may be possible to set up the pool later, if the situation changes: {}", err); - if !ds.is_empty() { - self.stopped_pools.insert(pool_uuid, ds); - } + self.handle_stopped_pool(pool_uuid, ds); return Err(err); } }; @@ -566,9 +601,7 @@ impl LiminalDevices { Err((err, bdas)) => { info!("Attempt to set up pool failed, but it may be possible to set up the pool later, if the situation changes: {}", err); let device_set = reconstruct_stratis_infos(infos, bdas); - if !device_set.is_empty() { - self.stopped_pools.insert(pool_uuid, device_set); - } + self.handle_stopped_pool(pool_uuid, device_set); Err(err) } } @@ -620,9 +653,7 @@ impl LiminalDevices { "Some of the devices in pool with UUID {pool_uuid} are unopened" )); info!("Attempt to set up pool failed, but it may be possible to set up the pool later, if the situation changes: {}", err); - if !ds.is_empty() { - self.stopped_pools.insert(pool_uuid, ds); - } + self.handle_stopped_pool(pool_uuid, ds); return None; } }; @@ -655,9 +686,7 @@ impl LiminalDevices { } Ok(Either::Right(bdas)) => { let device_set = reconstruct_stratis_infos(infos, bdas); - if !device_set.is_empty() { - self.stopped_pools.insert(pool_uuid, device_set); - } + self.handle_stopped_pool(pool_uuid, device_set); None } Err((err, bdas)) => { @@ -842,6 +871,29 @@ impl LiminalDevices { None } } + + pub fn handle_stopped_pool(&mut self, pool_uuid: PoolUuid, device_set: DeviceSet) { + if !device_set.is_empty() { + let device_uuids = device_set + .iter() + .map(|(dev_uuid, _)| *dev_uuid) + .collect::>(); + if has_leftover_devices(pool_uuid, &device_uuids) { + match stop_partially_constructed_pool(pool_uuid, &device_set) { + 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); + } + } + } else { + self.stopped_pools.insert(pool_uuid, device_set); + } + } + } } impl<'a> Into for &'a LiminalDevices { @@ -858,6 +910,17 @@ impl<'a> Into for &'a LiminalDevices { }) .collect() ), + "partially_constructed_pools": Value::Array( + self.partially_constructed_pools + .iter() + .map(|(uuid, set)| { + json!({ + "pool_uuid": uuid.to_string(), + "devices": <&DeviceSet as Into>::into(set), + }) + }) + .collect() + ), "path_to_ids_map": Value::Object( self.uuid_lookup .iter() diff --git a/src/engine/strat_engine/names.rs b/src/engine/strat_engine/names.rs index 553c966a4e2..cfc068a0393 100644 --- a/src/engine/strat_engine/names.rs +++ b/src/engine/strat_engine/names.rs @@ -61,12 +61,13 @@ impl KeyDescription { /// < 128 /// /// which is equivalent to len(format!("{}", FORMAT_VERSION) < 73 -pub fn format_crypt_name(dev_uuid: &DevUuid) -> String { - format!( +pub fn format_crypt_name(dev_uuid: &DevUuid) -> DmNameBuf { + let value = format!( "stratis-{}-private-{}-crypt", FORMAT_VERSION, uuid_to_string!(dev_uuid) - ) + ); + DmNameBuf::new(value).expect("FORMAT_VERSION < 73") } #[derive(Clone, Copy)] diff --git a/src/engine/strat_engine/pool.rs b/src/engine/strat_engine/pool.rs index cd47739af19..4bca374e674 100644 --- a/src/engine/strat_engine/pool.rs +++ b/src/engine/strat_engine/pool.rs @@ -19,8 +19,8 @@ use crate::{ }, strat_engine::{ backstore::{Backstore, ProcessedPathInfos, StratBlockDev, UnownedDevices}, - liminal::{DeviceInfo, DeviceSet, LInfo}, - metadata::MDADataSize, + liminal::DeviceSet, + metadata::{MDADataSize, BDA}, serde_structs::{FlexDevsSave, PoolSave, Recordable}, shared::tiers_to_bdas, thinpool::{StratFilesystem, ThinPool, ThinPoolSizeParams, DATA_BLOCK_SIZE}, @@ -318,7 +318,7 @@ impl StratPool { /// Teardown a pool. #[cfg(test)] pub fn teardown(&mut self) -> StratisResult<()> { - self.thin_pool.teardown()?; + self.thin_pool.teardown().map_err(|(e, _)| e)?; self.backstore.teardown()?; Ok(()) } @@ -394,9 +394,9 @@ impl StratPool { /// /// This method is not a mutating action as the pool should be allowed /// to be destroyed even if the metadata is inconsistent. - pub fn destroy(&mut self) -> StratisResult<()> { + pub fn destroy(&mut self) -> Result<(), (StratisError, bool)> { self.thin_pool.teardown()?; - self.backstore.destroy()?; + self.backstore.destroy().map_err(|e| (e, false))?; Ok(()) } @@ -412,17 +412,27 @@ impl StratPool { /// Stop a pool, consuming it and converting it into a set of devices to be /// set up again later. - pub fn stop(&mut self, pool_name: &Name) -> StratisResult { + pub fn stop(&mut self, pool_name: &Name) -> Result { self.thin_pool.teardown()?; let mut data = self.record(pool_name); data.started = Some(false); - let json = serde_json::to_string(&data)?; - self.backstore.save_state(json.as_bytes())?; - let bds = self.backstore.teardown()?; - Ok(bds - .into_iter() - .map(|bd| (bd.uuid(), LInfo::from(DeviceInfo::from(bd)))) - .collect::()) + let json = serde_json::to_string(&data).map_err(|e| (StratisError::from(e), false))?; + self.backstore + .save_state(json.as_bytes()) + .map_err(|e| (e, false))?; + self.backstore.teardown().map_err(|e| (e, false))?; + let bds = self.backstore.drain_bds(); + Ok(DeviceSet::from(bds)) + } + + /// Convert a pool into a record of BDAs for the given block devices in the pool. + pub fn into_bdas(self) -> HashMap { + self.backstore.into_bdas() + } + + /// Drain pool block devices into a record of block devices in the pool. + pub fn drain_bds(&mut self) -> Vec { + self.backstore.drain_bds() } #[cfg(test)] diff --git a/src/engine/strat_engine/thinpool/thinpool.rs b/src/engine/strat_engine/thinpool/thinpool.rs index e1687af00fd..2f492befa7d 100644 --- a/src/engine/strat_engine/thinpool/thinpool.rs +++ b/src/engine/strat_engine/thinpool/thinpool.rs @@ -807,16 +807,18 @@ impl ThinPool { /// Tear down the components managed here: filesystems, the MDV, /// and the actual thinpool device itself. - pub fn teardown(&mut self) -> StratisResult<()> { + pub fn teardown(&mut self) -> Result<(), (StratisError, bool)> { // Must succeed in tearing down all filesystems before the // thinpool.. for (_, _, ref mut fs) in &mut self.filesystems { - fs.teardown()?; + fs.teardown().map_err(|e| (e, true))?; } - self.thin_pool.teardown(get_dm())?; + self.thin_pool + .teardown(get_dm()) + .map_err(|e| (StratisError::from(e), false))?; // ..but MDV has no DM dependencies with the above - self.mdv.teardown()?; + self.mdv.teardown().map_err(|e| (e, false))?; Ok(()) } diff --git a/src/engine/types/mod.rs b/src/engine/types/mod.rs index 8accf010cf5..23e50318164 100644 --- a/src/engine/types/mod.rs +++ b/src/engine/types/mod.rs @@ -249,6 +249,7 @@ pub struct StoppedPoolInfo { #[derive(Default, Debug, Eq, PartialEq)] pub struct StoppedPoolsInfo { pub stopped: HashMap, + pub partially_constructed: HashMap, pub name_to_uuid: HashMap, pub uuid_to_name: HashMap, } diff --git a/src/jsonrpc/server/pool.rs b/src/jsonrpc/server/pool.rs index d9c2df1b8df..21006e906f1 100644 --- a/src/jsonrpc/server/pool.rs +++ b/src/jsonrpc/server/pool.rs @@ -231,16 +231,19 @@ where { let stopped = engine.stopped_pools().await; if engine.get_pool(id.clone()).await.is_some() { - Ok(false) - } else if stopped + return Ok(false); + } + let pool_uuid = match id { + PoolIdentifier::Uuid(ref u) => u, + PoolIdentifier::Name(ref n) => stopped + .name_to_uuid + .get(n) + .ok_or_else(|| StratisError::Msg(format!("Could not find pool with name {n}")))?, + }; + if stopped .stopped - .get(match id { - PoolIdentifier::Uuid(ref u) => u, - PoolIdentifier::Name(ref n) => stopped - .name_to_uuid - .get(n) - .ok_or_else(|| StratisError::Msg(format!("Could not find pool with name {n}")))?, - }) + .get(pool_uuid) + .or_else(|| stopped.partially_constructed.get(pool_uuid)) .is_some() { Ok(true)