Extract timeouts to compos_common

Centralise the logic for deciding if we need to allow for nested
virtualization. Put the timeouts in one place to remove
duplication. Extend the logic to composd_cmd which was previously
always using the long timeout (which is very annoying for manual
testing).

Bug: 186126194
Test: Presubmits
Change-Id: I7104fc19616e4c745a11c228ada2a0b5be00e8ed

compos/common/Android.bp

@@ -14,6 +14,7 @@ rust_library {
         "libbinder_rpc_unstable_bindgen",
         "libbinder_rs",
         "liblog_rust",
+        "librustutils",
     ],
     shared_libs: [
         "libbinder_rpc_unstable",

compos/common/compos_client.rs

@@ -16,6 +16,7 @@
 
 //! Support for starting CompOS in a VM and connecting to the service
 
+use crate::timeouts::timeouts;
 use crate::{COMPOS_APEX_ROOT, COMPOS_DATA_ROOT, COMPOS_VSOCK_PORT};
 use android_system_virtualizationservice::aidl::android::system::virtualizationservice::{
     IVirtualMachine::IVirtualMachine,
@@ -42,7 +43,6 @@ use std::os::unix::io::IntoRawFd;
 use std::path::Path;
 use std::sync::{Arc, Condvar, Mutex};
 use std::thread;
-use std::time::Duration;
 
 /// This owns an instance of the CompOS VM.
 pub struct VmInstance {
@@ -235,14 +235,13 @@ impl VmStateMonitor {
     }
 
     fn wait_until_ready(&self) -> Result<i32> {
-        // 10s is long enough on real hardware, but it can take 90s when using nested
-        // virtualization.
-        // TODO(b/200924405): Reduce timeout/detect nested virtualization
         let (state, result) = self
             .state_ready
-            .wait_timeout_while(self.mutex.lock().unwrap(), Duration::from_secs(120), |state| {
-                state.cid.is_none() && !state.has_died
-            })
+            .wait_timeout_while(
+                self.mutex.lock().unwrap(),
+                timeouts()?.vm_max_time_to_ready,
+                |state| state.cid.is_none() && !state.has_died,
+            )
             .unwrap();
         if result.timed_out() {
             bail!("Timed out waiting for VM")

compos/common/lib.rs

@@ -17,6 +17,7 @@
 //! Common items used by CompOS server and/or clients
 
 pub mod compos_client;
+pub mod timeouts;
 
 /// Special CID indicating "any".
 pub const VMADDR_CID_ANY: u32 = -1i32 as u32;

compos/common/timeouts.rs

@@ -0,0 +1,64 @@
+/*
+ * Copyright 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+//! Timeouts for common situations, with support for longer timeouts when using nested
+//! virtualization.
+
+use anyhow::Result;
+use rustutils::system_properties;
+use std::time::Duration;
+
+/// Holder for the various timeouts we use.
+#[derive(Debug, Copy, Clone)]
+pub struct Timeouts {
+    /// Total time that odrefresh may take to perform compilation
+    pub odrefresh_max_execution_time: Duration,
+    /// Time allowed for a single compilation step run by odrefresh
+    pub odrefresh_max_child_process_time: Duration,
+    /// Time allowed for the CompOS VM to start up and become ready.
+    pub vm_max_time_to_ready: Duration,
+}
+
+/// Whether the current platform requires extra time for operations inside a VM.
+pub fn need_extra_time() -> Result<bool> {
+    // Nested virtualization is slow. Check if we are running on vsoc as a proxy for this.
+    let value = system_properties::read("ro.build.product")?;
+    Ok(value == "vsoc_x86_64" || value == "vsoc_x86")
+}
+
+/// Return the timeouts that are appropriate on the current platform.
+pub fn timeouts() -> Result<&'static Timeouts> {
+    if need_extra_time()? {
+        Ok(&EXTENDED_TIMEOUTS)
+    } else {
+        Ok(&NORMAL_TIMEOUTS)
+    }
+}
+
+/// The timeouts that we use normally.
+pub const NORMAL_TIMEOUTS: Timeouts = Timeouts {
+    // Note: the source of truth for these odrefresh timeouts is art/odrefresh/odr_config.h.
+    odrefresh_max_execution_time: Duration::from_secs(300),
+    odrefresh_max_child_process_time: Duration::from_secs(90),
+    vm_max_time_to_ready: Duration::from_secs(10),
+};
+
+/// The timeouts that we use when need_extra_time() returns true.
+pub const EXTENDED_TIMEOUTS: Timeouts = Timeouts {
+    odrefresh_max_execution_time: Duration::from_secs(480),
+    odrefresh_max_child_process_time: Duration::from_secs(150),
+    vm_max_time_to_ready: Duration::from_secs(120),
+};

compos/composd/Android.bp

@@ -19,7 +19,6 @@ rust_binary {
         "libcomposd_native_rust",
         "libnum_traits",
         "liblog_rust",
-        "librustutils",
         "libshared_child",
     ],
     proc_macros: ["libnum_derive"],

compos/composd/src/odrefresh.rs

@@ -17,10 +17,10 @@
 //! Handle the details of executing odrefresh to generate compiled artifacts.
 
 use anyhow::{bail, Context, Result};
+use compos_common::timeouts::{need_extra_time, EXTENDED_TIMEOUTS};
 use compos_common::VMADDR_CID_ANY;
 use num_derive::FromPrimitive;
 use num_traits::FromPrimitive;
-use rustutils::system_properties;
 use shared_child::SharedChild;
 use std::process::Command;
 
@@ -43,18 +43,20 @@ pub struct Odrefresh {
     child: SharedChild,
 }
 
-fn need_extra_time() -> Result<bool> {
-    // Special case to add more time in nested VM
-    let value = system_properties::read("ro.build.product")?;
-    Ok(value == "vsoc_x86_64" || value == "vsoc_x86")
-}
-
 impl Odrefresh {
     pub fn spawn_forced_compile(target_dir: &str) -> Result<Self> {
         // We don`t need to capture stdout/stderr - odrefresh writes to the log
         let mut cmdline = Command::new(ODREFRESH_BIN);
         if need_extra_time()? {
-            cmdline.arg("--max-execution-seconds=480").arg("--max-child-process-seconds=150");
+            cmdline
+                .arg(format!(
+                    "--max-execution-seconds={}",
+                    EXTENDED_TIMEOUTS.odrefresh_max_execution_time.as_secs()
+                ))
+                .arg(format!(
+                    "--max-child-process-seconds={}",
+                    EXTENDED_TIMEOUTS.odrefresh_max_child_process_time.as_secs()
+                ));
         }
         cmdline
             .arg(format!("--use-compilation-os={}", VMADDR_CID_ANY as i32))

compos/composd_cmd/Android.bp

@@ -11,6 +11,7 @@ rust_binary {
         "libanyhow",
         "libbinder_rs",
         "libclap",
+        "libcompos_common",
     ],
     prefer_rlib: true,
     apex_available: [

compos/composd_cmd/composd_cmd.rs

@@ -25,6 +25,7 @@ use android_system_composd::{
 };
 use anyhow::{bail, Context, Result};
 use binder::BinderFeatures;
+use compos_common::timeouts::timeouts;
 use std::sync::{Arc, Condvar, Mutex};
 use std::time::Duration;
 
@@ -115,7 +116,7 @@ fn run_forced_compile_for_test() -> Result<()> {
 
     println!("Waiting");
 
-    match state.wait(Duration::from_secs(480)) {
+    match state.wait(timeouts()?.odrefresh_max_execution_time) {
         Ok(Outcome::Succeeded) => Ok(()),
         Ok(Outcome::Failed) => bail!("Compilation failed"),
         Err(e) => {