Use binder::IntoBinderResult to simplify error handling
In addition to the trait, a local trait `LogResult` is also added for
the case of logging an error while returning it. ex:
```
let x = some_function()
.context("some message")
.with_log() // if error, log the error message
.or_binder_exception(ExceptionCode::INVALID_ARGUMENT)?;
```
Bug: 294348831
Test: m com.android.virt
Change-Id: I461ba6501be66014f73831f4320a50cd9eee0ae7
diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index dab5e86..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;
@@ -48,6 +48,20 @@
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,10 +176,10 @@
) -> 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>> {
@@ -405,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 9952496..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,67 +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);
};
let unbind_path = path.join("driver/unbind");
if unbind_path.exists() {
- write(&unbind_path, device_str.as_bytes()).map_err(|e| {
- Status::new_exception_str(
- ExceptionCode::SERVICE_SPECIFIC,
- Some(format!("could not unbind {device_str}: {e:?}")),
- )
- })?;
+ 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)