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

Split namespace syscall content for building on non-Linux#554

Merged
crosbymichael merged 1 commit intodocker-archive:masterfrom
estesp:namespace_linux_split
Apr 28, 2015
Merged

Split namespace syscall content for building on non-Linux#554
crosbymichael merged 1 commit intodocker-archive:masterfrom
estesp:namespace_linux_split

Conversation

@estesp
Copy link
Contributor

@estesp estesp commented Apr 24, 2015

libcontainer/configs is used by the docker user namespace proposed
patchset to use IDMap for uid/gid maps across the codebase. Given the
client uses some of this code, it needs to build on non-Linux. This
separates out the Linux-only syscalls using build tags.

Signed-off-by: Phil Estes estesp@linux.vnet.ibm.com (github: estesp)

@estesp
Spit namespace syscall content for building on non-Linux
7f1bcd5 
libcontainer/configs is used by the docker user namespace proposed
patchset to use IDMap for uid/gid maps across the codebase.  Given the
client uses some of this code, it needs to build on non-Linux.  This
separates out the Linux-only syscalls using build tags.

Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com> (github: estesp)
@estesp estesp mentioned this pull request Apr 24, 2015
Merged
@dqminh
Copy link
Contributor

dqminh commented Apr 24, 2015

LGTM

@crosbymichael
Copy link
Contributor

crosbymichael commented Apr 24, 2015

What does the client use?

@estesp
Copy link
Contributor Author

estesp commented Apr 24, 2015

pkg/archive

Sent from my iPhone

On Apr 24, 2015, at 6:28 PM, Michael Crosby notifications@github.com wrote:

What does the client use?

—
Reply to this email directly or view it on GitHub.

@crosbymichael
Copy link
Contributor

crosbymichael commented Apr 24, 2015

what does that depend on?

@estesp
Copy link
Contributor Author

estesp commented Apr 24, 2015

needs def'n for config.IDMap to handle optional remapping on tar/untar/unpack, etc. Trying to split out client usage of TarWithOptions and Untar from pkg/archive looks very tricky, even though client (today) doesn't need to use the optional IDMap(s) for what it is doing.

@crosbymichael
Copy link
Contributor

crosbymichael commented Apr 24, 2015

Humm, i don't think an archive package needs to depend on a libcontainer package. Seems like it's not the right depedency

@estesp
Copy link
Contributor Author

estesp commented Apr 24, 2015

I need []configs.IDMap all over docker, and of course, as you know, the archiver has to have those maps to do remapping on layer untar. Given configs and user are not dependent on the rest of libcontainer they seem like reasonable "pieces" to rely on for shared definitions. If there is a better way, I'm game to do something else.

@dqminh
Copy link
Contributor

dqminh commented Apr 24, 2015

Even if they dont have to share libcontainer/configs ( probably just create their own structure and remap to libcontainer at the execdriver level is better ), I still think that spliting syscall support to _linux in config is an ok thing to do.

However this will probably make libct branch not working because of the build tags.

@estesp
Copy link
Contributor Author

estesp commented Apr 25, 2015

@crosbymichael let me know if you want me to have a Docker "shadow" IDMap definition that gets cast to libcontainer config.IDMap when needed in the execdriver (as @dqminh mentioned above). That touches a lot of places and I would be happy to refactor the whole user namespace PR, but would like your opinion so I can go one way or the other.

@estesp estesp changed the title Spit namespace syscall content for building on non-Linux Split namespace syscall content for building on non-Linux Apr 25, 2015
@crosbymichael
Copy link
Contributor

crosbymichael commented Apr 28, 2015

I would rather have this struct not in the code of docker right. So if you could make the type defined there that would be great.

@crosbymichael
Copy link
Contributor

crosbymichael commented Apr 28, 2015

Anyways, this is a good idea to do.

LGTM

crosbymichael added a commit that referenced this pull request Apr 28, 2015
@crosbymichael
Merge pull request #554 from estesp/namespace_linux_split
d70569a 
Split namespace syscall content for building on non-Linux
@crosbymichael crosbymichael merged commit d70569a into docker-archive:master Apr 28, 2015
@estesp estesp deleted the namespace_linux_split branch April 28, 2015 01:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

status/0-needs-triage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@estesp @dqminh @crosbymichael @GordonTheTurtle