From 9900f3d73372b0f10439a70d12d1a5446ac6c348 Mon Sep 17 00:00:00 2001 From: Jooyung Han Date: Tue, 6 Jul 2021 10:27:54 +0900 Subject: [PATCH] virtualizationservice: lazy-load ApexInfoList Loading ApexInfoList can't be used to initialize VirtualizationService because when it fails VirtualizationService enters into infinite loop due to service restart. Now it is loaded lazily. Bug: 192892281 Test: MicrodroidHostTestCases Change-Id: I3c082d200549c348ff8fa03ff25ca73f9fb694cd --- virtualizationservice/Android.bp | 1 + virtualizationservice/src/aidl.rs | 32 ++++---------- virtualizationservice/src/main.rs | 2 +- virtualizationservice/src/payload.rs | 63 ++++++++++++++++------------ 4 files changed, 47 insertions(+), 51 deletions(-) diff --git a/virtualizationservice/Android.bp b/virtualizationservice/Android.bp index c209f106..0b8f2e51 100644 --- a/virtualizationservice/Android.bp +++ b/virtualizationservice/Android.bp @@ -29,6 +29,7 @@ rust_defaults { "liblog_rust", "libmicrodroid_metadata", "libmicrodroid_payload_config", + "libonce_cell", "libprotobuf", "libprotos", "libregex", diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs index dc22e99b..bc1761b2 100644 --- a/virtualizationservice/src/aidl.rs +++ b/virtualizationservice/src/aidl.rs @@ -16,7 +16,7 @@ use crate::composite::make_composite_image; use crate::crosvm::{CrosvmConfig, DiskFile, VmInstance}; -use crate::payload::{make_payload_disk, ApexInfoList}; +use crate::payload::make_payload_disk; use crate::{Cid, FIRST_GUEST_CID}; use android_system_virtualizationservice::aidl::android::system::virtualizationservice::IVirtualizationService::IVirtualizationService; @@ -62,19 +62,9 @@ const MICRODROID_REQUIRED_APEXES: [&str; 4] = ["com.android.adbd", "com.android.i18n", "com.android.os.statsd", "com.android.sdkext"]; /// Implementation of `IVirtualizationService`, the entry point of the AIDL service. -#[derive(Debug)] +#[derive(Debug, Default)] pub struct VirtualizationService { state: Mutex, - apex_info_list: ApexInfoList, -} - -impl VirtualizationService { - pub fn new() -> Result { - Ok(VirtualizationService { - state: Default::default(), - apex_info_list: ApexInfoList::load()?, - }) - } } impl Interface for VirtualizationService {} @@ -119,15 +109,13 @@ impl IVirtualizationService for VirtualizationService { let config = match config { VirtualMachineConfig::AppConfig(config) => BorrowedOrOwned::Owned( - load_app_config(&self.apex_info_list, config, &temporary_directory).map_err( - |e| { - error!("Failed to load app config from {}: {}", &config.configPath, e); - new_binder_exception( - ExceptionCode::SERVICE_SPECIFIC, - format!("Failed to load app config from {}: {}", &config.configPath, e), - ) - }, - )?, + load_app_config(config, &temporary_directory).map_err(|e| { + error!("Failed to load app config from {}: {}", &config.configPath, e); + new_binder_exception( + ExceptionCode::SERVICE_SPECIFIC, + format!("Failed to load app config from {}: {}", &config.configPath, e), + ) + })?, ), VirtualMachineConfig::RawConfig(config) => BorrowedOrOwned::Borrowed(config), }; @@ -298,7 +286,6 @@ fn assemble_disk_image( } fn load_app_config( - apex_info_list: &ApexInfoList, config: &VirtualMachineAppConfig, temporary_directory: &Path, ) -> Result { @@ -328,7 +315,6 @@ fn load_app_config( apexes.dedup_by(|a, b| a.name == b.name); vm_config.disks.push(make_payload_disk( - apex_info_list, format!("/proc/self/fd/{}", apk_file.as_raw_fd()).into(), format!("/proc/self/fd/{}", idsig_file.as_raw_fd()).into(), config_path, diff --git a/virtualizationservice/src/main.rs b/virtualizationservice/src/main.rs index 3f2e99c6..658203b5 100644 --- a/virtualizationservice/src/main.rs +++ b/virtualizationservice/src/main.rs @@ -39,7 +39,7 @@ fn main() { android_logger::Config::default().with_tag(LOG_TAG).with_min_level(Level::Trace), ); - let service = VirtualizationService::new().unwrap(); + let service = VirtualizationService::default(); let service = BnVirtualizationService::new_binder( service, BinderFeatures { set_requesting_sid: true, ..BinderFeatures::default() }, diff --git a/virtualizationservice/src/payload.rs b/virtualizationservice/src/payload.rs index d3dc2897..18e7de51 100644 --- a/virtualizationservice/src/payload.rs +++ b/virtualizationservice/src/payload.rs @@ -16,9 +16,10 @@ use crate::composite::align_to_partition_size; -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, bail, Result}; use microdroid_metadata::{ApexPayload, ApkPayload, Metadata}; use microdroid_payload_config::ApexConfig; +use once_cell::sync::OnceCell; use regex::Regex; use std::fs; use std::fs::OpenOptions; @@ -29,7 +30,7 @@ use vmconfig::{DiskImage, Partition}; /// Represents the list of APEXes #[derive(Debug)] -pub struct ApexInfoList { +struct ApexInfoList { list: Vec, } @@ -41,30 +42,38 @@ struct ApexInfo { impl ApexInfoList { /// Loads ApexInfoList - pub fn load() -> Result { - // TODO(b/191601801): look up /apex/apex-info-list.xml instead of apexservice - // Each APEX prints the line: - // Module: <...> Version: <...> VersionName: <...> Path: <...> IsActive: <...> IsFactory: <...> - // We only care about "Module:" and "Path:" tagged values for now. - let info_pattern = Regex::new(r"^Module: (?P[^ ]*) .* Path: (?P[^ ]*) .*$")?; - let output = Command::new("cmd") - .arg("-w") - .arg("apexservice") - .arg("getActivePackages") - .output() - .expect("failed to execute apexservice cmd"); - let list = BufReader::new(output.stdout.as_slice()) - .lines() - .map(|line| -> Result { - let line = line?; - let captures = - info_pattern.captures(&line).ok_or_else(|| anyhow!("can't parse: {}", line))?; - let name = captures.name("name").unwrap(); - let path = captures.name("path").unwrap(); - Ok(ApexInfo { name: name.as_str().to_owned(), path: path.as_str().into() }) - }) - .collect::>>()?; - Ok(ApexInfoList { list }) + fn load() -> Result<&'static ApexInfoList> { + static INSTANCE: OnceCell = OnceCell::new(); + INSTANCE.get_or_try_init(|| { + // TODO(b/191601801): look up /apex/apex-info-list.xml instead of apexservice + // Each APEX prints the line: + // Module: <...> Version: <...> VersionName: <...> Path: <...> IsActive: <...> IsFactory: <...> + // We only care about "Module:" and "Path:" tagged values for now. + let info_pattern = + Regex::new(r"^Module: (?P[^ ]*) .* Path: (?P[^ ]*) .*$")?; + let output = Command::new("cmd") + .arg("-w") + .arg("apexservice") + .arg("getActivePackages") + .output() + .expect("failed to execute apexservice cmd"); + let list = BufReader::new(output.stdout.as_slice()) + .lines() + .map(|line| -> Result { + let line = line?; + let captures = info_pattern + .captures(&line) + .ok_or_else(|| anyhow!("can't parse: {}", line))?; + let name = captures.name("name").unwrap(); + let path = captures.name("path").unwrap(); + Ok(ApexInfo { name: name.as_str().to_owned(), path: path.as_str().into() }) + }) + .collect::>>()?; + if list.is_empty() { + bail!("failed to load apex info: empty"); + } + Ok(ApexInfoList { list }) + }) } fn get_path_for(&self, apex_name: &str) -> Result { @@ -120,7 +129,6 @@ fn make_no_filler(_size: u64, _filler_path: &Path) -> Result { /// microdroid-apk: [apk, zero filler] /// microdroid-apk-idsig: idsig pub fn make_payload_disk( - apex_info_list: &ApexInfoList, apk_file: PathBuf, idsig_file: PathBuf, config_path: &str, @@ -155,6 +163,7 @@ pub fn make_payload_disk( writable: false, }]; + let apex_info_list = ApexInfoList::load()?; let mut filler_count = 0; for (i, apex) in apexes.iter().enumerate() { partitions.push(make_partition(