Skip to content

Skip the mount tests on kernel 4.4.0 #633

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

Merged
merged 1 commit into from
Jul 2, 2017
Merged

Conversation

asomers
Copy link
Member

@asomers asomers commented Jun 28, 2017

Some versions of that kernel have a known bug with tmpfs in namespaces.
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1659087

Fixes #610

@Susurrus
Copy link
Contributor

We've got a bunch of failures because of rustup and then a openpty failure. You mentioned that rustup failure in another PR, I don't know what's going on there. Did you ever look into that?

@asomers
Copy link
Member Author

asomers commented Jun 28, 2017

@Susurrus I'm looking at the rustup failure now, but I don't know what's going on. It looks like it's affected every PR for the last 3 days. Travis updated their build images 6 days ago, but we've had successful builds since then, so I don't think that's it. Is there any way to get an interactive session in Travis?

@Susurrus
Copy link
Contributor

I don't think Travis has interactive sessions. I'm wondering if you could reproduce in a VM, maybe it's a Rustup bug?

@Susurrus
Copy link
Contributor

For the record, this fails for me on 4.11.3-202 on Fedora 25.

@asomers
Copy link
Member Author

asomers commented Jun 29, 2017

Perhaps we should just screw the namespaces and require the mount tests to run as root? We're already using sudo in Travis.

@Susurrus
Copy link
Contributor

That's fine. Is there anyway to test for root easily within the tests?

@asomers
Copy link
Member Author

asomers commented Jun 29, 2017

Sure. you can just use getpid() == 0. But I don't know how to tell Cargo to use sudo for the mount tests.

@asomers
Copy link
Member Author

asomers commented Jun 29, 2017

Actually, we're making this way too complicated. There is absolutely no reason why open(2) should return EOVERFLOW. If we see that error code, then we know we're running on a buggy kernel. So we can delete the kernel version check. All we have to do is skip (or pass, since Cargo doesn't know about skipped tests yet) this test whenever we see EOVERFLOW.

@Susurrus
Copy link
Contributor

open() can actually return EOVERFLOW, but I agree it shouldn't in these situations given the manpages for open(2):

 EOVERFLOW
         pathname refers to a regular file that is too large to be opened.  The usual  sce‐
         nario  here  is  that  an  application  compiled  on  a  32-bit  platform  without
         -D_FILE_OFFSET_BITS=64 tried to open a file whose size  exceeds  (1<<31)-1  bytes;
         see  also  O_LARGEFILE  above.  This is the error specified by POSIX.1; in kernels
         before 2.6.24, Linux gave the error EFBIG for this case.

@Susurrus
Copy link
Contributor

Just tested this locally and it works on my end. So I think we might be on to something here. @henninglive Care to give this branch a spin and confirm it fixes things on your end?

@asomers
Copy link
Member Author

asomers commented Jul 1, 2017

I fixed #634, so this PR is the last thing standing between us and green builds again.

@Susurrus
Copy link
Contributor

Susurrus commented Jul 1, 2017

One thing I'd point out here is that these tests don't just fail on Linux 4.4, they also fail on my 4.11.3 box, so it's something more than that Ubuntu error you linked. I'd expand the comment to include this information as well.

Starting somewhere in 4.4.0 some versions of Linux have a known bug with
tmpfs in namespaces.  It's unknown exactly which versions are affected
(and likely distro-dependent), but easy to detect.  When open(2) returns
EOVERFLOW, skip the rest of the test.
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1659087

Fixes nix-rust#610
@asomers
Copy link
Member Author

asomers commented Jul 2, 2017

I'm going to go ahead and merge this, without feedback from @henninglive, since the build is broken without it and all other PRs are blocked.

bors r+

bors bot added a commit that referenced this pull request Jul 2, 2017
633: Skip the mount tests on kernel 4.4.0 r=asomers

Some versions of that kernel have a known bug with tmpfs in namespaces.
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1659087

Fixes #610
@bors
Copy link
Contributor

bors bot commented Jul 2, 2017

Build failed

@asomers
Copy link
Member Author

asomers commented Jul 2, 2017

bors failed because test_aio_cancel_all failed for powerpc on linux. I haven't seen that before, and I don't know what could cause it. I'm going to try again, because we need this mount fix.

bors r+

bors bot added a commit that referenced this pull request Jul 2, 2017
633: Skip the mount tests on kernel 4.4.0 r=asomers

Some versions of that kernel have a known bug with tmpfs in namespaces.
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1659087

Fixes #610
@bors
Copy link
Contributor

bors bot commented Jul 2, 2017

@bors bors bot merged commit 899d20b into nix-rust:master Jul 2, 2017
@asomers asomers deleted the issue_610 branch July 2, 2017 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants