From 2cab794138b2e7eafd4589abbda38f651e1798a5 Mon Sep 17 00:00:00 2001 From: John Baublitz Date: Fri, 7 Jun 2024 10:15:01 -0400 Subject: [PATCH] Add ability to report features on stopped pools --- src/dbus_api/api/prop_conv.rs | 13 ++++ src/engine/sim_engine/engine.rs | 11 ++- .../strat_engine/liminal/device_info.rs | 73 ++++++++++++------- src/engine/strat_engine/liminal/liminal.rs | 46 ++++-------- src/engine/strat_engine/liminal/setup.rs | 60 +++++++++++++-- src/engine/strat_engine/serde_structs.rs | 10 ++- src/engine/types/mod.rs | 6 ++ 7 files changed, 151 insertions(+), 68 deletions(-) diff --git a/src/dbus_api/api/prop_conv.rs b/src/dbus_api/api/prop_conv.rs index f4bbc177fa..9e4f5822d0 100644 --- a/src/dbus_api/api/prop_conv.rs +++ b/src/dbus_api/api/prop_conv.rs @@ -119,6 +119,19 @@ pub fn stopped_pools_to_prop(pools: &StoppedPoolsInfo, metadata: bool) -> Stoppe None => Variant(Box::new((false, 0))), }, ); + map.insert( + "features".to_string(), + match stopped.features { + Some(ref f) => { + let mut feat = HashMap::new(); + if f.encryption { + feat.insert("encryption".to_string(), true); + } + Variant(Box::new((true, feat))) + } + None => Variant(Box::new((false, HashMap::::new()))), + }, + ); } (uuid_to_string!(u), map) }) diff --git a/src/engine/sim_engine/engine.rs b/src/engine/sim_engine/engine.rs index 65546d1c60..45f2a034a7 100644 --- a/src/engine/sim_engine/engine.rs +++ b/src/engine/sim_engine/engine.rs @@ -24,10 +24,10 @@ use crate::{ SomeLockWriteGuard, Table, }, types::{ - CreateAction, DeleteAction, DevUuid, EncryptionInfo, FilesystemUuid, LockedPoolsInfo, - Name, PoolDevice, PoolDiff, PoolIdentifier, PoolUuid, RenameAction, ReportType, - SetUnlockAction, StartAction, StopAction, StoppedPoolInfo, StoppedPoolsInfo, - StratFilesystemDiff, UdevEngineEvent, UnlockMethod, + CreateAction, DeleteAction, DevUuid, EncryptionInfo, Features, FilesystemUuid, + LockedPoolsInfo, Name, PoolDevice, PoolDiff, PoolIdentifier, PoolUuid, RenameAction, + ReportType, SetUnlockAction, StartAction, StopAction, StoppedPoolInfo, + StoppedPoolsInfo, StratFilesystemDiff, UdevEngineEvent, UnlockMethod, }, StratSigblockVersion, }, @@ -256,6 +256,9 @@ impl Engine for SimEngine { }) .collect::>(), metadata_version: Some(StratSigblockVersion::V2), + features: Some(Features { + encryption: pool.is_encrypted(), + }), }, ); st diff --git a/src/engine/strat_engine/liminal/device_info.rs b/src/engine/strat_engine/liminal/device_info.rs index a367a3255c..ec89be4bd9 100644 --- a/src/engine/strat_engine/liminal/device_info.rs +++ b/src/engine/strat_engine/liminal/device_info.rs @@ -21,12 +21,12 @@ use crate::{ backstore::blockdev::{v1, v2}, liminal::{ identify::{DeviceInfo, LuksInfo, StratisDevInfo, StratisInfo}, - setup::get_name, + setup::{get_feature_set, get_name}, }, metadata::{StratisIdentifiers, BDA}, }, types::{ - DevUuid, EncryptionInfo, LockedPoolInfo, MaybeInconsistent, Name, PoolDevice, + DevUuid, EncryptionInfo, Features, LockedPoolInfo, MaybeInconsistent, Name, PoolDevice, PoolEncryptionInfo, PoolUuid, StoppedPoolInfo, StratSigblockVersion, }, }, @@ -611,11 +611,15 @@ impl DeviceSet { ) } - /// Get the name, if available, of the pool formed by the devices + /// Get the required pool level metadata information, if available, of the pool formed by the devices /// in this DeviceSet. - pub fn pool_name(&self) -> StratisResult>> { + pub fn pool_level_metadata_info( + &self, + ) -> StratisResult<(MaybeInconsistent>, Option)> { match self.as_opened_set() { - Some(set) => get_name(set).map(MaybeInconsistent::No), + Some(set) => get_name(&set).map(MaybeInconsistent::No).and_then(|name| { + get_feature_set(&set).map(|feat| (name, feat.map(Features::from))) + }), None => gather_pool_name( self.internal.len(), self.internal.values().map(|info| match info { @@ -623,7 +627,12 @@ impl DeviceSet { LInfo::Luks(l) => Some(l.pool_name.as_ref()), }), ) - .map(|opt| opt.expect("self.as_opened_set().is_some() if pool is unencrypted")), + .map(|opt| { + ( + opt.expect("self.as_opened_set().is_some() if pool is unencrypted"), + Some(Features { encryption: true }), + ) + }), } } @@ -688,27 +697,37 @@ impl DeviceSet { self.internal.values().map(|info| info.encryption_info()), ) .ok() - .map(|info| StoppedPoolInfo { - info, - devices: self - .internal - .iter() - .map(|(uuid, l)| { - let devnode = match l { - LInfo::Stratis(strat_info) => strat_info - .luks - .as_ref() - .map(|l| l.dev_info.devnode.clone()) - .unwrap_or_else(|| strat_info.dev_info.devnode.clone()), - LInfo::Luks(luks_info) => luks_info.dev_info.devnode.clone(), - }; - PoolDevice { - devnode, - uuid: *uuid, - } - }) - .collect::>(), - metadata_version: self.metadata_version().ok(), + .map(|info| { + let features = match self.pool_level_metadata_info() { + Ok((_, opt)) => opt, + Err(e) => { + warn!("Failed to read metadata for pool: {e}"); + None + } + }; + StoppedPoolInfo { + info, + devices: self + .internal + .iter() + .map(|(uuid, l)| { + let devnode = match l { + LInfo::Stratis(strat_info) => strat_info + .luks + .as_ref() + .map(|l| l.dev_info.devnode.clone()) + .unwrap_or_else(|| strat_info.dev_info.devnode.clone()), + LInfo::Luks(luks_info) => luks_info.dev_info.devnode.clone(), + }; + PoolDevice { + devnode, + uuid: *uuid, + } + }) + .collect::>(), + metadata_version: self.metadata_version().ok(), + features, + } }) } diff --git a/src/engine/strat_engine/liminal/liminal.rs b/src/engine/strat_engine/liminal/liminal.rs index a42baa726d..77fe579c57 100644 --- a/src/engine/strat_engine/liminal/liminal.rs +++ b/src/engine/strat_engine/liminal/liminal.rs @@ -189,7 +189,7 @@ impl LiminalDevices { fn start_pool_failure( pools: &Table, pool_uuid: PoolUuid, - luks_info: StratisResult<(Option, MaybeInconsistent>)>, + luks_info: StratisResult>, infos: &HashMap, bdas: HashMap, meta_res: StratisResult<(DateTime, PoolSave)>, @@ -276,9 +276,7 @@ impl LiminalDevices { assert!(pools.get_by_uuid(pool_uuid).is_none()); assert!(!self.stopped_pools.contains_key(&pool_uuid)); - let encryption_info = stopped_pool.encryption_info(); - let pool_name = stopped_pool.pool_name(); - let luks_info = encryption_info.and_then(|ei| pool_name.map(|pn| (ei, pn))); + let luks_info = stopped_pool.encryption_info(); let infos = match stopped_pool.into_opened_set() { Either::Left(i) => i, Either::Right(ds) => { @@ -768,8 +766,8 @@ impl LiminalDevices { info_map.process_info_add(info); } - match info_map.pool_name() { - Ok(MaybeInconsistent::No(Some(name))) => { + match info_map.pool_level_metadata_info() { + Ok((MaybeInconsistent::No(Some(name)), _)) => { if let Some(maybe_conflict) = self.name_to_uuid.get_mut(&name) { maybe_conflict.add(*pool_uuid); if let UuidOrConflict::Conflict(set) = maybe_conflict { @@ -847,7 +845,7 @@ impl LiminalDevices { fn try_setup_started_pool_failure( pools: &Table, pool_uuid: PoolUuid, - luks_info: StratisResult<(Option, MaybeInconsistent>)>, + luks_info: StratisResult>, infos: &HashMap, bdas: HashMap, metadata_version: StratisResult, @@ -895,9 +893,7 @@ impl LiminalDevices { assert!(!self.stopped_pools.contains_key(&pool_uuid)); let metadata_version = device_set.metadata_version(); - let encryption_info = device_set.encryption_info(); - let pool_name = device_set.pool_name(); - let luks_info = encryption_info.and_then(|ei| pool_name.map(|pn| (ei, pn))); + let luks_info = device_set.encryption_info(); let infos = match device_set.into_opened_set() { Either::Left(i) => i, Either::Right(ds) => { @@ -1074,8 +1070,8 @@ impl LiminalDevices { .insert(device_path.to_path_buf(), (pool_uuid, device_uuid)); devices.process_info_add(info); - match devices.pool_name() { - Ok(MaybeInconsistent::No(Some(name))) => { + match devices.pool_level_metadata_info() { + Ok((MaybeInconsistent::No(Some(name)), _)) => { if let Some(maybe_conflict) = self.name_to_uuid.get_mut(&name) { maybe_conflict.add(pool_uuid); if let UuidOrConflict::Conflict(set) = maybe_conflict { @@ -1111,8 +1107,8 @@ impl LiminalDevices { devices.process_info_remove(device_path, pool_uuid, dev_uuid); self.uuid_lookup.remove(device_path); - match devices.pool_name() { - Ok(MaybeInconsistent::No(Some(name))) => { + match devices.pool_level_metadata_info() { + Ok((MaybeInconsistent::No(Some(name)), _)) => { if let Some(maybe_conflict) = self.name_to_uuid.get_mut(&name) { maybe_conflict.add(pool_uuid); if let UuidOrConflict::Conflict(set) = maybe_conflict { @@ -1261,7 +1257,7 @@ fn load_stratis_metadata( ))); } - match get_metadata(infos) { + match get_metadata(&infos) { Ok(opt) => opt .ok_or_else(|| { StratisError::Msg(format!( @@ -1288,7 +1284,7 @@ fn load_stratis_metadata( fn setup_pool_legacy( pools: &Table, pool_uuid: PoolUuid, - luks_info: StratisResult<(Option, MaybeInconsistent>)>, + luks_info: StratisResult>, infos: &HashMap, bdas: HashMap, timestamp: DateTime, @@ -1317,7 +1313,7 @@ fn setup_pool_legacy( Ok((datadevs, cachedevs)) => (datadevs, cachedevs), }; - let (pool_einfo, pool_name) = match luks_info { + let pool_einfo = match luks_info { Ok(inner) => inner, Err(_) => { // NOTE: This is not actually a hopeless situation. It may be @@ -1336,21 +1332,9 @@ fn setup_pool_legacy( v1::StratPool::setup(pool_uuid, datadevs, cachedevs, timestamp, &metadata, pool_einfo) .map(|(name, mut pool)| { - if matches!(pool_name, MaybeInconsistent::Yes | MaybeInconsistent::No(None)) || MaybeInconsistent::No(Some(&name)) != pool_name.as_ref() || pool.blockdevs().iter().map(|(_, _, bd)| { + if pool.blockdevs().iter().map(|(_, _, bd)| { bd.pool_name() - }).fold(false, |acc, next| { - match next { - Some(Some(name)) => { - if MaybeInconsistent::No(Some(name)) == pool_name.as_ref() { - acc - } else { - true - } - }, - Some(None) => true, - None => false, - } - }) { + }).any(|name| name != Some(Some(&Name::new(metadata.name.clone()))) || matches!(name, Some(None))) { if let Err(e) = pool.rename_pool(&name) { warn!("Pool will not be able to be started by name; pool name metadata in LUKS2 token is not consistent across all devices: {}", e); } diff --git a/src/engine/strat_engine/liminal/setup.rs b/src/engine/strat_engine/liminal/setup.rs index ef84bb69da..b64ecd9131 100644 --- a/src/engine/strat_engine/liminal/setup.rs +++ b/src/engine/strat_engine/liminal/setup.rs @@ -26,7 +26,7 @@ use crate::{ device::blkdev_size, liminal::device_info::{LStratisDevInfo, LStratisInfo}, metadata::BDA, - serde_structs::{BackstoreSave, BaseBlockDevSave, PoolSave}, + serde_structs::{BackstoreSave, BaseBlockDevSave, EnabledPoolFeatures, PoolSave}, shared::{bds_to_bdas, tiers_to_bdas}, types::{BDARecordResult, BDAResult}, }, @@ -44,7 +44,7 @@ use crate::{ /// /// Precondition: infos and bdas have identical sets of keys pub fn get_metadata( - infos: HashMap, + infos: &HashMap, ) -> StratisResult, PoolSave)>> { // Try to read from all available devnodes that could contain most // recent metadata. In the event of errors, continue to try until all are @@ -82,9 +82,7 @@ pub fn get_metadata( /// metadata could be written. /// Returns an error if devices provided don't match the devices recorded in the /// metadata. -/// -/// Precondition: infos and bdas have identical sets of keys -pub fn get_name(infos: HashMap) -> StratisResult> { +pub fn get_name(infos: &HashMap) -> StratisResult> { let found_uuids = infos.keys().copied().collect::>(); match get_metadata(infos)? { Some((_, pool)) => { @@ -128,6 +126,58 @@ pub fn get_name(infos: HashMap) -> StratisResult, +) -> StratisResult> { + let found_uuids = infos.keys().copied().collect::>(); + match get_metadata(infos)? { + Some((_, pool)) => { + let v = []; + let meta_uuids = pool + .backstore + .data_tier + .blockdev + .devs + .iter() + .map(|bd| bd.uuid) + .chain( + pool.backstore + .cache_tier + .as_ref() + .map(|ct| ct.blockdev.devs.iter()) + .unwrap_or_else(|| v.iter()) + .map(|bd| bd.uuid), + ) + .collect::>(); + + if found_uuids != meta_uuids { + return Err(StratisError::Msg(format!( + "UUIDs in metadata ({}) did not match UUIDs found ({})", + Itertools::intersperse( + meta_uuids.into_iter().map(|u| u.to_string()), + ", ".to_string(), + ) + .collect::(), + Itertools::intersperse( + found_uuids.into_iter().map(|u| u.to_string()), + ", ".to_string(), + ) + .collect::(), + ))); + } + + Ok(Some(pool.features)) + } + None => Ok(None), + } +} + /// Get all the blockdevs corresponding to this pool that can be obtained from /// the given devices. Sort the blockdevs in the order in which they were /// recorded in the metadata. diff --git a/src/engine/strat_engine/serde_structs.rs b/src/engine/strat_engine/serde_structs.rs index 9ecc5304fc..5dd05657f5 100644 --- a/src/engine/strat_engine/serde_structs.rs +++ b/src/engine/strat_engine/serde_structs.rs @@ -17,7 +17,7 @@ use serde::{Serialize, Serializer}; use devicemapper::{Sectors, ThinDevId}; -use crate::engine::types::{DevUuid, FilesystemUuid}; +use crate::engine::types::{DevUuid, Features, FilesystemUuid}; const MAXIMUM_STRING_SIZE: usize = 255; @@ -102,6 +102,14 @@ impl EnabledPoolFeatures { } } +impl From for Features { + fn from(enabled: EnabledPoolFeatures) -> Self { + Features { + encryption: enabled.contains(PoolFeatures::Encryption), + } + } +} + // ALL structs that represent variable length metadata in pre-order // depth-first traversal order. Note that when organized by types rather than // values the structure is a DAG not a tree. This just means that there are diff --git a/src/engine/types/mod.rs b/src/engine/types/mod.rs index 6f2c626ce5..a763596558 100644 --- a/src/engine/types/mod.rs +++ b/src/engine/types/mod.rs @@ -247,6 +247,12 @@ pub struct StoppedPoolInfo { pub info: Option, pub devices: Vec, pub metadata_version: Option, + pub features: Option, +} + +#[derive(Debug, Eq, PartialEq)] +pub struct Features { + pub encryption: bool, } #[derive(Default, Debug, Eq, PartialEq)]