-
Notifications
You must be signed in to change notification settings - Fork 18.8k
Match device driver on name and ignore capabilities #50717
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
Conversation
702bbb7
to
80f1b0b
Compare
daemon/devices.go
Outdated
"driver": req.Driver, | ||
"capabilities": req.Capabilities, | ||
}).Debugf("Selecting device driver by driver name; possibly ignoring capabilities") | ||
return dd.updateSpec(spec, &deviceInstance{req: req}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thaJeztah I moved this over from #50099 since I think it may be easier to reason about as a standalone change following the discussion in #50099 (comment).
This change ignores requested capabilities when a driver is explicitly requested. This simplifies the logic for selecting a driver and means that users need not spefify redundant capabilities. With the exception of the catch-all "gpu" capability the remaining capabilities are only relevant for the "nvidia" driver. Signed-off-by: Evan Lezar <elezar@nvidia.com>
80f1b0b
to
4803834
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGMT
"requested": req.Capabilities, | ||
"selected": selected, | ||
}, | ||
}).Debugf("Selecting device driver by driver name; possibly ignoring capabilities") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
}).Debugf("Selecting device driver by driver name; possibly ignoring capabilities") | |
}).Debug("Selecting device driver by driver name; possibly ignoring capabilities") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in #50228
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This change ignores requested capabilities when a driver is explicitly requested. This simplifies the logic for selecting a driver and means that users need not spefify redundant capabilities. This aligns the behaviour for all drivers with the CDI driver.
With the exception of the catch-all "gpu" capability the remaining capabilities are only relevant for the "nvidia" driver.
See also the discussion in #50099
- What I did
Changed the logic for selecting a device driver to ignore capabilities when selecting a driver by name. If no driver name is specified, the current behaviour remains and an appropriate driver is selected based on the set of required capabilities.
- How I did it
Updated the
handleDevice
implementation- How to verify it
When running a modified daemon in debug mode on a system with
nvidia
devices (and therefore annvidia
driver registered):Check the logs:
Check the logs:
Check the logs
- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)