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

Skip graphics modifier in CSV mode #874

Merged
merged 2 commits into from
Jan 22, 2025
Merged

Conversation

elezar
Copy link
Member

@elezar elezar commented Jan 22, 2025

In CSV mode the CSV files at /etc/nvidia-container-runtime/host-files-for-container.d/ should be the source of truth for container modifications. This change skips graphics modifications to a container. This prevents conflicts when handling files such as vulkan icd files which are already defined in the CSV file.

Applying both modifications leads to errors such as:

time="2025-01-21T18:55:11Z" level=info msg="Symlinking /var/lib/docker/overlay2/8592b399d9308cdbef42010765f81af3f921ed86aa07c7e686b59f1e72fc3e2d/merged/etc/vulkan/icd.d/nvidia_icd.json to /usr/lib/aarch64-linux-gnu/nvidia/nvidia_icd.json"
time="2025-01-21T18:55:11Z" level=error msg="failed to create link [/usr/lib/aarch64-linux-gnu/nvidia/nvidia_icd.json /etc/vulkan/icd.d/nvidia_icd.json]: failed to create symlink: failed to remove existing file: remove /var/lib/docker/overlay2/8592b399d9308cdbef42010765f81af3f921ed86aa07c7e686b59f1e72fc3e2d/merged/etc/vulkan/icd.d/nvidia_icd.json: device or resource busy": unknown.

@elezar elezar added the must-backport The changes in PR need to be backported to at least one stable release branch. label Jan 22, 2025
@elezar elezar self-assigned this Jan 22, 2025
@elezar elezar requested a review from cdesiniotis January 22, 2025 09:30
@klueska
Copy link
Contributor

klueska commented Jan 22, 2025

Seems reasonable, albeit a little hacky

@elezar elezar force-pushed the skip-graphics-for-csv branch from fd99379 to fea991f Compare January 22, 2025 12:52
@elezar
Copy link
Member Author

elezar commented Jan 22, 2025

Seems reasonable, albeit a little hacky

I have refactored this a bit based on our discussions. Note that the intent here is to have CSV mode in the NVIDIA Container Runtime behave the same way as what CDI mode would if the spec was generated from the CSV files. This is not currently the case due to the "graphics" modifiers being triggered when NVIDIA_DRIVER_CAPABILITIES includes graphics.

I understand that we don't want to "silently" ignore options that the user passes, but since this is already the case in CDI mode, I feel that doing the same in the case of CSV mode is justified.

In CSV mode the CSV files at /etc/nvidia-container-runtime/host-files-for-container.d/
should be the source of truth for container modifications. This change skips graphics
modifications to a container. This prevents conflicts when handling files such as
vulkan icd files which are already defined in the CSV file.

Signed-off-by: Evan Lezar <[email protected]>
@elezar elezar force-pushed the skip-graphics-for-csv branch from fea991f to 991b9c2 Compare January 22, 2025 12:58
@klueska
Copy link
Contributor

klueska commented Jan 22, 2025

Thanks for the rework. It reads much better now.

@elezar elezar merged commit 6375e83 into NVIDIA:main Jan 22, 2025
10 checks passed
@elezar elezar deleted the skip-graphics-for-csv branch January 22, 2025 13:37
@elezar elezar mentioned this pull request Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
must-backport The changes in PR need to be backported to at least one stable release branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants