Enable specifying directory as device on container with --device #2412
Conversation
QiWang19
changed the title
podkan run --device
QiWang19
force-pushed
the
iss2380
branch
10 times, most recently
from
92a3d8e
to
004a3e8
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: QiWang19, vrothberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
pkg/spec/config_linux.go
Outdated
| found := false | ||
| // mount the internal devices recursively | ||
| if err := filepath.Walk(resolvedDevicePath, func(dpath string, f os.FileInfo, e error) error { | ||
|
|
||
| if f.Mode()&os.ModeDevice == os.ModeDevice { | ||
| found = true | ||
| device := dpath | ||
|
|
||
| if len(devs) > 1 { | ||
| if len(devs) > 1 && devs[1][0] == '/' { |
There was a problem hiding this comment.
Can we add a comment to that? I wasn't familiar with those specific parts and it took me a while to understand it. Although the compiler might be smart enough to move len(devs) > 1 && devs[1][0] == '/' outside the walk func, can we move the checks outside the walk and assign them to some booleans that describe it a bit more?
Something as follows (just an example) might be easier to read:
destSpecified := len(devs) > 1 && devs[1][0] == '/'
modeSpecified := len(devs) > 1 && devs[1][0] != '/'
destAndModeSpecified := len(devs) > 2
destOrModeSpecified := len(devs) > 1
// mount the internal devices recursively
if err := filepath.Walk(resolvedDevicePath, fun [...]
|
/retest |
pkg/spec/config_linux.go
Outdated
| found := false | ||
| destSpecified := len(devs) > 1 && devs[1][0] == '/' |
There was a problem hiding this comment.
Could this crash if len(devs[1]) == 0? I don't know how we generate the devs array
There was a problem hiding this comment.
Oh, that's a good catch. It can crash, see a minimal example: https://play.golang.org/p/Yk52BfKvh2q
QiWang19
force-pushed
the
iss2380
branch
2 times, most recently
from
417610f
to
6ab52f4
Compare
pkg/spec/config_linux.go
Outdated
| if len(devs) > 2 { | ||
| return errors.Wrapf(unix.EINVAL, "not allowed to specify destination with a directory %s", devicePath) | ||
| if devmode != "" { | ||
| return errors.Errorf("invalid device specification %s", devicePath) |
There was a problem hiding this comment.
return errors.Wrapf(unix.EINVAL, ...)
test/e2e/run_device_test.go
Outdated
| @@ -72,4 +72,12 @@ var _ = Describe("Podman run device", func() { | |||
| session.WaitWithDefaultTimeout() | |||
| Expect(session.ExitCode()).To(Not(Equal(0))) | |||
| }) | |||
|
|
|||
| FIt("podman run device host device and container device parameter are directories", func() { | |||
QiWang19
force-pushed
the
iss2380
branch
2 times, most recently
from
04ef91c
to
1b90c72
Compare
|
Needs to update RELEASE_NOTES.md with changes |
|
Nevermind the noise, we'll do this instead |
|
@QiWang19 Can you fix my comment so we can get this one in. |
|
fixed them. See the CI tests. |
|
/retest |
Signed-off-by: Qi Wang <qiwan@redhat.com>
|
bot, retest this please |
|
Needs lgtm label. 🤔 |
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
If device on host is specified a directory, enable user to specify the directory on container.
$ sudo podman run --device /dev/snd:/dev/snd:rmw localhost/qifix #2380
Signed-off-by: Qi Wang qiwan@redhat.com