Skip to content

Commit 2086d47

Browse files
mulkieranmulkieran
authored
Merge pull request #4034 from jbaublitz/issues-stratisd-4028
Clean up for registration and deregistration code
2 parents 4b17b94 + 3b08852 commit 2086d47

7 files changed

Lines changed: 140 additions & 248 deletions

File tree

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

Lines changed: 3 additions & 25 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,
@@ -135,12 +133,12 @@ pub async fn create_pool_method(
135133
{
136134
Ok(CreateAction::Created(uuid)) => {
137135
match register_pool(engine, connection, manager, counter, uuid).await {
138-
Ok((pool_path, fs_paths)) => (
136+
Ok((pool_path, _, bd_paths)) => (
139137
(
140138
true,
141139
(
142140
OwnedObjectPath::from(pool_path),
143-
fs_paths.into_iter().map(OwnedObjectPath::from).collect(),
141+
bd_paths.into_iter().map(OwnedObjectPath::from).collect(),
144142
),
145143
),
146144
DbusErrorEnum::OK as u16,
@@ -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: 42 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,8 @@ use zbus::{
1515

1616
use crate::{
1717
dbus::{
18-
blockdev::unregister_blockdev,
1918
consts::OK_STRING,
20-
filesystem::{register_filesystem, unregister_filesystem},
19+
filesystem::unregister_filesystem,
2120
manager::Manager,
2221
pool::{register_pool, unregister_pool},
2322
types::DbusErrorEnum,
@@ -83,30 +82,11 @@ pub async fn start_pool_method(
8382
);
8483
}
8584
};
86-
let mut fs_paths = Vec::default();
87-
for fs_uuid in guard
88-
.filesystems()
89-
.into_iter()
90-
.map(|(_, fs_uuid, _)| fs_uuid)
91-
.collect::<Vec<_>>()
92-
{
93-
let fs_path = match register_filesystem(
94-
engine, connection, manager, counter, pool_uuid, fs_uuid,
95-
)
96-
.await
97-
{
98-
Ok(fp) => fp,
99-
Err(e) => {
100-
let (rc, rs) = engine_to_dbus_err_tuple(&e);
101-
return (default_return, rc, rs);
102-
}
103-
};
104-
fs_paths.push(OwnedObjectPath::from(fs_path));
105-
}
106-
let (pool_path, dev_paths) =
85+
let (pool_path, fs_paths, dev_paths) =
10786
match register_pool(engine, connection, manager, counter, pool_uuid).await {
108-
Ok((pp, dp)) => (
87+
Ok((pp, fp, dp)) => (
10988
OwnedObjectPath::from(pp),
89+
fp.into_iter().map(OwnedObjectPath::from).collect(),
11090
dp.into_iter().map(OwnedObjectPath::from).collect(),
11191
),
11292
Err(e) => {
@@ -171,61 +151,48 @@ pub async fn stop_pool_method(
171151
.await
172152
);
173153

174-
match action {
175-
Ok(StopAction::Stopped(_) | StopAction::Partial(_)) | Err(_) => {
176-
let (dev_uuids_rem, fs_uuids_rem) =
177-
match engine.get_pool(PoolIdentifier::Uuid(pool_uuid)).await {
178-
Some(p) => (
179-
p.blockdevs()
180-
.into_iter()
181-
.map(|(u, _, _)| u)
182-
.collect::<HashSet<_>>(),
183-
p.filesystems()
184-
.into_iter()
185-
.map(|(_, u, _)| u)
186-
.collect::<HashSet<_>>(),
187-
),
188-
None => (HashSet::default(), HashSet::default()),
189-
};
190-
191-
for fs_uuid in fs_uuids.into_iter().filter(|u| !fs_uuids_rem.contains(u)) {
192-
let maybe_fs_path = manager.write().await.filesystem_get_path(&fs_uuid).cloned();
193-
if let Some(fs_path) = maybe_fs_path {
194-
if let Err(e) = unregister_filesystem(connection, manager, &fs_path).await {
195-
warn!(
196-
"Failed to unregister {fs_path} representing filesystem {fs_uuid}: {e}"
197-
);
198-
}
199-
}
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}");
200159
}
201-
202-
for dev_uuid in dev_uuids.into_iter().filter(|u| !dev_uuids_rem.contains(u)) {
203-
let maybe_dev_path = manager.write().await.blockdev_get_path(&dev_uuid).cloned();
204-
if let Some(dev_path) = maybe_dev_path {
205-
if let Err(e) = unregister_blockdev(connection, manager, &dev_path).await {
206-
warn!(
207-
"Failed to unregister {dev_path} representing blockdev {dev_uuid}: {e}"
208-
);
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+
}
209187
}
210188
}
211189
}
212-
}
213-
_ => {}
214-
}
215-
216-
if let Ok(StopAction::Stopped(_) | StopAction::Partial(_)) = action {
217-
if let Err(e) = unregister_pool(connection, manager, &pool).await {
218-
warn!("Failed to remove pool with path {pool} from the D-Bus: {e}");
219-
}
220-
send_stopped_pools_signals(connection).await;
221-
let stopped_pools = engine.stopped_pools().await;
222-
let stopped = stopped_pools
223-
.stopped
224-
.get(&pool_uuid)
225-
.or_else(|| stopped_pools.partially_constructed.get(&pool_uuid));
226-
if stopped.map(|s| s.info.is_some()).unwrap_or(false) {
227-
send_locked_pools_signals(connection).await;
228-
}
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"),
229196
}
230197

231198
match action {

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

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ use zbus::{zvariant::OwnedObjectPath, Connection};
1010
use crate::{
1111
dbus::{
1212
consts::OK_STRING,
13-
filesystem::register_filesystem,
1413
manager::Manager,
1514
pool::register_pool,
1615
types::DbusErrorEnum,
@@ -81,30 +80,11 @@ pub async fn start_pool_method(
8180
);
8281
}
8382
};
84-
let mut fs_paths = Vec::default();
85-
for fs_uuid in guard
86-
.filesystems()
87-
.into_iter()
88-
.map(|(_, fs_uuid, _)| fs_uuid)
89-
.collect::<Vec<_>>()
90-
{
91-
let fs_path = match register_filesystem(
92-
engine, connection, manager, counter, pool_uuid, fs_uuid,
93-
)
94-
.await
95-
{
96-
Ok(fp) => fp,
97-
Err(e) => {
98-
let (rc, rs) = engine_to_dbus_err_tuple(&e);
99-
return (default_return, rc, rs);
100-
}
101-
};
102-
fs_paths.push(OwnedObjectPath::from(fs_path));
103-
}
104-
let (pool_path, dev_paths) =
83+
let (pool_path, fs_paths, dev_paths) =
10584
match register_pool(engine, connection, manager, counter, pool_uuid).await {
106-
Ok((pp, dp)) => (
85+
Ok((pp, fp, dp)) => (
10786
OwnedObjectPath::from(pp),
87+
fp.into_iter().map(OwnedObjectPath::from).collect(),
10888
dp.into_iter().map(OwnedObjectPath::from).collect(),
10989
),
11090
Err(e) => {

‎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)

0 commit comments

Comments
 (0)