Skip to content

Commit 3b08852

Browse files
jbaublitzjbaublitz
committed
Simplify pool deregistration to include blockdevs and filesystems
Assisted-by: Claude <noreply@anthropic.com>
1 parent ad22760 commit 3b08852

4 files changed

Lines changed: 114 additions & 137 deletions

File tree

‎src/dbus/manager/manager_3_0/methods.rs‎

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@ use zbus::{
1717

1818
use crate::{
1919
dbus::{
20-
blockdev::unregister_blockdev,
2120
consts::OK_STRING,
22-
filesystem::unregister_filesystem,
2321
manager::Manager,
2422
pool::{register_pool, unregister_pool},
2523
types::DbusErrorEnum,
@@ -196,27 +194,7 @@ pub async fn destroy_pool_method(
196194

197195
match engine.destroy_pool(uuid).await {
198196
Ok(DeleteAction::Deleted(uuid)) => {
199-
for dev_uuid in dev_uuids {
200-
let maybe_dev_path = manager.write().await.blockdev_get_path(&dev_uuid).cloned();
201-
if let Some(dev_path) = maybe_dev_path {
202-
if let Err(e) = unregister_blockdev(connection, manager, &dev_path).await {
203-
warn!(
204-
"Failed to unregister {dev_path} representing blockdev {dev_uuid}: {e}"
205-
);
206-
}
207-
}
208-
}
209-
for fs_uuid in fs_uuids {
210-
let maybe_fs_path = manager.write().await.filesystem_get_path(&fs_uuid).cloned();
211-
if let Some(fs_path) = maybe_fs_path {
212-
if let Err(e) = unregister_filesystem(connection, manager, &fs_path).await {
213-
warn!(
214-
"Failed to unregister {fs_path} representing filesystem {fs_uuid}: {e}"
215-
);
216-
}
217-
}
218-
}
219-
match unregister_pool(connection, manager, &pool).await {
197+
match unregister_pool(connection, manager, &pool, &fs_uuids, &dev_uuids).await {
220198
Ok(u) => {
221199
assert_eq!(uuid, u);
222200
(

‎src/dbus/manager/manager_3_2/methods.rs‎

Lines changed: 38 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ use zbus::{
1515

1616
use crate::{
1717
dbus::{
18-
blockdev::unregister_blockdev,
1918
consts::OK_STRING,
2019
filesystem::unregister_filesystem,
2120
manager::Manager,
@@ -152,61 +151,48 @@ pub async fn stop_pool_method(
152151
.await
153152
);
154153

155-
match action {
156-
Ok(StopAction::Stopped(_) | StopAction::Partial(_)) | Err(_) => {
157-
let (dev_uuids_rem, fs_uuids_rem) =
158-
match engine.get_pool(PoolIdentifier::Uuid(pool_uuid)).await {
159-
Some(p) => (
160-
p.blockdevs()
161-
.into_iter()
162-
.map(|(u, _, _)| u)
163-
.collect::<HashSet<_>>(),
164-
p.filesystems()
165-
.into_iter()
166-
.map(|(_, u, _)| u)
167-
.collect::<HashSet<_>>(),
168-
),
169-
None => (HashSet::default(), HashSet::default()),
170-
};
171-
172-
for fs_uuid in fs_uuids.into_iter().filter(|u| !fs_uuids_rem.contains(u)) {
173-
let maybe_fs_path = manager.write().await.filesystem_get_path(&fs_uuid).cloned();
174-
if let Some(fs_path) = maybe_fs_path {
175-
if let Err(e) = unregister_filesystem(connection, manager, &fs_path).await {
176-
warn!(
177-
"Failed to unregister {fs_path} representing filesystem {fs_uuid}: {e}"
178-
);
179-
}
180-
}
154+
match &action {
155+
Ok(StopAction::Stopped(_) | StopAction::Partial(_)) => {
156+
if let Err(e) = unregister_pool(connection, manager, &pool, &fs_uuids, &dev_uuids).await
157+
{
158+
warn!("Failed to remove pool with path {pool} from the D-Bus: {e}");
181159
}
182-
183-
for dev_uuid in dev_uuids.into_iter().filter(|u| !dev_uuids_rem.contains(u)) {
184-
let maybe_dev_path = manager.write().await.blockdev_get_path(&dev_uuid).cloned();
185-
if let Some(dev_path) = maybe_dev_path {
186-
if let Err(e) = unregister_blockdev(connection, manager, &dev_path).await {
187-
warn!(
188-
"Failed to unregister {dev_path} representing blockdev {dev_uuid}: {e}"
189-
);
160+
send_stopped_pools_signals(connection).await;
161+
let stopped_pools = engine.stopped_pools().await;
162+
let stopped = stopped_pools
163+
.stopped
164+
.get(&pool_uuid)
165+
.or_else(|| stopped_pools.partially_constructed.get(&pool_uuid));
166+
if stopped.map(|s| s.info.is_some()).unwrap_or(false) {
167+
send_locked_pools_signals(connection).await;
168+
}
169+
}
170+
Err(_) => match engine.get_pool(PoolIdentifier::Uuid(pool_uuid)).await {
171+
Some(g) => {
172+
let rem_fs = g
173+
.filesystems()
174+
.into_iter()
175+
.map(|(_, uuid, _)| uuid)
176+
.collect::<HashSet<_>>();
177+
for fs_uuid in fs_uuids.iter().filter(|u| !rem_fs.contains(u)) {
178+
match manager.read().await.filesystem_get_path(fs_uuid).cloned() {
179+
Some(p) => {
180+
if let Err(e) = unregister_filesystem(connection, manager, &p).await {
181+
warn!("Failed to unregister filesystem: {e}");
182+
}
183+
}
184+
None => {
185+
warn!("Could not find filesystem path for UUID {fs_uuid}");
186+
}
190187
}
191188
}
192189
}
193-
}
194-
_ => {}
195-
}
196-
197-
if let Ok(StopAction::Stopped(_) | StopAction::Partial(_)) = action {
198-
if let Err(e) = unregister_pool(connection, manager, &pool).await {
199-
warn!("Failed to remove pool with path {pool} from the D-Bus: {e}");
200-
}
201-
send_stopped_pools_signals(connection).await;
202-
let stopped_pools = engine.stopped_pools().await;
203-
let stopped = stopped_pools
204-
.stopped
205-
.get(&pool_uuid)
206-
.or_else(|| stopped_pools.partially_constructed.get(&pool_uuid));
207-
if stopped.map(|s| s.info.is_some()).unwrap_or(false) {
208-
send_locked_pools_signals(connection).await;
209-
}
190+
None => {
191+
warn!("Failed to find pool with UUID {pool_uuid} even though pool failed to stop");
192+
}
193+
},
194+
Ok(StopAction::Identity) => {}
195+
Ok(StopAction::CleanedUp(_)) => unreachable!("!has_partially_constructed above"),
210196
}
211197

212198
match action {

‎src/dbus/manager/manager_3_6/methods.rs‎

Lines changed: 47 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ use zbus::Connection;
99

1010
use crate::{
1111
dbus::{
12-
blockdev::unregister_blockdev,
1312
consts::OK_STRING,
1413
filesystem::unregister_filesystem,
1514
manager::Manager,
@@ -57,72 +56,61 @@ pub async fn stop_pool_method(
5756

5857
let action = handle_action!(engine.stop_pool(id.clone(), true).await);
5958

60-
match action {
61-
Ok(StopAction::Stopped(_) | StopAction::Partial(_)) | Err(_) => {
62-
let (dev_uuids_rem, fs_uuids_rem) = match engine.get_pool(id).await {
63-
Some(p) => (
64-
p.blockdevs()
65-
.into_iter()
66-
.map(|(u, _, _)| u)
67-
.collect::<HashSet<_>>(),
68-
p.filesystems()
69-
.into_iter()
70-
.map(|(_, u, _)| u)
71-
.collect::<HashSet<_>>(),
72-
),
73-
None => (HashSet::default(), HashSet::default()),
74-
};
75-
76-
for fs_uuid in fs_uuids.into_iter().filter(|u| !fs_uuids_rem.contains(u)) {
77-
let maybe_fs_path = manager.write().await.filesystem_get_path(&fs_uuid).cloned();
78-
if let Some(fs_path) = maybe_fs_path {
79-
if let Err(e) = unregister_filesystem(connection, manager, &fs_path).await {
80-
warn!(
81-
"Failed to unregister {fs_path} representing filesystem {fs_uuid}: {e}"
82-
);
59+
match &action {
60+
Ok(StopAction::Stopped(pool_uuid) | StopAction::Partial(pool_uuid)) => {
61+
let path = manager.read().await.pool_get_path(pool_uuid).cloned();
62+
match path {
63+
Some(pool) => {
64+
if let Err(e) =
65+
unregister_pool(connection, manager, &pool.as_ref(), &fs_uuids, &dev_uuids)
66+
.await
67+
{
68+
warn!("Failed to remove pool with path {pool} from the D-Bus: {e}");
8369
}
8470
}
85-
}
86-
87-
for dev_uuid in dev_uuids.into_iter().filter(|u| !dev_uuids_rem.contains(u)) {
88-
let maybe_dev_path = manager.write().await.blockdev_get_path(&dev_uuid).cloned();
89-
if let Some(dev_path) = maybe_dev_path {
90-
if let Err(e) = unregister_blockdev(connection, manager, &dev_path).await {
91-
warn!(
92-
"Failed to unregister {dev_path} representing blockdev {dev_uuid}: {e}"
93-
);
94-
}
71+
None => {
72+
warn!("Failed to unregister the stopped pool from the D-Bus");
9573
}
74+
};
75+
send_stopped_pools_signals(connection).await;
76+
let stopped = {
77+
let stopped_pools = engine.stopped_pools().await;
78+
stopped_pools
79+
.stopped
80+
.get(pool_uuid)
81+
.or_else(|| stopped_pools.partially_constructed.get(pool_uuid))
82+
.map(|s| s.info.is_some())
83+
.unwrap_or(false)
84+
};
85+
if stopped {
86+
send_locked_pools_signals(connection).await;
9687
}
9788
}
98-
_ => {}
99-
}
100-
101-
if let Ok(StopAction::Stopped(pool_uuid) | StopAction::Partial(pool_uuid)) = action {
102-
let path = manager.read().await.pool_get_path(&pool_uuid).cloned();
103-
match path {
104-
Some(pool) => {
105-
if let Err(e) = unregister_pool(connection, manager, &pool.as_ref()).await {
106-
warn!("Failed to remove pool with path {pool} from the D-Bus: {e}");
89+
Err(_) => match engine.get_pool(id.clone()).await {
90+
Some(g) => {
91+
let rem_fs = g
92+
.filesystems()
93+
.into_iter()
94+
.map(|(_, uuid, _)| uuid)
95+
.collect::<HashSet<_>>();
96+
for fs_uuid in fs_uuids.iter().filter(|u| !rem_fs.contains(u)) {
97+
match manager.read().await.filesystem_get_path(fs_uuid).cloned() {
98+
Some(p) => {
99+
if let Err(e) = unregister_filesystem(connection, manager, &p).await {
100+
warn!("Failed to unregister filesystem: {e}");
101+
}
102+
}
103+
None => {
104+
warn!("Could not find filesystem path for UUID {fs_uuid}");
105+
}
106+
}
107107
}
108108
}
109109
None => {
110-
warn!("Failed to unregister the stopped pool from the D-Bus");
110+
warn!("Failed to find pool with ID {id:?} even though pool failed to stop");
111111
}
112-
};
113-
send_stopped_pools_signals(connection).await;
114-
let stopped = {
115-
let stopped_pools = engine.stopped_pools().await;
116-
stopped_pools
117-
.stopped
118-
.get(&pool_uuid)
119-
.or_else(|| stopped_pools.partially_constructed.get(&pool_uuid))
120-
.map(|s| s.info.is_some())
121-
.unwrap_or(false)
122-
};
123-
if stopped {
124-
send_locked_pools_signals(connection).await;
125-
}
112+
},
113+
Ok(StopAction::Identity | StopAction::CleanedUp(_)) => {}
126114
}
127115

128116
match action {
@@ -131,7 +119,7 @@ pub async fn stop_pool_method(
131119
DbusErrorEnum::OK as u16,
132120
OK_STRING.to_string(),
133121
),
134-
Ok(StopAction::Stopped(pool_uuid)) => (
122+
Ok(StopAction::Stopped(pool_uuid) | StopAction::CleanedUp(pool_uuid)) => (
135123
(true, pool_uuid.simple().to_string()),
136124
DbusErrorEnum::OK as u16,
137125
OK_STRING.to_string(),
@@ -141,7 +129,6 @@ pub async fn stop_pool_method(
141129
DbusErrorEnum::ERROR as u16,
142130
"Pool was stopped, but some component devices were not torn down".to_string(),
143131
),
144-
Ok(StopAction::CleanedUp(_)) => unreachable!("!has_partially_constructed above"),
145132
Err(e) => {
146133
let (rc, rs) = engine_to_dbus_err_tuple(&e);
147134
(default_return, rc, rs)

‎src/dbus/pool/mod.rs‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,11 @@ use tokio::sync::RwLock;
1111
use zbus::{zvariant::ObjectPath, Connection};
1212

1313
use crate::{
14-
dbus::{consts, register_blockdev, register_filesystem, Manager},
15-
engine::{Engine, Lockable, PoolIdentifier, PoolUuid},
14+
dbus::{
15+
blockdev::unregister_blockdev, consts, filesystem::unregister_filesystem,
16+
register_blockdev, register_filesystem, Manager,
17+
},
18+
engine::{DevUuid, Engine, FilesystemUuid, Lockable, PoolIdentifier, PoolUuid},
1619
stratis::{StratisError, StratisResult},
1720
};
1821

@@ -211,7 +214,30 @@ pub async fn unregister_pool(
211214
connection: &Arc<Connection>,
212215
manager: &Lockable<Arc<RwLock<Manager>>>,
213216
path: &ObjectPath<'_>,
217+
fs_uuids: &[FilesystemUuid],
218+
dev_uuids: &[DevUuid],
214219
) -> StratisResult<PoolUuid> {
220+
// Unregister all filesystems
221+
for fs_uuid in fs_uuids {
222+
let maybe_fs_path = manager.write().await.filesystem_get_path(fs_uuid).cloned();
223+
if let Some(fs_path) = maybe_fs_path {
224+
if let Err(e) = unregister_filesystem(connection, manager, &fs_path).await {
225+
warn!("Failed to unregister {fs_path} representing filesystem {fs_uuid}: {e}");
226+
}
227+
}
228+
}
229+
230+
// Unregister all blockdevs
231+
for dev_uuid in dev_uuids {
232+
let maybe_dev_path = manager.write().await.blockdev_get_path(dev_uuid).cloned();
233+
if let Some(dev_path) = maybe_dev_path {
234+
if let Err(e) = unregister_blockdev(connection, manager, &dev_path).await {
235+
warn!("Failed to unregister {dev_path} representing blockdev {dev_uuid}: {e}");
236+
}
237+
}
238+
}
239+
240+
// Unregister the pool itself
215241
let uuid = {
216242
let mut lock = manager.write().await;
217243
let uuid = lock

0 commit comments

Comments
 (0)