Remove cid parameter from VirtualMachineService

Cid parameter has been passed just to identify the guest VMs. This makes
the server identify itself, with incoming vsock address.

Bug: 199259751
Test: atest MicrodroidHostTestCases ComposHostTestCases
Change-Id: I8dacfec80574fe765b96139cdecb6f3192728577
This commit is contained in:
Inseob Kim 2021-10-25 14:28:10 +00:00
parent d45caf03ea
commit c7d28c7d78
6 changed files with 50 additions and 92 deletions

View File

@ -36,9 +36,6 @@ use binder::{
use binder_common::rpc_server::run_rpc_server;
use compos_common::COMPOS_VSOCK_PORT;
use log::{debug, error};
use nix::ioctl_read_bad;
use std::fs::OpenOptions;
use std::os::unix::io::AsRawFd;
/// The CID representing the host VM
const VMADDR_CID_HOST: u32 = 2;
@ -64,12 +61,11 @@ fn try_main() -> Result<()> {
let service = compsvc::new_binder()?.as_binder();
let vm_service = get_vm_service()?;
let local_cid = get_local_cid()?;
debug!("compsvc is starting as a rpc service.");
let retval = run_rpc_server(service, COMPOS_VSOCK_PORT, || {
if let Err(e) = vm_service.notifyPayloadReady(local_cid as i32) {
if let Err(e) = vm_service.notifyPayloadReady() {
error!("Unable to notify ready: {}", e);
}
});
@ -94,26 +90,3 @@ fn get_vm_service() -> Result<Strong<dyn IVirtualMachineService>> {
FromIBinder::try_from(ibinder).context("Connecting to IVirtualMachineService")
}
// TODO(b/199259751): remove this after VS can check the peer addresses of binder clients
fn get_local_cid() -> Result<u32> {
let f = OpenOptions::new()
.read(true)
.write(false)
.open("/dev/vsock")
.context("Failed to open /dev/vsock")?;
let mut cid = 0;
// SAFETY: the kernel only modifies the given u32 integer.
unsafe { vm_sockets_get_local_cid(f.as_raw_fd(), &mut cid) }
.context("Failed to get local CID")?;
Ok(cid)
}
// TODO(b/199259751): remove this after VS can check the peer addresses of binder clients
const IOCTL_VM_SOCKETS_GET_LOCAL_CID: usize = 0x7b9;
ioctl_read_bad!(
/// Gets local cid from /dev/vsock
vm_sockets_get_local_cid,
IOCTL_VM_SOCKETS_GET_LOCAL_CID,
u32
);

View File

@ -27,12 +27,11 @@ use idsig::V4Signature;
use log::{error, info, warn};
use microdroid_metadata::{write_metadata, Metadata};
use microdroid_payload_config::{Task, TaskType, VmPayloadConfig};
use nix::ioctl_read_bad;
use payload::{get_apex_data_from_payload, load_metadata, to_metadata};
use rustutils::system_properties;
use rustutils::system_properties::PropertyWatcher;
use std::fs::{self, File, OpenOptions};
use std::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd};
use std::os::unix::io::{FromRawFd, IntoRawFd};
use std::path::Path;
use std::process::{Command, Stdio};
use std::str;
@ -67,27 +66,6 @@ fn get_vms_rpc_binder() -> Result<Strong<dyn IVirtualMachineService>> {
}
}
const IOCTL_VM_SOCKETS_GET_LOCAL_CID: usize = 0x7b9;
ioctl_read_bad!(
/// Gets local cid from /dev/vsock
vm_sockets_get_local_cid,
IOCTL_VM_SOCKETS_GET_LOCAL_CID,
u32
);
// TODO: remove this after VS can check the peer addresses of binder clients
fn get_local_cid() -> Result<u32> {
let f = OpenOptions::new()
.read(true)
.write(false)
.open("/dev/vsock")
.context("failed to open /dev/vsock")?;
let mut ret = 0;
// SAFETY: the kernel only modifies the given u32 integer.
unsafe { vm_sockets_get_local_cid(f.as_raw_fd(), &mut ret) }?;
Ok(ret)
}
fn main() {
if let Err(e) = try_main() {
error!("failed with {:?}", e);
@ -242,14 +220,13 @@ fn exec_task(task: &Task, service: &Strong<dyn IVirtualMachineService>) -> Resul
info!("executing main task {:?}...", task);
let mut command = build_command(task)?;
let local_cid = get_local_cid()?;
info!("notifying payload started");
service.notifyPayloadStarted(local_cid as i32)?;
service.notifyPayloadStarted()?;
let exit_status = command.spawn()?.wait()?;
if let Some(code) = exit_status.code() {
info!("notifying payload finished");
service.notifyPayloadFinished(local_cid as i32, code)?;
service.notifyPayloadFinished(code)?;
if code == 0 {
info!("task successfully finished");

View File

@ -198,21 +198,6 @@ Result<T> report_test(std::string name, Result<T> result) {
return result;
}
Result<unsigned> get_local_cid() {
// TODO: remove this after VS can check the peer addresses of binder clients
unique_fd fd(open("/dev/vsock", O_RDONLY));
if (fd.get() == -1) {
return ErrnoError() << "failed to open /dev/vsock";
}
unsigned cid;
if (ioctl(fd.get(), IOCTL_VM_SOCKETS_GET_LOCAL_CID, &cid) == -1) {
return ErrnoError() << "failed to IOCTL_VM_SOCKETS_GET_LOCAL_CID";
}
return cid;
}
Result<void> start_test_service() {
class TestService : public aidl::com::android::microdroid::testservice::BnTestService {
ndk::ScopedAStatus addInteger(int32_t a, int32_t b, int32_t* out) override {
@ -232,9 +217,7 @@ Result<void> start_test_service() {
std::cerr << "failed to connect VirtualMachineService";
return;
}
if (auto res = get_local_cid(); !res.ok()) {
std::cerr << "failed to get local cid: " << res.error();
} else if (!virtualMachineService->notifyPayloadReady(res.value()).isOk()) {
if (!virtualMachineService->notifyPayloadReady().isOk()) {
std::cerr << "failed to notify payload ready to virtualizationservice";
}
};

View File

@ -31,19 +31,16 @@ interface IVirtualMachineService {
/**
* Notifies that the payload has started.
* TODO(b/191845268): remove cid parameter
*/
void notifyPayloadStarted(int cid);
void notifyPayloadStarted();
/**
* Notifies that the payload is ready to serve.
* TODO(b/191845268): remove cid parameter
*/
void notifyPayloadReady(int cid);
void notifyPayloadReady();
/**
* Notifies that the payload has finished.
* TODO(b/191845268): remove cid parameter
*/
void notifyPayloadFinished(int cid, int exitCode);
void notifyPayloadFinished(int exitCode);
}

View File

@ -55,8 +55,10 @@ use std::ffi::CStr;
use std::fs::{create_dir, File, OpenOptions};
use std::io::{Error, ErrorKind, Write};
use std::num::NonZeroU32;
use std::os::raw;
use std::os::unix::io::{FromRawFd, IntoRawFd};
use std::path::{Path, PathBuf};
use std::ptr::null_mut;
use std::sync::{Arc, Mutex, Weak};
use vmconfig::VmConfig;
use vsock::{SockAddr, VsockListener, VsockStream};
@ -345,15 +347,18 @@ impl VirtualizationService {
});
// binder server for vm
let state = service.state.clone(); // reference to state (not the state itself) is copied
let mut state = service.state.clone(); // reference to state (not the state itself) is copied
std::thread::spawn(move || {
let mut service = VirtualMachineService::new_binder(state).as_binder();
let state_ptr = &mut state as *mut _ as *mut raw::c_void;
debug!("virtual machine service is starting as an RPC service.");
// SAFETY: Service ownership is transferring to the server and won't be valid afterward.
// Plus the binder objects are threadsafe.
// SAFETY: factory function is only ever called by RunRpcServerWithFactory, within the
// lifetime of the state, with context taking the pointer value above (so a properly
// aligned non-null pointer to an initialized instance).
let retval = unsafe {
binder_rpc_unstable_bindgen::RunRpcServer(
service.as_native_mut() as *mut binder_rpc_unstable_bindgen::AIBinder,
binder_rpc_unstable_bindgen::RunRpcServerWithFactory(
Some(VirtualMachineService::factory),
state_ptr,
VM_BINDER_SERVICE_PORT as u32,
)
};
@ -846,13 +851,14 @@ impl<'a, T> AsRef<T> for BorrowedOrOwned<'a, T> {
#[derive(Debug, Default)]
struct VirtualMachineService {
state: Arc<Mutex<State>>,
cid: Cid,
}
impl Interface for VirtualMachineService {}
impl IVirtualMachineService for VirtualMachineService {
fn notifyPayloadStarted(&self, cid: i32) -> binder::Result<()> {
let cid = cid as Cid;
fn notifyPayloadStarted(&self) -> binder::Result<()> {
let cid = self.cid;
if let Some(vm) = self.state.lock().unwrap().get_vm(cid) {
info!("VM having CID {} started payload", cid);
vm.update_payload_state(PayloadState::Started)
@ -869,8 +875,8 @@ impl IVirtualMachineService for VirtualMachineService {
}
}
fn notifyPayloadReady(&self, cid: i32) -> binder::Result<()> {
let cid = cid as Cid;
fn notifyPayloadReady(&self) -> binder::Result<()> {
let cid = self.cid;
if let Some(vm) = self.state.lock().unwrap().get_vm(cid) {
info!("VM having CID {} payload is ready", cid);
vm.update_payload_state(PayloadState::Ready)
@ -886,8 +892,8 @@ impl IVirtualMachineService for VirtualMachineService {
}
}
fn notifyPayloadFinished(&self, cid: i32, exit_code: i32) -> binder::Result<()> {
let cid = cid as Cid;
fn notifyPayloadFinished(&self, exit_code: i32) -> binder::Result<()> {
let cid = self.cid;
if let Some(vm) = self.state.lock().unwrap().get_vm(cid) {
info!("VM having CID {} finished payload", cid);
vm.update_payload_state(PayloadState::Finished)
@ -905,9 +911,26 @@ impl IVirtualMachineService for VirtualMachineService {
}
impl VirtualMachineService {
fn new_binder(state: Arc<Mutex<State>>) -> Strong<dyn IVirtualMachineService> {
// SAFETY: Service ownership is held by state, and the binder objects are threadsafe.
pub unsafe extern "C" fn factory(
cid: Cid,
context: *mut raw::c_void,
) -> *mut binder_rpc_unstable_bindgen::AIBinder {
let state_ptr = context as *mut Arc<Mutex<State>>;
let state = state_ptr.as_ref().unwrap();
if let Some(vm) = state.lock().unwrap().get_vm(cid) {
let mut vm_service = vm.vm_service.lock().unwrap();
let service = vm_service.get_or_insert_with(|| Self::new_binder(state.clone(), cid));
service.as_binder().as_native_mut() as *mut binder_rpc_unstable_bindgen::AIBinder
} else {
error!("connection from cid={} is not from a guest VM", cid);
null_mut()
}
}
fn new_binder(state: Arc<Mutex<State>>, cid: Cid) -> Strong<dyn IVirtualMachineService> {
BnVirtualMachineService::new_binder(
VirtualMachineService { state },
VirtualMachineService { state, cid },
BinderFeatures::default(),
)
}

View File

@ -29,6 +29,8 @@ use std::process::Command;
use std::sync::{Arc, Mutex};
use std::thread;
use vsock::VsockStream;
use android_system_virtualmachineservice::binder::Strong;
use android_system_virtualmachineservice::aidl::android::system::virtualmachineservice::IVirtualMachineService::IVirtualMachineService;
const CROSVM_PATH: &str = "/apex/com.android.virt/bin/crosvm";
@ -132,6 +134,8 @@ pub struct VmInstance {
pub callbacks: VirtualMachineCallbacks,
/// Input/output stream of the payload run in the VM.
pub stream: Mutex<Option<VsockStream>>,
/// VirtualMachineService binder object for the VM.
pub vm_service: Mutex<Option<Strong<dyn IVirtualMachineService>>>,
/// The latest lifecycle state which the payload reported itself to be in.
payload_state: Mutex<PayloadState>,
}
@ -158,6 +162,7 @@ impl VmInstance {
requester_debug_pid,
callbacks: Default::default(),
stream: Mutex::new(None),
vm_service: Mutex::new(None),
payload_state: Mutex::new(PayloadState::Starting),
})
}