Skip to content

Changes required for WSL2 - #1180

Merged
cdesiniotis merged 2 commits into
mainfrom
wsl
Jan 14, 2025
Merged

Changes required for WSL2#1180
cdesiniotis merged 2 commits into
mainfrom
wsl

Conversation

@cdesiniotis

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread validator/main.go Outdated

func validateHostDriver(silent bool) error {
log.Info("Attempting to validate a pre-installed driver on the host")
if fileInfo, err := os.Lstat("/host/usr/lib/wsl/lib/nvidia-smi"); err == nil && fileInfo.Size() != 0 {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does it make sense to use the info package from go-nvlib here instead (for example https://github.com/NVIDIA/k8s-device-plugin/blob/44b263c37e13370f857ef0691b8ab8ff02135c0d/internal/plugin/factory.go#L129).

This will allow for consistent detection across components.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree with the overall notion of aligning the detection across components, so I have added a commit to use the info package as you have suggested.

However, the usage of the platform resolver in the driver-validator does not work as expected and returns unknown as the platform. This is because the libdxcore.so library is not in the ldcache, and likewise, neither is libnvidia-ml.so. The platform resolver works in the device-plugin because the NVIDIA Container Toolkit takes care of injecting the libdxcore.so library into the container and updating the ldcache.

Let me know if you have any suggestions to get the platform detection working in the driver-validator, otherwise I will revert the latest commit I added in favor of the original logic.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking at the implementiong in the info package, I don't think there's a way to get this working with the current implementation. I think for the time being, let's revert the proposed change and stick with the existing implementation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have reverted the commit.

Signed-off-by: Christopher Desiniotis <cdesiniotis@nvidia.com>
This change is motivated by WSL2 where the /sys/class/dmi directory does not exist.
Mount all of /sys as read-only is a safe approach to account for systems where this
directory does not exist.

Signed-off-by: Christopher Desiniotis <cdesiniotis@nvidia.com>
Comment thread validator/main.go
@cdesiniotis

Copy link
Copy Markdown
Contributor Author

@tariq1890 PTAL

@cdesiniotis
cdesiniotis merged commit 13b9b0d into main Jan 14, 2025
@tariq1890
tariq1890 deleted the wsl branch January 14, 2025 22:50
@cdesiniotis cdesiniotis mentioned this pull request Nov 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants