From 2364d47f12ddc5f6feff0f65b61c06a51ad7baf9 Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Tue, 25 Feb 2025 12:59:39 +0000 Subject: [PATCH 1/3] test: Modernize test_send_ctrl_alt_del Instead of sleeping for 2 seconds to wait for booting, add a network device and wait for ssh availability (happens automatically in .start()). Use mark_killed() to assert that Firecracker exited instead of "waitpid". Using `waitpid(2)` here is wrong, because it only works on child processes, and Firecracker is no child of the pytest process (it is daemonized, for starters). The reason we never caught this being broken is because the test also simply ignores all exceptions raised. But there's more issues here: The test never caused _killed to be set to `True`, so if Firecracker really did exit, then we would have been seeing intermittent failures during microvm teardown (as Microvm.kill() asserts that the process is still alive if _killed is False). In fact, our guest kernels do not have the required drivers compiled in to support CTRL+ALT+DEL (thanks Riccardo for figuring this part out). Signed-off-by: Patrick Roy --- tests/integration_tests/functional/test_api.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/tests/integration_tests/functional/test_api.py b/tests/integration_tests/functional/test_api.py index b21e86e0d60..eb2cd1b608e 100644 --- a/tests/integration_tests/functional/test_api.py +++ b/tests/integration_tests/functional/test_api.py @@ -767,24 +767,14 @@ def test_send_ctrl_alt_del(uvm_plain_any): test_microvm.spawn() test_microvm.basic_config() + test_microvm.add_net_iface() test_microvm.start() - # Wait around for the guest to boot up and initialize the user space - time.sleep(2) - test_microvm.api.actions.put(action_type="SendCtrlAltDel") - firecracker_pid = test_microvm.firecracker_pid - # If everything goes as expected, the guest OS will issue a reboot, # causing Firecracker to exit. - # waitpid should block until the Firecracker process has exited. If - # it has already exited by the time we call waitpid, WNOHANG causes - # waitpid to raise a ChildProcessError exception. - try: - os.waitpid(firecracker_pid, os.WNOHANG) - except ChildProcessError: - pass + test_microvm.mark_killed() def _drive_patch(test_microvm, io_engine): From e75afb40a9daa4369d80a692e2309157df2de84d Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Wed, 12 Mar 2025 16:52:48 +0000 Subject: [PATCH 2/3] fix: drop i8042.nopnp argument from default kernel cmdline Since Firecracker gained ACPI support, disabling the ACPI probing of the keyboard device caused the CTRL+DEL+ALT functionality to no longer work unless the user explicitly modified the default kernel command lines in the PUT /machine-config endpoint. Restore pre-ACPI behavior of not needing to do this by dropping this parameter from the default set. Signed-off-by: Patrick Roy --- src/vmm/src/vmm_config/boot_source.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/vmm/src/vmm_config/boot_source.rs b/src/vmm/src/vmm_config/boot_source.rs index d62338fc94b..37ba08be449 100644 --- a/src/vmm/src/vmm_config/boot_source.rs +++ b/src/vmm/src/vmm_config/boot_source.rs @@ -14,10 +14,9 @@ use serde::{Deserialize, Serialize}; /// - `8250.nr_uarts=0` disable 8250 serial interface; /// - `i8042.noaux` do not probe the i8042 controller for an attached mouse (save boot time); /// - `i8042.nomux` do not probe i8042 for a multiplexing controller (save boot time); -/// - `i8042.nopnp` do not use ACPIPnP to discover KBD/AUX controllers (save boot time); /// - `i8042.dumbkbd` do not attempt to control kbd state via the i8042 (save boot time). -pub const DEFAULT_KERNEL_CMDLINE: &str = "reboot=k panic=1 pci=off nomodule 8250.nr_uarts=0 \ - i8042.noaux i8042.nomux i8042.nopnp i8042.dumbkbd"; +pub const DEFAULT_KERNEL_CMDLINE: &str = + "reboot=k panic=1 pci=off nomodule 8250.nr_uarts=0 i8042.noaux i8042.nomux i8042.dumbkbd"; /// Strongly typed data structure used to configure the boot source of the /// microvm. From 8a88560446f2d7c39b5beece3c6037d9808e2c11 Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Thu, 13 Mar 2025 16:47:02 +0000 Subject: [PATCH 3/3] doc: add changelog entry about fixing CTRL+ALT+DEL add a changelog entry to let people know we fixed it Signed-off-by: Patrick Roy --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 965c763e524..b904f448b6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,11 @@ and this project adheres to ### Fixed +- #\[[5074](https://github.com/firecracker-microvm/firecracker/pull/5074)\] Fix + the `SendCtrlAltDel` command not working for ACPI-enabled guest kernels, by + dropping the i8042.nopnp argument from the default kernel command line + Firecracker constructs. + ## [1.11.0] ### Added