Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

Conversation

imain
Copy link
Contributor

@imain imain commented Jun 11, 2014

This patch add a C function to create a new device node inside the container
by entering the containers mount namespace. This is used to add devices
to the container at runtime.

Docker-DCO-1.1-Signed-off-by: Ian Main [email protected] (github: imain)

@rjnagal
Copy link
Contributor

rjnagal commented Jun 12, 2014

We currently have one way to enter an existing container. It would be beneficial to extend that to handle different set of capabilities rather than adding a method for each specific mutation we want to apply inside a container.

I would love to have a single place which handles setns logic. 'nsenter enter.json' should do all the work. The config should carry the command to run inside and any capability and permissions that the command might need. It feels like that will solve a larger set of problems.

@imain
Copy link
Contributor Author

imain commented Jun 12, 2014

@rjnagal You are right but that sounds more like a refactoring job that should be done by the maintainer. Ive also learned you can try to 'future proof' things too much sometimes. IMO for now this is a reasonable way to perform a somewhat difficult problem.

imain and others added 2 commits June 18, 2014 19:51
This patch add a C function to create a new device node inside the container
by entering the containers mount namespace.  This is used to add devices
to the container at runtime.

Docker-DCO-1.1-Signed-off-by: Ian Main <[email protected]> (github: imain)
* This is a C implementation to make the unlink system call in a single
  threaded process in the container namespace

Co-Authored-By: Chris Alfonso <[email protected]>
Docker-DCO-1.1-Signed-off-by: Ian Main <[email protected]> (github: imain)
@vmarmol
Copy link
Contributor

vmarmol commented Jun 20, 2014

I agree with @rjnagal in that we can probably service this better with a more forgiving enter. @imain is this change urgent or can it wait on that refactoring/enhancement?

@imain
Copy link
Contributor Author

imain commented Jun 20, 2014

@vmarmol It is pretty urgent really. I'm working on trying to get docker support back into openstack for the juno release. In order for that to happen we have to have it basically functional within the next month or so. This is a key part of that functionality.

I think it would be better to merge this and then refactor afterwards. If nothing else it will give you another use case to prove your refactoring.

I don't want to sound offensive at all but at some level I find the argument a bit silly.. there is maybe 15 lines of real C code here? There are hundreds of lines of golang to support device add.. this is just a small part of it and it will be just as easy to refactor with this in as without it.

@crosbymichael
Copy link
Contributor

Why do you have to enter the namespaces just to do a mknod?

@crosbymichael
Copy link
Contributor

My comment above was wrong. Sorry.

I think we have some time to do what @rjnagal suggested. I'll close this PR for now and we can either keep discussing here or in an issue for dynamically updating a container because we will be needing this for modifying cgroups, devices, and other features.

@imain
Copy link
Contributor Author

imain commented Jun 24, 2014

There is some time, yes. If that is the way you guys want to do it that's cool so long as it's done in a timely manner.

I'm curious how you want to implement this? It seems to me it would be hard to get eg json into C and parse it and have a switch block of what you want to do? Also adds more complexity and is more difficult to prove it is thread safe?

calfonso added a commit to calfonso/nova-docker that referenced this pull request Jun 26, 2014
* This commit is dependent on several pull requests being merged into
docker and libcontainer that expose the devadd and devrm API.
moby/moby#6369
docker-archive/libcontainer#6
docker-archive/libcontainer#7

Change-Id: I6dcd7d809027bfe6ddcd9d521e519656a1ad526a
@calfonso
Copy link

@rjnagal Do you have thoughts about the above comment? I'd like to know how we should move forward with this functionality since so many pending patches depend on this ability to mknod in a namespace.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants