Skip to content

OCPBUGS-56597: Fix multipath configuration - #1676

Merged
jlebon merged 4 commits into
coreos:mainfrom
ravanelli:pr/multipath_fix
Aug 19, 2025
Merged

OCPBUGS-56597: Fix multipath configuration#1676
jlebon merged 4 commits into
coreos:mainfrom
ravanelli:pr/multipath_fix

Conversation

@ravanelli

Copy link
Copy Markdown
Member
  • To ensure multipath continues working even if a path fails, the WWID must be specified both as a
    kernel argument and in /etc/multipath/wwids.
    Without these, multipath may fail to detect
    devices when paths go down, leading to device
    mapping issues.
  • This PR adds mpath.wwid kernel argument needed for the installer.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary of Changes

Hello @ravanelli, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces new functionality to detect the system's multipath World Wide Identifier (WWID) by querying the multipath -ll command. The stated intent is to ensure this WWID is correctly passed as a kernel argument for robust multipath configuration, although the current implementation focuses on the detection and logging aspect, with the generated kernel argument not yet being integrated into the final kernel command line.

Highlights

  • Multipath WWID Detection Logic: Introduces a new private helper function get_multipath_configured that executes the multipath -ll command, parses its output to extract the first detected World Wide Identifier (WWID), and constructs a kernel argument string in the format mpath.wwid=<WWID>.
  • Integration into Rootmap Process: Calls the newly introduced get_multipath_configured() function within the rootmap function (on line 71). While the get_multipath_configured function generates the mpath.wwid kernel argument, its return value is currently not used to modify the main kargs list in the rootmap function.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

The code changes add a function to retrieve the WWID of a multipath device and add it as a kernel argument. The review suggests improvements to handle multiple WWIDs and return an error if no WWIDs are found.

Comment thread src/bin/rdcore/rootmap.rs Outdated
Comment thread src/bin/rdcore/rootmap.rs Outdated
Comment thread src/bin/rdcore/rootmap.rs
@ravanelli

ravanelli commented Jul 8, 2025

Copy link
Copy Markdown
Member Author

Adding some notes here: RHCOS uses multipath aliases, which generate mpath device names (like mpatha, mpathb, etc.), while FCOS does not and instead shows devices as dm-0. I’m not very familiar with multipath, so I tried to get the first listed device to extract the WWID. I’m not sure if there might be other edge cases to consider when identifying the WWID, if this order would affect it.

Other than that, we still need to finish the fix adding the WWID in the /etc/multipath/wwids via overlay.d "override" for now. And enable the test in kola for it, it needs more testing before getting merged.

Editing, I just realized the formats are different for the output as well, I was using fcos as base, I will fix it:

FCOS:
[root@cosa-devsh core]# multipath -ll
0x48a4930d278dabe9 dm-0 NVME,VirtualMultipath
RHCOS:
[root@cosa-devsh core]# multipath -ll
mpatha (0x794b6a95103d982d) dm-0 NVME,VirtualMultipath

@ravanelli
ravanelli requested a review from jlebon July 8, 2025 17:42
@ravanelli
ravanelli requested review from dustymabe and travier July 8, 2025 19:03
Comment thread src/bin/rdcore/rootmap.rs Outdated
Comment thread src/bin/rdcore/rootmap.rs Outdated
Comment thread src/bin/rdcore/rootmap.rs Outdated
line[start + 1..].split(')').next()
} else {
line.split_whitespace().next()
};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we validate the wwid string is in a format we expect here before we pass it as a karg?

@ravanelli
ravanelli force-pushed the pr/multipath_fix branch 2 times, most recently from c321a0c to 6503cbf Compare July 17, 2025 13:20
Comment thread src/bin/rdcore/rootmap.rs Outdated
Comment thread src/bin/rdcore/rootmap.rs Outdated
Comment thread src/bin/rdcore/rootmap.rs Outdated
Comment thread src/bin/rdcore/rootmap.rs Outdated
Comment thread src/bin/rdcore/rootmap.rs Outdated
Comment thread src/bin/rdcore/rootmap.rs Outdated

@jlebon jlebon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased for conflicts and passed through rustfmt.

Thanks @ravanelli!

@jlebon
jlebon enabled auto-merge (rebase) August 18, 2025 19:35
- To ensure multipath continues working even if a
path fails, the WWID must be specified both as a
kernel argument and in /etc/multipath/wwids.
Without these, multipath may fail to detect
devices when paths go down, leading to device
mapping issues.
- This PR adds mpath.wwid kernel argument needed
  for the installer.

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
jlebon added 3 commits August 18, 2025 20:39
The latter is deprecated.

While we're here, drop the unnecessary quotes for `features`.
The bits that pulled that in (flate2 -> miniz_oxide) in our dependency
tree have updated to adler2 and we no longer even pull in adler.

Update our reference to it in the dev profile.
@jlebon
jlebon merged commit 91c9f7f into coreos:main Aug 19, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants