De-dupe running odrefresh in VM
We had two copies of this code, one destined to be deleted. But I want
to modify it a bit, so first I'm refactoring to remove the
duplication.
This is intended as pretty much a pure refactoring. There is an
existing issue around target dir (.../com.android.art/dalvik-cache)
and staging dir (.../com.android.art/staging) which I am preserving
for now; next CL will resolve this.
Bug: 209572296
Test: composd_cmd forced-odrefresh
Test: composd_cmd async-odrefresh
Test: (both with selinux in microdroid disabled)
Change-Id: I4dd0fa843662ffb9acbcf6ce0dfadbd4ececfe52
diff --git a/compos/composd/src/odrefresh.rs b/compos/composd/src/odrefresh.rs
index 9debf00..3f6bdf1 100644
--- a/compos/composd/src/odrefresh.rs
+++ b/compos/composd/src/odrefresh.rs
@@ -16,23 +16,31 @@
//! Handle the details of executing odrefresh to generate compiled artifacts.
+use crate::fd_server_helper::FdServerConfig;
use anyhow::{bail, Context, Result};
+use compos_aidl_interface::aidl::com::android::compos::ICompOsService::ICompOsService;
+use compos_aidl_interface::binder::Strong;
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::fs::{File, OpenOptions};
+use std::os::unix::fs::OpenOptionsExt;
+use std::os::unix::io::AsRawFd;
+use std::path::Path;
use std::process::Command;
// TODO: What if this changes?
-const EX_MAX: i32 = 78;
+const EX_MAX: i8 = 78;
const ODREFRESH_BIN: &str = "/apex/com.android.art/bin/odrefresh";
#[derive(Debug, PartialEq, Eq, FromPrimitive)]
-#[repr(i32)]
+#[repr(i8)]
pub enum ExitCode {
// Copied from art/odrefresh/include/odrefresh/odrefresh.h
- Okay = 0i32,
+ Okay = 0i8,
CompilationRequired = EX_MAX + 1,
CompilationSuccess = EX_MAX + 2,
CompilationFailed = EX_MAX + 3,
@@ -88,3 +96,38 @@
self.child.kill().context("Killing odrefresh process failed")
}
}
+
+pub fn run_in_vm(service: Strong<dyn ICompOsService>, _target_dir_name: &str) -> Result<ExitCode> {
+ // TODO: Use target_dir_name
+ let staging_dir = open_dir(composd_native::palette_create_odrefresh_staging_directory()?)?;
+ let system_dir = open_dir(Path::new("/system"))?;
+
+ // Spawn a fd_server to serve the FDs.
+ let fd_server_config = FdServerConfig {
+ ro_dir_fds: vec![system_dir.as_raw_fd()],
+ rw_dir_fds: vec![staging_dir.as_raw_fd()],
+ ..Default::default()
+ };
+ let fd_server_raii = fd_server_config.into_fd_server()?;
+
+ let zygote_arch = system_properties::read("ro.zygote")?;
+ let exit_code =
+ service.odrefresh(system_dir.as_raw_fd(), staging_dir.as_raw_fd(), &zygote_arch)?.exitCode;
+
+ drop(fd_server_raii);
+ if let Some(exit_code) = FromPrimitive::from_i8(exit_code) {
+ Ok(exit_code)
+ } else {
+ bail!("odrefresh exited with {}", exit_code)
+ }
+}
+
+/// Returns an owned FD of the directory. It currently returns a `File` as a FD owner, but
+/// it's better to use `std::os::unix::io::OwnedFd` once/if it becomes standard.
+fn open_dir(path: &Path) -> Result<File> {
+ OpenOptions::new()
+ .custom_flags(libc::O_DIRECTORY)
+ .read(true) // O_DIRECTORY can only be opened with read
+ .open(path)
+ .with_context(|| format!("Failed to open {:?} directory as path fd", path))
+}
diff --git a/compos/composd/src/odrefresh_task.rs b/compos/composd/src/odrefresh_task.rs
index f017f67..64565dd 100644
--- a/compos/composd/src/odrefresh_task.rs
+++ b/compos/composd/src/odrefresh_task.rs
@@ -14,21 +14,15 @@
* limitations under the License.
*/
-use crate::fd_server_helper::FdServerConfig;
use crate::instance_starter::CompOsInstance;
use crate::odrefresh;
-use crate::service::open_dir;
use android_system_composd::aidl::android::system::composd::{
ICompilationTask::ICompilationTask, ICompilationTaskCallback::ICompilationTaskCallback,
};
use android_system_composd::binder::{Interface, Result as BinderResult, Strong};
-use anyhow::{bail, Result};
+use anyhow::Result;
use compos_aidl_interface::aidl::com::android::compos::ICompOsService::ICompOsService;
use log::{error, warn};
-use num_traits::FromPrimitive;
-use rustutils::system_properties;
-use std::os::unix::io::AsRawFd;
-use std::path::{Path, PathBuf};
use std::sync::{Arc, Mutex};
use std::thread;
@@ -58,21 +52,21 @@
pub fn start(
comp_os: Arc<CompOsInstance>,
- output_dir_path: PathBuf,
+ target_dir_name: String,
callback: &Strong<dyn ICompilationTaskCallback>,
) -> Result<OdrefreshTask> {
let service = comp_os.get_service();
let task = RunningTask { comp_os, callback: callback.clone() };
let task = OdrefreshTask { running_task: Arc::new(Mutex::new(Some(task))) };
- task.clone().start_thread(service, output_dir_path);
+ task.clone().start_thread(service, target_dir_name);
Ok(task)
}
- fn start_thread(self, service: Strong<dyn ICompOsService>, output_dir_path: PathBuf) {
+ fn start_thread(self, service: Strong<dyn ICompOsService>, target_dir_name: String) {
thread::spawn(move || {
- let exit_code = try_odrefresh(service, &output_dir_path);
+ let exit_code = odrefresh::run_in_vm(service, &target_dir_name);
let task = self.take();
// We don't do the callback if cancel has already happened.
@@ -96,33 +90,6 @@
}
}
-fn try_odrefresh(
- service: Strong<dyn ICompOsService>,
- output_dir_path: &Path,
-) -> Result<odrefresh::ExitCode> {
- let output_dir = open_dir(output_dir_path)?;
- let system_dir = open_dir(Path::new("/system"))?;
-
- // Spawn a fd_server to serve the FDs.
- let fd_server_config = FdServerConfig {
- ro_dir_fds: vec![system_dir.as_raw_fd()],
- rw_dir_fds: vec![output_dir.as_raw_fd()],
- ..Default::default()
- };
- let fd_server_raii = fd_server_config.into_fd_server()?;
-
- let zygote_arch = system_properties::read("ro.zygote")?;
- let exit_code =
- service.odrefresh(system_dir.as_raw_fd(), output_dir.as_raw_fd(), &zygote_arch)?.exitCode;
-
- drop(fd_server_raii);
- if let Some(exit_code) = FromPrimitive::from_i8(exit_code) {
- Ok(exit_code)
- } else {
- bail!("odrefresh exited with {}", exit_code)
- }
-}
-
struct RunningTask {
callback: Strong<dyn ICompilationTaskCallback>,
#[allow(dead_code)] // Keeps the CompOS VM alive
diff --git a/compos/composd/src/service.rs b/compos/composd/src/service.rs
index ab28e0a..3f31adb 100644
--- a/compos/composd/src/service.rs
+++ b/compos/composd/src/service.rs
@@ -18,8 +18,8 @@
//! desired.
use crate::compilation_task::CompilationTask;
-use crate::fd_server_helper::FdServerConfig;
use crate::instance_manager::InstanceManager;
+use crate::odrefresh;
use crate::odrefresh_task::OdrefreshTask;
use crate::util::to_binder_result;
use android_system_composd::aidl::android::system::composd::{
@@ -31,11 +31,7 @@
self, BinderFeatures, ExceptionCode, Interface, Status, Strong, ThreadState,
};
use anyhow::{Context, Result};
-use rustutils::{system_properties, users::AID_ROOT, users::AID_SYSTEM};
-use std::fs::{File, OpenOptions};
-use std::os::unix::fs::OpenOptionsExt;
-use std::os::unix::io::AsRawFd;
-use std::path::Path;
+use rustutils::{users::AID_ROOT, users::AID_SYSTEM};
use std::sync::Arc;
pub struct IsolatedCompilationService {
@@ -110,12 +106,10 @@
&self,
callback: &Strong<dyn ICompilationTaskCallback>,
) -> Result<Strong<dyn ICompilationTask>> {
- let output_dir_path =
- composd_native::palette_create_odrefresh_staging_directory()?.to_path_buf();
-
let comp_os = self.instance_manager.start_test_instance().context("Starting CompOS")?;
- let task = OdrefreshTask::start(comp_os, output_dir_path, callback)?;
+ let target_dir_name = "test-instance".to_owned();
+ let task = OdrefreshTask::start(comp_os, target_dir_name, callback)?;
Ok(BnCompilationTask::new_binder(task, BinderFeatures::default()))
}
@@ -125,26 +119,10 @@
.instance_manager
.start_test_instance()
.context("Starting CompOS for odrefresh test")?;
+ let service = compos.get_service();
- let output_dir = open_dir(composd_native::palette_create_odrefresh_staging_directory()?)?;
- let system_dir = open_dir(Path::new("/system"))?;
-
- // Spawn a fd_server to serve the FDs.
- let fd_server_config = FdServerConfig {
- ro_dir_fds: vec![system_dir.as_raw_fd()],
- rw_dir_fds: vec![output_dir.as_raw_fd()],
- ..Default::default()
- };
- let fd_server_raii = fd_server_config.into_fd_server()?;
-
- let zygote_arch = system_properties::read("ro.zygote")?;
- let result = compos.get_service().odrefresh(
- system_dir.as_raw_fd(),
- output_dir.as_raw_fd(),
- &zygote_arch,
- );
- drop(fd_server_raii);
- Ok(result?.exitCode)
+ let exit_code = odrefresh::run_in_vm(service, "test-artifacts")?;
+ Ok(exit_code as i8)
}
}
@@ -157,13 +135,3 @@
Ok(())
}
}
-
-/// Returns an owned FD of the directory. It currently returns a `File` as a FD owner, but
-/// it's better to use `std::os::unix::io::OwnedFd` once/if it becomes standard.
-pub fn open_dir(path: &Path) -> Result<File> {
- OpenOptions::new()
- .custom_flags(libc::O_DIRECTORY)
- .read(true) // O_DIRECTORY can only be opened with read
- .open(path)
- .with_context(|| format!("Failed to open {:?} directory as path fd", path))
-}