Skip to content

aufs: apply dirperm1 by default if supported - #11799

Merged
crosbymichael merged 3 commits into
moby:masterfrom
dqminh:aufs-dirperm1
Mar 30, 2015
Merged

aufs: apply dirperm1 by default if supported#11799
crosbymichael merged 3 commits into
moby:masterfrom
dqminh:aufs-dirperm1

Conversation

@dqminh

@dqminh dqminh commented Mar 26, 2015

Copy link
Copy Markdown
Contributor

Automatically detect support for aufs dirperm1 option and apply it.
dirperm1 tells aufs to check the permission bits of the directory on the
topmost branch and ignore the permission bits on all lower branches.
It can be used to fix aufs' permission bug (i.e., upper layer having
broader mask than the lower layer).

dirperm1 man page is at: http://aufs.sourceforge.net/aufs3/man.html

Fixes #783

@dqminh

dqminh commented Mar 26, 2015

Copy link
Copy Markdown
Contributor Author

cc @tiborvass

Also it seems like enabling dirperm1 makes remounting layer significantly slower (1+ms in my VM) ( happens when the user has a lot of layers that total length of mount options exceeds PAGE_SIZE ). TestMountMoreThan42Layers is much more slower with this patch.

@tiborvass

Copy link
Copy Markdown
Contributor

+1 for this, I like it.

Ping @vbatts @unclejack feel free to step in to say if you don't like the way this is implemented.

Comment thread daemon/graphdriver/aufs/aufs.go Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should be

base, err := ioutil.TempDir("", "docker-aufs-base")
if err != nil {

Same for union, err :=

@tiborvass

Copy link
Copy Markdown
Contributor

@jfrazelle this would be a great fix that a lot of people would welcome. Also it would close an issue with a 3-digit number :P

@dqminh would you mind removing the part that mentions the issue in some KNOWN issues section somewhere?

Automatically detect support for aufs `dirperm1` option and apply it.
`dirperm1` tells aufs to check the permission bits of the directory on the
topmost branch and ignore the permission bits on all lower branches.
It can be used to fix aufs' permission bug (i.e., upper layer having
broader mask than the lower layer).

More information about the bug can be found at moby#783
`dirperm1` man page is at: http://aufs.sourceforge.net/aufs3/man.html

Signed-off-by: Daniel, Dao Quang Minh <dqminh89@gmail.com>
@dqminh

dqminh commented Mar 26, 2015

Copy link
Copy Markdown
Contributor Author

would you mind removing the part that mentions the issue in some KNOWN issues section somewhere?

@tiborvass this won't fix it for system that doesnt have dirperm1 patch though, so I think we still need to keep the note. I can add some note about dirperm1 being set as the default option if it's available.

@tiborvass

Copy link
Copy Markdown
Contributor

@dqminh fair enough! Thanks :)

Since `dirperm1` requires a more recent aufs patch than many current OS release,
we cant remove moby#783 completely. This documents that docker will apply `dirperm1`
automatically for systems that support it

Signed-off-by: Daniel, Dao Quang Minh <dqminh89@gmail.com>
@crosbymichael

Copy link
Copy Markdown
Contributor

LGTM

It's easier for users to check if their systems support dirperm1 just by using
docker info

Signed-off-by: Daniel, Dao Quang Minh <dqminh89@gmail.com>
@dqminh

dqminh commented Mar 26, 2015

Copy link
Copy Markdown
Contributor Author

@tiborvass i added dirperm1 support status to docker info output too so there's an easy way to tell user whether their system is still affected by the permission bug.

@dqminh

dqminh commented Mar 30, 2015

Copy link
Copy Markdown
Contributor Author

@tiborvass is this merge-able for 1.6, or is it too late now ?

@jessfraz jessfraz added this to the 1.6.0 milestone Mar 30, 2015
@jessfraz

Copy link
Copy Markdown
Contributor

I added the milestone @crosbymichael is that ok with you

@vbatts

vbatts commented Mar 30, 2015

Copy link
Copy Markdown
Contributor

LGTM

crosbymichael added a commit that referenced this pull request Mar 30, 2015
aufs: apply dirperm1 by default if supported
@crosbymichael
crosbymichael merged commit 14fed35 into moby:master Mar 30, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

While investigating some other (transient) docker problem, I saw this error (once) in my kernel log — is it a harmless error produced here while testing the presence of dirperm1? Google wasn't very helpful.

[   16.825747] aufs au_opts_parse:1155:docker[1679]: unknown option dirperm1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

OK, I'm guessing yes:

# docker info | grep Dirperm1
 Dirperm1 Supported: false

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

unexpected file permission error in container

7 participants