Make joinDevices public.#209
Conversation
|
LGTM. |
|
Are there any changes that need to be made in the |
|
@crosbymichael the code will be the same. This just happens to live in systemd for some reason... |
|
In the docker PR you are calling Apply for |
|
This has been a problem for quite a while. The systemd version of Apply cannot be called into more than once. We did try to fix this once before but it was not easy as we didn't know the code well. Also it has changed since then. Calling systemd apply: [error] server.go:110 HTTP Error: statusCode=500 Cannot apply modification to container elegant_brattain: Unit docker-fb9f2ed2dd41f19d77b8f93c21f686f7d89bd88fd5dbcb7f56dd3385a89fa |
|
I actually think it makes more sense to just call apply on the portions of the cgroups that you are modifying. We could try to pull out a similar fs devices apply? |
|
I'm happy with it the way it is but let me know if you want something changed here. |
|
Yes, I don't like how the two different implementations have two different ways to update. It would be nicer but take more time to make sure that they are consistent even if it means creating an interface that both these implementations must honor. |
For our work on adding dynamic device support to Docker we needed to be able to call this to update the list of allowed devices. This works for both systemd and fs based cgroups implementations. Co-Authored-By: Chris Alfonso <[email protected]> (github: calfonso) Docker-DCO-1.1-Signed-off-by: Ian Main <[email protected]> (github: imain)
|
I changed it so the APIs are the same for each implementation. For some reason calling the fs version to update devices only didn't work when the cgroups were originally set up using the systemd implementation. This now creates symmetrical APIs that we can call and works well. |
|
@crosbymichael see what you think.. ? |
|
LGTM @vmarmol what do you think about this for now? |
|
LGTM This is fine by me. We do need to re-work the way we handle cgroups but it shouldn't block anything. |
4 participants
For our work on adding dynamic device support to Docker we needed to be
able to call this to update the list of allowed devices.
Co-Authored-By: Chris Alfonso [email protected] (github: calfonso)
Docker-DCO-1.1-Signed-off-by: Ian Main [email protected] (github: imain)