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.

Sign up for GitHub

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

Jump to bottom

Drop support for guest Linux 4.14 #4804

Merged
merged 6 commits into from
Sep 17, 2024
Merged

Drop support for guest Linux 4.14 #4804

merged 6 commits into from
Sep 17, 2024

Conversation

pb8o
Copy link
Contributor

@pb8o pb8o commented Sep 16, 2024 •
edited
Loading

Changes

Drop support for Linux guest 4.14

Reason

Linux guest support was deprecated in v1.9, remove support here.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.

PR Checklist

  • If a specific issue led to this PR, this PR closes the issue.
  • The description of changes is clear and encompassing.
  • Any required documentation changes (code and docs) are included in this
    PR.
  • API changes follow the Runbook for Firecracker API changes.
  • User-facing changes are mentioned in CHANGELOG.md.
  • All added/changed functionality is tested.
  • New TODOs link to an issue.
  • Commits meet
    contribution quality standards.
  • This functionality cannot be added in rust-vmm.

@pb8o pb8o added Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` Status: Awaiting author Indicates that an issue or pull request requires author action labels Sep 16, 2024
@pb8o pb8o self-assigned this Sep 16, 2024
@pb8o pb8o requested a review from roypat September 16, 2024 09:06
@codecov Codecov
Copy link

codecov bot commented Sep 16, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.32%. Comparing base (24819a0) to head (77bafa7).
Report is 3 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #4804   +/-   ##
=======================================
  Coverage   84.32%   84.32%           
=======================================
  Files         249      249           
  Lines       27512    27512           
=======================================
+ Hits        23199    23200    +1     
+ Misses       4313     4312    -1
Flag Coverage Δ
5.10-c5n.metal 84.54% <ø> (ø)
5.10-m5n.metal 84.53% <ø> (ø)
5.10-m6a.metal 83.82% <ø> (ø)
5.10-m6g.metal 80.90% <ø> (ø)
5.10-m6i.metal 84.52% <ø> (-0.01%) ⬇️
5.10-m7g.metal 80.90% <ø> (ø)
6.1-c5n.metal 84.54% <ø> (?)
6.1-m5n.metal 84.53% <ø> (?)
6.1-m6a.metal 83.82% <ø> (?)
6.1-m6g.metal 80.89% <ø> (?)
6.1-m6i.metal 84.52% <ø> (?)
6.1-m7g.metal 80.90% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pb8o pb8o force-pushed the drop-guest-414 branch 2 times, most recently from 8d8c5a9 to b7b0083 Compare September 16, 2024 10:16
@roypat
Copy link
Contributor

roypat commented Sep 16, 2024

Closes #4778

@pb8o pb8o force-pushed the drop-guest-414 branch 2 times, most recently from 66529e0 to 2c91159 Compare September 16, 2024 12:14
@pb8o pb8o marked this pull request as ready for review September 16, 2024 15:01
roypat
roypat reviewed Sep 16, 2024
View reviewed changes
Copy link
Contributor

@roypat roypat left a comment

Choose a reason for hiding this comment

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

Since #4801 got merged, this now needs

diff --git a/resources/guest_configs/patches/0003-enable-ftrace.patch b/resources/guest_configs/patches/0003-enable-ftrace.patch
index ddc44888d..9c64d8bb3 100644
--- a/resources/guest_configs/patches/0003-enable-ftrace.patch
+++ b/resources/guest_configs/patches/0003-enable-ftrace.patch
@@ -1,26 +1,3 @@
-diff --git a/resources/guest_configs/microvm-kernel-ci-aarch64-4.14.config b/resources/guest_configs/microvm-kernel-ci-aarch64-4.14.config
-index 694058ca9..bf06466ab 100644
---- a/resources/guest_configs/microvm-kernel-ci-aarch64-4.14.config
-+++ b/resources/guest_configs/microvm-kernel-ci-aarch64-4.14.config
-@@ -2482,7 +2482,17 @@ CONFIG_HAVE_FTRACE_MCOUNT_RECORD=y
- CONFIG_HAVE_SYSCALL_TRACEPOINTS=y
- CONFIG_HAVE_C_RECORDMCOUNT=y
- CONFIG_TRACING_SUPPORT=y
--# CONFIG_FTRACE is not set
-+CONFIG_FTRACE=y
-+CONFIG_FUNCTION_TRACER=y
-+CONFIG_FUNCTION_GRAPH_TRACER=y
-+CONFIG_IRQSOFF_TRACER=y
-+CONFIG_PREEMPT_TRACER=y
-+CONFIG_SCHED_TRACER=y
-+CONFIG_STACK_TRACER=y
-+CONFIG_BLK_DEV_IO_TRACE=y
-+CONFIG_FUNCTION_PROFILER=y
-+CONFIG_FTRACE_MCOUNT_RECORD=y
-+CONFIG_FTRACE_SYSCALLS=y
- # CONFIG_DMA_API_DEBUG is not set
- # CONFIG_KUNIT is not set
- 
 diff --git a/resources/guest_configs/microvm-kernel-ci-aarch64-5.10.config b/resources/guest_configs/microvm-kernel-ci-aarch64-5.10.config
 index ac44904c1..2a6f2310f 100644
 --- a/resources/guest_configs/microvm-kernel-ci-aarch64-5.10.config
@@ -66,30 +43,7 @@ index 26b87a658..f0f765298 100644
 +CONFIG_FTRACE_SYSCALLS=y
  # CONFIG_SAMPLES is not set
  # CONFIG_STRICT_DEVMEM is not set
- 
-diff --git a/resources/guest_configs/microvm-kernel-ci-x86_64-4.14.config b/resources/guest_configs/microvm-kernel-ci-x86_64-4.14.config
-index ee6df5ffc..3ab74b395 100644
---- a/resources/guest_configs/microvm-kernel-ci-x86_64-4.14.config
-+++ b/resources/guest_configs/microvm-kernel-ci-x86_64-4.14.config
-@@ -2694,7 +2694,17 @@ CONFIG_HAVE_SYSCALL_TRACEPOINTS=y
- CONFIG_HAVE_FENTRY=y
- CONFIG_HAVE_C_RECORDMCOUNT=y
- CONFIG_TRACING_SUPPORT=y
--# CONFIG_FTRACE is not set
-+CONFIG_FTRACE=y
-+CONFIG_FUNCTION_TRACER=y
-+CONFIG_FUNCTION_GRAPH_TRACER=y
-+CONFIG_IRQSOFF_TRACER=y
-+CONFIG_PREEMPT_TRACER=y
-+CONFIG_SCHED_TRACER=y
-+CONFIG_STACK_TRACER=y
-+CONFIG_BLK_DEV_IO_TRACE=y
-+CONFIG_FUNCTION_PROFILER=y
-+CONFIG_FTRACE_MCOUNT_RECORD=y
-+CONFIG_FTRACE_SYSCALLS=y
- # CONFIG_PROVIDE_OHCI1394_DMA_INIT is not set
- # CONFIG_DMA_API_DEBUG is not set
- # CONFIG_KUNIT is not set
+
 diff --git a/resources/guest_configs/microvm-kernel-ci-x86_64-5.10-no-acpi.config b/resources/guest_configs/microvm-kernel-ci-x86_64-5.10-no-acpi.config
 index b87fb3e44..fc45dda19 100644
 --- a/resources/guest_configs/microvm-kernel-ci-x86_64-5.10-no-acpi.config

I think

pb8o added 3 commits September 16, 2024 20:23
@pb8o
test: remove old boottime test
b41c69b 
Removing these tests as we don't emit metrics from them and they are
still using kernel 4.14.

There is no point on preserving these tests as we have now track boot
time in test_boottime.py

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
@pb8o
ci: drop support for guest kernel 4.14
2fdb523 
Remove support for generating kernel 4.14 and tests.

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
@pb8o
tests: pass -vv option to pytest
8c348f7 
This should help in cases where it truncates the output.

    E   AssertionError: assert {'aes', 'asim...simdrdm', ...} [...]
    E     Extra items in the left set:
    E     'fp'
    E     'asimdhp'
    E     'sha2'
    E     'ssbs'
    E     'asimd'
    E     'crc32'...
    E
    E     ...Full output truncated (47 lines hidden), use '-vv' to show

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
@pb8o pb8o added Status: Awaiting review Indicates that a pull request is ready to be reviewed and removed Status: Awaiting author Indicates that an issue or pull request requires author action labels Sep 16, 2024
xmarcalx
xmarcalx approved these changes Sep 17, 2024
View reviewed changes
Copy link
Contributor

@xmarcalx xmarcalx left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the changes Pablo!

@pb8o pb8o merged commit a4b3d93 into firecracker-microvm:main Sep 17, 2024
6 of 7 checks passed
@pb8o pb8o deleted the drop-guest-414 branch September 17, 2024 16:15
@pb8o pb8o mentioned this pull request Sep 17, 2024
Closed
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roypat roypat roypat approved these changes

@kalyazin kalyazin kalyazin approved these changes

@xmarcalx xmarcalx xmarcalx approved these changes

Assignees

@pb8o pb8o

Labels
Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` Status: Awaiting review Indicates that a pull request is ready to be reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pb8o @roypat @kalyazin @xmarcalx