Move VM callback to vmclient
Instead of having clients directly register a callback with VS,
implement a Rust level callback interface in vmclient. This saves an
extra binder call on each notification, a bunch of boilerplate code,
and allows us to provide a slightly better interface (e.g. we can use
the Rust DeathReason enum, as elsewhere in vmclient, for instantly
better logging).
I also replaced all our usages of <some_interface>::binder::{...} with
direct access to binder::{...}. That makes it clearer what depends on
the interface itself and what is just generic binder code. I realise
this should be a separate change, but I only realised that after doing
bits of both.
Test: composd_cmd test-compile, observe logs (on both success & failure)
Test: atest -b (to make sure all our tests build)
Test: Presubmits
Change-Id: Iceda8d7b8f8008f9d7a2c51106c2794f09bb378e
diff --git a/compos/common/Android.bp b/compos/common/Android.bp
index 0773652..23a1eb9 100644
--- a/compos/common/Android.bp
+++ b/compos/common/Android.bp
@@ -11,6 +11,7 @@
"android.system.virtualizationservice-rust",
"compos_aidl_interface-rust",
"libanyhow",
+ "libbinder_rs",
"liblazy_static",
"liblog_rust",
"libnested_virt",
diff --git a/compos/common/binder.rs b/compos/common/binder.rs
index 59726c0..d3550f7 100644
--- a/compos/common/binder.rs
+++ b/compos/common/binder.rs
@@ -16,7 +16,7 @@
//! Helper for converting Error types to what Binder expects
-use android_system_virtualizationservice::binder::{Result as BinderResult, Status};
+use binder::{Result as BinderResult, Status};
use log::warn;
use std::fmt::Debug;
diff --git a/compos/common/compos_client.rs b/compos/common/compos_client.rs
index cd1ece4..770f489 100644
--- a/compos/common/compos_client.rs
+++ b/compos/common/compos_client.rs
@@ -19,16 +19,12 @@
use crate::timeouts::TIMEOUTS;
use crate::{COMPOS_APEX_ROOT, COMPOS_DATA_ROOT, COMPOS_VSOCK_PORT, DEFAULT_VM_CONFIG_PATH};
use android_system_virtualizationservice::aidl::android::system::virtualizationservice::{
- DeathReason::DeathReason,
- IVirtualMachineCallback::{BnVirtualMachineCallback, IVirtualMachineCallback},
IVirtualizationService::IVirtualizationService,
VirtualMachineAppConfig::{DebugLevel::DebugLevel, VirtualMachineAppConfig},
VirtualMachineConfig::VirtualMachineConfig,
};
-use android_system_virtualizationservice::binder::{
- BinderFeatures, Interface, ParcelFileDescriptor, Result as BinderResult, Strong,
-};
use anyhow::{bail, Context, Result};
+use binder::{ParcelFileDescriptor, Strong};
use compos_aidl_interface::aidl::com::android::compos::ICompOsService::ICompOsService;
use log::{info, warn};
use rustutils::system_properties;
@@ -37,7 +33,7 @@
use std::num::NonZeroU32;
use std::path::{Path, PathBuf};
use std::thread;
-use vmclient::{VmInstance, VmWaitError};
+use vmclient::{DeathReason, VmInstance, VmWaitError};
/// This owns an instance of the CompOS VM.
pub struct ComposClient(VmInstance);
@@ -119,13 +115,10 @@
taskProfiles: parameters.task_profiles.clone(),
});
- let instance = VmInstance::create(service, &config, console_fd, log_fd)
+ let callback = Box::new(Callback {});
+ let instance = VmInstance::create(service, &config, console_fd, log_fd, Some(callback))
.context("Failed to create VM")?;
- let callback =
- BnVirtualMachineCallback::new_binder(VmCallback(), BinderFeatures::default());
- instance.vm.registerCallback(&callback)?;
-
instance.start()?;
let ready = instance.wait_until_ready(TIMEOUTS.vm_max_time_to_ready);
@@ -163,7 +156,7 @@
fn wait_for_shutdown(self) {
let death_reason = self.0.wait_for_death_with_timeout(TIMEOUTS.vm_max_time_to_exit);
match death_reason {
- Some(vmclient::DeathReason::Shutdown) => info!("VM has exited normally"),
+ Some(DeathReason::Shutdown) => info!("VM has exited normally"),
Some(reason) => warn!("VM died with reason {:?}", reason),
None => warn!("VM failed to exit, dropping"),
}
@@ -228,53 +221,36 @@
bail!("No VM support available")
}
-struct VmCallback();
-
-impl Interface for VmCallback {}
-
-impl IVirtualMachineCallback for VmCallback {
- fn onDied(&self, cid: i32, reason: DeathReason) -> BinderResult<()> {
- log::warn!("VM died, cid = {}, reason = {:?}", cid, reason);
- Ok(())
- }
-
- fn onPayloadStarted(
- &self,
- cid: i32,
- stream: Option<&ParcelFileDescriptor>,
- ) -> BinderResult<()> {
- if let Some(pfd) = stream {
- if let Err(e) = start_logging(pfd) {
+struct Callback {}
+impl vmclient::VmCallback for Callback {
+ fn on_payload_started(&self, cid: i32, stream: Option<&File>) {
+ if let Some(file) = stream {
+ if let Err(e) = start_logging(file) {
warn!("Can't log vm output: {}", e);
};
}
log::info!("VM payload started, cid = {}", cid);
- Ok(())
}
- fn onPayloadReady(&self, cid: i32) -> BinderResult<()> {
+ fn on_payload_ready(&self, cid: i32) {
log::info!("VM payload ready, cid = {}", cid);
- Ok(())
}
- fn onPayloadFinished(&self, cid: i32, exit_code: i32) -> BinderResult<()> {
+ fn on_payload_finished(&self, cid: i32, exit_code: i32) {
log::warn!("VM payload finished, cid = {}, exit code = {}", cid, exit_code);
- Ok(())
}
- fn onError(&self, cid: i32, error_code: i32, message: &str) -> BinderResult<()> {
- log::warn!("VM error, cid = {}, error code = {}, message = {}", cid, error_code, message,);
- Ok(())
+ fn on_error(&self, cid: i32, error_code: i32, message: &str) {
+ log::warn!("VM error, cid = {}, error code = {}, message = {}", cid, error_code, message);
}
- fn onRamdump(&self, _cid: i32, _ramdump: &ParcelFileDescriptor) -> BinderResult<()> {
- // TODO(b/238295267) send this to tombstone?
- Ok(())
+ fn on_died(&self, cid: i32, death_reason: DeathReason) {
+ log::warn!("VM died, cid = {}, reason = {:?}", cid, death_reason);
}
}
-fn start_logging(pfd: &ParcelFileDescriptor) -> Result<()> {
- let reader = BufReader::new(pfd.as_ref().try_clone().context("Cloning fd failed")?);
+fn start_logging(file: &File) -> Result<()> {
+ let reader = BufReader::new(file.try_clone().context("Cloning file failed")?);
thread::spawn(move || {
for line in reader.lines() {
match line {
diff --git a/compos/composd/src/composd_main.rs b/compos/composd/src/composd_main.rs
index ebcd689..5315828 100644
--- a/compos/composd/src/composd_main.rs
+++ b/compos/composd/src/composd_main.rs
@@ -25,8 +25,8 @@
mod service;
use crate::instance_manager::InstanceManager;
-use android_system_composd::binder::{register_lazy_service, ProcessState};
use anyhow::{Context, Result};
+use binder::{register_lazy_service, ProcessState};
use log::{error, info};
use std::panic;
use std::sync::Arc;
diff --git a/compos/composd/src/instance_manager.rs b/compos/composd/src/instance_manager.rs
index 9a0935c..75671d7 100644
--- a/compos/composd/src/instance_manager.rs
+++ b/compos/composd/src/instance_manager.rs
@@ -20,7 +20,7 @@
use crate::instance_starter::{CompOsInstance, InstanceStarter};
use android_system_virtualizationservice::aidl::android::system::virtualizationservice;
use anyhow::{bail, Result};
-use compos_aidl_interface::binder::Strong;
+use binder::Strong;
use compos_common::compos_client::VmParameters;
use compos_common::{
CURRENT_INSTANCE_DIR, DEX2OAT_CPU_SET_PROP_NAME, DEX2OAT_THREADS_PROP_NAME,
diff --git a/compos/composd/src/instance_starter.rs b/compos/composd/src/instance_starter.rs
index 111c719..2923ee0 100644
--- a/compos/composd/src/instance_starter.rs
+++ b/compos/composd/src/instance_starter.rs
@@ -21,9 +21,9 @@
IVirtualizationService::IVirtualizationService, PartitionType::PartitionType,
};
use anyhow::{Context, Result};
+use binder::{ParcelFileDescriptor, Strong};
use binder_common::lazy_service::LazyServiceGuard;
use compos_aidl_interface::aidl::com::android::compos::ICompOsService::ICompOsService;
-use compos_aidl_interface::binder::{ParcelFileDescriptor, Strong};
use compos_common::compos_client::{ComposClient, VmParameters};
use compos_common::{COMPOS_DATA_ROOT, IDSIG_FILE, IDSIG_MANIFEST_APK_FILE, INSTANCE_IMAGE_FILE};
use log::info;
diff --git a/compos/composd/src/odrefresh_task.rs b/compos/composd/src/odrefresh_task.rs
index 8fd574c..100fc50 100644
--- a/compos/composd/src/odrefresh_task.rs
+++ b/compos/composd/src/odrefresh_task.rs
@@ -22,8 +22,8 @@
ICompilationTask::ICompilationTask,
ICompilationTaskCallback::{FailureReason::FailureReason, ICompilationTaskCallback},
};
-use android_system_composd::binder::{Interface, Result as BinderResult, Strong};
use anyhow::{Context, Result};
+use binder::{Interface, Result as BinderResult, Strong};
use compos_aidl_interface::aidl::com::android::compos::ICompOsService::{
CompilationMode::CompilationMode, ICompOsService,
};
diff --git a/compos/composd/src/service.rs b/compos/composd/src/service.rs
index a9b8202..49cfd3a 100644
--- a/compos/composd/src/service.rs
+++ b/compos/composd/src/service.rs
@@ -26,10 +26,8 @@
ApexSource::ApexSource, BnIsolatedCompilationService, IIsolatedCompilationService,
},
};
-use android_system_composd::binder::{
- self, BinderFeatures, ExceptionCode, Interface, Status, Strong, ThreadState,
-};
use anyhow::{Context, Result};
+use binder::{self, BinderFeatures, ExceptionCode, Interface, Status, Strong, ThreadState};
use compos_aidl_interface::aidl::com::android::compos::ICompOsService::CompilationMode::CompilationMode;
use compos_common::binder::to_binder_result;
use compos_common::odrefresh::{PENDING_ARTIFACTS_SUBDIR, TEST_ARTIFACTS_SUBDIR};
diff --git a/compos/src/compilation.rs b/compos/src/compilation.rs
index e14cd94..ab228e1 100644
--- a/compos/src/compilation.rs
+++ b/compos/src/compilation.rs
@@ -32,7 +32,7 @@
},
IAuthFsService::IAuthFsService,
};
-use authfs_aidl_interface::binder::Strong;
+use binder::Strong;
use compos_aidl_interface::aidl::com::android::compos::ICompOsService::CompilationMode::CompilationMode;
use compos_common::odrefresh::ExitCode;
diff --git a/compos/src/compsvc.rs b/compos/src/compsvc.rs
index 088d41a..baf444e 100644
--- a/compos/src/compsvc.rs
+++ b/compos/src/compsvc.rs
@@ -30,12 +30,10 @@
use crate::artifact_signer::ArtifactSigner;
use crate::compilation::{odrefresh, OdrefreshContext};
use crate::compos_key;
+use binder::{BinderFeatures, ExceptionCode, Interface, Result as BinderResult, Status, Strong};
use compos_aidl_interface::aidl::com::android::compos::ICompOsService::{
BnCompOsService, CompilationMode::CompilationMode, ICompOsService,
};
-use compos_aidl_interface::binder::{
- BinderFeatures, ExceptionCode, Interface, Result as BinderResult, Status, Strong,
-};
use compos_common::binder::to_binder_result;
use compos_common::odrefresh::{is_system_property_interesting, ODREFRESH_PATH};
@@ -128,7 +126,7 @@
system_server_compiler_filter,
))?;
- let authfs_service = authfs_aidl_interface::binder::get_interface(AUTHFS_SERVICE_NAME)?;
+ let authfs_service = binder::get_interface(AUTHFS_SERVICE_NAME)?;
let exit_code = to_binder_result(
odrefresh(&self.odrefresh_path, context, authfs_service, |output_dir| {
// authfs only shows us the files we created, so it's ok to just sign everything
diff --git a/compos/verify/verify.rs b/compos/verify/verify.rs
index 224cde7..2ece8f5 100644
--- a/compos/verify/verify.rs
+++ b/compos/verify/verify.rs
@@ -19,7 +19,7 @@
use android_logger::LogId;
use anyhow::{bail, Context, Result};
-use compos_aidl_interface::binder::ProcessState;
+use binder::ProcessState;
use compos_common::compos_client::{ComposClient, VmParameters};
use compos_common::odrefresh::{
CURRENT_ARTIFACTS_SUBDIR, ODREFRESH_OUTPUT_ROOT_DIR, PENDING_ARTIFACTS_SUBDIR,