Conditionally mount the host's /dev over the container's /dev #479
Conversation
github-project-automation
bot
moved this to Backlog
in Planning Board: k8s-dra-driver-gpu
klueska
changed the title
klueska
force-pushed
the
conditionally-mount-host-dev
branch
2 times, most recently
from
c75825c to
0f2aa79
Compare
jgehrcke
left a comment
jgehrcke
left a comment
There was a problem hiding this comment.
contingent on devRoot == "/"
makes sense!
Thanks, Kevin. I agree that a tiny patch here is a smart path forward (de-risking the introduction of new problems)
| return helper(root) | ||
| } | ||
|
|
||
| // conditionallyBindMountHostDev bind mounts hostDevPath over /dev when devRoot is "/". |
There was a problem hiding this comment.
nit: Should we add a note as to why this is required?
| nvidiaCapsDeviceName = "nvidia-caps" | ||
| nvidiaCapsImexChannelsDeviceName = "nvidia-caps-imex-channels" | ||
| nvidiaCapFabricImexMgmtPath = "/proc/driver/nvidia/capabilities/fabric-imex-mgmt" | ||
| hostDevPath = "/host/dev" |
There was a problem hiding this comment.
nit: Is this hostDevContainerPath?
| mounter := mount.New(mountExecutable) | ||
|
|
||
| // Bind mount hostDevPath over /dev | ||
| if err := mounter.Mount(hostDevPath, "/dev", "", []string{"bind"}); err != nil { |
There was a problem hiding this comment.
Are we concerned that this leaks mounts to the host? Is this mount cleaned up if the container is terminated? (e.g. if mount propagation is bidirectional)?
There was a problem hiding this comment.
The host dev folder isn't mounted bidirectionally, but regardless I don't think anything will be leaked.
Unlike with the driver container where we perform mounts into a path that (in itself) has been hostPath-mounted into the container -- the bind mount we are performing here is over a path solely within the mount namespace of the container.
klueska
force-pushed
the
conditionally-mount-host-dev
branch
from
0f2aa79 to
2b83dad
Compare
|
Pushed changes to address comments from @elezar |
klueska
force-pushed
the
conditionally-mount-host-dev
branch
from
2b83dad to
ee4efec
Compare
Currently, in the compute domain driver, we unconditionally mount the host's /dev over the container's /dev regardless of what the devRoot of the GPU driver is. This change was introduced in NVIDIA#307. Unfortunately, this causes some problems on certain systems as described in NVIDIA#477. This patch makes this mount contingent on devRoot == "/" (which is the only case where we actually ever wanted / needed to have this mount in the first place. Signed-off-by: Kevin Klues <[email protected]>
klueska
force-pushed
the
conditionally-mount-host-dev
branch
from
ee4efec to
9797d24
Compare
3 participants
Currently, in the compute domain driver, we unconditionally mount the host's /dev over the container's /dev regardless of what the devRoot of the GPU driver is. This change was introduced in #307.
Unfortunately, this causes some problems on certain systems as described in #477.
This patch makes this mount contingent on devRoot == "/" (which is the only case where we actually ever wanted / needed to have this mount in the first place.
Fixes: #477