diff --git a/src/engine/sim_engine/pool.rs b/src/engine/sim_engine/pool.rs index 20a8caabce..976f742295 100644 --- a/src/engine/sim_engine/pool.rs +++ b/src/engine/sim_engine/pool.rs @@ -125,13 +125,6 @@ impl SimPool { } } - fn filesystems_mut(&mut self) -> Vec<(Name, FilesystemUuid, &mut SimFilesystem)> { - self.filesystems - .iter_mut() - .map(|(name, uuid, x)| (name.clone(), *uuid, x)) - .collect() - } - pub fn record(&self, name: &str) -> PoolSave { PoolSave { name: name.to_owned(), @@ -489,30 +482,52 @@ impl Pool for SimPool { _pool_name: &str, fs_uuids: &HashSet, ) -> StratisResult> { - let mut removed = Vec::new(); - for &uuid in fs_uuids { - if self.filesystems.remove_by_uuid(uuid).is_some() { - removed.push(uuid); - } - } - - let updated_origins: Vec = self - .filesystems_mut() - .iter_mut() + let snapshots = self + .filesystems() + .iter() .filter_map(|(_, u, fs)| { fs.origin().and_then(|x| { - if removed.contains(&x) { - if fs.unset_origin() { - Some(*u) - } else { - None - } + if fs_uuids.contains(&x) { + Some((x, (*u, fs.merge_scheduled()))) } else { None } }) }) - .collect(); + .fold(HashMap::new(), |mut acc, (u, v)| { + acc.entry(u) + .and_modify(|e: &mut Vec<_>| e.push(v)) + .or_insert(vec![v]); + acc + }); + + let scheduled_for_merge = snapshots + .iter() + .filter(|(_, snaps)| snaps.iter().any(|(_, scheduled)| *scheduled)) + .collect::>(); + if !scheduled_for_merge.is_empty() { + let err_str = format!("The filesystem destroy operation can not be begun until the revert operations for the following filesystem snapshots have been cancelled: {}", scheduled_for_merge.iter().map(|(u, _)| u.to_string()).collect::>().join(", ")); + return Err(StratisError::Msg(err_str)); + } + + let (mut removed, mut updated_origins) = (Vec::new(), Vec::new()); + for &uuid in fs_uuids { + if self.filesystems.remove_by_uuid(uuid).is_some() { + removed.push(uuid); + + let snaps = snapshots + .get(&uuid) + .expect("snapshots map construction ensures key is present"); + // The filesystems in snaps may have been removed; any one of + // them may also be a filesystem that was scheduled for removal. + for (sn_uuid, _) in snaps { + if let Some((_, sn)) = self.filesystems.get_mut_by_uuid(*sn_uuid) { + assert!(sn.unset_origin()); + updated_origins.push(*sn_uuid); + }; + } + } + } Ok(SetDeleteAction::new(removed, updated_origins)) } diff --git a/src/engine/strat_engine/pool/v1.rs b/src/engine/strat_engine/pool/v1.rs index dfea378c55..3a5987ed02 100644 --- a/src/engine/strat_engine/pool/v1.rs +++ b/src/engine/strat_engine/pool/v1.rs @@ -1023,36 +1023,7 @@ impl Pool for StratPool { pool_name: &str, fs_uuids: &HashSet, ) -> StratisResult> { - let mut removed = Vec::new(); - for &uuid in fs_uuids { - if let Some(uuid) = self.thin_pool.destroy_filesystem(pool_name, uuid)? { - removed.push(uuid); - } - } - - let snapshots: Vec = self - .thin_pool - .filesystems() - .iter() - .filter_map(|(_, u, fs)| { - fs.origin() - .and_then(|x| if removed.contains(&x) { Some(*u) } else { None }) - }) - .collect(); - - let mut updated_origins = vec![]; - for sn_uuid in snapshots { - if let Err(err) = self.thin_pool.unset_fs_origin(sn_uuid) { - warn!( - "Failed to write null origin to metadata for filesystem with UUID {}: {}", - sn_uuid, err - ); - } else { - updated_origins.push(sn_uuid); - } - } - - Ok(SetDeleteAction::new(removed, updated_origins)) + self.thin_pool.destroy_filesystems(pool_name, fs_uuids) } #[pool_mutating_action("NoRequests")] diff --git a/src/engine/strat_engine/pool/v2.rs b/src/engine/strat_engine/pool/v2.rs index 38554ff888..2d68869261 100644 --- a/src/engine/strat_engine/pool/v2.rs +++ b/src/engine/strat_engine/pool/v2.rs @@ -928,36 +928,7 @@ impl Pool for StratPool { pool_name: &str, fs_uuids: &HashSet, ) -> StratisResult> { - let mut removed = Vec::new(); - for &uuid in fs_uuids { - if let Some(uuid) = self.thin_pool.destroy_filesystem(pool_name, uuid)? { - removed.push(uuid); - } - } - - let snapshots: Vec = self - .thin_pool - .filesystems() - .iter() - .filter_map(|(_, u, fs)| { - fs.origin() - .and_then(|x| if removed.contains(&x) { Some(*u) } else { None }) - }) - .collect(); - - let mut updated_origins = vec![]; - for sn_uuid in snapshots { - if let Err(err) = self.thin_pool.unset_fs_origin(sn_uuid) { - warn!( - "Failed to write null origin to metadata for filesystem with UUID {}: {}", - sn_uuid, err - ); - } else { - updated_origins.push(sn_uuid); - } - } - - Ok(SetDeleteAction::new(removed, updated_origins)) + self.thin_pool.destroy_filesystems(pool_name, fs_uuids) } #[pool_mutating_action("NoRequests")] diff --git a/src/engine/strat_engine/thinpool/thinpool.rs b/src/engine/strat_engine/thinpool/thinpool.rs index e1e4831e84..654409aa51 100644 --- a/src/engine/strat_engine/thinpool/thinpool.rs +++ b/src/engine/strat_engine/thinpool/thinpool.rs @@ -24,7 +24,7 @@ use devicemapper::{ use crate::{ engine::{ - engine::{DumpState, StateDiff}, + engine::{DumpState, Filesystem, StateDiff}, strat_engine::{ backstore::backstore::{v1, v2, InternalBackstore}, cmd::{thin_check, thin_metadata_size, thin_repair}, @@ -38,7 +38,10 @@ use crate::{ writing::wipe_sectors, }, structures::Table, - types::{Compare, Diff, FilesystemUuid, Name, PoolUuid, StratFilesystemDiff, ThinPoolDiff}, + types::{ + Compare, Diff, FilesystemUuid, Name, PoolUuid, SetDeleteAction, StratFilesystemDiff, + ThinPoolDiff, + }, }, stratis::{StratisError, StratisResult}, }; @@ -603,7 +606,7 @@ impl ThinPool { /// * Ok(Some(uuid)) provides the uuid of the destroyed filesystem /// * Ok(None) is returned if the filesystem did not exist /// * Err(_) is returned if the filesystem could not be destroyed - pub fn destroy_filesystem( + fn destroy_filesystem( &mut self, pool_name: &str, uuid: FilesystemUuid, @@ -765,6 +768,64 @@ impl ThinPool { ) .map_err(|e| e.into()) } + + pub fn destroy_filesystems( + &mut self, + pool_name: &str, + fs_uuids: &HashSet, + ) -> StratisResult> { + let snapshots = self + .filesystems() + .iter() + .filter_map(|(_, u, fs)| { + fs.origin().and_then(|x| { + if fs_uuids.contains(&x) { + Some((x, (*u, fs.merge_scheduled()))) + } else { + None + } + }) + }) + .fold(HashMap::new(), |mut acc, (u, v)| { + acc.entry(u) + .and_modify(|e: &mut Vec<_>| e.push(v)) + .or_insert(vec![v]); + acc + }); + + let scheduled_for_merge = snapshots + .iter() + .filter(|(_, snaps)| snaps.iter().any(|(_, scheduled)| *scheduled)) + .collect::>(); + if !scheduled_for_merge.is_empty() { + let err_str = format!("The filesystem destroy operation can not be begun until the revert operations for the following filesystem snapshots have been cancelled: {}", scheduled_for_merge.iter().map(|(u, _)| u.to_string()).collect::>().join(", ")); + return Err(StratisError::Msg(err_str)); + } + + let (mut removed, mut updated_origins) = (Vec::new(), Vec::new()); + for &uuid in fs_uuids { + if let Some(uuid) = self.destroy_filesystem(pool_name, uuid)? { + removed.push(uuid); + + let snaps = snapshots + .get(&uuid) + .expect("snapshots map construction ensures key is present"); + // The filesystems in snaps may have been removed; any one of + // them may also be a filesystem that was scheduled for removal. + for (sn_uuid, _) in snaps { + if let Some((_, sn)) = self.get_mut_filesystem_by_uuid(*sn_uuid) { + assert!(sn.unset_origin()); + updated_origins.push(*sn_uuid); + + let (name, sn) = self.get_filesystem_by_uuid(*sn_uuid).expect("just got"); + self.mdv.save_fs(&name, *sn_uuid, sn)?; + }; + } + } + } + + Ok(SetDeleteAction::new(removed, updated_origins)) + } } impl ThinPool { @@ -1737,22 +1798,6 @@ where Ok(changed) } - pub fn unset_fs_origin(&mut self, fs_uuid: FilesystemUuid) -> StratisResult { - let changed = { - let (_, fs) = self.get_mut_filesystem_by_uuid(fs_uuid).ok_or_else(|| { - StratisError::Msg(format!("No filesystem with UUID {fs_uuid} found")) - })?; - fs.unset_origin() - }; - if changed { - let (name, fs) = self - .get_filesystem_by_uuid(fs_uuid) - .expect("Looked up above."); - self.mdv.save_fs(&name, fs_uuid, fs)?; - } - Ok(changed) - } - /// Set the filesystem merge scheduled value for filesystem with given UUID. pub fn set_fs_merge_scheduled( &mut self,