LGTM

@tiborvass can we please link to the dirperm1 long explanation from the known issues in the docs/sources/release-notes.md

also, while the description of using that option is technically correct, it doesn't make it obvious that there is a disadvantage to using the dirperm1 option - and so the a user would reasonably ask - why don't we make that the default, thus fixing the long standing bug?

@SvenDowideit what do you suggest? This PR is supposed to close #783. Should we still keep that in the known issues? Let me know what you'd like to see and I'll add it.

@SvenDowideit @tiborvass afaik only ubuntu 14.10 and after carry the dirperm1 patches by default, or if you build the aufs patches yourself. So i dont think it can be the default yet.

Perhaps we should add a note to #783 to tell people about the workaround ?

@dqminh so there are no downsides? It's just that if it's not available there's no fix.

I think this PR fixes that issue. Ping @SvenDowideit

@tiborvass Yes, if it's not available, you cant use it :D

I think that in Docker usecase, there's no downside. Not enable dirperm1 prevents user from changing permission bits of a directory in lower layer to a broader level, but you can already do it in other graph drivers so this point is moot.
The (pro) side effect of dirperm1 is that it's a bit faster since it doesnt have to walk all the layers to check for permissions.

@tiborvass I think @dqminh's point is probably what Docker should focus on as far as the "why dirperm1 makes sense to enable" --for those on recent Ubuntu distros who can take advantage. The fact that the bug is extremely confusing when you hit it (I remember trying to help @duglin figure out a Dockerfile that worked until you switched the order of a chmod and an ADD last fall! very confusing because the standard ls -l perms look right, but you can't access the files/dirs), not to mention the fact that switching graph drivers makes the "bug" go away. So, dirperm1 where available makes AUFS backend have the same COW/layer characteristics as the other graph drivers.

@dqminh @estesp so would it make sense to enable it by default? Does it mean we should remove it from --storage-opt ?

@dqminh stated it isn't available until Ubuntu 14.10, so other than the fact that it may not be supported depending on base OS, I believe on by default is a reasonable approach.

My only other thought: would we want to allow for it to be disabled in case there is a strange use case scenario where someone is relying on current behavior?

@estesp your last question is basically a better formulation of "what's the downside of using dirperm1?", and what I've been told so far is that there is none.

@tiborvass how is this for a not-so-helpful response: there is no downside unless someone saw an upside to weird permissions errors from Dockerfile ordering problems :-} I don't know how to answer that without changing the default and seeing who screams. I would hope no one, and looking at the long painful history of #783 (and all its dups and related issues), you would think changing the default will make a lot of people very very happy.

Thanks @estesp! I'll wait for @dqminh's answer too.
If anyone has opinions on this, now is the time!

@dqminh I should probably do something like in #10534

@tiborvass #10534 case is different though, because afair directio is widely supported ( back to ubuntu 12.04 ), while dirperm1 is only supported since 14.10 ( ubuntu is probably the only distribution that is shipping aufs by default afaik ).

Enabling dirperm1 by default will then make the older systems not able to run aufs driver unless they recompile the aufs extension, which is sort of bad because ubuntu LTS stack falls into those systems.

I think it's safer for dirperm1 to be opted in for now, and maybe enabled by default after 15.04.

@dqminh but isn't there a way to detect whether dirperm1 is available and enable it if it is?

Hmm, maybe we can also test mounting with dirperm1 at first mount to detect
if dirperm1 is supported ? If we fail to set dirperm1, then retry without
dirperm1 for the rest of the ops. How does it sound ?

On Sunday, 8 February 2015, Tibor Vass [email protected] wrote:

@dqminh https://github.com/dqminh but isn't there a way to detect
whether dirperm1 is available and enable it if it is?

—
Reply to this email directly or view it on GitHub
#10390 (comment).

@dqminh IANTM, but sure why not!

Ping @crosbymichael @tianon @unclejack

@tiborvass Thanks for letting me know about this.

Perhaps we could test if this dirperm1 feature is available during the aufs init phase? @dqminh I'd be +1 to doing that check as you've also written above.

We should try to detect this on the fly and print a warning if it's not supported. I think this is the best approach and that it would be a better experience for the user.

@unclejack Thanks for your input! I'll see what I can do to do that, but if someone wants to send me a PR, i'll cherry-pick their commits onto this branch :)

I think if we find a good way to check for dirperm1 in the code (and thus prefer it), we should also add CONFIG_AUFS_DLGT to check-config.sh in the AUFS section (http://sourceforge.net/p/aufs/mailman/message/18822722/). 👍

@tiborvass http://docs.docker.com/release-notes/#known-issues lists the problem atm, and this is (if i understand correctly) a fix which may not be available.

so yes, I'd keep the known issue there for now, but add a link from it to the docs you added about the mount-opt - pointing out that it needs settings that can be checked with check-config.sh.

Additionally - @tianon can we make sure boot2docker has things set up right?

Ping @tiborvass: it's unclear if you have something left to do here, or if this should be moved to 3-code-review.

@icecrime thanks for the reminder, I do have something to do here :( Will try to update shortly

I'll reopen when I update today, sorry about the delay