Merge pull request #3285 from jbaublitz/setup-rollback

Add rollback of Pool setup when starting a pool fails

src/dbus_api/api/prop_conv.rs

@@ -70,6 +70,7 @@ pub fn stopped_pools_to_prop(pools: &StoppedPoolsInfo) -> StoppedOrLockedPools {
     pools
         .stopped
         .iter()
+        .chain(pools.partially_constructed.iter())
         .map(|(u, stopped)| {
             let mut map = HashMap::new();
             if let Some(enc_info) = stopped.info.as_ref() {

src/engine/sim_engine/engine.rs

@@ -84,8 +84,9 @@ impl<'a> Into<Value> for &'a SimEngine {
                         unreachable!("json!() output is always JSON object");
                     }
                 })
-                .collect()
+                .collect(),
             ),
+            "partially_constructed_pools": Value::Array(Vec::new())
         })
     }
 }

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,44 +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<Vec<StratBlockDev>> {
-        let mut devs = 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())?;
-                }
-                Vec::new()
-            }
-        };
-        devs.extend(self.data_tier.block_mgr.teardown()?);
-        Ok(devs)
+    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
@@ -605,6 +584,18 @@ impl Backstore {
             .collect::<HashMap<_, _>>()
     }
 
+    /// Drain the backstore devices into a set of all data and cache devices.
+    pub fn drain_bds(&mut self) -> Vec<StratBlockDev> {
+        let mut bds = self.data_tier.block_mgr.drain_bds();
+        bds.extend(
+            self.cache_tier
+                .as_mut()
+                .map(|ct| ct.block_mgr.drain_bds())
+                .unwrap_or_default(),
+        );
+        bds
+    }
+
     /// Return the device that this tier is currently using.
     /// This changes, depending on whether the backstore is supporting a cache
     /// or not. There may be no device if no data has yet been allocated from
@@ -1241,7 +1232,7 @@ mod tests {
             CacheDevStatus::Fail => panic!("cache is in a failed state"),
         }
 
-        backstore.destroy().unwrap();
+        backstore.destroy(pool_uuid).unwrap();
     }
 
     #[test]
@@ -1322,7 +1313,7 @@ mod tests {
 
         assert_ne!(backstore.device(), old_device);
 
-        backstore.destroy().unwrap();
+        backstore.destroy(pool_uuid).unwrap();
     }
 
     #[test]

src/engine/strat_engine/backstore/blockdevmgr.rs

@@ -81,6 +81,11 @@ impl BlockDevMgr {
         bds_to_bdas(self.block_devs)
     }
 
+    /// Drain the BlockDevMgr block devices into a collection of block devices.
+    pub fn drain_bds(&mut self) -> Vec<StratBlockDev> {
+        self.block_devs.drain(..).collect::<Vec<_>>()
+    }
+
     /// Get a hashmap that maps UUIDs to Devices.
     pub fn uuid_to_devno(&self) -> HashMap<DevUuid, Device> {
         self.block_devs
@@ -399,27 +404,16 @@ impl BlockDevMgr {
     }
 
     /// Tear down devicemapper devices for the block devices in this BlockDevMgr.
-    pub fn teardown(&mut self) -> StratisResult<Vec<StratBlockDev>> {
-        let (oks, errs) = self
-            .block_devs
-            .drain(..)
-            .map(|mut bd| {
-                bd.teardown()?;
-                Ok(bd)
-            })
-            .partition::<Vec<_>, _>(|res| res.is_ok());
-        let (oks, errs) = (
-            oks.into_iter().filter_map(|ok| ok.ok()).collect::<Vec<_>>(),
-            errs.into_iter()
-                .filter_map(|err| match err {
-                    Ok(_) => None,
-                    Err(e) => Some(e),
-                })
-                .collect::<Vec<_>>(),
-        );
+    pub fn teardown(&mut self) -> StratisResult<()> {
+        let errs = self.block_devs.iter_mut().fold(Vec::new(), |mut errs, bd| {
+            if let Err(e) = bd.teardown() {
+                errs.push(e);
+            }
+            errs
+        });
 
         if errs.is_empty() {
-            Ok(oks)
+            Ok(())
         } else {
             Err(StratisError::BestEffortError("Failed to remove devicemapper devices for some or all physical devices in the pool".to_string(), errs))
         }

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

@@ -11,7 +11,7 @@ use std::{
 use either::Either;
 use serde_json::Value;
 
-use devicemapper::{Device, Sectors};
+use devicemapper::{Device, DmName, DmNameBuf, Sectors};
 use libcryptsetup_rs::{
     c_uint,
     consts::{flags::CryptActivate, vals::EncryptionFormat},
@@ -64,7 +64,7 @@ impl CryptHandle {
         physical_path: DevicePath,
         identifiers: StratisIdentifiers,
         encryption_info: EncryptionInfo,
-        activation_name: String,
+        activation_name: DmNameBuf,
         pool_name: Option<Name>,
     ) -> StratisResult<CryptHandle> {
         let device = get_devno_from_path(&physical_path)?;
@@ -82,8 +82,8 @@ impl CryptHandle {
         metadata_handle: CryptMetadataHandle,
     ) -> StratisResult<CryptHandle> {
         let activated_path = DevicePath::new(
-            &once(DEVICEMAPPER_PATH)
-                .chain(once(metadata_handle.activation_name()))
+            &once(DEVICEMAPPER_PATH.to_string())
+                .chain(once(metadata_handle.activation_name().to_string()))
                 .collect::<PathBuf>(),
         )?;
         Ok(CryptHandle {
@@ -138,7 +138,7 @@ impl CryptHandle {
     }
 
     /// Return the name of the activated devicemapper device.
-    pub fn activation_name(&self) -> &str {
+    pub fn activation_name(&self) -> &DmName {
         self.metadata_handle.activation_name()
     }
 
@@ -408,7 +408,7 @@ impl CryptHandle {
         let name = self.activation_name().to_owned();
         let active_device = log_on_failure!(
             self.acquire_crypt_device()?
-                .runtime_handle(&name)
+                .runtime_handle(&name.to_string())
                 .get_active_device(),
             "Failed to get device size for encrypted logical device"
         );
@@ -449,7 +449,7 @@ impl CryptHandle {
         )?;
         crypt
             .context_handle()
-            .resize(self.activation_name(), processed_size)
+            .resize(&self.activation_name().to_string(), processed_size)
             .map_err(StratisError::Crypt)
     }
 }

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

@@ -5,8 +5,9 @@
 use std::path::Path;
 
 use either::Either;
-use serde_json::Value;
+use serde_json::{to_value, Value};
 
+use devicemapper::{DmName, DmNameBuf};
 use libcryptsetup_rs::{
     consts::{
         flags::CryptVolumeKey,
@@ -43,7 +44,7 @@ use crate::{
 pub struct CryptInitializer {
     physical_path: DevicePath,
     identifiers: StratisIdentifiers,
-    activation_name: String,
+    activation_name: DmNameBuf,
 }
 
 impl CryptInitializer {
@@ -236,12 +237,11 @@ impl CryptInitializer {
         log_on_failure!(
             device.token_handle().json_set(TokenInput::ReplaceToken(
                 STRATIS_TOKEN_ID,
-                &StratisLuks2Token {
+                &to_value(&StratisLuks2Token {
                     devname: self.activation_name.clone(),
                     identifiers: self.identifiers,
                     pool_name: Some(pool_name.clone()),
-                }
-                .into(),
+                })?,
             )),
             "Failed to create the Stratis token"
         );
@@ -259,7 +259,7 @@ impl CryptInitializer {
     pub fn rollback(
         device: &mut CryptDevice,
         physical_path: &Path,
-        name: &str,
+        name: &DmName,
     ) -> StratisResult<()> {
         ensure_wiped(device, physical_path, name)
     }

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

@@ -15,44 +15,3 @@ macro_rules! log_on_failure {
         result?
     }}
 }
-
-macro_rules! check_key {
-    ($condition:expr, $key:tt, $value:tt) => {
-        if $condition {
-            return Err($crate::stratis::StratisError::Msg(format!(
-                "Stratis token key '{}' requires a value of '{}'",
-                $key, $value,
-            )));
-        }
-    };
-}
-
-macro_rules! check_and_get_key {
-    ($get:expr, $key:tt) => {
-        if let Some(v) = $get {
-            v
-        } else {
-            return Err($crate::stratis::StratisError::Msg(format!(
-                "Stratis token is missing key '{}' or the value is of the wrong type",
-                $key
-            )));
-        }
-    };
-    ($get:expr, $func:expr, $key:tt, $ty:ty) => {
-        if let Some(ref v) = $get {
-            $func(v).map_err(|e| {
-                $crate::stratis::StratisError::Msg(format!(
-                    "Failed to convert value for key '{}' to type {}: {}",
-                    $key,
-                    stringify!($ty),
-                    e
-                ))
-            })?
-        } else {
-            return Err($crate::stratis::StratisError::Msg(format!(
-                "Stratis token is missing key '{}' or the value is of the wrong type",
-                $key
-            )));
-        }
-    };
-}

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

@@ -4,7 +4,7 @@
 
 use std::path::Path;
 
-use devicemapper::Device;
+use devicemapper::{Device, DmName, DmNameBuf};
 
 use crate::{
     engine::{
@@ -23,7 +23,7 @@ pub struct CryptMetadataHandle {
     pub(super) physical_path: DevicePath,
     pub(super) identifiers: StratisIdentifiers,
     pub(super) encryption_info: EncryptionInfo,
-    pub(super) activation_name: String,
+    pub(super) activation_name: DmNameBuf,
     pub(super) pool_name: Option<Name>,
     pub(super) device: Device,
 }
@@ -33,7 +33,7 @@ impl CryptMetadataHandle {
         physical_path: DevicePath,
         identifiers: StratisIdentifiers,
         encryption_info: EncryptionInfo,
-        activation_name: String,
+        activation_name: DmNameBuf,
         pool_name: Option<Name>,
         device: Device,
     ) -> Self {
@@ -72,7 +72,7 @@ impl CryptMetadataHandle {
     }
 
     /// Get the name of the activated device when it is activated.
-    pub fn activation_name(&self) -> &str {
+    pub fn activation_name(&self) -> &DmName {
         &self.activation_name
     }
 

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

@@ -274,11 +274,11 @@ mod tests {
                 libc::close(fd);
             };
 
-            let device_name = handle.activation_name().to_owned();
+            let device_name = handle.activation_name();
             loop {
                 match libcryptsetup_rs::status(
                     Some(&mut handle.acquire_crypt_device().unwrap()),
-                    &device_name,
+                    &device_name.to_string(),
                 ) {
                     Ok(CryptStatusInfo::Busy) => (),
                     Ok(CryptStatusInfo::Active) => break,