Merge "Add device assignment host test" into main
diff --git a/microdroid_manager/src/vm_payload_service.rs b/microdroid_manager/src/vm_payload_service.rs
index bcddc3a..b44b0db 100644
--- a/microdroid_manager/src/vm_payload_service.rs
+++ b/microdroid_manager/src/vm_payload_service.rs
@@ -18,13 +18,27 @@
use android_system_virtualization_payload::aidl::android::system::virtualization::payload::IVmPayloadService::{
BnVmPayloadService, IVmPayloadService, VM_PAYLOAD_SERVICE_SOCKET_NAME};
use android_system_virtualmachineservice::aidl::android::system::virtualmachineservice::IVirtualMachineService::IVirtualMachineService;
-use anyhow::Result;
-use binder::{Interface, BinderFeatures, ExceptionCode, Status, Strong};
+use anyhow::{anyhow, Context, Result};
+use binder::{Interface, BinderFeatures, ExceptionCode, Strong, IntoBinderResult};
use diced_open_dice::{DiceArtifacts, OwnedDiceArtifacts};
use log::{error, info};
use rpcbinder::RpcServer;
use std::os::unix::io::OwnedFd;
+/// Convenient trait for logging an error while returning it
+trait LogResult<T, E> {
+ fn with_log(self) -> std::result::Result<T, E>;
+}
+
+impl<T, E: std::fmt::Debug> LogResult<T, E> for std::result::Result<T, E> {
+ fn with_log(self) -> std::result::Result<T, E> {
+ self.map_err(|e| {
+ error!("{e:?}");
+ e
+ })
+ }
+}
+
/// Implementation of `IVmPayloadService`.
struct VmPayloadService {
allow_restricted_apis: bool,
@@ -39,7 +53,8 @@
fn getVmInstanceSecret(&self, identifier: &[u8], size: i32) -> binder::Result<Vec<u8>> {
if !(0..=32).contains(&size) {
- return Err(Status::new_exception(ExceptionCode::ILLEGAL_ARGUMENT, None));
+ return Err(anyhow!("size {size} not in range (0..=32)"))
+ .or_binder_exception(ExceptionCode::ILLEGAL_ARGUMENT);
}
// Use a fixed salt to scope the derivation to this API. It was randomly generated.
let salt = [
@@ -48,10 +63,10 @@
0xB7, 0xA8, 0x43, 0x92,
];
let mut secret = vec![0; size.try_into().unwrap()];
- derive_sealing_key(&self.dice, &salt, identifier, &mut secret).map_err(|e| {
- error!("Failed to derive VM instance secret: {:?}", e);
- Status::new_service_specific_error(-1, None)
- })?;
+ derive_sealing_key(&self.dice, &salt, identifier, &mut secret)
+ .context("Failed to derive VM instance secret")
+ .with_log()
+ .or_service_specific_exception(-1)?;
Ok(secret)
}
@@ -60,7 +75,7 @@
if let Some(bcc) = self.dice.bcc() {
Ok(bcc.to_vec())
} else {
- Err(Status::new_exception_str(ExceptionCode::ILLEGAL_STATE, Some("bcc is none")))
+ Err(anyhow!("bcc is none")).or_binder_exception(ExceptionCode::ILLEGAL_STATE)
}
}
@@ -91,8 +106,9 @@
if self.allow_restricted_apis {
Ok(())
} else {
- error!("Use of restricted APIs is not allowed");
- Err(Status::new_exception_str(ExceptionCode::SECURITY, Some("Use of restricted APIs")))
+ Err(anyhow!("Use of restricted APIs is not allowed"))
+ .with_log()
+ .or_binder_exception(ExceptionCode::SECURITY)
}
}
}
diff --git a/virtualizationmanager/src/aidl.rs b/virtualizationmanager/src/aidl.rs
index 91bd60b..ae81a17 100644
--- a/virtualizationmanager/src/aidl.rs
+++ b/virtualizationmanager/src/aidl.rs
@@ -53,6 +53,7 @@
use binder::{
self, wait_for_interface, BinderFeatures, ExceptionCode, Interface, ParcelFileDescriptor,
Status, StatusCode, Strong,
+ IntoBinderResult,
};
use disk::QcowFile;
use lazy_static::lazy_static;
@@ -76,6 +77,20 @@
use vsock::VsockStream;
use zip::ZipArchive;
+/// Convenient trait for logging an error while returning it
+trait LogResult<T, E> {
+ fn with_log(self) -> std::result::Result<T, E>;
+}
+
+impl<T, E: std::fmt::Debug> LogResult<T, E> for std::result::Result<T, E> {
+ fn with_log(self) -> std::result::Result<T, E> {
+ self.map_err(|e| {
+ error!("{e:?}");
+ e
+ })
+ }
+}
+
/// The unique ID of a VM used (together with a port number) for vsock communication.
pub type Cid = u32;
@@ -179,7 +194,6 @@
Ok(())
}
}
-
impl IVirtualizationService for VirtualizationService {
/// Creates (but does not start) a new VM with the given configuration, assigning it the next
/// available CID.
@@ -212,27 +226,17 @@
partition_type: PartitionType,
) -> binder::Result<()> {
check_manage_access()?;
- let size_bytes = size_bytes.try_into().map_err(|e| {
- Status::new_exception_str(
- ExceptionCode::ILLEGAL_ARGUMENT,
- Some(format!("Invalid size {}: {:?}", size_bytes, e)),
- )
- })?;
+ let size_bytes = size_bytes
+ .try_into()
+ .with_context(|| format!("Invalid size: {}", size_bytes))
+ .or_binder_exception(ExceptionCode::ILLEGAL_ARGUMENT)?;
let size_bytes = round_up(size_bytes, PARTITION_GRANULARITY_BYTES);
let image = clone_file(image_fd)?;
// initialize the file. Any data in the file will be erased.
- image.set_len(0).map_err(|e| {
- Status::new_service_specific_error_str(
- -1,
- Some(format!("Failed to reset a file: {:?}", e)),
- )
- })?;
- let mut part = QcowFile::new(image, size_bytes).map_err(|e| {
- Status::new_service_specific_error_str(
- -1,
- Some(format!("Failed to create QCOW2 image: {:?}", e)),
- )
- })?;
+ image.set_len(0).context("Failed to reset a file").or_service_specific_exception(-1)?;
+ let mut part = QcowFile::new(image, size_bytes)
+ .context("Failed to create QCOW2 image")
+ .or_service_specific_exception(-1)?;
match partition_type {
PartitionType::RAW => Ok(()),
@@ -243,12 +247,8 @@
format!("Unsupported partition type {:?}", partition_type),
)),
}
- .map_err(|e| {
- Status::new_service_specific_error_str(
- -1,
- Some(format!("Failed to initialize partition as {:?}: {:?}", partition_type, e)),
- )
- })?;
+ .with_context(|| format!("Failed to initialize partition as {:?}", partition_type))
+ .or_service_specific_exception(-1)?;
Ok(())
}
@@ -261,8 +261,7 @@
) -> binder::Result<()> {
check_manage_access()?;
- create_or_update_idsig_file(input_fd, idsig_fd)
- .map_err(|e| Status::new_service_specific_error_str(-1, Some(format!("{:?}", e))))?;
+ create_or_update_idsig_file(input_fd, idsig_fd).or_service_specific_exception(-1)?;
Ok(())
}
@@ -309,10 +308,8 @@
}
}
}
- Err(Status::new_service_specific_error_str(
- -1,
- Some("Too many attempts to create VM context failed."),
- ))
+ Err(anyhow!("Too many attempts to create VM context failed"))
+ .or_service_specific_exception(-1)
}
fn create_vm_internal(
@@ -381,12 +378,12 @@
let (is_app_config, config) = match config {
VirtualMachineConfig::RawConfig(config) => (false, BorrowedOrOwned::Borrowed(config)),
VirtualMachineConfig::AppConfig(config) => {
- let config =
- load_app_config(config, &debug_config, &temporary_directory).map_err(|e| {
+ let config = load_app_config(config, &debug_config, &temporary_directory)
+ .or_service_specific_exception_with(-1, |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))
+ message
})?;
(true, BorrowedOrOwned::Owned(config))
}
@@ -410,26 +407,21 @@
}
})
.try_for_each(check_label_for_partition)
- .map_err(|e| Status::new_service_specific_error_str(-1, Some(format!("{:?}", e))))?;
+ .or_service_specific_exception(-1)?;
let kernel = maybe_clone_file(&config.kernel)?;
let initrd = maybe_clone_file(&config.initrd)?;
// In a protected VM, we require custom kernels to come from a trusted source (b/237054515).
if config.protectedVm {
- check_label_for_kernel_files(&kernel, &initrd).map_err(|e| {
- Status::new_service_specific_error_str(-1, Some(format!("{:?}", e)))
- })?;
+ check_label_for_kernel_files(&kernel, &initrd).or_service_specific_exception(-1)?;
}
let zero_filler_path = temporary_directory.join("zero.img");
- write_zero_filler(&zero_filler_path).map_err(|e| {
- error!("Failed to make composite image: {:?}", e);
- Status::new_service_specific_error_str(
- -1,
- Some(format!("Failed to make composite image: {:?}", e)),
- )
- })?;
+ write_zero_filler(&zero_filler_path)
+ .context("Failed to make composite image")
+ .with_log()
+ .or_service_specific_exception(-1)?;
// Assemble disk images if needed.
let disks = config
@@ -450,28 +442,21 @@
CpuTopology::MATCH_HOST => (None, true),
CpuTopology::ONE_CPU => (NonZeroU32::new(1), false),
val => {
- error!("Unexpected value of CPU topology: {:?}", val);
- return Err(Status::new_service_specific_error_str(
- -1,
- Some(format!("Failed to parse CPU topology value: {:?}", val)),
- ));
+ return Err(anyhow!("Failed to parse CPU topology value {:?}", val))
+ .with_log()
+ .or_service_specific_exception(-1);
}
};
let devices_dtbo = if !config.devices.is_empty() {
let mut set = HashSet::new();
for device in config.devices.iter() {
- let path = canonicalize(device).map_err(|e| {
- Status::new_exception_str(
- ExceptionCode::ILLEGAL_ARGUMENT,
- Some(format!("can't canonicalize {device}: {e:?}")),
- )
- })?;
+ let path = canonicalize(device)
+ .with_context(|| format!("can't canonicalize {device}"))
+ .or_service_specific_exception(-1)?;
if !set.insert(path) {
- return Err(Status::new_exception_str(
- ExceptionCode::ILLEGAL_ARGUMENT,
- Some(format!("duplicated device {device}")),
- ));
+ return Err(anyhow!("duplicated device {device}"))
+ .or_binder_exception(ExceptionCode::ILLEGAL_ARGUMENT);
}
}
let dtbo_path = temporary_directory.join("dtbo");
@@ -533,13 +518,9 @@
requester_debug_pid,
vm_context,
)
- .map_err(|e| {
- error!("Failed to create VM with config {:?}: {:?}", config, e);
- Status::new_service_specific_error_str(
- -1,
- Some(format!("Failed to create VM: {:?}", e)),
- )
- })?,
+ .with_context(|| format!("Failed to create VM with config {:?}", config))
+ .with_log()
+ .or_service_specific_exception(-1)?,
);
state.add_vm(Arc::downgrade(&instance));
Ok(VirtualMachine::create(instance))
@@ -590,10 +571,8 @@
let image = if !disk.partitions.is_empty() {
if disk.image.is_some() {
warn!("DiskImage {:?} contains both image and partitions.", disk);
- return Err(Status::new_exception_str(
- ExceptionCode::ILLEGAL_ARGUMENT,
- Some("DiskImage contains both image and partitions."),
- ));
+ return Err(anyhow!("DiskImage contains both image and partitions"))
+ .or_binder_exception(ExceptionCode::ILLEGAL_ARGUMENT);
}
let composite_image_filenames =
@@ -605,13 +584,9 @@
&composite_image_filenames.header,
&composite_image_filenames.footer,
)
- .map_err(|e| {
- error!("Failed to make composite image with config {:?}: {:?}", disk, e);
- Status::new_service_specific_error_str(
- -1,
- Some(format!("Failed to make composite image: {:?}", e)),
- )
- })?;
+ .with_context(|| format!("Failed to make composite disk image with config {:?}", disk))
+ .with_log()
+ .or_service_specific_exception(-1)?;
// Pass the file descriptors for the various partition files to crosvm when it
// is run.
@@ -622,10 +597,8 @@
clone_file(image)?
} else {
warn!("DiskImage {:?} didn't contain image or partitions.", disk);
- return Err(Status::new_exception_str(
- ExceptionCode::ILLEGAL_ARGUMENT,
- Some("DiskImage didn't contain image or partitions."),
- ));
+ return Err(anyhow!("DiskImage didn't contain image or partitions."))
+ .or_binder_exception(ExceptionCode::ILLEGAL_ARGUMENT);
};
Ok(DiskFile { image, writable: disk.writable })
@@ -783,10 +756,8 @@
if perm_svc.checkPermission(perm, calling_pid, calling_uid as i32)? {
Ok(())
} else {
- Err(Status::new_exception_str(
- ExceptionCode::SECURITY,
- Some(format!("does not have the {} permission", perm)),
- ))
+ Err(anyhow!("does not have the {} permission", perm))
+ .or_binder_exception(ExceptionCode::SECURITY)
}
}
@@ -892,40 +863,41 @@
}
fn start(&self) -> binder::Result<()> {
- self.instance.start().map_err(|e| {
- error!("Error starting VM with CID {}: {:?}", self.instance.cid, e);
- Status::new_service_specific_error_str(-1, Some(e.to_string()))
- })
+ self.instance
+ .start()
+ .with_context(|| format!("Error starting VM with CID {}", self.instance.cid))
+ .with_log()
+ .or_service_specific_exception(-1)
}
fn stop(&self) -> binder::Result<()> {
- self.instance.kill().map_err(|e| {
- error!("Error stopping VM with CID {}: {:?}", self.instance.cid, e);
- Status::new_service_specific_error_str(-1, Some(e.to_string()))
- })
+ self.instance
+ .kill()
+ .with_context(|| format!("Error stopping VM with CID {}", self.instance.cid))
+ .with_log()
+ .or_service_specific_exception(-1)
}
fn onTrimMemory(&self, level: MemoryTrimLevel) -> binder::Result<()> {
- self.instance.trim_memory(level).map_err(|e| {
- error!("Error trimming VM with CID {}: {:?}", self.instance.cid, e);
- Status::new_service_specific_error_str(-1, Some(e.to_string()))
- })
+ self.instance
+ .trim_memory(level)
+ .with_context(|| format!("Error trimming VM with CID {}", self.instance.cid))
+ .with_log()
+ .or_service_specific_exception(-1)
}
fn connectVsock(&self, port: i32) -> binder::Result<ParcelFileDescriptor> {
if !matches!(&*self.instance.vm_state.lock().unwrap(), VmState::Running { .. }) {
- return Err(Status::new_service_specific_error_str(-1, Some("VM is not running")));
+ return Err(anyhow!("VM is not running")).or_service_specific_exception(-1);
}
let port = port as u32;
if port < 1024 {
- return Err(Status::new_service_specific_error_str(
- -1,
- Some(format!("Can't connect to privileged port {port}")),
- ));
+ return Err(anyhow!("Can't connect to privileged port {port}"))
+ .or_service_specific_exception(-1);
}
- let stream = VsockStream::connect_with_cid_port(self.instance.cid, port).map_err(|e| {
- Status::new_service_specific_error_str(-1, Some(format!("Failed to connect: {:?}", e)))
- })?;
+ let stream = VsockStream::connect_with_cid_port(self.instance.cid, port)
+ .context("Failed to connect")
+ .or_service_specific_exception(-1)?;
Ok(vsock_stream_to_pfd(stream))
}
}
@@ -1051,17 +1023,15 @@
}
/// Converts a `&ParcelFileDescriptor` to a `File` by cloning the file.
-pub fn clone_file(file: &ParcelFileDescriptor) -> Result<File, Status> {
- file.as_ref().try_clone().map_err(|e| {
- Status::new_exception_str(
- ExceptionCode::BAD_PARCELABLE,
- Some(format!("Failed to clone File from ParcelFileDescriptor: {:?}", e)),
- )
- })
+pub fn clone_file(file: &ParcelFileDescriptor) -> binder::Result<File> {
+ file.as_ref()
+ .try_clone()
+ .context("Failed to clone File from ParcelFileDescriptor")
+ .or_binder_exception(ExceptionCode::BAD_PARCELABLE)
}
/// Converts an `&Option<ParcelFileDescriptor>` to an `Option<File>` by cloning the file.
-fn maybe_clone_file(file: &Option<ParcelFileDescriptor>) -> Result<Option<File>, Status> {
+fn maybe_clone_file(file: &Option<ParcelFileDescriptor>) -> binder::Result<Option<File>> {
file.as_ref().map(clone_file).transpose()
}
@@ -1073,13 +1043,10 @@
}
/// Parses the platform version requirement string.
-fn parse_platform_version_req(s: &str) -> Result<VersionReq, Status> {
- VersionReq::parse(s).map_err(|e| {
- Status::new_exception_str(
- ExceptionCode::BAD_PARCELABLE,
- Some(format!("Invalid platform version requirement {}: {:?}", s, e)),
- )
- })
+fn parse_platform_version_req(s: &str) -> binder::Result<VersionReq> {
+ VersionReq::parse(s)
+ .with_context(|| format!("Invalid platform version requirement {}", s))
+ .or_binder_exception(ExceptionCode::BAD_PARCELABLE)
}
/// Create the empty ramdump file
@@ -1088,13 +1055,10 @@
// VM will emit ramdump to. `ramdump_read` will be sent back to the client (i.e. the VM
// owner) for readout.
let ramdump_path = temporary_directory.join("ramdump");
- let ramdump = File::create(ramdump_path).map_err(|e| {
- error!("Failed to prepare ramdump file: {:?}", e);
- Status::new_service_specific_error_str(
- -1,
- Some(format!("Failed to prepare ramdump file: {:?}", e)),
- )
- })?;
+ let ramdump = File::create(ramdump_path)
+ .context("Failed to prepare ramdump file")
+ .with_log()
+ .or_service_specific_exception(-1)?;
Ok(ramdump)
}
@@ -1107,20 +1071,16 @@
fn check_gdb_allowed(config: &VirtualMachineConfig) -> binder::Result<()> {
if is_protected(config) {
- return Err(Status::new_exception_str(
- ExceptionCode::SECURITY,
- Some("can't use gdb with protected VMs"),
- ));
+ return Err(anyhow!("Can't use gdb with protected VMs"))
+ .or_binder_exception(ExceptionCode::SECURITY);
}
match config {
VirtualMachineConfig::RawConfig(_) => Ok(()),
VirtualMachineConfig::AppConfig(config) => {
if config.debugLevel != DebugLevel::FULL {
- Err(Status::new_exception_str(
- ExceptionCode::SECURITY,
- Some("can't use gdb with non-debuggable VMs"),
- ))
+ Err(anyhow!("Can't use gdb with non-debuggable VMs"))
+ .or_binder_exception(ExceptionCode::SECURITY)
} else {
Ok(())
}
@@ -1150,9 +1110,8 @@
return Ok(None);
};
- let (raw_read_fd, raw_write_fd) = pipe().map_err(|e| {
- Status::new_service_specific_error_str(-1, Some(format!("Failed to create pipe: {:?}", e)))
- })?;
+ let (raw_read_fd, raw_write_fd) =
+ pipe().context("Failed to create pipe").or_service_specific_exception(-1)?;
// SAFETY: We are the sole owner of this FD as we just created it, and it is valid and open.
let mut reader = BufReader::new(unsafe { File::from_raw_fd(raw_read_fd) });
@@ -1212,9 +1171,8 @@
let cid = self.cid;
if let Some(vm) = self.state.lock().unwrap().get_vm(cid) {
info!("VM with CID {} started payload", cid);
- vm.update_payload_state(PayloadState::Started).map_err(|e| {
- Status::new_exception_str(ExceptionCode::ILLEGAL_STATE, Some(e.to_string()))
- })?;
+ vm.update_payload_state(PayloadState::Started)
+ .or_binder_exception(ExceptionCode::ILLEGAL_STATE)?;
vm.callbacks.notify_payload_started(cid);
let vm_start_timestamp = vm.vm_metric.lock().unwrap().start_timestamp;
@@ -1222,10 +1180,7 @@
Ok(())
} else {
error!("notifyPayloadStarted is called from an unknown CID {}", cid);
- Err(Status::new_service_specific_error_str(
- -1,
- Some(format!("cannot find a VM with CID {}", cid)),
- ))
+ Err(anyhow!("cannot find a VM with CID {}", cid)).or_service_specific_exception(-1)
}
}
@@ -1233,17 +1188,13 @@
let cid = self.cid;
if let Some(vm) = self.state.lock().unwrap().get_vm(cid) {
info!("VM with CID {} reported payload is ready", cid);
- vm.update_payload_state(PayloadState::Ready).map_err(|e| {
- Status::new_exception_str(ExceptionCode::ILLEGAL_STATE, Some(e.to_string()))
- })?;
+ vm.update_payload_state(PayloadState::Ready)
+ .or_binder_exception(ExceptionCode::ILLEGAL_STATE)?;
vm.callbacks.notify_payload_ready(cid);
Ok(())
} else {
error!("notifyPayloadReady is called from an unknown CID {}", cid);
- Err(Status::new_service_specific_error_str(
- -1,
- Some(format!("cannot find a VM with CID {}", cid)),
- ))
+ Err(anyhow!("cannot find a VM with CID {}", cid)).or_service_specific_exception(-1)
}
}
@@ -1251,17 +1202,13 @@
let cid = self.cid;
if let Some(vm) = self.state.lock().unwrap().get_vm(cid) {
info!("VM with CID {} finished payload", cid);
- vm.update_payload_state(PayloadState::Finished).map_err(|e| {
- Status::new_exception_str(ExceptionCode::ILLEGAL_STATE, Some(e.to_string()))
- })?;
+ vm.update_payload_state(PayloadState::Finished)
+ .or_binder_exception(ExceptionCode::ILLEGAL_STATE)?;
vm.callbacks.notify_payload_finished(cid, exit_code);
Ok(())
} else {
error!("notifyPayloadFinished is called from an unknown CID {}", cid);
- Err(Status::new_service_specific_error_str(
- -1,
- Some(format!("cannot find a VM with CID {}", cid)),
- ))
+ Err(anyhow!("cannot find a VM with CID {}", cid)).or_service_specific_exception(-1)
}
}
@@ -1269,17 +1216,13 @@
let cid = self.cid;
if let Some(vm) = self.state.lock().unwrap().get_vm(cid) {
info!("VM with CID {} encountered an error", cid);
- vm.update_payload_state(PayloadState::Finished).map_err(|e| {
- Status::new_exception_str(ExceptionCode::ILLEGAL_STATE, Some(e.to_string()))
- })?;
+ vm.update_payload_state(PayloadState::Finished)
+ .or_binder_exception(ExceptionCode::ILLEGAL_STATE)?;
vm.callbacks.notify_error(cid, error_code, message);
Ok(())
} else {
error!("notifyError is called from an unknown CID {}", cid);
- Err(Status::new_service_specific_error_str(
- -1,
- Some(format!("cannot find a VM with CID {}", cid)),
- ))
+ Err(anyhow!("cannot find a VM with CID {}", cid)).or_service_specific_exception(-1)
}
}
@@ -1287,10 +1230,8 @@
let cid = self.cid;
let Some(vm) = self.state.lock().unwrap().get_vm(cid) else {
error!("requestCertificate is called from an unknown CID {cid}");
- return Err(Status::new_service_specific_error_str(
- -1,
- Some(format!("cannot find a VM with CID {}", cid)),
- ));
+ return Err(anyhow!("cannot find a VM with CID {}", cid))
+ .or_service_specific_exception(-1);
};
let instance_img_path = vm.temporary_directory.join("rkpvm_instance.img");
let instance_img = OpenOptions::new()
@@ -1298,13 +1239,9 @@
.read(true)
.write(true)
.open(instance_img_path)
- .map_err(|e| {
- error!("Failed to create rkpvm_instance.img file: {:?}", e);
- Status::new_service_specific_error_str(
- -1,
- Some(format!("Failed to create rkpvm_instance.img file: {:?}", e)),
- )
- })?;
+ .context("Failed to create rkpvm_instance.img file")
+ .with_log()
+ .or_service_specific_exception(-1)?;
GLOBAL_SERVICE.requestCertificate(csr, &ParcelFileDescriptor::new(instance_img))
}
}
diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index 2e667d4..6dc5485 100644
--- a/virtualizationservice/src/aidl.rs
+++ b/virtualizationservice/src/aidl.rs
@@ -33,7 +33,7 @@
};
use android_system_virtualmachineservice::aidl::android::system::virtualmachineservice::IVirtualMachineService::VM_TOMBSTONES_SERVICE_PORT;
use anyhow::{anyhow, ensure, Context, Result};
-use binder::{self, wait_for_interface, BinderFeatures, ExceptionCode, Interface, LazyServiceGuard, Status, Strong};
+use binder::{self, wait_for_interface, BinderFeatures, ExceptionCode, Interface, LazyServiceGuard, Status, Strong, IntoBinderResult};
use libc::VMADDR_CID_HOST;
use log::{error, info, warn};
use rustutils::system_properties;
@@ -42,12 +42,26 @@
use std::io::{Read, Write};
use std::os::unix::fs::PermissionsExt;
use std::os::unix::raw::{pid_t, uid_t};
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
use std::sync::{Arc, Mutex, Weak};
use tombstoned_client::{DebuggerdDumpType, TombstonedConnection};
use vsock::{VsockListener, VsockStream};
use nix::unistd::{chown, Uid};
+/// Convenient trait for logging an error while returning it
+trait LogResult<T, E> {
+ fn with_log(self) -> std::result::Result<T, E>;
+}
+
+impl<T, E: std::fmt::Debug> LogResult<T, E> for std::result::Result<T, E> {
+ fn with_log(self) -> std::result::Result<T, E> {
+ self.map_err(|e| {
+ error!("{e:?}");
+ e
+ })
+ }
+}
+
/// The unique ID of a VM used (together with a port number) for vsock communication.
pub type Cid = u32;
@@ -102,15 +116,10 @@
match ret {
0 => Ok(()),
- -1 => Err(Status::new_exception_str(
- ExceptionCode::ILLEGAL_STATE,
- Some(std::io::Error::last_os_error().to_string()),
- )),
- n => Err(Status::new_exception_str(
- ExceptionCode::ILLEGAL_STATE,
- Some(format!("Unexpected return value from prlimit(): {n}")),
- )),
+ -1 => Err(std::io::Error::last_os_error().into()),
+ n => Err(anyhow!("Unexpected return value from prlimit(): {n}")),
}
+ .or_binder_exception(ExceptionCode::ILLEGAL_STATE)
}
fn allocateGlobalVmContext(
@@ -122,9 +131,9 @@
let requester_uid = get_calling_uid();
let requester_debug_pid = requester_debug_pid as pid_t;
let state = &mut *self.state.lock().unwrap();
- state.allocate_vm_context(requester_uid, requester_debug_pid).map_err(|e| {
- Status::new_exception_str(ExceptionCode::ILLEGAL_STATE, Some(e.to_string()))
- })
+ state
+ .allocate_vm_context(requester_uid, requester_debug_pid)
+ .or_binder_exception(ExceptionCode::ILLEGAL_STATE)
}
fn atomVmBooted(&self, atom: &AtomVmBooted) -> Result<(), Status> {
@@ -167,20 +176,22 @@
) -> binder::Result<Vec<u8>> {
check_manage_access()?;
info!("Received csr. Getting certificate...");
- request_certificate(csr, instance_img_fd).map_err(|e| {
- error!("Failed to get certificate. Error: {e:?}");
- Status::new_exception_str(ExceptionCode::SERVICE_SPECIFIC, Some(e.to_string()))
- })
+ request_certificate(csr, instance_img_fd)
+ .context("Failed to get certificate")
+ .with_log()
+ .or_service_specific_exception(-1)
}
fn getAssignableDevices(&self) -> binder::Result<Vec<AssignableDevice>> {
check_use_custom_virtual_machine()?;
// TODO(b/291191362): read VM DTBO to find assignable devices.
- Ok(vec![AssignableDevice {
- kind: "eh".to_owned(),
- node: "/sys/bus/platform/devices/16d00000.eh".to_owned(),
- }])
+ let mut devices = Vec::new();
+ let eh_path = "/sys/bus/platform/devices/16d00000.eh";
+ if Path::new(eh_path).exists() {
+ devices.push(AssignableDevice { kind: "eh".to_owned(), node: eh_path.to_owned() });
+ }
+ Ok(devices)
}
fn bindDevicesToVfioDriver(
@@ -403,10 +414,8 @@
if perm_svc.checkPermission(perm, calling_pid, calling_uid as i32)? {
Ok(())
} else {
- Err(Status::new_exception_str(
- ExceptionCode::SECURITY,
- Some(format!("does not have the {} permission", perm)),
- ))
+ Err(anyhow!("does not have the {} permission", perm))
+ .or_binder_exception(ExceptionCode::SECURITY)
}
}
diff --git a/virtualizationservice/vfio_handler/Android.bp b/virtualizationservice/vfio_handler/Android.bp
index efbb7b5..9ed17f2 100644
--- a/virtualizationservice/vfio_handler/Android.bp
+++ b/virtualizationservice/vfio_handler/Android.bp
@@ -22,10 +22,11 @@
rustlibs: [
"android.system.virtualizationservice_internal-rust",
"libandroid_logger",
+ "libanyhow",
"libbinder_rs",
+ "liblazy_static",
"liblog_rust",
"libnix",
- "liblazy_static",
],
apex_available: ["com.android.virt"],
}
diff --git a/virtualizationservice/vfio_handler/src/aidl.rs b/virtualizationservice/vfio_handler/src/aidl.rs
index f082aba..a826c75 100644
--- a/virtualizationservice/vfio_handler/src/aidl.rs
+++ b/virtualizationservice/vfio_handler/src/aidl.rs
@@ -14,9 +14,10 @@
//! Implementation of the AIDL interface of the VirtualizationService.
+use anyhow::{anyhow, Context};
use android_system_virtualizationservice_internal::aidl::android::system::virtualizationservice_internal::IVfioHandler::IVfioHandler;
use android_system_virtualizationservice_internal::binder::ParcelFileDescriptor;
-use binder::{self, ExceptionCode, Interface, Status};
+use binder::{self, ExceptionCode, Interface, IntoBinderResult};
use lazy_static::lazy_static;
use std::fs::{read_link, write};
use std::io::Write;
@@ -41,27 +42,21 @@
) -> binder::Result<()> {
// permission check is already done by IVirtualizationServiceInternal.
if !*IS_VFIO_SUPPORTED {
- return Err(Status::new_exception_str(
- ExceptionCode::UNSUPPORTED_OPERATION,
- Some("VFIO-platform not supported"),
- ));
+ return Err(anyhow!("VFIO-platform not supported"))
+ .or_binder_exception(ExceptionCode::UNSUPPORTED_OPERATION);
}
devices.iter().try_for_each(|x| bind_device(Path::new(x)))?;
- let mut dtbo = dtbo.as_ref().try_clone().map_err(|e| {
- Status::new_exception_str(
- ExceptionCode::BAD_PARCELABLE,
- Some(format!("Failed to clone File from ParcelFileDescriptor: {e:?}")),
- )
- })?;
+ let mut dtbo = dtbo
+ .as_ref()
+ .try_clone()
+ .context("Failed to clone File from ParcelFileDescriptor")
+ .or_binder_exception(ExceptionCode::BAD_PARCELABLE)?;
// TODO(b/291191362): write DTBO for devices to dtbo.
- dtbo.write(b"\n").map_err(|e| {
- Status::new_exception_str(
- ExceptionCode::BAD_PARCELABLE,
- Some(format!("Can't write to ParcelFileDescriptor: {e:?}")),
- )
- })?;
+ dtbo.write(b"\n")
+ .context("Can't write to ParcelFileDescriptor")
+ .or_binder_exception(ExceptionCode::BAD_PARCELABLE)?;
Ok(())
}
}
@@ -81,17 +76,13 @@
fn check_platform_device(path: &Path) -> binder::Result<()> {
if !path.exists() {
- return Err(Status::new_exception_str(
- ExceptionCode::ILLEGAL_ARGUMENT,
- Some(format!("no such device {path:?}")),
- ));
+ return Err(anyhow!("no such device {path:?}"))
+ .or_binder_exception(ExceptionCode::ILLEGAL_ARGUMENT);
}
if !path.starts_with(SYSFS_PLATFORM_DEVICES_PATH) {
- return Err(Status::new_exception_str(
- ExceptionCode::ILLEGAL_ARGUMENT,
- Some(format!("{path:?} is not a platform device")),
- ));
+ return Err(anyhow!("{path:?} is not a platform device"))
+ .or_binder_exception(ExceptionCode::ILLEGAL_ARGUMENT);
}
Ok(())
@@ -121,64 +112,48 @@
// unbind
let Some(device) = path.file_name() else {
- return Err(Status::new_exception_str(
- ExceptionCode::ILLEGAL_ARGUMENT,
- Some(format!("can't get device name from {path:?}")),
- ));
+ return Err(anyhow!("can't get device name from {path:?}"))
+ .or_binder_exception(ExceptionCode::ILLEGAL_ARGUMENT);
};
let Some(device_str) = device.to_str() else {
- return Err(Status::new_exception_str(
- ExceptionCode::ILLEGAL_ARGUMENT,
- Some(format!("invalid filename {device:?}")),
- ));
+ return Err(anyhow!("invalid filename {device:?}"))
+ .or_binder_exception(ExceptionCode::ILLEGAL_ARGUMENT);
};
- write(path.join("driver/unbind"), device_str.as_bytes()).map_err(|e| {
- Status::new_exception_str(
- ExceptionCode::SERVICE_SPECIFIC,
- Some(format!("could not unbind {device_str}: {e:?}")),
- )
- })?;
+ let unbind_path = path.join("driver/unbind");
+ if unbind_path.exists() {
+ write(&unbind_path, device_str.as_bytes())
+ .with_context(|| format!("could not unbind {device_str}"))
+ .or_service_specific_exception(-1)?;
+ }
// bind to VFIO
- write(path.join("driver_override"), b"vfio-platform").map_err(|e| {
- Status::new_exception_str(
- ExceptionCode::SERVICE_SPECIFIC,
- Some(format!("could not bind {device_str} to vfio-platform: {e:?}")),
- )
- })?;
+ write(path.join("driver_override"), b"vfio-platform")
+ .with_context(|| format!("could not bind {device_str} to vfio-platform"))
+ .or_service_specific_exception(-1)?;
- write(SYSFS_PLATFORM_DRIVERS_PROBE_PATH, device_str.as_bytes()).map_err(|e| {
- Status::new_exception_str(
- ExceptionCode::SERVICE_SPECIFIC,
- Some(format!("could not write {device_str} to drivers-probe: {e:?}")),
- )
- })?;
+ write(SYSFS_PLATFORM_DRIVERS_PROBE_PATH, device_str.as_bytes())
+ .with_context(|| format!("could not write {device_str} to drivers-probe"))
+ .or_service_specific_exception(-1)?;
// final check
if !is_bound_to_vfio_driver(path) {
- return Err(Status::new_exception_str(
- ExceptionCode::SERVICE_SPECIFIC,
- Some(format!("{path:?} still not bound to vfio driver")),
- ));
+ return Err(anyhow!("{path:?} still not bound to vfio driver"))
+ .or_service_specific_exception(-1);
}
if get_device_iommu_group(path).is_none() {
- return Err(Status::new_exception_str(
- ExceptionCode::SERVICE_SPECIFIC,
- Some(format!("can't get iommu group for {path:?}")),
- ));
+ return Err(anyhow!("can't get iommu group for {path:?}"))
+ .or_service_specific_exception(-1);
}
Ok(())
}
fn bind_device(path: &Path) -> binder::Result<()> {
- let path = path.canonicalize().map_err(|e| {
- Status::new_exception_str(
- ExceptionCode::ILLEGAL_ARGUMENT,
- Some(format!("can't canonicalize {path:?}: {e:?}")),
- )
- })?;
+ let path = path
+ .canonicalize()
+ .with_context(|| format!("can't canonicalize {path:?}"))
+ .or_binder_exception(ExceptionCode::ILLEGAL_ARGUMENT)?;
check_platform_device(&path)?;
bind_vfio_driver(&path)