Call odrefresh in VM from composd
composd needs to prepare the directory, run a fd_server, etc. then
request to run odrefresh in the VM.
`FdServerConfig` and `FdServer` are introduced to make starting a
fd_server from composd easier.
Also, add a testing command in composd_cmd.
Bug: 205750213
Test: atest ComposHostTestCases
Test: With some local hacks in ART, with SELinux disabled in the VM,
odrefresh completed with exit code 80 and output files that
look normal (at least sizes are).
Change-Id: I52c9d1ad369eea6d423831adb42087a3bcf30d66
diff --git a/compos/composd/Android.bp b/compos/composd/Android.bp
index 2ab12e3..735b9a5 100644
--- a/compos/composd/Android.bp
+++ b/compos/composd/Android.bp
@@ -21,6 +21,7 @@
"libminijail_rust",
"libnix",
"libnum_traits",
+ "liblibc",
"liblog_rust",
"librustutils",
"libshared_child",
diff --git a/compos/composd/aidl/android/system/composd/IIsolatedCompilationService.aidl b/compos/composd/aidl/android/system/composd/IIsolatedCompilationService.aidl
index 6b3a6bb..6f4476c 100644
--- a/compos/composd/aidl/android/system/composd/IIsolatedCompilationService.aidl
+++ b/compos/composd/aidl/android/system/composd/IIsolatedCompilationService.aidl
@@ -21,12 +21,25 @@
interface IIsolatedCompilationService {
/**
* Run "odrefresh --dalvik-cache=pending-test --force-compile" in a test instance of CompOS.
+ *
* This compiles BCP extensions and system server, even if the system artifacts are up to date,
* and writes the results to a test directory to avoid disrupting any real artifacts in
* existence.
+ *
* Compilation continues in the background, and success/failure is reported via the supplied
* callback, unless the returned ICompilationTask is cancelled. The caller should maintain
* a reference to the ICompilationTask until compilation completes or is cancelled.
*/
ICompilationTask startTestCompile(ICompilationTaskCallback callback);
+
+ /**
+ * Run odrefresh in a test instance of CompOS until completed or failed.
+ *
+ * This compiles BCP extensions and system server, even if the system artifacts are up to date,
+ * and writes the results to a test directory to avoid disrupting any real artifacts in
+ * existence.
+ *
+ * TODO(205750213): Change the API to async.
+ */
+ byte startTestOdrefresh();
}
diff --git a/compos/composd/src/composd_main.rs b/compos/composd/src/composd_main.rs
index 57f64a5..67b5974 100644
--- a/compos/composd/src/composd_main.rs
+++ b/compos/composd/src/composd_main.rs
@@ -35,8 +35,10 @@
use std::sync::Arc;
fn try_main() -> Result<()> {
+ let debuggable = env!("TARGET_BUILD_VARIANT") != "user";
+ let log_level = if debuggable { log::Level::Debug } else { log::Level::Info };
android_logger::init_once(
- android_logger::Config::default().with_tag("composd").with_min_level(log::Level::Info),
+ android_logger::Config::default().with_tag("composd").with_min_level(log_level),
);
ProcessState::start_thread_pool();
diff --git a/compos/composd/src/fd_server_helper.rs b/compos/composd/src/fd_server_helper.rs
index 2dab9a3..24dc9e7 100644
--- a/compos/composd/src/fd_server_helper.rs
+++ b/compos/composd/src/fd_server_helper.rs
@@ -15,46 +15,92 @@
*/
//! A helper library to start a fd_server.
-//!
-//! TODO(205750213): Make it easy to spawn a fd_server.
use anyhow::{Context, Result};
-use log::debug;
+use log::{debug, warn};
use minijail::Minijail;
use nix::fcntl::OFlag;
use nix::unistd::pipe2;
use std::fs::File;
use std::io::Read;
-use std::os::unix::io::{AsRawFd, FromRawFd};
+use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
use std::path::Path;
const FD_SERVER_BIN: &str = "/apex/com.android.virt/bin/fd_server";
-#[allow(dead_code)]
-fn spawn_fd_server(input_fds: &[i32], output_fds: &[i32], ready_file: File) -> Result<Minijail> {
- let mut inheritable_fds = Vec::new();
- let mut args = vec![FD_SERVER_BIN.to_string()];
- for fd in input_fds {
- args.push("--ro-fds".to_string());
- args.push(fd.to_string());
- inheritable_fds.push(*fd);
- }
- for fd in output_fds {
- args.push("--rw-fds".to_string());
- args.push(fd.to_string());
- inheritable_fds.push(*fd);
- }
- let ready_fd = ready_file.as_raw_fd();
- args.push("--ready-fd".to_string());
- args.push(ready_fd.to_string());
- inheritable_fds.push(ready_fd);
-
- let jail = Minijail::new()?;
- let _pid = jail.run(Path::new(FD_SERVER_BIN), &inheritable_fds, &args)?;
- Ok(jail)
+/// Config for starting a `FdServer`
+#[derive(Default)]
+pub struct FdServerConfig {
+ /// List of file FDs exposed for read-only operations.
+ pub ro_file_fds: Vec<RawFd>,
+ /// List of file FDs exposed for read-write operations.
+ pub rw_file_fds: Vec<RawFd>,
+ /// List of directory FDs exposed for read-only operations.
+ pub ro_dir_fds: Vec<RawFd>,
+ /// List of directory FDs exposed for read-write operations.
+ pub rw_dir_fds: Vec<RawFd>,
}
-#[allow(dead_code)]
+impl FdServerConfig {
+ /// Creates a `FdServer` based on the current config.
+ pub fn into_fd_server(self) -> Result<FdServer> {
+ let (ready_read_fd, ready_write_fd) = create_pipe()?;
+ let fd_server_jail = self.do_spawn_fd_server(ready_write_fd)?;
+ wait_for_fd_server_ready(ready_read_fd)?;
+ Ok(FdServer { jailed_process: fd_server_jail })
+ }
+
+ fn do_spawn_fd_server(self, ready_file: File) -> Result<Minijail> {
+ let mut inheritable_fds = Vec::new();
+ let mut args = vec![FD_SERVER_BIN.to_string()];
+ for fd in self.ro_file_fds {
+ args.push("--ro-fds".to_string());
+ args.push(fd.to_string());
+ inheritable_fds.push(fd);
+ }
+ for fd in self.rw_file_fds {
+ args.push("--rw-fds".to_string());
+ args.push(fd.to_string());
+ inheritable_fds.push(fd);
+ }
+ for fd in self.ro_dir_fds {
+ args.push("--ro-dirs".to_string());
+ args.push(fd.to_string());
+ inheritable_fds.push(fd);
+ }
+ for fd in self.rw_dir_fds {
+ args.push("--rw-dirs".to_string());
+ args.push(fd.to_string());
+ inheritable_fds.push(fd);
+ }
+ let ready_fd = ready_file.as_raw_fd();
+ args.push("--ready-fd".to_string());
+ args.push(ready_fd.to_string());
+ inheritable_fds.push(ready_fd);
+
+ debug!("Spawn fd_server {:?} (inheriting FDs: {:?})", args, inheritable_fds);
+ let jail = Minijail::new()?;
+ let _pid = jail.run(Path::new(FD_SERVER_BIN), &inheritable_fds, &args)?;
+ Ok(jail)
+ }
+}
+
+/// `FdServer` represents a running `fd_server` process. The process lifetime is associated with
+/// the instance lifetime.
+pub struct FdServer {
+ jailed_process: Minijail,
+}
+
+impl Drop for FdServer {
+ fn drop(&mut self) {
+ if let Err(e) = self.jailed_process.kill() {
+ if !matches!(e, minijail::Error::Killed(_)) {
+ warn!("Failed to kill fd_server: {}", e);
+ }
+ }
+ }
+}
+
fn create_pipe() -> Result<(File, File)> {
let (raw_read, raw_write) = pipe2(OFlag::O_CLOEXEC)?;
// SAFETY: We are the sole owners of these fds as they were just created.
@@ -63,7 +109,6 @@
Ok((read_fd, write_fd))
}
-#[allow(dead_code)]
fn wait_for_fd_server_ready(mut ready_fd: File) -> Result<()> {
let mut buffer = [0];
// When fd_server is ready it closes its end of the pipe. And if it exits, the pipe is also
diff --git a/compos/composd/src/service.rs b/compos/composd/src/service.rs
index d9963d1..3738e18 100644
--- a/compos/composd/src/service.rs
+++ b/compos/composd/src/service.rs
@@ -18,7 +18,9 @@
//! desired.
use crate::compilation_task::CompilationTask;
+use crate::fd_server_helper::FdServerConfig;
use crate::instance_manager::InstanceManager;
+use crate::instance_starter::CompOsInstance;
use crate::util::to_binder_result;
use android_system_composd::aidl::android::system::composd::{
ICompilationTask::{BnCompilationTask, ICompilationTask},
@@ -29,7 +31,12 @@
self, BinderFeatures, ExceptionCode, Interface, Status, Strong, ThreadState,
};
use anyhow::{Context, Result};
-use rustutils::users::{AID_ROOT, AID_SYSTEM};
+use compos_common::COMPOS_DATA_ROOT;
+use rustutils::{system_properties, users::AID_ROOT, users::AID_SYSTEM};
+use std::fs::{create_dir, File, OpenOptions};
+use std::os::unix::fs::OpenOptionsExt;
+use std::os::unix::io::AsRawFd;
+use std::path::{Path, PathBuf};
use std::sync::Arc;
pub struct IsolatedCompilationService {
@@ -50,13 +57,14 @@
&self,
callback: &Strong<dyn ICompilationTaskCallback>,
) -> binder::Result<Strong<dyn ICompilationTask>> {
- let calling_uid = ThreadState::get_calling_uid();
- // This should only be called by system server, or root while testing
- if calling_uid != AID_SYSTEM && calling_uid != AID_ROOT {
- return Err(Status::new_exception(ExceptionCode::SECURITY, None));
- }
+ check_test_permissions()?;
to_binder_result(self.do_start_test_compile(callback))
}
+
+ fn startTestOdrefresh(&self) -> binder::Result<i8> {
+ check_test_permissions()?;
+ to_binder_result(self.do_odrefresh_for_test())
+ }
}
impl IsolatedCompilationService {
@@ -70,4 +78,62 @@
Ok(BnCompilationTask::new_binder(task, BinderFeatures::default()))
}
+
+ fn do_odrefresh_for_test(&self) -> Result<i8> {
+ let mut staging_dir_path = PathBuf::from(COMPOS_DATA_ROOT);
+ staging_dir_path.push("test-artifacts");
+ to_binder_result(create_dir(&staging_dir_path))?;
+
+ let compos = self
+ .instance_manager
+ .start_test_instance()
+ .context("Starting CompOS for odrefresh test")?;
+ self.do_odrefresh(compos, &staging_dir_path)
+ }
+
+ fn do_odrefresh(&self, compos: Arc<CompOsInstance>, staging_dir_path: &Path) -> Result<i8> {
+ let output_dir = open_dir_path(staging_dir_path)?;
+ let system_dir = open_dir_path(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)
+ }
+}
+
+fn check_test_permissions() -> binder::Result<()> {
+ let calling_uid = ThreadState::get_calling_uid();
+ // This should only be called by system server, or root while testing
+ if calling_uid != AID_SYSTEM && calling_uid != AID_ROOT {
+ Err(Status::new_exception(ExceptionCode::SECURITY, None))
+ } else {
+ Ok(())
+ }
+}
+
+/// Returns an owned FD of the directory path. 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: &Path) -> Result<File> {
+ OpenOptions::new()
+ .custom_flags(libc::O_PATH | libc::O_DIRECTORY)
+ // The custom flags above is not taken into consideration by the unix implementation of
+ // OpenOptions for flag validation. So even though the man page of open(2) says that
+ // most flags include access mode are ignored, we still need to set a "valid" mode to
+ // make the library happy. The value does not appear to matter elsewhere in the library.
+ .read(true)
+ .open(path)
+ .with_context(|| format!("Failed to open {} directory as path fd", path.display()))
}
diff --git a/compos/composd/src/util.rs b/compos/composd/src/util.rs
index 091fb15..54d7751 100644
--- a/compos/composd/src/util.rs
+++ b/compos/composd/src/util.rs
@@ -18,8 +18,9 @@
use anyhow::Result;
use binder_common::new_binder_service_specific_error;
use log::error;
+use std::fmt::Debug;
-pub fn to_binder_result<T>(result: Result<T>) -> BinderResult<T> {
+pub fn to_binder_result<T, E: Debug>(result: Result<T, E>) -> BinderResult<T> {
result.map_err(|e| {
let message = format!("{:?}", e);
error!("Returning binder error: {}", &message);