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

update kernel configs, make kernel in integ test #659

Merged
merged 1 commit into from
Jun 2, 2022

Conversation

ginglis13
Copy link
Contributor

Signed-off-by: Gavin Inglis giinglis@amazon.com

Issue #, if available:

#658

Description of changes:

in upgrading to firecracker v1.1, we noticed some configuration options
prompted for using when make kernel. These options are specific to
kernel 4.14. Additionally, this upgrade changed the build output from
firecracker's devtool to exclude the full kernel version, and installing would fail as it expects the path to include the patch number.

Some relevant links from the linux kernel driver db:

This PR also updates the integ test Docker image to use a freshly built
kernel such that FUSE is enabled.

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

@ginglis13 ginglis13 requested a review from Kern-- June 1, 2022 00:43
@ginglis13 ginglis13 requested a review from a team as a code owner June 1, 2022 00:43
kernel \
install-kernel \
Copy link
Contributor

Choose a reason for hiding this comment

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

What other options do we have other than building the kernel at runtime? This is failing the PR build due to write permissions to the build directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see... write permission may have to do with not building the image as privileged. We can investigate having a pre-built kernel for this use case.

Copy link
Contributor

Choose a reason for hiding this comment

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

How long would it take to build a Linux kernel? I'd like to provide pre-built kernel images to skip the build time if we can.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1 to pre-built kernel image for avoiding increased build time. Here are some sample times running locally.

Without the firecracker image public.ecr.aws/firecracker/fcuvm:v35 not yet built:

$ time make kernel
...
real    1m23.834s

With firecracker image already built:

$ time make kernel
...
real    0m47.606s

@@ -1208,6 +1208,7 @@ CONFIG_VIRTIO_BLK=y
# CONFIG_ENCLOSURE_SERVICES is not set
# CONFIG_SRAM is not set
# CONFIG_C2PORT is not set
# CONFIG_R3964 is not set
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these x86 specific or do we also need these options in the ARM config?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

KMEMCHECK, CRYPTO_SALSA20_X86_64, ORC_UNWINDER, and FRAME_POINTER_UNWINDER are all x86 specific. the Siemens device driver R3964 is available on both - I've updated to include that in ARM as well.

@ginglis13 ginglis13 force-pushed the kernel-config branch 2 times, most recently from a171a11 to 43e0bea Compare June 1, 2022 20:30
Makefile Outdated
@@ -60,7 +60,7 @@ KERNEL_CONFIG=tools/kernel-configs/microvm-kernel-$(host_arch)-$(KERNEL_VERSION)
# Copied from https://github.com/firecracker-microvm/firecracker/blob/v1.0.0/tools/devtool#L2015
# This allows us to specify a kernel without the patch version, but still get the correct build path to reference the kernel binary
KERNEL_FULL_VERSION=$(shell cat "$(KERNEL_CONFIG)" | grep -Po "^\# Linux\/$(kernel_config_pattern) (([0-9]+.){2}[0-9]+)" | cut -d ' ' -f 3)
Copy link
Contributor

Choose a reason for hiding this comment

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

I normally prefer to clean up unused defined variables. In this case looks like KERNEL_FULL_VERSION is no longer being used. I will let you decide if we need to keep this around for future use.

Copy link
Contributor Author

@ginglis13 ginglis13 Jun 1, 2022

Choose a reason for hiding this comment

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

found issue - the regex for kernel version slightly changed in firecracker v1.1.0: https://github.com/firecracker-microvm/firecracker/blob/v1.1.0/tools/devtool#L2082 . updated the regex in our Makefile and reverted back to using KERNEL_FULL_VERSION

in upgrading to firecracker v1.1, we noticed some configuration options
prompted for using when `make kernel`. These options are specific to
kernel 4.14. Update KERNEL_FULL_VERSION regex to that used by
firecracker v1.1.0

This PR also updates the integ test Docker image to use a freshly built
kernel such that FUSE is enabled.

Signed-off-by: Gavin Inglis <giinglis@amazon.com>
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