diff --git a/src/engine/strat_engine/backstore/backstore.rs b/src/engine/strat_engine/backstore/backstore.rs index eaa802713b..ba93b4590e 100644 --- a/src/engine/strat_engine/backstore/backstore.rs +++ b/src/engine/strat_engine/backstore/backstore.rs @@ -26,7 +26,6 @@ use crate::{ data_tier::DataTier, devices::UnownedDevices, shared::BlockSizeSummary, - transaction::RequestTransaction, }, dm::{get_dm, list_of_backstore_devices, remove_optional_devices}, metadata::{MDADataSize, BDA}, @@ -416,59 +415,41 @@ impl Backstore { /// retained as a convenience to the caller. /// Postcondition: /// forall i, result_i.0 = result_(i - 1).0 + result_(i - 1).1 - pub fn request_alloc( + /// + /// WARNING: metadata changing event + pub fn alloc( &mut self, + pool_uuid: PoolUuid, sizes: &[Sectors], - ) -> StratisResult> { - let mut transaction = match self.data_tier.alloc_request(sizes)? { - Some(t) => t, - None => return Ok(None), - }; + ) -> StratisResult>> { + let total_required = sizes.iter().cloned().sum(); + if self.available_in_backstore() < total_required { + return Ok(None); + } - let mut next = self.next; + if self.data_tier.alloc(sizes) { + self.extend_cap_device(pool_uuid)?; + } else { + return Ok(None); + } + + let mut chunks = Vec::new(); for size in sizes { - transaction.add_seg_req((next, *size)); - next += *size + chunks.push((self.next, *size)); + self.next += *size; } // Assert that the postcondition holds. assert_eq!( sizes, - transaction - .get_backstore() + chunks .iter() .map(|x| x.1) .collect::>() .as_slice() ); - Ok(Some(transaction)) - } - - /// Commit space requested by request_alloc() to metadata. - /// - /// This method commits the newly allocated data segments and then extends the cap device - /// to be the same size as the allocated data size. - pub fn commit_alloc( - &mut self, - pool_uuid: PoolUuid, - transaction: RequestTransaction, - ) -> StratisResult<()> { - let segs = transaction.get_backstore(); - self.data_tier.alloc_commit(transaction)?; - // This must occur after the segments have been updated in the data tier - self.extend_cap_device(pool_uuid)?; - - assert!(self.next <= self.size()); - - self.next += segs - .into_iter() - .fold(Sectors(0), |mut size, (_, next_size)| { - size += next_size; - size - }); - - Ok(()) + Ok(Some(chunks)) } /// Get only the datadevs in the pool. @@ -542,6 +523,7 @@ impl Backstore { /// DM devices. But the devicemapper library stores the data from which /// the size of each DM device is calculated; the result is computed and /// no ioctl is required. + #[cfg(test)] fn size(&self) -> Sectors { self.linear .as_ref() @@ -1179,11 +1161,10 @@ mod tests { invariant(&backstore); // Allocate space from the backstore so that the cap device is made. - let transaction = backstore - .request_alloc(&[INITIAL_BACKSTORE_ALLOCATION]) + backstore + .alloc(pool_uuid, &[INITIAL_BACKSTORE_ALLOCATION]) .unwrap() .unwrap(); - backstore.commit_alloc(pool_uuid, transaction).unwrap(); let cache_uuids = backstore .init_cache(pool_name.clone(), pool_uuid, initcachedevs, None) @@ -1295,11 +1276,10 @@ mod tests { invariant(&backstore); // Allocate space from the backstore so that the cap device is made. - let transaction = backstore - .request_alloc(&[INITIAL_BACKSTORE_ALLOCATION]) + backstore + .alloc(pool_uuid, &[INITIAL_BACKSTORE_ALLOCATION]) .unwrap() .unwrap(); - backstore.commit_alloc(pool_uuid, transaction).unwrap(); let old_device = backstore.device(); diff --git a/src/engine/strat_engine/backstore/blockdev.rs b/src/engine/strat_engine/backstore/blockdev.rs index b73b798d79..f2fe6ce10f 100644 --- a/src/engine/strat_engine/backstore/blockdev.rs +++ b/src/engine/strat_engine/backstore/blockdev.rs @@ -25,7 +25,6 @@ use crate::{ crypt::CryptHandle, devices::BlockSizes, range_alloc::{PerDevSegments, RangeAllocator}, - transaction::RequestTransaction, }, device::blkdev_size, metadata::{ @@ -267,19 +266,8 @@ impl StratBlockDev { /// Find some sector ranges that could be allocated. If more /// sectors are needed than are available, return partial results. - pub fn request_space( - &self, - size: Sectors, - transaction: &RequestTransaction, - ) -> StratisResult { - self.used.request(self.uuid(), size, transaction) - } - - /// Commit allocation requested by request_space(). - /// - /// This method will record the requested allocations in the metadata. - pub fn commit_space(&mut self, segs: PerDevSegments) { - self.used.commit(segs); + pub fn alloc(&mut self, size: Sectors) -> PerDevSegments { + self.used.alloc(size) } // ALL SIZE METHODS (except size(), which is in BlockDev impl.) diff --git a/src/engine/strat_engine/backstore/blockdevmgr.rs b/src/engine/strat_engine/backstore/blockdevmgr.rs index ce38284d32..b012cfdd90 100644 --- a/src/engine/strat_engine/backstore/blockdevmgr.rs +++ b/src/engine/strat_engine/backstore/blockdevmgr.rs @@ -21,9 +21,7 @@ use crate::{ blockdev::StratBlockDev, crypt::CryptHandle, devices::{initialize_devices, wipe_blockdevs, UnownedDevices}, - range_alloc::PerDevSegments, shared::{BlkDevSegment, Segment}, - transaction::RequestTransaction, }, metadata::{MDADataSize, BDA}, serde_structs::{BaseBlockDevSave, Recordable}, @@ -194,16 +192,17 @@ impl BlockDevMgr { /// not possible to satisfy the request. /// This method is atomic, it either allocates all requested or allocates /// nothing. - pub fn request_space(&self, sizes: &[Sectors]) -> StratisResult> { - let mut transaction = RequestTransaction::default(); - + pub fn alloc(&mut self, sizes: &[Sectors]) -> Option>> { let total_needed: Sectors = sizes.iter().cloned().sum(); if self.avail_space() < total_needed { - return Ok(None); + return None; } - for (idx, &needed) in sizes.iter().enumerate() { + let mut lists = Vec::new(); + + for &needed in sizes.iter() { let mut alloc = Sectors(0); + let mut segs = Vec::new(); // TODO: Consider greater efficiency for allocation generally. // Over time, the blockdevs at the start will be exhausted. It // might be a good idea to keep an auxiliary structure, so that @@ -211,63 +210,23 @@ impl BlockDevMgr { // In the context of this major inefficiency that ensues over time // the obvious but more minor inefficiency of this inner loop is // not worth worrying about. - for bd in &self.block_devs { + for bd in &mut self.block_devs { if alloc == needed { break; } - let r_segs = bd.request_space(needed - alloc, &transaction)?; - for (&start, &length) in r_segs.iter() { - transaction.add_bd_seg_req( - idx, - BlkDevSegment::new(bd.uuid(), Segment::new(*bd.device(), start, length)), - ); - } + let r_segs = bd.alloc(needed - alloc); + let blkdev_segs = r_segs.iter().map(|(&start, &length)| { + BlkDevSegment::new(bd.uuid(), Segment::new(*bd.device(), start, length)) + }); + segs.extend(blkdev_segs); alloc += r_segs.sum(); } assert_eq!(alloc, needed); + lists.push(segs); } - Ok(Some(transaction)) - } - - /// Commit the allocations calculated by the request_space() method. - /// - /// This method converts the block device segments into the necessary data - /// structure and dispatches them to the corresponding block devices to - /// update the internal records of allocated space. - pub fn commit_space(&mut self, mut transaction: RequestTransaction) -> StratisResult<()> { - let mut segs = transaction.drain_blockdevmgr().try_fold( - HashMap::::new(), - |mut map, seg| -> StratisResult<_> { - if let Some(segs) = map.get_mut(&seg.uuid) { - segs.insert(&(seg.segment.start, seg.segment.length))?; - } else { - let mut segs = PerDevSegments::new( - self.block_devs - .iter() - .find(|bd| bd.uuid() == seg.uuid) - .expect( - "Block dev was determined to be present during allocation request", - ) - .total_size() - .sectors(), - ); - segs.insert(&(seg.segment.start, seg.segment.length))?; - map.insert(seg.uuid, segs); - } - - Ok(map) - }, - )?; - - for (uuid, bd) in self.blockdevs_mut() { - if let Some(segs) = segs.remove(&uuid) { - bd.commit_space(segs); - } - } - - Ok(()) + Some(lists) } /// Write the given data to all blockdevs marking with current time. @@ -472,8 +431,7 @@ mod tests { assert_eq!(mgr.avail_space() + mgr.metadata_size(), mgr.size()); let allocated = Sectors(2); - let transaction = mgr.request_space(&[allocated]).unwrap().unwrap(); - mgr.commit_space(transaction).unwrap(); + mgr.alloc(&[allocated]).unwrap(); assert_eq!( mgr.avail_space() + allocated + mgr.metadata_size(), mgr.size() diff --git a/src/engine/strat_engine/backstore/cache_tier.rs b/src/engine/strat_engine/backstore/cache_tier.rs index 7950fcaa8d..5439be1288 100644 --- a/src/engine/strat_engine/backstore/cache_tier.rs +++ b/src/engine/strat_engine/backstore/cache_tier.rs @@ -128,17 +128,14 @@ impl CacheTier { ))); } - let trans = self + let segments = self .block_mgr - .request_space(&[avail_space])? - .expect("asked for exactly the space available, must get"); - let segments = trans.get_blockdevmgr(); - if let Err(e) = self.block_mgr.commit_space(trans) { - self.block_mgr.remove_blockdevs(&uuids)?; - return Err(StratisError::Msg(format!( - "Failed to commit metadata changes: {e}" - ))); - } + .alloc(&[avail_space]) + .expect("asked for exactly the space available, must get") + .iter() + .flat_map(|s| s.iter()) + .cloned() + .collect::>(); self.cache_segments.coalesce_blkdevsegs(&segments); Ok((uuids, (true, false))) @@ -170,21 +167,15 @@ impl CacheTier { ))); } - let trans = block_mgr - .request_space(&[meta_space, avail_space - meta_space])? + let mut segments = block_mgr + .alloc(&[meta_space, avail_space - meta_space]) .expect("asked for exactly the space available, must get"); - let meta_segments = AllocatedAbove { - inner: trans.get_segs_for_req(0).expect("segments.len() == 2"), - }; let cache_segments = AllocatedAbove { - inner: trans.get_segs_for_req(1).expect("segments.len() == 2"), + inner: segments.pop().expect("segments.len() == 2"), + }; + let meta_segments = AllocatedAbove { + inner: segments.pop().expect("segments.len() == 1"), }; - if let Err(e) = block_mgr.commit_space(trans) { - block_mgr.destroy_all()?; - return Err(StratisError::Msg(format!( - "Failed to commit metadata changes: {e}" - ))); - } Ok(CacheTier { block_mgr, diff --git a/src/engine/strat_engine/backstore/data_tier.rs b/src/engine/strat_engine/backstore/data_tier.rs index ea6f16570c..a87dbbb2c5 100644 --- a/src/engine/strat_engine/backstore/data_tier.rs +++ b/src/engine/strat_engine/backstore/data_tier.rs @@ -17,7 +17,6 @@ use crate::{ blockdevmgr::BlockDevMgr, devices::UnownedDevices, shared::{metadata_to_segment, AllocatedAbove, BlkDevSegment, BlockDevPartition}, - transaction::RequestTransaction, }, serde_structs::{BaseDevSave, BlockDevSave, DataTierSave, Recordable}, types::BDARecordResult, @@ -89,20 +88,22 @@ impl DataTier { } /// Allocate a region for all sector size requests from unallocated segments in - /// block devices belonging to the data tier. Return Some(_) if requested - /// amount or more was allocated, otherwise, None. - pub fn alloc_request(&self, requests: &[Sectors]) -> StratisResult> { - self.block_mgr.request_space(requests) - } - - /// Commit an allocation that was determined to be valid by alloc_request() - /// to metadata. - pub fn alloc_commit(&mut self, transaction: RequestTransaction) -> StratisResult<()> { - let segments = transaction.get_blockdevmgr(); - self.block_mgr.commit_space(transaction)?; - self.segments.coalesce_blkdevsegs(&segments); - - Ok(()) + /// block devices belonging to the data tier. Return true if requested + /// amount or more was allocated, otherwise, false. + pub fn alloc(&mut self, requests: &[Sectors]) -> bool { + self.block_mgr + .alloc(requests) + .map(|segments| { + self.segments.coalesce_blkdevsegs( + &segments + .iter() + .flat_map(|s| s.iter()) + .cloned() + .collect::>(), + ); + true + }) + .unwrap_or(false) } /// The sum of the lengths of all the sectors that have been mapped to an @@ -260,8 +261,7 @@ mod tests { let request_amount = data_tier.block_mgr.avail_space() / 2usize; assert!(request_amount != Sectors(0)); - let transaction = data_tier.alloc_request(&[request_amount]).unwrap().unwrap(); - data_tier.alloc_commit(transaction).unwrap(); + assert!(data_tier.alloc(&[request_amount])); data_tier.invariant(); // A data tier w/ some amount allocated @@ -279,11 +279,7 @@ mod tests { size = data_tier.size(); // Allocate enough to get into the newly added block devices - let transaction = data_tier - .alloc_request(&[last_request_amount]) - .unwrap() - .unwrap(); - data_tier.alloc_commit(transaction).unwrap(); + assert!(data_tier.alloc(&[last_request_amount])); data_tier.invariant(); assert!(data_tier.allocated() >= request_amount + last_request_amount); diff --git a/src/engine/strat_engine/backstore/mod.rs b/src/engine/strat_engine/backstore/mod.rs index b88af56959..446981a959 100644 --- a/src/engine/strat_engine/backstore/mod.rs +++ b/src/engine/strat_engine/backstore/mod.rs @@ -12,7 +12,6 @@ mod data_tier; mod devices; mod range_alloc; mod shared; -mod transaction; pub use self::{ backstore::Backstore, diff --git a/src/engine/strat_engine/backstore/range_alloc.rs b/src/engine/strat_engine/backstore/range_alloc.rs index da2bdf96b3..6651fca34d 100644 --- a/src/engine/strat_engine/backstore/range_alloc.rs +++ b/src/engine/strat_engine/backstore/range_alloc.rs @@ -10,10 +10,7 @@ use std::{ use devicemapper::Sectors; use crate::{ - engine::{ - strat_engine::{backstore::transaction::RequestTransaction, metadata::BlockdevSize}, - types::DevUuid, - }, + engine::strat_engine::metadata::BlockdevSize, stratis::{StratisError, StratisResult}, }; @@ -352,38 +349,11 @@ impl RangeAllocator { /// Attempt to allocate. /// Returns a PerDevSegments object containing the allocated ranges. - /// The device UUID is used to filter out all segment requests on devices - /// other than the specified device. - pub fn request( - &self, - uuid: DevUuid, - amount: Sectors, - transaction: &RequestTransaction, - ) -> StratisResult { - let trans_used = transaction - .get_blockdevmgr() - .into_iter() - .filter_map(|seg| { - if seg.uuid == uuid { - Some((seg.segment.start, seg.segment.length)) - } else { - None - } - }) - .try_fold( - PerDevSegments::new(self.segments.limit()), - |mut segs, seg| { - segs.insert(&seg)?; - StratisResult::Ok(segs) - }, - )?; - - let total_segments = self.segments.union(&trans_used)?; - + pub fn alloc(&mut self, amount: Sectors) -> PerDevSegments { let mut segs = PerDevSegments::new(self.segments.limit()); let mut needed = amount; - for (&start, &len) in total_segments.complement().iter() { + for (&start, &len) in self.segments.complement().iter() { if needed == Sectors(0) { break; } @@ -393,18 +363,12 @@ impl RangeAllocator { .expect("wholly disjoint from other elements in segs"); needed -= to_use; } - Ok(segs) - } - - /// Commit an allocation determined to be valid. - /// - /// This method does not actually modify metadata but is required for bookkeeping - /// for the internal block device allocation data structure. - pub fn commit(&mut self, segs: PerDevSegments) { self.segments = self .segments .union(&segs) .expect("all segments verified to be in available ranges"); + + segs } /// Increase the available size of the RangeAlloc data structure. @@ -445,24 +409,14 @@ mod tests { assert_eq!(allocator.used(), Sectors(100)); assert_eq!(allocator.available(), Sectors(28)); - let seg = allocator - .request( - DevUuid::new_v4(), - Sectors(50), - &RequestTransaction::default(), - ) - .unwrap(); - allocator.commit(seg.clone()); + let seg = allocator.alloc(Sectors(50)); assert_eq!(seg.len(), 2); assert_eq!(seg.sum(), Sectors(28)); assert_eq!(allocator.used(), Sectors(128)); assert_eq!(allocator.available(), Sectors(0)); let available = allocator.available(); - let seg = allocator - .request(DevUuid::new_v4(), available, &RequestTransaction::default()) - .unwrap(); - allocator.commit(seg); + allocator.alloc(available); assert_eq!(allocator.available(), Sectors(0)); } @@ -535,14 +489,7 @@ mod tests { fn test_allocator_failures_range_overwrite() { let mut allocator = RangeAllocator::new(BlockdevSize::new(Sectors(128)), &[]).unwrap(); - let seg = allocator - .request( - DevUuid::new_v4(), - Sectors(128), - &RequestTransaction::default(), - ) - .unwrap(); - allocator.commit(seg.clone()); + let seg = allocator.alloc(Sectors(128)); assert_eq!(allocator.used(), Sectors(128)); assert_eq!( seg.iter().collect::>(), diff --git a/src/engine/strat_engine/backstore/transaction.rs b/src/engine/strat_engine/backstore/transaction.rs deleted file mode 100644 index 3fe188cbc0..0000000000 --- a/src/engine/strat_engine/backstore/transaction.rs +++ /dev/null @@ -1,120 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// 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 std::{ - collections::{HashMap, HashSet}, - iter::once, -}; - -use devicemapper::Sectors; - -use crate::engine::strat_engine::backstore::shared::BlkDevSegment; - -/// This transaction data structure keeps a list of segments associated with block -/// devices, segments from the cap device, and a map associating each cap device -/// segment with one or more block device segments that make it up. Because the -/// request and allocated space are both vectors in the same order, the API -/// for this data structure relies heavily on indices. -#[derive(Default)] -pub struct RequestTransaction { - /// Block device segments - blockdevmgr: Vec, - /// Cap device segments - backstore: Vec<(Sectors, Sectors)>, - /// Map between a cap device segment and its corresponding block device segments - map: HashMap>, -} - -impl RequestTransaction { - /// Add a cap device segment request to be committed later. - pub fn add_seg_req(&mut self, seg_req: (Sectors, Sectors)) { - self.backstore.push(seg_req); - } - - /// Add a block device segment request to be committed later. - /// - /// The index must correspond to the appropriate index of the cap device segment - /// that has been requested. This will permit cancelling part of the request - /// but not another. - pub fn add_bd_seg_req(&mut self, seg_req_idx: usize, seg: BlkDevSegment) { - self.blockdevmgr.push(seg); - if let Some(is) = self.map.get_mut(&seg_req_idx) { - is.insert(self.blockdevmgr.len() - 1); - } else { - self.map.insert( - seg_req_idx, - once(self.blockdevmgr.len() - 1).collect::>(), - ); - } - } - - /// Drain the block device segments from this transaction data structure and - /// make them available as an iterator. - pub fn drain_blockdevmgr(&mut self) -> impl Iterator + '_ { - self.blockdevmgr.drain(..) - } - - /// Get a vector of all block device segments for this transaction. - pub fn get_blockdevmgr(&self) -> Vec { - self.blockdevmgr.clone() - } - - /// Get a single cap device segment for this transaction by the index associated - /// with the request. - pub fn get_backstore_elem(&mut self, idx: usize) -> Option<(Sectors, Sectors)> { - self.backstore.get(idx).cloned() - } - - /// Get a list of all cap device segments associated with this transaction. - pub fn get_backstore(&self) -> Vec<(Sectors, Sectors)> { - self.backstore.clone() - } - - /// Drain the cap device segments from this transaction data structure and - /// make them available as an iterator. - pub fn drain_backstore(&mut self) -> impl Iterator + '_ { - self.backstore.drain(..) - } - - /// Get all block device segments associated with the cap device request located - /// at index idx. - pub fn get_segs_for_req(&self, idx: usize) -> Option> { - self.map.get(&idx).map(|set| { - self.blockdevmgr - .iter() - .cloned() - .enumerate() - .filter_map(|(seg_idx, seg)| { - if set.contains(&seg_idx) { - Some(seg) - } else { - None - } - }) - .collect::>() - }) - } - - /// Remove all cap device and block device requests associated with the cap - /// device request at index idx. - pub fn remove_request(&mut self, idx: usize) { - self.backstore.remove(idx); - let removal_is = self - .map - .get(&idx) - .expect("Cannot have a backstore allocation without blockdev allocations"); - self.blockdevmgr = self - .blockdevmgr - .drain(..) - .enumerate() - .filter_map(|(idx, seg)| { - if !removal_is.contains(&idx) { - Some(seg) - } else { - None - } - }) - .collect::>(); - } -} diff --git a/src/engine/strat_engine/thinpool/thinpool.rs b/src/engine/strat_engine/thinpool/thinpool.rs index 60857dae01..c80e314bce 100644 --- a/src/engine/strat_engine/thinpool/thinpool.rs +++ b/src/engine/strat_engine/thinpool/thinpool.rs @@ -315,17 +315,16 @@ impl ThinPool { data_block_size: Sectors, backstore: &mut Backstore, ) -> StratisResult { - let mut segments_list = match backstore.request_alloc(&[ - thin_pool_size.meta_size(), - thin_pool_size.meta_size(), - thin_pool_size.data_size(), - thin_pool_size.mdv_size(), - ])? { - Some(trans) => { - let segs = trans.get_backstore(); - backstore.commit_alloc(pool_uuid, trans)?; - segs - } + let mut segments_list = match backstore.alloc( + pool_uuid, + &[ + thin_pool_size.meta_size(), + thin_pool_size.meta_size(), + thin_pool_size.data_size(), + thin_pool_size.mdv_size(), + ], + )? { + Some(segs) => segs, None => { let err_msg = "Could not allocate sufficient space for thinpool devices"; return Err(StratisError::Msg(err_msg.into())); @@ -918,11 +917,8 @@ impl ThinPool { let requests = vec![data_extend_size]; let data_index = 0; - match backstore.request_alloc(&requests) { - Ok(Some(transaction)) => { - let backstore_segs = transaction.get_backstore(); - backstore.commit_alloc(pool_uuid, transaction)?; - + match backstore.alloc(pool_uuid, &requests) { + Ok(Some(backstore_segs)) => { let data_segment = backstore_segs.get(data_index).cloned(); let data_segments = data_segment.map(|seg| coalesce_segs(data_existing_segments, &[seg])); @@ -1017,11 +1013,8 @@ impl ThinPool { let requests = vec![meta_extend_size, meta_extend_size]; let meta_index = 0; let spare_index = 1; - match backstore.request_alloc(&requests) { - Ok(Some(transaction)) => { - let backstore_segs = transaction.get_backstore(); - backstore.commit_alloc(pool_uuid, transaction)?; - + match backstore.alloc(pool_uuid, &requests) { + Ok(Some(backstore_segs)) => { let meta_and_spare_segment = backstore_segs.get(meta_index).and_then(|seg| { backstore_segs.get(spare_index).map(|seg_s| (*seg, *seg_s)) });