Send devnode invalidate signals on pool and filesystem rename by jbaublitz · Pull Request #3988 · stratis-storage/stratisd

jbaublitz · 2026-03-16T17:33:51Z

Closes #3987

Summary by CodeRabbit

Bug Fixes

Pool rename operations now properly emit filesystem notifications to ensure connected components receive updates when pools are renamed.

packit-as-a-service · 2026-03-16T17:58:12Z

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

sudo dnf install -y 'dnf*-command(copr)'
dnf copr enable packit/stratis-storage-stratisd-3988-copr_pull
And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

mulkieran · 2026-03-16T19:30:54Z

Every Cockpit Stratis test is recorded as success.

mulkieran · 2026-03-16T19:45:20Z

@coderabbitai review

coderabbitai · 2026-03-16T19:45:29Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai · 2026-03-16T19:52:46Z

Walkthrough

The changes enhance filesystem rename signal handling by collecting affected filesystem paths during pool rename operations and emitting additional devnode invalidation signals for each filesystem through the DBus layer.

Changes

Cohort / File(s)	Summary
Pool rename method enhancement `src/dbus/pool/pool_3_0/methods.rs`	Modified `set_name_method` to retrieve the renamed pool object, collect filesystem paths by translating filesystem UUIDs via read lock, and pass these paths to the signal handler. Includes warning logging for missing filesystem paths.
DBus signal emission expansion `src/dbus/util.rs`	Updated `send_pool_name_signal` function signature to accept filesystem paths parameter and added loop to emit `devnode_invalidate` signals for each filesystem alongside the pool name change signal.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

#3945: Directly modifies the same files (src/dbus/pool/pool_3_0/methods.rs and src/dbus/util.rs) to implement filesystem devnode invalidation signals on rename operations.

Suggested labels

maintenance

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main changes: sending devnode invalidate signals on both pool and filesystem rename operations.
Linked Issues check	✅ Passed	The PR addresses the primary objective of issue `#3987` by implementing devnode invalidate signal emission on filesystem rename, with additional coverage for pool rename.
Out of Scope Changes check	✅ Passed	All changes are scoped to implementing devnode invalidate signal functionality for pool and filesystem rename operations as specified in the linked issue.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📝 Coding Plan

Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Tip

CodeRabbit can suggest fixes for GitHub Check annotations.

Configure the reviews.tools.github-checks setting to adjust the time to wait for GitHub Checks to complete.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/dbus/pool/pool_3_0/methods.rs`:
- Around line 583-600: The current code abandons sending the pool name signal if
any filesystem path lookup fails; change the filesystem-path collection to be
best-effort so send_pool_name_signal is always called with whatever paths were
found (possibly an empty Vec). In the Some(pool) arm (where pool.filesystems()
is used), replace the filter_map chain with an explicit loop/iteration that for
each (_, fs_uuid, _) calls read_lock.filesystem_get_path(&fs_uuid) and pushes
the path into a mutable Vec when Some(p), and logs the warn when None, then
after the loop call send_pool_name_signal(connection, &p.as_ref(),
fs_paths).await so the name_changed signal is emitted regardless of missing
filesystem lookups.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 55874661-67e3-4ef0-b1ea-06f706b68502

📥 Commits

Reviewing files that changed from the base of the PR and between 6957484 and 6dc53fa.

📒 Files selected for processing (2)

src/dbus/pool/pool_3_0/methods.rs
src/dbus/util.rs

coderabbitai · 2026-03-16T19:52:49Z

src/dbus/pool/pool_3_0/methods.rs

+            let read_lock = manager.read().await;
+            match read_lock.pool_get_path(&pool_uuid) {
+                Some(p) => match engine.get_pool(PoolIdentifier::Uuid(pool_uuid)).await {
+                    Some(pool) => {
+                        let fs_paths = pool.filesystems().into_iter().filter_map(|(_, fs_uuid, _)| match read_lock.filesystem_get_path(&fs_uuid) {
+                                Some(p) => Some(p),
+                                None => {
+                                    warn!("Could not find filesystem path for filesystem with UUID {fs_uuid}; cannot send devnode invalidate signal");
+                                    None
+                                }
+                            })
+                            .collect::<Vec<_>>();
+                        send_pool_name_signal(connection, &p.as_ref(), fs_paths).await;
+                    }
+                    None => {
+                        warn!("Could not find filesystems associated with pool with UUID {uuid}; cannot send devnode invalidate signal");
+                    }
+                },


⚠️ Potential issue | 🟠 Major

Don’t drop the pool rename signal when the filesystem lookup misses.

The warning on Line 598 says only the devnode invalidation is lost, but if Line 585 returns None this branch never calls send_pool_name_signal(), so a successful rename can complete without any pool name_changed emission. Please make filesystem-path collection best-effort and still send the pool signal with an empty fs-path list.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@src/dbus/pool/pool_3_0/methods.rs` around lines 583 - 600, The current code abandons sending the pool name signal if any filesystem path lookup fails; change the filesystem-path collection to be best-effort so send_pool_name_signal is always called with whatever paths were found (possibly an empty Vec). In the Some(pool) arm (where pool.filesystems() is used), replace the filter_map chain with an explicit loop/iteration that for each (_, fs_uuid, _) calls read_lock.filesystem_get_path(&fs_uuid) and pushes the path into a mutable Vec when Some(p), and logs the warn when None, then after the loop call send_pool_name_signal(connection, &p.as_ref(), fs_paths).await so the name_changed signal is emitted regardless of missing filesystem lookups.

mulkieran

@jbaublitz coderabbitai has a point, can you do what it asks?

jbaublitz · 2026-03-18T09:15:29Z

@mulkieran Okay this should be resolved.

Send devnode invalidate signal on filesystem rename

a36be80

jbaublitz added this to the v3.9.0 milestone Mar 16, 2026

jbaublitz requested a review from mulkieran March 16, 2026 17:33

jbaublitz self-assigned this Mar 16, 2026

jbaublitz added the bug label Mar 16, 2026

jbaublitz added this to 2026March Mar 16, 2026

mulkieran moved this to In Review in 2026March Mar 16, 2026

mulkieran approved these changes Mar 16, 2026

View reviewed changes

coderabbitai bot reviewed Mar 16, 2026

View reviewed changes

mulkieran requested changes Mar 16, 2026

View reviewed changes

mulkieran moved this from In Review to In Progress in 2026March Mar 16, 2026

Send devnode invalidate signal on pool rename

86ec8c7

jbaublitz force-pushed the issue-stratisd-3987 branch from 6dc53fa to 86ec8c7 Compare March 18, 2026 09:15

mulkieran merged commit 192bd41 into stratis-storage:master Mar 18, 2026
46 of 47 checks passed

github-project-automation bot moved this from In Review to Done in 2026March Mar 18, 2026

mulkieran moved this from Done to Done(2) in 2026March Mar 23, 2026

mulkieran moved this from Done(2) to Done(3) in 2026March Mar 23, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Send devnode invalidate signals on pool and filesystem rename#3988

Send devnode invalidate signals on pool and filesystem rename#3988
mulkieran merged 2 commits intostratis-storage:masterfrom
jbaublitz:issue-stratisd-3987

jbaublitz commented Mar 16, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

packit-as-a-service bot commented Mar 16, 2026

Uh oh!

mulkieran commented Mar 16, 2026

Uh oh!

mulkieran commented Mar 16, 2026

Uh oh!

coderabbitai bot commented Mar 16, 2026

Uh oh!

coderabbitai bot commented Mar 16, 2026

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot Mar 16, 2026

Uh oh!

mulkieran left a comment

Uh oh!

jbaublitz commented Mar 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jbaublitz commented Mar 16, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Bug Fixes

Uh oh!

packit-as-a-service bot commented Mar 16, 2026

Uh oh!

mulkieran commented Mar 16, 2026

Uh oh!

mulkieran commented Mar 16, 2026

Uh oh!

coderabbitai bot commented Mar 16, 2026

Uh oh!

coderabbitai bot commented Mar 16, 2026

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

mulkieran left a comment

Choose a reason for hiding this comment

Uh oh!

jbaublitz commented Mar 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jbaublitz commented Mar 16, 2026 •

edited by coderabbitai bot

Loading