Introduce VirtualMachineAppConfig::CustomConfig struct
This struct contains all the parameters that require
USE_CUSTOM_VIRTUAL_MACHINE permission.
Bug: 286225150
Test: atest MicrodroidTestApp
Change-Id: I3d78459c47dc6848642692106847544268e9e4e0
diff --git a/compos/common/compos_client.rs b/compos/common/compos_client.rs
index b03addf..77a1204 100644
--- a/compos/common/compos_client.rs
+++ b/compos/common/compos_client.rs
@@ -24,7 +24,10 @@
use android_system_virtualizationservice::aidl::android::system::virtualizationservice::{
CpuTopology::CpuTopology,
IVirtualizationService::IVirtualizationService,
- VirtualMachineAppConfig::{DebugLevel::DebugLevel, Payload::Payload, VirtualMachineAppConfig},
+ VirtualMachineAppConfig::{
+ CustomConfig::CustomConfig, DebugLevel::DebugLevel, Payload::Payload,
+ VirtualMachineAppConfig,
+ },
VirtualMachineConfig::VirtualMachineConfig,
};
use anyhow::{anyhow, bail, Context, Result};
@@ -128,9 +131,10 @@
protectedVm: protected_vm,
memoryMib: parameters.memory_mib.unwrap_or(0), // 0 means use the default
cpuTopology: cpu_topology,
- taskProfiles: parameters.task_profiles.clone(),
- gdbPort: 0, // Don't start gdb-server
- customKernelImage: None,
+ customConfig: Some(CustomConfig {
+ taskProfiles: parameters.task_profiles.clone(),
+ ..Default::default()
+ }),
});
// Let logs go to logcat.
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java b/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
index 93e65db..5f24f5b 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
@@ -61,7 +61,6 @@
@SystemApi
public final class VirtualMachineConfig {
private static final String TAG = "VirtualMachineConfig";
- private static final String[] EMPTY_STRING_ARRAY = {};
// These define the schema of the config file persisted on disk.
private static final int VERSION = 6;
@@ -482,8 +481,6 @@
vsConfig.cpuTopology = android.system.virtualizationservice.CpuTopology.ONE_CPU;
break;
}
- // Don't allow apps to set task profiles ... at least for now.
- vsConfig.taskProfiles = EMPTY_STRING_ARRAY;
return vsConfig;
}
diff --git a/virtualizationmanager/src/aidl.rs b/virtualizationmanager/src/aidl.rs
index 9cd70e6..968d2d2 100644
--- a/virtualizationmanager/src/aidl.rs
+++ b/virtualizationmanager/src/aidl.rs
@@ -317,15 +317,13 @@
VirtualMachineConfig::RawConfig(_) => true,
VirtualMachineConfig::AppConfig(config) => {
// Some features are reserved for platform apps only, even when using
- // VirtualMachineAppConfig:
+ // VirtualMachineAppConfig. Almost all of these features are grouped in the
+ // CustomConfig struct:
// - controlling CPUs;
- // - specifying a config file in the APK;
+ // - 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.
- !config.taskProfiles.is_empty()
- || matches!(config.payload, Payload::ConfigPath(_))
- || config.gdbPort > 0
- || config.customKernelImage.as_ref().is_some()
+ config.customConfig.is_some() || matches!(config.payload, Payload::ConfigPath(_))
}
};
if is_custom {
@@ -606,8 +604,12 @@
let vm_config_file = File::open(vm_config_path)?;
let mut vm_config = VmConfig::load(&vm_config_file)?.to_parcelable()?;
- if let Some(file) = config.customKernelImage.as_ref() {
- vm_config.kernel = Some(ParcelFileDescriptor::new(clone_file(file)?))
+ if let Some(custom_config) = &config.customConfig {
+ if let Some(file) = custom_config.customKernelImage.as_ref() {
+ vm_config.kernel = Some(ParcelFileDescriptor::new(clone_file(file)?))
+ }
+ vm_config.taskProfiles = custom_config.taskProfiles.clone();
+ vm_config.gdbPort = custom_config.gdbPort;
}
if config.memoryMib > 0 {
@@ -617,8 +619,6 @@
vm_config.name = config.name.clone();
vm_config.protectedVm = config.protectedVm;
vm_config.cpuTopology = config.cpuTopology;
- vm_config.taskProfiles = config.taskProfiles.clone();
- vm_config.gdbPort = config.gdbPort;
// Microdroid takes additional init ramdisk & (optionally) storage image
add_microdroid_system_images(config, instance_file, storage_image, &mut vm_config)?;
@@ -1053,7 +1053,9 @@
fn extract_gdb_port(config: &VirtualMachineConfig) -> Option<NonZeroU16> {
match config {
VirtualMachineConfig::RawConfig(config) => NonZeroU16::new(config.gdbPort as u16),
- VirtualMachineConfig::AppConfig(config) => NonZeroU16::new(config.gdbPort as u16),
+ VirtualMachineConfig::AppConfig(config) => {
+ NonZeroU16::new(config.customConfig.as_ref().map(|c| c.gdbPort).unwrap_or(0) as u16)
+ }
}
}
diff --git a/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineAppConfig.aidl b/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineAppConfig.aidl
index 5e05bb9..6a0bf7c 100644
--- a/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineAppConfig.aidl
+++ b/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineAppConfig.aidl
@@ -45,6 +45,8 @@
union Payload {
/**
* Path to a JSON file in an APK containing the configuration.
+ *
+ * <p>Setting this field requires android.permission.USE_CUSTOM_VIRTUAL_MACHINE
*/
@utf8InCpp String configPath;
@@ -70,16 +72,6 @@
/** Debug level of the VM */
DebugLevel debugLevel = DebugLevel.NONE;
- /**
- * Port at which crosvm will start a gdb server to debug guest kernel.
- * If set to zero, then gdb server won't be started.
- *
- * Note: Specifying a value here requires android.permission.USE_CUSTOM_VIRTUAL_MACHINE.
- *
- * TODO(b/286225150): move to a separate struct
- */
- int gdbPort = 0;
-
/** Whether the VM should be a protected VM. */
boolean protectedVm;
@@ -93,20 +85,28 @@
CpuTopology cpuTopology = CpuTopology.ONE_CPU;
/**
- * List of task profile names to apply for the VM
- *
- * Note: Specifying a value here requires android.permission.USE_CUSTOM_VIRTUAL_MACHINE.
- *
- * TODO(b/286225150): move to a separate struct
+ * Encapsulates parameters that require android.permission.USE_CUSTOM_VIRTUAL_MACHINE.
*/
- String[] taskProfiles;
+ parcelable CustomConfig {
+ /**
+ * If specified, boot Microdroid VM with the given kernel.
+ *
+ */
+ @nullable ParcelFileDescriptor customKernelImage;
- /**
- * If specified, boot Microdroid VM with the given kernel.
- *
- * Note: Specifying a value here requires android.permission.USE_CUSTOM_VIRTUAL_MACHINE.
- *
- * TODO(b/286225150): move to a separate struct
- */
- @nullable ParcelFileDescriptor customKernelImage;
+ /**
+ * Port at which crosvm will start a gdb server to debug guest kernel.
+ * If set to zero, then gdb server won't be started.
+ *
+ */
+ int gdbPort = 0;
+
+ /**
+ * List of task profile names to apply for the VM
+ */
+ String[] taskProfiles;
+ }
+
+ /** Configuration parameters guarded by android.permission.USE_CUSTOM_VIRTUAL_MACHINE */
+ @nullable CustomConfig customConfig;
}
diff --git a/vm/src/run.rs b/vm/src/run.rs
index 54c1de4..663fa25 100644
--- a/vm/src/run.rs
+++ b/vm/src/run.rs
@@ -19,7 +19,10 @@
CpuTopology::CpuTopology,
IVirtualizationService::IVirtualizationService,
PartitionType::PartitionType,
- VirtualMachineAppConfig::{DebugLevel::DebugLevel, Payload::Payload, VirtualMachineAppConfig},
+ VirtualMachineAppConfig::{
+ CustomConfig::CustomConfig, DebugLevel::DebugLevel, Payload::Payload,
+ VirtualMachineAppConfig,
+ },
VirtualMachineConfig::VirtualMachineConfig,
VirtualMachinePayloadConfig::VirtualMachinePayloadConfig,
VirtualMachineState::VirtualMachineState,
@@ -136,6 +139,12 @@
let payload_config_str = format!("{:?}!{:?}", apk, payload);
+ let custom_config = CustomConfig {
+ customKernelImage: kernel,
+ gdbPort: gdb.map(u16::from).unwrap_or(0) as i32, // 0 means no gdb
+ taskProfiles: task_profiles,
+ };
+
let config = VirtualMachineConfig::AppConfig(VirtualMachineAppConfig {
name: name.unwrap_or_else(|| String::from("VmRunApp")),
apk: apk_fd.into(),
@@ -148,9 +157,7 @@
protectedVm: protected,
memoryMib: mem.unwrap_or(0) as i32, // 0 means use the VM default
cpuTopology: cpu_topology,
- taskProfiles: task_profiles,
- gdbPort: gdb.map(u16::from).unwrap_or(0) as i32, // 0 means no gdb
- customKernelImage: kernel,
+ customConfig: Some(custom_config),
});
run(service, &config, &payload_config_str, console_path, log_path)
}