Make extra APKs easier to specify
Allow extra APKs to be specified in VirtualMachineAppConfig as an
alternative to the config file, so the paths don't need to be fixed.
This involves adding:
- the extra_apk_count to the metadata proto, which is persisted in the
instance image.
- the extra APK fds to VirtualMachinePayloadConfig, so it can be
included in VirtualMachineAppConfig iff there is no config file.
And then a whole lot of plumbing to make it all work.
Using this requires the USE_CUSTOM_VIRTUAL_MACHINE permission, and is
behind the multi-tenant feature flag.
I've not attempted to add API yet (this CL is already big), but I have
extended the vm command to allow it to be exercised.
Bug: 303201498
Test: Start a VM with an extra APK specified on the `vm run-app`
command line.
Change-Id: Ib5da40a33960fd9639b62e8d77e865aeb1f6398b
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java b/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
index a6381ac..e577f40 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
@@ -49,6 +49,7 @@
import java.io.OutputStream;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
+import java.util.Collections;
import java.util.Objects;
import java.util.zip.ZipFile;
@@ -519,6 +520,7 @@
VirtualMachinePayloadConfig payloadConfig = new VirtualMachinePayloadConfig();
payloadConfig.payloadBinaryName = mPayloadBinaryName;
payloadConfig.osName = mOs;
+ payloadConfig.extraApks = Collections.emptyList();
vsConfig.payload =
VirtualMachineAppConfig.Payload.payloadConfig(payloadConfig);
} else {
diff --git a/microdroid/payload/metadata.proto b/microdroid/payload/metadata.proto
index b03d466..e47fc83 100644
--- a/microdroid/payload/metadata.proto
+++ b/microdroid/payload/metadata.proto
@@ -74,4 +74,8 @@
// Required.
// Name of the payload binary file inside the APK.
string payload_binary_name = 1;
+
+ // Optional.
+ // The number of extra APKs that are present.
+ uint32 extra_apk_count = 2;
}
diff --git a/microdroid_manager/src/main.rs b/microdroid_manager/src/main.rs
index bf95cff..86284a5 100644
--- a/microdroid_manager/src/main.rs
+++ b/microdroid_manager/src/main.rs
@@ -43,7 +43,7 @@
use libc::VMADDR_CID_HOST;
use log::{error, info};
use microdroid_metadata::PayloadMetadata;
-use microdroid_payload_config::{OsConfig, Task, TaskType, VmPayloadConfig};
+use microdroid_payload_config::{ApkConfig, OsConfig, Task, TaskType, VmPayloadConfig};
use nix::sys::signal::Signal;
use openssl::hkdf::hkdf;
use openssl::md::Md;
@@ -580,11 +580,15 @@
type_: TaskType::MicrodroidLauncher,
command: payload_config.payload_binary_name,
};
+ // We don't care about the paths, only the number of extra APKs really matters.
+ let extra_apks = (0..payload_config.extra_apk_count)
+ .map(|i| ApkConfig { path: format!("extra-apk-{i}") })
+ .collect();
Ok(VmPayloadConfig {
os: OsConfig { name: "microdroid".to_owned() },
task: Some(task),
apexes: vec![],
- extra_apks: vec![],
+ extra_apks,
prefer_staged: false,
export_tombstones: None,
enable_authfs: false,
diff --git a/virtualizationmanager/src/aidl.rs b/virtualizationmanager/src/aidl.rs
index 1b1cabd..602c670 100644
--- a/virtualizationmanager/src/aidl.rs
+++ b/virtualizationmanager/src/aidl.rs
@@ -15,8 +15,7 @@
//! Implementation of the AIDL interface of the VirtualizationService.
use crate::{get_calling_pid, get_calling_uid};
-use crate::atom::{
- write_vm_booted_stats, write_vm_creation_stats};
+use crate::atom::{write_vm_booted_stats, write_vm_creation_stats};
use crate::composite::make_composite_image;
use crate::crosvm::{CrosvmConfig, DiskFile, PayloadState, VmContext, VmInstance, VmState};
use crate::debug_config::DebugConfig;
@@ -73,7 +72,7 @@
use glob::glob;
use lazy_static::lazy_static;
use log::{debug, error, info, warn};
-use microdroid_payload_config::{OsConfig, Task, TaskType, VmPayloadConfig};
+use microdroid_payload_config::{ApkConfig, OsConfig, Task, TaskType, VmPayloadConfig};
use nix::unistd::pipe;
use rpcbinder::RpcServer;
use rustutils::system_properties;
@@ -366,21 +365,7 @@
// Allocating VM context checks the MANAGE_VIRTUAL_MACHINE permission.
let (vm_context, cid, temporary_directory) = self.create_vm_context(requester_debug_pid)?;
- let is_custom = match config {
- VirtualMachineConfig::RawConfig(_) => true,
- VirtualMachineConfig::AppConfig(config) => {
- // Some features are reserved for platform apps only, even when using
- // VirtualMachineAppConfig. Almost all of these features are grouped in the
- // CustomConfig struct:
- // - controlling CPUs;
- // - specifying a config file in the APK; (this one is not part of CustomConfig)
- // - gdbPort is set, meaning that crosvm will start a gdb server;
- // - using anything other than the default kernel;
- // - specifying devices to be assigned.
- config.customConfig.is_some() || matches!(config.payload, Payload::ConfigPath(_))
- }
- };
- if is_custom {
+ if is_custom_config(config) {
check_use_custom_virtual_machine()?;
}
@@ -565,6 +550,35 @@
}
}
+/// Returns whether a VM config represents a "custom" virtual machine, which requires the
+/// USE_CUSTOM_VIRTUAL_MACHINE.
+fn is_custom_config(config: &VirtualMachineConfig) -> bool {
+ match config {
+ // Any raw (non-Microdroid) VM is considered custom.
+ VirtualMachineConfig::RawConfig(_) => true,
+ VirtualMachineConfig::AppConfig(config) => {
+ // Some features are reserved for platform apps only, even when using
+ // VirtualMachineAppConfig. Almost all of these features are grouped in the
+ // CustomConfig struct:
+ // - controlling CPUs;
+ // - gdbPort is set, meaning that crosvm will start a gdb server;
+ // - using anything other than the default kernel;
+ // - specifying devices to be assigned.
+ if config.customConfig.is_some() {
+ true
+ } else {
+ // Additional custom features not included in CustomConfig:
+ // - specifying a config file;
+ // - specifying extra APKs.
+ match &config.payload {
+ Payload::ConfigPath(_) => true,
+ Payload::PayloadConfig(payload_config) => !payload_config.extraApks.is_empty(),
+ }
+ }
+ }
+ }
+}
+
fn write_zero_filler(zero_filler_path: &Path) -> Result<()> {
let file = OpenOptions::new()
.create_new(true)
@@ -694,12 +708,28 @@
None
};
- let vm_payload_config = match &config.payload {
+ let vm_payload_config;
+ let extra_apk_files: Vec<_>;
+ match &config.payload {
Payload::ConfigPath(config_path) => {
- load_vm_payload_config_from_file(&apk_file, config_path.as_str())
- .with_context(|| format!("Couldn't read config from {}", config_path))?
+ vm_payload_config =
+ load_vm_payload_config_from_file(&apk_file, config_path.as_str())
+ .with_context(|| format!("Couldn't read config from {}", config_path))?;
+ extra_apk_files = vm_payload_config
+ .extra_apks
+ .iter()
+ .enumerate()
+ .map(|(i, apk)| {
+ File::open(PathBuf::from(&apk.path))
+ .with_context(|| format!("Failed to open extra apk #{i} {}", apk.path))
+ })
+ .collect::<Result<_>>()?;
}
- Payload::PayloadConfig(payload_config) => create_vm_payload_config(payload_config)?,
+ Payload::PayloadConfig(payload_config) => {
+ vm_payload_config = create_vm_payload_config(payload_config)?;
+ extra_apk_files =
+ payload_config.extraApks.iter().map(clone_file).collect::<binder::Result<_>>()?;
+ }
};
// For now, the only supported OS is Microdroid and Microdroid GKI
@@ -747,6 +777,7 @@
temporary_directory,
apk_file,
idsig_file,
+ extra_apk_files,
&vm_payload_config,
&mut vm_config,
)?;
@@ -774,11 +805,17 @@
let task = Task { type_: TaskType::MicrodroidLauncher, command: payload_binary_name.clone() };
let name = payload_config.osName.clone();
+
+ // The VM only cares about how many there are, these names are actually ignored.
+ let extra_apk_count = payload_config.extraApks.len();
+ let extra_apks =
+ (0..extra_apk_count).map(|i| ApkConfig { path: format!("extra-apk-{i}") }).collect();
+
Ok(VmPayloadConfig {
os: OsConfig { name },
task: Some(task),
apexes: vec![],
- extra_apks: vec![],
+ extra_apks,
prefer_staged: false,
export_tombstones: None,
enable_authfs: false,
@@ -1193,6 +1230,16 @@
Ok(())
}
+fn check_no_extra_apks(config: &VirtualMachineConfig) -> binder::Result<()> {
+ let VirtualMachineConfig::AppConfig(config) = config else { return Ok(()) };
+ let Payload::PayloadConfig(payload_config) = &config.payload else { return Ok(()) };
+ if !payload_config.extraApks.is_empty() {
+ return Err(anyhow!("multi-tenant feature is disabled"))
+ .or_binder_exception(ExceptionCode::UNSUPPORTED_OPERATION);
+ }
+ Ok(())
+}
+
fn check_config_features(config: &VirtualMachineConfig) -> binder::Result<()> {
if !cfg!(vendor_modules) {
check_no_vendor_modules(config)?;
@@ -1200,6 +1247,9 @@
if !cfg!(device_assignment) {
check_no_devices(config)?;
}
+ if !cfg!(multi_tenant) {
+ check_no_extra_apks(config)?;
+ }
Ok(())
}
diff --git a/virtualizationmanager/src/payload.rs b/virtualizationmanager/src/payload.rs
index c19c103..05626d3 100644
--- a/virtualizationmanager/src/payload.rs
+++ b/virtualizationmanager/src/payload.rs
@@ -194,7 +194,8 @@
let payload_metadata = match &app_config.payload {
Payload::PayloadConfig(payload_config) => PayloadMetadata::Config(PayloadConfig {
payload_binary_name: payload_config.payloadBinaryName.clone(),
- ..Default::default()
+ extra_apk_count: payload_config.extraApks.len().try_into()?,
+ special_fields: Default::default(),
}),
Payload::ConfigPath(config_path) => {
PayloadMetadata::ConfigPath(format!("/mnt/apk/{}", config_path))
@@ -258,10 +259,11 @@
debug_config: &DebugConfig,
apk_file: File,
idsig_file: File,
+ extra_apk_files: Vec<File>,
vm_payload_config: &VmPayloadConfig,
temporary_directory: &Path,
) -> Result<DiskImage> {
- if vm_payload_config.extra_apks.len() != app_config.extraIdsigs.len() {
+ if extra_apk_files.len() != app_config.extraIdsigs.len() {
bail!(
"payload config has {} apks, but app config has {} idsigs",
vm_payload_config.extra_apks.len(),
@@ -309,26 +311,23 @@
});
// we've already checked that extra_apks and extraIdsigs are in the same size.
- let extra_apks = &vm_payload_config.extra_apks;
let extra_idsigs = &app_config.extraIdsigs;
- for (i, (extra_apk, extra_idsig)) in extra_apks.iter().zip(extra_idsigs.iter()).enumerate() {
+ for (i, (extra_apk_file, extra_idsig)) in
+ extra_apk_files.into_iter().zip(extra_idsigs.iter()).enumerate()
+ {
partitions.push(Partition {
- label: format!("extra-apk-{}", i),
- image: Some(ParcelFileDescriptor::new(
- File::open(PathBuf::from(&extra_apk.path)).with_context(|| {
- format!("Failed to open the extra apk #{} {}", i, extra_apk.path)
- })?,
- )),
+ label: format!("extra-apk-{i}"),
+ image: Some(ParcelFileDescriptor::new(extra_apk_file)),
writable: false,
});
partitions.push(Partition {
- label: format!("extra-idsig-{}", i),
+ label: format!("extra-idsig-{i}"),
image: Some(ParcelFileDescriptor::new(
extra_idsig
.as_ref()
.try_clone()
- .with_context(|| format!("Failed to clone the extra idsig #{}", i))?,
+ .with_context(|| format!("Failed to clone the extra idsig #{i}"))?,
)),
writable: false,
});
@@ -459,12 +458,14 @@
Ok(())
}
+#[allow(clippy::too_many_arguments)] // TODO: Fewer arguments
pub fn add_microdroid_payload_images(
config: &VirtualMachineAppConfig,
debug_config: &DebugConfig,
temporary_directory: &Path,
apk_file: File,
idsig_file: File,
+ extra_apk_files: Vec<File>,
vm_payload_config: &VmPayloadConfig,
vm_config: &mut VirtualMachineRawConfig,
) -> Result<()> {
@@ -473,6 +474,7 @@
debug_config,
apk_file,
idsig_file,
+ extra_apk_files,
vm_payload_config,
temporary_directory,
)?);
diff --git a/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachinePayloadConfig.aidl b/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachinePayloadConfig.aidl
index f3f54f3..7ca5b62 100644
--- a/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachinePayloadConfig.aidl
+++ b/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachinePayloadConfig.aidl
@@ -28,4 +28,7 @@
* Name of the OS to run the payload. Currently "microdroid" and "microdroid_gki" is supported.
*/
@utf8InCpp String osName = "microdroid";
+
+ /** Any extra APKs. */
+ List<ParcelFileDescriptor> extraApks;
}
diff --git a/vm/src/main.rs b/vm/src/main.rs
index 5c07eed..de9291c 100644
--- a/vm/src/main.rs
+++ b/vm/src/main.rs
@@ -34,7 +34,7 @@
#[derive(Debug)]
struct Idsigs(Vec<PathBuf>);
-#[derive(Args)]
+#[derive(Args, Default)]
/// Collection of flags that are at VM level and therefore applicable to all subcommands
pub struct CommonConfig {
/// Name of VM
@@ -59,7 +59,7 @@
protected: bool,
}
-#[derive(Args)]
+#[derive(Args, Default)]
/// Collection of flags for debugging
pub struct DebugConfig {
/// Debug level of the VM. Supported values: "full" (default), and "none".
@@ -84,7 +84,7 @@
gdb: Option<NonZeroU16>,
}
-#[derive(Args)]
+#[derive(Args, Default)]
/// Collection of flags that are Microdroid specific
pub struct MicrodroidConfig {
/// Path to the file backing the storage.
@@ -145,7 +145,7 @@
}
}
-#[derive(Args)]
+#[derive(Args, Default)]
/// Flags for the run_app subcommand
pub struct RunAppConfig {
#[command(flatten)]
@@ -175,12 +175,30 @@
#[arg(alias = "payload_path")]
payload_binary_name: Option<String>,
+ /// Paths to extra apk files.
+ #[cfg(multi_tenant)]
+ #[arg(long = "extra-apk")]
+ #[clap(conflicts_with = "config_path")]
+ extra_apks: Vec<PathBuf>,
+
/// Paths to extra idsig files.
#[arg(long = "extra-idsig")]
extra_idsigs: Vec<PathBuf>,
}
-#[derive(Args)]
+impl RunAppConfig {
+ #[cfg(multi_tenant)]
+ fn extra_apks(&self) -> &[PathBuf] {
+ &self.extra_apks
+ }
+
+ #[cfg(not(multi_tenant))]
+ fn extra_apks(&self) -> &[PathBuf] {
+ &[]
+ }
+}
+
+#[derive(Args, Default)]
/// Flags for the run_microdroid subcommand
pub struct RunMicrodroidConfig {
#[command(flatten)]
@@ -199,7 +217,7 @@
work_dir: Option<PathBuf>,
}
-#[derive(Args)]
+#[derive(Args, Default)]
/// Flags for the run subcommand
pub struct RunCustomVmConfig {
#[command(flatten)]
diff --git a/vm/src/run.rs b/vm/src/run.rs
index cfc5454..1d2f48b 100644
--- a/vm/src/run.rs
+++ b/vm/src/run.rs
@@ -48,7 +48,7 @@
let extra_apks = match config.config_path.as_deref() {
Some(path) => parse_extra_apk_list(&config.apk, path)?,
- None => vec![],
+ None => config.extra_apks().to_vec(),
};
if extra_apks.len() != config.extra_idsigs.len() {
@@ -101,8 +101,7 @@
let vendor =
config.microdroid.vendor().as_ref().map(|p| open_parcel_file(p, false)).transpose()?;
- let extra_idsig_files: Result<Vec<File>, _> =
- config.extra_idsigs.iter().map(File::open).collect();
+ let extra_idsig_files: Result<Vec<_>, _> = config.extra_idsigs.iter().map(File::open).collect();
let extra_idsig_fds = extra_idsig_files?.into_iter().map(ParcelFileDescriptor::new).collect();
let payload = if let Some(config_path) = config.config_path {
@@ -119,9 +118,14 @@
} else {
"microdroid".to_owned()
};
+
+ let extra_apk_files: Result<Vec<_>, _> = extra_apks.iter().map(File::open).collect();
+ let extra_apk_fds = extra_apk_files?.into_iter().map(ParcelFileDescriptor::new).collect();
+
Payload::PayloadConfig(VirtualMachinePayloadConfig {
payloadBinaryName: payload_binary_name,
osName: os_name,
+ extraApks: extra_apk_fds,
})
} else {
bail!("Either --config-path or --payload-binary-name must be defined")
@@ -208,9 +212,8 @@
apk,
idsig,
instance: instance_img,
- config_path: None,
payload_binary_name: Some("MicrodroidEmptyPayloadJniLib.so".to_owned()),
- extra_idsigs: [].to_vec(),
+ ..Default::default()
};
command_run_app(app_config)
}
@@ -310,11 +313,11 @@
Ok(())
}
-fn parse_extra_apk_list(apk: &Path, config_path: &str) -> Result<Vec<String>, Error> {
+fn parse_extra_apk_list(apk: &Path, config_path: &str) -> Result<Vec<PathBuf>, Error> {
let mut archive = ZipArchive::new(File::open(apk)?)?;
let config_file = archive.by_name(config_path)?;
let config: VmPayloadConfig = serde_json::from_reader(config_file)?;
- Ok(config.extra_apks.into_iter().map(|x| x.path).collect())
+ Ok(config.extra_apks.into_iter().map(|x| x.path.into()).collect())
}
struct Callback {}