Ignore debug policy with sysprop
Read system property hypervisor.virtualizationmanager.debug_policy.path
to decide whether to ignore debug policy or not.
Follow-up CLs will use the system property to disable debug policy for
tests and also allows to set custom debug policy for debug policy test.
Bug: 272496125, Bug: 275047565
Test: Run following tests twice with/without the sysprop \
1) `atest MicrodroidTestApp MicrodroidHostTestCases` for \
checking no regression. \
2) Launched VM on a debug policy enabled device without debug level
Change-Id: I9f3a829c84a55441942953979e3aec893c0405fd
diff --git a/virtualizationmanager/src/aidl.rs b/virtualizationmanager/src/aidl.rs
index 749d75f..468ee19 100644
--- a/virtualizationmanager/src/aidl.rs
+++ b/virtualizationmanager/src/aidl.rs
@@ -19,8 +19,7 @@
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::should_prepare_console_output;
-use crate::debug_config::is_ramdump_needed;
+use crate::debug_config::DebugConfig;
use crate::payload::{add_microdroid_payload_images, add_microdroid_system_images};
use crate::selinux::{getfilecon, SeContext};
use android_os_permissions_aidl::aidl::android::os::IPermissionController;
@@ -327,21 +326,22 @@
check_gdb_allowed(config)?;
}
- let ramdump = if is_ramdump_needed(config) {
+ let debug_level = match config {
+ VirtualMachineConfig::AppConfig(config) => config.debugLevel,
+ _ => DebugLevel::NONE,
+ };
+ let debug_config = DebugConfig::new(debug_level);
+
+ let ramdump = if debug_config.is_ramdump_needed() {
Some(prepare_ramdump_file(&temporary_directory)?)
} else {
None
};
- let debug_level = match config {
- VirtualMachineConfig::AppConfig(app_config) => app_config.debugLevel,
- _ => DebugLevel::NONE,
- };
-
let state = &mut *self.state.lock().unwrap();
let console_fd =
- clone_or_prepare_logger_fd(config, console_fd, format!("Console({})", cid))?;
- let log_fd = clone_or_prepare_logger_fd(config, log_fd, format!("Log({})", cid))?;
+ clone_or_prepare_logger_fd(&debug_config, console_fd, format!("Console({})", cid))?;
+ let log_fd = clone_or_prepare_logger_fd(&debug_config, log_fd, format!("Log({})", cid))?;
// Counter to generate unique IDs for temporary image files.
let mut next_temporary_image_id = 0;
@@ -352,12 +352,13 @@
let (is_app_config, config) = match config {
VirtualMachineConfig::RawConfig(config) => (false, BorrowedOrOwned::Borrowed(config)),
VirtualMachineConfig::AppConfig(config) => {
- let config = load_app_config(config, &temporary_directory).map_err(|e| {
- *is_protected = config.protectedVm;
- let message = format!("Failed to load app config: {:?}", e);
- error!("{}", message);
- Status::new_service_specific_error_str(-1, Some(message))
- })?;
+ let config =
+ load_app_config(config, &debug_config, &temporary_directory).map_err(|e| {
+ *is_protected = config.protectedVm;
+ let message = format!("Failed to load app config: {:?}", e);
+ error!("{}", message);
+ Status::new_service_specific_error_str(-1, Some(message))
+ })?;
(true, BorrowedOrOwned::Owned(config))
}
};
@@ -438,7 +439,7 @@
disks,
params: config.params.to_owned(),
protected: *is_protected,
- debug_level,
+ debug_config,
memory_mib: config.memoryMib.try_into().ok().and_then(NonZeroU32::new),
cpus,
host_cpu_topology,
@@ -559,6 +560,7 @@
fn load_app_config(
config: &VirtualMachineAppConfig,
+ debug_config: &DebugConfig,
temporary_directory: &Path,
) -> Result<VirtualMachineRawConfig> {
let apk_file = clone_file(config.apk.as_ref().unwrap())?;
@@ -607,6 +609,7 @@
// Include Microdroid payload disk (contains apks, idsigs) in vm config
add_microdroid_payload_images(
config,
+ debug_config,
temporary_directory,
apk_file,
idsig_file,
@@ -1038,7 +1041,7 @@
}
fn clone_or_prepare_logger_fd(
- config: &VirtualMachineConfig,
+ debug_config: &DebugConfig,
fd: Option<&ParcelFileDescriptor>,
tag: String,
) -> Result<Option<File>, Status> {
@@ -1046,10 +1049,7 @@
return Ok(Some(clone_file(fd)?));
}
- let VirtualMachineConfig::AppConfig(app_config) = config else {
- return Ok(None);
- };
- if !should_prepare_console_output(app_config.debugLevel) {
+ if !debug_config.should_prepare_console_output() {
return Ok(None);
};
diff --git a/virtualizationmanager/src/crosvm.rs b/virtualizationmanager/src/crosvm.rs
index 8497564..a8cad94 100644
--- a/virtualizationmanager/src/crosvm.rs
+++ b/virtualizationmanager/src/crosvm.rs
@@ -16,7 +16,7 @@
use crate::aidl::{remove_temporary_files, Cid, VirtualMachineCallbacks};
use crate::atom::{get_num_cpus, write_vm_exited_stats};
-use crate::debug_config::should_prepare_console_output;
+use crate::debug_config::DebugConfig;
use anyhow::{anyhow, bail, Context, Error, Result};
use command_fds::CommandFdExt;
use lazy_static::lazy_static;
@@ -102,7 +102,7 @@
pub disks: Vec<DiskFile>,
pub params: Option<String>,
pub protected: bool,
- pub debug_level: DebugLevel,
+ pub debug_config: DebugConfig,
pub memory_mib: Option<NonZeroU32>,
pub cpus: Option<NonZeroU32>,
pub host_cpu_topology: bool,
@@ -735,7 +735,9 @@
} else if config.ramdump.is_some() {
command.arg("--params").arg(format!("crashkernel={RAMDUMP_RESERVED_MIB}M"));
}
- if config.debug_level == DebugLevel::NONE && should_prepare_console_output(config.debug_level) {
+ if config.debug_config.debug_level == DebugLevel::NONE
+ && config.debug_config.should_prepare_console_output()
+ {
// bootconfig.normal will be used, but we need log.
command.arg("--params").arg("printk.devkmsg=on");
command.arg("--params").arg("console=hvc0");
diff --git a/virtualizationmanager/src/debug_config.rs b/virtualizationmanager/src/debug_config.rs
index ec3d591..e8863c7 100644
--- a/virtualizationmanager/src/debug_config.rs
+++ b/virtualizationmanager/src/debug_config.rs
@@ -15,10 +15,27 @@
//! Functions for AVF debug policy and debug level
use android_system_virtualizationservice::aidl::android::system::virtualizationservice::{
- VirtualMachineAppConfig::DebugLevel::DebugLevel, VirtualMachineConfig::VirtualMachineConfig,
+ VirtualMachineAppConfig::DebugLevel::DebugLevel,
};
use std::fs::File;
use std::io::Read;
+use log::info;
+use rustutils::system_properties;
+
+const DEBUG_POLICY_LOG_PATH: &str = "/proc/device-tree/avf/guest/common/log";
+const DEBUG_POLICY_RAMDUMP_PATH: &str = "/proc/device-tree/avf/guest/common/ramdump";
+const DEBUG_POLICY_ADB_PATH: &str = "/proc/device-tree/avf/guest/microdroid/adb";
+
+const SYSPROP_CUSTOM_DEBUG_POLICY_PATH: &str = "hypervisor.virtualizationmanager.debug_policy.path";
+
+/// Debug configurations for both debug level and debug policy
+#[derive(Debug)]
+pub struct DebugConfig {
+ pub debug_level: DebugLevel,
+ debug_policy_log: bool,
+ debug_policy_ramdump: bool,
+ debug_policy_adb: bool,
+}
/// Get debug policy value in bool. It's true iff the value is explicitly set to <1>.
fn get_debug_policy_bool(path: &'static str) -> Option<bool> {
@@ -29,28 +46,49 @@
Some(u32::from_be_bytes(log) == 1)
}
-/// Get whether console output should be configred for VM to leave console and adb log.
-/// Caller should create pipe and prepare for receiving VM log with it.
-pub fn should_prepare_console_output(debug_level: DebugLevel) -> bool {
- debug_level != DebugLevel::NONE
- || get_debug_policy_bool("/proc/device-tree/avf/guest/common/log").unwrap_or_default()
- || get_debug_policy_bool("/proc/device-tree/avf/guest/microdroid/adb").unwrap_or_default()
-}
+impl DebugConfig {
+ pub fn new(debug_level: DebugLevel) -> Self {
+ match system_properties::read(SYSPROP_CUSTOM_DEBUG_POLICY_PATH).unwrap_or_default() {
+ Some(debug_policy_path) if !debug_policy_path.is_empty() => {
+ // TODO: Read debug policy file and override log, adb, ramdump for testing.
+ info!("Debug policy is disabled by sysprop");
+ Self {
+ debug_level,
+ debug_policy_log: false,
+ debug_policy_ramdump: false,
+ debug_policy_adb: false,
+ }
+ }
+ _ => {
+ let debug_config = Self {
+ debug_level,
+ debug_policy_log: get_debug_policy_bool(DEBUG_POLICY_LOG_PATH)
+ .unwrap_or_default(),
+ debug_policy_ramdump: get_debug_policy_bool(DEBUG_POLICY_RAMDUMP_PATH)
+ .unwrap_or_default(),
+ debug_policy_adb: get_debug_policy_bool(DEBUG_POLICY_ADB_PATH)
+ .unwrap_or_default(),
+ };
+ info!("Loaded debug policy from host OS: {:?}", debug_config);
-/// Get whether debug apexes (MICRODROID_REQUIRED_APEXES_DEBUG) are required.
-pub fn should_include_debug_apexes(debug_level: DebugLevel) -> bool {
- debug_level != DebugLevel::NONE
- || get_debug_policy_bool("/proc/device-tree/avf/guest/microdroid/adb").unwrap_or_default()
-}
+ debug_config
+ }
+ }
+ }
-/// Decision to support ramdump
-pub fn is_ramdump_needed(config: &VirtualMachineConfig) -> bool {
- let enabled_in_dp =
- get_debug_policy_bool("/proc/device-tree/avf/guest/common/ramdump").unwrap_or_default();
- let debuggable = match config {
- VirtualMachineConfig::RawConfig(_) => false,
- VirtualMachineConfig::AppConfig(config) => config.debugLevel == DebugLevel::FULL,
- };
+ /// Get whether console output should be configred for VM to leave console and adb log.
+ /// Caller should create pipe and prepare for receiving VM log with it.
+ pub fn should_prepare_console_output(&self) -> bool {
+ self.debug_level != DebugLevel::NONE || self.debug_policy_log || self.debug_policy_adb
+ }
- enabled_in_dp || debuggable
+ /// Get whether debug apexes (MICRODROID_REQUIRED_APEXES_DEBUG) are required.
+ pub fn should_include_debug_apexes(&self) -> bool {
+ self.debug_level != DebugLevel::NONE || self.debug_policy_adb
+ }
+
+ /// Decision to support ramdump
+ pub fn is_ramdump_needed(&self) -> bool {
+ self.debug_level != DebugLevel::NONE || self.debug_policy_ramdump
+ }
}
diff --git a/virtualizationmanager/src/payload.rs b/virtualizationmanager/src/payload.rs
index 99aea01..33659d4 100644
--- a/virtualizationmanager/src/payload.rs
+++ b/virtualizationmanager/src/payload.rs
@@ -14,7 +14,7 @@
//! Payload disk image
-use crate::debug_config::should_include_debug_apexes;
+use crate::debug_config::DebugConfig;
use android_system_virtualizationservice::aidl::android::system::virtualizationservice::{
DiskImage::DiskImage,
Partition::Partition,
@@ -257,6 +257,7 @@
/// ..
fn make_payload_disk(
app_config: &VirtualMachineAppConfig,
+ debug_config: &DebugConfig,
apk_file: File,
idsig_file: File,
vm_payload_config: &VmPayloadConfig,
@@ -274,8 +275,7 @@
let apex_list = pm.get_apex_list(vm_payload_config.prefer_staged)?;
// collect APEXes from config
- let mut apex_infos =
- collect_apex_infos(&apex_list, &vm_payload_config.apexes, app_config.debugLevel);
+ let mut apex_infos = collect_apex_infos(&apex_list, &vm_payload_config.apexes, debug_config);
// Pass sorted list of apexes. Sorting key shouldn't use `path` because it will change after
// reboot with prefer_staged. `last_update_seconds` is added to distinguish "samegrade"
@@ -380,10 +380,10 @@
fn collect_apex_infos<'a>(
apex_list: &'a ApexInfoList,
apex_configs: &[ApexConfig],
- debug_level: DebugLevel,
+ debug_config: &DebugConfig,
) -> Vec<&'a ApexInfo> {
let mut additional_apexes: Vec<&str> = MICRODROID_REQUIRED_APEXES.to_vec();
- if should_include_debug_apexes(debug_level) {
+ if debug_config.should_include_debug_apexes() {
additional_apexes.extend(MICRODROID_REQUIRED_APEXES_DEBUG.to_vec());
}
@@ -437,6 +437,7 @@
pub fn add_microdroid_payload_images(
config: &VirtualMachineAppConfig,
+ debug_config: &DebugConfig,
temporary_directory: &Path,
apk_file: File,
idsig_file: File,
@@ -445,6 +446,7 @@
) -> Result<()> {
vm_config.disks.push(make_payload_disk(
config,
+ debug_config,
apk_file,
idsig_file,
vm_payload_config,
@@ -582,7 +584,7 @@
ApexConfig { name: "{CLASSPATH}".to_string() },
];
assert_eq!(
- collect_apex_infos(&apex_info_list, &apex_configs, DebugLevel::FULL),
+ collect_apex_infos(&apex_info_list, &apex_configs, &DebugConfig::new(DebugLevel::FULL)),
vec![
// Pass active/required APEXes
&apex_info_list.list[0],