Change destroy_filesystems to work with merging

Move all the functionality into thinpool implementation. Do more checks to avoid deletings filesystems that shouldn't be deleted. Check for situations where multiple snapshots are referring to the same deleted filesystem. Remove a now unused function. Signed-off-by: mulhern <[email protected]>
stratis-storage · Aug 16, 2024 · 34daa5b · 34daa5b
1 parent eb9e90b
commit 34daa5b
Show file tree

Hide file tree

Showing 4 changed files with 271 additions and 103 deletions.
diff --git 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,50 @@ impl Pool for SimPool {
         _pool_name: &str,
         fs_uuids: &HashSet<FilesystemUuid>,
     ) -> StratisResult<SetDeleteAction<FilesystemUuid, FilesystemUuid>> {
-        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<FilesystemUuid> = self
-            .filesystems_mut()
-            .iter_mut()
+        let mut 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::<Vec<_>>();
+        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::<Vec<_>>().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);
+
+                for (sn_uuid, _) in snapshots.remove(&uuid).unwrap_or_else(Vec::new) {
+                    // The filesystems may have been removed; any one of
+                    // them may also be a filesystem that was scheduled for
+                    // removal.
+                    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
@@ -1023,36 +1023,7 @@ impl Pool for StratPool {
         pool_name: &str,
         fs_uuids: &HashSet<FilesystemUuid>,
     ) -> StratisResult<SetDeleteAction<FilesystemUuid, FilesystemUuid>> {
-        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<FilesystemUuid> = 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
@@ -928,36 +928,7 @@ impl Pool for StratPool {
         pool_name: &str,
         fs_uuids: &HashSet<FilesystemUuid>,
     ) -> StratisResult<SetDeleteAction<FilesystemUuid, FilesystemUuid>> {
-        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<FilesystemUuid> = 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")]