diff --git a/src/engine/sim_engine/filesystem.rs b/src/engine/sim_engine/filesystem.rs index 61c77f5d41..291cd74f2d 100644 --- a/src/engine/sim_engine/filesystem.rs +++ b/src/engine/sim_engine/filesystem.rs @@ -106,8 +106,11 @@ impl SimFilesystem { pub fn set_merge_scheduled(&mut self, scheduled: bool) -> StratisResult { if self.merge_scheduled == scheduled { Ok(false) + } else if scheduled && self.origin.is_none() { + Err(StratisError::Msg( + "Filesystem has no origin filesystem; can not schedule a merge".into(), + )) } else { - // TODO: reject if conflict or no origin self.merge_scheduled = scheduled; Ok(true) } diff --git a/src/engine/sim_engine/pool.rs b/src/engine/sim_engine/pool.rs index 33b8b2b7fc..6075c743f8 100644 --- a/src/engine/sim_engine/pool.rs +++ b/src/engine/sim_engine/pool.rs @@ -798,17 +798,51 @@ impl Pool for SimPool { fn set_fs_merge_scheduled( &mut self, fs_uuid: FilesystemUuid, - schedule: bool, + scheduled: bool, ) -> StratisResult> { - let (_, fs) = self.filesystems.get_mut_by_uuid(fs_uuid).ok_or_else(|| { - StratisError::Msg(format!("Filesystem with UUID {fs_uuid} not found")) + let (_, fs) = self + .filesystems + .get_by_uuid(fs_uuid) + .ok_or_else(|| StratisError::Msg(format!("No filesystem with UUID {fs_uuid} found")))?; + + let origin = fs.origin().ok_or_else(|| { + StratisError::Msg(format!( + "Filesystem {fs_uuid} has no origin, revert cannot be scheduled" + )) })?; - let changed = fs.set_merge_scheduled(schedule)?; - if changed { - Ok(PropChangeAction::NewValue(schedule)) - } else { - Ok(PropChangeAction::Identity) + + if fs.merge_scheduled() == scheduled { + return Ok(PropChangeAction::Identity); } + + if scheduled { + let others_scheduled = self + .filesystems + .iter() + .filter(|(_, _, f)| { + f.origin().map(|o| o == origin).unwrap_or(false) && f.merge_scheduled() + }) + .collect::>(); + + assert!(others_scheduled.len() < 2); + + if let Some((n, u, _)) = others_scheduled.first() { + return Err(StratisError::Msg(format!( + "Filesystem {:} with UUID {:} is already scheduled to be reverted into origin filesystem {:}", + n, u, origin, + ))); + } + } + + assert!(self + .filesystems + .get_mut_by_uuid(fs_uuid) + .expect("Looked up above") + .1 + .set_merge_scheduled(scheduled) + .expect("fs.origin() is not None")); + + Ok(PropChangeAction::NewValue(scheduled)) } } diff --git a/src/engine/strat_engine/thinpool/filesystem.rs b/src/engine/strat_engine/thinpool/filesystem.rs index 78ebf97164..280f6cccc4 100644 --- a/src/engine/strat_engine/thinpool/filesystem.rs +++ b/src/engine/strat_engine/thinpool/filesystem.rs @@ -460,8 +460,11 @@ impl StratFilesystem { pub fn set_merge_scheduled(&mut self, scheduled: bool) -> StratisResult { if self.merge_scheduled == scheduled { Ok(false) + } else if scheduled && self.origin.is_none() { + Err(StratisError::Msg( + "Filesystem has no origin filesystem; can not schedule a merge".into(), + )) } else { - // TODO: reject if conflict or no origin self.merge_scheduled = scheduled; Ok(true) } diff --git a/src/engine/strat_engine/thinpool/thinpool.rs b/src/engine/strat_engine/thinpool/thinpool.rs index d7471ebbb8..e357a89760 100644 --- a/src/engine/strat_engine/thinpool/thinpool.rs +++ b/src/engine/strat_engine/thinpool/thinpool.rs @@ -1811,24 +1811,59 @@ where } /// Set the filesystem merge scheduled value for filesystem with given UUID. + /// Returns true if the value was changed from the filesystem's, previous + /// value, otherwise false. pub fn set_fs_merge_scheduled( &mut self, fs_uuid: FilesystemUuid, scheduled: bool, ) -> 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.set_merge_scheduled(scheduled)? - }; + let (_, fs) = self + .get_filesystem_by_uuid(fs_uuid) + .ok_or_else(|| StratisError::Msg(format!("No filesystem with UUID {fs_uuid} found")))?; + + let origin = fs.origin().ok_or_else(|| { + StratisError::Msg(format!( + "Filesystem {fs_uuid} has no origin, revert cannot be scheduled or unscheduled" + )) + })?; + + if fs.merge_scheduled() == scheduled { + return Ok(false); + } + + if scheduled { + let others_scheduled = self + .filesystems + .iter() + .filter(|(_, _, f)| { + f.origin().map(|o| o == origin).unwrap_or(false) && f.merge_scheduled() + }) + .collect::>(); + + assert!(others_scheduled.len() < 2); + + if let Some((n, u, _)) = others_scheduled.first() { + return Err(StratisError::Msg(format!( + "Filesystem {:} with UUID {:} is already scheduled to be reverted into origin filesystem {:}", + n, u, origin, + ))); + } + } + + assert!(self + .get_mut_filesystem_by_uuid(fs_uuid) + .expect("Looked up above") + .1 + .set_merge_scheduled(scheduled) + .expect("fs.origin() is not None")); + let (name, fs) = self .get_filesystem_by_uuid(fs_uuid) .expect("Looked up above"); - if changed { - self.mdv.save_fs(&name, fs_uuid, fs)?; - } - Ok(changed) + + self.mdv.save_fs(&name, fs_uuid, fs)?; + Ok(true) } }