fix(node-disk-manager): support nvme virtual paths #678
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dborman-hpe
force-pushed
the
nvme-alternate-paths
branch
from
e10254c
to
13d7aa2
Compare
|
There was one more place where it was only looking for |
akhilerm
suggested changes
Aug 16, 2022
dborman-hpe
force-pushed
the
nvme-alternate-paths
branch
from
2e28e00
to
281573a
Compare
|
Thanks for the review @akhilerm. I think I have addressed all your comments. In addition, I made a fix for If you are happy with these changes, I will squash things back down to a single commit. |
Enhance getParentBlockDevice() by looking not only for /nvme/ in the path, but also /nvme-subystem/, which is the name under /sys/devices/virtual on OpenSUSE 15.3. Clean up the osdiskexcludefilter.go Start() routine so that for each mount point it will first check /proc/1/mounts and if that fails, then check /proc/self/mounts, and then if that also fails, finally log an error message. The observation is that things will be found in one or the other, but not both, so the old code would print out an error message in once case, even when the mountpoint had been added to the filter list by the other case. Change "make test" so that it will run all the go tests and not stop at the first failure. Signed-off-by: David Borman <77121405+dborman-hpe@users.noreply.github.com>
dborman-hpe
force-pushed
the
nvme-alternate-paths
branch
from
281573a
to
5d3fb53
Compare
akhilerm
approved these changes
Aug 24, 2022
Here's the output I get from |
|
So, https://github.com/openebs/node-disk-manager/blob/develop/api-service/node/services/listBlockDeviceDetails_test.go#L30-L33 is hardcoded to Second, https://github.com/openebs/node-disk-manager/blob/683879d938b72464361e595171a9287d0f93fe4d/pkg/udev/mockdata.go#L117 is going to find the root disk, which in my case is an NVMe disk, and that leads to assumptions failing, like |
|
ok. got it. thank you. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Enhance getParentBlockDevice() by looking not only
for /nvme/ in the path, but also /nvme-subystem/,
which is the name under /sys/devices/virtual on
OpenSUSE 15.3.
Also clean up the osdiskexcludefilter.go Start()
routine so that for each mount point it will first
check /proc/1/mounts and if that fails, then check
/proc/self/mounts, and then if that also fails, finally
log an error message. The observation is that things
will be found in one or the other, but not both, so
the old code would print out an error message in one
case, even when the mountpoint had been added to the
filter list by the other case.
Signed-off-by: David Borman 77121405+dborman-hpe@users.noreply.github.com
Pull Request template
Why is this PR required? What issue does it fix?:
NDM failed to filter out nvme disks on OpenSUSE 15.3 due a difference in the path name
What this PR does?:
Adds looking for
nvme-subsystemin addition tonvmeDoes this PR require any upgrade changes?:
No
If the changes in this PR are manually verified, list down the scenarios covered::
I have one mount point to be filtered out,
/mnt/openebs-localwhich is mounted on/dev/nvme0n1p1I looked at the logs, and now see lines like:
whereas before it didn't have that regex filter.
For the logs on creating the filters, it now has:
whereas before it had:
Any additional information for your reviewer? :
No
Checklist:
<type>(<scope>): <subject>