virtmgr: drop support for qcow2 and composite as partition images
We'll now throw an error if qcow2 or composite disks are passed as
partition images. The code for detecting the image type and calculating
the partition size has been forked from crosvm and significantly
simplified.
We still depend on crosvm's library for generating composite disk files
and GUID Partition Tables.
=== Why? ===
The real motivation for this change is that the `disk` crate APIs have
changed in crosvm and it doesn't seem possible to use the new API
correctly from virtmgr. The main issue is that the `disk` crate now
wants to open the file itself from a path (so that it can ensure
consistent file locking and use of features like O_DIRECT and overlapped
IO). virtmgr only has an FD for the partition images, so it would need
to give the crosvm library a "/proc/self/fd/..." path. However, crosvm
code only allows those paths if they've come from a parent process and
it attempts to validate that by checking for the absense of the CLOEXEC
flag. We can't safely clear the CLOEXEC flag on a file in virtmgr
without introducing a race condition with exec calls on other threads.
You might think "the real problem is the crosvm API change, it should be
reverted", but these are crosvm internal APIs and we aren't supposed to
be using them to begin with. I'm the author the API changes and I can't
think of a good alternative that doesn't meaningfully make the crosvm
code worse and possibly reintroduce some of the bugs that are being
fixed.
=== Are we losing something useful? ===
I think these two image formats aren't actually useful as partition
images in the current AVF design.
If a VM is requested with a DiskImage containing a list of partition
images, virtmgr generates a composite disk image from those partitions.
The individual partition images are passed as FDs, at least in part
because virtmgr and crosvm processes are unlikely to have permissions to
open them from a filesystem path.
When one of the partition images themselves is a composite image things
get tricky. The crosvm process would need to open the files specified
within that image using their paths (which it doesn't have permissions
to do, as mentioned above). One might try working around that using a
"/proc/self/fd/..." path, however, there is no way to pass those FDs the
virtmgr process and virtmgr doesn't have any logic to pass them along to
the crosvm process.
QCOW2 has similar issues for its "backing file" feature. Given the
subset of QCOW2 features supported by crosvm right now, I believe the
only good reason to use it is for the "backing file" functionality.
Note that these two formats will continue to work (to the limited degree
they did before) when passed directly as the root of a DiskImage.
=== Is there a better long term solution? ===
The virtmgr code for creating a composite file looks like it has been
forked from the `crosvm create_composite` CLI command. One idea is to
de-fork by having virtmgr invoke that command. At least one hurdle in
that direction is that we'll probably want that invocation of crosvm to
use a different SELinux domain so that we don't have to give the main
VMM instance permissions to create files and such.
Another idea is to push the complexity onto the caller. Make them pass
the partition size explicitly instead of inferring it from the image
file.
Bug: 364645148, 330911976, 269356487
Test: launch microdroid
Change-Id: Ibb686b51b646c52993c3c02f5738bcb2356f7432
diff --git a/android/virtmgr/Android.bp b/android/virtmgr/Android.bp
index f0b6881..0148ff6 100644
--- a/android/virtmgr/Android.bp
+++ b/android/virtmgr/Android.bp
@@ -70,6 +70,7 @@
"liblibfdt",
"libfsfdt",
"libhypervisor_props",
+ "libzerocopy",
"libuuid",
// TODO(b/202115393) stabilize the interface
"packagemanager_aidl-rust",
diff --git a/android/virtmgr/src/composite.rs b/android/virtmgr/src/composite.rs
index 681ec59..1219150 100644
--- a/android/virtmgr/src/composite.rs
+++ b/android/virtmgr/src/composite.rs
@@ -15,13 +15,16 @@
//! Functions for creating a composite disk image.
use android_system_virtualizationservice::aidl::android::system::virtualizationservice::Partition::Partition;
-use anyhow::{anyhow, Context, Error};
-use disk::{
- create_composite_disk, create_disk_file, ImagePartitionType, PartitionInfo, MAX_NESTING_DEPTH,
-};
+use anyhow::{bail, Context, Error};
+use disk::{create_composite_disk, ImagePartitionType, PartitionInfo};
use std::fs::{File, OpenOptions};
+use std::io::ErrorKind;
+use std::os::unix::fs::FileExt;
use std::os::unix::io::AsRawFd;
use std::path::{Path, PathBuf};
+use zerocopy::AsBytes;
+use zerocopy::FromBytes;
+use zerocopy::FromZeroes;
use uuid::Uuid;
@@ -98,7 +101,7 @@
.context("Failed to clone partition image file descriptor")?
.into();
let path = fd_path_for_file(&file);
- let size = get_partition_size(&file, &path)?;
+ let size = get_partition_size(&file)?;
files.push(file);
Ok(PartitionInfo {
@@ -122,16 +125,74 @@
/// Find the size of the partition image in the given file by parsing the header.
///
-/// This will work for raw, QCOW2, composite and Android sparse images.
-fn get_partition_size(partition: &File, path: &Path) -> Result<u64, Error> {
- // TODO: Use `context` once disk::Error implements std::error::Error.
- // TODO: Add check for is_sparse_file
- Ok(create_disk_file(
- partition.try_clone()?,
- /* is_sparse_file */ false,
- MAX_NESTING_DEPTH,
- path,
- )
- .map_err(|e| anyhow!("Failed to open partition image: {}", e))?
- .get_len()?)
+/// This will work for raw and Android sparse images. QCOW2 and composite images aren't supported.
+fn get_partition_size(file: &File) -> Result<u64, Error> {
+ match detect_image_type(file).context("failed to detect partition image type")? {
+ ImageType::Raw => Ok(file.metadata().context("failed to get metadata")?.len()),
+ ImageType::AndroidSparse => {
+ // Source: system/core/libsparse/sparse_format.h
+ #[repr(C)]
+ #[derive(Clone, Copy, Debug, AsBytes, FromZeroes, FromBytes)]
+ struct SparseHeader {
+ magic: u32,
+ major_version: u16,
+ minor_version: u16,
+ file_hdr_sz: u16,
+ chunk_hdr_size: u16,
+ blk_sz: u32,
+ total_blks: u32,
+ total_chunks: u32,
+ image_checksum: u32,
+ }
+ let mut header = SparseHeader::new_zeroed();
+ file.read_exact_at(header.as_bytes_mut(), 0)
+ .context("failed to read android sparse header")?;
+ let len = u64::from(header.total_blks)
+ .checked_mul(header.blk_sz.into())
+ .context("android sparse image len too big")?;
+ Ok(len)
+ }
+ t => bail!("unsupported partition image type: {t:?}"),
+ }
+}
+
+/// Image file types we can detect.
+#[derive(Debug, PartialEq, Eq)]
+enum ImageType {
+ Raw,
+ Qcow2,
+ CompositeDisk,
+ AndroidSparse,
+}
+
+/// Detect image type by looking for magic bytes.
+fn detect_image_type(file: &File) -> std::io::Result<ImageType> {
+ const CDISK_MAGIC: &str = "composite_disk\x1d";
+ const QCOW_MAGIC: u32 = 0x5146_49fb;
+ const SPARSE_HEADER_MAGIC: u32 = 0xed26ff3a;
+
+ let mut magic4 = [0u8; 4];
+ match file.read_exact_at(&mut magic4[..], 0) {
+ Ok(()) => {}
+ Err(e) if e.kind() == ErrorKind::UnexpectedEof => return Ok(ImageType::Raw),
+ Err(e) => return Err(e),
+ }
+ if magic4 == QCOW_MAGIC.to_be_bytes() {
+ return Ok(ImageType::Qcow2);
+ }
+ if magic4 == SPARSE_HEADER_MAGIC.to_le_bytes() {
+ return Ok(ImageType::AndroidSparse);
+ }
+
+ let mut buf = [0u8; CDISK_MAGIC.len()];
+ match file.read_exact_at(buf.as_bytes_mut(), 0) {
+ Ok(()) => {}
+ Err(e) if e.kind() == ErrorKind::UnexpectedEof => return Ok(ImageType::Raw),
+ Err(e) => return Err(e),
+ }
+ if buf == CDISK_MAGIC.as_bytes() {
+ return Ok(ImageType::CompositeDisk);
+ }
+
+ Ok(ImageType::Raw)
}