Do not use size filler
instead always fill zeros when there's gap between composite disk
components.
Bug: 192991318
Test: MicrodroidHostTestCases
Change-Id: I89234e047b6d5a48fee773dac0e7971ab465242c
diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index 44d89e7..12c46ee 100644
--- a/virtualizationservice/src/aidl.rs
+++ b/virtualizationservice/src/aidl.rs
@@ -35,13 +35,13 @@
use android_system_virtualizationservice::binder::{
self, BinderFeatures, ExceptionCode, Interface, ParcelFileDescriptor, Status, Strong, ThreadState,
};
-use anyhow::{bail, Result};
+use anyhow::{bail, Context, Result};
use disk::QcowFile;
use log::{debug, error, warn, info};
use microdroid_payload_config::{ApexConfig, VmPayloadConfig};
use std::convert::TryInto;
use std::ffi::CString;
-use std::fs::{File, create_dir};
+use std::fs::{File, OpenOptions, create_dir};
use std::num::NonZeroU32;
use std::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd};
use std::path::{Path, PathBuf};
@@ -67,6 +67,10 @@
/// payload output
const PORT_VIRT_SERVICE: u32 = 3000;
+/// The size of zero.img.
+/// Gaps in composite disk images are filled with a shared zero.img.
+const ZERO_FILLER_SIZE: u64 = 4096;
+
/// Implementation of `IVirtualizationService`, the entry point of the AIDL service.
#[derive(Debug, Default)]
pub struct VirtualizationService {
@@ -128,6 +132,16 @@
};
let config = config.as_ref();
+ let zero_filler_path = temporary_directory.join("zero.img");
+ let zero_filler_file = write_zero_filler(&zero_filler_path).map_err(|e| {
+ error!("Failed to make composite image: {}", e);
+ new_binder_exception(
+ ExceptionCode::SERVICE_SPECIFIC,
+ format!("Failed to make composite image: {}", e),
+ )
+ })?;
+ indirect_files.push(zero_filler_file);
+
// Assemble disk images if needed.
let disks = config
.disks
@@ -135,6 +149,7 @@
.map(|disk| {
assemble_disk_image(
disk,
+ &zero_filler_path,
&temporary_directory,
&mut next_temporary_image_id,
&mut indirect_files,
@@ -281,11 +296,23 @@
Ok(())
}
+fn write_zero_filler(zero_filler_path: &Path) -> Result<File> {
+ let file = OpenOptions::new()
+ .create_new(true)
+ .read(true)
+ .write(true)
+ .open(zero_filler_path)
+ .with_context(|| "Failed to create zero.img")?;
+ file.set_len(ZERO_FILLER_SIZE)?;
+ Ok(file)
+}
+
/// Given the configuration for a disk image, assembles the `DiskFile` to pass to crosvm.
///
/// This may involve assembling a composite disk from a set of partition images.
fn assemble_disk_image(
disk: &DiskImage,
+ zero_filler_path: &Path,
temporary_directory: &Path,
next_temporary_image_id: &mut u64,
indirect_files: &mut Vec<File>,
@@ -303,6 +330,7 @@
make_composite_image_filenames(temporary_directory, next_temporary_image_id);
let (image, partition_files) = make_composite_image(
&disk.partitions,
+ zero_filler_path,
&composite_image_filenames.composite,
&composite_image_filenames.header,
&composite_image_filenames.footer,
diff --git a/virtualizationservice/src/composite.rs b/virtualizationservice/src/composite.rs
index ed277e6..378ed78 100644
--- a/virtualizationservice/src/composite.rs
+++ b/virtualizationservice/src/composite.rs
@@ -73,7 +73,7 @@
}
/// Round `val` to partition size(4K)
-pub fn align_to_partition_size(val: u64) -> u64 {
+fn align_to_partition_size(val: u64) -> u64 {
align_to_power_of_2(val, PARTITION_SIZE_SHIFT)
}
@@ -168,7 +168,7 @@
fn create_component_disks(
partition: &PartitionInfo,
offset: u64,
- header_path: &str,
+ zero_filler_path: &str,
) -> Result<Vec<ComponentDisk>, Error> {
let aligned_size = partition.aligned_size();
@@ -208,7 +208,7 @@
);
component_disks.push(ComponentDisk {
offset: offset + file_size_sum,
- file_path: header_path.to_owned(),
+ file_path: zero_filler_path.to_owned(),
read_write_capability: ReadWriteCapability::READ_ONLY,
..ComponentDisk::new()
});
@@ -222,19 +222,22 @@
/// files.
pub fn create_composite_disk(
partitions: &[PartitionInfo],
+ zero_filler_path: &Path,
header_path: &Path,
header_file: &mut File,
footer_path: &Path,
footer_file: &mut File,
output_composite: &mut File,
) -> Result<(), Error> {
+ let zero_filler_path =
+ zero_filler_path.to_str().context("Invalid zero filler path")?.to_string();
let header_path = header_path.to_str().context("Invalid header path")?.to_string();
let footer_path = footer_path.to_str().context("Invalid footer path")?.to_string();
let mut composite_proto = CompositeDisk::new();
composite_proto.version = COMPOSITE_DISK_VERSION;
composite_proto.component_disks.push(ComponentDisk {
- file_path: header_path.clone(),
+ file_path: header_path,
offset: 0,
read_write_capability: ReadWriteCapability::READ_ONLY,
..ComponentDisk::new()
@@ -249,7 +252,9 @@
for partition in partitions {
create_gpt_entry(partition, next_disk_offset).write_bytes(&mut writer)?;
- for component_disk in create_component_disks(partition, next_disk_offset, &header_path)? {
+ for component_disk in
+ create_component_disks(partition, next_disk_offset, &zero_filler_path)?
+ {
composite_proto.component_disks.push(component_disk);
}
@@ -304,6 +309,7 @@
/// the form `/proc/self/fd/N` for the partition images.
pub fn make_composite_image(
partitions: &[Partition],
+ zero_filler_path: &Path,
output_path: &Path,
header_path: &Path,
footer_path: &Path,
@@ -327,6 +333,7 @@
create_composite_disk(
&partitions,
+ zero_filler_path,
header_path,
&mut header_file,
footer_path,
diff --git a/virtualizationservice/src/payload.rs b/virtualizationservice/src/payload.rs
index 1df537c..9c6deae 100644
--- a/virtualizationservice/src/payload.rs
+++ b/virtualizationservice/src/payload.rs
@@ -14,17 +14,13 @@
//! Payload disk image
-use crate::composite::align_to_partition_size;
-
use anyhow::{anyhow, Context, Result};
use microdroid_metadata::{ApexPayload, ApkPayload, Metadata};
use microdroid_payload_config::ApexConfig;
use once_cell::sync::OnceCell;
use serde::Deserialize;
use serde_xml_rs::from_reader;
-use std::fs;
use std::fs::{File, OpenOptions};
-use std::io::{Seek, SeekFrom, Write};
use std::path::{Path, PathBuf};
use vmconfig::{DiskImage, Partition};
@@ -69,46 +65,12 @@
}
}
-/// When passing a host APEX file as a block device in a payload disk image,
-/// the size of the original file needs to be stored in the last 4 bytes so that
-/// other programs (e.g. apexd) can read it as a zip.
-/// Returns true always since the filler is created.
-fn make_size_filler(size: u64, filler_path: &Path) -> Result<bool> {
- let partition_size = align_to_partition_size(size + 4);
- let mut file = OpenOptions::new().create_new(true).write(true).open(filler_path)?;
- file.set_len(partition_size - size)?;
- file.seek(SeekFrom::End(-4))?;
- file.write_all(&(size as i32).to_be_bytes())?;
- Ok(true)
-}
-
-/// When passing a host APK file as a block device in a payload disk image and it is
-/// mounted via dm-verity, we need to make the device zero-padded up to 4K boundary.
-/// Otherwise, integrity checks via hashtree will fail.
-/// Returns true if filler is created.
-fn make_zero_filler(size: u64, filler_path: &Path) -> Result<bool> {
- let partition_size = align_to_partition_size(size);
- if partition_size <= size {
- return Ok(false);
- }
- let file = OpenOptions::new().create_new(true).write(true).open(filler_path)?;
- file.set_len(partition_size - size)?;
- Ok(true)
-}
-
-/// When passing a host idsig file as a block device, we don't need any filler because it is read
-/// in length-prefixed way.
-/// Returns false always because the filler is not created.
-fn make_no_filler(_size: u64, _filler_path: &Path) -> Result<bool> {
- Ok(false)
-}
-
/// Creates a DiskImage with partitions:
/// metadata: metadata
-/// microdroid-apex-0: [apex 0, size filler]
-/// microdroid-apex-1: [apex 1, size filler]
+/// microdroid-apex-0: apex 0
+/// microdroid-apex-1: apex 1
/// ..
-/// microdroid-apk: [apk, zero filler]
+/// microdroid-apk: apk
/// microdroid-apk-idsig: idsig
pub fn make_payload_disk(
apk_file: PathBuf,
@@ -146,48 +108,23 @@
}];
let apex_info_list = ApexInfoList::load()?;
- let mut filler_count = 0;
for (i, apex) in apexes.iter().enumerate() {
- partitions.push(make_partition(
- format!("microdroid-apex-{}", i),
- apex_info_list.get_path_for(&apex.name)?,
- temporary_directory,
- &mut filler_count,
- &make_size_filler,
- )?);
+ partitions.push(Partition {
+ label: format!("microdroid-apex-{}", i),
+ paths: vec![apex_info_list.get_path_for(&apex.name)?],
+ writable: false,
+ });
}
- partitions.push(make_partition(
- "microdroid-apk".to_owned(),
- apk_file,
- temporary_directory,
- &mut filler_count,
- &make_zero_filler,
- )?);
- partitions.push(make_partition(
- "microdroid-apk-idsig".to_owned(),
- idsig_file,
- temporary_directory,
- &mut filler_count,
- &make_no_filler,
- )?);
+ partitions.push(Partition {
+ label: "microdroid-apk".to_owned(),
+ paths: vec![apk_file],
+ writable: false,
+ });
+ partitions.push(Partition {
+ label: "microdroid-apk-idsig".to_owned(),
+ paths: vec![idsig_file],
+ writable: false,
+ });
Ok(DiskImage { image: None, partitions, writable: false })
}
-
-fn make_partition(
- label: String,
- path: PathBuf,
- temporary_directory: &Path,
- filler_count: &mut u32,
- make_filler: &dyn Fn(u64, &Path) -> Result<bool>,
-) -> Result<Partition> {
- let filler_path = temporary_directory.join(format!("filler-{}", filler_count));
- let size = fs::metadata(&path)?.len();
-
- if make_filler(size, &filler_path)? {
- *filler_count += 1;
- Ok(Partition { label, paths: vec![path, filler_path], writable: false })
- } else {
- Ok(Partition { label, paths: vec![path], writable: false })
- }
-}