skip directories when resolving the nvml library path - #2025
rootkiller6788 wants to merge 1 commit into
Conversation
|
Thanks for your PR, please ensure the following preconditions are met:
|
| @@ -0,0 +1,96 @@ | |||
| /** | |||
| # Copyright 2024 NVIDIA CORPORATION | |||
There was a problem hiding this comment.
| # Copyright 2024 NVIDIA CORPORATION | |
| # Copyright (c) NVIDIA CORPORATION. All rights reserved. |
|
Thanks for the review. The copyright header on the new test file is fixed as suggested — it now reads On the three preconditions: the newest commit carries a Could you please take another look? |
|
DCO check is still failing. Please squash the commit history as well |
tryResolveLibrary returned the first candidate path that EvalSymlinks could resolve, and EvalSymlinks resolves directories just as happily as it resolves files. A directory named libnvidia-ml.so.1 sitting in an earlier search path therefore won over the real library further down the list, and the dlopen that follows failed with a not-found error. Only take a candidate if it actually resolves to a regular file. Same fix as go-nvlib#108, since this file carries its own copy of that code. Adds a test for the resolution helper covering the plain lookup, the shadowing case, a symlinked library (driver roots are full of those) and the fallback to the bare library name. The copyright header on the test file follows the form the rest of the repo uses. Signed-off-by: rootkiller6788 <c8688rickowens@outlook.com>
671ba4c to
9a9ebdd
Compare
|
@rootkiller6788 Your commit signature is unverified. |
This branch has not been deployed
2 participants
root.tryResolveLibraryin this package carries its own copy of the library-discovery code that go-nvlib has inpkg/nvlib/info/root.go, so it has the same bug that go-nvlib#107 reported there: it returns the first candidate thatfilepath.EvalSymlinksresolves, and EvalSymlinks resolves directories just as happily as it resolves files.So a directory named
libnvidia-ml.so.1under/usr/lib64(searched first) wins over the real library in/usr/lib/x86_64-linux-gnu, and thedlopenthat follows fails with a not-found error. Not hypothetical — that is what was reported in kubernetes-sigs/dra-driver-nvidia-gpu#1272, which ships the same copy of this code.The path matters here: the default
--container-driver-rootis/driver-root, not/, so this search loop is live, andmain.go:362hands its result straight tonvml.WithLibraryPath. Get that wrong and the plugin cannot load NVML at all. The same function also feedsnvinfo'sassertHasLibrarywhen the platform is being resolved.The change stats the resolved path and skips candidates that are not regular files, keeping the search going, mirroring NVIDIA/go-nvlib#108 upstream.
The test covers the shadowing case plus the ordinary lookup, a symlinked library (driver roots are mostly symlinks) and the fallback to the bare library name. I checked it fails without the change: the shadowing case returns the directory.
One thing worth flagging: this does not close #1945. That one is about the vendored go-nvlib copy, which still needs a bump once it is settled how (a release or a pseudo-version pin). I deliberately left
go.modandvendor/alone.