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
diff --git a/compos/common/Android.bp b/compos/common/Android.bp
index d8fec81..5893fd6 100644
--- a/compos/common/Android.bp
+++ b/compos/common/Android.bp
@@ -14,6 +14,7 @@
"libbinder_rpc_unstable_bindgen",
"libbinder_rs",
"liblog_rust",
+ "librustutils",
],
shared_libs: [
"libbinder_rpc_unstable",
diff --git a/compos/common/compos_client.rs b/compos/common/compos_client.rs
index 6277a55..c60a602 100644
--- a/compos/common/compos_client.rs
+++ b/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::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 @@
}
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")
diff --git a/compos/common/lib.rs b/compos/common/lib.rs
index 0b84a28..4bfa81f 100644
--- a/compos/common/lib.rs
+++ b/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;
diff --git a/compos/common/timeouts.rs b/compos/common/timeouts.rs
new file mode 100644
index 0000000..42cfe69
--- /dev/null
+++ b/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),
+};
diff --git a/compos/composd/Android.bp b/compos/composd/Android.bp
index 8391ed6..ecfea61 100644
--- a/compos/composd/Android.bp
+++ b/compos/composd/Android.bp
@@ -19,7 +19,6 @@
"libcomposd_native_rust",
"libnum_traits",
"liblog_rust",
- "librustutils",
"libshared_child",
],
proc_macros: ["libnum_derive"],
diff --git a/compos/composd/src/odrefresh.rs b/compos/composd/src/odrefresh.rs
index 46ea0c0..16dcb0f 100644
--- a/compos/composd/src/odrefresh.rs
+++ b/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 @@
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))
diff --git a/compos/composd_cmd/Android.bp b/compos/composd_cmd/Android.bp
index 0081a0d..c230e13 100644
--- a/compos/composd_cmd/Android.bp
+++ b/compos/composd_cmd/Android.bp
@@ -11,6 +11,7 @@
"libanyhow",
"libbinder_rs",
"libclap",
+ "libcompos_common",
],
prefer_rlib: true,
apex_available: [
diff --git a/compos/composd_cmd/composd_cmd.rs b/compos/composd_cmd/composd_cmd.rs
index 0b7bd25..baad035 100644
--- a/compos/composd_cmd/composd_cmd.rs
+++ b/compos/composd_cmd/composd_cmd.rs
@@ -25,6 +25,7 @@
};
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 @@
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) => {