Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(9)

Issue 126190043: syscall: Adds support for User Namespaces in Linux by a...

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 7 months ago by mrunalp
Modified:
9 years, 6 months ago
Reviewers:
iant
CC:
golang-codereviews, bradfitz, iant
Visibility:
Public.

Description

syscall: Adds support for User Namespaces in Linux by allowing to specify UID/GID mappings in syscall.Credential. Fixes issue 8447.

Patch Set 1 #

Patch Set 2 : diff -r 2cc71cd8fe3e466a2338b1fdf8053931d47a9e31 https://code.google.com/p/go #

Total comments: 20

Patch Set 3 : diff -r 1c674c3eefc7c3c2c73378bd2482a08b933839ee https://code.google.com/p/go #

Total comments: 16

Patch Set 4 : diff -r 1c674c3eefc7c3c2c73378bd2482a08b933839ee https://code.google.com/p/go #

Total comments: 3

Patch Set 5 : diff -r ce32e953ef6f5418efbcce5c25175aec25204eac https://code.google.com/p/go #

Total comments: 17

Patch Set 6 : diff -r ce32e953ef6f5418efbcce5c25175aec25204eac https://code.google.com/p/go #

Total comments: 4

Patch Set 7 : diff -r ce32e953ef6f5418efbcce5c25175aec25204eac https://code.google.com/p/go #

Total comments: 2

Patch Set 8 : diff -r ce32e953ef6f5418efbcce5c25175aec25204eac https://code.google.com/p/go #

Unified diffs Side-by-side diffs Delta from patch set Stats (+111 lines, -12 lines) Patch
M src/syscall/exec_linux.go View 1 2 3 4 5 6 7 6 chunks +111 lines, -12 lines 0 comments Download

Messages

Total messages: 25
mrunalp
The code adds support for User Namespaces to golang. For issues and reason behind why ...
9 years, 6 months ago (2014-09-24 21:49:19 UTC) #1
bradfitz
R=iant On Wed, Sep 24, 2014 at 2:49 PM, <mrunalp@gmail.com> wrote: > Reviewers: golang-codereviews, > ...
9 years, 6 months ago (2014-09-29 18:43:43 UTC) #2
iant
The first line in the CL description should be a single line with a description ...
9 years, 6 months ago (2014-09-30 14:14:18 UTC) #3
mrunalp
I am working on addressing the comments. Meanwhile there is one design decision to be ...
9 years, 6 months ago (2014-09-30 18:42:06 UTC) #4
iant
https://codereview.appspot.com/126190043/diff/20001/src/pkg/syscall/exec_unix.go File src/pkg/syscall/exec_unix.go (right): https://codereview.appspot.com/126190043/diff/20001/src/pkg/syscall/exec_unix.go#newcode116 src/pkg/syscall/exec_unix.go:116: UidMappings string On 2014/09/30 18:42:06, mrunalp wrote: > We ...
9 years, 6 months ago (2014-09-30 18:50:09 UTC) #5
mrunalp
> > We can introduce a IdMap struct with 3 fields > > > > ...
9 years, 6 months ago (2014-09-30 19:22:37 UTC) #6
iant
On Tue, Sep 30, 2014 at 12:22 PM, <mrunalp@gmail.com> wrote: >> > We can introduce ...
9 years, 6 months ago (2014-09-30 19:50:23 UTC) #7
mrunalp
> > Understood, but not wholly relevant. We want the right API for Go, > ...
9 years, 6 months ago (2014-09-30 20:28:17 UTC) #8
mrunalp
I have updated the code to introduce IdMap struct and pushed down the functionality into ...
9 years, 6 months ago (2014-10-01 01:37:12 UTC) #9
bradfitz
drive-by https://codereview.appspot.com/126190043/diff/40001/src/syscall/exec_linux.go File src/syscall/exec_linux.go (right): https://codereview.appspot.com/126190043/diff/40001/src/syscall/exec_linux.go#newcode13 src/syscall/exec_linux.go:13: // Holds Container to Host ID mappings used ...
9 years, 6 months ago (2014-10-01 03:40:08 UTC) #10
mrunalp
Addressed stylistic issue (hopefully) and fixed fd leak.
9 years, 6 months ago (2014-10-01 05:14:52 UTC) #11
iant
I don't think there is anything wrong with changing the signature of the function, as ...
9 years, 6 months ago (2014-10-01 14:55:08 UTC) #12
mrunalp
I have addressed the comments and reworked the code a bit. Now, I am using ...
9 years, 6 months ago (2014-10-01 21:48:43 UTC) #13
iant
Getting close, I think. https://codereview.appspot.com/126190043/diff/80001/src/syscall/exec_linux.go File src/syscall/exec_linux.go (right): https://codereview.appspot.com/126190043/diff/80001/src/syscall/exec_linux.go#newcode13 src/syscall/exec_linux.go:13: // IdMap holds Container ID ...
9 years, 6 months ago (2014-10-01 22:27:18 UTC) #14
mrunalp
Thanks for the review comments Ian! I have addressed all the comments (I think). I ...
9 years, 6 months ago (2014-10-02 00:07:04 UTC) #15
iant
https://codereview.appspot.com/126190043/diff/100001/src/syscall/exec_linux.go File src/syscall/exec_linux.go (right): https://codereview.appspot.com/126190043/diff/100001/src/syscall/exec_linux.go#newcode132 src/syscall/exec_linux.go:132: if err1 != 0 || r1 != unsafe.Sizeof(uintptr(0)) { ...
9 years, 6 months ago (2014-10-02 00:33:34 UTC) #16
mrunalp
I added EINVAL for the case described earlier. Thanks, Mrunal https://codereview.appspot.com/126190043/diff/100001/src/syscall/exec_linux.go File src/syscall/exec_linux.go (right): https://codereview.appspot.com/126190043/diff/100001/src/syscall/exec_linux.go#newcode132 ...
9 years, 6 months ago (2014-10-02 05:14:25 UTC) #17
iant
Have you signed the CLA as described at http://golang.org/doc/contribute.html ?
9 years, 6 months ago (2014-10-02 15:51:32 UTC) #18
iant
https://codereview.appspot.com/126190043/diff/120001/src/syscall/exec_linux.go File src/syscall/exec_linux.go (right): https://codereview.appspot.com/126190043/diff/120001/src/syscall/exec_linux.go#newcode135 src/syscall/exec_linux.go:135: if r1 != unsafe.Sizeof(uintptr(0)) { Let's make this unsafe.Sizeof(err2), ...
9 years, 6 months ago (2014-10-02 15:53:45 UTC) #19
mrunalp
I made the suggested change. Also, I think there is a corporate CLA between Google ...
9 years, 6 months ago (2014-10-02 17:01:45 UTC) #20
iant
On Thu, Oct 2, 2014 at 10:01 AM, <mrunalp@gmail.com> wrote: > > Also, I think ...
9 years, 6 months ago (2014-10-02 17:08:25 UTC) #21
iant
LGTM Thanks for your patience with this.
9 years, 6 months ago (2014-10-02 17:20:20 UTC) #22
iant
*** Submitted as https://code.google.com/p/go/source/detail?r=d56326309b20 *** syscall: support UID/GID map files for Linux user namespaces Fixes ...
9 years, 6 months ago (2014-10-02 18:37:10 UTC) #23
mrunalp
On 2014/10/02 18:37:10, iant wrote: > *** Submitted as https://code.google.com/p/go/source/detail?r=d56326309b20 *** > > syscall: support ...
9 years, 6 months ago (2014-10-02 19:36:06 UTC) #24
mrunalp
9 years, 6 months ago (2014-10-02 19:37:03 UTC) #25
On 2014/10/02 19:36:06, mrunalp wrote:
> On 2014/10/02 18:37:10, iant wrote:
> > *** Submitted as https://code.google.com/p/go/source/detail?r=d56326309b20
***
> > 
> > syscall: support UID/GID map files for Linux user namespaces
> > 
> > Fixes issue 8447.
> > 
> > LGTM=iant
> > R=golang-codereviews, bradfitz, iant
> > CC=golang-codereviews
> > https://codereview.appspot.com/126190043
> > 
> > Committer: Ian Lance Taylor <mailto:iant@golang.org>
> 
> Thanks Ian!
> Is this making into go 1.4?
> 
> - Mrunal

Never mind. I saw your response on the issue.

Thanks again!
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b