Reject merge scheduling if invalid

Signed-off-by: mulhern <amulhern@redhat.com>
stratis-storage · Aug 16, 2024 · f468a09 · f468a09
1 parent 34daa5b
commit f468a09
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 7 deletions.
diff --git 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<bool> {
         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
@@ -800,9 +800,39 @@ impl Pool for SimPool {
         fs_uuid: FilesystemUuid,
         schedule: bool,
     ) -> StratisResult<PropChangeAction<bool>> {
-        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(
+                "Filesystem {fs_uuid} has no origin, revert cannot be scheduled".to_string(),
+            )
         })?;
+
+        let others_scheduled = self
+            .filesystems
+            .iter()
+            .filter(|(_, _, f)| {
+                f.origin().map(|o| o == origin).unwrap_or(false) && f.merge_scheduled()
+            })
+            .collect::<Vec<_>>();
+
+        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,
+            )));
+        }
+
+        let (_, fs) = self
+            .filesystems
+            .get_mut_by_uuid(fs_uuid)
+            .expect("Looked up above");
+
         let changed = fs.set_merge_scheduled(schedule)?;
         if changed {
             Ok(PropChangeAction::NewValue(schedule))

diff --git 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<bool> {
         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
@@ -1816,10 +1816,37 @@ where
         fs_uuid: FilesystemUuid,
         scheduled: bool,
     ) -> StratisResult<bool> {
+        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(
+                "Filesystem {fs_uuid} has no origin, revert cannot be scheduled".to_string(),
+            )
+        })?;
+
+        let others_scheduled = self
+            .filesystems
+            .iter()
+            .filter(|(_, _, f)| {
+                f.origin().map(|o| o == origin).unwrap_or(false) && f.merge_scheduled()
+            })
+            .collect::<Vec<_>>();
+
+        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,
+            )));
+        }
+
         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"))
-            })?;
+            let (_, fs) = self
+                .get_mut_filesystem_by_uuid(fs_uuid)
+                .expect("Looked up above");
             fs.set_merge_scheduled(scheduled)?
         };
         let (name, fs) = self