Skip to content

fix: Discover /dev/nvidia-modeset instead of assuming it exists #388

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.

Sign up for GitHub

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

Open
ehfd wants to merge 1 commit into
base: main
Choose a base branch
from
+2 −3
Open

fix: Discover /dev/nvidia-modeset instead of assuming it exists #388

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/nvc_info.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,9 +543,8 @@ lookup_devices(struct error *err, struct dxcore_context *dxcore, struct nvc_driv
return (-1);
}
if (!(flags & OPT_NO_MODESET)) {
modeset.path = (char *)NV_MODESET_DEVICE_PATH;
modeset.id = makedev(NV_DEVICE_MAJOR, NV_MODESET_DEVICE_MINOR);
has_modeset = 1;
if ((has_modeset = find_device_node(err, root, NV_MODESET_DEVICE_PATH, &modeset)) < 0)
return (-1);
Comment on lines +546 to +547

@henry118 henry118 Aug 14, 2026

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.

what's the rational for this change? I'm not sure if it's necessary for the purpose of this PR.

@ehfd ehfd Aug 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.

@ehfd ehfd Aug 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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;
        }

@henry118 henry118 Aug 17, 2026

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.

Per the original logic the existence of a modeset device is a prerequisite. But the after this change it is no longer true. Have you seen a real failure case? If not I'd rather keep the original logic intact.

@ehfd ehfd Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Explanation:

The original logic doesn't check for the device — that's the part I'd like to correct.

Before this change, lookup_devices() registered the node unconditionally: a fixed makedev(NV_DEVICE_MAJOR, NV_MODESET_DEVICE_MINOR) and has_modeset = 1, with no stat anywhere. The existence check happens later, at mount time — mount_device() stats the source (xstat(), src/nvc_mount.c:222) and returns NULL if it isn't there, which sends nvc_driver_mount() to goto fail and aborts the container start.

So the prerequisite isn't "a modeset device exists". It's "a modeset device exists, or the container fails to start."

The failure case: a host where /dev/nvidia-modeset was never created. The driver doesn't register control nodes with devtmpfs, so on a headless host that's simply the state until something creates them. A container requesting display there, with nvidia-container-cli invoked without --load-kmods, fails to start — on a node that isn't required for anything else it asked for.

That's rare today only because display is rare. It stops being rare with the other half of this PR: once the gate includes graphics, every graphics container on such a host takes the same hard failure. That's why the change is in this PR rather than standing alone — it's the guard that makes widening the gate safe, not a cleanup.

After the change the node behaves exactly like nvidia-uvm and nvidia-uvm-tools immediately above it: find_device_node() stats it, warns missing device %s and skips on ENOENT, and records the real st_rdev instead of assuming 195:254 — which is also what mount_device() validates against (s.st_rdev != dev->id).

Ordering is unchanged. nvc_init() runs load_kernel_modules() under OPT_LOAD_KMODS (src/nvc.c:423), and the CLI calls libnvc.init() at src/cli/configure.c:304, before driver_info_new() at :323. So --load-kmods still creates the node before discovery looks for it. The only behavior that changes is the genuinely-absent case, which goes from "abort the container start" to "warn and skip".

@ehfd ehfd Aug 22, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

$ sudo rm -f /dev/nvidia-modeset
$ nvidia-container-cli --debug=/dev/stderr configure … --display --utility /tmp/rootfs
I nvc_info.c:572] listing device /dev/nvidia-modeset
nvidia-container-cli: mount error: stat failed: /dev/nvidia-modeset: no such file or directory
exit code 1

# with the patch
W nvc_info.c:327] missing device /dev/nvidia-modeset
exit code 0

}
nvidiactl.path = (char *)NV_CTL_DEVICE_PATH;
nvidiactl.id = makedev(NV_DEVICE_MAJOR, NV_CTL_DEVICE_MINOR);
Expand Down