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

Add support for building with system Rust toolchain (glibc) #888

Merged
merged 6 commits into from
Mar 17, 2019

Conversation

blitz
Copy link
Contributor

@blitz blitz commented Jan 24, 2019

This pull request adds support for building Firecracker with the default Rust toolchain and without the docker container. This is nice, because building Firecracker on Ubuntu 18.04 is now as simple as:

sudo apt install rustc cargo
cargo build --target x86_64-unknown-linux-gnu

No Docker is required and building uses familiar Rust tools directly.

#647 tried to achieve the same. I'll let the Firecracker experts decide whether any of the changes in that PR are still necessary.

@andreeaflorescu was so nice to add integration testing (and went slightly overboard with refactoring 👍 ).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@andreeaflorescu
Copy link
Member

With the refactoring of the build test, we will get the following output:

0.17s call     integration_tests/build/test_build.py::test_build[-debug-x86_64-unknown-linux-gnu]
0.16s call     integration_tests/build/test_build.py::test_build[vsock-release-x86_64-unknown-linux-gnu]
0.16s call     integration_tests/build/test_build.py::test_build[vsock-release-x86_64-unknown-linux-musl]
0.16s call     integration_tests/build/test_build.py::test_build[vsock-debug-x86_64-unknown-linux-gnu]
0.16s call     integration_tests/build/test_build.py::test_build[vsock-debug-x86_64-unknown-linux-musl]
0.15s call     integration_tests/build/test_build.py::test_build[-release-x86_64-unknown-linux-gnu]
0.15s call     integration_tests/build/test_build.py::test_build[-debug-x86_64-unknown-linux-musl]
0.15s call     integration_tests/build/test_build.py::test_build[-release-x86_64-unknown-linux-musl]

Between the square brackets we will get the features, the build type and the build target in this order.

@alexandruag
Copy link
Contributor

Hi, everything looks really nice but I was thinking that we defer merging this a bit more until we make the switch to Rust 1.32, update the syscall whitelist, and see which additional syscalls have to be allowed when building with glibc so that version is also usable with seccomp filtering. Also, it would be great if we add a couple of words to the Getting Started or FAQ docs regarding building Firecracker without the container with glibc, and that we strongly recommend using the same Rust version as the one in the container.

@andreeaflorescu
Copy link
Member

@blitz Firecracker is now built with rust version 1.32. Do you want to update your PR as per Alex suggestions?

@blitz
Copy link
Contributor Author

blitz commented Feb 6, 2019

I'm going to try looking into this tomorrow.

@blitz
Copy link
Contributor Author

blitz commented Feb 7, 2019

Updating the seccomp whitelist is still left to be done, but feel free to comment on the blurb I added to the FAQ.

@andreeaflorescu
Copy link
Member

@blitz the FAQ looks good to me, please update the seccomp whitelist.

@blitz blitz force-pushed the glibc-build branch 2 times, most recently from d1eb73a to cb80f60 Compare February 27, 2019 10:11
@blitz
Copy link
Contributor Author

blitz commented Feb 27, 2019

I've updated the seccomp filters for glibc. At least locally everything works for me now. This PR will likely conflict with #961.

FAQ.md Outdated
@@ -72,6 +72,19 @@ Firecracker supports Linux host and guest operating systems with kernel versions
4.14 and above. The long-term support plan is still under discussion. A leading
option is to support Firecracker for the last two Linux stable branch releases.

### What toolchains are supported by Firecracker?
Copy link
Contributor

Choose a reason for hiding this comment

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

To me, the gist of this section feels more at home in the Getting the Firecracker Binary from docs/getting-started.md (and maybe we should rename that section to Building the Firecracker Binaries btw). README.md mentions how to build using the container, and sends ppl to getting-started.md for more build information, so everything should be easy to find.

@andreeaflorescu
Copy link
Member

@blitz can you please update the PR as per Alex suggestion so we can merge this?

blitz and others added 3 commits March 14, 2019 00:55
Signed-off-by: Julian Stecklina <js@alien8.de>
Signed-off-by: Julian Stecklina <js@alien8.de>
The test_build cartesian product now also includes the build target.

Signed-off-by: Andreea Florescu <fandree@amazon.com>
Signed-off-by: Julian Stecklina <jsteckli@amazon.de>
@blitz
Copy link
Contributor Author

blitz commented Mar 14, 2019

I've moved the docs to the place @alexandruag suggested and rebased the branch.

@alexandruag alexandruag merged commit 942f6c3 into firecracker-microvm:master Mar 17, 2019
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.

5 participants