microdroid_manager: wait for failure reason to transmit
Writing only 16 bytes at a time makes the race condition less likely to
happen, but, theoretically, it isn't a complete fix. Instead, use
`tcdrain` to block until the serial port driver knows all the data has
been trasmitted.
In the very unlikely case that the failure reason is larger than the
pipe capacity, this could cause a deadlock, so the pipe read is moved
into a new thread.
I was not able to reproduce the race condition either before or after
this change.
Bug: 220071963
Test: packages/modules/Virtualization/android/vm/vm_shell.sh start-microdroid
Change-Id: I3626e942f4a4ec86422c40befb28ee34f2682f22
diff --git a/android/virtmgr/src/crosvm.rs b/android/virtmgr/src/crosvm.rs
index 94379a9..b0944fc 100644
--- a/android/virtmgr/src/crosvm.rs
+++ b/android/virtmgr/src/crosvm.rs
@@ -475,10 +475,23 @@
fn monitor_vm_exit(
&self,
child: Arc<SharedChild>,
- mut failure_pipe_read: File,
+ failure_pipe_read: File,
vfio_devices: Vec<VfioDevice>,
tap: Option<File>,
) {
+ let failure_reason_thread = std::thread::spawn(move || {
+ // Read the pipe to see if any failure reason is written
+ let mut failure_reason = String::new();
+ // Arbitrary max size in case of misbehaving guest.
+ const MAX_SIZE: u64 = 50_000;
+ match failure_pipe_read.take(MAX_SIZE).read_to_string(&mut failure_reason) {
+ Err(e) => error!("Error reading VM failure reason from pipe: {}", e),
+ Ok(len) if len > 0 => error!("VM returned failure reason '{}'", &failure_reason),
+ _ => (),
+ };
+ failure_reason
+ });
+
let result = child.wait();
match &result {
Err(e) => error!("Error waiting for crosvm({}) instance to die: {}", child.id(), e),
@@ -492,20 +505,14 @@
}
}
+ let failure_reason = failure_reason_thread.join().expect("failure_reason_thread panic'd");
+
let mut vm_state = self.vm_state.lock().unwrap();
*vm_state = VmState::Dead;
// Ensure that the mutex is released before calling the callbacks.
drop(vm_state);
info!("{} exited", &self);
- // Read the pipe to see if any failure reason is written
- let mut failure_reason = String::new();
- match failure_pipe_read.read_to_string(&mut failure_reason) {
- Err(e) => error!("Error reading VM failure reason from pipe: {}", e),
- Ok(len) if len > 0 => info!("VM returned failure reason '{}'", &failure_reason),
- _ => (),
- };
-
// In case of hangup, the pipe doesn't give us any information because the hangup can't be
// detected on the VM side (otherwise, it isn't a hangup), but in the
// monitor_payload_hangup function below which updates the payload state to Hangup.