virtualizationservice: clean up

This is a follow up of 21e9b928ef7683ecbfc6fff17a5b588c791d385a and
adfb76c3cf021ae645341e94f8e85ac46d6791ec.

* fix typo
* use more rusty style
* add more comments

Bug: n/a
Test: MicrodroidHostTestCases
Change-Id: I1e763defa062eb61e243438542893f0046165366
diff --git a/compos/apk/Android.bp b/compos/apk/Android.bp
index c5d0c31..95788bb 100644
--- a/compos/apk/Android.bp
+++ b/compos/apk/Android.bp
@@ -8,7 +8,7 @@
     apex_available: ["com.android.compos"],
 }
 
-// TODO(b/190409306) this is temporal until we have a solid way to pass merkle tree
+// TODO(b/190409306) this is temporary until we have a solid way to pass merkle tree
 java_genrule {
     name: "CompOSPayloadApp.signing",
     out: [
diff --git a/microdroid/README.md b/microdroid/README.md
index f48a35c..d51c8d0 100644
--- a/microdroid/README.md
+++ b/microdroid/README.md
@@ -73,14 +73,22 @@
   "task": {
     "type": "microdroid_launcher",
     "command": "MyMicrodroidApp.so"
-  },
-  "apexes" : [ ... ]
+  }
 }
 ```
 
 The value of `task.command` should match with the name of the shared library
-defined above. The `apexes` array is the APEXes that will be imported to
-microdroid.
+defined above. If your app rquires APEXes to be imported, you can declare the list
+in `apexes` key like following.
+```json
+{
+  "os": ...,
+  "task": ...,
+  "apexes": [
+    {"name": "com.android.awesome_apex"}
+  ]
+}
+```
 
 Embed the shared library and the VM configuration file in an APK:
 
@@ -150,7 +158,6 @@
 TEST_ROOT=/data/local/tmp/virt
 adb root
 adb shell setenforce 0
-adb shell start virtualizationservice
 adb shell /apex/com.android.virt/bin/vm run-app --daemonize --log $TEST_ROOT/log.txt PATH_TO_YOUR_APP $TEST_ROOT/MyApp.apk.idsig assets/VM_CONFIG_FILE
 ```
 
diff --git a/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineConfig.aidl b/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineConfig.aidl
index 00a7937..4e8b440 100644
--- a/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineConfig.aidl
+++ b/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineConfig.aidl
@@ -20,7 +20,7 @@
 
 /** Configuration for running a VM */
 union VirtualMachineConfig {
-    /** Configuration for a VM to run an APP */
+    /** Configuration for a VM to run an app */
     VirtualMachineAppConfig appConfig;
 
     /** Configuration for a VM with low-level configuration */
diff --git a/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineRawConfig.aidl b/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineRawConfig.aidl
index 7848ed5..9e1dee2 100644
--- a/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineRawConfig.aidl
+++ b/virtualizationservice/aidl/android/system/virtualizationservice/VirtualMachineRawConfig.aidl
@@ -17,7 +17,7 @@
 
 import android.system.virtualizationservice.DiskImage;
 
-/** Raw Configuration for running a VM. */
+/** Raw configuration for running a VM. */
 parcelable VirtualMachineRawConfig {
     /** The kernel image, if any. */
     @nullable ParcelFileDescriptor kernel;
diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index a0b5217..b841ad0 100644
--- a/virtualizationservice/src/aidl.rs
+++ b/virtualizationservice/src/aidl.rs
@@ -34,7 +34,7 @@
 use android_system_virtualizationservice::binder::{
     self, BinderFeatures, ExceptionCode, Interface, ParcelFileDescriptor, Status, Strong, ThreadState,
 };
-use anyhow::Result;
+use anyhow::{bail, Result};
 use disk::QcowFile;
 use log::{debug, error, warn};
 use microdroid_payload_config::{ApexConfig, VmPayloadConfig};
@@ -45,6 +45,7 @@
 use std::path::{Path, PathBuf};
 use std::sync::{Arc, Mutex, Weak};
 use vmconfig::VmConfig;
+use zip::ZipArchive;
 
 pub const BINDER_SERVICE_IDENTIFIER: &str = "android.system.virtualizationservice";
 
@@ -55,6 +56,11 @@
 /// Only processes running with one of these UIDs are allowed to call debug methods.
 const DEBUG_ALLOWED_UIDS: [u32; 2] = [0, 2000];
 
+/// The list of APEXes which microdroid requires.
+/// TODO(b/192200378) move this to microdroid.json?
+const MICRODROID_REQUIRED_APEXES: [&str; 4] =
+    ["com.android.adbd", "com.android.i18n", "com.android.os.statsd", "com.android.sdkext"];
+
 /// Implementation of `IVirtualizationService`, the entry point of the AIDL service.
 #[derive(Debug, Default)]
 pub struct VirtualizationService {
@@ -101,21 +107,19 @@
             )
         })?;
 
-        let mut opt_raw_config = None;
         let config = match config {
-            VirtualMachineConfig::AppConfig(config) => {
-                let raw_config = load_app_config(config, &temporary_directory).map_err(|e| {
+            VirtualMachineConfig::AppConfig(config) => BorrowedOrOwned::Owned(
+                load_app_config(config, &temporary_directory).map_err(|e| {
                     error!("Failed to load app config from {}: {}", &config.configPath, e);
                     new_binder_exception(
                         ExceptionCode::SERVICE_SPECIFIC,
                         format!("Failed to load app config from {}: {}", &config.configPath, e),
                     )
-                })?;
-                opt_raw_config.replace(raw_config);
-                opt_raw_config.as_ref().unwrap()
-            }
-            VirtualMachineConfig::RawConfig(config) => config,
+                })?,
+            ),
+            VirtualMachineConfig::RawConfig(config) => BorrowedOrOwned::Borrowed(config),
         };
+        let config = config.as_ref();
 
         // Assemble disk images if needed.
         let disks = config
@@ -289,23 +293,24 @@
     let idsig_file = config.idsig.as_ref().unwrap().as_ref();
     let config_path = &config.configPath;
 
-    let mut apk_zip = zip::ZipArchive::new(apk_file)?;
+    let mut apk_zip = ZipArchive::new(apk_file)?;
     let config_file = apk_zip.by_name(config_path)?;
     let vm_payload_config: VmPayloadConfig = serde_json::from_reader(config_file)?;
 
     let os_name = &vm_payload_config.os.name;
+    // For now, the only supported "os" value is "microdroid"
+    if os_name != "microdroid" {
+        bail!("unknown os: {}", os_name);
+    }
     let vm_config_path = PathBuf::from(format!("/apex/com.android.virt/etc/{}.json", os_name));
     let vm_config_file = File::open(vm_config_path)?;
     let mut vm_config = VmConfig::load(&vm_config_file)?;
 
     // Microdroid requires additional payload disk image
     if os_name == "microdroid" {
-        // TODO (b/192200378) move this to microdroid.json?
         let mut apexes = vm_payload_config.apexes.clone();
         apexes.extend(
-            ["com.android.adbd", "com.android.i18n", "com.android.os.statsd", "com.android.sdkext"]
-                .iter()
-                .map(|name| ApexConfig { name: name.to_string() }),
+            MICRODROID_REQUIRED_APEXES.iter().map(|name| ApexConfig { name: name.to_string() }),
         );
         apexes.dedup_by(|a, b| a.name == b.name);
 
@@ -520,3 +525,19 @@
 fn new_binder_exception<T: AsRef<str>>(exception: ExceptionCode, message: T) -> Status {
     Status::new_exception(exception, CString::new(message.as_ref()).ok().as_deref())
 }
+
+/// Simple utility for referencing Borrowed or Owned. Similar to std::borrow::Cow, but
+/// it doesn't require that T implements Clone.
+enum BorrowedOrOwned<'a, T> {
+    Borrowed(&'a T),
+    Owned(T),
+}
+
+impl<'a, T> AsRef<T> for BorrowedOrOwned<'a, T> {
+    fn as_ref(&self) -> &T {
+        match self {
+            Self::Borrowed(b) => b,
+            Self::Owned(o) => &o,
+        }
+    }
+}
diff --git a/virtualizationservice/src/composite.rs b/virtualizationservice/src/composite.rs
index 1af0eed..ed277e6 100644
--- a/virtualizationservice/src/composite.rs
+++ b/virtualizationservice/src/composite.rs
@@ -79,7 +79,7 @@
 
 impl PartitionInfo {
     fn aligned_size(&self) -> u64 {
-        align_to_power_of_2(self.files.iter().map(|file| file.size).sum(), PARTITION_SIZE_SHIFT)
+        align_to_partition_size(self.files.iter().map(|file| file.size).sum())
     }
 }
 
diff --git a/virtualizationservice/src/payload.rs b/virtualizationservice/src/payload.rs
index aaf43e4..b6e9c04 100644
--- a/virtualizationservice/src/payload.rs
+++ b/virtualizationservice/src/payload.rs
@@ -16,7 +16,7 @@
 
 use crate::composite::align_to_partition_size;
 
-use anyhow::{Error, Result};
+use anyhow::Result;
 use microdroid_metadata::{ApexPayload, ApkPayload, Metadata};
 use microdroid_payload_config::ApexConfig;
 use std::fs;
@@ -36,6 +36,7 @@
 /// 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)?;
@@ -47,7 +48,8 @@
 
 /// 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, intergrity checks via hashtree will fail.
+/// 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 {
@@ -60,6 +62,7 @@
 
 /// 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)
 }
@@ -83,12 +86,12 @@
         version: 1u32,
         apexes: apexes
             .iter()
-            .map(|apex| ApexPayload { name: String::from(&apex.name), ..Default::default() })
+            .map(|apex| ApexPayload { name: apex.name.clone(), ..Default::default() })
             .collect(),
         apk: Some(ApkPayload {
-            name: String::from("apk"),
-            payload_partition_name: String::from("microdroid-apk"),
-            idsig_partition_name: String::from("microdroid-apk-idsig"),
+            name: "apk".to_owned(),
+            payload_partition_name: "microdroid-apk".to_owned(),
+            idsig_partition_name: "microdroid-apk-idsig".to_owned(),
             ..Default::default()
         })
         .into(),
@@ -101,39 +104,53 @@
 
     // put metadata at the first partition
     let mut partitions = vec![Partition {
-        label: String::from("metadata"),
+        label: "metadata".to_owned(),
         paths: vec![metadata_path],
         writable: false,
     }];
 
     let mut filler_count = 0;
-    let mut make_partition = |label: String,
-                              path: PathBuf,
-                              make_filler: &dyn Fn(u64, &Path) -> Result<bool, Error>|
-     -> 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 })
-        }
-    };
     for (i, apex) in apexes.iter().enumerate() {
         partitions.push(make_partition(
             format!("microdroid-apex-{}", i),
             get_path(&apex.name)?,
+            temporary_directory,
+            &mut filler_count,
             &make_size_filler,
         )?);
     }
-    partitions.push(make_partition(String::from("microdroid-apk"), apk_file, &make_zero_filler)?);
     partitions.push(make_partition(
-        String::from("microdroid-apk-idsig"),
+        "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,
     )?);
 
     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 })
+    }
+}
diff --git a/vm/src/run.rs b/vm/src/run.rs
index 1ae94ea..96ce745 100644
--- a/vm/src/run.rs
+++ b/vm/src/run.rs
@@ -93,7 +93,7 @@
     let vm = service.startVm(config, stdout.as_ref()).context("Failed to start VM")?;
 
     let cid = vm.getCid().context("Failed to get CID")?;
-    println!("Started VM from {:?} with CID {}.", config_path, cid);
+    println!("Started VM from {} with CID {}.", config_path, cid);
 
     if daemonize {
         // Pass the VM reference back to VirtualizationService and have it hold it in the