From d21764ca113abdfa6a6e0dd97f87ca1cb9e661e0 Mon Sep 17 00:00:00 2001 From: Alan Stokes Date: Mon, 25 Oct 2021 15:33:40 +0100 Subject: [PATCH] Enable debug mode for test instance of CompOS This is useful in case the tests fail, and during development. While I was refactoring this I also arranged to not create a new instance of VirtualizationService when we already have one to hand. Bug: 186126194 Test: Presubmits Test: Manual: Was able to get VM logs again Change-Id: I1540979c9f80f8e32c31bd1b382d14576233117d --- compos/common/compos_client.rs | 22 ++++++++++++++----- compos/composd/src/instance_manager.rs | 19 ++++++++++------ compos/composd/src/instance_starter.rs | 30 +++++++++++++++++--------- compos/verify_key/verify_key.rs | 6 ++++-- 4 files changed, 53 insertions(+), 24 deletions(-) diff --git a/compos/common/compos_client.rs b/compos/common/compos_client.rs index a69538ec..6277a559 100644 --- a/compos/common/compos_client.rs +++ b/compos/common/compos_client.rs @@ -21,7 +21,7 @@ use android_system_virtualizationservice::aidl::android::system::virtualizations IVirtualMachine::IVirtualMachine, IVirtualMachineCallback::{BnVirtualMachineCallback, IVirtualMachineCallback}, IVirtualizationService::IVirtualizationService, - VirtualMachineAppConfig::VirtualMachineAppConfig, + VirtualMachineAppConfig::{DebugLevel::DebugLevel, VirtualMachineAppConfig}, VirtualMachineConfig::VirtualMachineConfig, }; use android_system_virtualizationservice::binder::{ @@ -51,6 +51,13 @@ pub struct VmInstance { cid: i32, } +/// Parameters to be used when creating a virtual machine instance. +#[derive(Default, Debug, Clone)] +pub struct VmParameters { + /// Whether the VM should be debuggable. + pub debug_mode: bool, +} + impl VmInstance { /// Return a new connection to the Virtualization Service binder interface. This will start the /// service if necessary. @@ -59,8 +66,12 @@ impl VmInstance { .context("Failed to find VirtualizationService") } - /// Start a new CompOS VM instance using the specified instance image file. - pub fn start(instance_image: File) -> Result { + /// Start a new CompOS VM instance using the specified instance image file and parameters. + pub fn start( + service: &dyn IVirtualizationService, + instance_image: File, + parameters: &VmParameters, + ) -> Result { let instance_fd = ParcelFileDescriptor::new(instance_image); let apex_dir = Path::new(COMPOS_APEX_ROOT); @@ -78,16 +89,17 @@ impl VmInstance { let log_fd = File::create(data_dir.join("vm.log")).context("Failed to create log file")?; let log_fd = ParcelFileDescriptor::new(log_fd); + let debug_level = if parameters.debug_mode { DebugLevel::FULL } else { DebugLevel::NONE }; + let config = VirtualMachineConfig::AppConfig(VirtualMachineAppConfig { apk: Some(apk_fd), idsig: Some(idsig_fd), instanceImage: Some(instance_fd), configPath: "assets/vm_config.json".to_owned(), + debugLevel: debug_level, ..Default::default() }); - let service = Self::connect_to_virtualization_service()?; - let vm = service.createVm(&config, Some(&log_fd)).context("Failed to create VM")?; let vm_state = Arc::new(VmStateMonitor::default()); diff --git a/compos/composd/src/instance_manager.rs b/compos/composd/src/instance_manager.rs index e31296d9..6291d59e 100644 --- a/compos/composd/src/instance_manager.rs +++ b/compos/composd/src/instance_manager.rs @@ -22,6 +22,7 @@ use android_system_virtualizationservice::aidl::android::system::virtualizations use anyhow::{bail, Context, Result}; use compos_aidl_interface::aidl::com::android::compos::ICompOsService::ICompOsService; use compos_aidl_interface::binder::Strong; +use compos_common::compos_client::VmParameters; use compos_common::{CURRENT_INSTANCE_DIR, TEST_INSTANCE_DIR}; use std::sync::{Arc, Mutex, Weak}; use virtualizationservice::IVirtualizationService::IVirtualizationService; @@ -44,20 +45,26 @@ impl InstanceManager { #[allow(dead_code)] // TODO: Make use of this pub fn start_current_instance(&self) -> Result> { - self.start_instance(CURRENT_INSTANCE_DIR) + self.start_instance(CURRENT_INSTANCE_DIR, VmParameters::default()) } pub fn start_test_instance(&self) -> Result> { - self.start_instance(TEST_INSTANCE_DIR) + let vm_parameters = VmParameters { debug_mode: true }; + self.start_instance(TEST_INSTANCE_DIR, vm_parameters) } - fn start_instance(&self, instance_name: &str) -> Result> { + fn start_instance( + &self, + instance_name: &str, + vm_parameters: VmParameters, + ) -> Result> { let mut state = self.state.lock().unwrap(); state.mark_starting()?; // Don't hold the lock while we start the instance to avoid blocking other callers. drop(state); - let instance = self.try_start_instance(instance_name); + let instance_starter = InstanceStarter::new(instance_name, vm_parameters); + let instance = self.try_start_instance(instance_starter); let mut state = self.state.lock().unwrap(); if let Ok(ref instance) = instance { @@ -68,10 +75,8 @@ impl InstanceManager { instance } - fn try_start_instance(&self, instance_name: &str) -> Result> { - let instance_starter = InstanceStarter::new(instance_name); + fn try_start_instance(&self, instance_starter: InstanceStarter) -> Result> { let compos_instance = instance_starter.create_or_start_instance(&*self.service)?; - Ok(Arc::new(compos_instance)) } } diff --git a/compos/composd/src/instance_starter.rs b/compos/composd/src/instance_starter.rs index 63aefb88..1a6e5928 100644 --- a/compos/composd/src/instance_starter.rs +++ b/compos/composd/src/instance_starter.rs @@ -23,7 +23,7 @@ use android_system_virtualizationservice::aidl::android::system::virtualizations use anyhow::{bail, Context, Result}; use compos_aidl_interface::aidl::com::android::compos::ICompOsService::ICompOsService; use compos_aidl_interface::binder::{ParcelFileDescriptor, Strong}; -use compos_common::compos_client::VmInstance; +use compos_common::compos_client::{VmInstance, VmParameters}; use compos_common::{ COMPOS_DATA_ROOT, INSTANCE_IMAGE_FILE, PRIVATE_KEY_BLOB_FILE, PUBLIC_KEY_FILE, }; @@ -50,10 +50,11 @@ pub struct InstanceStarter { instance_image: PathBuf, key_blob: PathBuf, public_key: PathBuf, + vm_parameters: VmParameters, } impl InstanceStarter { - pub fn new(instance_name: &str) -> Self { + pub fn new(instance_name: &str, vm_parameters: VmParameters) -> Self { let instance_root = Path::new(COMPOS_DATA_ROOT).join(instance_name); let instant_root_path = instance_root.as_path(); let instance_image = instant_root_path.join(INSTANCE_IMAGE_FILE); @@ -65,23 +66,27 @@ impl InstanceStarter { instance_image, key_blob, public_key, + vm_parameters, } } pub fn create_or_start_instance( &self, - service: &dyn IVirtualizationService, + virtualization_service: &dyn IVirtualizationService, ) -> Result { - let compos_instance = self.start_existing_instance(); + let compos_instance = self.start_existing_instance(virtualization_service); match compos_instance { Ok(_) => return compos_instance, Err(e) => warn!("Failed to start: {}", e), } - self.start_new_instance(service) + self.start_new_instance(virtualization_service) } - fn start_existing_instance(&self) -> Result { + fn start_existing_instance( + &self, + virtualization_service: &dyn IVirtualizationService, + ) -> Result { // No point even trying if the files we need aren't there. self.check_files_exist()?; @@ -90,7 +95,7 @@ impl InstanceStarter { let key_blob = fs::read(&self.key_blob).context("Reading private key blob")?; let public_key = fs::read(&self.public_key).context("Reading public key")?; - let compos_instance = self.start_vm()?; + let compos_instance = self.start_vm(virtualization_service)?; let service = &compos_instance.service; if !service.verifySigningKey(&key_blob, &public_key).context("Verifying key pair")? { @@ -117,7 +122,7 @@ impl InstanceStarter { self.create_instance_image(virtualization_service)?; - let compos_instance = self.start_vm()?; + let compos_instance = self.start_vm(virtualization_service)?; let service = &compos_instance.service; let key_data = service.generateSigningKey().context("Generating signing key")?; @@ -149,13 +154,18 @@ impl InstanceStarter { Ok(()) } - fn start_vm(&self) -> Result { + fn start_vm( + &self, + virtualization_service: &dyn IVirtualizationService, + ) -> Result { let instance_image = fs::OpenOptions::new() .read(true) .write(true) .open(&self.instance_image) .context("Failed to open instance image")?; - let vm_instance = VmInstance::start(instance_image).context("Starting VM")?; + let vm_instance = + VmInstance::start(virtualization_service, instance_image, &self.vm_parameters) + .context("Starting VM")?; let service = vm_instance.get_service().context("Connecting to CompOS")?; Ok(CompOsInstance { vm_instance, service }) } diff --git a/compos/verify_key/verify_key.rs b/compos/verify_key/verify_key.rs index a5b0b6bd..0a9d36ba 100644 --- a/compos/verify_key/verify_key.rs +++ b/compos/verify_key/verify_key.rs @@ -19,7 +19,7 @@ use anyhow::{bail, Context, Result}; use compos_aidl_interface::binder::ProcessState; -use compos_common::compos_client::VmInstance; +use compos_common::compos_client::{VmInstance, VmParameters}; use compos_common::{ COMPOS_DATA_ROOT, CURRENT_INSTANCE_DIR, INSTANCE_IMAGE_FILE, PENDING_INSTANCE_DIR, PRIVATE_KEY_BLOB_FILE, PUBLIC_KEY_FILE, @@ -91,7 +91,9 @@ fn verify(instance_dir: &Path) -> Result<()> { let public_key = read_small_file(public_key).context("Failed to read public key")?; let instance_image = File::open(instance_image).context("Failed to open instance image")?; - let vm_instance = VmInstance::start(instance_image)?; + let virtualization_service = VmInstance::connect_to_virtualization_service()?; + let vm_instance = + VmInstance::start(&*virtualization_service, instance_image, &VmParameters::default())?; let service = vm_instance.get_service()?; let result = service.verifySigningKey(&blob, &public_key).context("Verifying signing key")?;