Skip to content

Commit

Permalink
fix: Refactor error propagation
Browse files Browse the repository at this point in the history
Refactors error propagation to avoid logging and printing misleading
error messages.

Signed-off-by: Jonathan Woollett-Light <[email protected]>
  • Loading branch information
Jonathan Woollett-Light authored and roypat committed Oct 20, 2023
1 parent e6fb1f3 commit 92bed78
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 9 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@

## Unreleased

### Changed

- [#4191](https://github.com/firecracker-microvm/firecracker/pull/4191):
Refactored error propagation to avoid logging and printing an error on
exits with a zero exit code. Now, on successful exit
"Firecracker exited successfully" is logged.

### Fixed

- [#4179](https://github.com/firecracker-microvm/firecracker/pull/4179):
Expand Down
13 changes: 9 additions & 4 deletions src/firecracker/src/api_server_adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl ApiServerAdapter {
vm_resources: VmResources,
vmm: Arc<Mutex<Vmm>>,
event_manager: &mut EventManager,
) -> FcExitCode {
) -> Result<(), FcExitCode> {
let api_adapter = Arc::new(Mutex::new(Self {
api_event_fd,
from_api,
Expand All @@ -64,10 +64,14 @@ impl ApiServerAdapter {
event_manager
.run()
.expect("EventManager events driver fatal error");
if let Some(exit_code) = vmm.lock().unwrap().shutdown_exit_code() {
return exit_code;

match vmm.lock().unwrap().shutdown_exit_code() {
Some(FcExitCode::Ok) => break,
Some(exit_code) => return Err(exit_code),
None => continue,
}
}
Ok(())
}

fn handle_request(&mut self, req_action: VmmAction) {
Expand Down Expand Up @@ -245,7 +249,8 @@ pub(crate) fn run_with_api(
api_thread.join().expect("Api thread should join");

match result {
Ok(exit_code) => Err(ApiServerError::MicroVMStoppedWithoutError(exit_code)),
Ok(Ok(())) => Ok(()),
Ok(Err(exit_code)) => Err(ApiServerError::MicroVMStoppedWithoutError(exit_code)),
Err(exit_error) => Err(exit_error),
}
}
8 changes: 6 additions & 2 deletions src/firecracker/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ fn main() -> ExitCode {
eprintln!("Error: {err:?}");
ExitCode::from(err)
} else {
info!("Firecracker exited successfully");
ExitCode::SUCCESS
}
}
Expand Down Expand Up @@ -631,8 +632,11 @@ fn run_without_api(
.run()
.expect("Failed to start the event manager");

if let Some(exit_code) = vmm.lock().unwrap().shutdown_exit_code() {
return Err(RunWithoutApiError::Shutdown(exit_code));
match vmm.lock().unwrap().shutdown_exit_code() {
Some(FcExitCode::Ok) => break,
Some(exit_code) => return Err(RunWithoutApiError::Shutdown(exit_code)),
None => continue,
}
}
Ok(())
}
4 changes: 1 addition & 3 deletions tests/integration_tests/functional/test_cmd_line_start.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,7 @@ def test_config_start_no_api_exit(uvm_plain, vm_config_file):
time.sleep(3) # Wait for shutdown

# Check error log
test_microvm.check_log_message(
"RunWithoutApiError error: MicroVMStopped without an error: Ok"
)
test_microvm.check_log_message("Firecracker exited successfully")


@pytest.mark.parametrize(
Expand Down

0 comments on commit 92bed78

Please sign in to comment.