Patch up origin fields if deleted filesystem was an origin

Make this change visible on the D-Bus. Signed-off-by: mulhern <amulhern@redhat.com>
stratis-storage · Sep 10, 2024 · 79c6bfd · 79c6bfd
1 parent 325f480
commit 79c6bfd
Show file tree

Hide file tree

Showing 8 changed files with 37 additions and 24 deletions.
diff --git a/src/dbus_api/pool/pool_3_7/methods.rs b/src/dbus_api/pool/pool_3_7/methods.rs
@@ -74,16 +74,16 @@ pub fn destroy_filesystems(m: &MethodInfo<'_, MTSync<TData>, TData>) -> MethodRe
                         dbus_context.push_remove(op, filesystem_interface_list());
                     }
 
-                    for sn_op in m.tree.iter().filter(|op| {
-                        op.get_data()
-                            .as_ref()
-                            .map(|data| match data.uuid {
-                                StratisUuid::Fs(uuid) => updated_uuids.contains(&uuid),
-                                _ => false,
-                            })
-                            .unwrap_or(false)
+                    for (sn_op, origin) in m.tree.iter().filter_map(|op| {
+                        op.get_data().as_ref().and_then(|data| match data.uuid {
+                            StratisUuid::Fs(uuid) => updated_uuids
+                                .iter()
+                                .find(|(u, _)| *u == uuid)
+                                .map(|(_, origin)| (op, *origin)),
+                            _ => None,
+                        })
                     }) {
-                        dbus_context.push_filesystem_origin_change(sn_op.get_name(), None);
+                        dbus_context.push_filesystem_origin_change(sn_op.get_name(), origin);
                     }
 
                     changed_uuids

diff --git a/src/engine/engine.rs b/src/engine/engine.rs
@@ -212,7 +212,7 @@ pub trait Pool: Debug + Send + Sync {
         &mut self,
         pool_name: &str,
         fs_uuids: &HashSet<FilesystemUuid>,
-    ) -> StratisResult<SetDeleteAction<FilesystemUuid, FilesystemUuid>>;
+    ) -> StratisResult<SetDeleteAction<FilesystemUuid, (FilesystemUuid, Option<FilesystemUuid>)>>;
 
     /// Rename filesystem
     /// Rename pool with uuid to new_name.

diff --git a/src/engine/sim_engine/pool.rs b/src/engine/sim_engine/pool.rs
@@ -481,7 +481,8 @@ impl Pool for SimPool {
         &mut self,
         _pool_name: &str,
         fs_uuids: &HashSet<FilesystemUuid>,
-    ) -> StratisResult<SetDeleteAction<FilesystemUuid, FilesystemUuid>> {
+    ) -> StratisResult<SetDeleteAction<FilesystemUuid, (FilesystemUuid, Option<FilesystemUuid>)>>
+    {
         let mut snapshots = self
             .filesystems()
             .iter()
@@ -512,7 +513,11 @@ impl Pool for SimPool {
 
         let (mut removed, mut updated_origins) = (Vec::new(), Vec::new());
         for &uuid in fs_uuids {
-            if self.filesystems.remove_by_uuid(uuid).is_some() {
+            if let Some((_, fs)) = self.get_filesystem(uuid) {
+                let fs_origin = fs.origin();
+                self.filesystems
+                    .remove_by_uuid(uuid)
+                    .expect("just looked up");
                 removed.push(uuid);
 
                 for (sn_uuid, _) in snapshots.remove(&uuid).unwrap_or_else(Vec::new) {
@@ -521,10 +526,10 @@ impl Pool for SimPool {
                     // removal.
                     if let Some((_, sn)) = self.filesystems.get_mut_by_uuid(sn_uuid) {
                         assert!(
-                            sn.set_origin(None),
-                            "A snapshot can only have one origin, so it can be in snapshots.values() only once, so its origin value can be unset only once"
+                            sn.set_origin(fs_origin),
+                            "A snapshot can only have one origin, so it can be in snapshots.values() only once, so its origin value can be set only once"
                         );
-                        updated_origins.push(sn_uuid);
+                        updated_origins.push((sn_uuid, fs_origin));
                     };
                 }
             }

diff --git a/src/engine/strat_engine/pool/dispatch.rs b/src/engine/strat_engine/pool/dispatch.rs
@@ -123,7 +123,8 @@ impl Pool for AnyPool {
         &mut self,
         pool_name: &str,
         fs_uuids: &HashSet<FilesystemUuid>,
-    ) -> StratisResult<SetDeleteAction<FilesystemUuid, FilesystemUuid>> {
+    ) -> StratisResult<SetDeleteAction<FilesystemUuid, (FilesystemUuid, Option<FilesystemUuid>)>>
+    {
         match self {
             AnyPool::V1(p) => p.destroy_filesystems(pool_name, fs_uuids),
             AnyPool::V2(p) => p.destroy_filesystems(pool_name, fs_uuids),

diff --git a/src/engine/strat_engine/pool/v1.rs b/src/engine/strat_engine/pool/v1.rs
@@ -1022,7 +1022,8 @@ impl Pool for StratPool {
         &mut self,
         pool_name: &str,
         fs_uuids: &HashSet<FilesystemUuid>,
-    ) -> StratisResult<SetDeleteAction<FilesystemUuid, FilesystemUuid>> {
+    ) -> StratisResult<SetDeleteAction<FilesystemUuid, (FilesystemUuid, Option<FilesystemUuid>)>>
+    {
         self.thin_pool.destroy_filesystems(pool_name, fs_uuids)
     }
 

diff --git a/src/engine/strat_engine/pool/v2.rs b/src/engine/strat_engine/pool/v2.rs
@@ -927,7 +927,8 @@ impl Pool for StratPool {
         &mut self,
         pool_name: &str,
         fs_uuids: &HashSet<FilesystemUuid>,
-    ) -> StratisResult<SetDeleteAction<FilesystemUuid, FilesystemUuid>> {
+    ) -> StratisResult<SetDeleteAction<FilesystemUuid, (FilesystemUuid, Option<FilesystemUuid>)>>
+    {
         self.thin_pool.destroy_filesystems(pool_name, fs_uuids)
     }
 

diff --git a/src/engine/strat_engine/thinpool/thinpool.rs b/src/engine/strat_engine/thinpool/thinpool.rs
@@ -774,7 +774,8 @@ impl<B> ThinPool<B> {
         &mut self,
         pool_name: &str,
         fs_uuids: &HashSet<FilesystemUuid>,
-    ) -> StratisResult<SetDeleteAction<FilesystemUuid, FilesystemUuid>> {
+    ) -> StratisResult<SetDeleteAction<FilesystemUuid, (FilesystemUuid, Option<FilesystemUuid>)>>
+    {
         let to_be_merged = fs_uuids
             .iter()
             .filter(|u| {
@@ -819,7 +820,11 @@ impl<B> ThinPool<B> {
 
         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)? {
+            if let Some((_, fs)) = self.get_filesystem_by_uuid(uuid) {
+                let fs_origin = fs.origin();
+                let uuid = self
+                    .destroy_filesystem(pool_name, uuid)?
+                    .expect("just looked up");
                 removed.push(uuid);
 
                 for (sn_uuid, _) in snapshots.remove(&uuid).unwrap_or_else(Vec::new) {
@@ -828,10 +833,10 @@ impl<B> ThinPool<B> {
                     // removal.
                     if let Some((_, sn)) = self.get_mut_filesystem_by_uuid(sn_uuid) {
                         assert!(
-                            sn.set_origin(None),
-                            "A snapshot can only have one origin, so it can be in snapshots.values() only once, so its origin value can be unset only once"
+                            sn.set_origin(fs_origin),
+                            "A snapshot can only have one origin, so it can be in snapshots.values() only once, so its origin value can be set only once"
                         );
-                        updated_origins.push(sn_uuid);
+                        updated_origins.push((sn_uuid, fs_origin));
 
                         let (name, sn) = self.get_filesystem_by_uuid(sn_uuid).expect("just got");
                         self.mdv.save_fs(&name, sn_uuid, sn)?;

diff --git a/src/engine/types/actions.rs b/src/engine/types/actions.rs
@@ -612,7 +612,7 @@ impl<T, U> EngineAction for SetDeleteAction<T, U> {
     }
 }
 
-impl Display for SetDeleteAction<FilesystemUuid, FilesystemUuid> {
+impl Display for SetDeleteAction<FilesystemUuid, (FilesystemUuid, Option<FilesystemUuid>)> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         if self.changed.is_empty() {
             write!(