Pass file descriptors rather than filenames to Virt Manager.
This requires using a parcelable rather than a JSON file for the config.
Bug: 187181124
Test: atest VirtualizationTestCases
Change-Id: I1636010f5ed55da165f5acd82f1bd8b924e09b41
diff --git a/virtmanager/Android.bp b/virtmanager/Android.bp
index f1971dc..0e746b3 100644
--- a/virtmanager/Android.bp
+++ b/virtmanager/Android.bp
@@ -11,11 +11,10 @@
rustlibs: [
"android.system.virtmanager-rust",
"libandroid_logger",
- "liblog_rust",
- "libserde_json",
- "libserde",
- "libshared_child",
"libanyhow",
+ "libcommand_fds",
+ "liblog_rust",
+ "libshared_child",
],
apex_available: ["com.android.virt"],
}
diff --git a/virtmanager/aidl/android/system/virtmanager/DiskImage.aidl b/virtmanager/aidl/android/system/virtmanager/DiskImage.aidl
new file mode 100644
index 0000000..9abb350
--- /dev/null
+++ b/virtmanager/aidl/android/system/virtmanager/DiskImage.aidl
@@ -0,0 +1,25 @@
+/*
+ * Copyright 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package android.system.virtmanager;
+
+/** A disk image to be made available to the VM. */
+parcelable DiskImage {
+ /** The disk image. */
+ ParcelFileDescriptor image;
+
+ /** Whether this disk should be writable by the VM. */
+ boolean writable;
+}
diff --git a/virtmanager/aidl/android/system/virtmanager/IVirtManager.aidl b/virtmanager/aidl/android/system/virtmanager/IVirtManager.aidl
index c2fc719..650b847 100644
--- a/virtmanager/aidl/android/system/virtmanager/IVirtManager.aidl
+++ b/virtmanager/aidl/android/system/virtmanager/IVirtManager.aidl
@@ -16,6 +16,7 @@
package android.system.virtmanager;
import android.system.virtmanager.IVirtualMachine;
+import android.system.virtmanager.VirtualMachineConfig;
import android.system.virtmanager.VirtualMachineDebugInfo;
interface IVirtManager {
@@ -24,7 +25,7 @@
* then console logs from the VM will be sent to it.
*/
IVirtualMachine startVm(
- in ParcelFileDescriptor configFd, in @nullable ParcelFileDescriptor logFd);
+ in VirtualMachineConfig config, 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/VirtualMachineConfig.aidl b/virtmanager/aidl/android/system/virtmanager/VirtualMachineConfig.aidl
new file mode 100644
index 0000000..39be0f3
--- /dev/null
+++ b/virtmanager/aidl/android/system/virtmanager/VirtualMachineConfig.aidl
@@ -0,0 +1,42 @@
+/*
+ * Copyright 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package android.system.virtmanager;
+
+import android.system.virtmanager.DiskImage;
+
+/** Configuration for running a VM. */
+parcelable VirtualMachineConfig {
+ /** The kernel image, if any. */
+ @nullable ParcelFileDescriptor kernel;
+
+ /** The initial ramdisk for the kernel, if any. */
+ @nullable ParcelFileDescriptor initrd;
+
+ /**
+ * Parameters to pass to the kernel. As far as the VMM and boot protocol are concerned this is
+ * just a string, but typically it will contain multiple parameters separated by spaces.
+ */
+ @nullable String params;
+
+ /**
+ * The bootloader to use. If this is supplied then the kernel and initrd must not be supplied;
+ * the bootloader is instead responsibly for loading the kernel from one of the disks.
+ */
+ @nullable ParcelFileDescriptor bootloader;
+
+ /** Disk images to be made available to the VM. */
+ DiskImage[] disks;
+}
diff --git a/virtmanager/src/aidl.rs b/virtmanager/src/aidl.rs
index 2f96f9d..d8fb1ed 100644
--- a/virtmanager/src/aidl.rs
+++ b/virtmanager/src/aidl.rs
@@ -14,7 +14,6 @@
//! Implementation of the AIDL interface of the Virt Manager.
-use crate::config::VmConfig;
use crate::crosvm::VmInstance;
use crate::{Cid, FIRST_GUEST_CID};
use android_system_virtmanager::aidl::android::system::virtmanager::IVirtManager::IVirtManager;
@@ -22,12 +21,12 @@
BnVirtualMachine, IVirtualMachine,
};
use android_system_virtmanager::aidl::android::system::virtmanager::IVirtualMachineCallback::IVirtualMachineCallback;
+use android_system_virtmanager::aidl::android::system::virtmanager::VirtualMachineConfig::VirtualMachineConfig;
use android_system_virtmanager::aidl::android::system::virtmanager::VirtualMachineDebugInfo::VirtualMachineDebugInfo;
use android_system_virtmanager::binder::{
self, BinderFeatures, Interface, ParcelFileDescriptor, StatusCode, Strong, ThreadState,
};
use log::{debug, error};
-use std::fs::File;
use std::sync::{Arc, Mutex, Weak};
pub const BINDER_SERVICE_IDENTIFIER: &str = "android.system.virtmanager";
@@ -50,7 +49,7 @@
/// Returns a binder `IVirtualMachine` object referring to it, as a handle for the client.
fn startVm(
&self,
- config_fd: &ParcelFileDescriptor,
+ config: &VirtualMachineConfig,
log_fd: Option<&ParcelFileDescriptor>,
) -> binder::Result<Strong<dyn IVirtualMachine>> {
let state = &mut *self.state.lock().unwrap();
@@ -74,14 +73,18 @@
})?;
let requester_debug_pid = ThreadState::get_calling_pid();
let cid = state.allocate_cid()?;
- let instance = start_vm(
- config_fd.as_ref(),
+ let instance = VmInstance::start(
+ config,
cid,
log_fd,
requester_uid,
requester_sid,
requester_debug_pid,
- )?;
+ )
+ .map_err(|e| {
+ error!("Failed to start VM with config {:?}: {:?}", config, e);
+ StatusCode::UNKNOWN_ERROR
+ })?;
state.add_vm(Arc::downgrade(&instance));
Ok(VirtualMachine::create(instance))
}
@@ -261,24 +264,3 @@
State { next_cid: FIRST_GUEST_CID, vms: vec![], debug_held_vms: vec![] }
}
}
-
-/// Start a new VM instance from the given VM config file. This assumes the VM is not already
-/// running.
-fn start_vm(
- config_file: &File,
- cid: Cid,
- log_fd: Option<File>,
- requester_uid: u32,
- requester_sid: String,
- requester_debug_pid: i32,
-) -> binder::Result<Arc<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, log_fd, requester_uid, requester_sid, requester_debug_pid)
- .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
deleted file mode 100644
index 0b12c0b..0000000
--- a/virtmanager/src/config.rs
+++ /dev/null
@@ -1,67 +0,0 @@
-// Copyright 2021, The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-//! Function and types for VM configuration.
-
-use anyhow::{bail, Error};
-use serde::{Deserialize, Serialize};
-use std::fs::File;
-use std::io::BufReader;
-
-/// Configuration for a particular VM to be started.
-#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
-pub struct VmConfig {
- /// The filename of the kernel image, if any.
- pub kernel: Option<String>,
- /// The filename of the initial ramdisk for the kernel, if any.
- pub initrd: Option<String>,
- /// Parameters to pass to the kernel. As far as the VMM and boot protocol are concerned this is
- /// just a string, but typically it will contain multiple parameters separated by spaces.
- pub params: Option<String>,
- /// The bootloader to use. If this is supplied then the kernel and initrd must not be supplied;
- /// the bootloader is instead responsibly for loading the kernel from one of the disks.
- pub bootloader: Option<String>,
- /// Disk images to be made available to the VM.
- #[serde(default)]
- pub disks: Vec<DiskImage>,
-}
-
-impl VmConfig {
- /// Ensure that the configuration has a valid combination of fields set, or return an error if
- /// not.
- pub fn validate(&self) -> Result<(), Error> {
- if self.bootloader.is_none() && self.kernel.is_none() {
- bail!("VM must have either a bootloader or a kernel image.");
- }
- if self.bootloader.is_some() && (self.kernel.is_some() || self.initrd.is_some()) {
- bail!("Can't have both bootloader and kernel/initrd image.");
- }
- Ok(())
- }
-
- /// Load the configuration for a VM from the given JSON file.
- pub fn load(file: &File) -> Result<VmConfig, Error> {
- let buffered = BufReader::new(file);
- Ok(serde_json::from_reader(buffered)?)
- }
-}
-
-/// A disk image to be made available to the VM.
-#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
-pub struct DiskImage {
- /// The filename of the disk image.
- pub image: String,
- /// Whether this disk should be writable by the VM.
- pub writable: bool,
-}
diff --git a/virtmanager/src/crosvm.rs b/virtmanager/src/crosvm.rs
index 60e063c..1b11fd6 100644
--- a/virtmanager/src/crosvm.rs
+++ b/virtmanager/src/crosvm.rs
@@ -15,12 +15,14 @@
//! Functions for running instances of `crosvm`.
use crate::aidl::VirtualMachineCallbacks;
-use crate::config::VmConfig;
use crate::Cid;
-use anyhow::Error;
-use log::{error, info};
+use android_system_virtmanager::aidl::android::system::virtmanager::VirtualMachineConfig::VirtualMachineConfig;
+use anyhow::{bail, Context, Error};
+use command_fds::{CommandFdExt, FdMapping};
+use log::{debug, error, info};
use shared_child::SharedChild;
use std::fs::File;
+use std::os::unix::io::AsRawFd;
use std::process::Command;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::Arc;
@@ -71,7 +73,7 @@
/// 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,
+ config: &VirtualMachineConfig,
cid: Cid,
log_fd: Option<File>,
requester_uid: u32,
@@ -121,33 +123,74 @@
}
/// Start an instance of `crosvm` to manage a new VM.
-fn run_vm(config: &VmConfig, cid: Cid, log_fd: Option<File>) -> Result<SharedChild, Error> {
- config.validate()?;
+fn run_vm(
+ config: &VirtualMachineConfig,
+ cid: Cid,
+ log_fd: Option<File>,
+) -> Result<SharedChild, Error> {
+ validate_config(config)?;
let mut command = Command::new(CROSVM_PATH);
// TODO(qwandor): Remove --disable-sandbox.
command.arg("run").arg("--disable-sandbox").arg("--cid").arg(cid.to_string());
+
if let Some(log_fd) = log_fd {
command.stdout(log_fd);
} else {
// Ignore console output.
command.arg("--serial=type=sink");
}
+
+ // Keep track of what file descriptors should be mapped to the crosvm process.
+ let mut fd_mappings = vec![];
+
if let Some(bootloader) = &config.bootloader {
- command.arg("--bios").arg(bootloader);
+ command.arg("--bios").arg(add_fd_mapping(&mut fd_mappings, bootloader.as_ref()));
}
+
if let Some(initrd) = &config.initrd {
- command.arg("--initrd").arg(initrd);
+ command.arg("--initrd").arg(add_fd_mapping(&mut fd_mappings, initrd.as_ref()));
}
+
if let Some(params) = &config.params {
command.arg("--params").arg(params);
}
+
for disk in &config.disks {
- command.arg(if disk.writable { "--rwdisk" } else { "--disk" }).arg(&disk.image);
+ command.arg(if disk.writable { "--rwdisk" } else { "--disk" }).arg(add_fd_mapping(
+ &mut fd_mappings,
+ // TODO(b/187187765): This shouldn't be an Option.
+ disk.image.as_ref().context("Invalid disk image file descriptor")?.as_ref(),
+ ));
}
+
if let Some(kernel) = &config.kernel {
- command.arg(kernel);
+ command.arg(add_fd_mapping(&mut fd_mappings, kernel.as_ref()));
}
+
+ debug!("Setting mappings {:?}", fd_mappings);
+ command.fd_mappings(fd_mappings)?;
+
info!("Running {:?}", command);
- Ok(SharedChild::spawn(&mut command)?)
+ let result = SharedChild::spawn(&mut command)?;
+ Ok(result)
+}
+
+/// Ensure that the configuration has a valid combination of fields set, or return an error if not.
+fn validate_config(config: &VirtualMachineConfig) -> Result<(), Error> {
+ if config.bootloader.is_none() && config.kernel.is_none() {
+ bail!("VM must have either a bootloader or a kernel image.");
+ }
+ if config.bootloader.is_some() && (config.kernel.is_some() || config.initrd.is_some()) {
+ bail!("Can't have both bootloader and kernel/initrd image.");
+ }
+ Ok(())
+}
+
+/// Adds a mapping for `file` to `fd_mappings`, and returns a string of the form "/proc/self/fd/N"
+/// where N is the file descriptor for the child process.
+fn add_fd_mapping(fd_mappings: &mut Vec<FdMapping>, file: &File) -> String {
+ let fd = file.as_raw_fd();
+ fd_mappings.push(FdMapping { parent_fd: fd, child_fd: fd });
+ format!("/proc/self/fd/{}", fd)
}
diff --git a/virtmanager/src/main.rs b/virtmanager/src/main.rs
index 4c98c41..762e2f8 100644
--- a/virtmanager/src/main.rs
+++ b/virtmanager/src/main.rs
@@ -15,7 +15,6 @@
//! Android Virt Manager
mod aidl;
-mod config;
mod crosvm;
use crate::aidl::{VirtManager, BINDER_SERVICE_IDENTIFIER};