Skip to content

Commit 1f1e910

Browse files
committed
fix: Ensure to exit with error if any vcpu exits with error
Previously, when a VM exited, we looked for the first vcpu that reported an exit status, and then indiscriminately reported that back. However, it is possible to one vcpu to exit successfully while another exits with an error, and this could lead us to report "Firecracker Exited Successfully" even though it did not. Now, we explicitly look for the status code of all vcpus. If any of them report an error, this takes precedence over non-error status codes. Signed-off-by: Patrick Roy <[email protected]>
1 parent 3487347 commit 1f1e910

File tree

1 file changed

+20
-16
lines changed

1 file changed

+20
-16
lines changed

src/vmm/src/lib.rs

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ pub mod vstate;
110110
use std::collections::HashMap;
111111
use std::io;
112112
use std::os::unix::io::AsRawFd;
113-
use std::sync::mpsc::{RecvTimeoutError, TryRecvError};
113+
use std::sync::mpsc::RecvTimeoutError;
114114
use std::sync::{Arc, Barrier, Mutex};
115115
use std::time::Duration;
116116

@@ -903,23 +903,27 @@ impl MutEventSubscriber for Vmm {
903903
// Exit event handling should never do anything more than call 'self.stop()'.
904904
let _ = self.vcpus_exit_evt.read();
905905

906-
let mut exit_code = None;
907-
// Query each vcpu for their exit_code.
908-
for handle in &self.vcpus_handles {
909-
match handle.response_receiver().try_recv() {
910-
Ok(VcpuResponse::Exited(status)) => {
911-
exit_code = Some(status);
912-
// Just use the first encountered exit-code.
913-
break;
914-
}
915-
Ok(_response) => {} // Don't care about these, we are exiting.
916-
Err(TryRecvError::Empty) => {} // Nothing pending in channel
917-
Err(err) => {
918-
panic!("Error while looking for VCPU exit status: {}", err);
906+
let exit_code = 'exit_code: {
907+
// Query each vcpu for their exit_code.
908+
for handle in &self.vcpus_handles {
909+
// Drain all vcpu responses that are pending from this vcpu until we find an
910+
// exit status.
911+
for response in handle.response_receiver().try_iter() {
912+
if let VcpuResponse::Exited(status) = response {
913+
// It could be that some vcpus exited successfully while others
914+
// errored out. Thus make sure that error exits from one vcpu always
915+
// takes precedence over "ok" exits
916+
if status != FcExitCode::Ok {
917+
break 'exit_code status;
918+
}
919+
}
919920
}
920921
}
921-
}
922-
self.stop(exit_code.unwrap_or(FcExitCode::Ok));
922+
923+
// No CPUs exited with error status code, report "Ok"
924+
FcExitCode::Ok
925+
};
926+
self.stop(exit_code);
923927
} else {
924928
error!("Spurious EventManager event for handler: Vmm");
925929
}

0 commit comments

Comments
 (0)