Skip to content
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

config-linux: Clarify where device nodes can be created #1148

Merged
merged 1 commit into from
Feb 15, 2023

Conversation

c3d
Copy link
Contributor

@c3d c3d commented May 3, 2022

Clarify that device nodes need not be under /dev, but that the runtime need to
be informed of all the device nodes that are used by the
container.

Virtual-machine based runtimes such as Kata Containers need to be able to
perform adjustment on device nodes, and cannot be required to deep-scan
file-systems to do so.

The proposed wording was chosen to avoid any regression for any workload
mounding nodes elsewhere, while at the same time clarifying that correct
behaviour cannot be guaranteed if a device node is created on the host and used
by the container without being passed in the devices list.

This fixes issue #1147.

Signed-off-by: Christophe de Dinechin christophe@dinechin.org

@c3d
Copy link
Contributor Author

c3d commented May 3, 2022

A relevant issue in OpenShift sandboxed containers can be found here

config-linux.md Outdated Show resolved Hide resolved
@c3d c3d force-pushed the issue/1147-device-location branch from 78c1161 to 30370e6 Compare May 5, 2022 09:05
Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@TomSweeneyRedHat TomSweeneyRedHat left a comment

Choose a reason for hiding this comment

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

LGTM

@c3d
Copy link
Contributor Author

c3d commented May 23, 2022

@TomSweeneyRedHat If this looks good to you, could you please change the status of your review to match?

@c3d
Copy link
Contributor Author

c3d commented Jun 29, 2022

Ping?
Here is a new instance of the problem

config-linux.md Outdated
@@ -126,6 +127,12 @@ Each entry has the following structure:

The same `type`, `major` and `minor` SHOULD NOT be used for multiple devices.

Containers MAY NOT access any device node that is not explicitly referenced in
the **`devices`** array. Rationale: runtimes based on virtual machines need to
Copy link
Member

Choose a reason for hiding this comment

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

Conflicts with the configLinuxDefaultDevices section

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AkihiroSuda Good point. Reworded to indicate this.

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Seems inconsistent with the configLinuxDefaultDevices section

Clarify that device nodes need not be under `/dev`, but that the runtimes need
to be informed of all the device nodes that are used by the container.

Virtual-machine based runtimes such as Kata Containers need to be able to
perform adjustment on device nodes, and cannot be required to deep-scan
file-systems to do so.

The proposed wording was chosen to avoid any regression for any workload
mounding nodes elsewhere, while at the same time clarifying that correct
behaviour cannot be guaranteed if a device node is created on the host and used
by the container without being passed in the devices list.

This fixes issue opencontainers#1147.

Signed-off-by: Christophe de Dinechin <christophe@dinechin.org>
@c3d c3d force-pushed the issue/1147-device-location branch from 30370e6 to 3565df5 Compare August 10, 2022 08:25
@AkihiroSuda AkihiroSuda added this to the v1.1.0 milestone Feb 12, 2023
@AkihiroSuda AkihiroSuda merged commit 58ec43f into opencontainers:main Feb 15, 2023
@AkihiroSuda AkihiroSuda mentioned this pull request Mar 28, 2023
@AkihiroSuda AkihiroSuda mentioned this pull request Jun 26, 2023
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants