Pass VM config by FD rather than filename.
Bug: 184131756
Test: atest VirtualizationTestCases
Change-Id: Iba739bc8de941b68a88090438743d9f54ba0380c
diff --git a/tests/vsock_test.cc b/tests/vsock_test.cc
index 57a03ca..0df1c5d 100644
--- a/tests/vsock_test.cc
+++ b/tests/vsock_test.cc
@@ -27,10 +27,10 @@
#include "android-base/logging.h"
#include "android-base/parseint.h"
#include "android-base/unique_fd.h"
-
#include "virt/VirtualizationTest.h"
using namespace android::base;
+using namespace android::os;
namespace virt {
@@ -58,7 +58,9 @@
ASSERT_EQ(ret, 0) << strerror(errno);
sp<IVirtualMachine> vm;
- status = mVirtManager->startVm(String16(kVmConfigPath), std::nullopt, &vm);
+ unique_fd vm_config_fd(open(kVmConfigPath, O_RDONLY | O_CLOEXEC));
+ status =
+ mVirtManager->startVm(ParcelFileDescriptor(std::move(vm_config_fd)), std::nullopt, &vm);
ASSERT_TRUE(status.isOk()) << "Error starting VM: " << status;
int32_t cid;
diff --git a/virtmanager/aidl/android/system/virtmanager/IVirtManager.aidl b/virtmanager/aidl/android/system/virtmanager/IVirtManager.aidl
index ab03c18..c2fc719 100644
--- a/virtmanager/aidl/android/system/virtmanager/IVirtManager.aidl
+++ b/virtmanager/aidl/android/system/virtmanager/IVirtManager.aidl
@@ -23,7 +23,8 @@
* Start the VM with the given config file, and return a handle to it. If `logFd` is provided
* then console logs from the VM will be sent to it.
*/
- IVirtualMachine startVm(String configPath, in @nullable ParcelFileDescriptor logFd);
+ IVirtualMachine startVm(
+ in ParcelFileDescriptor configFd, in @nullable ParcelFileDescriptor logFd);
/**
* Get a list of all currently running VMs. This method is only intended for debug purposes,
diff --git a/virtmanager/aidl/android/system/virtmanager/VirtualMachineDebugInfo.aidl b/virtmanager/aidl/android/system/virtmanager/VirtualMachineDebugInfo.aidl
index d877a56..9003bc3 100644
--- a/virtmanager/aidl/android/system/virtmanager/VirtualMachineDebugInfo.aidl
+++ b/virtmanager/aidl/android/system/virtmanager/VirtualMachineDebugInfo.aidl
@@ -19,10 +19,4 @@
parcelable VirtualMachineDebugInfo {
/** The CID assigned to the VM. */
int cid;
-
- /**
- * The filename of the config file used to start the VM. This may have changed since it was
- * read so it shouldn't be trusted; it is only stored for debugging purposes.
- */
- String configPath;
}
diff --git a/virtmanager/src/aidl.rs b/virtmanager/src/aidl.rs
index 8c963d2..ff8ab30 100644
--- a/virtmanager/src/aidl.rs
+++ b/virtmanager/src/aidl.rs
@@ -50,7 +50,7 @@
/// Returns a binder `IVirtualMachine` object referring to it, as a handle for the client.
fn startVm(
&self,
- config_path: &str,
+ config_fd: &ParcelFileDescriptor,
log_fd: Option<&ParcelFileDescriptor>,
) -> binder::Result<Strong<dyn IVirtualMachine>> {
let state = &mut *self.state.lock().unwrap();
@@ -58,7 +58,7 @@
let log_fd = log_fd
.map(|fd| fd.as_ref().try_clone().map_err(|_| StatusCode::UNKNOWN_ERROR))
.transpose()?;
- let instance = Arc::new(start_vm(config_path, cid, log_fd)?);
+ let instance = Arc::new(start_vm(config_fd.as_ref(), cid, log_fd)?);
// TODO(qwandor): keep track of which CIDs are currently in use so that we can reuse them.
state.next_cid = state.next_cid.checked_add(1).ok_or(StatusCode::UNKNOWN_ERROR)?;
state.add_vm(Arc::downgrade(&instance));
@@ -74,13 +74,8 @@
let state = &mut *self.state.lock().unwrap();
let vms = state.vms();
- let cids = vms
- .into_iter()
- .map(|vm| VirtualMachineDebugInfo {
- cid: vm.cid as i32,
- configPath: vm.config_path.clone(),
- })
- .collect();
+ let cids =
+ vms.into_iter().map(|vm| VirtualMachineDebugInfo { cid: vm.cid as i32 }).collect();
Ok(cids)
}
@@ -193,13 +188,13 @@
/// Start a new VM instance from the given VM config filename. This assumes the VM is not already
/// running.
-fn start_vm(config_path: &str, cid: Cid, log_fd: Option<File>) -> binder::Result<VmInstance> {
- let config = VmConfig::load(config_path).map_err(|e| {
- error!("Failed to load VM config {}: {:?}", config_path, e);
+fn start_vm(config_file: &File, cid: Cid, log_fd: Option<File>) -> binder::Result<VmInstance> {
+ let config = VmConfig::load(config_file).map_err(|e| {
+ error!("Failed to load VM config from {:?}: {:?}", config_file, e);
StatusCode::BAD_VALUE
})?;
- Ok(VmInstance::start(&config, cid, config_path, log_fd).map_err(|e| {
- error!("Failed to start VM {}: {:?}", config_path, e);
+ Ok(VmInstance::start(&config, cid, log_fd).map_err(|e| {
+ error!("Failed to start VM from {:?}: {:?}", config_file, e);
StatusCode::UNKNOWN_ERROR
})?)
}
diff --git a/virtmanager/src/config.rs b/virtmanager/src/config.rs
index d8cb06f..0b12c0b 100644
--- a/virtmanager/src/config.rs
+++ b/virtmanager/src/config.rs
@@ -14,7 +14,7 @@
//! Function and types for VM configuration.
-use anyhow::{bail, Context, Error};
+use anyhow::{bail, Error};
use serde::{Deserialize, Serialize};
use std::fs::File;
use std::io::BufReader;
@@ -51,8 +51,7 @@
}
/// Load the configuration for a VM from the given JSON file.
- pub fn load(path: &str) -> Result<VmConfig, Error> {
- let file = File::open(path).with_context(|| format!("Failed to open {}", path))?;
+ pub fn load(file: &File) -> Result<VmConfig, Error> {
let buffered = BufReader::new(file);
Ok(serde_json::from_reader(buffered)?)
}
diff --git a/virtmanager/src/crosvm.rs b/virtmanager/src/crosvm.rs
index 814a1a7..c865357 100644
--- a/virtmanager/src/crosvm.rs
+++ b/virtmanager/src/crosvm.rs
@@ -30,27 +30,19 @@
child: Child,
/// The CID assigned to the VM for vsock communication.
pub cid: Cid,
- /// The filename of the config file that was used to start the VM. This may have changed since
- /// it was read so it shouldn't be trusted; it is only stored for debugging purposes.
- pub config_path: String,
}
impl VmInstance {
/// Create a new `VmInstance` for the given process.
- fn new(child: Child, cid: Cid, config_path: &str) -> VmInstance {
- VmInstance { child, cid, config_path: config_path.to_owned() }
+ fn new(child: Child, cid: Cid) -> VmInstance {
+ VmInstance { child, cid }
}
/// Start an instance of `crosvm` to manage a new VM. The `crosvm` instance will be killed when
/// the `VmInstance` is dropped.
- pub fn start(
- config: &VmConfig,
- cid: Cid,
- config_path: &str,
- log_fd: Option<File>,
- ) -> Result<VmInstance, Error> {
+ pub fn start(config: &VmConfig, cid: Cid, log_fd: Option<File>) -> Result<VmInstance, Error> {
let child = run_vm(config, cid, log_fd)?;
- Ok(VmInstance::new(child, cid, config_path))
+ Ok(VmInstance::new(child, cid))
}
}
diff --git a/vm/src/main.rs b/vm/src/main.rs
index 8c2a084..1f39e4a 100644
--- a/vm/src/main.rs
+++ b/vm/src/main.rs
@@ -77,9 +77,12 @@
daemonize: bool,
) -> Result<(), Error> {
let config_filename = config_path.to_str().context("Failed to parse VM config path")?;
+ let config_file = ParcelFileDescriptor::new(
+ File::open(config_filename).context("Failed to open config file")?,
+ );
let stdout_file = ParcelFileDescriptor::new(duplicate_stdout()?);
let stdout = if daemonize { None } else { Some(&stdout_file) };
- let vm = virt_manager.startVm(config_filename, stdout).context("Failed to start VM")?;
+ let vm = virt_manager.startVm(&config_file, stdout).context("Failed to start VM")?;
let cid = vm.getCid().context("Failed to get CID")?;
println!("Started VM from {} with CID {}.", config_filename, cid);