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

Allow to detect devices with the iso9660 file system as optical media #2746

Merged
merged 3 commits into from
Jul 22, 2020

Conversation

poncovka
Copy link
Contributor

@poncovka poncovka commented Jul 21, 2020

Test that the DBus method FindOpticalMedia identifies devices with the iso9660 file
system as optical media, so it is able to find NVDIMM devices with iso9660.

The DBus method GetDevicesToIgnore of the NVDIMM module shouldn't return NVDIMM
devices with the iso9660 file system. They can be used as an installation source.

Protect all devices with the iso9660 file system. It will protect, for example, NVDIMM
devices with the iso9660 file system that can be used only as an installation source
anyway.

Related: rhbz#1856264

@pep8speaks
Copy link

pep8speaks commented Jul 21, 2020

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

Line 24:1: E402 module level import not at top of file

Comment last updated at 2020-07-21 18:03:14 UTC

@poncovka poncovka force-pushed the rhel-8-nvdimm_iso9660 branch 2 times, most recently from fbf8e7f to 9c14d83 Compare July 21, 2020 10:29
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, thanks a lot! :)

@jkonecny12
Copy link
Member

jenkins, test this please

1 similar comment
@jkonecny12
Copy link
Member

jenkins, test this please

Copy link
Member

@jkonecny12 jkonecny12 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. Thanks for your help!

Test that the method identifies devices with the iso9660 file system
as optical media, so it is able to find NVDIMM devices with iso9660.

Related: rhbz#1856264
The DBus method GetDevicesToIgnore of the NVDIMM module shouldn't return NVDIMM
devices with the iso9660 file system. They can be used as an installation source.

Related: rhbz#1856264
It will protect, for example, NVDIMM devices with the iso9660 file system that
can be used only as an installation source anyway.

Related: rhbz#1856264
@poncovka
Copy link
Contributor Author

I have replaced one of the commits with unit tests. The implementation of find_optical_media is correct.

Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

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

Still looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants