-
Notifications
You must be signed in to change notification settings - Fork 18.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add error when running overlay over btrfs. #9834
Conversation
Ah shit sorry the broken commit pushed, at the airport will update soon |
Thanks for picking this up so quickly! |
808bc8f
to
ef216ff
Compare
np, it was an easy update, ok pushed :) |
i dont know who has the env to test this other than @LK4D4 |
@bobrik if you want to test too that would be cool, no worries if not ;) |
@jfrazelle I can check if I get an error if you tell me where to get the build. |
I will make some binaries and push to s3 |
I've got a little paranoid and decided to compile binary myself. Guess what?
|
whoa thats weird... maybe a SNI problem? and I swear I wouldnt give you a virus lol, unless it was an adobe flash plugin |
Okay, I compiled docker myself, it fails to start as expected:
Still works fine on ext4. LGTM then. Forget about SSL issue, looks like an issue on my side. |
yayyy thanks! |
return nil, err | ||
} | ||
|
||
if graphdriver.FsMagic(buf.Type) == graphdriver.FsMagicBtrfs { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is little confusing :) Message is about using over ext4
, but you checking for btrfs
. Maybe I am using reiser
.
Should we deny to run overlay over any fs which is not ext4
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SvenDowideit mentioned below request:
can we also add the underlying FS to docker info? (for all graph drivers that have them..)
it also need consider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xiaods I think that can be a separate pr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LK4D4 I will update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated!
ef216ff
to
4ba83ea
Compare
I got the magic number for ext4 from http://www.cs.fsu.edu/~baker/devices/lxr/http/source/linux/include/linux/magic.h |
4ba83ea
to
884aabb
Compare
LGTM |
Review session with @tiborvass @crosbymichael @unclejack @LK4D4 We're actually disagreeing with your previous comment: we'd rather have a blacklist of unsupported fs (btrfs + aufs for the moment) than a whitelist of supported fs (especially as ext4 shouldn't be the only one). What about things such as xfs? |
Overlayfs is supported with XFS |
ah ok cool so should I just blacklist Aufs and btrfs what about reiser |
884aabb
to
32f1025
Compare
ok I updated, I put two seperate errors, but I would combine into one if, but I was just trying to avoid someone opening an issue about how we don't tell them what they are running and the error is "vague" |
Docker-DCO-1.1-Signed-off-by: Jessica Frazelle <jess@docker.com> (github: jfrazelle)
LGTM |
Add error when running overlay over btrfs.
Fixes #9820.