Add imex mode to CDI spec generation #807
Conversation
elezar
requested review from
klueska,
tariq1890,
ArangoGutierrez and
cdesiniotis
| @@ -183,17 +183,6 @@ func (m command) validateFlags(c *cli.Context, opts *options) error { | |||
| return fmt.Errorf("invalid output format: %v", opts.format) | |||
| } | |||
|
|
|||
| opts.mode = strings.ToLower(opts.mode) | |||
There was a problem hiding this comment.
Question -- why was this removed? Also, is there a reason for not documenting all of these valid modes in the --discovery-mode usage string?
There was a problem hiding this comment.
This was removed because I didn't want to maintain the list in two places. Let me pull in some changes that I have in another commit to improve things
pkg/nvcdi/lib-imex.go
Outdated
|
|
||
| // GetSpec returns a CDI spec for all available IMEX channels. | ||
| func (l *imexlib) GetSpec() (spec.Interface, error) { | ||
| return nil, nil |
There was a problem hiding this comment.
Why is this not implemented?
There was a problem hiding this comment.
We actually never call GetSpec at this level and always call GetSpec of the wrapper. Definitely needs a cleanup, but that refactor is out of scope for this PR.
| cdiVersion: 0.5.0 | ||
| containerEdits: | ||
| env: | ||
| - NVIDIA_VISIBLE_DEVICES=void |
There was a problem hiding this comment.
Question -- where in the code is this set? Are we always adding this envvar to our specs regardless of the mode (i.e. nvml, wsl, imex)?
There was a problem hiding this comment.
This is set in the wrapper GetSpec in lib.go. I think these changes call out that this may not be the best place to do it since it seems a little out of place. The intent is to ENSURE that we don't inadvertently trigger runtime or hook injection when we're already applying changes using CDI.
update: It's in GetCommonEdits:
nvidia-container-toolkit/pkg/nvcdi/lib.go
Lines 199 to 207 in 5c3ffc2
elezar
force-pushed
the
add-cdi-imex-channels
branch
from
e827f76 to
3384ce2
Compare
Signed-off-by: Evan Lezar <[email protected]>
Signed-off-by: Evan Lezar <[email protected]>
This change adds a imex mode to CDI spec generation. This mode detected generates CDI specifications for existing IMEX channels. By default these devices have the fully qualified CDI device names: nvidia.com/imex-channel=<ID> Signed-off-by: Evan Lezar <[email protected]>
elezar
force-pushed
the
add-cdi-imex-channels
branch
from
3384ce2 to
8603d60
Compare
This change adds a imex mode to CDI spec generation. This mode detected generates CDI specifications for existing IMEX channels. By default these devices have the fully qualified CDI device names:
nvidia.com/imex-channel={{ .ID }}