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 #5357

Merged
merged 2 commits into from
Jan 9, 2024

Conversation

vojtechtrefny
Copy link
Contributor

Port of #5328 to RHEL 9. We don't need any additional changes for RHEL, the NVMe module was added to blivet in 3.4.0 (3.6.0 is now required on RHEL 9, we have some additional fixes for the module not yet build for RHEL, but the public startup and write methods are available since 9.1) and the libblockdev NVMe plugin dependency is covered by depending on the libblockdev-plugins-all meta package.

@pep8speaks
Copy link

pep8speaks commented Dec 1, 2023

Hello @vojtechtrefny! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2023-12-12 13:14:51 UTC

@vojtechtrefny
Copy link
Contributor Author

 /usr/lib/python3.9/site-packages/blivet/nvme.py:25: in <module>
    gi.require_version("BlockDev", "3.0")
/usr/lib64/python3.9/site-packages/gi/__init__.py:117: in require_version
    raise ValueError('Namespace %s is already loaded with version %s' %
E   ValueError: Namespace BlockDev is already loaded with version 2.0

This is a bug in blivet on RHEL, we need to fix this before this can be merged, moving the PR to draft for now.

Copy link
Contributor

@M4rtinK M4rtinK left a comment

Choose a reason for hiding this comment

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

Module structure looks good for me. :)

pyanaconda/modules/storage/storage.py Outdated Show resolved Hide resolved
@tbzatek
Copy link
Contributor

tbzatek commented Dec 11, 2023

Okay, with a simple fix of the above and updated blivet, everything works as expected. Tested via updates.img on RHEL-9.4.0-20231209.15.

@jstodola
Copy link
Contributor

Please add the Jira issue to the commit messages: https://github.com/rhinstaller/anaconda/blob/rhel-9/CONTRIBUTING.rst#commits-for-rhel-branches

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.

Resolves: jira#RHEL-10216
Resolves: jira#RHEL-9310
Resolves: jira#RHEL-11543
Resolves: jira#RHEL-11544
Related: jira#RHEL-10216
Related: jira#RHEL-9310
Related: jira#RHEL-11543
Related: jira#RHEL-11544
@vojtechtrefny vojtechtrefny marked this pull request as ready for review January 3, 2024 13:48
@vojtechtrefny
Copy link
Contributor Author

Marking as ready for review, the fixed version of blivet is now available in RHEL 9.

@vojtechtrefny
Copy link
Contributor Author

/kickstart-test --testtype smoke

Copy link
Contributor

@M4rtinK M4rtinK 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. :)

@M4rtinK M4rtinK added the ready to merge The PR can be merged. It should have all BZ flags required for releasing set (usually release+). label Jan 9, 2024
@M4rtinK M4rtinK merged commit 444bb05 into rhinstaller:rhel-9 Jan 9, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge The PR can be merged. It should have all BZ flags required for releasing set (usually release+). rhel-9
Development

Successfully merging this pull request may close these issues.

5 participants