Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a simple NVMe module for NVMe Fabrics support #5328

Merged

Conversation

vojtechtrefny
Copy link
Contributor

For now we want to support only NVMe Fabrics devices that don't require manual configuration so we need to just make sure the config files in /etc/nvme are fully populated before the installer starts and after the installation copied to the installed system. This is covered by calling the 'startup' and 'write' functions of the Blivet's NVMe module.

@vojtechtrefny
Copy link
Contributor Author

This doesn't include any tests, I am not sure what we be good approach to test this, because the NVMe module basically doesn't really do anything other than calling blivet.nvme.write.

@vojtechtrefny vojtechtrefny added the port to RHEL9 The pull request needs to be ported to RHEL 9. label Nov 16, 2023
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

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

I guess we will need some unit tests on this. Shall we find someone to support you on this @vojtechtrefny ?

Update: I just saw your comment. @poncovka do you have some suggestion maybe?

Copy link
Contributor

@poncovka poncovka left a comment

Choose a reason for hiding this comment

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

It needs unit tests, but if you just copy-paste and modify tests of one of the other module, it should be pretty straightforward.

@vojtechtrefny
Copy link
Contributor Author

I've added a simple unit test based on the FCoE test case and rebased the PR to the latest master to fix conflicts with the nvdimm removal change.

Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you!

@vojtechtrefny
Copy link
Contributor Author

/kickstart-test --testtype smoke

For now we want to support only NVMe Fabrics devices that don't
require manual configuration so we need to just make sure the
config files in /etc/nvme are fully populated before the installer
starts and after the installation copied to the installed system.
This is covered by calling the 'startup' and 'write' functions of
the Blivet's NVMe module.
@vojtechtrefny
Copy link
Contributor Author

/kickstart-test --testtype smoke

@vojtechtrefny vojtechtrefny merged commit dcd494b into rhinstaller:master Jan 10, 2024
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f40 port to RHEL9 The pull request needs to be ported to RHEL 9.
Development

Successfully merging this pull request may close these issues.

5 participants