Replace devicemapper-rs high level teardown with name-based teardown

src/engine/strat_engine/backstore/backstore.rs

@@ -26,7 +26,7 @@ use crate::{
                 shared::BlockSizeSummary,
                 transaction::RequestTransaction,
             },
-            dm::get_dm,
+            dm::{get_dm, list_of_backstore_devices, remove_optional_devices},
             metadata::{MDADataSize, BDA},
             names::{format_backstore_ids, CacheRole},
             serde_structs::{BackstoreSave, CapSave, Recordable},
@@ -550,43 +550,23 @@ impl Backstore {
     }
 
     /// Destroy the entire store.
-    pub fn destroy(&mut self) -> StratisResult<()> {
-        match self.cache {
-            Some(ref mut cache) => {
-                cache.teardown(get_dm())?;
-                self.cache_tier
-                    .as_mut()
-                    .expect("if dm_device is cache, cache tier exists")
-                    .destroy()?;
-            }
-            None => {
-                if let Some(ref mut linear) = self.linear {
-                    linear.teardown(get_dm())?;
-                }
-            }
-        };
+    pub fn destroy(&mut self, pool_uuid: PoolUuid) -> StratisResult<()> {
+        let devs = list_of_backstore_devices(pool_uuid);
+        remove_optional_devices(devs)?;
+        if let Some(ref mut cache_tier) = self.cache_tier {
+            cache_tier.destroy()?;
+        }
         self.data_tier.destroy()
     }
 
     /// Teardown the DM devices in the backstore.
-    pub fn teardown(&mut self) -> StratisResult<()> {
-        match self
-            .cache
-            .as_mut()
-            .and_then(|c| self.cache_tier.as_mut().map(|ct| (c, ct)))
-        {
-            Some((cache, cache_tier)) => {
-                cache.teardown(get_dm())?;
-                cache_tier.block_mgr.teardown()?;
-            }
-            None => {
-                if let Some(ref mut linear) = self.linear {
-                    linear.teardown(get_dm())?;
-                }
-            }
-        };
-        self.data_tier.block_mgr.teardown()?;
-        Ok(())
+    pub fn teardown(&mut self, pool_uuid: PoolUuid) -> StratisResult<()> {
+        let devs = list_of_backstore_devices(pool_uuid);
+        remove_optional_devices(devs)?;
+        if let Some(ref mut cache_tier) = self.cache_tier {
+            cache_tier.block_mgr.teardown()?;
+        }
+        self.data_tier.block_mgr.teardown()
     }
 
     /// Consume the backstore and convert it into a set of BDAs representing
@@ -1249,7 +1229,7 @@ mod tests {
             CacheDevStatus::Fail => panic!("cache is in a failed state"),
         }
 
-        backstore.destroy().unwrap();
+        backstore.destroy(pool_uuid).unwrap();
     }
 
     #[test]
@@ -1330,7 +1310,7 @@ mod tests {
 
         assert_ne!(backstore.device(), old_device);
 
-        backstore.destroy().unwrap();
+        backstore.destroy(pool_uuid).unwrap();
     }
 
     #[test]

src/engine/strat_engine/backstore/crypt/shared.rs

@@ -11,7 +11,6 @@ use std::{
 
 use data_encoding::BASE64URL_NOPAD;
 use either::Either;
-use retry::{delay::Fixed, retry_with_index, Error as RetryError};
 use serde::{
     de::{Error, MapAccess, Visitor},
     ser::SerializeMap,
@@ -21,11 +20,11 @@ use serde_json::{from_value, to_value, Map, Value};
 use sha2::{Digest, Sha256};
 use tempfile::TempDir;
 
-use devicemapper::{Bytes, DmName, DmNameBuf};
+use devicemapper::{Bytes, DevId, DmName, DmNameBuf, DmOptions};
 use libcryptsetup_rs::{
     c_uint,
     consts::{
-        flags::{CryptActivate, CryptDeactivate, CryptVolumeKey, CryptWipe},
+        flags::{CryptActivate, CryptVolumeKey, CryptWipe},
         vals::{
             CryptDebugLevel, CryptLogLevel, CryptStatusInfo, CryptWipePattern, EncryptionFormat,
         },
@@ -51,6 +50,7 @@ use crate::{
                 devices::get_devno_from_path,
             },
             cmd::clevis_luks_unlock,
+            dm::get_dm,
             keys,
             metadata::StratisIdentifiers,
         },
@@ -863,31 +863,8 @@ pub fn ensure_inactive(device: &mut CryptDevice, name: &DmName) -> StratisResult
         name
     );
     match status {
-        CryptStatusInfo::Active => {
-            log_on_failure!(
-                device
-                    .activate_handle()
-                    .deactivate(&name.to_string(), CryptDeactivate::empty()),
-                "Failed to deactivate the crypt device with name {}",
-                name
-            );
-        }
-        CryptStatusInfo::Busy => {
-            retry_with_index(Fixed::from_millis(100).take(2), |i| {
-                trace!("Crypt device deactivate attempt {}", i);
-                device
-                    .activate_handle()
-                    .deactivate(&name.to_string(), CryptDeactivate::empty())
-                    .map_err(StratisError::Crypt)
-            })
-            .map_err(|e| match e {
-                RetryError::Internal(s) => StratisError::Chained(
-                    "Retries for crypt device deactivation failed with an internal error"
-                        .to_string(),
-                    Box::new(StratisError::Msg(s)),
-                ),
-                RetryError::Operation { error, .. } => error,
-            })?;
+        CryptStatusInfo::Active | CryptStatusInfo::Busy => {
+            get_dm().device_remove(&DevId::Name(name), DmOptions::default())?;
         }
         _ => (),
     }

src/engine/strat_engine/dm.rs

@@ -10,14 +10,11 @@ use devicemapper::{DevId, DmNameBuf, DmOptions, DmResult, DM};
 
 use crate::{
     engine::{
-        strat_engine::{
-            liminal::DeviceSet,
-            names::{
-                format_backstore_ids, format_crypt_name, format_flex_ids, format_thinpool_ids,
-                CacheRole, FlexRole, ThinPoolRole,
-            },
+        strat_engine::names::{
+            format_backstore_ids, format_crypt_name, format_flex_ids, format_thin_ids,
+            format_thinpool_ids, CacheRole, FlexRole, ThinPoolRole, ThinRole,
         },
-        types::{DevUuid, PoolUuid},
+        types::{DevUuid, FilesystemUuid, PoolUuid},
     },
     stratis::{StratisError, StratisResult},
 };
@@ -48,21 +45,14 @@ pub fn get_dm() -> &'static DM {
     )
 }
 
-pub fn stop_partially_constructed_pool(
-    pool_uuid: PoolUuid,
-    device_set: &DeviceSet,
-) -> StratisResult<bool> {
+pub fn remove_optional_devices(devs: Vec<DmNameBuf>) -> StratisResult<bool> {
     let mut did_something = false;
     let devices = get_dm()
         .list_devices()?
         .into_iter()
         .map(|(name, _, _)| name)
         .collect::<Vec<_>>();
-    let dev_uuids = device_set
-        .iter()
-        .map(|(dev_uuid, _)| *dev_uuid)
-        .collect::<Vec<_>>();
-    for device in list_of_devices(pool_uuid, &dev_uuids) {
+    for device in devs {
         if devices.contains(&device) {
             did_something = true;
             get_dm().device_remove(&DevId::Name(&device), DmOptions::default())?;
@@ -71,11 +61,22 @@ pub fn stop_partially_constructed_pool(
     Ok(did_something)
 }
 
-/// List of device names for removal on partially constructed pool stop. Does not have
-/// filesystem names because partially constructed pools are guaranteed not to have any
-/// active filesystems.
-fn list_of_devices(pool_uuid: PoolUuid, dev_uuids: &[DevUuid]) -> Vec<DmNameBuf> {
+pub fn stop_partially_constructed_pool(
+    pool_uuid: PoolUuid,
+    dev_uuids: &[DevUuid],
+) -> StratisResult<bool> {
+    let devs = list_of_partial_pool_devices(pool_uuid, dev_uuids);
+    remove_optional_devices(devs)
+}
+
+pub fn thin_device(pool_uuid: PoolUuid, fs_uuid: FilesystemUuid) -> DmNameBuf {
+    let (dm_name, _) = format_thin_ids(pool_uuid, ThinRole::Filesystem(fs_uuid));
+    dm_name
+}
+
+pub fn list_of_thin_pool_devices(pool_uuid: PoolUuid) -> Vec<DmNameBuf> {
     let mut devs = Vec::new();
+
     let (thin_pool, _) = format_thinpool_ids(pool_uuid, ThinPoolRole::Pool);
     devs.push(thin_pool);
     let (thin_data, _) = format_flex_ids(pool_uuid, FlexRole::ThinData);
@@ -84,8 +85,18 @@ fn list_of_devices(pool_uuid: PoolUuid, dev_uuids: &[DevUuid]) -> Vec<DmNameBuf>
     devs.push(thin_meta);
     let (thin_meta_spare, _) = format_flex_ids(pool_uuid, FlexRole::ThinMetaSpare);
     devs.push(thin_meta_spare);
+
+    devs
+}
+
+pub fn mdv_device(pool_uuid: PoolUuid) -> DmNameBuf {
     let (thin_mdv, _) = format_flex_ids(pool_uuid, FlexRole::MetadataVolume);
-    devs.push(thin_mdv);
+    thin_mdv
+}
+
+pub fn list_of_backstore_devices(pool_uuid: PoolUuid) -> Vec<DmNameBuf> {
+    let mut devs = Vec::new();
+
     let (cache, _) = format_backstore_ids(pool_uuid, CacheRole::Cache);
     devs.push(cache);
     let (cache_sub, _) = format_backstore_ids(pool_uuid, CacheRole::CacheSub);
@@ -95,6 +106,12 @@ fn list_of_devices(pool_uuid: PoolUuid, dev_uuids: &[DevUuid]) -> Vec<DmNameBuf>
     let (origin, _) = format_backstore_ids(pool_uuid, CacheRole::OriginSub);
     devs.push(origin);
 
+    devs
+}
+
+pub fn list_of_crypt_devices(dev_uuids: &[DevUuid]) -> Vec<DmNameBuf> {
+    let mut devs = Vec::new();
+
     for dev_uuid in dev_uuids.iter() {
         let crypt = format_crypt_name(dev_uuid);
         devs.push(crypt);
@@ -103,10 +120,27 @@ fn list_of_devices(pool_uuid: PoolUuid, dev_uuids: &[DevUuid]) -> Vec<DmNameBuf>
     devs
 }
 
+/// List of device names for removal on partially constructed pool stop. Does not have
+/// filesystem names because partially constructed pools are guaranteed not to have any
+/// active filesystems.
+fn list_of_partial_pool_devices(pool_uuid: PoolUuid, dev_uuids: &[DevUuid]) -> Vec<DmNameBuf> {
+    let mut devs = Vec::new();
+
+    devs.extend(list_of_thin_pool_devices(pool_uuid));
+
+    devs.push(mdv_device(pool_uuid));
+
+    devs.extend(list_of_backstore_devices(pool_uuid));
+
+    devs.extend(list_of_crypt_devices(dev_uuids));
+
+    devs
+}
+
 /// Check whether there are any leftover devicemapper devices from the pool.
 pub fn has_leftover_devices(pool_uuid: PoolUuid, dev_uuids: &[DevUuid]) -> bool {
     let mut has_leftover = false;
-    let devices = list_of_devices(pool_uuid, dev_uuids);
+    let devices = list_of_partial_pool_devices(pool_uuid, dev_uuids);
     match get_dm().list_devices() {
         Ok(l) => {
             let listed_devices = l

src/engine/strat_engine/engine.rs

@@ -233,7 +233,7 @@ impl StratEngine {
         let mut untorndown_pools = Vec::new();
         let mut write_all = block_on(self.pools.write_all());
         for (_, uuid, pool) in write_all.iter_mut() {
-            pool.teardown()
+            pool.teardown(*uuid)
                 .unwrap_or_else(|_| untorndown_pools.push(uuid));
         }
         if untorndown_pools.is_empty() {
@@ -453,7 +453,7 @@ impl Engine for StratEngine {
             .remove_by_uuid(uuid)
             .expect("Must succeed since self.pools.get_by_uuid() returned a value");
 
-        let (res, mut pool) = spawn_blocking!((pool.destroy(), pool))?;
+        let (res, mut pool) = spawn_blocking!((pool.destroy(uuid), pool))?;
         if let Err((err, true)) = res {
             guard.insert(pool_name, uuid, pool);
             Err(err)

src/engine/strat_engine/liminal/device_info.rs

@@ -47,8 +47,13 @@ impl fmt::Display for LLuksInfo {
         write!(
             f,
             "{}, {}, {}",
-            self.dev_info, self.identifiers, self.encryption_info
-        )
+            self.dev_info, self.identifiers, self.encryption_info,
+        )?;
+        if let Some(ref pn) = self.pool_name {
+            write!(f, ", {}", pn)
+        } else {
+            Ok(())
+        }
     }
 }
 
@@ -502,6 +507,15 @@ impl FromIterator<(DevUuid, LInfo)> for DeviceSet {
     }
 }
 
+impl IntoIterator for DeviceSet {
+    type Item = <HashMap<DevUuid, LInfo> as IntoIterator>::Item;
+    type IntoIter = <HashMap<DevUuid, LInfo> as IntoIterator>::IntoIter;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.internal.into_iter()
+    }
+}
+
 impl From<Vec<StratBlockDev>> for DeviceSet {
     fn from(vec: Vec<StratBlockDev>) -> Self {
         vec.into_iter()

src/engine/strat_engine/liminal/liminal.rs

@@ -273,7 +273,7 @@ impl LiminalDevices {
         pool_uuid: PoolUuid,
         mut pool: StratPool,
     ) -> StratisResult<()> {
-        let (devices, err) = match pool.stop(&pool_name) {
+        let (devices, err) = match pool.stop(&pool_name, pool_uuid) {
             Ok(devs) => (devs, None),
             Err((e, true)) => {
                 pools.insert(pool_name, pool_uuid, pool);
@@ -881,9 +881,37 @@ impl LiminalDevices {
                 .map(|(dev_uuid, _)| *dev_uuid)
                 .collect::<Vec<_>>();
             if has_leftover_devices(pool_uuid, &device_uuids) {
-                match stop_partially_constructed_pool(pool_uuid, &device_set) {
+                let dev_uuids = device_set
+                    .iter()
+                    .map(|(dev_uuid, _)| *dev_uuid)
+                    .collect::<Vec<_>>();
+                let res = stop_partially_constructed_pool(pool_uuid, &dev_uuids);
+                let device_set = device_set
+                    .into_iter()
+                    .map(|(dev_uuid, info)| {
+                        (
+                            dev_uuid,
+                            match info {
+                                LInfo::Luks(l) => LInfo::Luks(l),
+                                LInfo::Stratis(mut s) => {
+                                    if let Some(l) = s.luks {
+                                        if !s.dev_info.devnode.exists() {
+                                            LInfo::Luks(l)
+                                        } else {
+                                            s.luks = Some(l);
+                                            LInfo::Stratis(s)
+                                        }
+                                    } else {
+                                        LInfo::Stratis(s)
+                                    }
+                                }
+                            },
+                        )
+                    })
+                    .collect::<DeviceSet>();
+                match res {
                     Ok(_) => {
-                        assert!(!has_leftover_devices(pool_uuid, &device_uuids,));
+                        assert!(!has_leftover_devices(pool_uuid, &device_uuids));
                         self.stopped_pools.insert(pool_uuid, device_set);
                     }
                     Err(_) => {