Merge "Migrate VirtualMachine API to @SystemApi"
diff --git a/microdroid_manager/src/main.rs b/microdroid_manager/src/main.rs
index a53b401..a706dbe 100644
--- a/microdroid_manager/src/main.rs
+++ b/microdroid_manager/src/main.rs
@@ -25,9 +25,7 @@
use crate::instance::{ApexData, ApkData, InstanceDisk, MicrodroidData, RootHash};
use crate::vm_payload_service::register_vm_payload_service;
use android_system_virtualizationcommon::aidl::android::system::virtualizationcommon::ErrorCode::ErrorCode;
-use android_system_virtualmachineservice::aidl::android::system::virtualmachineservice::IVirtualMachineService::{
- IVirtualMachineService, VM_BINDER_SERVICE_PORT,
-};
+use android_system_virtualmachineservice::aidl::android::system::virtualmachineservice::IVirtualMachineService::IVirtualMachineService;
use android_system_virtualization_payload::aidl::android::system::virtualization::payload::IVmPayloadService::{
VM_APK_CONTENTS_PATH,
VM_PAYLOAD_SERVICE_SOCKET_NAME,
@@ -160,8 +158,11 @@
}
fn get_vms_rpc_binder() -> Result<Strong<dyn IVirtualMachineService>> {
- get_vsock_rpc_interface(VMADDR_CID_HOST, VM_BINDER_SERVICE_PORT as u32)
- .context("Cannot connect to RPC service")
+ // The host is running a VirtualMachineService for this VM on a port equal
+ // to the CID of this VM.
+ let port = vsock::get_local_cid().context("Could not determine local CID")?;
+ get_vsock_rpc_interface(VMADDR_CID_HOST, port)
+ .context("Could not connect to IVirtualMachineService")
}
fn main() -> Result<()> {
diff --git a/tests/benchmark_hostside/java/android/avf/test/AVFHostTestCase.java b/tests/benchmark_hostside/java/android/avf/test/AVFHostTestCase.java
index 1996f4b..c47e915 100644
--- a/tests/benchmark_hostside/java/android/avf/test/AVFHostTestCase.java
+++ b/tests/benchmark_hostside/java/android/avf/test/AVFHostTestCase.java
@@ -149,7 +149,9 @@
}
private void appStartupHelper(String launchIntentPackage) throws Exception {
- assumeTrue("Skip on non-protected VMs", isProtectedVmSupported());
+ assumeTrue(
+ "Skip on non-protected VMs",
+ ((TestDevice) getDevice()).supportsMicrodroid(/*protectedVm=*/ true));
StartupTimeMetricCollection mCollection =
new StartupTimeMetricCollection(getPackageName(launchIntentPackage), ROUND_COUNT);
diff --git a/tests/hostside/helper/java/com/android/microdroid/test/host/MicrodroidHostTestCaseBase.java b/tests/hostside/helper/java/com/android/microdroid/test/host/MicrodroidHostTestCaseBase.java
index ac37ee0..e5aa908 100644
--- a/tests/hostside/helper/java/com/android/microdroid/test/host/MicrodroidHostTestCaseBase.java
+++ b/tests/hostside/helper/java/com/android/microdroid/test/host/MicrodroidHostTestCaseBase.java
@@ -237,8 +237,4 @@
.stdoutTrimmed()
.isEqualTo("microdroid");
}
-
- public boolean isProtectedVmSupported() throws DeviceNotAvailableException {
- return getDevice().getBooleanProperty("ro.boot.hypervisor.protected_vm.supported", false);
- }
}
diff --git a/tests/hostside/java/com/android/microdroid/test/MicrodroidHostTests.java b/tests/hostside/java/com/android/microdroid/test/MicrodroidHostTests.java
index cffaae1..f0c89c9 100644
--- a/tests/hostside/java/com/android/microdroid/test/MicrodroidHostTests.java
+++ b/tests/hostside/java/com/android/microdroid/test/MicrodroidHostTests.java
@@ -51,7 +51,6 @@
import com.android.tradefed.util.xml.AbstractXmlParser;
import org.json.JSONArray;
-import org.json.JSONException;
import org.json.JSONObject;
import org.junit.After;
import org.junit.Before;
@@ -433,7 +432,9 @@
@CddTest(requirements = {"9.17/C-2-1", "9.17/C-2-2", "9.17/C-2-6"})
public void testBootFailsWhenProtectedVmStartsWithImagesSignedWithDifferentKey()
throws Exception {
- assumeTrue("Protected VMs are not supported", isProtectedVmSupported());
+ assumeTrue(
+ "Protected VMs are not supported",
+ getAndroidDevice().supportsMicrodroid(/*protectedVm=*/ true));
File key = findTestFile("test.com.android.virt.pem");
Map<String, File> keyOverrides = Map.of();
@@ -490,12 +491,12 @@
private boolean isTombstoneGeneratedWithConfig(String configPath) throws Exception {
// Note this test relies on logcat values being printed by tombstone_transmit on
// and the reeceiver on host (virtualization_service)
- mMicrodroidDevice = MicrodroidBuilder
- .fromDevicePath(getPathForPackage(PACKAGE_NAME), configPath)
- .debugLevel("full")
- .memoryMib(minMemorySize())
- .numCpus(NUM_VCPUS)
- .build((TestDevice) getDevice());
+ mMicrodroidDevice =
+ MicrodroidBuilder.fromDevicePath(getPathForPackage(PACKAGE_NAME), configPath)
+ .debugLevel("full")
+ .memoryMib(minMemorySize())
+ .numCpus(NUM_VCPUS)
+ .build(getAndroidDevice());
mMicrodroidDevice.waitForBootComplete(BOOT_COMPLETE_TIMEOUT);
mMicrodroidDevice.enableAdbRoot();
@@ -546,7 +547,7 @@
ConfigUtils.uploadConfigForPushedAtoms(getDevice(), PACKAGE_NAME, atomIds);
// Create VM with microdroid
- TestDevice device = (TestDevice) getDevice();
+ TestDevice device = getAndroidDevice();
final String configPath = "assets/vm_config_apex.json"; // path inside the APK
ITestDevice microdroid =
MicrodroidBuilder.fromDevicePath(getPathForPackage(PACKAGE_NAME), configPath)
@@ -614,7 +615,7 @@
.debugLevel("full")
.memoryMib(minMemorySize())
.numCpus(NUM_VCPUS)
- .build((TestDevice) getDevice());
+ .build(getAndroidDevice());
mMicrodroidDevice.waitForBootComplete(BOOT_COMPLETE_TIMEOUT);
CommandRunner microdroid = new CommandRunner(mMicrodroidDevice);
@@ -676,12 +677,12 @@
@Test
public void testMicrodroidRamUsage() throws Exception {
final String configPath = "assets/vm_config.json";
- mMicrodroidDevice = MicrodroidBuilder
- .fromDevicePath(getPathForPackage(PACKAGE_NAME), configPath)
- .debugLevel("full")
- .memoryMib(minMemorySize())
- .numCpus(NUM_VCPUS)
- .build((TestDevice) getDevice());
+ mMicrodroidDevice =
+ MicrodroidBuilder.fromDevicePath(getPathForPackage(PACKAGE_NAME), configPath)
+ .debugLevel("full")
+ .memoryMib(minMemorySize())
+ .numCpus(NUM_VCPUS)
+ .build(getAndroidDevice());
mMicrodroidDevice.waitForBootComplete(BOOT_COMPLETE_TIMEOUT);
mMicrodroidDevice.enableAdbRoot();
@@ -716,9 +717,10 @@
}
@Test
- public void testCustomVirtualMachinePermission()
- throws DeviceNotAvailableException, IOException, JSONException {
- assumeTrue("Protected VMs are not supported", isProtectedVmSupported());
+ public void testCustomVirtualMachinePermission() throws Exception {
+ assumeTrue(
+ "Protected VMs are not supported",
+ getAndroidDevice().supportsMicrodroid(/*protectedVm=*/ true));
CommandRunner android = new CommandRunner(getDevice());
// Pull etc/microdroid.json
@@ -767,7 +769,7 @@
@After
public void shutdown() throws Exception {
if (mMicrodroidDevice != null) {
- ((TestDevice) getDevice()).shutdownMicrodroid(mMicrodroidDevice);
+ getAndroidDevice().shutdownMicrodroid(mMicrodroidDevice);
}
cleanUpVirtualizationTestSetup(getDevice());
@@ -785,4 +787,10 @@
SHELL_PACKAGE_NAME,
"android.permission.USE_CUSTOM_VIRTUAL_MACHINE");
}
+
+ private TestDevice getAndroidDevice() {
+ TestDevice androidDevice = (TestDevice) getDevice();
+ assertThat(androidDevice).isNotNull();
+ return androidDevice;
+ }
}
diff --git a/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl b/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl
index f2d92af..3fdb48a 100644
--- a/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl
+++ b/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl
@@ -21,12 +21,6 @@
interface IVirtualMachineService {
/**
* Port number that VirtualMachineService listens on connections from the guest VMs for the
- * VirtualMachineService binder service.
- */
- const int VM_BINDER_SERVICE_PORT = 5000;
-
- /**
- * Port number that VirtualMachineService listens on connections from the guest VMs for the
* tombtones
*/
const int VM_TOMBSTONES_SERVICE_PORT = 2000;
diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index 578960c..040c0d8 100644
--- a/virtualizationservice/src/aidl.rs
+++ b/virtualizationservice/src/aidl.rs
@@ -16,7 +16,7 @@
use crate::atom::{write_vm_booted_stats, write_vm_creation_stats};
use crate::composite::make_composite_image;
-use crate::crosvm::{CrosvmConfig, DiskFile, PayloadState, VmInstance, VmState};
+use crate::crosvm::{CrosvmConfig, DiskFile, PayloadState, VmContext, VmInstance, VmState};
use crate::payload::{add_microdroid_payload_images, add_microdroid_system_images};
use crate::selinux::{getfilecon, SeContext};
use android_os_permissions_aidl::aidl::android::os::IPermissionController;
@@ -41,22 +41,22 @@
IVirtualizationServiceInternal::{BnVirtualizationServiceInternal, IVirtualizationServiceInternal},
};
use android_system_virtualmachineservice::aidl::android::system::virtualmachineservice::IVirtualMachineService::{
- BnVirtualMachineService, IVirtualMachineService, VM_BINDER_SERVICE_PORT,
- VM_TOMBSTONES_SERVICE_PORT,
+ BnVirtualMachineService, IVirtualMachineService, VM_TOMBSTONES_SERVICE_PORT,
};
use anyhow::{anyhow, bail, Context, Result};
use apkverify::{HashAlgorithm, V4Signature};
use binder::{
self, BinderFeatures, ExceptionCode, Interface, LazyServiceGuard, ParcelFileDescriptor,
- SpIBinder, Status, StatusCode, Strong, ThreadState,
+ Status, StatusCode, Strong, ThreadState,
};
use disk::QcowFile;
use libc::VMADDR_CID_HOST;
use log::{debug, error, info, warn};
use microdroid_payload_config::{OsConfig, Task, TaskType, VmPayloadConfig};
-use rpcbinder::run_vsock_rpc_server_with_factory;
+use rpcbinder::RpcServer;
use rustutils::system_properties;
use semver::VersionReq;
+use std::collections::HashMap;
use std::convert::TryInto;
use std::ffi::CStr;
use std::fs::{create_dir, File, OpenOptions};
@@ -80,7 +80,8 @@
/// The first CID to assign to a guest VM managed by the VirtualizationService. CIDs lower than this
/// are reserved for the host or other usage.
-const FIRST_GUEST_CID: Cid = 10;
+const GUEST_CID_MIN: Cid = 2048;
+const GUEST_CID_MAX: Cid = 65535;
const SYSPROP_LAST_CID: &str = "virtualizationservice.state.last_cid";
@@ -100,6 +101,19 @@
const UNFORMATTED_STORAGE_MAGIC: &str = "UNFORMATTED-STORAGE";
+fn is_valid_guest_cid(cid: Cid) -> bool {
+ (GUEST_CID_MIN..=GUEST_CID_MAX).contains(&cid)
+}
+
+fn next_guest_cid(cid: Cid) -> Cid {
+ assert!(is_valid_guest_cid(cid));
+ if cid == GUEST_CID_MAX {
+ GUEST_CID_MIN
+ } else {
+ cid + 1
+ }
+}
+
/// Singleton service for allocating globally-unique VM resources, such as the CID, and running
/// singleton servers, like tombstone receiver.
#[derive(Debug, Default)]
@@ -136,25 +150,65 @@
/// The mutable state of the VirtualizationServiceInternal. There should only be one instance
/// of this struct.
#[derive(Debug, Default)]
-struct GlobalState {}
+struct GlobalState {
+ /// CIDs currently allocated to running VMs. A CID is never recycled as long
+ /// as there is a strong reference held by a GlobalVmContext.
+ held_cids: HashMap<Cid, Weak<Cid>>,
+}
impl GlobalState {
/// Get the next available CID, or an error if we have run out. The last CID used is stored in
/// a system property so that restart of virtualizationservice doesn't reuse CID while the host
/// Android is up.
- fn allocate_cid(&mut self) -> Result<Cid> {
- let cid = match system_properties::read(SYSPROP_LAST_CID)? {
- Some(val) => match val.parse::<Cid>() {
- Ok(num) => num.checked_add(1).ok_or_else(|| anyhow!("ran out of CIDs"))?,
+ fn allocate_cid(&mut self) -> Result<Arc<Cid>> {
+ // Garbage collect unused CIDs.
+ self.held_cids.retain(|_, cid| cid.strong_count() > 0);
+
+ // Start trying to find a CID from the last used CID + 1. This ensures
+ // that we do not eagerly recycle CIDs. It makes debugging easier but
+ // also means that retrying to allocate a CID, eg. because it is
+ // erroneously occupied by a process, will not recycle the same CID.
+ let last_cid_prop =
+ system_properties::read(SYSPROP_LAST_CID)?.and_then(|val| match val.parse::<Cid>() {
+ Ok(num) => {
+ if is_valid_guest_cid(num) {
+ Some(num)
+ } else {
+ error!("Invalid value '{}' of property '{}'", num, SYSPROP_LAST_CID);
+ None
+ }
+ }
Err(_) => {
error!("Invalid value '{}' of property '{}'", val, SYSPROP_LAST_CID);
- FIRST_GUEST_CID
+ None
}
- },
- None => FIRST_GUEST_CID,
+ });
+
+ let first_cid = if let Some(last_cid) = last_cid_prop {
+ next_guest_cid(last_cid)
+ } else {
+ GUEST_CID_MIN
};
- system_properties::write(SYSPROP_LAST_CID, &format!("{}", cid))?;
- Ok(cid)
+
+ let cid = self
+ .find_available_cid(first_cid..=GUEST_CID_MAX)
+ .or_else(|| self.find_available_cid(GUEST_CID_MIN..first_cid));
+
+ if let Some(cid) = cid {
+ let cid_arc = Arc::new(cid);
+ self.held_cids.insert(cid, Arc::downgrade(&cid_arc));
+ system_properties::write(SYSPROP_LAST_CID, &format!("{}", cid))?;
+ Ok(cid_arc)
+ } else {
+ Err(anyhow!("Could not find an available CID."))
+ }
+ }
+
+ fn find_available_cid<I>(&self, mut range: I) -> Option<Cid>
+ where
+ I: Iterator<Item = Cid>,
+ {
+ range.find(|cid| !self.held_cids.contains_key(cid))
}
}
@@ -162,14 +216,14 @@
#[derive(Debug, Default)]
struct GlobalVmContext {
/// The unique CID assigned to the VM for vsock communication.
- cid: Cid,
- /// Keeps our service process running as long as this VM instance exists.
+ cid: Arc<Cid>,
+ /// Keeps our service process running as long as this VM context exists.
#[allow(dead_code)]
lazy_service_guard: LazyServiceGuard,
}
impl GlobalVmContext {
- fn create(cid: Cid) -> Strong<dyn IGlobalVmContext> {
+ fn create(cid: Arc<Cid>) -> Strong<dyn IGlobalVmContext> {
let binder = GlobalVmContext { cid, ..Default::default() };
BnGlobalVmContext::new_binder(binder, BinderFeatures::default())
}
@@ -179,7 +233,7 @@
impl IGlobalVmContext for GlobalVmContext {
fn getCid(&self) -> binder::Result<i32> {
- Ok(self.cid as i32)
+ Ok(*self.cid as i32)
}
}
@@ -341,8 +395,10 @@
}
fn handle_stream_connection_tombstoned() -> Result<()> {
+ // Should not listen for tombstones on a guest VM's port.
+ assert!(!is_valid_guest_cid(VM_TOMBSTONES_SERVICE_PORT as Cid));
let listener =
- VsockListener::bind_with_cid_port(VMADDR_CID_HOST, VM_TOMBSTONES_SERVICE_PORT as u32)?;
+ VsockListener::bind_with_cid_port(VMADDR_CID_HOST, VM_TOMBSTONES_SERVICE_PORT as Cid)?;
for incoming_stream in listener.incoming() {
let mut incoming_stream = match incoming_stream {
Err(e) => {
@@ -394,22 +450,31 @@
let global_service =
BnVirtualizationServiceInternal::new_binder(global_service, BinderFeatures::default());
- let service = VirtualizationService { global_service, state: Default::default() };
+ VirtualizationService { global_service, state: Default::default() }
+ }
- // binder server for vm
- // reference to state (not the state itself) is copied
- let state = service.state.clone();
- std::thread::spawn(move || {
- debug!("VirtualMachineService is starting as an RPC service.");
- if run_vsock_rpc_server_with_factory(VM_BINDER_SERVICE_PORT as u32, |cid| {
- VirtualMachineService::factory(cid, &state)
- }) {
- debug!("RPC server has shut down gracefully");
- } else {
- panic!("Premature termination of RPC server");
+ fn create_vm_context(&self) -> Result<(VmContext, Cid)> {
+ const NUM_ATTEMPTS: usize = 5;
+
+ for _ in 0..NUM_ATTEMPTS {
+ let global_context = self.global_service.allocateGlobalVmContext()?;
+ let cid = global_context.getCid()? as Cid;
+ let service = VirtualMachineService::new_binder(self.state.clone(), cid).as_binder();
+
+ // Start VM service listening for connections from the new CID on port=CID.
+ // TODO(b/245727626): Only accept connections from the new VM.
+ let port = cid;
+ match RpcServer::new_vsock(service, port) {
+ Ok(vm_server) => {
+ vm_server.start();
+ return Ok((VmContext::new(global_context, vm_server), cid));
+ }
+ Err(err) => {
+ warn!("Could not start RpcServer on port {}: {}", port, err);
+ }
}
- });
- service
+ }
+ bail!("Too many attempts to create VM context failed.");
}
fn create_vm_internal(
@@ -435,8 +500,13 @@
check_use_custom_virtual_machine()?;
}
- let vm_context = self.global_service.allocateGlobalVmContext()?;
- let cid = vm_context.getCid()? as Cid;
+ let (vm_context, cid) = self.create_vm_context().map_err(|e| {
+ error!("Failed to create VmContext: {:?}", e);
+ Status::new_service_specific_error_str(
+ -1,
+ Some(format!("Failed to create VmContext: {:?}", e)),
+ )
+ })?;
let state = &mut *self.state.lock().unwrap();
let console_fd = console_fd.map(clone_file).transpose()?;
@@ -1188,17 +1258,6 @@
}
impl VirtualMachineService {
- fn factory(cid: Cid, state: &Arc<Mutex<State>>) -> Option<SpIBinder> {
- 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));
- Some(service.as_binder())
- } else {
- error!("connection from cid={} is not from a guest VM", cid);
- None
- }
- }
-
fn new_binder(state: Arc<Mutex<State>>, cid: Cid) -> Strong<dyn IVirtualMachineService> {
BnVirtualMachineService::new_binder(
VirtualMachineService { state, cid },
diff --git a/virtualizationservice/src/crosvm.rs b/virtualizationservice/src/crosvm.rs
index 68324c5..76e18db 100644
--- a/virtualizationservice/src/crosvm.rs
+++ b/virtualizationservice/src/crosvm.rs
@@ -42,6 +42,7 @@
use binder::Strong;
use android_system_virtualmachineservice::aidl::android::system::virtualmachineservice::IVirtualMachineService::IVirtualMachineService;
use tombstoned_client::{TombstonedConnection, DebuggerdDumpType};
+use rpcbinder::RpcServer;
/// external/crosvm
use base::UnixSeqpacketListener;
@@ -198,14 +199,30 @@
}
}
+/// Internal struct that holds the handles to globally unique resources of a VM.
+#[derive(Debug)]
+pub struct VmContext {
+ #[allow(dead_code)] // Keeps the global context alive
+ global_context: Strong<dyn IGlobalVmContext>,
+ #[allow(dead_code)] // Keeps the server alive
+ vm_server: RpcServer,
+}
+
+impl VmContext {
+ /// Construct new VmContext.
+ pub fn new(global_context: Strong<dyn IGlobalVmContext>, vm_server: RpcServer) -> VmContext {
+ VmContext { global_context, vm_server }
+ }
+}
+
/// Information about a particular instance of a VM which may be running.
#[derive(Debug)]
pub struct VmInstance {
/// The current state of the VM.
pub vm_state: Mutex<VmState>,
- /// Handle to global resources allocated for this VM.
- #[allow(dead_code)] // The handle is never read, we only need to hold it.
- vm_context: Strong<dyn IGlobalVmContext>,
+ /// Global resources allocated for this VM.
+ #[allow(dead_code)] // Keeps the context alive
+ vm_context: VmContext,
/// The CID assigned to the VM for vsock communication.
pub cid: Cid,
/// The name of the VM.
@@ -238,7 +255,7 @@
temporary_directory: PathBuf,
requester_uid: u32,
requester_debug_pid: i32,
- vm_context: Strong<dyn IGlobalVmContext>,
+ vm_context: VmContext,
) -> Result<VmInstance, Error> {
validate_config(&config)?;
let cid = config.cid;
diff --git a/vm_payload/Android.bp b/vm_payload/Android.bp
index dd2a937..6be6f22 100644
--- a/vm_payload/Android.bp
+++ b/vm_payload/Android.bp
@@ -17,6 +17,7 @@
"liblibc",
"liblog_rust",
"librpcbinder_rs",
+ "libvsock",
],
apex_available: [
"com.android.compos",