diff --git a/Cargo.lock b/Cargo.lock index 75d36ef2a0..e0b41c35e5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -612,6 +612,12 @@ version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d2fabcfbdc87f4758337ca535fb41a6d701b65693ce38287d856d1674551ec9b" +[[package]] +name = "heck" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" + [[package]] name = "hermit-abi" version = "0.2.6" @@ -1193,6 +1199,12 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "rustversion" +version = "1.0.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0e819f2bc632f285be6d7cd36e25940d45b2391dd6d9b939e79de557f7014248" + [[package]] name = "rusty-fork" version = "0.3.0" @@ -1337,6 +1349,8 @@ dependencies = [ "serde_json", "sha2", "stratisd_proc_macros", + "strum", + "strum_macros", "tempfile", "termios", "tokio", @@ -1358,6 +1372,25 @@ version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" +[[package]] +name = "strum" +version = "0.26.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8fec0f0aef304996cf250b31b5a10dee7980c85da9d759361292b8bca5a18f06" + +[[package]] +name = "strum_macros" +version = "0.26.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4c6bee85a5a24955dc440386795aa378cd9cf82acd5f764469152d2270e581be" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "rustversion", + "syn", +] + [[package]] name = "syn" version = "2.0.79" diff --git a/Cargo.toml b/Cargo.toml index 9eae046da8..90d307a36b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -213,6 +213,14 @@ version = "0.2.1" optional = true path = "./stratisd_proc_macros" +[dependencies.strum] +version = "0.26.0" +optional = true + +[dependencies.strum_macros] +version = "0.26.0" +optional = true + [dependencies.tempfile] version = "3.4.0" optional = true @@ -279,6 +287,8 @@ engine = [ "dep:serde_json", "dep:sha2", "dep:stratisd_proc_macros", + "dep:strum", + "dep:strum_macros", "dep:tempfile", "dep:tokio", "dep:uuid" diff --git a/src/bin/stratis-min/stratis-min.rs b/src/bin/stratis-min/stratis-min.rs index 5ec1b039ca..32aad13a39 100644 --- a/src/bin/stratis-min/stratis-min.rs +++ b/src/bin/stratis-min/stratis-min.rs @@ -13,7 +13,7 @@ use stratisd::{ CLEVIS_TANG_TRUST_URL, }, jsonrpc::client::{filesystem, key, pool, report}, - stratis::VERSION, + stratis::{StratisError, VERSION}, }; fn parse_args() -> Command { @@ -237,7 +237,9 @@ fn main() -> Result<(), String> { }; let unlock_method = match args.get_one::("unlock_method").map(|s| s.as_str()) { - Some(um) => Some(UnlockMethod::try_from(um)?), + Some(um) => Some(UnlockMethod::try_from(um).map_err(|_| { + StratisError::Msg(format!("{um} is an invalid unlock method")) + })?), None => None, }; let prompt = args.get_flag("prompt"); diff --git a/src/dbus_api/api/manager_3_0/methods.rs b/src/dbus_api/api/manager_3_0/methods.rs index e92584852b..46a2d256e7 100644 --- a/src/dbus_api/api/manager_3_0/methods.rs +++ b/src/dbus_api/api/manager_3_0/methods.rs @@ -179,7 +179,9 @@ pub fn unlock_pool(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { }; let unlock_method = { let unlock_method_str: &str = get_next_arg(&mut iter, 1)?; - match UnlockMethod::try_from(unlock_method_str) { + match UnlockMethod::try_from(unlock_method_str).map_err(|_| { + StratisError::Msg(format!("{unlock_method_str} is an invalid unlock method")) + }) { Ok(um) => um, Err(e) => { let (rc, rs) = engine_to_dbus_err_tuple(&e); diff --git a/src/dbus_api/api/manager_3_2/methods.rs b/src/dbus_api/api/manager_3_2/methods.rs index 0e89fd79a4..b601e331b6 100644 --- a/src/dbus_api/api/manager_3_2/methods.rs +++ b/src/dbus_api/api/manager_3_2/methods.rs @@ -46,13 +46,17 @@ pub fn start_pool(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { let unlock_method = { let unlock_method_tup: (bool, &str) = get_next_arg(&mut iter, 1)?; match tuple_to_option(unlock_method_tup) { - Some(unlock_method_str) => match UnlockMethod::try_from(unlock_method_str) { - Ok(um) => Some(um), - Err(e) => { - let (rc, rs) = engine_to_dbus_err_tuple(&e); - return Ok(vec![return_message.append3(default_return, rc, rs)]); + Some(unlock_method_str) => { + match UnlockMethod::try_from(unlock_method_str).map_err(|_| { + StratisError::Msg(format!("{unlock_method_str} is an invalid unlock method")) + }) { + Ok(um) => Some(um), + Err(e) => { + let (rc, rs) = engine_to_dbus_err_tuple(&e); + return Ok(vec![return_message.append3(default_return, rc, rs)]); + } } - }, + } None => None, } }; diff --git a/src/dbus_api/api/manager_3_4/methods.rs b/src/dbus_api/api/manager_3_4/methods.rs index 27a6f551c9..b0da65c317 100644 --- a/src/dbus_api/api/manager_3_4/methods.rs +++ b/src/dbus_api/api/manager_3_4/methods.rs @@ -52,13 +52,17 @@ pub fn start_pool(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { let unlock_method = { let unlock_method_tup: (bool, &str) = get_next_arg(&mut iter, 2)?; match tuple_to_option(unlock_method_tup) { - Some(unlock_method_str) => match UnlockMethod::try_from(unlock_method_str) { - Ok(um) => Some(um), - Err(e) => { - let (rc, rs) = engine_to_dbus_err_tuple(&e); - return Ok(vec![return_message.append3(default_return, rc, rs)]); + Some(unlock_method_str) => { + match UnlockMethod::try_from(unlock_method_str).map_err(|_| { + StratisError::Msg(format!("{unlock_method_str} is an invalid unlock method")) + }) { + Ok(um) => Some(um), + Err(e) => { + let (rc, rs) = engine_to_dbus_err_tuple(&e); + return Ok(vec![return_message.append3(default_return, rc, rs)]); + } } - }, + } None => None, } }; diff --git a/src/dbus_api/api/manager_3_8/methods.rs b/src/dbus_api/api/manager_3_8/methods.rs index de1367e63b..5b7a40d943 100644 --- a/src/dbus_api/api/manager_3_8/methods.rs +++ b/src/dbus_api/api/manager_3_8/methods.rs @@ -52,13 +52,17 @@ pub fn start_pool(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { let unlock_method = { let unlock_method_tup: (bool, &str) = get_next_arg(&mut iter, 2)?; match tuple_to_option(unlock_method_tup) { - Some(unlock_method_str) => match UnlockMethod::try_from(unlock_method_str) { - Ok(um) => Some(um), - Err(e) => { - let (rc, rs) = engine_to_dbus_err_tuple(&e); - return Ok(vec![return_message.append3(default_return, rc, rs)]); + Some(unlock_method_str) => { + match UnlockMethod::try_from(unlock_method_str).map_err(|_| { + StratisError::Msg(format!("{unlock_method_str} is an invalid unlock method")) + }) { + Ok(um) => Some(um), + Err(e) => { + let (rc, rs) = engine_to_dbus_err_tuple(&e); + return Ok(vec![return_message.append3(default_return, rc, rs)]); + } } - }, + } None => None, } }; diff --git a/src/dbus_api/api/report_3_0/methods.rs b/src/dbus_api/api/report_3_0/methods.rs index 2c6167949e..e0f9c8d410 100644 --- a/src/dbus_api/api/report_3_0/methods.rs +++ b/src/dbus_api/api/report_3_0/methods.rs @@ -11,6 +11,7 @@ use crate::{ util::{engine_to_dbus_err_tuple, get_next_arg}, }, engine::ReportType, + stratis::StratisError, }; pub fn get_report(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { @@ -21,7 +22,9 @@ pub fn get_report(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { let return_message = message.method_return(); let default_return = String::new(); - let report_type = match ReportType::try_from(report_name) { + let report_type = match ReportType::try_from(report_name) + .map_err(|_| StratisError::Msg(format!("Report name {report_name} not understood"))) + { Ok(rt) => rt, Err(e) => { let (rc, rs) = engine_to_dbus_err_tuple(&e); diff --git a/src/engine/strat_engine/metadata/static_header.rs b/src/engine/strat_engine/metadata/static_header.rs index aeb8f45cd8..195562d1b7 100644 --- a/src/engine/strat_engine/metadata/static_header.rs +++ b/src/engine/strat_engine/metadata/static_header.rs @@ -497,7 +497,7 @@ impl StaticHeader { let mut buf = [0u8; bytes!(static_header_size::SIGBLOCK_SECTORS)]; buf[4..20].clone_from_slice(STRAT_MAGIC); LittleEndian::write_u64(&mut buf[20..28], *self.blkdev_size.sectors()); - buf[28] = u8::from(self.sigblock_version); + buf[28] = self.sigblock_version as u8; buf[32..64].clone_from_slice(uuid_to_string!(self.identifiers.pool_uuid).as_bytes()); buf[64..96].clone_from_slice(uuid_to_string!(self.identifiers.device_uuid).as_bytes()); LittleEndian::write_u64(&mut buf[96..104], *self.mda_size.sectors()); @@ -532,7 +532,8 @@ impl StaticHeader { let blkdev_size = BlockdevSize::new(Sectors(LittleEndian::read_u64(&buf[20..28]))); let version_buf = buf[28]; - let version = StratSigblockVersion::try_from(version_buf)?; + let version = StratSigblockVersion::from_repr(version_buf) + .ok_or_else(|| StratisError::Msg(format!("Unknown sigblock version: {version_buf}")))?; let pool_uuid = PoolUuid::parse_str(from_utf8(&buf[32..64])?)?; let dev_uuid = DevUuid::parse_str(from_utf8(&buf[64..96])?)?; diff --git a/src/engine/strat_engine/names.rs b/src/engine/strat_engine/names.rs index 63e79447c5..ce3470b28f 100644 --- a/src/engine/strat_engine/names.rs +++ b/src/engine/strat_engine/names.rs @@ -6,6 +6,8 @@ use std::fmt::{self, Display}; +use strum_macros::{self}; + use devicemapper::{DmNameBuf, DmUuidBuf}; pub use crate::engine::types::KeyDescription; @@ -90,25 +92,18 @@ pub fn format_crypt_backstore_name(pool_uuid: &PoolUuid) -> DmNameBuf { DmNameBuf::new(value).expect("FORMAT_VERSION display length < 73") } -#[derive(Clone, Copy)] +#[derive(Clone, Copy, strum_macros::Display)] pub enum FlexRole { + #[strum(serialize = "mdv")] MetadataVolume, + #[strum(serialize = "thindata")] ThinData, + #[strum(serialize = "thinmeta")] ThinMeta, + #[strum(serialize = "thinmetaspare")] ThinMetaSpare, } -impl Display for FlexRole { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match *self { - FlexRole::MetadataVolume => write!(f, "mdv"), - FlexRole::ThinData => write!(f, "thindata"), - FlexRole::ThinMeta => write!(f, "thinmeta"), - FlexRole::ThinMetaSpare => write!(f, "thinmetaspare"), - } - } -} - #[derive(Clone, Copy)] pub enum ThinRole { Filesystem(FilesystemUuid), @@ -122,21 +117,15 @@ impl Display for ThinRole { } } -#[derive(Clone, Copy)] +#[derive(Clone, Copy, strum_macros::Display)] +#[strum(serialize_all = "lowercase")] pub enum ThinPoolRole { Pool, } -impl Display for ThinPoolRole { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match *self { - ThinPoolRole::Pool => write!(f, "pool"), - } - } -} - /// The various roles taken on by DM devices in the cache tier. -#[derive(Clone, Copy)] +#[derive(Clone, Copy, strum_macros::Display)] +#[strum(serialize_all = "lowercase")] pub enum CacheRole { /// The DM cache device, contains the other three devices. Cache, @@ -148,17 +137,6 @@ pub enum CacheRole { OriginSub, } -impl Display for CacheRole { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match *self { - CacheRole::Cache => write!(f, "cache"), - CacheRole::CacheSub => write!(f, "cachesub"), - CacheRole::MetaSub => write!(f, "metasub"), - CacheRole::OriginSub => write!(f, "originsub"), - } - } -} - /// Format a name & uuid for the flex layer. /// /// Prerequisite: len(format!("{}", FORMAT_VERSION) diff --git a/src/engine/strat_engine/thinpool/thinpool.rs b/src/engine/strat_engine/thinpool/thinpool.rs index 1580f4d47b..545e65ba95 100644 --- a/src/engine/strat_engine/thinpool/thinpool.rs +++ b/src/engine/strat_engine/thinpool/thinpool.rs @@ -7,7 +7,6 @@ use std::{ cmp::{max, min, Ordering}, collections::{hash_map::Entry, HashMap, HashSet}, - fmt, marker::PhantomData, thread::scope, }; @@ -78,6 +77,14 @@ mod consts { pub const DATA_LOWATER: DataBlocks = DataBlocks(4 * IEC::Ki); } +#[derive(strum_macros::AsRefStr)] +#[strum(serialize_all = "snake_case")] +enum FeatureArg { + ErrorIfNoSpace, + NoDiscardPassdown, + SkipBlockZeroing, +} + fn sectors_to_datablocks(sectors: Sectors) -> DataBlocks { DataBlocks(sectors / DATA_BLOCK_SIZE) } @@ -171,12 +178,19 @@ struct Segments { /// that can be checked for equality. This way, two statuses, /// collected at different times can be checked to determine whether their /// gross, as opposed to fine, differences are significant. -#[derive(Clone, Copy, Debug, Eq, PartialEq)] +/// In this implementation convert the status designations to strings which +/// match those strings that the kernel uses to identify the different states +#[derive(Clone, Copy, Debug, Eq, PartialEq, strum_macros::AsRefStr)] pub enum ThinPoolStatusDigest { + #[strum(serialize = "Fail")] Fail, + #[strum(serialize = "Error")] Error, + #[strum(serialize = "rw")] Good, + #[strum(serialize = "ro")] ReadOnly, + #[strum(serialize = "out_of_data_space")] OutOfSpace, } @@ -194,21 +208,6 @@ impl From<&ThinPoolStatus> for ThinPoolStatusDigest { } } -/// In this implementation convert the status designations to strings which -/// match those strings that the kernel uses to identify the different states -/// in the ioctl result. -impl fmt::Display for ThinPoolStatusDigest { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - ThinPoolStatusDigest::Good => write!(f, "rw"), - ThinPoolStatusDigest::ReadOnly => write!(f, "ro"), - ThinPoolStatusDigest::OutOfSpace => write!(f, "out_of_data_space"), - ThinPoolStatusDigest::Fail => write!(f, "Fail"), - ThinPoolStatusDigest::Error => write!(f, "Error"), - } - } -} - /// Calculate the room available for data that is not taken up by metadata. fn room_for_data(usable_size: Sectors, meta_size: Sectors) -> Sectors { Sectors( @@ -346,8 +345,9 @@ impl ThinPool { if current_status != new_status { let current_status_str = current_status - .map(|x| x.to_string()) - .unwrap_or_else(|| "none".to_string()); + .as_ref() + .map(|x| x.as_ref()) + .unwrap_or_else(|| "none"); if new_status != Some(ThinPoolStatusDigest::Good) { warn!( @@ -355,8 +355,9 @@ impl ThinPool { thin_pool_identifiers(&self.thin_pool), current_status_str, new_status - .map(|s| s.to_string()) - .unwrap_or_else(|| "none".to_string()), + .as_ref() + .map(|s| s.as_ref()) + .unwrap_or_else(|| "none"), ); } else { info!( @@ -364,8 +365,9 @@ impl ThinPool { thin_pool_identifiers(&self.thin_pool), current_status_str, new_status - .map(|s| s.to_string()) - .unwrap_or_else(|| "none".to_string()), + .as_ref() + .map(|s| s.as_ref()) + .unwrap_or_else(|| "none"), ); } } @@ -452,7 +454,7 @@ impl ThinPool { .table .params .feature_args - .contains("error_if_no_space") + .contains(FeatureArg::ErrorIfNoSpace.as_ref()) } pub fn get_filesystem_by_uuid(&self, uuid: FilesystemUuid) -> Option<(Name, &StratFilesystem)> { @@ -865,8 +867,8 @@ impl ThinPool { DataBlocks((data_dev_size / DATA_BLOCK_SIZE) / 2), ), vec![ - "no_discard_passdown".to_string(), - "skip_block_zeroing".to_string(), + FeatureArg::NoDiscardPassdown.as_ref().to_string(), + FeatureArg::SkipBlockZeroing.as_ref().to_string(), ], )?; @@ -1052,8 +1054,8 @@ impl ThinPool { DataBlocks((data_dev_size / DATA_BLOCK_SIZE) / 2), ), vec![ - "no_discard_passdown".to_string(), - "skip_block_zeroing".to_string(), + FeatureArg::NoDiscardPassdown.as_ref().to_string(), + FeatureArg::SkipBlockZeroing.as_ref().to_string(), ], )?; @@ -1244,9 +1246,9 @@ where .unwrap_or_else(|| { migrate = true; vec![ - "no_discard_passdown".to_owned(), - "skip_block_zeroing".to_owned(), - "error_if_no_space".to_owned(), + FeatureArg::NoDiscardPassdown.as_ref().to_string(), + FeatureArg::SkipBlockZeroing.as_ref().to_string(), + FeatureArg::ErrorIfNoSpace.as_ref().to_string(), ] }), )?; diff --git a/src/engine/types/mod.rs b/src/engine/types/mod.rs index ba108a21af..1909d6487d 100644 --- a/src/engine/types/mod.rs +++ b/src/engine/types/mod.rs @@ -16,6 +16,7 @@ use std::{ use libudev::EventType; use serde::{Deserialize, Serialize}; use serde_json::Value; +use strum_macros::{self, EnumString, FromRepr}; use uuid::Uuid; pub use crate::engine::{ @@ -130,28 +131,14 @@ impl Display for StratisUuid { } /// Use Clevis or keyring to unlock LUKS volume. -#[derive(Serialize, Deserialize, Clone, Copy, Eq, PartialEq, Debug)] +#[derive(Serialize, Deserialize, Clone, Copy, Eq, PartialEq, Debug, EnumString)] +#[strum(serialize_all = "snake_case")] pub enum UnlockMethod { Clevis, Keyring, Any, } -impl<'a> TryFrom<&'a str> for UnlockMethod { - type Error = StratisError; - - fn try_from(s: &str) -> StratisResult { - match s { - "keyring" => Ok(UnlockMethod::Keyring), - "clevis" => Ok(UnlockMethod::Clevis), - "any" => Ok(UnlockMethod::Any), - _ => Err(StratisError::Msg(format!( - "{s} is an invalid unlock method" - ))), - } - } -} - /// Blockdev tier. Used to distinguish between blockdevs used for /// data and blockdevs used for a cache. #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -206,23 +193,12 @@ impl fmt::Display for Name { /// /// * `ErroredPoolDevices` returns the state of devices that caused an error while /// attempting to reconstruct a pool. +#[derive(EnumString)] +#[strum(serialize_all = "snake_case")] pub enum ReportType { StoppedPools, } -impl<'a> TryFrom<&'a str> for ReportType { - type Error = StratisError; - - fn try_from(name: &str) -> StratisResult { - match name { - "stopped_pools" => Ok(ReportType::StoppedPools), - _ => Err(StratisError::Msg(format!( - "Report name {name} not understood" - ))), - } - } -} - #[derive(Debug, Eq, PartialEq)] pub struct PoolDevice { pub devnode: PathBuf, @@ -384,31 +360,20 @@ impl Deref for DevicePath { } /// Represents what actions this pool can accept. -#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone)] +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, strum_macros::Display)] pub enum ActionAvailability { /// Full set of actions may be taken + #[strum(serialize = "fully_operational")] Full = 0, /// No requests via an IPC mechanism may be taken + #[strum(serialize = "no_ipc_requests")] NoRequests = 1, /// No changes may be made to the pool including background changes /// like reacting to devicemapper events + #[strum(serialize = "no_pool_changes")] NoPoolChanges = 2, } -impl Display for ActionAvailability { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - "{}", - match self { - ActionAvailability::Full => "fully_operational", - ActionAvailability::NoRequests => "no_ipc_requests", - ActionAvailability::NoPoolChanges => "no_pool_changes", - } - ) - } -} - /// Indicates that a property that should be consistent across block devices /// in a pool may be inconsistent. #[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)] @@ -510,31 +475,9 @@ impl UuidOrConflict { } } -#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, FromRepr)] +#[repr(u8)] pub enum StratSigblockVersion { V1 = 1, V2 = 2, } - -impl TryFrom for StratSigblockVersion { - type Error = StratisError; - - fn try_from(value: u8) -> Result { - match value { - 1u8 => Ok(StratSigblockVersion::V1), - 2u8 => Ok(StratSigblockVersion::V2), - _ => Err(StratisError::Msg(format!( - "Unknown sigblock version: {value}" - ))), - } - } -} - -impl From for u8 { - fn from(version: StratSigblockVersion) -> Self { - match version { - StratSigblockVersion::V1 => 1u8, - StratSigblockVersion::V2 => 2u8, - } - } -}