Skip to content
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

Invalid cross-device link (os error 18) when upgrading on a docker OverlayFS #1239

Open
wraithan opened this Issue Aug 21, 2017 · 5 comments

Comments

3 participants
@wraithan
Copy link

wraithan commented Aug 21, 2017

$ rustup update nightly
info: syncing channel updates for 'nightly-x86_64-unknown-linux-gnu'
info: latest update on 2017-08-21, rust version 1.21.0-nightly (8c303ed87 2017-08-20)
info: downloading component 'rustc'
info: downloading component 'rust-std'
info: downloading component 'cargo'
info: downloading component 'rust-docs'
info: removing component 'rustc'
info: rolling back changes
error: could not rename component directory from '/root/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/etc' to '/root/.rustup/tmp/x5u5mnp0hhtywco8_dir/bk'
info: caused by: Invalid cross-device link (os error 18)

std::fs::rename() basically doesn't work on OverlayFS as far as I can tell by looking at other similar reports for various languages and projects hitting cross-device link errors on OverlayFS is boils down to using the rename syscall.

I'd like to propose wrapping the std::fs::rename() calls and if on linux detect os error 18 attempt to do a copy and delete instead. There are periodic other reports of errors like this on various platforms, the wrapper could try to handle the other OS cases too if they have a similar error code (or maybe even the same one if this is standard, I'm not sure).

Interestingly there is the bootstrap/update problem where folks who are experiencing may be unable to update their rustup install and not be able get the update that fixes the problem once there is a solution. Those folks will need to be advised to reinstall their rustup.

If the proposed solution to the problem works for the dev team, I'll attempt to provide a PR within a week of getting the go ahead.

This is relevant because some people use a common Docker image for their CI environments that may not be updated frequently enough for beta/nightly and have rustup update $desired_env in their script. Which is how I found this problem.

@wraithan

This comment has been minimized.

Copy link
Author

wraithan commented Aug 21, 2017

Spoke with @nrc and @alexcrichton on IRC and they said this seemed reasonable. I'll put forward an implementation this week.

@cyplo

This comment has been minimized.

Copy link
Contributor

cyplo commented Dec 17, 2017

Heyo ! Any news on this one ? I encounter this bug regularly when doing builds on dockerized CI. Let me know if there is any more info I can provide.

@cyplo

This comment has been minimized.

Copy link
Contributor

cyplo commented Dec 17, 2017

Looking at the sources, there already exists a wrapper function called utils::rename_file, it's used by components and transaction. Would that be a good candidate here to replace every other call to fs::rename ?

@cyplo cyplo referenced this issue Jan 10, 2018

Open

Rust nightly #154

@ishitatsuyuki

This comment has been minimized.

Copy link
Member

ishitatsuyuki commented Apr 5, 2018

For those affected by this bug, see the renaming section of the kernel documentation.

@wraithan fs::rename inside std implies atomicity. For a renaming operation that doesn't fail, we should put it in a separate crate, as copying will likely involve locking.

@cyplo

This comment has been minimized.

Copy link
Contributor

cyplo commented Apr 6, 2018

Heya, thank you to @nrc for taking a look :) (https://internals.rust-lang.org/t/contributing-to-rustup-help-with-code-structure-needed/7193). I'm thinking of trying to tackle this bug, I like writing the replication test first, so would probably focus on this.; to try to inject the fault in the test and see what's what. Let me know if someone else wants to look into that as well, we can combine forces :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.