From f111f330b276b1b683ba9c8cc18d720f6fcca7a2 Mon Sep 17 00:00:00 2001 From: John Baublitz Date: Thu, 7 Nov 2024 10:03:58 -0500 Subject: [PATCH 01/11] Record integrity parameters in metadata --- src/bin/utils/predict_usage.rs | 12 +- src/engine/mod.rs | 1 + .../strat_engine/backstore/backstore/v2.rs | 44 ++++-- .../strat_engine/backstore/blockdev/mod.rs | 11 -- .../strat_engine/backstore/blockdev/v1.rs | 140 +++++++++--------- .../strat_engine/backstore/blockdev/v2.rs | 110 ++++++++------ .../strat_engine/backstore/blockdevmgr.rs | 37 +++-- .../strat_engine/backstore/data_tier.rs | 69 +++++++-- src/engine/strat_engine/backstore/mod.rs | 3 + src/engine/strat_engine/mod.rs | 5 +- src/engine/strat_engine/pool/v2.rs | 10 +- src/engine/strat_engine/serde_structs.rs | 8 +- src/engine/strat_engine/thinpool/thinpool.rs | 20 +++ 13 files changed, 310 insertions(+), 160 deletions(-) diff --git a/src/bin/utils/predict_usage.rs b/src/bin/utils/predict_usage.rs index a7757fa4b5..f9c5f80d81 100644 --- a/src/bin/utils/predict_usage.rs +++ b/src/bin/utils/predict_usage.rs @@ -13,7 +13,10 @@ use serde_json::{json, Value}; use devicemapper::{Bytes, Sectors}; -use stratisd::engine::{crypt_metadata_size, integrity_meta_space, ThinPoolSizeParams, BDA}; +use stratisd::engine::{ + crypt_metadata_size, integrity_meta_space, ThinPoolSizeParams, BDA, + DEFAULT_INTEGRITY_BLOCK_SIZE, DEFAULT_INTEGRITY_JOURNAL_SIZE, DEFAULT_INTEGRITY_TAG_SIZE, +}; // 2^FS_SIZE_START_POWER is the logical size of the smallest Stratis // filesystem for which usage data exists in FSSizeLookup::internal, i.e., @@ -168,7 +171,12 @@ fn predict_pool_metadata_usage(device_sizes: Vec) -> Result, + integrity_journal_size: Option, + integrity_tag_size: Option, ) -> StratisResult { - let data_tier = DataTier::::new(BlockDevMgr::::initialize( - pool_uuid, - devices, - mda_data_size, - )?); + let data_tier = DataTier::::new( + BlockDevMgr::::initialize(pool_uuid, devices, mda_data_size)?, + integrity_journal_size, + None, + integrity_tag_size, + ); let mut backstore = Backstore { data_tier, @@ -1248,8 +1252,15 @@ mod tests { let initdatadevs = get_devices(initdatapaths).unwrap(); let initcachedevs = get_devices(initcachepaths).unwrap(); - let mut backstore = - Backstore::initialize(pool_uuid, initdatadevs, MDADataSize::default(), None).unwrap(); + let mut backstore = Backstore::initialize( + pool_uuid, + initdatadevs, + MDADataSize::default(), + None, + None, + None, + ) + .unwrap(); invariant(&backstore); @@ -1340,8 +1351,15 @@ mod tests { let devices1 = get_devices(paths1).unwrap(); let devices2 = get_devices(paths2).unwrap(); - let mut backstore = - Backstore::initialize(pool_uuid, devices1, MDADataSize::default(), None).unwrap(); + let mut backstore = Backstore::initialize( + pool_uuid, + devices1, + MDADataSize::default(), + None, + None, + None, + ) + .unwrap(); for path in paths1 { assert_eq!( @@ -1404,6 +1422,8 @@ mod tests { "tang".to_string(), json!({"url": env::var("TANG_URL").expect("TANG_URL env var required"), "stratis:tang:trust_url": true}), ))), + None, + None, ) .unwrap(); backstore.alloc(pool_uuid, &[Sectors(512)]).unwrap(); @@ -1478,6 +1498,8 @@ mod tests { json!({"url": env::var("TANG_URL").expect("TANG_URL env var required"), "stratis:tang:trust_url": true}), ), )), + None, + None, ).unwrap(); cmd::udev_settle().unwrap(); diff --git a/src/engine/strat_engine/backstore/blockdev/mod.rs b/src/engine/strat_engine/backstore/blockdev/mod.rs index 986a2a8a2b..9fa01c99dc 100644 --- a/src/engine/strat_engine/backstore/blockdev/mod.rs +++ b/src/engine/strat_engine/backstore/blockdev/mod.rs @@ -95,17 +95,6 @@ pub trait InternalBlockDev { /// * Otherwise, `Some(_)` fn calc_new_size(&self) -> StratisResult>; - /// Grow the block device if the underlying physical device has grown in size. - /// Return an error and leave the size as is if the device has shrunk. - /// Do nothing if the device is the same size as recorded in the metadata. - /// - /// This method does not need to block IO to the extended crypt device prior - /// to rollback because of per-pool locking. Growing the device will acquire - /// an exclusive lock on the pool and therefore the thin pool cannot be - /// extended to use the larger or unencrypted block device size until the - /// transaction has been completed successfully. - fn grow(&mut self) -> StratisResult; - /// Load the pool-level metadata for the given block device. fn load_state(&self) -> StratisResult, &DateTime)>>; diff --git a/src/engine/strat_engine/backstore/blockdev/v1.rs b/src/engine/strat_engine/backstore/blockdev/v1.rs index f5002cd952..dd628fca0f 100644 --- a/src/engine/strat_engine/backstore/blockdev/v1.rs +++ b/src/engine/strat_engine/backstore/blockdev/v1.rs @@ -336,72 +336,16 @@ impl StratBlockDev { } } - #[cfg(test)] - pub fn invariant(&self) { - assert!(self.total_size() == self.used.size()); - } -} - -impl InternalBlockDev for StratBlockDev { - fn uuid(&self) -> DevUuid { - self.bda.dev_uuid() - } - - fn device(&self) -> &Device { - &self.dev - } - - fn physical_path(&self) -> &Path { - self.devnode() - } - - fn blksizes(&self) -> StratSectorSizes { - self.blksizes - } - - fn metadata_version(&self) -> StratSigblockVersion { - self.bda.sigblock_version() - } - - fn total_size(&self) -> BlockdevSize { - self.bda.dev_size() - } - - fn available(&self) -> Sectors { - self.used.available() - } - - fn metadata_size(&self) -> Sectors { - self.bda.extended_size().sectors() - } - - fn max_stratis_metadata_size(&self) -> MDADataSize { - self.bda.max_data_size() - } - - fn in_use(&self) -> bool { - self.used.used() > self.metadata_size() - } - - fn alloc(&mut self, size: Sectors) -> PerDevSegments { - self.used.alloc_front(size) - } - - fn calc_new_size(&self) -> StratisResult> { - let s = Self::scan_blkdev_size( - self.physical_path(), - self.underlying_device.crypt_handle().is_some(), - )?; - if Some(s) == self.new_size - || (self.new_size.is_none() && s == self.bda.dev_size().sectors()) - { - Ok(None) - } else { - Ok(Some(s)) - } - } - - fn grow(&mut self) -> StratisResult { + /// Grow the block device if the underlying physical device has grown in size. + /// Return an error and leave the size as is if the device has shrunk. + /// Do nothing if the device is the same size as recorded in the metadata. + /// + /// This method does not need to block IO to the extended crypt device prior + /// to rollback because of per-pool locking. Growing the device will acquire + /// an exclusive lock on the pool and therefore the thin pool cannot be + /// extended to use the larger or unencrypted block device size until the + /// transaction has been completed successfully. + pub fn grow(&mut self) -> StratisResult { /// Precondition: size > h.blkdev_size fn needs_rollback(bd: &mut StratBlockDev, size: BlockdevSize) -> StratisResult<()> { let mut f = OpenOptions::new() @@ -472,6 +416,70 @@ impl InternalBlockDev for StratBlockDev { } } } + #[cfg(test)] + pub fn invariant(&self) { + assert!(self.total_size() == self.used.size()); + } +} + +impl InternalBlockDev for StratBlockDev { + fn uuid(&self) -> DevUuid { + self.bda.dev_uuid() + } + + fn device(&self) -> &Device { + &self.dev + } + + fn physical_path(&self) -> &Path { + self.devnode() + } + + fn blksizes(&self) -> StratSectorSizes { + self.blksizes + } + + fn metadata_version(&self) -> StratSigblockVersion { + self.bda.sigblock_version() + } + + fn total_size(&self) -> BlockdevSize { + self.bda.dev_size() + } + + fn available(&self) -> Sectors { + self.used.available() + } + + fn metadata_size(&self) -> Sectors { + self.bda.extended_size().sectors() + } + + fn max_stratis_metadata_size(&self) -> MDADataSize { + self.bda.max_data_size() + } + + fn in_use(&self) -> bool { + self.used.used() > self.metadata_size() + } + + fn alloc(&mut self, size: Sectors) -> PerDevSegments { + self.used.alloc_front(size) + } + + fn calc_new_size(&self) -> StratisResult> { + let s = Self::scan_blkdev_size( + self.physical_path(), + self.underlying_device.crypt_handle().is_some(), + )?; + if Some(s) == self.new_size + || (self.new_size.is_none() && s == self.bda.dev_size().sectors()) + { + Ok(None) + } else { + Ok(Some(s)) + } + } fn load_state(&self) -> StratisResult, &DateTime)>> { let mut f = OpenOptions::new() diff --git a/src/engine/strat_engine/backstore/blockdev/v2.rs b/src/engine/strat_engine/backstore/blockdev/v2.rs index 4c872b0cb6..06c5a745f3 100644 --- a/src/engine/strat_engine/backstore/blockdev/v2.rs +++ b/src/engine/strat_engine/backstore/blockdev/v2.rs @@ -14,7 +14,7 @@ use std::{ use chrono::{DateTime, Utc}; use serde_json::Value; -use devicemapper::{Bytes, Device, Sectors, IEC}; +use devicemapper::{Bytes, Device, Sectors}; use crate::{ engine::{ @@ -43,14 +43,19 @@ use crate::{ /// Return the amount of space required for integrity for a device of the given size. /// -/// This is a slight overestimation for the sake of simplicity. Because it uses the whole disk +/// This default is a slight overestimation for the sake of simplicity. Because it uses the whole disk /// size, once the integrity metadata size is calculated, the remaining data size is now smaller /// than the metadata region could support for integrity. /// The result is divisible by 8 sectors. -pub fn integrity_meta_space(total_space: Sectors) -> Sectors { +pub fn integrity_meta_space( + total_space: Sectors, + journal_size: Sectors, + block_size: Bytes, + tag_size: Bytes, +) -> Sectors { Bytes(4096).sectors() - + Bytes::from(64 * IEC::Mi).sectors() - + Bytes::from((*total_space * 32u64 + 4095) & !4095).sectors() + + journal_size + + Bytes::from(((*total_space.bytes() / *block_size) * *tag_size + 4095) & !4095).sectors() } #[derive(Debug)] @@ -206,6 +211,61 @@ impl StratBlockDev { } } + /// Grow the block device if the underlying physical device has grown in size. + /// Return an error and leave the size as is if the device has shrunk. + /// Do nothing if the device is the same size as recorded in the metadata. + /// + /// This will also extend integrity metadata reservations according to the new + /// size of the device. + pub fn grow( + &mut self, + integrity_journal_size: Sectors, + integrity_block_size: Bytes, + integrity_tag_size: Bytes, + ) -> StratisResult { + let size = BlockdevSize::new(Self::scan_blkdev_size(self.devnode())?); + let metadata_size = self.bda.dev_size(); + match size.cmp(&metadata_size) { + Ordering::Less => Err(StratisError::Msg( + "The underlying device appears to have shrunk; you may experience data loss" + .to_string(), + )), + Ordering::Equal => Ok(false), + Ordering::Greater => { + let mut f = OpenOptions::new() + .write(true) + .read(true) + .open(self.devnode())?; + let mut h = static_header(&mut f)?.ok_or_else(|| { + StratisError::Msg(format!( + "No static header found on device {}", + self.devnode().display() + )) + })?; + + h.blkdev_size = size; + let h = StaticHeader::write_header(&mut f, h, MetadataLocation::Both)?; + + self.bda.header = h; + self.used.increase_size(size.sectors()); + + let integrity_grow = integrity_meta_space( + size.sectors(), + integrity_journal_size, + integrity_block_size, + integrity_tag_size, + ) - self + .integrity_meta_allocs + .iter() + .map(|(_, len)| *len) + .sum::(); + self.alloc_int_meta_back(integrity_grow); + + Ok(true) + } + } + } + #[cfg(test)] pub fn invariant(&self) { assert!(self.total_size() == self.used.size()); @@ -269,46 +329,6 @@ impl InternalBlockDev for StratBlockDev { } } - fn grow(&mut self) -> StratisResult { - let size = BlockdevSize::new(Self::scan_blkdev_size(self.devnode())?); - let metadata_size = self.bda.dev_size(); - match size.cmp(&metadata_size) { - Ordering::Less => Err(StratisError::Msg( - "The underlying device appears to have shrunk; you may experience data loss" - .to_string(), - )), - Ordering::Equal => Ok(false), - Ordering::Greater => { - let mut f = OpenOptions::new() - .write(true) - .read(true) - .open(self.devnode())?; - let mut h = static_header(&mut f)?.ok_or_else(|| { - StratisError::Msg(format!( - "No static header found on device {}", - self.devnode().display() - )) - })?; - - h.blkdev_size = size; - let h = StaticHeader::write_header(&mut f, h, MetadataLocation::Both)?; - - self.bda.header = h; - self.used.increase_size(size.sectors()); - - let integrity_grow = integrity_meta_space(size.sectors()) - - self - .integrity_meta_allocs - .iter() - .map(|(_, len)| *len) - .sum::(); - self.alloc_int_meta_back(integrity_grow); - - Ok(true) - } - } - } - fn load_state(&self) -> StratisResult, &DateTime)>> { let mut f = OpenOptions::new().read(true).open(&*self.devnode)?; match (self.bda.load_state(&mut f)?, self.bda.last_update_time()) { diff --git a/src/engine/strat_engine/backstore/blockdevmgr.rs b/src/engine/strat_engine/backstore/blockdevmgr.rs index 04525081c3..6a1807886e 100644 --- a/src/engine/strat_engine/backstore/blockdevmgr.rs +++ b/src/engine/strat_engine/backstore/blockdevmgr.rs @@ -165,6 +165,15 @@ impl BlockDevMgr { self.encryption_info().is_some() } + pub fn grow(&mut self, dev: DevUuid) -> StratisResult { + let bd = self + .block_devs + .iter_mut() + .find(|bd| bd.uuid() == dev) + .ok_or_else(|| StratisError::Msg(format!("Block device with UUID {dev} not found")))?; + bd.grow() + } + #[cfg(test)] fn invariant(&self) { let pool_uuids = self @@ -234,6 +243,25 @@ impl BlockDevMgr { Ok(bdev_uuids) } + pub fn grow( + &mut self, + dev: DevUuid, + integrity_journal_size: Sectors, + integrity_block_size: Bytes, + integrity_tag_size: Bytes, + ) -> StratisResult { + let bd = self + .block_devs + .iter_mut() + .find(|bd| bd.uuid() == dev) + .ok_or_else(|| StratisError::Msg(format!("Block device with UUID {dev} not found")))?; + bd.grow( + integrity_journal_size, + integrity_block_size, + integrity_tag_size, + ) + } + #[cfg(test)] fn invariant(&self) { let pool_uuids = self @@ -467,15 +495,6 @@ where self.block_devs.iter().map(|bd| bd.metadata_size()).sum() } - pub fn grow(&mut self, dev: DevUuid) -> StratisResult { - let bd = self - .block_devs - .iter_mut() - .find(|bd| bd.uuid() == dev) - .ok_or_else(|| StratisError::Msg(format!("Block device with UUID {dev} not found")))?; - bd.grow() - } - /// Tear down devicemapper devices for the block devices in this BlockDevMgr. pub fn teardown(&mut self) -> StratisResult<()> { let errs = self.block_devs.iter_mut().fold(Vec::new(), |mut errs, bd| { diff --git a/src/engine/strat_engine/backstore/data_tier.rs b/src/engine/strat_engine/backstore/data_tier.rs index 04fd6fc9b5..b6d4a44065 100644 --- a/src/engine/strat_engine/backstore/data_tier.rs +++ b/src/engine/strat_engine/backstore/data_tier.rs @@ -7,7 +7,7 @@ #[cfg(test)] use std::collections::HashSet; -use devicemapper::Sectors; +use devicemapper::{Bytes, Sectors, IEC}; use crate::{ engine::{ @@ -32,6 +32,10 @@ use crate::{ stratis::StratisResult, }; +pub const DEFAULT_INTEGRITY_JOURNAL_SIZE: Bytes = Bytes(128 * IEC::Mi as u128); +pub const DEFAULT_INTEGRITY_BLOCK_SIZE: Bytes = Bytes(4 * IEC::Ki as u128); +pub const DEFAULT_INTEGRITY_TAG_SIZE: Bytes = Bytes(64u128); + /// Handles the lowest level, base layer of this tier. #[derive(Debug)] pub struct DataTier { @@ -39,6 +43,12 @@ pub struct DataTier { pub(super) block_mgr: BlockDevMgr, /// The list of segments granted by block_mgr and used by dm_device pub(super) segments: AllocatedAbove, + /// Integrity journal size. + integrity_journal_size: Option, + /// Integrity block size. + integrity_block_size: Option, + /// Integrity tag size. + integrity_tag_size: Option, } impl DataTier { @@ -52,6 +62,9 @@ impl DataTier { DataTier { block_mgr, segments: AllocatedAbove { inner: vec![] }, + integrity_journal_size: None, + integrity_block_size: None, + integrity_tag_size: None, } } @@ -97,6 +110,10 @@ impl DataTier { pub fn blockdevs_mut(&mut self) -> Vec<(DevUuid, &mut v1::StratBlockDev)> { self.block_mgr.blockdevs_mut() } + + pub fn grow(&mut self, dev: DevUuid) -> StratisResult { + self.block_mgr.grow(dev) + } } impl DataTier { @@ -105,16 +122,33 @@ impl DataTier { /// Initially 0 segments are allocated. /// /// WARNING: metadata changing event - pub fn new(mut block_mgr: BlockDevMgr) -> DataTier { + pub fn new( + mut block_mgr: BlockDevMgr, + integrity_journal_size: Option, + integrity_block_size: Option, + integrity_tag_size: Option, + ) -> DataTier { + let integrity_journal_size = + integrity_journal_size.unwrap_or_else(|| DEFAULT_INTEGRITY_JOURNAL_SIZE.sectors()); + let integrity_block_size = integrity_block_size.unwrap_or(DEFAULT_INTEGRITY_BLOCK_SIZE); + let integrity_tag_size = integrity_tag_size.unwrap_or(DEFAULT_INTEGRITY_TAG_SIZE); for (_, bd) in block_mgr.blockdevs_mut() { // NOTE: over-allocates integrity metadata slightly. Some of the // total size of the device will not make use of the integrity // metadata. - bd.alloc_int_meta_back(integrity_meta_space(bd.total_size().sectors())); + bd.alloc_int_meta_back(integrity_meta_space( + bd.total_size().sectors(), + integrity_journal_size, + integrity_block_size, + integrity_tag_size, + )); } DataTier { block_mgr, segments: AllocatedAbove { inner: vec![] }, + integrity_journal_size: Some(integrity_journal_size), + integrity_block_size: Some(integrity_block_size), + integrity_tag_size: Some(integrity_tag_size), } } @@ -142,10 +176,10 @@ impl DataTier { assert_eq!(bds.len(), uuids.len()); for bd in bds { bd.alloc_int_meta_back(integrity_meta_space( - // NOTE: Subtracting metadata size works here because the only metadata currently - // recorded in a newly created block device is the BDA. If this becomes untrue in - // the future, this code will no longer work. - bd.total_size().sectors() - bd.metadata_size(), + bd.total_size().sectors(), + self.integrity_journal_size.expect("Must be some in V2"), + self.integrity_block_size.expect("Must be some in V2"), + self.integrity_tag_size.expect("Must be some in V2"), )); } Ok(uuids) @@ -179,6 +213,15 @@ impl DataTier { pub fn blockdevs_mut(&mut self) -> Vec<(DevUuid, &mut v2::StratBlockDev)> { self.block_mgr.blockdevs_mut() } + + pub fn grow(&mut self, dev: DevUuid) -> StratisResult { + self.block_mgr.grow( + dev, + self.integrity_journal_size.expect("Must be Some in V2"), + self.integrity_block_size.expect("Must be Some in V2"), + self.integrity_tag_size.expect("Must be Some in V2"), + ) + } } impl DataTier @@ -207,6 +250,9 @@ where Ok(DataTier { block_mgr, segments, + integrity_journal_size: data_tier_save.integrity_journal_size, + integrity_block_size: data_tier_save.integrity_block_size, + integrity_tag_size: data_tier_save.integrity_tag_size, }) } @@ -265,10 +311,6 @@ where self.block_mgr.load_state() } - pub fn grow(&mut self, dev: DevUuid) -> StratisResult { - self.block_mgr.grow(dev) - } - /// Return the partition of the block devs that are in use and those /// that are not. pub fn partition_by_use(&self) -> BlockDevPartition<'_, B> { @@ -301,6 +343,9 @@ where allocs: vec![self.segments.record()], devs: self.block_mgr.record(), }, + integrity_journal_size: self.integrity_journal_size, + integrity_block_size: self.integrity_block_size, + integrity_tag_size: self.integrity_tag_size, } } } @@ -439,7 +484,7 @@ mod tests { ) .unwrap(); - let mut data_tier = DataTier::::new(mgr); + let mut data_tier = DataTier::::new(mgr, None, None, None); data_tier.invariant(); // A data_tier w/ some devices but nothing allocated diff --git a/src/engine/strat_engine/backstore/mod.rs b/src/engine/strat_engine/backstore/mod.rs index 58add8867b..3311b24253 100644 --- a/src/engine/strat_engine/backstore/mod.rs +++ b/src/engine/strat_engine/backstore/mod.rs @@ -14,6 +14,9 @@ mod shared; pub use self::{ blockdev::v2::integrity_meta_space, + data_tier::{ + DEFAULT_INTEGRITY_BLOCK_SIZE, DEFAULT_INTEGRITY_JOURNAL_SIZE, DEFAULT_INTEGRITY_TAG_SIZE, + }, devices::{find_stratis_devs_by_uuid, get_devno_from_path, ProcessedPathInfos, UnownedDevices}, }; diff --git a/src/engine/strat_engine/mod.rs b/src/engine/strat_engine/mod.rs index 37fd872abf..1a1c12fd08 100644 --- a/src/engine/strat_engine/mod.rs +++ b/src/engine/strat_engine/mod.rs @@ -29,7 +29,10 @@ pub use self::{backstore::ProcessedPathInfos, pool::v1::StratPool}; pub use self::pool::inspection as pool_inspection; pub use self::{ - backstore::integrity_meta_space, + backstore::{ + integrity_meta_space, DEFAULT_INTEGRITY_BLOCK_SIZE, DEFAULT_INTEGRITY_JOURNAL_SIZE, + DEFAULT_INTEGRITY_TAG_SIZE, + }, crypt::{ crypt_metadata_size, register_clevis_token, set_up_crypt_logging, CLEVIS_TANG_TRUST_URL, }, diff --git a/src/engine/strat_engine/pool/v2.rs b/src/engine/strat_engine/pool/v2.rs index d28c5a1831..8ec0a5b540 100644 --- a/src/engine/strat_engine/pool/v2.rs +++ b/src/engine/strat_engine/pool/v2.rs @@ -159,8 +159,14 @@ impl StratPool { // FIXME: Initializing with the minimum MDA size is not necessarily // enough. If there are enough devices specified, more space will be // required. - let mut backstore = - Backstore::initialize(pool_uuid, devices, MDADataSize::default(), encryption_info)?; + let mut backstore = Backstore::initialize( + pool_uuid, + devices, + MDADataSize::default(), + encryption_info, + None, + None, + )?; let thinpool = ThinPool::::new( pool_uuid, diff --git a/src/engine/strat_engine/serde_structs.rs b/src/engine/strat_engine/serde_structs.rs index a9c907139d..bb23225b44 100644 --- a/src/engine/strat_engine/serde_structs.rs +++ b/src/engine/strat_engine/serde_structs.rs @@ -14,7 +14,7 @@ use serde::{Serialize, Serializer}; -use devicemapper::{Sectors, ThinDevId}; +use devicemapper::{Bytes, Sectors, ThinDevId}; use crate::engine::types::{DevUuid, Features, FilesystemUuid}; @@ -117,6 +117,12 @@ pub struct BackstoreSave { #[derive(Debug, Deserialize, Eq, PartialEq, Serialize)] pub struct DataTierSave { pub blockdev: BlockDevSave, + #[serde(skip_serializing_if = "Option::is_none")] + pub integrity_journal_size: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub integrity_block_size: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub integrity_tag_size: Option, } #[derive(Debug, Deserialize, Eq, PartialEq, Serialize)] diff --git a/src/engine/strat_engine/thinpool/thinpool.rs b/src/engine/strat_engine/thinpool/thinpool.rs index aea8df8a71..d95d72b96d 100644 --- a/src/engine/strat_engine/thinpool/thinpool.rs +++ b/src/engine/strat_engine/thinpool/thinpool.rs @@ -3157,6 +3157,8 @@ mod tests { devices, MDADataSize::default(), None, + None, + None, ) .unwrap(); let size = ThinPoolSizeParams::new(backstore.datatier_usable_size()).unwrap(); @@ -3259,6 +3261,8 @@ mod tests { first_devices, MDADataSize::default(), None, + None, + None, ) .unwrap(); let mut pool = ThinPool::::new( @@ -3394,6 +3398,8 @@ mod tests { devices, MDADataSize::default(), None, + None, + None, ) .unwrap(); warn!("Available: {}", backstore.available_in_backstore()); @@ -3519,6 +3525,8 @@ mod tests { devices, MDADataSize::default(), None, + None, + None, ) .unwrap(); let mut pool = ThinPool::::new( @@ -3588,6 +3596,8 @@ mod tests { devices, MDADataSize::default(), None, + None, + None, ) .unwrap(); let mut pool = ThinPool::::new( @@ -3667,6 +3677,8 @@ mod tests { devices, MDADataSize::default(), None, + None, + None, ) .unwrap(); let mut pool = ThinPool::::new( @@ -3733,6 +3745,8 @@ mod tests { devices, MDADataSize::default(), None, + None, + None, ) .unwrap(); let mut pool = ThinPool::::new( @@ -3794,6 +3808,8 @@ mod tests { devices, MDADataSize::default(), None, + None, + None, ) .unwrap(); let mut pool = ThinPool::::new( @@ -3900,6 +3916,8 @@ mod tests { devices, MDADataSize::default(), None, + None, + None, ) .unwrap(); let mut pool = ThinPool::::new( @@ -4038,6 +4056,8 @@ mod tests { devices, MDADataSize::default(), None, + None, + None, ) .unwrap(); let mut pool = ThinPool::::new( From 1d033e2a2c202ef57b66d7a3ce273f798bfec0f0 Mon Sep 17 00:00:00 2001 From: John Baublitz Date: Fri, 22 Nov 2024 10:14:50 -0500 Subject: [PATCH 02/11] Add integrity tag and journal size parameters to D-Bus --- src/dbus_api/api/manager_3_0/methods.rs | 5 +- src/dbus_api/api/manager_3_5/methods.rs | 2 + src/dbus_api/api/manager_3_8/api.rs | 49 ++++++- src/dbus_api/api/manager_3_8/methods.rs | 122 +++++++++++++++++- src/dbus_api/api/manager_3_8/mod.rs | 2 +- src/dbus_api/api/mod.rs | 2 +- src/dbus_api/api/shared.rs | 2 + src/engine/engine.rs | 2 + src/engine/sim_engine/engine.rs | 44 +++++-- src/engine/sim_engine/pool.rs | 24 ++++ .../strat_engine/backstore/backstore/v2.rs | 1 - .../strat_engine/backstore/data_tier.rs | 5 +- src/engine/strat_engine/engine.rs | 23 +++- src/engine/strat_engine/pool/v2.rs | 24 ++-- src/jsonrpc/server/pool.rs | 5 +- 15 files changed, 272 insertions(+), 40 deletions(-) diff --git a/src/dbus_api/api/manager_3_0/methods.rs b/src/dbus_api/api/manager_3_0/methods.rs index 46a2d256e7..7f9cda5e7e 100644 --- a/src/dbus_api/api/manager_3_0/methods.rs +++ b/src/dbus_api/api/manager_3_0/methods.rs @@ -13,6 +13,7 @@ use futures::executor::block_on; use crate::{ dbus_api::{ + api::shared::EncryptionParams, blockdev::create_dbus_blockdev, consts, filesystem::create_dbus_filesystem, @@ -28,8 +29,6 @@ use crate::{ stratis::StratisError, }; -type EncryptionParams = (Option<(bool, String)>, Option<(bool, (String, String))>); - pub fn destroy_pool(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { let message: &Message = m.msg; let mut iter = message.iter_init(); @@ -329,6 +328,8 @@ pub fn create_pool(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { name, &devs.map(Path::new).collect::>(), EncryptionInfo::from_options((key_desc, clevis_info)).as_ref(), + None, + None, ))); match create_result { Ok(pool_uuid_action) => match pool_uuid_action { diff --git a/src/dbus_api/api/manager_3_5/methods.rs b/src/dbus_api/api/manager_3_5/methods.rs index 8cd3a05093..e3f12233da 100644 --- a/src/dbus_api/api/manager_3_5/methods.rs +++ b/src/dbus_api/api/manager_3_5/methods.rs @@ -65,6 +65,8 @@ pub fn create_pool(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { name, &devs.map(Path::new).collect::>(), EncryptionInfo::from_options((key_desc, clevis_info)).as_ref(), + None, + None, ))); match create_result { Ok(pool_uuid_action) => match pool_uuid_action { diff --git a/src/dbus_api/api/manager_3_8/api.rs b/src/dbus_api/api/manager_3_8/api.rs index 627d7fd955..37067df11c 100644 --- a/src/dbus_api/api/manager_3_8/api.rs +++ b/src/dbus_api/api/manager_3_8/api.rs @@ -6,7 +6,10 @@ use dbus_tree::{Access, EmitsChangedSignal, Factory, MTSync, Method, Property}; use crate::dbus_api::{ api::{ - manager_3_8::{methods::start_pool, props::get_stopped_pools}, + manager_3_8::{ + methods::{create_pool, start_pool}, + props::get_stopped_pools, + }, prop_conv::StoppedOrLockedPools, }, consts, @@ -31,6 +34,50 @@ pub fn start_pool_method(f: &Factory, TData>) -> Method, TData>) -> Method, TData> { + f.method("CreatePool", (), create_pool) + .in_arg(("name", "s")) + .in_arg(("devices", "as")) + // Optional key description of key in the kernel keyring + // b: true if the pool should be encrypted and able to be + // unlocked with a passphrase associated with this key description. + // s: key description + // + // Rust representation: (bool, String) + .in_arg(("key_desc", "(bs)")) + // Optional Clevis information for binding on initialization. + // b: true if the pool should be encrypted and able to be unlocked + // using Clevis. + // s: pin name + // s: JSON config for Clevis use + // + // Rust representation: (bool, (String, String)) + .in_arg(("clevis_info", "(b(ss))")) + // Optional journal size for integrity metadata reservation. + // b: true if the size should be specified. + // false if the default should be used. + // i: Integer representing journal size in bytes. + // + // Rust representation: (bool, u64) + .in_arg(("journal_size", "(bt)")) + // Optional tag size for integrity metadata reservation. + // b: true if the size should be specified. + // false if the default should be used. + // i: Integer representing tag size in bytes. + // + // Rust representation: (bool, u8) + .in_arg(("tag_size", "(by)")) + // In order from left to right: + // b: true if a pool was created and object paths were returned + // o: Object path for Pool + // a(o): Array of object paths for block devices + // + // Rust representation: (bool, (dbus::Path, Vec)) + .out_arg(("result", "(b(oao))")) + .out_arg(("return_code", "q")) + .out_arg(("return_string", "s")) +} + pub fn stopped_pools_property(f: &Factory, TData>) -> Property, TData> { f.property::(consts::STOPPED_POOLS_PROP, ()) .access(Access::Read) diff --git a/src/dbus_api/api/manager_3_8/methods.rs b/src/dbus_api/api/manager_3_8/methods.rs index 5b7a40d943..8f8bdb3929 100644 --- a/src/dbus_api/api/manager_3_8/methods.rs +++ b/src/dbus_api/api/manager_3_8/methods.rs @@ -2,19 +2,30 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at http://mozilla.org/MPL/2.0/. -use dbus::{arg::OwnedFd, Message, Path}; +use std::path::Path; + +use dbus::{ + arg::{Array, OwnedFd}, + Message, +}; use dbus_tree::{MTSync, MethodInfo, MethodResult}; use futures::executor::block_on; +use devicemapper::Bytes; + use crate::{ dbus_api::{ + api::shared::EncryptionParams, blockdev::create_dbus_blockdev, filesystem::create_dbus_filesystem, pool::create_dbus_pool, types::{DbusErrorEnum, TData, OK_STRING}, util::{engine_to_dbus_err_tuple, get_next_arg, tuple_to_option}, }, - engine::{Name, PoolIdentifier, PoolUuid, StartAction, UnlockMethod}, + engine::{ + CreateAction, EncryptionInfo, KeyDescription, Name, PoolIdentifier, PoolUuid, StartAction, + UnlockMethod, + }, stratis::StratisError, }; @@ -25,8 +36,12 @@ pub fn start_pool(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { let dbus_context = m.tree.get_data(); let default_return: ( bool, - (Path<'static>, Vec>, Vec>), - ) = (false, (Path::default(), Vec::new(), Vec::new())); + ( + dbus::Path<'static>, + Vec>, + Vec>, + ), + ) = (false, (dbus::Path::default(), Vec::new(), Vec::new())); let return_message = message.method_return(); let id_str: &str = get_next_arg(&mut iter, 0)?; @@ -130,3 +145,102 @@ pub fn start_pool(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { OK_STRING.to_string(), )]) } + +pub fn create_pool(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { + let base_path = m.path.get_name(); + let message: &Message = m.msg; + let mut iter = message.iter_init(); + + let name: &str = get_next_arg(&mut iter, 0)?; + let devs: Array<'_, &str, _> = get_next_arg(&mut iter, 1)?; + let (key_desc_tuple, clevis_tuple): EncryptionParams = ( + Some(get_next_arg(&mut iter, 2)?), + Some(get_next_arg(&mut iter, 3)?), + ); + let journal_size_tuple: (bool, u64) = get_next_arg(&mut iter, 4)?; + let tag_size_tuple: (bool, u8) = get_next_arg(&mut iter, 5)?; + + let return_message = message.method_return(); + + let default_return: (bool, (dbus::Path<'static>, Vec>)) = + (false, (dbus::Path::default(), Vec::new())); + + let key_desc = match key_desc_tuple.and_then(tuple_to_option) { + Some(kds) => match KeyDescription::try_from(kds) { + Ok(kd) => Some(kd), + Err(e) => { + let (rc, rs) = engine_to_dbus_err_tuple(&e); + return Ok(vec![return_message.append3(default_return, rc, rs)]); + } + }, + None => None, + }; + + let clevis_info = match clevis_tuple.and_then(tuple_to_option) { + Some((pin, json_string)) => match serde_json::from_str(json_string.as_str()) { + Ok(j) => Some((pin, j)), + Err(e) => { + let (rc, rs) = engine_to_dbus_err_tuple(&StratisError::Serde(e)); + return Ok(vec![return_message.append3(default_return, rc, rs)]); + } + }, + None => None, + }; + + let journal_size = tuple_to_option(journal_size_tuple).map(|i| Bytes::from(i)); + let tag_size = tuple_to_option(tag_size_tuple).map(Bytes::from); + + let dbus_context = m.tree.get_data(); + let create_result = handle_action!(block_on(dbus_context.engine.create_pool( + name, + &devs.map(Path::new).collect::>(), + EncryptionInfo::from_options((key_desc, clevis_info)).as_ref(), + journal_size, + tag_size, + ))); + match create_result { + Ok(pool_uuid_action) => match pool_uuid_action { + CreateAction::Created(uuid) => { + let guard = match block_on(dbus_context.engine.get_pool(PoolIdentifier::Uuid(uuid))) + { + Some(g) => g, + None => { + let (rc, rs) = engine_to_dbus_err_tuple(&StratisError::Msg( + format!("Pool with UUID {uuid} was successfully started but appears to have been removed before it could be exposed on the D-Bus") + )); + return Ok(vec![return_message.append3(default_return, rc, rs)]); + } + }; + + let (pool_name, pool_uuid, pool) = guard.as_tuple(); + let pool_path = + create_dbus_pool(dbus_context, base_path.clone(), &pool_name, pool_uuid, pool); + let mut bd_paths = Vec::new(); + for (bd_uuid, tier, bd) in pool.blockdevs() { + bd_paths.push(create_dbus_blockdev( + dbus_context, + pool_path.clone(), + bd_uuid, + tier, + bd, + )); + } + + Ok(vec![return_message.append3( + (true, (pool_path, bd_paths)), + DbusErrorEnum::OK as u16, + OK_STRING.to_string(), + )]) + } + CreateAction::Identity => Ok(vec![return_message.append3( + default_return, + DbusErrorEnum::OK as u16, + OK_STRING.to_string(), + )]), + }, + Err(x) => { + let (rc, rs) = engine_to_dbus_err_tuple(&x); + Ok(vec![return_message.append3(default_return, rc, rs)]) + } + } +} diff --git a/src/dbus_api/api/manager_3_8/mod.rs b/src/dbus_api/api/manager_3_8/mod.rs index 48fc8b4d99..eb29edbbb5 100644 --- a/src/dbus_api/api/manager_3_8/mod.rs +++ b/src/dbus_api/api/manager_3_8/mod.rs @@ -6,4 +6,4 @@ mod api; mod methods; mod props; -pub use api::{start_pool_method, stopped_pools_property}; +pub use api::{create_pool_method, start_pool_method, stopped_pools_property}; diff --git a/src/dbus_api/api/mod.rs b/src/dbus_api/api/mod.rs index 84533c63bf..3775a53d88 100644 --- a/src/dbus_api/api/mod.rs +++ b/src/dbus_api/api/mod.rs @@ -152,7 +152,7 @@ pub fn get_base_tree<'a>( ) .add( f.interface(consts::MANAGER_INTERFACE_NAME_3_8, ()) - .add_m(manager_3_5::create_pool_method(&f)) + .add_m(manager_3_8::create_pool_method(&f)) .add_m(manager_3_0::set_key_method(&f)) .add_m(manager_3_0::unset_key_method(&f)) .add_m(manager_3_0::list_keys_method(&f)) diff --git a/src/dbus_api/api/shared.rs b/src/dbus_api/api/shared.rs index e2132b6d88..ec21bccc13 100644 --- a/src/dbus_api/api/shared.rs +++ b/src/dbus_api/api/shared.rs @@ -22,6 +22,8 @@ use crate::{ engine::{AllLockReadGuard, DevUuid, Engine, FilesystemUuid, Pool, PoolUuid, StratisUuid}, }; +pub type EncryptionParams = (Option<(bool, String)>, Option<(bool, (String, String))>); + pub fn get_managed_objects_method( f: &Factory, TData>, ) -> Method, TData> { diff --git a/src/engine/engine.rs b/src/engine/engine.rs index dc20fd76fa..ed16f88e6e 100644 --- a/src/engine/engine.rs +++ b/src/engine/engine.rs @@ -381,6 +381,8 @@ pub trait Engine: Debug + Report + Send + Sync { name: &str, blockdev_paths: &[&Path], encryption_info: Option<&EncryptionInfo>, + journal_size: Option, + tag_size: Option, ) -> StratisResult>; /// Handle a libudev event. diff --git a/src/engine/sim_engine/engine.rs b/src/engine/sim_engine/engine.rs index 1761c2c7e9..09c408b044 100644 --- a/src/engine/sim_engine/engine.rs +++ b/src/engine/sim_engine/engine.rs @@ -14,6 +14,8 @@ use futures::executor::block_on; use serde_json::{json, Value}; use tokio::sync::RwLock; +use devicemapper::Bytes; + use crate::{ engine::{ engine::{Engine, HandleEvents, KeyActions, Pool, Report}, @@ -129,6 +131,8 @@ impl Engine for SimEngine { name: &str, blockdev_paths: &[&Path], encryption_info: Option<&EncryptionInfo>, + _: Option, + _: Option, ) -> StratisResult> { validate_name(name)?; let name = Name::new(name.to_owned()); @@ -440,6 +444,8 @@ mod tests { "name", strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, + None, + None )) .unwrap() .changed() @@ -451,10 +457,11 @@ mod tests { /// Destroying a pool with devices should succeed fn destroy_pool_w_devices() { let engine = SimEngine::default(); - let uuid = test_async!(engine.create_pool("name", strs_to_paths!(["/s/d"]), None)) - .unwrap() - .changed() - .unwrap(); + let uuid = + test_async!(engine.create_pool("name", strs_to_paths!(["/s/d"]), None, None, None)) + .unwrap() + .changed() + .unwrap(); assert!(test_async!(engine.destroy_pool(uuid)).is_ok()); } @@ -463,10 +470,11 @@ mod tests { fn destroy_pool_w_filesystem() { let engine = SimEngine::default(); let pool_name = "pool_name"; - let uuid = test_async!(engine.create_pool(pool_name, strs_to_paths!(["/s/d"]), None)) - .unwrap() - .changed() - .unwrap(); + let uuid = + test_async!(engine.create_pool(pool_name, strs_to_paths!(["/s/d"]), None, None, None)) + .unwrap() + .changed() + .unwrap(); { let mut pool = test_async!(engine.get_mut_pool(PoolIdentifier::Uuid(uuid))).unwrap(); pool.create_filesystems(pool_name, uuid, &[("test", None, None)]) @@ -482,9 +490,9 @@ mod tests { let name = "name"; let engine = SimEngine::default(); let devices = strs_to_paths!(["/s/d"]); - test_async!(engine.create_pool(name, devices, None)).unwrap(); + test_async!(engine.create_pool(name, devices, None, None, None)).unwrap(); assert_matches!( - test_async!(engine.create_pool(name, devices, None)), + test_async!(engine.create_pool(name, devices, None, None, None)), Ok(CreateAction::Identity) ); } @@ -494,11 +502,13 @@ mod tests { fn create_pool_name_collision_different_args() { let name = "name"; let engine = SimEngine::default(); - test_async!(engine.create_pool(name, strs_to_paths!(["/s/d"]), None)).unwrap(); + test_async!(engine.create_pool(name, strs_to_paths!(["/s/d"]), None, None, None)).unwrap(); assert!(test_async!(engine.create_pool( name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, + None, + None, )) .is_err()); } @@ -509,7 +519,7 @@ mod tests { let path = "/s/d"; let engine = SimEngine::default(); assert_matches!( - test_async!(engine.create_pool("name", strs_to_paths!([path, path]), None)) + test_async!(engine.create_pool("name", strs_to_paths!([path, path]), None, None, None)) .unwrap() .changed() .map( @@ -541,6 +551,8 @@ mod tests { name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, + None, + None, )) .unwrap() .changed() @@ -559,6 +571,8 @@ mod tests { "old_name", strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, + None, + None, )) .unwrap() .changed() @@ -578,6 +592,8 @@ mod tests { "old_name", strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, + None, + None, )) .unwrap() .changed() @@ -586,6 +602,8 @@ mod tests { new_name, strs_to_paths!(["/dev/four", "/dev/five", "/dev/six"]), None, + None, + None, )) .unwrap(); assert!(test_async!(engine.rename_pool(uuid, new_name)).is_err()); @@ -600,6 +618,8 @@ mod tests { new_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, + None, + None, )) .unwrap(); assert_matches!( diff --git a/src/engine/sim_engine/pool.rs b/src/engine/sim_engine/pool.rs index 3edc16d0f7..b475b5170e 100644 --- a/src/engine/sim_engine/pool.rs +++ b/src/engine/sim_engine/pool.rs @@ -897,6 +897,8 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, + None, + None, )) .unwrap() .changed() @@ -917,6 +919,8 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, + None, + None, )) .unwrap() .changed() @@ -945,6 +949,8 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, + None, + None, )) .unwrap() .changed() @@ -976,6 +982,8 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, + None, + None, )) .unwrap() .changed() @@ -996,6 +1004,8 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, + None, + None, )) .unwrap() .changed() @@ -1016,6 +1026,8 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, + None, + None, )) .unwrap() .changed() @@ -1036,6 +1048,8 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, + None, + None, )) .unwrap() .changed() @@ -1064,6 +1078,8 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, + None, + None, )) .unwrap() .changed() @@ -1082,6 +1098,8 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, + None, + None, )) .unwrap() .changed() @@ -1107,6 +1125,8 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, + None, + None, )) .unwrap() .changed() @@ -1130,6 +1150,8 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, + None, + None, )) .unwrap() .changed() @@ -1157,6 +1179,8 @@ mod tests { "pool_name", strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, + None, + None, )) .unwrap() .changed() diff --git a/src/engine/strat_engine/backstore/backstore/v2.rs b/src/engine/strat_engine/backstore/backstore/v2.rs index 560c3ded60..5be778c441 100644 --- a/src/engine/strat_engine/backstore/backstore/v2.rs +++ b/src/engine/strat_engine/backstore/backstore/v2.rs @@ -444,7 +444,6 @@ impl Backstore { let data_tier = DataTier::::new( BlockDevMgr::::initialize(pool_uuid, devices, mda_data_size)?, integrity_journal_size, - None, integrity_tag_size, ); diff --git a/src/engine/strat_engine/backstore/data_tier.rs b/src/engine/strat_engine/backstore/data_tier.rs index b6d4a44065..4f74f1d573 100644 --- a/src/engine/strat_engine/backstore/data_tier.rs +++ b/src/engine/strat_engine/backstore/data_tier.rs @@ -125,12 +125,11 @@ impl DataTier { pub fn new( mut block_mgr: BlockDevMgr, integrity_journal_size: Option, - integrity_block_size: Option, integrity_tag_size: Option, ) -> DataTier { let integrity_journal_size = integrity_journal_size.unwrap_or_else(|| DEFAULT_INTEGRITY_JOURNAL_SIZE.sectors()); - let integrity_block_size = integrity_block_size.unwrap_or(DEFAULT_INTEGRITY_BLOCK_SIZE); + let integrity_block_size = DEFAULT_INTEGRITY_BLOCK_SIZE; let integrity_tag_size = integrity_tag_size.unwrap_or(DEFAULT_INTEGRITY_TAG_SIZE); for (_, bd) in block_mgr.blockdevs_mut() { // NOTE: over-allocates integrity metadata slightly. Some of the @@ -484,7 +483,7 @@ mod tests { ) .unwrap(); - let mut data_tier = DataTier::::new(mgr, None, None, None); + let mut data_tier = DataTier::::new(mgr, None, None); data_tier.invariant(); // A data_tier w/ some devices but nothing allocated diff --git a/src/engine/strat_engine/engine.rs b/src/engine/strat_engine/engine.rs index ed7d5f40b3..8273ab5b83 100644 --- a/src/engine/strat_engine/engine.rs +++ b/src/engine/strat_engine/engine.rs @@ -17,7 +17,7 @@ use tokio::{ task::{spawn_blocking, JoinHandle}, }; -use devicemapper::DmNameBuf; +use devicemapper::{Bytes, DmNameBuf}; use crate::{ engine::{ @@ -494,9 +494,20 @@ impl Engine for StratEngine { name: &str, blockdev_paths: &[&Path], encryption_info: Option<&EncryptionInfo>, + journal_size: Option, + tag_size: Option, ) -> StratisResult> { validate_name(name)?; let name = Name::new(name.to_owned()); + let rounded_journal_size = match journal_size { + Some(b) => { + if b % 4096u64 != Bytes(0) { + return Err(StratisError::Msg(format!("{b} is not a multiple of 4096"))); + } + Some(b.sectors()) + } + None => None, + }; validate_paths(blockdev_paths)?; @@ -558,6 +569,8 @@ impl Engine for StratEngine { &cloned_name, unowned_devices, cloned_enc_info.as_ref(), + rounded_journal_size, + tag_size, ) })??; pools.insert(Name::new(name.to_string()), pool_uuid, AnyPool::V2(pool)); @@ -911,7 +924,7 @@ mod test { let engine = StratEngine::initialize().unwrap(); let name1 = "name1"; - let uuid1 = test_async!(engine.create_pool(name1, paths, None)) + let uuid1 = test_async!(engine.create_pool(name1, paths, None, None, None)) .unwrap() .changed() .unwrap(); @@ -1034,13 +1047,13 @@ mod test { let engine = StratEngine::initialize().unwrap(); let name1 = "name1"; - let uuid1 = test_async!(engine.create_pool(name1, paths1, None)) + let uuid1 = test_async!(engine.create_pool(name1, paths1, None, None, None)) .unwrap() .changed() .unwrap(); let name2 = "name2"; - let uuid2 = test_async!(engine.create_pool(name2, paths2, None)) + let uuid2 = test_async!(engine.create_pool(name2, paths2, None, None, None)) .unwrap() .changed() .unwrap(); @@ -1500,7 +1513,7 @@ mod test { fn test_start_stop(paths: &[&Path]) { let engine = StratEngine::initialize().unwrap(); let name = "pool_name"; - let uuid = test_async!(engine.create_pool(name, paths, None)) + let uuid = test_async!(engine.create_pool(name, paths, None, None, None)) .unwrap() .changed() .unwrap(); diff --git a/src/engine/strat_engine/pool/v2.rs b/src/engine/strat_engine/pool/v2.rs index 8ec0a5b540..1b57ccffd0 100644 --- a/src/engine/strat_engine/pool/v2.rs +++ b/src/engine/strat_engine/pool/v2.rs @@ -153,6 +153,8 @@ impl StratPool { name: &str, devices: UnownedDevices, encryption_info: Option<&EncryptionInfo>, + journal_size: Option, + tag_size: Option, ) -> StratisResult<(PoolUuid, StratPool)> { let pool_uuid = PoolUuid::new_v4(); @@ -164,8 +166,8 @@ impl StratPool { devices, MDADataSize::default(), encryption_info, - None, - None, + journal_size, + tag_size, )?; let thinpool = ThinPool::::new( @@ -1306,7 +1308,8 @@ mod tests { stratis_devices.error_on_not_empty().unwrap(); let name = "stratis-test-pool"; - let (uuid, mut pool) = StratPool::initialize(name, unowned_devices2, None).unwrap(); + let (uuid, mut pool) = + StratPool::initialize(name, unowned_devices2, None, None, None).unwrap(); invariant(&pool, name); let metadata1 = pool.record(name); @@ -1394,7 +1397,8 @@ mod tests { stratis_devices.error_on_not_empty().unwrap(); let name = "stratis-test-pool"; - let (uuid, mut pool) = StratPool::initialize(name, unowned_devices, None).unwrap(); + let (uuid, mut pool) = + StratPool::initialize(name, unowned_devices, None, None, None).unwrap(); invariant(&pool, name); pool.init_cache(uuid, name, cache_path, true).unwrap(); @@ -1434,7 +1438,8 @@ mod tests { stratis_devices.error_on_not_empty().unwrap(); let name = "stratis-test-pool"; - let (pool_uuid, mut pool) = StratPool::initialize(name, unowned_devices1, None).unwrap(); + let (pool_uuid, mut pool) = + StratPool::initialize(name, unowned_devices1, None, None, None).unwrap(); invariant(&pool, name); let fs_name = "stratis_test_filesystem"; @@ -1518,7 +1523,8 @@ mod tests { let (stratis_devices, unowned_devices) = devices.unpack(); stratis_devices.error_on_not_empty().unwrap(); - let (uuid, mut pool) = StratPool::initialize(name, unowned_devices, None).unwrap(); + let (uuid, mut pool) = + StratPool::initialize(name, unowned_devices, None, None, None).unwrap(); invariant(&pool, name); assert_eq!(pool.action_avail, ActionAvailability::Full); @@ -1534,7 +1540,7 @@ mod tests { let (stratis_devices, unowned_devices) = devices.unpack(); stratis_devices.error_on_not_empty().unwrap(); - let (_, mut pool) = StratPool::initialize(name, unowned_devices, None).unwrap(); + let (_, mut pool) = StratPool::initialize(name, unowned_devices, None, None, None).unwrap(); invariant(&pool, name); assert_eq!(pool.action_avail, ActionAvailability::Full); @@ -1574,7 +1580,7 @@ mod tests { stratis_devices.error_on_not_empty().unwrap(); let (pool_uuid, mut pool) = - StratPool::initialize(pool_name, unowned_devices, None).unwrap(); + StratPool::initialize(pool_name, unowned_devices, None, None, None).unwrap(); let (_, fs_uuid, _) = pool .create_filesystems( @@ -1684,7 +1690,7 @@ mod tests { fn test_grow_physical_pre_grow(paths: &[&Path]) { let pool_name = Name::new("pool".to_string()); let engine = StratEngine::initialize().unwrap(); - let pool_uuid = test_async!(engine.create_pool(&pool_name, paths, None)) + let pool_uuid = test_async!(engine.create_pool(&pool_name, paths, None, None, None)) .unwrap() .changed() .unwrap(); diff --git a/src/jsonrpc/server/pool.rs b/src/jsonrpc/server/pool.rs index 104c9459ae..bbb365b509 100644 --- a/src/jsonrpc/server/pool.rs +++ b/src/jsonrpc/server/pool.rs @@ -45,7 +45,10 @@ pub async fn pool_create<'a>( enc_info: Option<&'a EncryptionInfo>, ) -> StratisResult { Ok( - match engine.create_pool(name, blockdev_paths, enc_info).await? { + match engine + .create_pool(name, blockdev_paths, enc_info, None, None) + .await? + { CreateAction::Created(_) => true, CreateAction::Identity => false, }, From 40e9e59d7c7d5006d6c6ad7bbb4c1d51a2e4b253 Mon Sep 17 00:00:00 2001 From: John Baublitz Date: Mon, 25 Nov 2024 14:43:33 -0500 Subject: [PATCH 03/11] Add tag and journal size parameters to prediction script --- src/bin/utils/cmds.rs | 30 +++++++++++++++++++++++++ src/bin/utils/predict_usage.rs | 14 ++++++++---- src/dbus_api/api/manager_3_8/methods.rs | 2 +- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/bin/utils/cmds.rs b/src/bin/utils/cmds.rs index 1c283013f0..f3dfc9ae4f 100644 --- a/src/bin/utils/cmds.rs +++ b/src/bin/utils/cmds.rs @@ -86,6 +86,20 @@ pool is encrypted, setting this option has no effect on the prediction."), .value_parser(clap::value_parser!(u128)) .help("Size of filesystem to be made for this pool. May be specified multiple times, one for each filesystem. Units are bytes. Must be at least 512 MiB and less than 4 PiB.") .next_line_help(true) + ) + .arg( + Arg::new("integrity_tag_size") + .long("integrity-tag-size") + .num_args(1) + .help("Size of the integrity checksums to be stored in the integrity metadata. The checksum size depends on the algorithm used for checksums. Units are bytes.") + .next_line_help(true) + ) + .arg( + Arg::new("integrity_journal_size") + .long("integrity-journal-size") + .num_args(1) + .help("Size of the integrity journal. Default is 128 MiB. Units are bytes.") + .next_line_help(true) ), Command::new("filesystem") .about("Predicts the space usage when creating a Stratis filesystem.") @@ -126,6 +140,22 @@ impl<'a> UtilCommand<'a> for StratisPredictUsage { sub_m .get_many::("filesystem-size") .map(|szs| szs.map(|n| Bytes(*n)).collect::>()), + sub_m + .get_one::("integrity_journal_size") + .map(|s| s.parse::().map(Bytes::from)) + .transpose()? + .map(|b| { + if b % 4096u64 != Bytes(0) { + Err(format!("Value {b} is not aligned to 4096")) + } else { + Ok(b.sectors()) + } + }) + .transpose()?, + sub_m + .get_one::("integrity_tag_size") + .map(|s| s.parse::().map(Bytes::from)) + .transpose()?, LevelFilter::from_str( matches .get_one::("log-level") diff --git a/src/bin/utils/predict_usage.rs b/src/bin/utils/predict_usage.rs index f9c5f80d81..1c9d602f05 100644 --- a/src/bin/utils/predict_usage.rs +++ b/src/bin/utils/predict_usage.rs @@ -164,7 +164,11 @@ pub fn predict_filesystem_usage( Ok(()) } -fn predict_pool_metadata_usage(device_sizes: Vec) -> Result> { +fn predict_pool_metadata_usage( + device_sizes: Vec, + journal_size: Option, + tag_size: Option, +) -> Result> { let stratis_metadata_alloc = BDA::default().extended_size().sectors(); let stratis_avail_sizes = device_sizes .iter() @@ -173,9 +177,9 @@ fn predict_pool_metadata_usage(device_sizes: Vec) -> Result, filesystem_sizes: Option>, + journal_size: Option, + tag_size: Option, log_level: LevelFilter, ) -> Result<(), Box> { Builder::new().filter(None, log_level).init(); @@ -224,7 +230,7 @@ pub fn predict_pool_usage( let device_sizes = device_sizes.iter().map(|s| s.sectors()).collect::>(); let total_size: Sectors = device_sizes.iter().cloned().sum(); - let non_metadata_size = predict_pool_metadata_usage(device_sizes)?; + let non_metadata_size = predict_pool_metadata_usage(device_sizes, journal_size, tag_size)?; let size_params = ThinPoolSizeParams::new(non_metadata_size)?; let total_non_data = 2usize * size_params.meta_size() + size_params.mdv_size(); diff --git a/src/dbus_api/api/manager_3_8/methods.rs b/src/dbus_api/api/manager_3_8/methods.rs index 8f8bdb3929..de1660d462 100644 --- a/src/dbus_api/api/manager_3_8/methods.rs +++ b/src/dbus_api/api/manager_3_8/methods.rs @@ -187,7 +187,7 @@ pub fn create_pool(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { None => None, }; - let journal_size = tuple_to_option(journal_size_tuple).map(|i| Bytes::from(i)); + let journal_size = tuple_to_option(journal_size_tuple).map(Bytes::from); let tag_size = tuple_to_option(tag_size_tuple).map(Bytes::from); let dbus_context = m.tree.get_data(); From d4f00872bf52c7fd899657b9cde9e27efe4d9b2a Mon Sep 17 00:00:00 2001 From: mulhern Date: Mon, 16 Dec 2024 10:15:34 -0500 Subject: [PATCH 04/11] Return parser error on non-numeric value for integrity journal size Specify default integrity journal size in parser as string value. Signed-off-by: mulhern --- src/bin/utils/cmds.rs | 12 ++++++++---- src/bin/utils/predict_usage.rs | 8 ++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/bin/utils/cmds.rs b/src/bin/utils/cmds.rs index f3dfc9ae4f..663f17ac30 100644 --- a/src/bin/utils/cmds.rs +++ b/src/bin/utils/cmds.rs @@ -16,6 +16,8 @@ use log::LevelFilter; use devicemapper::Bytes; +use stratisd::engine::DEFAULT_INTEGRITY_JOURNAL_SIZE; + use crate::utils::predict_usage; #[cfg(feature = "systemd_compat")] @@ -98,6 +100,8 @@ pool is encrypted, setting this option has no effect on the prediction."), Arg::new("integrity_journal_size") .long("integrity-journal-size") .num_args(1) + .value_parser(clap::value_parser!(u64)) + .default_value(Box::leak((*DEFAULT_INTEGRITY_JOURNAL_SIZE).to_string().into_boxed_str()) as &'static str) .help("Size of the integrity journal. Default is 128 MiB. Units are bytes.") .next_line_help(true) ), @@ -141,9 +145,8 @@ impl<'a> UtilCommand<'a> for StratisPredictUsage { .get_many::("filesystem-size") .map(|szs| szs.map(|n| Bytes(*n)).collect::>()), sub_m - .get_one::("integrity_journal_size") - .map(|s| s.parse::().map(Bytes::from)) - .transpose()? + .get_one::("integrity_journal_size") + .map(|n| Bytes::from(*n)) .map(|b| { if b % 4096u64 != Bytes(0) { Err(format!("Value {b} is not aligned to 4096")) @@ -151,7 +154,8 @@ impl<'a> UtilCommand<'a> for StratisPredictUsage { Ok(b.sectors()) } }) - .transpose()?, + .transpose()? + .expect("default specified by parser"), sub_m .get_one::("integrity_tag_size") .map(|s| s.parse::().map(Bytes::from)) diff --git a/src/bin/utils/predict_usage.rs b/src/bin/utils/predict_usage.rs index 1c9d602f05..6a6c99138f 100644 --- a/src/bin/utils/predict_usage.rs +++ b/src/bin/utils/predict_usage.rs @@ -15,7 +15,7 @@ use devicemapper::{Bytes, Sectors}; use stratisd::engine::{ crypt_metadata_size, integrity_meta_space, ThinPoolSizeParams, BDA, - DEFAULT_INTEGRITY_BLOCK_SIZE, DEFAULT_INTEGRITY_JOURNAL_SIZE, DEFAULT_INTEGRITY_TAG_SIZE, + DEFAULT_INTEGRITY_BLOCK_SIZE, DEFAULT_INTEGRITY_TAG_SIZE, }; // 2^FS_SIZE_START_POWER is the logical size of the smallest Stratis @@ -166,7 +166,7 @@ pub fn predict_filesystem_usage( fn predict_pool_metadata_usage( device_sizes: Vec, - journal_size: Option, + journal_size: Sectors, tag_size: Option, ) -> Result> { let stratis_metadata_alloc = BDA::default().extended_size().sectors(); @@ -177,7 +177,7 @@ fn predict_pool_metadata_usage( let integrity_deduction = integrity_meta_space( s, - journal_size.unwrap_or(DEFAULT_INTEGRITY_JOURNAL_SIZE.sectors()), + journal_size, DEFAULT_INTEGRITY_BLOCK_SIZE, tag_size.unwrap_or(DEFAULT_INTEGRITY_TAG_SIZE), ); @@ -217,7 +217,7 @@ pub fn predict_pool_usage( overprovisioned: bool, device_sizes: Vec, filesystem_sizes: Option>, - journal_size: Option, + journal_size: Sectors, tag_size: Option, log_level: LevelFilter, ) -> Result<(), Box> { From bd5d5ddd03d138c3411bd82a0cba689b9a933882 Mon Sep 17 00:00:00 2001 From: mulhern Date: Sun, 15 Dec 2024 23:47:37 -0500 Subject: [PATCH 05/11] Make D-Bus signature for integrity tag spec a string Define an enum to represent the tag size. Store the IntegrityTagSpec value in the pool-level metadata. Nothing is gained by turning the enum into bits before storing it in the pool-level metadata. Have the string representation in the pool metadata match that for D-Bus API. Compute using the bytes value that is at least as large as the number of bits the tag spec represents. Use tag_spec instead of tag_size for the name of the field in the CreatePool method. Signed-off-by: mulhern --- src/bin/utils/cmds.rs | 49 +++++++++++-------- src/bin/utils/predict_usage.rs | 18 +++---- src/dbus_api/api/manager_3_8/api.rs | 9 ++-- src/dbus_api/api/manager_3_8/methods.rs | 21 ++++++-- src/engine/engine.rs | 13 ++--- src/engine/mod.rs | 6 +-- src/engine/sim_engine/engine.rs | 8 +-- .../strat_engine/backstore/backstore/v2.rs | 13 +++-- .../strat_engine/backstore/blockdev/v2.rs | 15 +++--- .../strat_engine/backstore/blockdevmgr.rs | 6 +-- .../strat_engine/backstore/data_tier.rs | 26 +++++----- src/engine/strat_engine/backstore/mod.rs | 2 +- src/engine/strat_engine/engine.rs | 11 +++-- src/engine/strat_engine/mod.rs | 2 +- src/engine/strat_engine/pool/v2.rs | 12 ++--- src/engine/strat_engine/serde_structs.rs | 4 +- src/engine/types/mod.rs | 42 +++++++++++++++- .../src/stratisd_client_dbus/_introspect.py | 2 + tests/client-dbus/tests/udev/_utils.py | 2 + 19 files changed, 162 insertions(+), 99 deletions(-) diff --git a/src/bin/utils/cmds.rs b/src/bin/utils/cmds.rs index 663f17ac30..47c739cb6b 100644 --- a/src/bin/utils/cmds.rs +++ b/src/bin/utils/cmds.rs @@ -8,15 +8,18 @@ use std::{ str::FromStr, }; -use clap::{Arg, ArgAction, Command}; +use clap::{builder::PossibleValuesParser, Arg, ArgAction, Command}; #[cfg(feature = "systemd_compat")] use clap::builder::Str; use log::LevelFilter; +use strum::VariantNames; use devicemapper::Bytes; -use stratisd::engine::DEFAULT_INTEGRITY_JOURNAL_SIZE; +use stratisd::engine::{ + IntegrityTagSpec, DEFAULT_INTEGRITY_JOURNAL_SIZE, DEFAULT_INTEGRITY_TAG_SPEC, +}; use crate::utils::predict_usage; @@ -88,22 +91,24 @@ pool is encrypted, setting this option has no effect on the prediction."), .value_parser(clap::value_parser!(u128)) .help("Size of filesystem to be made for this pool. May be specified multiple times, one for each filesystem. Units are bytes. Must be at least 512 MiB and less than 4 PiB.") .next_line_help(true) - ) - .arg( - Arg::new("integrity_tag_size") - .long("integrity-tag-size") - .num_args(1) - .help("Size of the integrity checksums to be stored in the integrity metadata. The checksum size depends on the algorithm used for checksums. Units are bytes.") - .next_line_help(true) - ) - .arg( - Arg::new("integrity_journal_size") - .long("integrity-journal-size") - .num_args(1) - .value_parser(clap::value_parser!(u64)) - .default_value(Box::leak((*DEFAULT_INTEGRITY_JOURNAL_SIZE).to_string().into_boxed_str()) as &'static str) - .help("Size of the integrity journal. Default is 128 MiB. Units are bytes.") - .next_line_help(true) + ) + .arg( + Arg::new("integrity_tag_spec") + .long("integrity-tag-spec") + .num_args(1) + .value_parser(PossibleValuesParser::new(IntegrityTagSpec::VARIANTS)) + .default_value(DEFAULT_INTEGRITY_TAG_SPEC.as_ref()) + .help("Integrity tag specification defining the size of the tag to store a checksum or other value for each block on a device.") + .next_line_help(true) + ) + .arg( + Arg::new("integrity_journal_size") + .long("integrity-journal-size") + .num_args(1) + .value_parser(clap::value_parser!(u64)) + .default_value(Box::leak((*DEFAULT_INTEGRITY_JOURNAL_SIZE).to_string().into_boxed_str()) as &'static str) + .help("Size of the integrity journal. Default is 128 MiB. Units are bytes.") + .next_line_help(true) ), Command::new("filesystem") .about("Predicts the space usage when creating a Stratis filesystem.") @@ -157,9 +162,11 @@ impl<'a> UtilCommand<'a> for StratisPredictUsage { .transpose()? .expect("default specified by parser"), sub_m - .get_one::("integrity_tag_size") - .map(|s| s.parse::().map(Bytes::from)) - .transpose()?, + .get_one::("integrity_tag_spec") + .map(|sz| { + IntegrityTagSpec::try_from(sz.as_str()).expect("parser ensures valid value") + }) + .expect("default specified by parser"), LevelFilter::from_str( matches .get_one::("log-level") diff --git a/src/bin/utils/predict_usage.rs b/src/bin/utils/predict_usage.rs index 6a6c99138f..3caa5205b7 100644 --- a/src/bin/utils/predict_usage.rs +++ b/src/bin/utils/predict_usage.rs @@ -14,8 +14,8 @@ use serde_json::{json, Value}; use devicemapper::{Bytes, Sectors}; use stratisd::engine::{ - crypt_metadata_size, integrity_meta_space, ThinPoolSizeParams, BDA, - DEFAULT_INTEGRITY_BLOCK_SIZE, DEFAULT_INTEGRITY_TAG_SIZE, + crypt_metadata_size, integrity_meta_space, IntegrityTagSpec, ThinPoolSizeParams, BDA, + DEFAULT_INTEGRITY_BLOCK_SIZE, }; // 2^FS_SIZE_START_POWER is the logical size of the smallest Stratis @@ -167,7 +167,7 @@ pub fn predict_filesystem_usage( fn predict_pool_metadata_usage( device_sizes: Vec, journal_size: Sectors, - tag_size: Option, + tag_spec: IntegrityTagSpec, ) -> Result> { let stratis_metadata_alloc = BDA::default().extended_size().sectors(); let stratis_avail_sizes = device_sizes @@ -175,12 +175,8 @@ fn predict_pool_metadata_usage( .map(|&s| { info!("Total size of device: {:}", s); - let integrity_deduction = integrity_meta_space( - s, - journal_size, - DEFAULT_INTEGRITY_BLOCK_SIZE, - tag_size.unwrap_or(DEFAULT_INTEGRITY_TAG_SIZE), - ); + let integrity_deduction = + integrity_meta_space(s, journal_size, DEFAULT_INTEGRITY_BLOCK_SIZE, tag_spec); info!( "Deduction for stratis metadata: {:}", stratis_metadata_alloc @@ -218,7 +214,7 @@ pub fn predict_pool_usage( device_sizes: Vec, filesystem_sizes: Option>, journal_size: Sectors, - tag_size: Option, + tag_spec: IntegrityTagSpec, log_level: LevelFilter, ) -> Result<(), Box> { Builder::new().filter(None, log_level).init(); @@ -230,7 +226,7 @@ pub fn predict_pool_usage( let device_sizes = device_sizes.iter().map(|s| s.sectors()).collect::>(); let total_size: Sectors = device_sizes.iter().cloned().sum(); - let non_metadata_size = predict_pool_metadata_usage(device_sizes, journal_size, tag_size)?; + let non_metadata_size = predict_pool_metadata_usage(device_sizes, journal_size, tag_spec)?; let size_params = ThinPoolSizeParams::new(non_metadata_size)?; let total_non_data = 2usize * size_params.meta_size() + size_params.mdv_size(); diff --git a/src/dbus_api/api/manager_3_8/api.rs b/src/dbus_api/api/manager_3_8/api.rs index 37067df11c..9d6a6329b4 100644 --- a/src/dbus_api/api/manager_3_8/api.rs +++ b/src/dbus_api/api/manager_3_8/api.rs @@ -60,13 +60,14 @@ pub fn create_pool_method(f: &Factory, TData>) -> Method, TData>) -> MethodResult { Some(get_next_arg(&mut iter, 3)?), ); let journal_size_tuple: (bool, u64) = get_next_arg(&mut iter, 4)?; - let tag_size_tuple: (bool, u8) = get_next_arg(&mut iter, 5)?; + let tag_spec_tuple: (bool, String) = get_next_arg(&mut iter, 5)?; let return_message = message.method_return(); @@ -188,7 +188,18 @@ pub fn create_pool(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { }; let journal_size = tuple_to_option(journal_size_tuple).map(Bytes::from); - let tag_size = tuple_to_option(tag_size_tuple).map(Bytes::from); + let tag_spec = match tuple_to_option(tag_spec_tuple) + .map(|s| IntegrityTagSpec::try_from(s.as_str())) + .transpose() + { + Ok(s) => s, + Err(e) => { + let (rc, rs) = engine_to_dbus_err_tuple(&StratisError::Msg(format!( + "Failed to parse integrity tag specification: {e}" + ))); + return Ok(vec![return_message.append3(default_return, rc, rs)]); + } + }; let dbus_context = m.tree.get_data(); let create_result = handle_action!(block_on(dbus_context.engine.create_pool( @@ -196,7 +207,7 @@ pub fn create_pool(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { &devs.map(Path::new).collect::>(), EncryptionInfo::from_options((key_desc, clevis_info)).as_ref(), journal_size, - tag_size, + tag_spec, ))); match create_result { Ok(pool_uuid_action) => match pool_uuid_action { diff --git a/src/engine/engine.rs b/src/engine/engine.rs index ed16f88e6e..5b08d62417 100644 --- a/src/engine/engine.rs +++ b/src/engine/engine.rs @@ -21,11 +21,12 @@ use crate::{ structures::{AllLockReadGuard, AllLockWriteGuard, SomeLockReadGuard, SomeLockWriteGuard}, types::{ ActionAvailability, BlockDevTier, Clevis, CreateAction, DeleteAction, DevUuid, - EncryptionInfo, FilesystemUuid, GrowAction, Key, KeyDescription, LockedPoolsInfo, - MappingCreateAction, MappingDeleteAction, Name, PoolDiff, PoolEncryptionInfo, - PoolIdentifier, PoolUuid, RegenAction, RenameAction, ReportType, SetCreateAction, - SetDeleteAction, SetUnlockAction, StartAction, StopAction, StoppedPoolsInfo, - StratFilesystemDiff, StratSigblockVersion, UdevEngineEvent, UnlockMethod, + EncryptionInfo, FilesystemUuid, GrowAction, IntegrityTagSpec, Key, KeyDescription, + LockedPoolsInfo, MappingCreateAction, MappingDeleteAction, Name, PoolDiff, + PoolEncryptionInfo, PoolIdentifier, PoolUuid, RegenAction, RenameAction, ReportType, + SetCreateAction, SetDeleteAction, SetUnlockAction, StartAction, StopAction, + StoppedPoolsInfo, StratFilesystemDiff, StratSigblockVersion, UdevEngineEvent, + UnlockMethod, }, }, stratis::StratisResult, @@ -382,7 +383,7 @@ pub trait Engine: Debug + Report + Send + Sync { blockdev_paths: &[&Path], encryption_info: Option<&EncryptionInfo>, journal_size: Option, - tag_size: Option, + tag_spec: Option, ) -> StratisResult>; /// Handle a libudev event. diff --git a/src/engine/mod.rs b/src/engine/mod.rs index 468fac880b..5281cbe2cb 100644 --- a/src/engine/mod.rs +++ b/src/engine/mod.rs @@ -16,13 +16,13 @@ pub use self::{ crypt_metadata_size, get_dm, get_dm_init, integrity_meta_space, register_clevis_token, set_up_crypt_logging, unshare_mount_namespace, StaticHeader, StaticHeaderResult, StratEngine, StratKeyActions, ThinPoolSizeParams, BDA, CLEVIS_TANG_TRUST_URL, - DEFAULT_INTEGRITY_BLOCK_SIZE, DEFAULT_INTEGRITY_JOURNAL_SIZE, DEFAULT_INTEGRITY_TAG_SIZE, + DEFAULT_INTEGRITY_BLOCK_SIZE, DEFAULT_INTEGRITY_JOURNAL_SIZE, DEFAULT_INTEGRITY_TAG_SPEC, }, structures::{AllLockReadGuard, ExclusiveGuard, SharedGuard, Table}, types::{ ActionAvailability, BlockDevTier, ClevisInfo, CreateAction, DeleteAction, DevUuid, Diff, - EncryptionInfo, EngineAction, FilesystemUuid, GrowAction, KeyDescription, Lockable, - LockedPoolInfo, LockedPoolsInfo, MappingCreateAction, MappingDeleteAction, + EncryptionInfo, EngineAction, FilesystemUuid, GrowAction, IntegrityTagSpec, KeyDescription, + Lockable, LockedPoolInfo, LockedPoolsInfo, MappingCreateAction, MappingDeleteAction, MaybeInconsistent, Name, PoolDiff, PoolEncryptionInfo, PoolIdentifier, PoolUuid, PropChangeAction, RenameAction, ReportType, SetCreateAction, SetDeleteAction, SetUnlockAction, StartAction, StopAction, StoppedPoolInfo, StoppedPoolsInfo, diff --git a/src/engine/sim_engine/engine.rs b/src/engine/sim_engine/engine.rs index 09c408b044..e6c84678ba 100644 --- a/src/engine/sim_engine/engine.rs +++ b/src/engine/sim_engine/engine.rs @@ -27,9 +27,9 @@ use crate::{ }, types::{ CreateAction, DeleteAction, DevUuid, EncryptionInfo, Features, FilesystemUuid, - LockedPoolsInfo, Name, PoolDevice, PoolDiff, PoolIdentifier, PoolUuid, RenameAction, - ReportType, SetUnlockAction, StartAction, StopAction, StoppedPoolInfo, - StoppedPoolsInfo, StratFilesystemDiff, UdevEngineEvent, UnlockMethod, + IntegrityTagSpec, LockedPoolsInfo, Name, PoolDevice, PoolDiff, PoolIdentifier, + PoolUuid, RenameAction, ReportType, SetUnlockAction, StartAction, StopAction, + StoppedPoolInfo, StoppedPoolsInfo, StratFilesystemDiff, UdevEngineEvent, UnlockMethod, }, StratSigblockVersion, }, @@ -132,7 +132,7 @@ impl Engine for SimEngine { blockdev_paths: &[&Path], encryption_info: Option<&EncryptionInfo>, _: Option, - _: Option, + _: Option, ) -> StratisResult> { validate_name(name)?; let name = Name::new(name.to_owned()); diff --git a/src/engine/strat_engine/backstore/backstore/v2.rs b/src/engine/strat_engine/backstore/backstore/v2.rs index 5be778c441..fe87952070 100644 --- a/src/engine/strat_engine/backstore/backstore/v2.rs +++ b/src/engine/strat_engine/backstore/backstore/v2.rs @@ -11,9 +11,8 @@ use either::Either; use serde_json::Value; use devicemapper::{ - Bytes, CacheDev, CacheDevTargetTable, CacheTargetParams, DevId, Device, DmDevice, DmFlags, - DmOptions, LinearDev, LinearDevTargetParams, LinearTargetParams, Sectors, TargetLine, - TargetTable, + CacheDev, CacheDevTargetTable, CacheTargetParams, DevId, Device, DmDevice, DmFlags, DmOptions, + LinearDev, LinearDevTargetParams, LinearTargetParams, Sectors, TargetLine, TargetTable, }; use crate::{ @@ -34,8 +33,8 @@ use crate::{ writing::wipe_sectors, }, types::{ - ActionAvailability, BlockDevTier, DevUuid, EncryptionInfo, KeyDescription, PoolUuid, - SizedKeyMemory, UnlockMethod, + ActionAvailability, BlockDevTier, DevUuid, EncryptionInfo, IntegrityTagSpec, + KeyDescription, PoolUuid, SizedKeyMemory, UnlockMethod, }, }, stratis::{StratisError, StratisResult}, @@ -439,12 +438,12 @@ impl Backstore { mda_data_size: MDADataSize, encryption_info: Option<&EncryptionInfo>, integrity_journal_size: Option, - integrity_tag_size: Option, + integrity_tag_spec: Option, ) -> StratisResult { let data_tier = DataTier::::new( BlockDevMgr::::initialize(pool_uuid, devices, mda_data_size)?, integrity_journal_size, - integrity_tag_size, + integrity_tag_spec, ); let mut backstore = Backstore { diff --git a/src/engine/strat_engine/backstore/blockdev/v2.rs b/src/engine/strat_engine/backstore/blockdev/v2.rs index 06c5a745f3..d21b02863e 100644 --- a/src/engine/strat_engine/backstore/blockdev/v2.rs +++ b/src/engine/strat_engine/backstore/blockdev/v2.rs @@ -34,8 +34,8 @@ use crate::{ types::BDAResult, }, types::{ - Compare, DevUuid, DevicePath, Diff, PoolUuid, StateDiff, StratBlockDevDiff, - StratSigblockVersion, + Compare, DevUuid, DevicePath, Diff, IntegrityTagSpec, PoolUuid, StateDiff, + StratBlockDevDiff, StratSigblockVersion, }, }, stratis::{StratisError, StratisResult}, @@ -51,11 +51,14 @@ pub fn integrity_meta_space( total_space: Sectors, journal_size: Sectors, block_size: Bytes, - tag_size: Bytes, + tag_spec: IntegrityTagSpec, ) -> Sectors { Bytes(4096).sectors() + journal_size - + Bytes::from(((*total_space.bytes() / *block_size) * *tag_size + 4095) & !4095).sectors() + + Bytes::from( + (*((total_space.bytes() / block_size) * tag_spec.as_bytes_ceil()) + 4095) & !4095, + ) + .sectors() } #[derive(Debug)] @@ -221,7 +224,7 @@ impl StratBlockDev { &mut self, integrity_journal_size: Sectors, integrity_block_size: Bytes, - integrity_tag_size: Bytes, + integrity_tag_spec: IntegrityTagSpec, ) -> StratisResult { let size = BlockdevSize::new(Self::scan_blkdev_size(self.devnode())?); let metadata_size = self.bda.dev_size(); @@ -253,7 +256,7 @@ impl StratBlockDev { size.sectors(), integrity_journal_size, integrity_block_size, - integrity_tag_size, + integrity_tag_spec, ) - self .integrity_meta_allocs .iter() diff --git a/src/engine/strat_engine/backstore/blockdevmgr.rs b/src/engine/strat_engine/backstore/blockdevmgr.rs index 6a1807886e..2e1213df7a 100644 --- a/src/engine/strat_engine/backstore/blockdevmgr.rs +++ b/src/engine/strat_engine/backstore/blockdevmgr.rs @@ -29,7 +29,7 @@ use crate::{ serde_structs::{BaseBlockDevSave, Recordable}, shared::bds_to_bdas, }, - types::{DevUuid, EncryptionInfo, Name, PoolEncryptionInfo, PoolUuid}, + types::{DevUuid, EncryptionInfo, IntegrityTagSpec, Name, PoolEncryptionInfo, PoolUuid}, }, stratis::{StratisError, StratisResult}, }; @@ -248,7 +248,7 @@ impl BlockDevMgr { dev: DevUuid, integrity_journal_size: Sectors, integrity_block_size: Bytes, - integrity_tag_size: Bytes, + integrity_tag_spec: IntegrityTagSpec, ) -> StratisResult { let bd = self .block_devs @@ -258,7 +258,7 @@ impl BlockDevMgr { bd.grow( integrity_journal_size, integrity_block_size, - integrity_tag_size, + integrity_tag_spec, ) } diff --git a/src/engine/strat_engine/backstore/data_tier.rs b/src/engine/strat_engine/backstore/data_tier.rs index 4f74f1d573..7a46e0a363 100644 --- a/src/engine/strat_engine/backstore/data_tier.rs +++ b/src/engine/strat_engine/backstore/data_tier.rs @@ -27,14 +27,14 @@ use crate::{ }, types::BDARecordResult, }, - types::{BlockDevTier, DevUuid, Name, PoolUuid}, + types::{BlockDevTier, DevUuid, IntegrityTagSpec, Name, PoolUuid}, }, stratis::StratisResult, }; pub const DEFAULT_INTEGRITY_JOURNAL_SIZE: Bytes = Bytes(128 * IEC::Mi as u128); pub const DEFAULT_INTEGRITY_BLOCK_SIZE: Bytes = Bytes(4 * IEC::Ki as u128); -pub const DEFAULT_INTEGRITY_TAG_SIZE: Bytes = Bytes(64u128); +pub const DEFAULT_INTEGRITY_TAG_SPEC: IntegrityTagSpec = IntegrityTagSpec::B512; /// Handles the lowest level, base layer of this tier. #[derive(Debug)] @@ -47,8 +47,8 @@ pub struct DataTier { integrity_journal_size: Option, /// Integrity block size. integrity_block_size: Option, - /// Integrity tag size. - integrity_tag_size: Option, + /// Integrity tag spec. + integrity_tag_spec: Option, } impl DataTier { @@ -64,7 +64,7 @@ impl DataTier { segments: AllocatedAbove { inner: vec![] }, integrity_journal_size: None, integrity_block_size: None, - integrity_tag_size: None, + integrity_tag_spec: None, } } @@ -125,12 +125,12 @@ impl DataTier { pub fn new( mut block_mgr: BlockDevMgr, integrity_journal_size: Option, - integrity_tag_size: Option, + integrity_tag_spec: Option, ) -> DataTier { let integrity_journal_size = integrity_journal_size.unwrap_or_else(|| DEFAULT_INTEGRITY_JOURNAL_SIZE.sectors()); let integrity_block_size = DEFAULT_INTEGRITY_BLOCK_SIZE; - let integrity_tag_size = integrity_tag_size.unwrap_or(DEFAULT_INTEGRITY_TAG_SIZE); + let integrity_tag_spec = integrity_tag_spec.unwrap_or(DEFAULT_INTEGRITY_TAG_SPEC); for (_, bd) in block_mgr.blockdevs_mut() { // NOTE: over-allocates integrity metadata slightly. Some of the // total size of the device will not make use of the integrity @@ -139,7 +139,7 @@ impl DataTier { bd.total_size().sectors(), integrity_journal_size, integrity_block_size, - integrity_tag_size, + integrity_tag_spec, )); } DataTier { @@ -147,7 +147,7 @@ impl DataTier { segments: AllocatedAbove { inner: vec![] }, integrity_journal_size: Some(integrity_journal_size), integrity_block_size: Some(integrity_block_size), - integrity_tag_size: Some(integrity_tag_size), + integrity_tag_spec: Some(integrity_tag_spec), } } @@ -178,7 +178,7 @@ impl DataTier { bd.total_size().sectors(), self.integrity_journal_size.expect("Must be some in V2"), self.integrity_block_size.expect("Must be some in V2"), - self.integrity_tag_size.expect("Must be some in V2"), + self.integrity_tag_spec.expect("Must be some in V2"), )); } Ok(uuids) @@ -218,7 +218,7 @@ impl DataTier { dev, self.integrity_journal_size.expect("Must be Some in V2"), self.integrity_block_size.expect("Must be Some in V2"), - self.integrity_tag_size.expect("Must be Some in V2"), + self.integrity_tag_spec.expect("Must be Some in V2"), ) } } @@ -251,7 +251,7 @@ where segments, integrity_journal_size: data_tier_save.integrity_journal_size, integrity_block_size: data_tier_save.integrity_block_size, - integrity_tag_size: data_tier_save.integrity_tag_size, + integrity_tag_spec: data_tier_save.integrity_tag_spec, }) } @@ -344,7 +344,7 @@ where }, integrity_journal_size: self.integrity_journal_size, integrity_block_size: self.integrity_block_size, - integrity_tag_size: self.integrity_tag_size, + integrity_tag_spec: self.integrity_tag_spec, } } } diff --git a/src/engine/strat_engine/backstore/mod.rs b/src/engine/strat_engine/backstore/mod.rs index 3311b24253..7b4ab59dea 100644 --- a/src/engine/strat_engine/backstore/mod.rs +++ b/src/engine/strat_engine/backstore/mod.rs @@ -15,7 +15,7 @@ mod shared; pub use self::{ blockdev::v2::integrity_meta_space, data_tier::{ - DEFAULT_INTEGRITY_BLOCK_SIZE, DEFAULT_INTEGRITY_JOURNAL_SIZE, DEFAULT_INTEGRITY_TAG_SIZE, + DEFAULT_INTEGRITY_BLOCK_SIZE, DEFAULT_INTEGRITY_JOURNAL_SIZE, DEFAULT_INTEGRITY_TAG_SPEC, }, devices::{find_stratis_devs_by_uuid, get_devno_from_path, ProcessedPathInfos, UnownedDevices}, }; diff --git a/src/engine/strat_engine/engine.rs b/src/engine/strat_engine/engine.rs index 8273ab5b83..23f19747ff 100644 --- a/src/engine/strat_engine/engine.rs +++ b/src/engine/strat_engine/engine.rs @@ -37,9 +37,10 @@ use crate::{ SomeLockWriteGuard, Table, }, types::{ - CreateAction, DeleteAction, DevUuid, EncryptionInfo, FilesystemUuid, LockedPoolsInfo, - PoolDiff, PoolIdentifier, RenameAction, ReportType, SetUnlockAction, StartAction, - StopAction, StoppedPoolsInfo, StratFilesystemDiff, UdevEngineEvent, UnlockMethod, + CreateAction, DeleteAction, DevUuid, EncryptionInfo, FilesystemUuid, IntegrityTagSpec, + LockedPoolsInfo, PoolDiff, PoolIdentifier, RenameAction, ReportType, SetUnlockAction, + StartAction, StopAction, StoppedPoolsInfo, StratFilesystemDiff, UdevEngineEvent, + UnlockMethod, }, Engine, Name, Pool, PoolUuid, Report, }, @@ -495,7 +496,7 @@ impl Engine for StratEngine { blockdev_paths: &[&Path], encryption_info: Option<&EncryptionInfo>, journal_size: Option, - tag_size: Option, + tag_spec: Option, ) -> StratisResult> { validate_name(name)?; let name = Name::new(name.to_owned()); @@ -570,7 +571,7 @@ impl Engine for StratEngine { unowned_devices, cloned_enc_info.as_ref(), rounded_journal_size, - tag_size, + tag_spec, ) })??; pools.insert(Name::new(name.to_string()), pool_uuid, AnyPool::V2(pool)); diff --git a/src/engine/strat_engine/mod.rs b/src/engine/strat_engine/mod.rs index 1a1c12fd08..8222a54725 100644 --- a/src/engine/strat_engine/mod.rs +++ b/src/engine/strat_engine/mod.rs @@ -31,7 +31,7 @@ pub use self::pool::inspection as pool_inspection; pub use self::{ backstore::{ integrity_meta_space, DEFAULT_INTEGRITY_BLOCK_SIZE, DEFAULT_INTEGRITY_JOURNAL_SIZE, - DEFAULT_INTEGRITY_TAG_SIZE, + DEFAULT_INTEGRITY_TAG_SPEC, }, crypt::{ crypt_metadata_size, register_clevis_token, set_up_crypt_logging, CLEVIS_TANG_TRUST_URL, diff --git a/src/engine/strat_engine/pool/v2.rs b/src/engine/strat_engine/pool/v2.rs index 1b57ccffd0..1034840c98 100644 --- a/src/engine/strat_engine/pool/v2.rs +++ b/src/engine/strat_engine/pool/v2.rs @@ -36,10 +36,10 @@ use crate::{ }, types::{ ActionAvailability, BlockDevTier, Clevis, Compare, CreateAction, DeleteAction, DevUuid, - Diff, EncryptionInfo, FilesystemUuid, GrowAction, Key, KeyDescription, Name, PoolDiff, - PoolEncryptionInfo, PoolUuid, PropChangeAction, RegenAction, RenameAction, - SetCreateAction, SetDeleteAction, SizedKeyMemory, StratFilesystemDiff, StratPoolDiff, - StratSigblockVersion, UnlockMethod, + Diff, EncryptionInfo, FilesystemUuid, GrowAction, IntegrityTagSpec, Key, + KeyDescription, Name, PoolDiff, PoolEncryptionInfo, PoolUuid, PropChangeAction, + RegenAction, RenameAction, SetCreateAction, SetDeleteAction, SizedKeyMemory, + StratFilesystemDiff, StratPoolDiff, StratSigblockVersion, UnlockMethod, }, }, stratis::{StratisError, StratisResult}, @@ -154,7 +154,7 @@ impl StratPool { devices: UnownedDevices, encryption_info: Option<&EncryptionInfo>, journal_size: Option, - tag_size: Option, + tag_spec: Option, ) -> StratisResult<(PoolUuid, StratPool)> { let pool_uuid = PoolUuid::new_v4(); @@ -167,7 +167,7 @@ impl StratPool { MDADataSize::default(), encryption_info, journal_size, - tag_size, + tag_spec, )?; let thinpool = ThinPool::::new( diff --git a/src/engine/strat_engine/serde_structs.rs b/src/engine/strat_engine/serde_structs.rs index bb23225b44..c773029e2e 100644 --- a/src/engine/strat_engine/serde_structs.rs +++ b/src/engine/strat_engine/serde_structs.rs @@ -16,7 +16,7 @@ use serde::{Serialize, Serializer}; use devicemapper::{Bytes, Sectors, ThinDevId}; -use crate::engine::types::{DevUuid, Features, FilesystemUuid}; +use crate::engine::types::{DevUuid, Features, FilesystemUuid, IntegrityTagSpec}; const MAXIMUM_STRING_SIZE: usize = 255; @@ -122,7 +122,7 @@ pub struct DataTierSave { #[serde(skip_serializing_if = "Option::is_none")] pub integrity_block_size: Option, #[serde(skip_serializing_if = "Option::is_none")] - pub integrity_tag_size: Option, + pub integrity_tag_spec: Option, } #[derive(Debug, Deserialize, Eq, PartialEq, Serialize)] diff --git a/src/engine/types/mod.rs b/src/engine/types/mod.rs index 287400c496..f6be492780 100644 --- a/src/engine/types/mod.rs +++ b/src/engine/types/mod.rs @@ -16,9 +16,11 @@ use std::{ use libudev::EventType; use serde::{Deserialize, Serialize}; use serde_json::Value; -use strum_macros::{self, EnumString, FromRepr, VariantNames}; +use strum_macros::{self, AsRefStr, EnumString, FromRepr, VariantNames}; use uuid::Uuid; +use devicemapper::Bytes; + pub use crate::engine::{ engine::StateDiff, structures::Lockable, @@ -481,3 +483,41 @@ pub enum StratSigblockVersion { V1 = 1, V2 = 2, } + +/// A way to specify an integrity tag size. It is possible for the specification +/// to be non-numeric but translatable to some number of bits. +#[derive( + Clone, + Copy, + Debug, + Eq, + PartialEq, + Hash, + Serialize, + Deserialize, + VariantNames, + EnumString, + AsRefStr, +)] +pub enum IntegrityTagSpec { + #[strum(serialize = "0b")] + #[serde(rename = "0b")] + B0, + #[strum(serialize = "32b")] + #[serde(rename = "32b")] + B32, + #[strum(serialize = "512b")] + #[serde(rename = "512b")] + B512, +} + +impl IntegrityTagSpec { + /// The smallest number of bytes containing the bits represented. + pub fn as_bytes_ceil(self) -> Bytes { + match self { + IntegrityTagSpec::B0 => Bytes(0), + IntegrityTagSpec::B32 => Bytes(4), + IntegrityTagSpec::B512 => Bytes(64), + } + } +} diff --git a/tests/client-dbus/src/stratisd_client_dbus/_introspect.py b/tests/client-dbus/src/stratisd_client_dbus/_introspect.py index 0f520d465e..6326c23624 100644 --- a/tests/client-dbus/src/stratisd_client_dbus/_introspect.py +++ b/tests/client-dbus/src/stratisd_client_dbus/_introspect.py @@ -13,6 +13,8 @@ + + diff --git a/tests/client-dbus/tests/udev/_utils.py b/tests/client-dbus/tests/udev/_utils.py index 1fc9aa589e..a5011c118b 100644 --- a/tests/client-dbus/tests/udev/_utils.py +++ b/tests/client-dbus/tests/udev/_utils.py @@ -150,6 +150,8 @@ def create_v2_pool(): "clevis_info": ( (False, ("", "")) if clevis_arg is None else (True, clevis_arg) ), + "journal_size": (False, 0), + "tag_spec": (False, ""), }, ) From 3226cdd05a450a6c3696ac43ca3f126b68c6273f Mon Sep 17 00:00:00 2001 From: mulhern Date: Wed, 18 Dec 2024 21:38:52 -0500 Subject: [PATCH 06/11] Use IntegritySpec struct to manage spec Combine both current specs in the struct. The default of both fields is None. Signed-off-by: mulhern --- src/dbus_api/api/manager_3_0/methods.rs | 5 +- src/dbus_api/api/manager_3_5/methods.rs | 5 +- src/dbus_api/api/manager_3_8/methods.rs | 10 +-- src/engine/engine.rs | 5 +- src/engine/mod.rs | 8 +-- src/engine/sim_engine/engine.rs | 95 ++++++++++++++----------- src/engine/sim_engine/pool.rs | 38 ++++------ src/engine/strat_engine/engine.rs | 17 +++-- src/engine/strat_engine/pool/v2.rs | 11 +-- src/engine/types/mod.rs | 6 ++ src/jsonrpc/server/pool.rs | 4 +- 11 files changed, 103 insertions(+), 101 deletions(-) diff --git a/src/dbus_api/api/manager_3_0/methods.rs b/src/dbus_api/api/manager_3_0/methods.rs index 7f9cda5e7e..5b6ee29ab0 100644 --- a/src/dbus_api/api/manager_3_0/methods.rs +++ b/src/dbus_api/api/manager_3_0/methods.rs @@ -22,7 +22,7 @@ use crate::{ util::{engine_to_dbus_err_tuple, get_next_arg, tuple_to_option}, }, engine::{ - CreateAction, DeleteAction, EncryptionInfo, EngineAction, KeyDescription, + CreateAction, DeleteAction, EncryptionInfo, EngineAction, IntegritySpec, KeyDescription, MappingCreateAction, MappingDeleteAction, PoolIdentifier, PoolUuid, SetUnlockAction, UnlockMethod, }, @@ -328,8 +328,7 @@ pub fn create_pool(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { name, &devs.map(Path::new).collect::>(), EncryptionInfo::from_options((key_desc, clevis_info)).as_ref(), - None, - None, + IntegritySpec::default(), ))); match create_result { Ok(pool_uuid_action) => match pool_uuid_action { diff --git a/src/dbus_api/api/manager_3_5/methods.rs b/src/dbus_api/api/manager_3_5/methods.rs index e3f12233da..573358cb60 100644 --- a/src/dbus_api/api/manager_3_5/methods.rs +++ b/src/dbus_api/api/manager_3_5/methods.rs @@ -15,7 +15,7 @@ use crate::{ types::{DbusErrorEnum, TData, OK_STRING}, util::{engine_to_dbus_err_tuple, get_next_arg, tuple_to_option}, }, - engine::{CreateAction, EncryptionInfo, KeyDescription, PoolIdentifier}, + engine::{CreateAction, EncryptionInfo, IntegritySpec, KeyDescription, PoolIdentifier}, stratis::StratisError, }; @@ -65,8 +65,7 @@ pub fn create_pool(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { name, &devs.map(Path::new).collect::>(), EncryptionInfo::from_options((key_desc, clevis_info)).as_ref(), - None, - None, + IntegritySpec::default(), ))); match create_result { Ok(pool_uuid_action) => match pool_uuid_action { diff --git a/src/dbus_api/api/manager_3_8/methods.rs b/src/dbus_api/api/manager_3_8/methods.rs index 2d3411862d..cece3e0fc8 100644 --- a/src/dbus_api/api/manager_3_8/methods.rs +++ b/src/dbus_api/api/manager_3_8/methods.rs @@ -23,8 +23,8 @@ use crate::{ util::{engine_to_dbus_err_tuple, get_next_arg, tuple_to_option}, }, engine::{ - CreateAction, EncryptionInfo, IntegrityTagSpec, KeyDescription, Name, PoolIdentifier, - PoolUuid, StartAction, UnlockMethod, + CreateAction, EncryptionInfo, IntegritySpec, IntegrityTagSpec, KeyDescription, Name, + PoolIdentifier, PoolUuid, StartAction, UnlockMethod, }, stratis::StratisError, }; @@ -206,8 +206,10 @@ pub fn create_pool(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { name, &devs.map(Path::new).collect::>(), EncryptionInfo::from_options((key_desc, clevis_info)).as_ref(), - journal_size, - tag_spec, + IntegritySpec { + journal_size, + tag_spec + }, ))); match create_result { Ok(pool_uuid_action) => match pool_uuid_action { diff --git a/src/engine/engine.rs b/src/engine/engine.rs index 5b08d62417..1b192fedc0 100644 --- a/src/engine/engine.rs +++ b/src/engine/engine.rs @@ -21,7 +21,7 @@ use crate::{ structures::{AllLockReadGuard, AllLockWriteGuard, SomeLockReadGuard, SomeLockWriteGuard}, types::{ ActionAvailability, BlockDevTier, Clevis, CreateAction, DeleteAction, DevUuid, - EncryptionInfo, FilesystemUuid, GrowAction, IntegrityTagSpec, Key, KeyDescription, + EncryptionInfo, FilesystemUuid, GrowAction, IntegritySpec, Key, KeyDescription, LockedPoolsInfo, MappingCreateAction, MappingDeleteAction, Name, PoolDiff, PoolEncryptionInfo, PoolIdentifier, PoolUuid, RegenAction, RenameAction, ReportType, SetCreateAction, SetDeleteAction, SetUnlockAction, StartAction, StopAction, @@ -382,8 +382,7 @@ pub trait Engine: Debug + Report + Send + Sync { name: &str, blockdev_paths: &[&Path], encryption_info: Option<&EncryptionInfo>, - journal_size: Option, - tag_spec: Option, + integrity_spec: IntegritySpec, ) -> StratisResult>; /// Handle a libudev event. diff --git a/src/engine/mod.rs b/src/engine/mod.rs index 5281cbe2cb..e5399799d2 100644 --- a/src/engine/mod.rs +++ b/src/engine/mod.rs @@ -21,10 +21,10 @@ pub use self::{ structures::{AllLockReadGuard, ExclusiveGuard, SharedGuard, Table}, types::{ ActionAvailability, BlockDevTier, ClevisInfo, CreateAction, DeleteAction, DevUuid, Diff, - EncryptionInfo, EngineAction, FilesystemUuid, GrowAction, IntegrityTagSpec, KeyDescription, - Lockable, LockedPoolInfo, LockedPoolsInfo, MappingCreateAction, MappingDeleteAction, - MaybeInconsistent, Name, PoolDiff, PoolEncryptionInfo, PoolIdentifier, PoolUuid, - PropChangeAction, RenameAction, ReportType, SetCreateAction, SetDeleteAction, + EncryptionInfo, EngineAction, FilesystemUuid, GrowAction, IntegritySpec, IntegrityTagSpec, + KeyDescription, Lockable, LockedPoolInfo, LockedPoolsInfo, MappingCreateAction, + MappingDeleteAction, MaybeInconsistent, Name, PoolDiff, PoolEncryptionInfo, PoolIdentifier, + PoolUuid, PropChangeAction, RenameAction, ReportType, SetCreateAction, SetDeleteAction, SetUnlockAction, StartAction, StopAction, StoppedPoolInfo, StoppedPoolsInfo, StratBlockDevDiff, StratFilesystemDiff, StratPoolDiff, StratSigblockVersion, StratisUuid, ThinPoolDiff, ToDisplay, UdevEngineEvent, UnlockMethod, diff --git a/src/engine/sim_engine/engine.rs b/src/engine/sim_engine/engine.rs index e6c84678ba..802e4f47d7 100644 --- a/src/engine/sim_engine/engine.rs +++ b/src/engine/sim_engine/engine.rs @@ -14,8 +14,6 @@ use futures::executor::block_on; use serde_json::{json, Value}; use tokio::sync::RwLock; -use devicemapper::Bytes; - use crate::{ engine::{ engine::{Engine, HandleEvents, KeyActions, Pool, Report}, @@ -27,9 +25,9 @@ use crate::{ }, types::{ CreateAction, DeleteAction, DevUuid, EncryptionInfo, Features, FilesystemUuid, - IntegrityTagSpec, LockedPoolsInfo, Name, PoolDevice, PoolDiff, PoolIdentifier, - PoolUuid, RenameAction, ReportType, SetUnlockAction, StartAction, StopAction, - StoppedPoolInfo, StoppedPoolsInfo, StratFilesystemDiff, UdevEngineEvent, UnlockMethod, + IntegritySpec, LockedPoolsInfo, Name, PoolDevice, PoolDiff, PoolIdentifier, PoolUuid, + RenameAction, ReportType, SetUnlockAction, StartAction, StopAction, StoppedPoolInfo, + StoppedPoolsInfo, StratFilesystemDiff, UdevEngineEvent, UnlockMethod, }, StratSigblockVersion, }, @@ -131,8 +129,7 @@ impl Engine for SimEngine { name: &str, blockdev_paths: &[&Path], encryption_info: Option<&EncryptionInfo>, - _: Option, - _: Option, + _: IntegritySpec, ) -> StratisResult> { validate_name(name)?; let name = Name::new(name.to_owned()); @@ -444,8 +441,7 @@ mod tests { "name", strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, - None, - None + IntegritySpec::default(), )) .unwrap() .changed() @@ -457,11 +453,15 @@ mod tests { /// Destroying a pool with devices should succeed fn destroy_pool_w_devices() { let engine = SimEngine::default(); - let uuid = - test_async!(engine.create_pool("name", strs_to_paths!(["/s/d"]), None, None, None)) - .unwrap() - .changed() - .unwrap(); + let uuid = test_async!(engine.create_pool( + "name", + strs_to_paths!(["/s/d"]), + None, + IntegritySpec::default() + )) + .unwrap() + .changed() + .unwrap(); assert!(test_async!(engine.destroy_pool(uuid)).is_ok()); } @@ -470,11 +470,15 @@ mod tests { fn destroy_pool_w_filesystem() { let engine = SimEngine::default(); let pool_name = "pool_name"; - let uuid = - test_async!(engine.create_pool(pool_name, strs_to_paths!(["/s/d"]), None, None, None)) - .unwrap() - .changed() - .unwrap(); + let uuid = test_async!(engine.create_pool( + pool_name, + strs_to_paths!(["/s/d"]), + None, + IntegritySpec::default() + )) + .unwrap() + .changed() + .unwrap(); { let mut pool = test_async!(engine.get_mut_pool(PoolIdentifier::Uuid(uuid))).unwrap(); pool.create_filesystems(pool_name, uuid, &[("test", None, None)]) @@ -490,9 +494,9 @@ mod tests { let name = "name"; let engine = SimEngine::default(); let devices = strs_to_paths!(["/s/d"]); - test_async!(engine.create_pool(name, devices, None, None, None)).unwrap(); + test_async!(engine.create_pool(name, devices, None, IntegritySpec::default())).unwrap(); assert_matches!( - test_async!(engine.create_pool(name, devices, None, None, None)), + test_async!(engine.create_pool(name, devices, None, IntegritySpec::default())), Ok(CreateAction::Identity) ); } @@ -502,13 +506,18 @@ mod tests { fn create_pool_name_collision_different_args() { let name = "name"; let engine = SimEngine::default(); - test_async!(engine.create_pool(name, strs_to_paths!(["/s/d"]), None, None, None)).unwrap(); + test_async!(engine.create_pool( + name, + strs_to_paths!(["/s/d"]), + None, + IntegritySpec::default() + )) + .unwrap(); assert!(test_async!(engine.create_pool( name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, - None, - None, + IntegritySpec::default(), )) .is_err()); } @@ -519,15 +528,20 @@ mod tests { let path = "/s/d"; let engine = SimEngine::default(); assert_matches!( - test_async!(engine.create_pool("name", strs_to_paths!([path, path]), None, None, None)) - .unwrap() - .changed() - .map( - |uuid| test_async!(engine.get_pool(PoolIdentifier::Uuid(uuid))) - .unwrap() - .blockdevs() - .len() - ), + test_async!(engine.create_pool( + "name", + strs_to_paths!([path, path]), + None, + IntegritySpec::default() + )) + .unwrap() + .changed() + .map( + |uuid| test_async!(engine.get_pool(PoolIdentifier::Uuid(uuid))) + .unwrap() + .blockdevs() + .len() + ), Some(1) ); } @@ -551,8 +565,7 @@ mod tests { name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, - None, - None, + IntegritySpec::default(), )) .unwrap() .changed() @@ -571,8 +584,7 @@ mod tests { "old_name", strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, - None, - None, + IntegritySpec::default(), )) .unwrap() .changed() @@ -592,8 +604,7 @@ mod tests { "old_name", strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, - None, - None, + IntegritySpec::default(), )) .unwrap() .changed() @@ -602,8 +613,7 @@ mod tests { new_name, strs_to_paths!(["/dev/four", "/dev/five", "/dev/six"]), None, - None, - None, + IntegritySpec::default(), )) .unwrap(); assert!(test_async!(engine.rename_pool(uuid, new_name)).is_err()); @@ -618,8 +628,7 @@ mod tests { new_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, - None, - None, + IntegritySpec::default(), )) .unwrap(); assert_matches!( diff --git a/src/engine/sim_engine/pool.rs b/src/engine/sim_engine/pool.rs index b475b5170e..1f7ec712e8 100644 --- a/src/engine/sim_engine/pool.rs +++ b/src/engine/sim_engine/pool.rs @@ -882,7 +882,7 @@ mod tests { use crate::engine::{ sim_engine::SimEngine, - types::{EngineAction, PoolIdentifier}, + types::{EngineAction, IntegritySpec, PoolIdentifier}, Engine, }; @@ -897,8 +897,7 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, - None, - None, + IntegritySpec::default(), )) .unwrap() .changed() @@ -919,8 +918,7 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, - None, - None, + IntegritySpec::default(), )) .unwrap() .changed() @@ -949,8 +947,7 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, - None, - None, + IntegritySpec::default(), )) .unwrap() .changed() @@ -982,8 +979,7 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, - None, - None, + IntegritySpec::default(), )) .unwrap() .changed() @@ -1004,8 +1000,7 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, - None, - None, + IntegritySpec::default(), )) .unwrap() .changed() @@ -1026,8 +1021,7 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, - None, - None, + IntegritySpec::default(), )) .unwrap() .changed() @@ -1048,8 +1042,7 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, - None, - None, + IntegritySpec::default(), )) .unwrap() .changed() @@ -1078,8 +1071,7 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, - None, - None, + IntegritySpec::default(), )) .unwrap() .changed() @@ -1098,8 +1090,7 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, - None, - None, + IntegritySpec::default(), )) .unwrap() .changed() @@ -1125,8 +1116,7 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, - None, - None, + IntegritySpec::default(), )) .unwrap() .changed() @@ -1150,8 +1140,7 @@ mod tests { pool_name, strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, - None, - None, + IntegritySpec::default(), )) .unwrap() .changed() @@ -1179,8 +1168,7 @@ mod tests { "pool_name", strs_to_paths!(["/dev/one", "/dev/two", "/dev/three"]), None, - None, - None, + IntegritySpec::default(), )) .unwrap() .changed() diff --git a/src/engine/strat_engine/engine.rs b/src/engine/strat_engine/engine.rs index 23f19747ff..284e20c74f 100644 --- a/src/engine/strat_engine/engine.rs +++ b/src/engine/strat_engine/engine.rs @@ -37,7 +37,7 @@ use crate::{ SomeLockWriteGuard, Table, }, types::{ - CreateAction, DeleteAction, DevUuid, EncryptionInfo, FilesystemUuid, IntegrityTagSpec, + CreateAction, DeleteAction, DevUuid, EncryptionInfo, FilesystemUuid, IntegritySpec, LockedPoolsInfo, PoolDiff, PoolIdentifier, RenameAction, ReportType, SetUnlockAction, StartAction, StopAction, StoppedPoolsInfo, StratFilesystemDiff, UdevEngineEvent, UnlockMethod, @@ -495,12 +495,11 @@ impl Engine for StratEngine { name: &str, blockdev_paths: &[&Path], encryption_info: Option<&EncryptionInfo>, - journal_size: Option, - tag_spec: Option, + integrity_spec: IntegritySpec, ) -> StratisResult> { validate_name(name)?; let name = Name::new(name.to_owned()); - let rounded_journal_size = match journal_size { + let rounded_journal_size = match integrity_spec.journal_size { Some(b) => { if b % 4096u64 != Bytes(0) { return Err(StratisError::Msg(format!("{b} is not a multiple of 4096"))); @@ -571,7 +570,7 @@ impl Engine for StratEngine { unowned_devices, cloned_enc_info.as_ref(), rounded_journal_size, - tag_spec, + integrity_spec.tag_spec, ) })??; pools.insert(Name::new(name.to_string()), pool_uuid, AnyPool::V2(pool)); @@ -925,7 +924,7 @@ mod test { let engine = StratEngine::initialize().unwrap(); let name1 = "name1"; - let uuid1 = test_async!(engine.create_pool(name1, paths, None, None, None)) + let uuid1 = test_async!(engine.create_pool(name1, paths, None, IntegritySpec::default())) .unwrap() .changed() .unwrap(); @@ -1048,13 +1047,13 @@ mod test { let engine = StratEngine::initialize().unwrap(); let name1 = "name1"; - let uuid1 = test_async!(engine.create_pool(name1, paths1, None, None, None)) + let uuid1 = test_async!(engine.create_pool(name1, paths1, None, IntegritySpec::default())) .unwrap() .changed() .unwrap(); let name2 = "name2"; - let uuid2 = test_async!(engine.create_pool(name2, paths2, None, None, None)) + let uuid2 = test_async!(engine.create_pool(name2, paths2, None, IntegritySpec::default())) .unwrap() .changed() .unwrap(); @@ -1514,7 +1513,7 @@ mod test { fn test_start_stop(paths: &[&Path]) { let engine = StratEngine::initialize().unwrap(); let name = "pool_name"; - let uuid = test_async!(engine.create_pool(name, paths, None, None, None)) + let uuid = test_async!(engine.create_pool(name, paths, None, IntegritySpec::default())) .unwrap() .changed() .unwrap(); diff --git a/src/engine/strat_engine/pool/v2.rs b/src/engine/strat_engine/pool/v2.rs index 1034840c98..82ecf5e65c 100644 --- a/src/engine/strat_engine/pool/v2.rs +++ b/src/engine/strat_engine/pool/v2.rs @@ -1265,7 +1265,7 @@ mod tests { tests::{loopbacked, real}, thinpool::ThinPoolStatusDigest, }, - types::{EngineAction, PoolIdentifier}, + types::{EngineAction, IntegritySpec, PoolIdentifier}, Engine, StratEngine, }; @@ -1690,10 +1690,11 @@ mod tests { fn test_grow_physical_pre_grow(paths: &[&Path]) { let pool_name = Name::new("pool".to_string()); let engine = StratEngine::initialize().unwrap(); - let pool_uuid = test_async!(engine.create_pool(&pool_name, paths, None, None, None)) - .unwrap() - .changed() - .unwrap(); + let pool_uuid = + test_async!(engine.create_pool(&pool_name, paths, None, IntegritySpec::default())) + .unwrap() + .changed() + .unwrap(); let mut guard = test_async!(engine.get_mut_pool(PoolIdentifier::Uuid(pool_uuid))).unwrap(); let (_, _, pool) = guard.as_mut_tuple(); diff --git a/src/engine/types/mod.rs b/src/engine/types/mod.rs index f6be492780..d166a6dc5e 100644 --- a/src/engine/types/mod.rs +++ b/src/engine/types/mod.rs @@ -521,3 +521,9 @@ impl IntegrityTagSpec { } } } + +#[derive(Default)] +pub struct IntegritySpec { + pub tag_spec: Option, + pub journal_size: Option, +} diff --git a/src/jsonrpc/server/pool.rs b/src/jsonrpc/server/pool.rs index bbb365b509..3b1d584591 100644 --- a/src/jsonrpc/server/pool.rs +++ b/src/jsonrpc/server/pool.rs @@ -10,7 +10,7 @@ use tokio::task::block_in_place; use crate::{ engine::{ BlockDevTier, CreateAction, DeleteAction, EncryptionInfo, Engine, EngineAction, - KeyDescription, Name, PoolIdentifier, PoolUuid, RenameAction, UnlockMethod, + IntegritySpec, KeyDescription, Name, PoolIdentifier, PoolUuid, RenameAction, UnlockMethod, }, jsonrpc::interface::PoolListType, stratis::{StratisError, StratisResult}, @@ -46,7 +46,7 @@ pub async fn pool_create<'a>( ) -> StratisResult { Ok( match engine - .create_pool(name, blockdev_paths, enc_info, None, None) + .create_pool(name, blockdev_paths, enc_info, IntegritySpec::default()) .await? { CreateAction::Created(_) => true, From 891df659a556eb4e64a2cae8c762344f23d3f25a Mon Sep 17 00:00:00 2001 From: mulhern Date: Thu, 19 Dec 2024 10:31:09 -0500 Subject: [PATCH 07/11] Add a ValidatedIntegritySpec type Use ValidatedIntegritySpec for serialization and to pass to methods invoked by engine's create_pool method. Signed-off-by: mulhern --- src/bin/utils/cmds.rs | 38 +++++------ src/bin/utils/predict_usage.rs | 14 ++-- src/engine/mod.rs | 4 +- src/engine/sim_engine/engine.rs | 7 +- src/engine/sim_engine/pool.rs | 12 +++- .../strat_engine/backstore/backstore/v2.rs | 39 +++++------ .../strat_engine/backstore/blockdev/v2.rs | 38 +++++------ .../strat_engine/backstore/blockdevmgr.rs | 14 ++-- .../strat_engine/backstore/data_tier.rs | 60 +++++------------ src/engine/strat_engine/backstore/mod.rs | 3 - src/engine/strat_engine/engine.rs | 17 ++--- src/engine/strat_engine/mod.rs | 5 +- src/engine/strat_engine/pool/v2.rs | 67 +++++++++++++------ src/engine/strat_engine/serde_structs.rs | 10 +-- src/engine/strat_engine/thinpool/thinpool.rs | 31 +++------ src/engine/types/mod.rs | 44 +++++++++++- 16 files changed, 206 insertions(+), 197 deletions(-) diff --git a/src/bin/utils/cmds.rs b/src/bin/utils/cmds.rs index 47c739cb6b..57826cc623 100644 --- a/src/bin/utils/cmds.rs +++ b/src/bin/utils/cmds.rs @@ -18,7 +18,8 @@ use strum::VariantNames; use devicemapper::Bytes; use stratisd::engine::{ - IntegrityTagSpec, DEFAULT_INTEGRITY_JOURNAL_SIZE, DEFAULT_INTEGRITY_TAG_SPEC, + IntegritySpec, IntegrityTagSpec, ValidatedIntegritySpec, DEFAULT_INTEGRITY_JOURNAL_SIZE, + DEFAULT_INTEGRITY_TAG_SPEC, }; use crate::utils::predict_usage; @@ -149,24 +150,23 @@ impl<'a> UtilCommand<'a> for StratisPredictUsage { sub_m .get_many::("filesystem-size") .map(|szs| szs.map(|n| Bytes(*n)).collect::>()), - sub_m - .get_one::("integrity_journal_size") - .map(|n| Bytes::from(*n)) - .map(|b| { - if b % 4096u64 != Bytes(0) { - Err(format!("Value {b} is not aligned to 4096")) - } else { - Ok(b.sectors()) - } - }) - .transpose()? - .expect("default specified by parser"), - sub_m - .get_one::("integrity_tag_spec") - .map(|sz| { - IntegrityTagSpec::try_from(sz.as_str()).expect("parser ensures valid value") - }) - .expect("default specified by parser"), + ValidatedIntegritySpec::try_from(IntegritySpec { + journal_size: Some( + sub_m + .get_one::("integrity_journal_size") + .map(|n| Bytes::from(*n)) + .expect("default specified by parser"), + ), + tag_spec: Some( + sub_m + .get_one::("integrity_tag_spec") + .map(|sz| { + IntegrityTagSpec::try_from(sz.as_str()) + .expect("parser ensures valid value") + }) + .expect("default specified by parser"), + ), + })?, LevelFilter::from_str( matches .get_one::("log-level") diff --git a/src/bin/utils/predict_usage.rs b/src/bin/utils/predict_usage.rs index 3caa5205b7..268452757b 100644 --- a/src/bin/utils/predict_usage.rs +++ b/src/bin/utils/predict_usage.rs @@ -14,8 +14,7 @@ use serde_json::{json, Value}; use devicemapper::{Bytes, Sectors}; use stratisd::engine::{ - crypt_metadata_size, integrity_meta_space, IntegrityTagSpec, ThinPoolSizeParams, BDA, - DEFAULT_INTEGRITY_BLOCK_SIZE, + crypt_metadata_size, integrity_meta_space, ThinPoolSizeParams, ValidatedIntegritySpec, BDA, }; // 2^FS_SIZE_START_POWER is the logical size of the smallest Stratis @@ -166,8 +165,7 @@ pub fn predict_filesystem_usage( fn predict_pool_metadata_usage( device_sizes: Vec, - journal_size: Sectors, - tag_spec: IntegrityTagSpec, + integrity_spec: ValidatedIntegritySpec, ) -> Result> { let stratis_metadata_alloc = BDA::default().extended_size().sectors(); let stratis_avail_sizes = device_sizes @@ -175,8 +173,7 @@ fn predict_pool_metadata_usage( .map(|&s| { info!("Total size of device: {:}", s); - let integrity_deduction = - integrity_meta_space(s, journal_size, DEFAULT_INTEGRITY_BLOCK_SIZE, tag_spec); + let integrity_deduction = integrity_meta_space(s, integrity_spec); info!( "Deduction for stratis metadata: {:}", stratis_metadata_alloc @@ -213,8 +210,7 @@ pub fn predict_pool_usage( overprovisioned: bool, device_sizes: Vec, filesystem_sizes: Option>, - journal_size: Sectors, - tag_spec: IntegrityTagSpec, + integrity_spec: ValidatedIntegritySpec, log_level: LevelFilter, ) -> Result<(), Box> { Builder::new().filter(None, log_level).init(); @@ -226,7 +222,7 @@ pub fn predict_pool_usage( let device_sizes = device_sizes.iter().map(|s| s.sectors()).collect::>(); let total_size: Sectors = device_sizes.iter().cloned().sum(); - let non_metadata_size = predict_pool_metadata_usage(device_sizes, journal_size, tag_spec)?; + let non_metadata_size = predict_pool_metadata_usage(device_sizes, integrity_spec)?; let size_params = ThinPoolSizeParams::new(non_metadata_size)?; let total_non_data = 2usize * size_params.meta_size() + size_params.mdv_size(); diff --git a/src/engine/mod.rs b/src/engine/mod.rs index e5399799d2..df0a86c2fc 100644 --- a/src/engine/mod.rs +++ b/src/engine/mod.rs @@ -16,7 +16,6 @@ pub use self::{ crypt_metadata_size, get_dm, get_dm_init, integrity_meta_space, register_clevis_token, set_up_crypt_logging, unshare_mount_namespace, StaticHeader, StaticHeaderResult, StratEngine, StratKeyActions, ThinPoolSizeParams, BDA, CLEVIS_TANG_TRUST_URL, - DEFAULT_INTEGRITY_BLOCK_SIZE, DEFAULT_INTEGRITY_JOURNAL_SIZE, DEFAULT_INTEGRITY_TAG_SPEC, }, structures::{AllLockReadGuard, ExclusiveGuard, SharedGuard, Table}, types::{ @@ -27,7 +26,8 @@ pub use self::{ PoolUuid, PropChangeAction, RenameAction, ReportType, SetCreateAction, SetDeleteAction, SetUnlockAction, StartAction, StopAction, StoppedPoolInfo, StoppedPoolsInfo, StratBlockDevDiff, StratFilesystemDiff, StratPoolDiff, StratSigblockVersion, StratisUuid, - ThinPoolDiff, ToDisplay, UdevEngineEvent, UnlockMethod, + ThinPoolDiff, ToDisplay, UdevEngineEvent, UnlockMethod, ValidatedIntegritySpec, + DEFAULT_INTEGRITY_JOURNAL_SIZE, DEFAULT_INTEGRITY_TAG_SPEC, }, }; diff --git a/src/engine/sim_engine/engine.rs b/src/engine/sim_engine/engine.rs index 802e4f47d7..c2cc44c595 100644 --- a/src/engine/sim_engine/engine.rs +++ b/src/engine/sim_engine/engine.rs @@ -28,6 +28,7 @@ use crate::{ IntegritySpec, LockedPoolsInfo, Name, PoolDevice, PoolDiff, PoolIdentifier, PoolUuid, RenameAction, ReportType, SetUnlockAction, StartAction, StopAction, StoppedPoolInfo, StoppedPoolsInfo, StratFilesystemDiff, UdevEngineEvent, UnlockMethod, + ValidatedIntegritySpec, }, StratSigblockVersion, }, @@ -129,13 +130,15 @@ impl Engine for SimEngine { name: &str, blockdev_paths: &[&Path], encryption_info: Option<&EncryptionInfo>, - _: IntegritySpec, + integrity_spec: IntegritySpec, ) -> StratisResult> { validate_name(name)?; let name = Name::new(name.to_owned()); validate_paths(blockdev_paths)?; + let integrity_spec = ValidatedIntegritySpec::try_from(integrity_spec)?; + if let Some(key_desc) = encryption_info.and_then(|ei| ei.key_description()) { if !self.key_handler.contains_key(key_desc) { return Err(StratisError::Msg(format!( @@ -157,7 +160,7 @@ impl Engine for SimEngine { let device_set: HashSet<_, RandomState> = HashSet::from_iter(blockdev_paths); let devices = device_set.into_iter().cloned().collect::>(); - let (pool_uuid, pool) = SimPool::new(&devices, encryption_info); + let (pool_uuid, pool) = SimPool::new(&devices, encryption_info, integrity_spec); self.pools.modify_all().await.insert( Name::new(name.to_owned()), diff --git a/src/engine/sim_engine/pool.rs b/src/engine/sim_engine/pool.rs index 1f7ec712e8..41ff7860a4 100644 --- a/src/engine/sim_engine/pool.rs +++ b/src/engine/sim_engine/pool.rs @@ -26,7 +26,7 @@ use crate::{ ActionAvailability, BlockDevTier, Clevis, CreateAction, DeleteAction, DevUuid, EncryptionInfo, FilesystemUuid, GrowAction, Key, KeyDescription, Name, PoolDiff, PoolEncryptionInfo, PoolUuid, RegenAction, RenameAction, SetCreateAction, - SetDeleteAction, StratSigblockVersion, + SetDeleteAction, StratSigblockVersion, ValidatedIntegritySpec, }, PropChangeAction, }, @@ -41,6 +41,7 @@ pub struct SimPool { fs_limit: u64, enable_overprov: bool, encryption_info: Option, + integrity_spec: ValidatedIntegritySpec, } #[derive(Debug, Eq, PartialEq, Serialize)] @@ -48,10 +49,15 @@ pub struct PoolSave { name: String, fs_limit: Option, enable_overprov: Option, + integrity_spec: Option, } impl SimPool { - pub fn new(paths: &[&Path], enc_info: Option<&EncryptionInfo>) -> (PoolUuid, SimPool) { + pub fn new( + paths: &[&Path], + enc_info: Option<&EncryptionInfo>, + integrity_spec: ValidatedIntegritySpec, + ) -> (PoolUuid, SimPool) { let devices: HashSet<_, RandomState> = HashSet::from_iter(paths); let device_pairs = devices.iter().map(|p| SimDev::new(p)); ( @@ -63,6 +69,7 @@ impl SimPool { fs_limit: 10, enable_overprov: true, encryption_info: enc_info.cloned(), + integrity_spec, }, ) } @@ -130,6 +137,7 @@ impl SimPool { name: name.to_owned(), enable_overprov: Some(self.enable_overprov), fs_limit: Some(self.fs_limit), + integrity_spec: Some(self.integrity_spec), } } } diff --git a/src/engine/strat_engine/backstore/backstore/v2.rs b/src/engine/strat_engine/backstore/backstore/v2.rs index fe87952070..ed10235457 100644 --- a/src/engine/strat_engine/backstore/backstore/v2.rs +++ b/src/engine/strat_engine/backstore/backstore/v2.rs @@ -33,8 +33,8 @@ use crate::{ writing::wipe_sectors, }, types::{ - ActionAvailability, BlockDevTier, DevUuid, EncryptionInfo, IntegrityTagSpec, - KeyDescription, PoolUuid, SizedKeyMemory, UnlockMethod, + ActionAvailability, BlockDevTier, DevUuid, EncryptionInfo, KeyDescription, PoolUuid, + SizedKeyMemory, UnlockMethod, ValidatedIntegritySpec, }, }, stratis::{StratisError, StratisResult}, @@ -437,13 +437,11 @@ impl Backstore { devices: UnownedDevices, mda_data_size: MDADataSize, encryption_info: Option<&EncryptionInfo>, - integrity_journal_size: Option, - integrity_tag_spec: Option, + integrity_spec: ValidatedIntegritySpec, ) -> StratisResult { let data_tier = DataTier::::new( BlockDevMgr::::initialize(pool_uuid, devices, mda_data_size)?, - integrity_journal_size, - integrity_tag_spec, + integrity_spec, ); let mut backstore = Backstore { @@ -1172,13 +1170,16 @@ mod tests { use devicemapper::{CacheDevStatus, DataBlocks, DmOptions, IEC}; - use crate::engine::strat_engine::{ - backstore::devices::{ProcessedPathInfos, UnownedDevices}, - cmd, - crypt::crypt_metadata_size, - metadata::device_identifiers, - ns::{unshare_mount_namespace, MemoryFilesystem}, - tests::{crypt, loopbacked, real}, + use crate::engine::{ + strat_engine::{ + backstore::devices::{ProcessedPathInfos, UnownedDevices}, + cmd, + crypt::crypt_metadata_size, + metadata::device_identifiers, + ns::{unshare_mount_namespace, MemoryFilesystem}, + tests::{crypt, loopbacked, real}, + }, + types::ValidatedIntegritySpec, }; use super::*; @@ -1255,8 +1256,7 @@ mod tests { initdatadevs, MDADataSize::default(), None, - None, - None, + ValidatedIntegritySpec::default(), ) .unwrap(); @@ -1354,8 +1354,7 @@ mod tests { devices1, MDADataSize::default(), None, - None, - None, + ValidatedIntegritySpec::default(), ) .unwrap(); @@ -1420,8 +1419,7 @@ mod tests { "tang".to_string(), json!({"url": env::var("TANG_URL").expect("TANG_URL env var required"), "stratis:tang:trust_url": true}), ))), - None, - None, + ValidatedIntegritySpec::default(), ) .unwrap(); backstore.alloc(pool_uuid, &[Sectors(512)]).unwrap(); @@ -1496,8 +1494,7 @@ mod tests { json!({"url": env::var("TANG_URL").expect("TANG_URL env var required"), "stratis:tang:trust_url": true}), ), )), - None, - None, + ValidatedIntegritySpec::default(), ).unwrap(); cmd::udev_settle().unwrap(); diff --git a/src/engine/strat_engine/backstore/blockdev/v2.rs b/src/engine/strat_engine/backstore/blockdev/v2.rs index d21b02863e..7c5555cb62 100644 --- a/src/engine/strat_engine/backstore/blockdev/v2.rs +++ b/src/engine/strat_engine/backstore/blockdev/v2.rs @@ -34,8 +34,8 @@ use crate::{ types::BDAResult, }, types::{ - Compare, DevUuid, DevicePath, Diff, IntegrityTagSpec, PoolUuid, StateDiff, - StratBlockDevDiff, StratSigblockVersion, + Compare, DevUuid, DevicePath, Diff, PoolUuid, StateDiff, StratBlockDevDiff, + StratSigblockVersion, ValidatedIntegritySpec, }, }, stratis::{StratisError, StratisResult}, @@ -49,14 +49,15 @@ use crate::{ /// The result is divisible by 8 sectors. pub fn integrity_meta_space( total_space: Sectors, - journal_size: Sectors, - block_size: Bytes, - tag_spec: IntegrityTagSpec, + integrity_spec: ValidatedIntegritySpec, ) -> Sectors { Bytes(4096).sectors() - + journal_size + + integrity_spec.journal_size + Bytes::from( - (*((total_space.bytes() / block_size) * tag_spec.as_bytes_ceil()) + 4095) & !4095, + (*((total_space.bytes() / integrity_spec.block_size) + * integrity_spec.tag_spec.as_bytes_ceil()) + + 4095) + & !4095, ) .sectors() } @@ -220,12 +221,7 @@ impl StratBlockDev { /// /// This will also extend integrity metadata reservations according to the new /// size of the device. - pub fn grow( - &mut self, - integrity_journal_size: Sectors, - integrity_block_size: Bytes, - integrity_tag_spec: IntegrityTagSpec, - ) -> StratisResult { + pub fn grow(&mut self, integrity_spec: ValidatedIntegritySpec) -> StratisResult { let size = BlockdevSize::new(Self::scan_blkdev_size(self.devnode())?); let metadata_size = self.bda.dev_size(); match size.cmp(&metadata_size) { @@ -252,16 +248,12 @@ impl StratBlockDev { self.bda.header = h; self.used.increase_size(size.sectors()); - let integrity_grow = integrity_meta_space( - size.sectors(), - integrity_journal_size, - integrity_block_size, - integrity_tag_spec, - ) - self - .integrity_meta_allocs - .iter() - .map(|(_, len)| *len) - .sum::(); + let integrity_grow = integrity_meta_space(size.sectors(), integrity_spec) + - self + .integrity_meta_allocs + .iter() + .map(|(_, len)| *len) + .sum::(); self.alloc_int_meta_back(integrity_grow); Ok(true) diff --git a/src/engine/strat_engine/backstore/blockdevmgr.rs b/src/engine/strat_engine/backstore/blockdevmgr.rs index 2e1213df7a..136371397b 100644 --- a/src/engine/strat_engine/backstore/blockdevmgr.rs +++ b/src/engine/strat_engine/backstore/blockdevmgr.rs @@ -29,7 +29,9 @@ use crate::{ serde_structs::{BaseBlockDevSave, Recordable}, shared::bds_to_bdas, }, - types::{DevUuid, EncryptionInfo, IntegrityTagSpec, Name, PoolEncryptionInfo, PoolUuid}, + types::{ + DevUuid, EncryptionInfo, Name, PoolEncryptionInfo, PoolUuid, ValidatedIntegritySpec, + }, }, stratis::{StratisError, StratisResult}, }; @@ -246,20 +248,14 @@ impl BlockDevMgr { pub fn grow( &mut self, dev: DevUuid, - integrity_journal_size: Sectors, - integrity_block_size: Bytes, - integrity_tag_spec: IntegrityTagSpec, + integrity_spec: ValidatedIntegritySpec, ) -> StratisResult { let bd = self .block_devs .iter_mut() .find(|bd| bd.uuid() == dev) .ok_or_else(|| StratisError::Msg(format!("Block device with UUID {dev} not found")))?; - bd.grow( - integrity_journal_size, - integrity_block_size, - integrity_tag_spec, - ) + bd.grow(integrity_spec) } #[cfg(test)] diff --git a/src/engine/strat_engine/backstore/data_tier.rs b/src/engine/strat_engine/backstore/data_tier.rs index 7a46e0a363..cb24d3e65d 100644 --- a/src/engine/strat_engine/backstore/data_tier.rs +++ b/src/engine/strat_engine/backstore/data_tier.rs @@ -7,7 +7,7 @@ #[cfg(test)] use std::collections::HashSet; -use devicemapper::{Bytes, Sectors, IEC}; +use devicemapper::Sectors; use crate::{ engine::{ @@ -27,15 +27,11 @@ use crate::{ }, types::BDARecordResult, }, - types::{BlockDevTier, DevUuid, IntegrityTagSpec, Name, PoolUuid}, + types::{BlockDevTier, DevUuid, Name, PoolUuid, ValidatedIntegritySpec}, }, stratis::StratisResult, }; -pub const DEFAULT_INTEGRITY_JOURNAL_SIZE: Bytes = Bytes(128 * IEC::Mi as u128); -pub const DEFAULT_INTEGRITY_BLOCK_SIZE: Bytes = Bytes(4 * IEC::Ki as u128); -pub const DEFAULT_INTEGRITY_TAG_SPEC: IntegrityTagSpec = IntegrityTagSpec::B512; - /// Handles the lowest level, base layer of this tier. #[derive(Debug)] pub struct DataTier { @@ -43,12 +39,8 @@ pub struct DataTier { pub(super) block_mgr: BlockDevMgr, /// The list of segments granted by block_mgr and used by dm_device pub(super) segments: AllocatedAbove, - /// Integrity journal size. - integrity_journal_size: Option, - /// Integrity block size. - integrity_block_size: Option, - /// Integrity tag spec. - integrity_tag_spec: Option, + /// Integrity spec + integrity_spec: Option, } impl DataTier { @@ -62,9 +54,7 @@ impl DataTier { DataTier { block_mgr, segments: AllocatedAbove { inner: vec![] }, - integrity_journal_size: None, - integrity_block_size: None, - integrity_tag_spec: None, + integrity_spec: None, } } @@ -124,30 +114,21 @@ impl DataTier { /// WARNING: metadata changing event pub fn new( mut block_mgr: BlockDevMgr, - integrity_journal_size: Option, - integrity_tag_spec: Option, + integrity_spec: ValidatedIntegritySpec, ) -> DataTier { - let integrity_journal_size = - integrity_journal_size.unwrap_or_else(|| DEFAULT_INTEGRITY_JOURNAL_SIZE.sectors()); - let integrity_block_size = DEFAULT_INTEGRITY_BLOCK_SIZE; - let integrity_tag_spec = integrity_tag_spec.unwrap_or(DEFAULT_INTEGRITY_TAG_SPEC); for (_, bd) in block_mgr.blockdevs_mut() { // NOTE: over-allocates integrity metadata slightly. Some of the // total size of the device will not make use of the integrity // metadata. bd.alloc_int_meta_back(integrity_meta_space( bd.total_size().sectors(), - integrity_journal_size, - integrity_block_size, - integrity_tag_spec, + integrity_spec, )); } DataTier { block_mgr, segments: AllocatedAbove { inner: vec![] }, - integrity_journal_size: Some(integrity_journal_size), - integrity_block_size: Some(integrity_block_size), - integrity_tag_spec: Some(integrity_tag_spec), + integrity_spec: Some(integrity_spec), } } @@ -176,9 +157,7 @@ impl DataTier { for bd in bds { bd.alloc_int_meta_back(integrity_meta_space( bd.total_size().sectors(), - self.integrity_journal_size.expect("Must be some in V2"), - self.integrity_block_size.expect("Must be some in V2"), - self.integrity_tag_spec.expect("Must be some in V2"), + self.integrity_spec.expect("Must be some in V2"), )); } Ok(uuids) @@ -214,12 +193,8 @@ impl DataTier { } pub fn grow(&mut self, dev: DevUuid) -> StratisResult { - self.block_mgr.grow( - dev, - self.integrity_journal_size.expect("Must be Some in V2"), - self.integrity_block_size.expect("Must be Some in V2"), - self.integrity_tag_spec.expect("Must be Some in V2"), - ) + self.block_mgr + .grow(dev, self.integrity_spec.expect("Must be Some in V2")) } } @@ -249,9 +224,7 @@ where Ok(DataTier { block_mgr, segments, - integrity_journal_size: data_tier_save.integrity_journal_size, - integrity_block_size: data_tier_save.integrity_block_size, - integrity_tag_spec: data_tier_save.integrity_tag_spec, + integrity_spec: data_tier_save.integrity_spec, }) } @@ -342,9 +315,7 @@ where allocs: vec![self.segments.record()], devs: self.block_mgr.record(), }, - integrity_journal_size: self.integrity_journal_size, - integrity_block_size: self.integrity_block_size, - integrity_tag_spec: self.integrity_tag_spec, + integrity_spec: self.integrity_spec, } } } @@ -483,7 +454,10 @@ mod tests { ) .unwrap(); - let mut data_tier = DataTier::::new(mgr, None, None); + let mut data_tier = DataTier::::new( + mgr, + ValidatedIntegritySpec::default(), + ); data_tier.invariant(); // A data_tier w/ some devices but nothing allocated diff --git a/src/engine/strat_engine/backstore/mod.rs b/src/engine/strat_engine/backstore/mod.rs index 7b4ab59dea..58add8867b 100644 --- a/src/engine/strat_engine/backstore/mod.rs +++ b/src/engine/strat_engine/backstore/mod.rs @@ -14,9 +14,6 @@ mod shared; pub use self::{ blockdev::v2::integrity_meta_space, - data_tier::{ - DEFAULT_INTEGRITY_BLOCK_SIZE, DEFAULT_INTEGRITY_JOURNAL_SIZE, DEFAULT_INTEGRITY_TAG_SPEC, - }, devices::{find_stratis_devs_by_uuid, get_devno_from_path, ProcessedPathInfos, UnownedDevices}, }; diff --git a/src/engine/strat_engine/engine.rs b/src/engine/strat_engine/engine.rs index 284e20c74f..1de9e4184f 100644 --- a/src/engine/strat_engine/engine.rs +++ b/src/engine/strat_engine/engine.rs @@ -17,7 +17,7 @@ use tokio::{ task::{spawn_blocking, JoinHandle}, }; -use devicemapper::{Bytes, DmNameBuf}; +use devicemapper::DmNameBuf; use crate::{ engine::{ @@ -40,7 +40,7 @@ use crate::{ CreateAction, DeleteAction, DevUuid, EncryptionInfo, FilesystemUuid, IntegritySpec, LockedPoolsInfo, PoolDiff, PoolIdentifier, RenameAction, ReportType, SetUnlockAction, StartAction, StopAction, StoppedPoolsInfo, StratFilesystemDiff, UdevEngineEvent, - UnlockMethod, + UnlockMethod, ValidatedIntegritySpec, }, Engine, Name, Pool, PoolUuid, Report, }, @@ -499,15 +499,7 @@ impl Engine for StratEngine { ) -> StratisResult> { validate_name(name)?; let name = Name::new(name.to_owned()); - let rounded_journal_size = match integrity_spec.journal_size { - Some(b) => { - if b % 4096u64 != Bytes(0) { - return Err(StratisError::Msg(format!("{b} is not a multiple of 4096"))); - } - Some(b.sectors()) - } - None => None, - }; + let integrity_spec = ValidatedIntegritySpec::try_from(integrity_spec)?; validate_paths(blockdev_paths)?; @@ -569,8 +561,7 @@ impl Engine for StratEngine { &cloned_name, unowned_devices, cloned_enc_info.as_ref(), - rounded_journal_size, - integrity_spec.tag_spec, + integrity_spec, ) })??; pools.insert(Name::new(name.to_string()), pool_uuid, AnyPool::V2(pool)); diff --git a/src/engine/strat_engine/mod.rs b/src/engine/strat_engine/mod.rs index 8222a54725..37fd872abf 100644 --- a/src/engine/strat_engine/mod.rs +++ b/src/engine/strat_engine/mod.rs @@ -29,10 +29,7 @@ pub use self::{backstore::ProcessedPathInfos, pool::v1::StratPool}; pub use self::pool::inspection as pool_inspection; pub use self::{ - backstore::{ - integrity_meta_space, DEFAULT_INTEGRITY_BLOCK_SIZE, DEFAULT_INTEGRITY_JOURNAL_SIZE, - DEFAULT_INTEGRITY_TAG_SPEC, - }, + backstore::integrity_meta_space, crypt::{ crypt_metadata_size, register_clevis_token, set_up_crypt_logging, CLEVIS_TANG_TRUST_URL, }, diff --git a/src/engine/strat_engine/pool/v2.rs b/src/engine/strat_engine/pool/v2.rs index 82ecf5e65c..6c49010375 100644 --- a/src/engine/strat_engine/pool/v2.rs +++ b/src/engine/strat_engine/pool/v2.rs @@ -36,10 +36,10 @@ use crate::{ }, types::{ ActionAvailability, BlockDevTier, Clevis, Compare, CreateAction, DeleteAction, DevUuid, - Diff, EncryptionInfo, FilesystemUuid, GrowAction, IntegrityTagSpec, Key, - KeyDescription, Name, PoolDiff, PoolEncryptionInfo, PoolUuid, PropChangeAction, - RegenAction, RenameAction, SetCreateAction, SetDeleteAction, SizedKeyMemory, - StratFilesystemDiff, StratPoolDiff, StratSigblockVersion, UnlockMethod, + Diff, EncryptionInfo, FilesystemUuid, GrowAction, Key, KeyDescription, Name, PoolDiff, + PoolEncryptionInfo, PoolUuid, PropChangeAction, RegenAction, RenameAction, + SetCreateAction, SetDeleteAction, SizedKeyMemory, StratFilesystemDiff, StratPoolDiff, + StratSigblockVersion, UnlockMethod, ValidatedIntegritySpec, }, }, stratis::{StratisError, StratisResult}, @@ -153,8 +153,7 @@ impl StratPool { name: &str, devices: UnownedDevices, encryption_info: Option<&EncryptionInfo>, - journal_size: Option, - tag_spec: Option, + integrity_spec: ValidatedIntegritySpec, ) -> StratisResult<(PoolUuid, StratPool)> { let pool_uuid = PoolUuid::new_v4(); @@ -166,8 +165,7 @@ impl StratPool { devices, MDADataSize::default(), encryption_info, - journal_size, - tag_spec, + integrity_spec, )?; let thinpool = ThinPool::::new( @@ -1308,8 +1306,13 @@ mod tests { stratis_devices.error_on_not_empty().unwrap(); let name = "stratis-test-pool"; - let (uuid, mut pool) = - StratPool::initialize(name, unowned_devices2, None, None, None).unwrap(); + let (uuid, mut pool) = StratPool::initialize( + name, + unowned_devices2, + None, + ValidatedIntegritySpec::default(), + ) + .unwrap(); invariant(&pool, name); let metadata1 = pool.record(name); @@ -1397,8 +1400,13 @@ mod tests { stratis_devices.error_on_not_empty().unwrap(); let name = "stratis-test-pool"; - let (uuid, mut pool) = - StratPool::initialize(name, unowned_devices, None, None, None).unwrap(); + let (uuid, mut pool) = StratPool::initialize( + name, + unowned_devices, + None, + ValidatedIntegritySpec::default(), + ) + .unwrap(); invariant(&pool, name); pool.init_cache(uuid, name, cache_path, true).unwrap(); @@ -1438,8 +1446,13 @@ mod tests { stratis_devices.error_on_not_empty().unwrap(); let name = "stratis-test-pool"; - let (pool_uuid, mut pool) = - StratPool::initialize(name, unowned_devices1, None, None, None).unwrap(); + let (pool_uuid, mut pool) = StratPool::initialize( + name, + unowned_devices1, + None, + ValidatedIntegritySpec::default(), + ) + .unwrap(); invariant(&pool, name); let fs_name = "stratis_test_filesystem"; @@ -1523,8 +1536,13 @@ mod tests { let (stratis_devices, unowned_devices) = devices.unpack(); stratis_devices.error_on_not_empty().unwrap(); - let (uuid, mut pool) = - StratPool::initialize(name, unowned_devices, None, None, None).unwrap(); + let (uuid, mut pool) = StratPool::initialize( + name, + unowned_devices, + None, + ValidatedIntegritySpec::default(), + ) + .unwrap(); invariant(&pool, name); assert_eq!(pool.action_avail, ActionAvailability::Full); @@ -1540,7 +1558,13 @@ mod tests { let (stratis_devices, unowned_devices) = devices.unpack(); stratis_devices.error_on_not_empty().unwrap(); - let (_, mut pool) = StratPool::initialize(name, unowned_devices, None, None, None).unwrap(); + let (_, mut pool) = StratPool::initialize( + name, + unowned_devices, + None, + ValidatedIntegritySpec::default(), + ) + .unwrap(); invariant(&pool, name); assert_eq!(pool.action_avail, ActionAvailability::Full); @@ -1579,8 +1603,13 @@ mod tests { let (stratis_devices, unowned_devices) = devices.unpack(); stratis_devices.error_on_not_empty().unwrap(); - let (pool_uuid, mut pool) = - StratPool::initialize(pool_name, unowned_devices, None, None, None).unwrap(); + let (pool_uuid, mut pool) = StratPool::initialize( + pool_name, + unowned_devices, + None, + ValidatedIntegritySpec::default(), + ) + .unwrap(); let (_, fs_uuid, _) = pool .create_filesystems( diff --git a/src/engine/strat_engine/serde_structs.rs b/src/engine/strat_engine/serde_structs.rs index c773029e2e..478bbd0727 100644 --- a/src/engine/strat_engine/serde_structs.rs +++ b/src/engine/strat_engine/serde_structs.rs @@ -14,9 +14,9 @@ use serde::{Serialize, Serializer}; -use devicemapper::{Bytes, Sectors, ThinDevId}; +use devicemapper::{Sectors, ThinDevId}; -use crate::engine::types::{DevUuid, Features, FilesystemUuid, IntegrityTagSpec}; +use crate::engine::types::{DevUuid, Features, FilesystemUuid, ValidatedIntegritySpec}; const MAXIMUM_STRING_SIZE: usize = 255; @@ -118,11 +118,7 @@ pub struct BackstoreSave { pub struct DataTierSave { pub blockdev: BlockDevSave, #[serde(skip_serializing_if = "Option::is_none")] - pub integrity_journal_size: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub integrity_block_size: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub integrity_tag_spec: Option, + pub integrity_spec: Option, } #[derive(Debug, Deserialize, Eq, PartialEq, Serialize)] diff --git a/src/engine/strat_engine/thinpool/thinpool.rs b/src/engine/strat_engine/thinpool/thinpool.rs index d95d72b96d..b7f2356986 100644 --- a/src/engine/strat_engine/thinpool/thinpool.rs +++ b/src/engine/strat_engine/thinpool/thinpool.rs @@ -2112,6 +2112,7 @@ mod tests { tests::{loopbacked, real}, writing::SyncAll, }, + types::ValidatedIntegritySpec, }; use super::*; @@ -3157,8 +3158,7 @@ mod tests { devices, MDADataSize::default(), None, - None, - None, + ValidatedIntegritySpec::default(), ) .unwrap(); let size = ThinPoolSizeParams::new(backstore.datatier_usable_size()).unwrap(); @@ -3261,8 +3261,7 @@ mod tests { first_devices, MDADataSize::default(), None, - None, - None, + ValidatedIntegritySpec::default(), ) .unwrap(); let mut pool = ThinPool::::new( @@ -3398,8 +3397,7 @@ mod tests { devices, MDADataSize::default(), None, - None, - None, + ValidatedIntegritySpec::default(), ) .unwrap(); warn!("Available: {}", backstore.available_in_backstore()); @@ -3525,8 +3523,7 @@ mod tests { devices, MDADataSize::default(), None, - None, - None, + ValidatedIntegritySpec::default(), ) .unwrap(); let mut pool = ThinPool::::new( @@ -3596,8 +3593,7 @@ mod tests { devices, MDADataSize::default(), None, - None, - None, + ValidatedIntegritySpec::default(), ) .unwrap(); let mut pool = ThinPool::::new( @@ -3677,8 +3673,7 @@ mod tests { devices, MDADataSize::default(), None, - None, - None, + ValidatedIntegritySpec::default(), ) .unwrap(); let mut pool = ThinPool::::new( @@ -3745,8 +3740,7 @@ mod tests { devices, MDADataSize::default(), None, - None, - None, + ValidatedIntegritySpec::default(), ) .unwrap(); let mut pool = ThinPool::::new( @@ -3808,8 +3802,7 @@ mod tests { devices, MDADataSize::default(), None, - None, - None, + ValidatedIntegritySpec::default(), ) .unwrap(); let mut pool = ThinPool::::new( @@ -3916,8 +3909,7 @@ mod tests { devices, MDADataSize::default(), None, - None, - None, + ValidatedIntegritySpec::default(), ) .unwrap(); let mut pool = ThinPool::::new( @@ -4056,8 +4048,7 @@ mod tests { devices, MDADataSize::default(), None, - None, - None, + ValidatedIntegritySpec::default(), ) .unwrap(); let mut pool = ThinPool::::new( diff --git a/src/engine/types/mod.rs b/src/engine/types/mod.rs index d166a6dc5e..ebc8b1636c 100644 --- a/src/engine/types/mod.rs +++ b/src/engine/types/mod.rs @@ -19,7 +19,7 @@ use serde_json::Value; use strum_macros::{self, AsRefStr, EnumString, FromRepr, VariantNames}; use uuid::Uuid; -use devicemapper::Bytes; +use devicemapper::{Bytes, Sectors, IEC}; pub use crate::engine::{ engine::StateDiff, @@ -39,6 +39,10 @@ pub use crate::engine::{ }; use crate::stratis::{StratisError, StratisResult}; +pub const DEFAULT_INTEGRITY_JOURNAL_SIZE: Bytes = Bytes(128 * IEC::Mi as u128); +pub const DEFAULT_INTEGRITY_BLOCK_SIZE: Bytes = Bytes(4 * IEC::Ki as u128); +pub const DEFAULT_INTEGRITY_TAG_SPEC: IntegrityTagSpec = IntegrityTagSpec::B512; + mod actions; mod diff; mod keys; @@ -527,3 +531,41 @@ pub struct IntegritySpec { pub tag_spec: Option, pub journal_size: Option, } + +#[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)] +pub struct ValidatedIntegritySpec { + pub tag_spec: IntegrityTagSpec, + pub journal_size: Sectors, + pub block_size: Bytes, +} + +impl Default for ValidatedIntegritySpec { + fn default() -> Self { + ValidatedIntegritySpec::try_from(IntegritySpec::default()).expect("default is valid") + } +} + +impl TryFrom for ValidatedIntegritySpec { + type Error = StratisError; + + fn try_from(spec: IntegritySpec) -> StratisResult { + let journal_size = match spec.journal_size { + Some(journal_size) => { + if journal_size % 4096u64 != Bytes(0) { + return Err(StratisError::Msg(format!( + "specified integrity journal size {journal_size} is not a multiple of 4096" + ))); + } else { + journal_size.sectors() + } + } + None => DEFAULT_INTEGRITY_JOURNAL_SIZE.sectors(), + }; + + Ok(ValidatedIntegritySpec { + journal_size, + tag_spec: spec.tag_spec.unwrap_or(DEFAULT_INTEGRITY_TAG_SPEC), + block_size: DEFAULT_INTEGRITY_BLOCK_SIZE, + }) + } +} From ec6106be06217f2471f2a07ad782affbc7dc6c16 Mon Sep 17 00:00:00 2001 From: mulhern Date: Fri, 20 Dec 2024 12:17:58 -0500 Subject: [PATCH 08/11] Add whether or not to allocate the integrity superblock This is an option in the D-Bus, the predict script and the pool-level metadata. Signed-off-by: mulhern --- src/bin/utils/cmds.rs | 8 ++++++++ src/dbus_api/api/manager_3_8/api.rs | 7 +++++++ src/dbus_api/api/manager_3_8/methods.rs | 6 +++++- src/engine/strat_engine/backstore/blockdev/v2.rs | 7 ++++++- src/engine/types/mod.rs | 3 +++ tests/client-dbus/src/stratisd_client_dbus/_introspect.py | 1 + tests/client-dbus/tests/udev/_utils.py | 1 + 7 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/bin/utils/cmds.rs b/src/bin/utils/cmds.rs index 57826cc623..a60b23ad6c 100644 --- a/src/bin/utils/cmds.rs +++ b/src/bin/utils/cmds.rs @@ -110,6 +110,13 @@ pool is encrypted, setting this option has no effect on the prediction."), .default_value(Box::leak((*DEFAULT_INTEGRITY_JOURNAL_SIZE).to_string().into_boxed_str()) as &'static str) .help("Size of the integrity journal. Default is 128 MiB. Units are bytes.") .next_line_help(true) + ) + .arg( + Arg::new("no_integrity_superblock") + .action(ArgAction::SetTrue) + .long("no-integrity-superblock") + .help("Do not allocate space for integrity superblock") + .next_line_help(true) ), Command::new("filesystem") .about("Predicts the space usage when creating a Stratis filesystem.") @@ -166,6 +173,7 @@ impl<'a> UtilCommand<'a> for StratisPredictUsage { }) .expect("default specified by parser"), ), + allocate_superblock: Some(!sub_m.get_flag("no_integrity_superblock")), })?, LevelFilter::from_str( matches diff --git a/src/dbus_api/api/manager_3_8/api.rs b/src/dbus_api/api/manager_3_8/api.rs index 9d6a6329b4..5b8e468599 100644 --- a/src/dbus_api/api/manager_3_8/api.rs +++ b/src/dbus_api/api/manager_3_8/api.rs @@ -68,6 +68,13 @@ pub fn create_pool_method(f: &Factory, TData>) -> Method, TData>) -> MethodResult { ); let journal_size_tuple: (bool, u64) = get_next_arg(&mut iter, 4)?; let tag_spec_tuple: (bool, String) = get_next_arg(&mut iter, 5)?; + let allocate_superblock_tuple: (bool, bool) = get_next_arg(&mut iter, 6)?; let return_message = message.method_return(); @@ -201,6 +202,8 @@ pub fn create_pool(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { } }; + let allocate_superblock = tuple_to_option(allocate_superblock_tuple); + let dbus_context = m.tree.get_data(); let create_result = handle_action!(block_on(dbus_context.engine.create_pool( name, @@ -208,7 +211,8 @@ pub fn create_pool(m: &MethodInfo<'_, MTSync, TData>) -> MethodResult { EncryptionInfo::from_options((key_desc, clevis_info)).as_ref(), IntegritySpec { journal_size, - tag_spec + tag_spec, + allocate_superblock, }, ))); match create_result { diff --git a/src/engine/strat_engine/backstore/blockdev/v2.rs b/src/engine/strat_engine/backstore/blockdev/v2.rs index 7c5555cb62..6414863eb5 100644 --- a/src/engine/strat_engine/backstore/blockdev/v2.rs +++ b/src/engine/strat_engine/backstore/blockdev/v2.rs @@ -51,7 +51,12 @@ pub fn integrity_meta_space( total_space: Sectors, integrity_spec: ValidatedIntegritySpec, ) -> Sectors { - Bytes(4096).sectors() + (if integrity_spec.allocate_superblock { + Bytes(4096) + } else { + Bytes(0) + }) + .sectors() + integrity_spec.journal_size + Bytes::from( (*((total_space.bytes() / integrity_spec.block_size) diff --git a/src/engine/types/mod.rs b/src/engine/types/mod.rs index ebc8b1636c..a7e00fdb0e 100644 --- a/src/engine/types/mod.rs +++ b/src/engine/types/mod.rs @@ -530,6 +530,7 @@ impl IntegrityTagSpec { pub struct IntegritySpec { pub tag_spec: Option, pub journal_size: Option, + pub allocate_superblock: Option, } #[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)] @@ -537,6 +538,7 @@ pub struct ValidatedIntegritySpec { pub tag_spec: IntegrityTagSpec, pub journal_size: Sectors, pub block_size: Bytes, + pub allocate_superblock: bool, } impl Default for ValidatedIntegritySpec { @@ -566,6 +568,7 @@ impl TryFrom for ValidatedIntegritySpec { journal_size, tag_spec: spec.tag_spec.unwrap_or(DEFAULT_INTEGRITY_TAG_SPEC), block_size: DEFAULT_INTEGRITY_BLOCK_SIZE, + allocate_superblock: spec.allocate_superblock.unwrap_or(true), }) } } diff --git a/tests/client-dbus/src/stratisd_client_dbus/_introspect.py b/tests/client-dbus/src/stratisd_client_dbus/_introspect.py index 6326c23624..9920f2e29d 100644 --- a/tests/client-dbus/src/stratisd_client_dbus/_introspect.py +++ b/tests/client-dbus/src/stratisd_client_dbus/_introspect.py @@ -15,6 +15,7 @@ + diff --git a/tests/client-dbus/tests/udev/_utils.py b/tests/client-dbus/tests/udev/_utils.py index a5011c118b..40c4fcd935 100644 --- a/tests/client-dbus/tests/udev/_utils.py +++ b/tests/client-dbus/tests/udev/_utils.py @@ -152,6 +152,7 @@ def create_v2_pool(): ), "journal_size": (False, 0), "tag_spec": (False, ""), + "allocate_superblock": (False, False), }, ) From c9ceaa5e0fa431df8a7bab95510329480391614b Mon Sep 17 00:00:00 2001 From: mulhern Date: Fri, 3 Jan 2025 09:54:32 -0500 Subject: [PATCH 09/11] Print the integrity spec when printing the metadata Signed-off-by: mulhern --- src/engine/strat_engine/pool/inspection.rs | 25 +++++++++++++++++----- src/engine/types/mod.rs | 9 ++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/engine/strat_engine/pool/inspection.rs b/src/engine/strat_engine/pool/inspection.rs index ab2bfd6911..01ac2b7c52 100644 --- a/src/engine/strat_engine/pool/inspection.rs +++ b/src/engine/strat_engine/pool/inspection.rs @@ -10,7 +10,10 @@ use std::{ use devicemapper::Sectors; use crate::{ - engine::{strat_engine::serde_structs::PoolSave, types::DevUuid}, + engine::{ + strat_engine::serde_structs::PoolSave, + types::{DevUuid, ValidatedIntegritySpec}, + }, stratis::{StratisError, StratisResult}, }; @@ -535,7 +538,9 @@ impl fmt::Display for FlexDevice { } // Calculate map of device UUIDs to data device representation from metadata. -fn data_devices(metadata: &PoolSave) -> StratisResult> { +fn data_devices( + metadata: &PoolSave, +) -> StratisResult<(HashMap, Option)> { let data_tier_metadata = &metadata.backstore.data_tier; let data_tier_devs = &data_tier_metadata.blockdev.devs; @@ -570,7 +575,7 @@ fn data_devices(metadata: &PoolSave) -> StratisResult for ValidatedIntegritySpec { }) } } + +impl Display for ValidatedIntegritySpec { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + writeln!(f, "Allocate Superblock: {}", self.allocate_superblock)?; + writeln!(f, "Tag Specification: {}", self.tag_spec.as_ref())?; + writeln!(f, "Journal Size: {}", self.journal_size)?; + writeln!(f, "Block Size: {}", self.block_size) + } +} From 8364c5aac763a5a1ef0ad1000ddf93f81072b8c1 Mon Sep 17 00:00:00 2001 From: mulhern Date: Fri, 3 Jan 2025 12:14:19 -0500 Subject: [PATCH 10/11] Do one check on integrity spec and integrity allocations Signed-off-by: mulhern --- src/engine/strat_engine/pool/inspection.rs | 36 +++++++++++++++++++--- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/src/engine/strat_engine/pool/inspection.rs b/src/engine/strat_engine/pool/inspection.rs index 01ac2b7c52..0cbe203d34 100644 --- a/src/engine/strat_engine/pool/inspection.rs +++ b/src/engine/strat_engine/pool/inspection.rs @@ -12,7 +12,7 @@ use devicemapper::Sectors; use crate::{ engine::{ strat_engine::serde_structs::PoolSave, - types::{DevUuid, ValidatedIntegritySpec}, + types::{DevUuid, IntegrityTagSpec, ValidatedIntegritySpec}, }, stratis::{StratisError, StratisResult}, }; @@ -273,10 +273,31 @@ impl DataDevice { errors } - fn check(&self) -> Vec { + fn _check_integrity(&self, integrity_spec: Option) -> Vec { + if let Some(integrity_spec) = integrity_spec { + if !integrity_spec.allocate_superblock + && integrity_spec.journal_size == Sectors(0) + && integrity_spec.tag_spec == IntegrityTagSpec::B0 + && self.sum(&[DataDeviceUse::IntegrityMetadata]) > Sectors(0) + { + vec![ + format!( + "Integrity specification should resolve to 0 allocations for integrity, but data device has space allocated for integrity." + ) + ] + } else { + vec![] + } + } else { + vec![] + } + } + + fn check(&self, integrity_spec: Option) -> Vec { let mut errors = Vec::new(); errors.extend(check_overlap(&self.extents, self.offset())); errors.extend(self._check_integrity_meta_round()); + errors.extend(self._check_integrity(integrity_spec)); errors } } @@ -674,9 +695,14 @@ pub mod inspectors { let encrypted = metadata.features.contains(&PoolFeatures::Encryption); - let (data_devices, _) = data_devices(metadata)?; - for data_device in data_devices.values() { - errors.extend(data_device.check()); + let (data_devices, integrity_spec) = data_devices(metadata)?; + for (uuid, data_device) in data_devices.iter() { + errors.extend( + data_device + .check(integrity_spec) + .iter() + .map(|s| format!("Device {uuid}: {s}")), + ); } let cache_devices = cache_devices(metadata)?; From eec882ad8b32033d1e9da8e995f533c30b07cf55 Mon Sep 17 00:00:00 2001 From: mulhern Date: Fri, 3 Jan 2025 12:15:38 -0500 Subject: [PATCH 11/11] Do not print out "Error encountered" The error will print out its own description of itself as an error, so this extra error text is redundant. Signed-off-by: mulhern --- src/bin/stratisd-tools.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/stratisd-tools.rs b/src/bin/stratisd-tools.rs index e057929a57..3ab951c410 100644 --- a/src/bin/stratisd-tools.rs +++ b/src/bin/stratisd-tools.rs @@ -77,7 +77,7 @@ fn main() { match c.run(stripped_args) { Ok(()) => {} Err(e) => { - eprintln!("Error encountered: {e}"); + eprintln!("{e}"); process::exit(1); } }