diff --git a/src/dbus_api/api/manager_3_2/methods.rs b/src/dbus_api/api/manager_3_2/methods.rs index a341e48c138..6e76396fef9 100644 --- a/src/dbus_api/api/manager_3_2/methods.rs +++ b/src/dbus_api/api/manager_3_2/methods.rs @@ -8,14 +8,12 @@ use futures::executor::block_on; use crate::{ dbus_api::{ - blockdev::create_dbus_blockdev, + api::shared::start_pool_shared, consts, - filesystem::create_dbus_filesystem, - pool::create_dbus_pool, types::{DbusErrorEnum, TData, OK_STRING}, - util::{engine_to_dbus_err_tuple, get_next_arg, tuple_to_option}, + util::{engine_to_dbus_err_tuple, get_next_arg}, }, - engine::{Engine, Pool, PoolIdentifier, PoolUuid, StartAction, StopAction, UnlockMethod}, + engine::{Engine, Pool, PoolIdentifier, PoolUuid, StopAction}, stratis::StratisError, }; @@ -45,84 +43,15 @@ where return Ok(vec![return_message.append3(default_return, rc, rs)]); } }; - let unlock_method = { - let unlock_method_tup: (bool, &str) = get_next_arg(&mut iter, 1)?; - match tuple_to_option(unlock_method_tup) { - Some(unlock_method_str) => match UnlockMethod::try_from(unlock_method_str) { - Ok(um) => Some(um), - Err(e) => { - let (rc, rs) = engine_to_dbus_err_tuple(&e); - return Ok(vec![return_message.append3(default_return, rc, rs)]); - } - }, - None => None, - } - }; - - let ret = match handle_action!(block_on( - dbus_context - .engine - .start_pool(PoolIdentifier::Uuid(pool_uuid), unlock_method) - )) { - Ok(StartAction::Started(_)) => { - let guard = match block_on( - dbus_context - .engine - .get_pool(PoolIdentifier::Uuid(pool_uuid)), - ) { - Some(g) => g, - None => { - let (rc, rs) = engine_to_dbus_err_tuple(&StratisError::Msg( - format!("Pool with UUID {pool_uuid} was successfully started but appears to have been removed before it could be exposed on the D-Bus") - )); - return Ok(vec![return_message.append3(default_return, rc, rs)]); - } - }; - - let (pool_name, _, pool) = guard.as_tuple(); - let pool_path = - create_dbus_pool(dbus_context, base_path.clone(), &pool_name, pool_uuid, pool); - let mut bd_paths = Vec::new(); - for (bd_uuid, tier, bd) in pool.blockdevs() { - bd_paths.push(create_dbus_blockdev( - dbus_context, - pool_path.clone(), - bd_uuid, - tier, - bd, - )); - } - let mut fs_paths = Vec::new(); - for (name, fs_uuid, fs) in pool.filesystems() { - fs_paths.push(create_dbus_filesystem( - dbus_context, - pool_path.clone(), - &pool_name, - &name, - fs_uuid, - fs, - )); - } - - if pool.is_encrypted() { - dbus_context.push_locked_pools(block_on(dbus_context.engine.locked_pools())); - } - dbus_context.push_stopped_pools(block_on(dbus_context.engine.stopped_pools())); - - (true, (pool_path, bd_paths, fs_paths)) - } - Ok(StartAction::Identity) => default_return, - Err(e) => { - let (rc, rs) = engine_to_dbus_err_tuple(&e); - return Ok(vec![return_message.append3(default_return, rc, rs)]); - } - }; - Ok(vec![return_message.append3( - ret, - DbusErrorEnum::OK as u16, - OK_STRING.to_string(), - )]) + start_pool_shared( + PoolIdentifier::Uuid(pool_uuid), + base_path, + &mut iter, + dbus_context, + default_return, + return_message, + ) } pub fn stop_pool(m: &MethodInfo<'_, MTSync>, TData>) -> MethodResult @@ -169,7 +98,11 @@ where }) .unwrap_or(false); - let msg = match handle_action!(block_on(dbus_context.engine.stop_pool(pool_uuid))) { + let msg = match handle_action!(block_on( + dbus_context + .engine + .stop_pool(PoolIdentifier::Uuid(pool_uuid), true) + )) { Ok(StopAction::Stopped(_)) => { dbus_context.push_remove(&pool_path, consts::pool_interface_list()); if send_locked_signal { @@ -182,6 +115,7 @@ where OK_STRING.to_string(), ) } + Ok(StopAction::CleanedUp(_)) => unreachable!("!has_partially_constructed above"), Ok(StopAction::Identity) => return_message.append3( default_return, DbusErrorEnum::OK as u16, diff --git a/src/dbus_api/api/manager_3_4/methods.rs b/src/dbus_api/api/manager_3_4/methods.rs index 9576700b219..9c73404f094 100644 --- a/src/dbus_api/api/manager_3_4/methods.rs +++ b/src/dbus_api/api/manager_3_4/methods.rs @@ -4,17 +4,14 @@ use dbus::{Message, Path}; use dbus_tree::{MTSync, MethodInfo, MethodResult}; -use futures::executor::block_on; use crate::{ dbus_api::{ - blockdev::create_dbus_blockdev, - filesystem::create_dbus_filesystem, - pool::create_dbus_pool, - types::{DbusErrorEnum, TData, OK_STRING}, - util::{engine_to_dbus_err_tuple, get_next_arg, tuple_to_option}, + api::shared::start_pool_shared, + types::TData, + util::{engine_to_dbus_err_tuple, get_next_arg}, }, - engine::{Engine, Name, Pool, PoolIdentifier, PoolUuid, StartAction, UnlockMethod}, + engine::{Engine, Name, PoolIdentifier, PoolUuid}, stratis::StratisError, }; @@ -52,76 +49,13 @@ where } } }; - let unlock_method = { - let unlock_method_tup: (bool, &str) = get_next_arg(&mut iter, 2)?; - match tuple_to_option(unlock_method_tup) { - Some(unlock_method_str) => match UnlockMethod::try_from(unlock_method_str) { - Ok(um) => Some(um), - Err(e) => { - let (rc, rs) = engine_to_dbus_err_tuple(&e); - return Ok(vec![return_message.append3(default_return, rc, rs)]); - } - }, - None => None, - } - }; - - let ret = match handle_action!(block_on( - dbus_context.engine.start_pool(id.clone(), unlock_method) - )) { - Ok(StartAction::Started(_)) => { - let guard = match block_on(dbus_context.engine.get_pool(id.clone())) { - Some(g) => g, - None => { - let (rc, rs) = engine_to_dbus_err_tuple(&StratisError::Msg( - format!("Pool with {id:?} was successfully started but appears to have been removed before it could be exposed on the D-Bus") - )); - return Ok(vec![return_message.append3(default_return, rc, rs)]); - } - }; - - let (pool_name, pool_uuid, pool) = guard.as_tuple(); - let pool_path = - create_dbus_pool(dbus_context, base_path.clone(), &pool_name, pool_uuid, pool); - let mut bd_paths = Vec::new(); - for (bd_uuid, tier, bd) in pool.blockdevs() { - bd_paths.push(create_dbus_blockdev( - dbus_context, - pool_path.clone(), - bd_uuid, - tier, - bd, - )); - } - let mut fs_paths = Vec::new(); - for (name, fs_uuid, fs) in pool.filesystems() { - fs_paths.push(create_dbus_filesystem( - dbus_context, - pool_path.clone(), - &pool_name, - &name, - fs_uuid, - fs, - )); - } - - if pool.is_encrypted() { - dbus_context.push_locked_pools(block_on(dbus_context.engine.locked_pools())); - } - dbus_context.push_stopped_pools(block_on(dbus_context.engine.stopped_pools())); - - (true, (pool_path, bd_paths, fs_paths)) - } - Ok(StartAction::Identity) => default_return, - Err(e) => { - let (rc, rs) = engine_to_dbus_err_tuple(&e); - return Ok(vec![return_message.append3(default_return, rc, rs)]); - } - }; - Ok(vec![return_message.append3( - ret, - DbusErrorEnum::OK as u16, - OK_STRING.to_string(), - )]) + start_pool_shared( + id, + base_path, + &mut iter, + dbus_context, + default_return, + return_message, + ) } diff --git a/src/dbus_api/api/manager_3_6/api.rs b/src/dbus_api/api/manager_3_6/api.rs new file mode 100644 index 00000000000..6ad03cf8651 --- /dev/null +++ b/src/dbus_api/api/manager_3_6/api.rs @@ -0,0 +1,29 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +use dbus_tree::{Factory, MTSync, Method}; + +use crate::{ + dbus_api::{api::manager_3_6::methods::stop_pool, types::TData}, + engine::Engine, +}; + +pub fn stop_pool_method( + f: &Factory>, TData>, +) -> Method>, TData> +where + E: 'static + Engine, +{ + f.method("StopPool", (), stop_pool) + .in_arg(("id", "s")) + .in_arg(("id_type", "s")) + // In order from left to right: + // b: true if the pool was newly stopped + // s: string representation of UUID of stopped pool + // + // Rust representation: (bool, String) + .out_arg(("result", "(bs)")) + .out_arg(("return_code", "q")) + .out_arg(("return_string", "s")) +} diff --git a/src/dbus_api/api/manager_3_6/methods.rs b/src/dbus_api/api/manager_3_6/methods.rs new file mode 100644 index 00000000000..980efc39508 --- /dev/null +++ b/src/dbus_api/api/manager_3_6/methods.rs @@ -0,0 +1,116 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +use dbus::Message; +use dbus_tree::{MTSync, MethodInfo, MethodResult}; +use futures::executor::block_on; + +use crate::{ + dbus_api::{ + consts, + types::{DbusErrorEnum, TData, OK_STRING}, + util::{engine_to_dbus_err_tuple, get_next_arg}, + }, + engine::{Engine, Name, Pool, PoolIdentifier, PoolUuid, StopAction, StratisUuid}, + stratis::StratisError, +}; + +pub fn stop_pool(m: &MethodInfo<'_, MTSync>, TData>) -> MethodResult +where + E: 'static + Engine, +{ + let message: &Message = m.msg; + let mut iter = message.iter_init(); + let dbus_context = m.tree.get_data(); + let default_return = (false, String::new()); + let return_message = message.method_return(); + + let id_str: &str = get_next_arg(&mut iter, 0)?; + let pool_id = { + let id_type_str: &str = get_next_arg(&mut iter, 1)?; + match id_type_str { + "uuid" => match PoolUuid::parse_str(id_str) { + Ok(u) => PoolIdentifier::Uuid(u), + Err(e) => { + let (rc, rs) = engine_to_dbus_err_tuple(&e); + return Ok(vec![return_message.append3(default_return, rc, rs)]); + } + }, + "name" => PoolIdentifier::Name(Name::new(id_str.to_string())), + _ => { + let (rc, rs) = engine_to_dbus_err_tuple(&StratisError::Msg(format!( + "ID type {id_type_str} not recognized" + ))); + return Ok(vec![return_message.append3(default_return, rc, rs)]); + } + } + }; + + // If Some(_), send a locked pool property change signal only if the pool is + // encrypted. If None, the pool may already be stopped or not exist at all. + // Both of these cases are handled by stop_pool and the value we provide + // for send_locked_signal does not matter as send_locked_signal is only + // used when a pool is newly stopped which can only occur if the pool is found + // here. + let send_locked_signal = block_on(dbus_context.engine.get_pool(pool_id.clone())) + .map(|g| { + let (_, _, p) = g.as_tuple(); + p.is_encrypted() + }) + .unwrap_or(false); + + let msg = match handle_action!(block_on(dbus_context.engine.stop_pool(pool_id, true))) { + Ok(StopAction::Stopped(pool_uuid)) => { + match m.tree.iter().find_map(|opath| { + opath + .get_data() + .as_ref() + .and_then(|op_cxt| match op_cxt.uuid { + StratisUuid::Pool(u) => { + if u == pool_uuid { + Some(opath.get_name()) + } else { + None + } + } + StratisUuid::Fs(_) => None, + StratisUuid::Dev(_) => None, + }) + }) { + Some(pool_path) => { + dbus_context.push_remove(pool_path, consts::pool_interface_list()); + if send_locked_signal { + dbus_context + .push_locked_pools(block_on(dbus_context.engine.locked_pools())); + } + dbus_context.push_stopped_pools(block_on(dbus_context.engine.stopped_pools())); + } + None => { + warn!("Could not find pool D-Bus path for the pool that was just stopped"); + } + } + return_message.append3( + (true, uuid_to_string!(pool_uuid)), + DbusErrorEnum::OK as u16, + OK_STRING.to_string(), + ) + } + Ok(StopAction::CleanedUp(pool_uuid)) => return_message.append3( + (true, uuid_to_string!(pool_uuid)), + DbusErrorEnum::OK as u16, + OK_STRING.to_string(), + ), + Ok(StopAction::Identity) => return_message.append3( + default_return, + DbusErrorEnum::OK as u16, + OK_STRING.to_string(), + ), + Err(e) => { + let (rc, rs) = engine_to_dbus_err_tuple(&e); + return Ok(vec![return_message.append3(default_return, rc, rs)]); + } + }; + + Ok(vec![msg]) +} diff --git a/src/dbus_api/api/manager_3_6/mod.rs b/src/dbus_api/api/manager_3_6/mod.rs new file mode 100644 index 00000000000..b50d39ede6f --- /dev/null +++ b/src/dbus_api/api/manager_3_6/mod.rs @@ -0,0 +1,4 @@ +mod api; +mod methods; + +pub use api::stop_pool_method; diff --git a/src/dbus_api/api/mod.rs b/src/dbus_api/api/mod.rs index 6d057aff175..89a71efc1a5 100644 --- a/src/dbus_api/api/mod.rs +++ b/src/dbus_api/api/mod.rs @@ -16,6 +16,7 @@ mod manager_3_0; mod manager_3_2; mod manager_3_4; mod manager_3_5; +mod manager_3_6; pub mod prop_conv; mod report_3_0; mod shared; @@ -135,7 +136,7 @@ where .add_m(manager_3_0::destroy_pool_method(&f)) .add_m(manager_3_0::engine_state_report_method(&f)) .add_m(manager_3_4::start_pool_method(&f)) - .add_m(manager_3_2::stop_pool_method(&f)) + .add_m(manager_3_6::stop_pool_method(&f)) .add_m(manager_3_2::refresh_state_method(&f)) .add_p(manager_3_0::version_property(&f)) .add_p(manager_3_2::stopped_pools_property(&f)), diff --git a/src/dbus_api/api/shared.rs b/src/dbus_api/api/shared.rs index 523748d9fe5..58e114934a0 100644 --- a/src/dbus_api/api/shared.rs +++ b/src/dbus_api/api/shared.rs @@ -4,7 +4,10 @@ use std::collections::HashMap; -use dbus::arg::IterAppend; +use dbus::{ + arg::{Iter, IterAppend}, + Message, Path, +}; use dbus_tree::{ Factory, MTSync, Method, MethodErr, MethodInfo, MethodResult, ObjectPath, PropInfo, Tree, }; @@ -13,13 +16,22 @@ use futures::executor::block_on; use crate::{ dbus_api::{ api::prop_conv::{self, StoppedOrLockedPools}, - blockdev::get_blockdev_properties, - filesystem::get_fs_properties, - pool::get_pool_properties, - types::{GetManagedObjects, InterfacesAddedThreadSafe, TData}, - util::thread_safe_to_dbus_sendable, + blockdev::{create_dbus_blockdev, get_blockdev_properties}, + filesystem::{create_dbus_filesystem, get_fs_properties}, + pool::{create_dbus_pool, get_pool_properties}, + types::{ + DbusContext, DbusErrorEnum, GetManagedObjects, InterfacesAddedThreadSafe, TData, + OK_STRING, + }, + util::{ + engine_to_dbus_err_tuple, get_next_arg, thread_safe_to_dbus_sendable, tuple_to_option, + }, + }, + engine::{ + DevUuid, Engine, FilesystemUuid, Pool, PoolIdentifier, PoolUuid, StartAction, StratisUuid, + Table, UnlockMethod, }, - engine::{DevUuid, Engine, FilesystemUuid, Pool, PoolUuid, StratisUuid, Table}, + stratis::StratisError, }; pub fn get_managed_objects_method( @@ -198,3 +210,91 @@ where { prop_conv::stopped_pools_to_prop(&block_on(e.stopped_pools())) } + +pub fn start_pool_shared( + id: PoolIdentifier, + base_path: &Path<'static>, + iter: &mut Iter<'_>, + dbus_context: &DbusContext, + default_return: ( + bool, + (Path<'static>, Vec>, Vec>), + ), + return_message: Message, +) -> MethodResult +where + E: 'static + Engine, +{ + let unlock_method = { + let unlock_method_tup: (bool, &str) = get_next_arg(iter, 1)?; + match tuple_to_option(unlock_method_tup) { + Some(unlock_method_str) => match UnlockMethod::try_from(unlock_method_str) { + Ok(um) => Some(um), + Err(e) => { + let (rc, rs) = engine_to_dbus_err_tuple(&e); + return Ok(vec![return_message.append3(default_return, rc, rs)]); + } + }, + None => None, + } + }; + + let ret = match handle_action!(block_on( + dbus_context.engine.start_pool(id.clone(), unlock_method) + )) { + Ok(StartAction::Started(_)) => { + let guard = match block_on(dbus_context.engine.get_pool(id.clone())) { + Some(g) => g, + None => { + let (rc, rs) = engine_to_dbus_err_tuple(&StratisError::Msg( + format!("Pool with {id:?} was successfully started but appears to have been removed before it could be exposed on the D-Bus") + )); + return Ok(vec![return_message.append3(default_return, rc, rs)]); + } + }; + + let (pool_name, pool_uuid, pool) = guard.as_tuple(); + let pool_path = + create_dbus_pool(dbus_context, base_path.clone(), &pool_name, pool_uuid, pool); + let mut bd_paths = Vec::new(); + for (bd_uuid, tier, bd) in pool.blockdevs() { + bd_paths.push(create_dbus_blockdev( + dbus_context, + pool_path.clone(), + bd_uuid, + tier, + bd, + )); + } + let mut fs_paths = Vec::new(); + for (name, fs_uuid, fs) in pool.filesystems() { + fs_paths.push(create_dbus_filesystem( + dbus_context, + pool_path.clone(), + &pool_name, + &name, + fs_uuid, + fs, + )); + } + + if pool.is_encrypted() { + dbus_context.push_locked_pools(block_on(dbus_context.engine.locked_pools())); + } + dbus_context.push_stopped_pools(block_on(dbus_context.engine.stopped_pools())); + + (true, (pool_path, bd_paths, fs_paths)) + } + Ok(StartAction::Identity) => default_return, + Err(e) => { + let (rc, rs) = engine_to_dbus_err_tuple(&e); + return Ok(vec![return_message.append3(default_return, rc, rs)]); + } + }; + + Ok(vec![return_message.append3( + ret, + DbusErrorEnum::OK as u16, + OK_STRING.to_string(), + )]) +} diff --git a/src/engine/engine.rs b/src/engine/engine.rs index e5db6b7a233..3f568102497 100644 --- a/src/engine/engine.rs +++ b/src/engine/engine.rs @@ -448,7 +448,11 @@ pub trait Engine: Debug + Report + Send + Sync { /// Stop and tear down a pool, storing the information for it to be started /// again later. - async fn stop_pool(&self, pool_uuid: PoolUuid) -> StratisResult>; + async fn stop_pool( + &self, + pool_id: PoolIdentifier, + has_partially_constructed: bool, + ) -> StratisResult>; /// Refresh the state of all pools and liminal devices. async fn refresh_state(&self) -> StratisResult<()>; diff --git a/src/engine/sim_engine/engine.rs b/src/engine/sim_engine/engine.rs index 4426d2c2015..9778c8c66c1 100644 --- a/src/engine/sim_engine/engine.rs +++ b/src/engine/sim_engine/engine.rs @@ -342,16 +342,34 @@ impl Engine for SimEngine { } } - async fn stop_pool(&self, pool_uuid: PoolUuid) -> StratisResult> { - if self - .stopped_pools - .read() - .await - .get_by_uuid(pool_uuid) - .is_some() - { - Ok(StopAction::Identity) - } else if let Some((name, pool)) = self.pools.write_all().await.remove_by_uuid(pool_uuid) { + async fn stop_pool( + &self, + pool_id: PoolIdentifier, + _: bool, + ) -> StratisResult> { + let is_stopped = match pool_id { + PoolIdentifier::Name(ref n) => self.stopped_pools.read().await.get_by_name(n).is_some(), + PoolIdentifier::Uuid(u) => self.stopped_pools.read().await.get_by_uuid(u).is_some(), + }; + if is_stopped { + return Ok(StopAction::Identity); + } + + let pool_entry = match pool_id { + PoolIdentifier::Name(ref n) => self + .pools + .write_all() + .await + .remove_by_name(n) + .map(|(u, p)| (n.clone(), u, p)), + PoolIdentifier::Uuid(u) => self + .pools + .write_all() + .await + .remove_by_uuid(u) + .map(|(n, p)| (n, u, p)), + }; + if let Some((name, pool_uuid, pool)) = pool_entry { self.stopped_pools .write() .await @@ -359,7 +377,7 @@ impl Engine for SimEngine { Ok(StopAction::Stopped(pool_uuid)) } else { Err(StratisError::Msg(format!( - "Pool with UUID {pool_uuid} was not found and cannot be stopped" + "Pool with ID {pool_id} was not found and cannot be stopped" ))) } } diff --git a/src/engine/strat_engine/engine.rs b/src/engine/strat_engine/engine.rs index bc83ed31893..d226105853a 100644 --- a/src/engine/strat_engine/engine.rs +++ b/src/engine/strat_engine/engine.rs @@ -647,29 +647,49 @@ impl Engine for StratEngine { } } - async fn stop_pool(&self, pool_uuid: PoolUuid) -> StratisResult> { + async fn stop_pool( + &self, + pool_id: PoolIdentifier, + has_partially_constructed: bool, + ) -> StratisResult> { + let id_str = pool_id.to_string(); + + let stopped_pools = self.liminal_devices.read().await.stopped_pools(); + let pool_uuid = match pool_id { + PoolIdentifier::Name(ref n) => stopped_pools.name_to_uuid.get(n), + PoolIdentifier::Uuid(ref u) => Some(u), + }; + if let Some(pool_uuid) = pool_uuid { + if has_partially_constructed { + if stopped_pools.stopped.get(pool_uuid).is_some() { + return Ok(StopAction::Identity); + } else if stopped_pools.partially_constructed.get(pool_uuid).is_some() { + self.liminal_devices + .write() + .await + .stop_partially_constructed_pool(*pool_uuid)?; + return Ok(StopAction::CleanedUp(*pool_uuid)); + } + } else if stopped_pools.stopped.get(pool_uuid).is_some() + || stopped_pools.partially_constructed.get(pool_uuid).is_some() + { + return Ok(StopAction::Identity); + } + } + let mut pools = self.pools.write_all().await; - if let Some((name, pool)) = pools.remove_by_uuid(pool_uuid) { + if let Some((name, pool_uuid, pool)) = match pool_id { + PoolIdentifier::Name(n) => pools.remove_by_name(&n).map(|(u, p)| (n, u, p)), + PoolIdentifier::Uuid(u) => pools.remove_by_uuid(u).map(|(n, p)| (n, u, p)), + } { self.liminal_devices .write() .await .stop_pool(&mut pools, name, pool_uuid, pool)?; - return Ok(StopAction::Stopped(pool_uuid)); - } - - drop(pools); - - let stopped_pools = self.liminal_devices.read().await.stopped_pools(); - if stopped_pools.stopped.get(&pool_uuid).is_some() - || stopped_pools - .partially_constructed - .get(&pool_uuid) - .is_some() - { - Ok(StopAction::Identity) + Ok(StopAction::Stopped(pool_uuid)) } else { Err(StratisError::Msg(format!( - "Pool with UUID {pool_uuid} could not be found and cannot be stopped" + "Pool with UUID {id_str} could not be found and cannot be stopped" ))) } } @@ -928,7 +948,7 @@ mod test { let res = init_res.and_then(|_| needs_clean_up(&mut pool, fail_device, operation)); drop(pool); - test_async!(engine.stop_pool(uuid))?; + test_async!(engine.stop_pool(PoolIdentifier::Uuid(uuid), true))?; res?; test_async!(engine.start_pool(PoolIdentifier::Uuid(uuid), Some(unlock_method)))?; @@ -1346,7 +1366,11 @@ mod test { .unwrap() .changed() .unwrap(); - assert!(test_async!(engine.stop_pool(uuid)).unwrap().is_changed()); + assert!( + test_async!(engine.stop_pool(PoolIdentifier::Uuid(uuid), true)) + .unwrap() + .is_changed() + ); assert_eq!(test_async!(engine.stopped_pools()).stopped.len(), 1); assert_eq!(test_async!(engine.pools()).len(), 0); diff --git a/src/engine/strat_engine/liminal/liminal.rs b/src/engine/strat_engine/liminal/liminal.rs index 4b78bc5e21d..3eb1f8c0c59 100644 --- a/src/engine/strat_engine/liminal/liminal.rs +++ b/src/engine/strat_engine/liminal/liminal.rs @@ -324,6 +324,20 @@ impl LiminalDevices { } } + /// Tear down a partially constructed pool. + pub fn stop_partially_constructed_pool(&mut self, pool_uuid: PoolUuid) -> StratisResult<()> { + if let Some(device_set) = self.partially_constructed_pools.get(&pool_uuid) { + stop_partially_constructed_pool( + pool_uuid, + &device_set + .iter() + .map(|(dev_uuid, _)| *dev_uuid) + .collect::>(), + )?; + } + Ok(()) + } + /// Get a mapping of pool UUIDs from all of the LUKS2 devices that are currently /// locked to their encryption info in the set of pools that are not yet set up. pub fn locked_pools(&self) -> LockedPoolsInfo { diff --git a/src/engine/types/actions.rs b/src/engine/types/actions.rs index c6068d39d51..58f6de7c301 100644 --- a/src/engine/types/actions.rs +++ b/src/engine/types/actions.rs @@ -655,6 +655,7 @@ impl EngineAction for StartAction { pub enum StopAction { Identity, Stopped(T), + CleanedUp(T), } impl Display for StopAction { @@ -664,6 +665,10 @@ impl Display for StopAction { f, "The requested pool is already stopped; no action was taken" ), + StopAction::CleanedUp(uuid) => write!( + f, + "The pool with UUID {uuid} was partially constructed and cleaned up successfully", + ), StopAction::Stopped(uuid) => { write!(f, "The pool with UUID {uuid} was successfully stopped") } diff --git a/src/jsonrpc/server/pool.rs b/src/jsonrpc/server/pool.rs index c697fe1a393..5273d53881c 100644 --- a/src/jsonrpc/server/pool.rs +++ b/src/jsonrpc/server/pool.rs @@ -40,18 +40,7 @@ pub async fn pool_stop(engine: Arc, id: PoolIdentifier) -> Strat where E: Engine, { - let pool_uuid = match id { - PoolIdentifier::Uuid(u) => u, - PoolIdentifier::Name(n) => { - engine - .pools() - .await - .get_by_name(&n) - .ok_or_else(|| StratisError::Msg(format!("Pool with name {n} not found")))? - .0 - } - }; - Ok(engine.stop_pool(pool_uuid).await?.is_changed()) + Ok(engine.stop_pool(id, true).await?.is_changed()) } // stratis-min pool create diff --git a/tests/client-dbus/src/stratisd_client_dbus/_introspect.py b/tests/client-dbus/src/stratisd_client_dbus/_introspect.py index f3681959ca4..841ad7d5624 100644 --- a/tests/client-dbus/src/stratisd_client_dbus/_introspect.py +++ b/tests/client-dbus/src/stratisd_client_dbus/_introspect.py @@ -53,7 +53,8 @@ - + + diff --git a/tests/client-dbus/tests/udev/test_udev.py b/tests/client-dbus/tests/udev/test_udev.py index 4a7e6ec3fc6..a013c934654 100644 --- a/tests/client-dbus/tests/udev/test_udev.py +++ b/tests/client-dbus/tests/udev/test_udev.py @@ -601,12 +601,14 @@ def _simple_stop_test(self): num_devices = 2 device_tokens = self._lb_mgr.create_devices(num_devices) devnodes = self._lb_mgr.device_files(device_tokens) + pool_name1 = random_string(5) + pool_name2 = random_string(5) with OptionalKeyServiceContextManager(): self.wait_for_pools(0) - (_, (pool_object_path, _)) = create_pool(random_string(5), devnodes[:1]) - create_pool(random_string(5), devnodes[1:]) + create_pool(pool_name1, devnodes[:1]) + create_pool(pool_name2, devnodes[1:]) self.wait_for_pools(2) @@ -621,7 +623,10 @@ def _simple_stop_test(self): ((changed, _), exit_code, _) = Manager.Methods.StopPool( get_object(TOP_OBJECT), - {"pool": pool_object_path}, + { + "id": pool_name1, + "id_type": "name", + }, ) self.assertEqual(exit_code, 0) self.assertEqual(changed, True) @@ -684,22 +689,20 @@ def _simple_start_by_name_test(self): ) as key_desc: self.wait_for_pools(0) - (_, (pool_object_path_enc, _)) = create_pool( - "encrypted", devnodes[:2], key_description=key_desc[0] - ) - (_, (pool_object_path, _)) = create_pool("unencrypted", devnodes[2:]) + create_pool("encrypted", devnodes[:2], key_description=key_desc[0]) + create_pool("unencrypted", devnodes[2:]) self.wait_for_pools(2) ((changed, _), exit_code, _) = Manager.Methods.StopPool( get_object(TOP_OBJECT), - {"pool": pool_object_path_enc}, + {"id": "encrypted", "id_type": "name"}, ) self.assertEqual(exit_code, 0) self.assertEqual(changed, True) ((changed, _), exit_code, _) = Manager.Methods.StopPool( get_object(TOP_OBJECT), - {"pool": pool_object_path}, + {"id": "unencrypted", "id_type": "name"}, ) self.assertEqual(exit_code, 0) self.assertEqual(changed, True)