Remove onRamdump callback
Ramdumps are still written to tombstoned, but we remove the callback
to the client.
Remove the option from vm that allows the ramdump to be written elsewhere.
I could have left the callback from VS and just ignored it in our
javalib, but adding extra work to all clients just to support the
niche vm usecase felt wrong.
Also fix a type.
Bug: 262538986
Bug: 261037705
Test: atest MicrodroidTests
Change-Id: If60ac5033a8b077fc35b499e056461782b5027fe
diff --git a/demo/java/com/android/microdroid/demo/MainActivity.java b/demo/java/com/android/microdroid/demo/MainActivity.java
index 77f2ee7..54d7420 100644
--- a/demo/java/com/android/microdroid/demo/MainActivity.java
+++ b/demo/java/com/android/microdroid/demo/MainActivity.java
@@ -238,13 +238,6 @@
mService.shutdownNow();
mStatus.postValue(VirtualMachine.STATUS_STOPPED);
}
-
- @Override
- public void onRamdump(VirtualMachine vm, ParcelFileDescriptor ramdump) {
- if (!mService.isShutdown()) {
- mPayloadOutput.postValue("(Kernel panic. Ramdump created)");
- }
- }
};
try {
diff --git a/docs/debug/ramdump.md b/docs/debug/ramdump.md
index a0d9bf2..771c608 100644
--- a/docs/debug/ramdump.md
+++ b/docs/debug/ramdump.md
@@ -1,6 +1,6 @@
# Doing RAM dump of a Microdroid VM and analyzing it
-A Microdroid VM creates a RAM dump of itself when the kernel panics. This
+A debuggable Microdroid VM creates a RAM dump of itself when the kernel panics. This
document explains how the dump can be obtained and analyzed.
## Force triggering a RAM dump
@@ -49,7 +49,7 @@
## Obtaining the RAM dump
-By default, RAM dumps are sent to tombstone. To see which tombstone file is for
+RAM dumps are sent to tombstone. To see which tombstone file is for
the RAM dump, look into the log.
```shell
@@ -64,15 +64,6 @@
$ adb root && adb pull /data/tombstones/tombstone_47 ramdump && adb unroot
```
-Alternatively, you can specify the path to where RAM dump is stored when
-launching the VM using the `--ramdump` option of the `vm` tool.
-
-```shell
-$ adb shelll /apex/com.android.virt/bin/vm run-app --ramdump /data/local/tmp/virt/ramdump ...
-```
-
-In the above example, the RAM dump is saved to `/data/local/tmp/virt/ramdump`.
-
## Analyzing the RAM dump
### Building the crash(8) tool
@@ -151,9 +142,3 @@
actually triggered a crash in the kernel.
For more commands of crash(8), refer to the man page, or embedded `help` command.
-
-
-
-
-
-
diff --git a/javalib/api/system-current.txt b/javalib/api/system-current.txt
index 04283c2..821c47e 100644
--- a/javalib/api/system-current.txt
+++ b/javalib/api/system-current.txt
@@ -28,7 +28,6 @@
method public void onPayloadFinished(@NonNull android.system.virtualmachine.VirtualMachine, int);
method public void onPayloadReady(@NonNull android.system.virtualmachine.VirtualMachine);
method public void onPayloadStarted(@NonNull android.system.virtualmachine.VirtualMachine);
- method public void onRamdump(@NonNull android.system.virtualmachine.VirtualMachine, @NonNull android.os.ParcelFileDescriptor);
method public void onStopped(@NonNull android.system.virtualmachine.VirtualMachine, int);
field public static final int ERROR_PAYLOAD_CHANGED = 2; // 0x2
field public static final int ERROR_PAYLOAD_INVALID_CONFIG = 3; // 0x3
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachine.java b/javalib/src/android/system/virtualmachine/VirtualMachine.java
index c061003..57ae51e 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachine.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachine.java
@@ -796,11 +796,6 @@
VirtualMachine.this, translatedReason));
}
}
-
- @Override
- public void onRamdump(int cid, ParcelFileDescriptor ramdump) {
- executeCallback((cb) -> cb.onRamdump(VirtualMachine.this, ramdump));
- }
});
mContext.registerComponentCallbacks(mMemoryManagementCallbacks);
service.asBinder().linkToDeath(deathRecipient, 0);
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java b/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java
index fad2fa9..9aaecf0 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java
@@ -20,7 +20,6 @@
import android.annotation.NonNull;
import android.annotation.SuppressLint;
import android.annotation.SystemApi;
-import android.os.ParcelFileDescriptor;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
@@ -155,7 +154,4 @@
/** Called when the VM has stopped. */
void onStopped(@NonNull VirtualMachine vm, @StopReason int reason);
-
- /** Called when kernel panic occurs and as a result ramdump is generated from the VM. */
- void onRamdump(@NonNull VirtualMachine vm, @NonNull ParcelFileDescriptor ramdump);
}
diff --git a/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java b/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java
index 72a0090..9aed34d 100644
--- a/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java
+++ b/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java
@@ -248,9 +248,6 @@
vm.clearCallback();
mExecutorService.shutdown();
}
-
- @Override
- public void onRamdump(VirtualMachine vm, ParcelFileDescriptor ramdump) {}
}
public static class BootResult {
diff --git a/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachineCallback.aidl b/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachineCallback.aidl
index a329fa6..34b6fa5 100644
--- a/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachineCallback.aidl
+++ b/virtualizationservice/aidl/android/system/virtualizationservice/IVirtualMachineCallback.aidl
@@ -50,9 +50,4 @@
* also use `link_to_death` to handle that.
*/
void onDied(int cid, in DeathReason reason);
-
- /**
- * Called when kernel panic occurs and as a result ramdump is generated from the VM.
- */
- void onRamdump(int cid, in ParcelFileDescriptor ramdump);
}
diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index 072afec..f98aeaa 100644
--- a/virtualizationservice/src/aidl.rs
+++ b/virtualizationservice/src/aidl.rs
@@ -1049,17 +1049,6 @@
}
}
- /// Call all registered callbacks to say that there was a ramdump to download.
- pub fn callback_on_ramdump(&self, cid: Cid, ramdump: File) {
- let callbacks = &*self.0.lock().unwrap();
- let pfd = ParcelFileDescriptor::new(ramdump);
- for callback in callbacks {
- if let Err(e) = callback.onRamdump(cid as i32, &pfd) {
- error!("Error notifying ramdump of VM CID {}: {:?}", cid, e);
- }
- }
- }
-
/// Add a new callback to the set.
fn add(&self, callback: Strong<dyn IVirtualMachineCallback>) {
self.0.lock().unwrap().push(callback);
diff --git a/virtualizationservice/src/crosvm.rs b/virtualizationservice/src/crosvm.rs
index fc85ca5..5125f19 100644
--- a/virtualizationservice/src/crosvm.rs
+++ b/virtualizationservice/src/crosvm.rs
@@ -520,15 +520,10 @@
Ok(())
}
- /// Checks if ramdump has been created. If so, send a notification to the user with the handle
- /// to read the ramdump.
+ /// Checks if ramdump has been created. If so, send it to tombstoned.
fn handle_ramdump(&self) -> Result<(), Error> {
let ramdump_path = self.temporary_directory.join("ramdump");
if std::fs::metadata(&ramdump_path)?.len() > 0 {
- let ramdump = File::open(&ramdump_path)
- .context(format!("Failed to open ramdump {:?} for reading", &ramdump_path))?;
- self.callbacks.callback_on_ramdump(self.cid, ramdump);
-
Self::send_ramdump_to_tombstoned(&ramdump_path)?;
}
Ok(())
@@ -536,7 +531,7 @@
fn send_ramdump_to_tombstoned(ramdump_path: &Path) -> Result<(), Error> {
let mut input = File::open(ramdump_path)
- .context(format!("Failed to open raudmp {:?} for reading", ramdump_path))?;
+ .context(format!("Failed to open ramdump {:?} for reading", ramdump_path))?;
let pid = std::process::id() as i32;
let conn = TombstonedConnection::connect(pid, DebuggerdDumpType::Tombstone)
diff --git a/vm/src/main.rs b/vm/src/main.rs
index 32b165b..3d2fc00 100644
--- a/vm/src/main.rs
+++ b/vm/src/main.rs
@@ -81,10 +81,6 @@
#[clap(long)]
log: Option<PathBuf>,
- /// Path to file where ramdump is recorded on kernel panic
- #[clap(long)]
- ramdump: Option<PathBuf>,
-
/// Debug level of the VM. Supported values: "none" (default), and "full".
#[clap(long, default_value = "none", value_parser = parse_debug_level)]
debug: DebugLevel,
@@ -144,10 +140,6 @@
#[clap(long)]
log: Option<PathBuf>,
- /// Path to file where ramdump is recorded on kernel panic
- #[clap(long)]
- ramdump: Option<PathBuf>,
-
/// Debug level of the VM. Supported values: "none" (default), and "full".
#[clap(long, default_value = "full", value_parser = parse_debug_level)]
debug: DebugLevel,
@@ -268,7 +260,6 @@
daemonize,
console,
log,
- ramdump,
debug,
protected,
mem,
@@ -288,7 +279,6 @@
daemonize,
console.as_deref(),
log.as_deref(),
- ramdump.as_deref(),
debug,
protected,
mem,
@@ -304,7 +294,6 @@
daemonize,
console,
log,
- ramdump,
debug,
protected,
mem,
@@ -319,7 +308,6 @@
daemonize,
console.as_deref(),
log.as_deref(),
- ramdump.as_deref(),
debug,
protected,
mem,
diff --git a/vm/src/run.rs b/vm/src/run.rs
index 3f25bba..6096913 100644
--- a/vm/src/run.rs
+++ b/vm/src/run.rs
@@ -52,7 +52,6 @@
daemonize: bool,
console_path: Option<&Path>,
log_path: Option<&Path>,
- ramdump_path: Option<&Path>,
debug_level: DebugLevel,
protected: bool,
mem: Option<u32>,
@@ -144,7 +143,7 @@
numCpus: cpus.unwrap_or(1) as i32,
taskProfiles: task_profiles,
});
- run(service, &config, &payload_config_str, daemonize, console_path, log_path, ramdump_path)
+ run(service, &config, &payload_config_str, daemonize, console_path, log_path)
}
const EMPTY_PAYLOAD_APK: &str = "com.android.microdroid.empty_payload";
@@ -182,7 +181,6 @@
daemonize: bool,
console_path: Option<&Path>,
log_path: Option<&Path>,
- ramdump_path: Option<&Path>,
debug_level: DebugLevel,
protected: bool,
mem: Option<u32>,
@@ -214,7 +212,6 @@
daemonize,
console_path,
log_path,
- ramdump_path,
debug_level,
protected,
mem,
@@ -259,7 +256,6 @@
daemonize,
console_path,
log_path,
- /* ramdump_path */ None,
)
}
@@ -282,7 +278,6 @@
daemonize: bool,
console_path: Option<&Path>,
log_path: Option<&Path>,
- ramdump_path: Option<&Path>,
) -> Result<(), Error> {
let console = if let Some(console_path) = console_path {
Some(
@@ -325,27 +320,12 @@
// Wait until the VM or VirtualizationService dies. If we just returned immediately then the
// IVirtualMachine Binder object would be dropped and the VM would be killed.
let death_reason = vm.wait_for_death();
-
- if let Some(path) = ramdump_path {
- save_ramdump_if_available(path, &vm)?;
- }
println!("VM ended: {:?}", death_reason);
}
Ok(())
}
-fn save_ramdump_if_available(path: &Path, vm: &VmInstance) -> Result<(), Error> {
- if let Some(mut ramdump) = vm.get_ramdump() {
- let mut file =
- File::create(path).context(format!("Failed to create ramdump file {:?}", path))?;
- let size = std::io::copy(&mut ramdump, &mut file)
- .context(format!("Failed to save ramdump to file {:?}", path))?;
- eprintln!("Ramdump ({} bytes) saved to {:?}", size, path);
- }
- Ok(())
-}
-
fn parse_extra_apk_list(apk: &Path, config_path: &str) -> Result<Vec<String>, Error> {
let mut archive = ZipArchive::new(File::open(apk)?)?;
let config_file = archive.by_name(config_path)?;
diff --git a/vmclient/src/lib.rs b/vmclient/src/lib.rs
index 20b7f02..7c05545 100644
--- a/vmclient/src/lib.rs
+++ b/vmclient/src/lib.rs
@@ -190,11 +190,6 @@
}
})
}
-
- /// Get ramdump
- pub fn get_ramdump(&self) -> Option<File> {
- self.state.get_ramdump()
- }
}
impl Debug for VmInstance {
@@ -222,7 +217,6 @@
struct VmState {
death_reason: Option<DeathReason>,
reported_state: VirtualMachineState,
- ramdump: Option<File>,
}
impl Monitor<VmState> {
@@ -239,14 +233,6 @@
self.state.lock().unwrap().reported_state = state;
self.cv.notify_all();
}
-
- fn set_ramdump(&self, ramdump: File) {
- self.state.lock().unwrap().ramdump = Some(ramdump);
- }
-
- fn get_ramdump(&self) -> Option<File> {
- self.state.lock().unwrap().ramdump.as_ref().and_then(|f| f.try_clone().ok())
- }
}
struct VirtualMachineCallback {
@@ -302,12 +288,6 @@
Ok(())
}
- fn onRamdump(&self, _cid: i32, ramdump: &ParcelFileDescriptor) -> BinderResult<()> {
- let ramdump: File = ramdump.as_ref().try_clone().unwrap();
- self.state.set_ramdump(ramdump);
- Ok(())
- }
-
fn onDied(&self, cid: i32, reason: AidlDeathReason) -> BinderResult<()> {
let reason = reason.into();
self.state.notify_death(reason);