Merge "Update AVF backcompat tests to ignore bootargs/kernel-size" into main
diff --git a/android/vm/src/run.rs b/android/vm/src/run.rs
index 0855fa0..0037327 100644
--- a/android/vm/src/run.rs
+++ b/android/vm/src/run.rs
@@ -339,10 +339,11 @@
} else {
None
};
- let vm = VmInstance::create(service, config, console_out, console_in, log, dump_dt)
- .context("Failed to create VM")?;
let callback = Box::new(Callback {});
- vm.start(Some(callback)).context("Failed to start VM")?;
+ let vm =
+ VmInstance::create(service, config, console_out, console_in, log, dump_dt, Some(callback))
+ .context("Failed to create VM")?;
+ vm.start().context("Failed to start VM")?;
let debug_level = get_debug_level(config).unwrap_or(DebugLevel::NONE);
diff --git a/guest/rialto/tests/test.rs b/guest/rialto/tests/test.rs
index c650046..d68c568 100644
--- a/guest/rialto/tests/test.rs
+++ b/guest/rialto/tests/test.rs
@@ -338,6 +338,7 @@
/* consoleIn */ None,
log,
/* dump_dt */ None,
+ None,
)
.context("Failed to create VM")
}
diff --git a/guest/trusty/security_vm/launcher/src/main.rs b/guest/trusty/security_vm/launcher/src/main.rs
index 2fedc16..8dd7c43 100644
--- a/guest/trusty/security_vm/launcher/src/main.rs
+++ b/guest/trusty/security_vm/launcher/src/main.rs
@@ -101,9 +101,10 @@
None, // console_out
None, // log
None, // dump_dt
+ None, // callback
)
.context("Failed to create VM")?;
- vm.start(None /* callback */).context("Failed to start VM")?;
+ vm.start().context("Failed to start VM")?;
println!("started trusty_security_vm_launcher VM");
let death_reason = vm.wait_for_death();
diff --git a/libs/libavf/include/android/virtualization.h b/libs/libavf/include/android/virtualization.h
index 03d04a9..ef57325 100644
--- a/libs/libavf/include/android/virtualization.h
+++ b/libs/libavf/include/android/virtualization.h
@@ -337,30 +337,17 @@
int AVirtualMachine_createRaw(const AVirtualizationService* _Nonnull service,
AVirtualMachineRawConfig* _Nonnull config, int consoleOutFd,
int consoleInFd, int logFd,
- const AVirtualMachine* _Null_unspecified* _Nonnull vm)
- __INTRODUCED_IN(36);
-
-/**
- * A callback to be called when virtual machine stops.
- *
- * \param vm stopped vm
- * \param reason stop reason
- */
-typedef void (*AVirtualMachine_stopCallback)(const AVirtualMachine* _Nonnull vm,
- enum AVirtualMachineStopReason reason);
+ AVirtualMachine* _Null_unspecified* _Nonnull vm) __INTRODUCED_IN(36);
/**
* Start a virtual machine. `AVirtualMachine_start` is synchronous and blocks until the virtual
* machine is initialized and free to start executing code, or until an error happens.
*
* \param vm a handle on a virtual machine.
- * \param callback an optional callback to be called when VM stops.
*
* \return If successful, it returns 0. Otherwise, it returns `-EIO`.
*/
-int AVirtualMachine_start(const AVirtualMachine* _Nonnull vm,
- const AVirtualMachine_stopCallback _Nullable callback)
- __INTRODUCED_IN(36);
+int AVirtualMachine_start(AVirtualMachine* _Nonnull vm) __INTRODUCED_IN(36);
/**
* Stop a virtual machine. Stopping a virtual machine is like pulling the plug on a real computer;
@@ -379,7 +366,7 @@
*
* \return If successful, it returns 0. Otherwise, it returns `-EIO`.
*/
-int AVirtualMachine_stop(const AVirtualMachine* _Nonnull vm) __INTRODUCED_IN(36);
+int AVirtualMachine_stop(AVirtualMachine* _Nonnull vm) __INTRODUCED_IN(36);
/**
* Open a vsock connection to the VM on the given port. The caller takes ownership of the returned
@@ -392,8 +379,7 @@
*
* \return If successful, it returns a valid file descriptor. Otherwise, it returns `-EIO`.
*/
-int AVirtualMachine_connectVsock(const AVirtualMachine* _Nonnull vm, uint32_t port)
- __INTRODUCED_IN(36);
+int AVirtualMachine_connectVsock(AVirtualMachine* _Nonnull vm, uint32_t port) __INTRODUCED_IN(36);
/**
* Wait until a virtual machine stops or the given timeout elapses.
@@ -407,7 +393,7 @@
* sets `reason` and returns true.
* - If the timeout expired, it returns `false`.
*/
-bool AVirtualMachine_waitForStop(const AVirtualMachine* _Nonnull vm,
+bool AVirtualMachine_waitForStop(AVirtualMachine* _Nonnull vm,
const struct timespec* _Nullable timeout,
enum AVirtualMachineStopReason* _Nonnull reason)
__INTRODUCED_IN(36);
@@ -422,6 +408,6 @@
*
* \param vm a handle on a virtual machine.
*/
-void AVirtualMachine_destroy(const AVirtualMachine* _Nullable vm) __INTRODUCED_IN(36);
+void AVirtualMachine_destroy(AVirtualMachine* _Nullable vm) __INTRODUCED_IN(36);
__END_DECLS
diff --git a/libs/libavf/src/lib.rs b/libs/libavf/src/lib.rs
index 256803f..6532ace 100644
--- a/libs/libavf/src/lib.rs
+++ b/libs/libavf/src/lib.rs
@@ -19,7 +19,6 @@
use std::os::fd::{FromRawFd, IntoRawFd};
use std::os::raw::{c_char, c_int};
use std::ptr;
-use std::sync::Arc;
use std::time::Duration;
use android_system_virtualizationservice::{
@@ -30,10 +29,10 @@
},
binder::{ParcelFileDescriptor, Strong},
};
-use avf_bindgen::{AVirtualMachineStopReason, AVirtualMachine_stopCallback};
+use avf_bindgen::AVirtualMachineStopReason;
use libc::timespec;
use log::error;
-use vmclient::{DeathReason, ErrorCode, VirtualizationService, VmCallback, VmInstance};
+use vmclient::{DeathReason, VirtualizationService, VmInstance};
/// Create a new virtual machine config object with no properties.
#[no_mangle]
@@ -343,49 +342,6 @@
}
}
-struct LocalVmInstance {
- vm: Arc<VmInstance>,
- callback: AVirtualMachine_stopCallback,
-}
-
-impl VmCallback for LocalVmInstance {
- fn on_payload_started(&self, _cid: i32) {
- // Microdroid only. no-op.
- }
-
- fn on_payload_ready(&self, _cid: i32) {
- // Microdroid only. no-op.
- }
-
- fn on_payload_finished(&self, _cid: i32, _exit_code: i32) {
- // Microdroid only. no-op.
- }
-
- fn on_error(&self, _cid: i32, _error_code: ErrorCode, _message: &str) {
- // Microdroid only. no-op.
- }
-
- fn on_died(&self, _cid: i32, death_reason: DeathReason) {
- let Some(callback) = self.callback else {
- return;
- };
- let stop_reason = death_reason_to_stop_reason(death_reason);
- let vm_ptr: *const VmInstance = Arc::into_raw(Arc::clone(&self.vm));
-
- // SAFETY: `callback` is assumed to be a valid, non-null function pointer passed by
- // `AVirtualMachine_start`.
- unsafe {
- callback(vm_ptr.cast(), stop_reason);
- }
-
- // drop ptr after use.
- // SAFETY: `vm_ptr` is a valid, non-null pointer casted above.
- unsafe {
- let _ = Arc::from_raw(vm_ptr);
- }
- }
-}
-
/// Create a virtual machine with given `config`.
///
/// # Safety
@@ -401,7 +357,7 @@
console_out_fd: c_int,
console_in_fd: c_int,
log_fd: c_int,
- vm_ptr: *mut *const VmInstance,
+ vm_ptr: *mut *mut VmInstance,
) -> c_int {
// SAFETY: `service` is assumed to be a valid, non-null pointer returned by
// `AVirtualizationService_create` or `AVirtualizationService_create_early`. It's the only
@@ -417,11 +373,12 @@
let console_in = get_file_from_fd(console_in_fd);
let log = get_file_from_fd(log_fd);
- match VmInstance::create(service.as_ref(), &config, console_out, console_in, log, None) {
+ match VmInstance::create(service.as_ref(), &config, console_out, console_in, log, None, None) {
Ok(vm) => {
// SAFETY: `vm_ptr` is assumed to be a valid, non-null pointer to a mutable raw pointer.
+ // `vm` is the only reference here and `vm_ptr` takes ownership.
unsafe {
- *vm_ptr = Arc::into_raw(Arc::new(vm));
+ *vm_ptr = Box::into_raw(Box::new(vm));
}
0
}
@@ -437,20 +394,11 @@
/// # Safety
/// `vm` must be a pointer returned by `AVirtualMachine_createRaw`.
#[no_mangle]
-pub unsafe extern "C" fn AVirtualMachine_start(
- vm: *const VmInstance,
- callback: AVirtualMachine_stopCallback,
-) -> c_int {
+pub unsafe extern "C" fn AVirtualMachine_start(vm: *const VmInstance) -> c_int {
// SAFETY: `vm` is assumed to be a valid, non-null pointer returned by
// `AVirtualMachine_createRaw`. It's the only reference to the object.
- let vm = unsafe { Arc::from_raw(vm) };
- let callback = callback.map(|_| {
- let cb: Box<dyn VmCallback + Send + Sync> =
- Box::new(LocalVmInstance { vm: Arc::clone(&vm), callback });
- cb
- });
-
- match vm.start(callback) {
+ let vm = unsafe { &*vm };
+ match vm.start() {
Ok(_) => 0,
Err(e) => {
error!("AVirtualMachine_start failed: {e:?}");
@@ -561,12 +509,12 @@
/// `vm` must be a pointer returned by `AVirtualMachine_createRaw`. `vm` must not be reused after
/// deletion.
#[no_mangle]
-pub unsafe extern "C" fn AVirtualMachine_destroy(vm: *const VmInstance) {
+pub unsafe extern "C" fn AVirtualMachine_destroy(vm: *mut VmInstance) {
if !vm.is_null() {
// SAFETY: `vm` is assumed to be a valid, non-null pointer returned by
// AVirtualMachine_create. It's the only reference to the object.
unsafe {
- let _ = Arc::from_raw(vm);
+ let _ = Box::from_raw(vm);
}
}
}
diff --git a/libs/libcompos_common/compos_client.rs b/libs/libcompos_common/compos_client.rs
index c2b4936..6872582 100644
--- a/libs/libcompos_common/compos_client.rs
+++ b/libs/libcompos_common/compos_client.rs
@@ -148,14 +148,19 @@
// Let logs go to logcat.
let (console_fd, log_fd) = (None, None);
+ let callback = Box::new(Callback {});
let instance = VmInstance::create(
- service, &config, console_fd, /* console_in_fd */ None, log_fd,
+ service,
+ &config,
+ console_fd,
+ /* console_in_fd */ None,
+ log_fd,
/* dump_dt */ None,
+ Some(callback),
)
.context("Failed to create VM")?;
- let callback = Box::new(Callback {});
- instance.start(Some(callback))?;
+ instance.start()?;
let ready = instance.wait_until_ready(TIMEOUTS.vm_max_time_to_ready);
if ready == Err(VmWaitError::Finished) && debug_level != DebugLevel::NONE {
diff --git a/libs/libservice_vm_manager/src/lib.rs b/libs/libservice_vm_manager/src/lib.rs
index 77e7a4a..0f322bb 100644
--- a/libs/libservice_vm_manager/src/lib.rs
+++ b/libs/libservice_vm_manager/src/lib.rs
@@ -152,7 +152,7 @@
let vsock_listener = VsockListener::bind_with_cid_port(VMADDR_CID_HOST, vm_type.port())?;
// Starts the service VM.
- vm.start(None).context("Failed to start service VM")?;
+ vm.start().context("Failed to start service VM")?;
info!("Service VM started");
// Accepts the connection from the service VM.
@@ -245,7 +245,8 @@
let console_in = None;
let log = Some(android_log_fd()?);
let dump_dt = None;
- VmInstance::create(service.as_ref(), &config, console_out, console_in, log, dump_dt)
+ let callback = None;
+ VmInstance::create(service.as_ref(), &config, console_out, console_in, log, dump_dt, callback)
.context("Failed to create service VM")
}
diff --git a/libs/libvmclient/src/lib.rs b/libs/libvmclient/src/lib.rs
index 2c6abb5..8dd3cd3 100644
--- a/libs/libvmclient/src/lib.rs
+++ b/libs/libvmclient/src/lib.rs
@@ -209,6 +209,7 @@
console_in: Option<File>,
log: Option<File>,
dump_dt: Option<File>,
+ callback: Option<Box<dyn VmCallback + Send + Sync>>,
) -> BinderResult<Self> {
let console_out = console_out.map(ParcelFileDescriptor::new);
let console_in = console_in.map(ParcelFileDescriptor::new);
@@ -225,19 +226,20 @@
let cid = vm.getCid()?;
+ // Register callback before starting VM, in case it dies immediately.
let state = Arc::new(Monitor::new(VmState::default()));
+ let callback = BnVirtualMachineCallback::new_binder(
+ VirtualMachineCallback { state: state.clone(), client_callback: callback },
+ BinderFeatures::default(),
+ );
+ vm.registerCallback(&callback)?;
let death_recipient = wait_for_binder_death(&mut vm.as_binder(), state.clone())?;
Ok(Self { vm, cid, state, _death_recipient: death_recipient })
}
/// Starts the VM.
- pub fn start(&self, callback: Option<Box<dyn VmCallback + Send + Sync>>) -> BinderResult<()> {
- let callback = BnVirtualMachineCallback::new_binder(
- VirtualMachineCallback { state: self.state.clone(), client_callback: callback },
- BinderFeatures::default(),
- );
- self.vm.registerCallback(&callback)?;
+ pub fn start(&self) -> BinderResult<()> {
self.vm.start()
}
diff --git a/microfuchsia/microfuchsiad/src/instance_starter.rs b/microfuchsia/microfuchsiad/src/instance_starter.rs
index 7c4f32d..e3c4e8d 100644
--- a/microfuchsia/microfuchsiad/src/instance_starter.rs
+++ b/microfuchsia/microfuchsiad/src/instance_starter.rs
@@ -96,6 +96,7 @@
console_in,
/* log= */ None,
/* dump_dt= */ None,
+ None,
)
.context("Failed to create VM")?;
if let Some(pty) = &pty {
@@ -104,7 +105,7 @@
.setHostConsoleName(&pty.follower_name)
.context("Setting host console name")?;
}
- vm_instance.start(None).context("Starting VM")?;
+ vm_instance.start().context("Starting VM")?;
Ok(MicrofuchsiaInstance {
_vm_instance: vm_instance,
diff --git a/tests/backcompat_test/src/main.rs b/tests/backcompat_test/src/main.rs
index 2bd1bec..bf0afa6 100644
--- a/tests/backcompat_test/src/main.rs
+++ b/tests/backcompat_test/src/main.rs
@@ -117,9 +117,10 @@
/* consoleIn */ None,
None,
Some(dump_dt),
+ None,
)
.context("Failed to create VM")?;
- vm.start(None).context("Failed to start VM")?;
+ vm.start().context("Failed to start VM")?;
info!("Started example VM.");
// Wait for VM to finish
diff --git a/tests/early_vm_test/src/main.rs b/tests/early_vm_test/src/main.rs
index 7d630f8..a3c80ca 100644
--- a/tests/early_vm_test/src/main.rs
+++ b/tests/early_vm_test/src/main.rs
@@ -96,6 +96,7 @@
None, // console_out
None, // log
None, // dump_dt
+ None, // callback
)
.context("Failed to create VM")?;
diff --git a/tests/old_images_avf_test/src/main.rs b/tests/old_images_avf_test/src/main.rs
index d3ab5eb..018a80e 100644
--- a/tests/old_images_avf_test/src/main.rs
+++ b/tests/old_images_avf_test/src/main.rs
@@ -92,7 +92,7 @@
AVirtualMachineRawConfig_setMemoryMiB(config, VM_MEMORY_MB);
}
- let mut vm = std::ptr::null();
+ let mut vm = std::ptr::null_mut();
let mut service = std::ptr::null_mut();
ensure!(
@@ -132,7 +132,7 @@
// SAFETY: vm is the only reference to a valid object
unsafe {
- AVirtualMachine_start(vm, None /* callback */);
+ AVirtualMachine_start(vm);
}
info!("VM started");
diff --git a/tests/vm_accessor/accessor/src/run.rs b/tests/vm_accessor/accessor/src/run.rs
index 5bdb8f1..6dcc507 100644
--- a/tests/vm_accessor/accessor/src/run.rs
+++ b/tests/vm_accessor/accessor/src/run.rs
@@ -129,9 +129,10 @@
None, /* console_in */
Some(android_log_fd()?), /* log */
None, /* dump_dt */
+ Some(Box::new(Callback {})),
)
.context("Failed to create VM")?;
- vm.start(Some(Box::new(Callback {}))).context("Failed to start VM")?;
+ vm.start().context("Failed to start VM")?;
info!("started IAccessor VM with CID {}", vm.cid());
diff --git a/tests/vmbase_example/src/main.rs b/tests/vmbase_example/src/main.rs
index d427164..cbe90d8 100644
--- a/tests/vmbase_example/src/main.rs
+++ b/tests/vmbase_example/src/main.rs
@@ -115,9 +115,10 @@
/* consoleIn */ None,
Some(log_writer),
/* dump_dt */ None,
+ None,
)
.context("Failed to create VM")?;
- vm.start(None).context("Failed to start VM")?;
+ vm.start().context("Failed to start VM")?;
info!("Started example VM.");
// Wait for VM to finish, and check that it shut down cleanly.
diff --git a/tests/vts/src/vts_libavf_test.rs b/tests/vts/src/vts_libavf_test.rs
index c59271a..dc37aad 100644
--- a/tests/vts/src/vts_libavf_test.rs
+++ b/tests/vts/src/vts_libavf_test.rs
@@ -19,8 +19,6 @@
use std::fs::File;
use std::io::{self, BufWriter, Write};
use std::os::fd::IntoRawFd;
-use std::sync::mpsc::{self, Sender};
-use std::sync::{LazyLock, Mutex};
use std::time::{Duration, Instant};
use vsock::{VsockListener, VsockStream, VMADDR_CID_HOST};
@@ -35,13 +33,6 @@
const WRITE_TIMEOUT: Duration = Duration::from_secs(10);
const STOP_TIMEOUT: timespec = timespec { tv_sec: 10, tv_nsec: 0 };
-static ON_STOPPED_EVENT: LazyLock<Mutex<Sender<(usize, AVirtualMachineStopReason)>>> =
- LazyLock::new(|| {
- // Returning stub here because `Receiver` isn't `Send`.
- let (tx, _) = mpsc::channel();
- Mutex::new(tx)
- });
-
/// Processes the request in the service VM.
fn process_request(vsock_stream: &mut VsockStream, request: Request) -> Result<Response> {
write_request(vsock_stream, &ServiceVmRequest::Process(request))?;
@@ -82,23 +73,11 @@
}
}
-unsafe extern "C" fn on_stopped(vm: *const AVirtualMachine, reason: AVirtualMachineStopReason) {
- ON_STOPPED_EVENT.lock().unwrap().send((vm as usize, reason)).unwrap();
-
- // SAFETY: `vm` is a valid pointer created by AVirtualMachine_create().
- unsafe {
- AVirtualMachine_destroy(vm);
- }
-}
-
fn run_rialto(protected_vm: bool) -> Result<()> {
let kernel_file =
File::open("/data/local/tmp/rialto.bin").context("Failed to open kernel file")?;
let kernel_fd = kernel_file.into_raw_fd();
- let (tx, rx) = mpsc::channel();
- (*ON_STOPPED_EVENT.lock().unwrap()) = tx;
-
// SAFETY: AVirtualMachineRawConfig_create() isn't unsafe but rust_bindgen forces it to be seen
// as unsafe
let config = unsafe { AVirtualMachineRawConfig_create() };
@@ -113,7 +92,7 @@
AVirtualMachineRawConfig_setMemoryMiB(config, VM_MEMORY_MB);
}
- let mut vm = std::ptr::null();
+ let mut vm = std::ptr::null_mut();
let mut service = std::ptr::null_mut();
ensure!(
@@ -122,6 +101,11 @@
"AVirtualizationService_create failed"
);
+ scopeguard::defer! {
+ // SAFETY: service is a valid pointer to AVirtualizationService
+ unsafe { AVirtualizationService_destroy(service); }
+ }
+
ensure!(
// SAFETY: &mut vm is a valid pointer to *AVirtualMachine
unsafe {
@@ -148,11 +132,9 @@
// SAFETY: vm is the only reference to a valid object
unsafe {
- AVirtualMachine_start(vm, Some(on_stopped));
+ AVirtualMachine_start(vm);
}
- let vm_ptr = vm as usize;
-
info!("VM started");
let mut vsock_stream = listener_thread.join().unwrap()?;
@@ -184,14 +166,6 @@
"AVirtualMachine_waitForStop failed"
);
- assert_eq!(AVirtualMachineStopReason::AVIRTUAL_MACHINE_SHUTDOWN, stop_reason);
-
- let timeout = Duration::from_secs(STOP_TIMEOUT.tv_sec.try_into().unwrap());
- let (stopped_vm_ptr, stopped_reason) =
- rx.recv_timeout(timeout).expect("Callback should have been called");
- assert_eq!(stopped_vm_ptr, vm_ptr);
- assert_eq!(stopped_reason, stop_reason);
-
info!("stopped");
Ok(())