fix: Provision /dev/nvidia-modeset in both display and graphics - #388
fix: Provision /dev/nvidia-modeset in both display and graphics#388ehfd wants to merge 1 commit into
/dev/nvidia-modeset in both display and graphics#388Conversation
There was a problem hiding this comment.
Pull request overview
This PR aligns nvidia-container-cli’s device injection behavior with the NVIDIA Container Toolkit’s OCI-level semantics by ensuring /dev/nvidia-modeset is mounted for containers that request either the display or graphics capability (previously only display).
Changes:
- Extend the modeset-device gating check in
nvc_driver_mount()to allow provisioning whenOPT_GRAPHICS_LIBSis set, not onlyOPT_DISPLAY. - Update the in-code comment to reflect the expanded gating condition.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
capabilities Signed-off-by: Seungmin Kim <8457324+ehfd@users.noreply.github.com>
|
/ok to test 43ec8ca |
| if ((has_modeset = find_device_node(err, root, NV_MODESET_DEVICE_PATH, &modeset)) < 0) | ||
| return (-1); |
There was a problem hiding this comment.
what's the rational for this change? I'm not sure if it's necessary for the purpose of this PR.
There was a problem hiding this comment.
The old code hard-coded the modeset entry: modeset.id = makedev(NV_DEVICE_MAJOR, NV_MODESET_DEVICE_MINOR); has_modeset = 1; so /dev/nvidia-modeset was listed unconditionally, even when the node doesn't exist on the host (i.e. the nvidia-modeset kernel module isn't loaded). Using find_device_node makes the entry conditional on the device actually being present: it stats the node and warn-and-skips on ENOENT instead of registering a device that would later fail to mount, and it records the real st_rdev rather than assuming the fixed major/minor. It also matches how nvidia-uvm and nvidia-uvm-tools are discovered in the same function, just above.
So, it's a sanity fix that closes a real source of failure.
/dev/nvidia-modeset is a userspace-created device, like /dev/nvidia-uvm or /dev/nvidia-uvm-tools. It should work identically.
There was a problem hiding this comment.
Full code around the edit location:
else {
if (!(flags & OPT_NO_UVM)) {
if ((has_uvm = find_device_node(err, root, NV_UVM_DEVICE_PATH, &uvm)) < 0)
return (-1);
if ((has_uvm_tools = find_device_node(err, root, NV_UVM_TOOLS_DEVICE_PATH, &uvm_tools)) < 0)
return (-1);
}
if (!(flags & OPT_NO_MODESET)) {
if ((has_modeset = find_device_node(err, root, NV_MODESET_DEVICE_PATH, &modeset)) < 0)
return (-1);
}
nvidiactl.path = (char *)NV_CTL_DEVICE_PATH;
nvidiactl.id = makedev(NV_DEVICE_MAJOR, NV_CTL_DEVICE_MINOR);
has_nvidiactl = 1;
}
Reviewers: @elezar @myeolenv
Associated with NVIDIA/nvidia-container-toolkit#1979.
Summary
The modeset device was only mounted into containers that requested the display capability; the graphics capability also requires it. The device mount is now gated on either capability.
Fixes an additional issue regarding
/dev/nvidia-modesetdiscovery in the legacy provisioning interface, withinnvc_info.c.Why This Exists
/dev/nvidia-modesetbacks the display-facing paths of the graphics APIs themselves — Vulkan direct-to-display, the EGLDevice/EGLOutput platform, and the presentation machinery used by Wayland compositor stacks — not only the X.Org driver. Inoptions.h,displayalready impliesOPT_GRAPHICS_LIBS, andOPT_DISPLAYgates exactly one thing: this device mount. A container requesting onlygraphicstherefore received the complete graphics library set but not the device node those APIs need.The NVIDIA Container Toolkit treats
graphicsanddisplayas a single gate for its graphics injection at the OCI level, and CDI mode injects the device unconditionally. This aligns thenvidia-container-clipath — used bydocker --gpuswithout the nvidia runtime — with that behavior.Resolution
The modeset device mount condition in
nvc_mount.cchecksOPT_DISPLAY|OPT_GRAPHICS_LIBSinstead ofOPT_DISPLAYalone, with the comment updated accordingly.Reviewer Considerations
graphicsanddisplayare functionally identical in this library sinceOPT_DISPLAYgates nothing else. This is intentional and matches the toolkit's OCI-level behavior and CDI semantics.load_kernel_modules()already loadsnvidia_modesetand creates the device node under--load-kmods, so no creation-side change is needed.!dxcore.initializedbranch.Behavior Changes
graphicswithoutdisplayreceive the modeset device mount and its device cgroup entry.compute,utility, andvideoremain unchanged;displaybehavior is unchanged.Implementation Summary
nvc_mount.c.Verification
sharedandtoolson Ubuntu x86_64 with GCC 14 (WITH_LIBELF=yes WITH_TIRPC=yes, version and revision provided for a tag-less tree).docker run --runtime=runc --gpus allexercises the hook path and the host node deleted before each run:NVIDIA_DRIVER_CAPABILITIES=graphics: no modeset device in the container;graphics:/dev/nvidia-modesetpresent and openable, with the host node created by--load-kmods;compute,utility: no modeset device;display: unchanged.