Merge "authfs: Support extra mount options"
diff --git a/TEST_MAPPING b/TEST_MAPPING
index 6b52514..d2a4821 100644
--- a/TEST_MAPPING
+++ b/TEST_MAPPING
@@ -4,12 +4,7 @@
       "name": "MicrodroidHostTestCases"
     },
     {
-      "name": "ComposHostTestCases",
-      "options": [
-        {
-          "exclude-filter": "android.compos.test.ComposKeyTestCase#testOdrefresh"
-        }
-      ]
+      "name": "ComposHostTestCases"
     }
   ],
   "postsubmit": [
@@ -18,15 +13,6 @@
     // somehow thinks that the test wasn't executed at all and reports it as a failure.
     {
       "name": "VirtualizationTestCases"
-    },
-    // TODO(victorhsieh): promotee this to presubmit once it's proven stable.
-    {
-      "name": "ComposHostTestCases",
-      "options": [
-        {
-          "include-filter": "android.compos.test.ComposKeyTestCase#testOdrefresh"
-        }
-      ]
     }
   ],
   "imports": [
diff --git a/microdroid/Android.bp b/microdroid/Android.bp
index a5b2898..a0215c3 100644
--- a/microdroid/Android.bp
+++ b/microdroid/Android.bp
@@ -35,6 +35,10 @@
         target: "/system/etc",
         name: "etc",
     },
+    {
+        target: "/system/bin",
+        name: "bin",
+    },
 ]
 
 android_system_image {
@@ -88,6 +92,12 @@
         "microdroid_keystore2_key_contexts",
         "microdroid_compatibility_matrix",
         "microdroid_manifest",
+
+        // TODO(b/195425111) these four should be added automatically
+        "android.hardware.security.secureclock-V1-ndk",
+        "android.hardware.security.sharedsecret-V1-ndk",
+        "libcrypto",
+        "liblzma",
     ] + microdroid_shell_and_utilities,
     multilib: {
         common: {
@@ -211,7 +221,7 @@
         },
         x86_64: {
             kernel_prebuilt: ":kernel_prebuilts-5.10-x86_64",
-            cmdline: microdroid_boot_cmdline + "pci=noacpi ",
+            cmdline: microdroid_boot_cmdline + "acpi=noirq",
         },
     },
     dtb_prebuilt: "dummy_dtb.img",
diff --git a/microdroid/init.rc b/microdroid/init.rc
index 043577d..d43ab22 100644
--- a/microdroid/init.rc
+++ b/microdroid/init.rc
@@ -117,17 +117,9 @@
     # The bind+remount combination allows this to work in containers.
     mount rootfs rootfs / remount bind ro nodev
 
-    start keystore2
-
-on late-fs
-    start vendor.keymint-microdroid
-
     # TODO(b/185767624): change the hard-coded size?
     mount tmpfs tmpfs /data noatime nosuid nodev rw size=128M
 
-on post-fs-data
-    mark_post_data
-
     # We chown/chmod /data again so because mount is run as root + defaults
     chown system system /data
     chmod 0771 /data
@@ -135,6 +127,21 @@
     # We restorecon /data in case the userdata partition has been reset.
     restorecon /data
 
+    # set up keystore directory structure first so that we can end early boot
+    # and start apexd
+    mkdir /data/misc 01771 system misc
+    mkdir /data/misc/keystore 0700 keystore keystore
+    # work around b/183668221
+    restorecon /data/misc /data/misc/keystore
+
+    start keystore2
+
+on late-fs
+    start vendor.keymint-microdroid
+
+on post-fs-data
+    mark_post_data
+
     mkdir /data/vendor 0771 root root
     mkdir /data/vendor_ce 0771 root root
     mkdir /data/vendor_de 0771 root root
@@ -148,13 +155,6 @@
 
     start tombstoned
 
-    # set up keystore directory structure first so that we can end early boot
-    # and start apexd
-    mkdir /data/misc 01771 system misc
-    mkdir /data/misc/keystore 0700 keystore keystore
-    # work around b/183668221
-    restorecon /data/misc /data/misc/keystore
-
     # Boot level 30
     # odsign signing keys have MAX_BOOT_LEVEL=30
     # This is currently the earliest boot level, but we start at 30
diff --git a/microdroid/payload/Android.bp b/microdroid/payload/Android.bp
index c7bc415..72711c3 100644
--- a/microdroid/payload/Android.bp
+++ b/microdroid/payload/Android.bp
@@ -49,30 +49,26 @@
     ],
 }
 
-cc_binary {
+cc_binary_host {
     name: "mk_payload",
     srcs: [
         "mk_payload.cc",
     ],
-    shared_libs: [
+    static_libs: [
+        "lib_microdroid_metadata_proto",
         "libbase",
+        "libcdisk_spec",
         "libcuttlefish_fs",
         "libcuttlefish_utils",
-        "liblog",
-        "libz",
-    ],
-    static_libs: [
-        "lib_microdroid_metadata_proto_lite",
-        "libcdisk_spec",
         "libext2_uuid",
         "libimage_aggregator",
         "libjsoncpp",
+        "liblog",
+        "libprotobuf-cpp-full",
         "libprotobuf-cpp-lite",
         "libsparse",
         "libxml2",
+        "libz",
     ],
-    generated_sources: ["apex-info-list"],
-    apex_available: [
-        "com.android.virt",
-    ],
+    static_executable: true,
 }
diff --git a/microdroid/payload/README.md b/microdroid/payload/README.md
index 35502c1..bf05c49 100644
--- a/microdroid/payload/README.md
+++ b/microdroid/payload/README.md
@@ -3,6 +3,9 @@
 Payload disk is a composite disk image referencing host APEXes and an APK so that microdroid
 mounts/activates APK/APEXes and executes a binary within the APK.
 
+Payload disk is created by [VirtualizationService](../../virtualizationservice) Service when
+starting a VM.
+
 ## Partitions
 
 Payload disk has 1 + N(number of APEX/APK payloads) partitions.
@@ -14,7 +17,7 @@
 
 * partition 1: Metadata partition
 * partition 2 ~ n: APEX payloads
-* partition n + 1: APK payload
+* partition n+1, n+2: APK payload and its idsig
 
 It's subject to change in the future, though.
 
@@ -34,52 +37,37 @@
 
 Each payload partition presents APEX or APK passed from the host.
 
-At the end of each payload partition the size of the original payload file (APEX or APK) is stored
-in 4-byte big endian.
+Note that each payload passed to the Guest is read by a block device. If a payload is not sized to a
+multiples of 4k, reading it would fail. To prevent that, "zero fillers" are added for those files.
+For example, if an APK is 8000 byte big, the APK partition would be padded with 192 bytes of zeros.
 
-For example, the following code shows how to get the original size of host apex file
-when the apex is read in microdroid as /dev/block/vdc2,
+# `mk_payload`
 
-    int fd = open("/dev/block/vdc2", O_RDONLY | O_BINARY | O_CLOEXEC);
-    uint32_t size;
-    lseek(fd, -sizeof(size), SEEK_END);
-    read(fd, &size, sizeof(size));
-    size = betoh32(size);
-
-## How to Create
-
-### `mk_payload`
-
-`mk_payload` creates a payload composite disk image as described in a JSON which is intentionlly
-similar to the schema of VM payload config.
+`mk_payload` is a small utility to create a payload disk image.
 
 ```
 $ cat payload_config.json
 {
-  "system_apexes": [
-    "com.android.adbd",
-  ],
   "apexes": [
     {
       "name": "com.my.hello",
-      "path": "hello.apex"
+      "path": "hello.apex",
     }
   ],
   "apk": {
     "name": "com.my.world",
-    "path": "/path/to/world.apk"
+    "path": "/path/to/world.apk",
+    "idsigPath": "/path/to/world.apk.idsig",
   }
 }
-$ adb push payload_config.json hello.apex /data/local/tmp/
-$ adb shell 'cd /data/local/tmp; /apex/com.android.virt/bin/mk_payload payload_config.json payload.img
-$ adb shell ls /data/local/tmp/*.img
+$ m mk_payload
+$ mk_payload payload_config.json payload.img
+$ ls
 payload.img
 payload-footer.img
 payload-header.img
 payload-metadata.img
-payload.img.0          # fillers
-payload.img.1
+payload-filler-0.img
+payload-filler-1.img
 ...
 ```
-
-In the future, [VirtualizationService](../../virtualizationservice) will handle this.
diff --git a/microdroid/payload/mk_payload.cc b/microdroid/payload/mk_payload.cc
index b27683c..33e91b9 100644
--- a/microdroid/payload/mk_payload.cc
+++ b/microdroid/payload/mk_payload.cc
@@ -26,7 +26,6 @@
 
 #include <android-base/file.h>
 #include <android-base/result.h>
-#include <com_android_apex.h>
 #include <image_aggregator.h>
 #include <json/json.h>
 
@@ -42,9 +41,6 @@
 using android::microdroid::Metadata;
 using android::microdroid::WriteMetadata;
 
-using com::android::apex::ApexInfoList;
-using com::android::apex::readApexInfoList;
-
 using cuttlefish::AlignToPartitionSize;
 using cuttlefish::CreateCompositeDisk;
 using cuttlefish::kLinuxFilesystem;
@@ -58,9 +54,9 @@
     return static_cast<uint32_t>(st.st_size);
 }
 
-std::string ToAbsolute(const std::string& path, const std::string& dirname) {
+std::string RelativeTo(const std::string& path, const std::string& dirname) {
     bool is_absolute = !path.empty() && path[0] == '/';
-    if (is_absolute) {
+    if (is_absolute || dirname == ".") {
         return path;
     } else {
         return dirname + "/" + path;
@@ -81,25 +77,20 @@
     std::string name; // the apex name
     std::string path; // the path to the apex file
                       // absolute or relative to the config file
-    std::optional<std::string> public_key;
-    std::optional<std::string> root_digest;
 };
 
 struct ApkConfig {
     std::string name;
     std::string path;
-    // TODO(jooyung) make this required?
-    std::optional<std::string> idsig_path;
+    std::string idsig_path;
 };
 
 struct Config {
     std::string dirname; // config file's direname to resolve relative paths in the config
 
-    // TODO(b/185956069) remove this when VirtualizationService can provide apex paths
-    std::vector<std::string> system_apexes;
-
     std::vector<ApexConfig> apexes;
     std::optional<ApkConfig> apk;
+    // This is a path in the guest side
     std::optional<std::string> payload_config_path;
 };
 
@@ -137,8 +128,6 @@
 Result<void> ParseJson(const Json::Value& value, ApexConfig& apex_config) {
     DO(ParseJson(value["name"], apex_config.name));
     DO(ParseJson(value["path"], apex_config.path));
-    DO(ParseJson(value["publicKey"], apex_config.public_key));
-    DO(ParseJson(value["rootDigest"], apex_config.root_digest));
     return {};
 }
 
@@ -150,7 +139,6 @@
 }
 
 Result<void> ParseJson(const Json::Value& value, Config& config) {
-    DO(ParseJson(value["system_apexes"], config.system_apexes));
     DO(ParseJson(value["apexes"], config.apexes));
     DO(ParseJson(value["apk"], config.apk));
     DO(ParseJson(value["payload_config_path"], config.payload_config_path));
@@ -163,7 +151,7 @@
     Json::Value root;
     Json::String errs;
     if (!parseFromStream(builder, in, &root, &errs)) {
-        return Error() << "bad config: " << errs;
+        return Error() << errs;
     }
 
     Config config;
@@ -174,63 +162,22 @@
 
 #undef DO
 
-Result<void> LoadSystemApexes(Config& config) {
-    static const char* kApexInfoListFile = "/apex/apex-info-list.xml";
-    std::optional<ApexInfoList> apex_info_list = readApexInfoList(kApexInfoListFile);
-    if (!apex_info_list.has_value()) {
-        return Error() << "Failed to read " << kApexInfoListFile;
-    }
-    auto get_apex_path = [&](const std::string& apex_name) -> std::optional<std::string> {
-        for (const auto& apex_info : apex_info_list->getApexInfo()) {
-            if (apex_info.getIsActive() && apex_info.getModuleName() == apex_name) {
-                return apex_info.getModulePath();
-            }
-        }
-        return std::nullopt;
-    };
-    for (const auto& apex_name : config.system_apexes) {
-        const auto& apex_path = get_apex_path(apex_name);
-        if (!apex_path.has_value()) {
-            return Error() << "Can't find the system apex: " << apex_name;
-        }
-        config.apexes.push_back(ApexConfig{
-                .name = apex_name,
-                .path = *apex_path,
-                .public_key = std::nullopt,
-                .root_digest = std::nullopt,
-        });
-    }
-    return {};
-}
-
 Result<void> MakeMetadata(const Config& config, const std::string& filename) {
     Metadata metadata;
     metadata.set_version(1);
 
+    int apex_index = 0;
     for (const auto& apex_config : config.apexes) {
         auto* apex = metadata.add_apexes();
-
-        // name
         apex->set_name(apex_config.name);
-
-        // publicKey
-        if (apex_config.public_key.has_value()) {
-            apex->set_publickey(apex_config.public_key.value());
-        }
-
-        // rootDigest
-        if (apex_config.root_digest.has_value()) {
-            apex->set_rootdigest(apex_config.root_digest.value());
-        }
+        apex->set_partition_name("microdroid-apex-" + std::to_string(apex_index++));
     }
 
     if (config.apk.has_value()) {
         auto* apk = metadata.mutable_apk();
         apk->set_name(config.apk->name);
         apk->set_payload_partition_name("microdroid-apk");
-        if (config.apk->idsig_path.has_value()) {
-            apk->set_idsig_partition_name("microdroid-apk-idsig");
-        }
+        apk->set_idsig_partition_name("microdroid-apk-idsig");
     }
 
     if (config.payload_config_path.has_value()) {
@@ -241,34 +188,8 @@
     return WriteMetadata(metadata, out);
 }
 
-// fill (zeros + original file's size) with aligning BLOCK_SIZE(4096) boundary
-// return true when the filler is generated.
-Result<bool> SizeFiller(const std::string& file_path, const std::string& filler_path) {
-    auto file_size = GetFileSize(file_path);
-    if (!file_size.ok()) {
-        return file_size.error();
-    }
-    auto disk_size = AlignToPartitionSize(*file_size + sizeof(uint32_t));
-
-    unique_fd fd(TEMP_FAILURE_RETRY(open(filler_path.c_str(), O_CREAT | O_WRONLY | O_TRUNC, 0600)));
-    if (fd.get() == -1) {
-        return ErrnoError() << "open(" << filler_path << ") failed.";
-    }
-    uint32_t size = htobe32(static_cast<uint32_t>(*file_size));
-    if (ftruncate(fd.get(), disk_size - *file_size) == -1) {
-        return ErrnoError() << "ftruncate(" << filler_path << ") failed.";
-    }
-    if (lseek(fd.get(), -sizeof(size), SEEK_END) == -1) {
-        return ErrnoError() << "lseek(" << filler_path << ") failed.";
-    }
-    if (write(fd.get(), &size, sizeof(size)) <= 0) {
-        return ErrnoError() << "write(" << filler_path << ") failed.";
-    }
-    return true;
-}
-
 // fill zeros to align |file_path|'s size to BLOCK_SIZE(4096) boundary.
-// return true when the filler is generated.
+// return true when the filler is needed.
 Result<bool> ZeroFiller(const std::string& file_path, const std::string& filler_path) {
     auto file_size = GetFileSize(file_path);
     if (!file_size.ok()) {
@@ -288,32 +209,17 @@
     return true;
 }
 
-// Do not generate any fillers
-// Note that CreateCompositeDisk() handles gaps between partitions.
-Result<bool> NoFiller(const std::string& file_path, const std::string& filler_path) {
-    (void)file_path;
-    (void)filler_path;
-    return false;
-}
-
 Result<void> MakePayload(const Config& config, const std::string& metadata_file,
                          const std::string& output_file) {
     std::vector<MultipleImagePartition> partitions;
 
-    // put metadata at the first partition
-    partitions.push_back(MultipleImagePartition{
-            .label = "payload-metadata",
-            .image_file_paths = {metadata_file},
-            .type = kLinuxFilesystem,
-            .read_only = true,
-    });
-
     int filler_count = 0;
-    auto add_partition = [&](auto partition_name, auto file_path, auto filler) -> Result<void> {
+    auto add_partition = [&](auto partition_name, auto file_path) -> Result<void> {
         std::vector<std::string> image_files{file_path};
 
-        std::string filler_path = output_file + "." + std::to_string(filler_count++);
-        if (auto ret = filler(file_path, filler_path); !ret.ok()) {
+        std::string filler_path =
+                AppendFileName(output_file, "-filler-" + std::to_string(filler_count++));
+        if (auto ret = ZeroFiller(file_path, filler_path); !ret.ok()) {
             return ret.error();
         } else if (*ret) {
             image_files.push_back(filler_path);
@@ -327,27 +233,31 @@
         return {};
     };
 
-    // put apexes at the subsequent partitions with "size" filler
+    // put metadata at the first partition
+    partitions.push_back(MultipleImagePartition{
+            .label = "payload-metadata",
+            .image_file_paths = {metadata_file},
+            .type = kLinuxFilesystem,
+            .read_only = true,
+    });
+    // put apexes at the subsequent partitions
     for (size_t i = 0; i < config.apexes.size(); i++) {
         const auto& apex_config = config.apexes[i];
-        std::string apex_path = ToAbsolute(apex_config.path, config.dirname);
-        if (auto ret = add_partition("microdroid-apex-" + std::to_string(i), apex_path, SizeFiller);
+        std::string apex_path = RelativeTo(apex_config.path, config.dirname);
+        if (auto ret = add_partition("microdroid-apex-" + std::to_string(i), apex_path);
             !ret.ok()) {
             return ret.error();
         }
     }
-    // put apk with "zero" filler.
-    // TODO(jooyung): partition name("microdroid-apk") is TBD
+    // put apk and its idsig
     if (config.apk.has_value()) {
-        std::string apk_path = ToAbsolute(config.apk->path, config.dirname);
-        if (auto ret = add_partition("microdroid-apk", apk_path, ZeroFiller); !ret.ok()) {
+        std::string apk_path = RelativeTo(config.apk->path, config.dirname);
+        if (auto ret = add_partition("microdroid-apk", apk_path); !ret.ok()) {
             return ret.error();
         }
-        if (config.apk->idsig_path.has_value()) {
-            std::string idsig_path = ToAbsolute(config.apk->idsig_path.value(), config.dirname);
-            if (auto ret = add_partition("microdroid-apk-idsig", idsig_path, NoFiller); !ret.ok()) {
-                return ret.error();
-            }
+        std::string idsig_path = RelativeTo(config.apk->idsig_path, config.dirname);
+        if (auto ret = add_partition("microdroid-apk-idsig", idsig_path); !ret.ok()) {
+            return ret.error();
         }
     }
 
@@ -365,12 +275,7 @@
 
     auto config = LoadConfig(argv[1]);
     if (!config.ok()) {
-        std::cerr << config.error() << '\n';
-        return 1;
-    }
-
-    if (const auto res = LoadSystemApexes(*config); !res.ok()) {
-        std::cerr << res.error() << '\n';
+        std::cerr << "bad config: " << config.error() << '\n';
         return 1;
     }
 
diff --git a/microdroid_manager/Android.bp b/microdroid_manager/Android.bp
index 4ea156a..0ea5d87 100644
--- a/microdroid_manager/Android.bp
+++ b/microdroid_manager/Android.bp
@@ -16,9 +16,9 @@
         "libmicrodroid_metadata",
         "libmicrodroid_payload_config",
         "libprotobuf",
+        "librustutils",
         "libserde",
         "libserde_json",
-        "libsystem_properties-rust",
         "libvsock",
     ],
     init_rc: ["microdroid_manager.rc"],
diff --git a/microdroid_manager/src/main.rs b/microdroid_manager/src/main.rs
index 9efa68a..2586737 100644
--- a/microdroid_manager/src/main.rs
+++ b/microdroid_manager/src/main.rs
@@ -20,13 +20,13 @@
 use anyhow::{anyhow, bail, Result};
 use log::{error, info, warn};
 use microdroid_payload_config::{Task, TaskType, VmPayloadConfig};
+use rustutils::system_properties::PropertyWatcher;
 use std::fs::{self, File};
 use std::os::unix::io::{FromRawFd, IntoRawFd};
 use std::path::Path;
 use std::process::{Command, Stdio};
 use std::str;
 use std::time::Duration;
-use system_properties::PropertyWatcher;
 use vsock::VsockStream;
 
 const WAIT_TIMEOUT: Duration = Duration::from_secs(10);
@@ -40,7 +40,7 @@
         let config = load_config(Path::new(&metadata.payload_config_path))?;
 
         let fake_secret = "This is a placeholder for a value that is derived from the images that are loaded in the VM.";
-        if let Err(err) = system_properties::write("ro.vmsecret.keymint", fake_secret) {
+        if let Err(err) = rustutils::system_properties::write("ro.vmsecret.keymint", fake_secret) {
             warn!("failed to set ro.vmsecret.keymint: {}", err);
         }
 
diff --git a/tests/hostside/java/android/virt/test/MicrodroidTestCase.java b/tests/hostside/java/android/virt/test/MicrodroidTestCase.java
index c05a841..aa7c9ab 100644
--- a/tests/hostside/java/android/virt/test/MicrodroidTestCase.java
+++ b/tests/hostside/java/android/virt/test/MicrodroidTestCase.java
@@ -69,15 +69,13 @@
         assertThat(runOnMicrodroid("getprop", "debug.microdroid.app.run"), is("true"));
         assertThat(runOnMicrodroid("getprop", "debug.microdroid.app.sublib.run"), is("true"));
 
-        // Manually execute the library and check the output
-        final String microdroidLauncher = "system/bin/microdroid_launcher";
-        assertThat(
-                runOnMicrodroid(microdroidLauncher, testLib, "arg1", "arg2"),
-                is("Hello Microdroid " + testLib + " arg1 arg2"));
-
-        // Check that keystore was found by the payload
+        // Check that keystore was found by the payload. Wait until the property is set.
+        tryRunOnMicrodroid("watch -e \"getprop debug.microdroid.test.keystore | grep '^$'\"");
         assertThat(runOnMicrodroid("getprop", "debug.microdroid.test.keystore"), is("PASS"));
 
+        // Check that no denials have happened so far
+        assertThat(runOnMicrodroid("logcat -d -e 'avc:[[:space:]]{1,2}denied'"), is(""));
+
         shutdownMicrodroid(getDevice(), cid);
     }
 
diff --git a/virtualizationservice/Android.bp b/virtualizationservice/Android.bp
index 239d729..cf92d5a 100644
--- a/virtualizationservice/Android.bp
+++ b/virtualizationservice/Android.bp
@@ -25,19 +25,15 @@
         "libandroid_logger",
         "libanyhow",
         "libcommand_fds",
-        "libcrc32fast",
         "libdisk",
         "liblog_rust",
         "libmicrodroid_metadata",
         "libmicrodroid_payload_config",
         "libonce_cell",
-        "libprotobuf",
-        "libprotos",
         "libserde_json",
         "libserde_xml_rs",
         "libserde",
         "libshared_child",
-        "libuuid",
         "libvmconfig",
         "libzip",
         "libvsock",
diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index 64d3913..dc38075 100644
--- a/virtualizationservice/src/aidl.rs
+++ b/virtualizationservice/src/aidl.rs
@@ -128,14 +128,13 @@
         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| {
+        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
@@ -291,7 +290,7 @@
     Ok(())
 }
 
-fn write_zero_filler(zero_filler_path: &Path) -> Result<File> {
+fn write_zero_filler(zero_filler_path: &Path) -> Result<()> {
     let file = OpenOptions::new()
         .create_new(true)
         .read(true)
@@ -299,7 +298,7 @@
         .open(zero_filler_path)
         .with_context(|| "Failed to create zero.img")?;
     file.set_len(ZERO_FILLER_SIZE)?;
-    Ok(file)
+    Ok(())
 }
 
 /// Given the configuration for a disk image, assembles the `DiskFile` to pass to crosvm.
diff --git a/virtualizationservice/src/composite.rs b/virtualizationservice/src/composite.rs
index 685d0e6..40c7e5e 100644
--- a/virtualizationservice/src/composite.rs
+++ b/virtualizationservice/src/composite.rs
@@ -14,285 +14,18 @@
 
 //! Functions for creating a composite disk image.
 
-use crate::gpt::{
-    write_gpt_header, write_protective_mbr, GptPartitionEntry, GPT_BEGINNING_SIZE, GPT_END_SIZE,
-    GPT_HEADER_SIZE, GPT_NUM_PARTITIONS, GPT_PARTITION_ENTRY_SIZE, SECTOR_SIZE,
-};
 use android_system_virtualizationservice::aidl::android::system::virtualizationservice::Partition::Partition;
-use anyhow::{anyhow, bail, Context, Error};
-use crc32fast::Hasher;
-use disk::create_disk_file;
-use log::{trace, warn};
-use protobuf::Message;
-use protos::cdisk_spec::{ComponentDisk, CompositeDisk, ReadWriteCapability};
-use std::convert::TryInto;
+use anyhow::{anyhow, Context, Error};
+use disk::{create_composite_disk, create_disk_file, ImagePartitionType, PartitionInfo};
 use std::fs::{File, OpenOptions};
-use std::io::Write;
 use std::os::unix::io::AsRawFd;
 use std::path::{Path, PathBuf};
-use uuid::Uuid;
-
-/// A magic string placed at the beginning of a composite disk file to identify it.
-const CDISK_MAGIC: &str = "composite_disk\x1d";
-/// The version of the composite disk format supported by this implementation.
-const COMPOSITE_DISK_VERSION: u64 = 1;
-/// The amount of padding needed between the last partition entry and the first partition, to align
-/// the partition appropriately. The two sectors are for the MBR and the GPT header.
-const PARTITION_ALIGNMENT_SIZE: usize = GPT_BEGINNING_SIZE as usize
-    - 2 * SECTOR_SIZE as usize
-    - GPT_NUM_PARTITIONS as usize * GPT_PARTITION_ENTRY_SIZE as usize;
-const HEADER_PADDING_LENGTH: usize = SECTOR_SIZE as usize - GPT_HEADER_SIZE as usize;
-// Keep all partitions 4k aligned for performance.
-const PARTITION_SIZE_SHIFT: u8 = 12;
-// Keep the disk size a multiple of 64k for crosvm's virtio_blk driver.
-const DISK_SIZE_SHIFT: u8 = 16;
-
-const LINUX_FILESYSTEM_GUID: Uuid = Uuid::from_u128(0x0FC63DAF_8483_4772_8E79_3D69D8477DE4);
-const EFI_SYSTEM_PARTITION_GUID: Uuid = Uuid::from_u128(0xC12A7328_F81F_11D2_BA4B_00A0C93EC93B);
-
-/// Information about a partition to create.
-#[derive(Clone, Debug, Eq, PartialEq)]
-pub struct PartitionInfo {
-    label: String,
-    path: PathBuf,
-    partition_type: ImagePartitionType,
-    writable: bool,
-    size: u64,
-}
-
-/// Round `val` up to the next multiple of 2**`align_log`.
-fn align_to_power_of_2(val: u64, align_log: u8) -> u64 {
-    let align = 1 << align_log;
-    ((val + (align - 1)) / align) * align
-}
-
-/// Round `val` to partition size(4K)
-fn align_to_partition_size(val: u64) -> u64 {
-    align_to_power_of_2(val, PARTITION_SIZE_SHIFT)
-}
-
-impl PartitionInfo {
-    fn aligned_size(&self) -> u64 {
-        align_to_partition_size(self.size)
-    }
-}
-
-/// The type of partition.
-#[allow(dead_code)]
-#[derive(Copy, Clone, Debug, Eq, PartialEq)]
-pub enum ImagePartitionType {
-    LinuxFilesystem,
-    EfiSystemPartition,
-}
-
-impl ImagePartitionType {
-    fn guid(self) -> Uuid {
-        match self {
-            Self::LinuxFilesystem => LINUX_FILESYSTEM_GUID,
-            Self::EfiSystemPartition => EFI_SYSTEM_PARTITION_GUID,
-        }
-    }
-}
-
-/// Write protective MBR and primary GPT table.
-fn write_beginning(
-    file: &mut impl Write,
-    disk_guid: Uuid,
-    partitions: &[u8],
-    partition_entries_crc32: u32,
-    secondary_table_offset: u64,
-    disk_size: u64,
-) -> Result<(), Error> {
-    // Write the protective MBR to the first sector.
-    write_protective_mbr(file, disk_size)?;
-
-    // Write the GPT header, and pad out to the end of the sector.
-    write_gpt_header(file, disk_guid, partition_entries_crc32, secondary_table_offset, false)?;
-    file.write_all(&[0; HEADER_PADDING_LENGTH])?;
-
-    // Write partition entries, including unused ones.
-    file.write_all(partitions)?;
-
-    // Write zeroes to align the first partition appropriately.
-    file.write_all(&[0; PARTITION_ALIGNMENT_SIZE])?;
-
-    Ok(())
-}
-
-/// Write secondary GPT table.
-fn write_end(
-    file: &mut impl Write,
-    disk_guid: Uuid,
-    partitions: &[u8],
-    partition_entries_crc32: u32,
-    secondary_table_offset: u64,
-    disk_size: u64,
-) -> Result<(), Error> {
-    // Write partition entries, including unused ones.
-    file.write_all(partitions)?;
-
-    // Write the GPT header, and pad out to the end of the sector.
-    write_gpt_header(file, disk_guid, partition_entries_crc32, secondary_table_offset, true)?;
-    file.write_all(&[0; HEADER_PADDING_LENGTH])?;
-
-    // Pad out to the aligned disk size.
-    let used_disk_size = secondary_table_offset + GPT_END_SIZE;
-    let padding = disk_size - used_disk_size;
-    file.write_all(&vec![0; padding as usize])?;
-
-    Ok(())
-}
-
-/// Create the `GptPartitionEntry` for the given partition.
-fn create_gpt_entry(partition: &PartitionInfo, offset: u64) -> GptPartitionEntry {
-    let mut partition_name: Vec<u16> = partition.label.encode_utf16().collect();
-    partition_name.resize(36, 0);
-
-    GptPartitionEntry {
-        partition_type_guid: partition.partition_type.guid(),
-        unique_partition_guid: Uuid::new_v4(),
-        first_lba: offset / SECTOR_SIZE,
-        last_lba: (offset + partition.aligned_size()) / SECTOR_SIZE - 1,
-        attributes: 0,
-        partition_name: partition_name.try_into().unwrap(),
-    }
-}
-
-/// Create one or more `ComponentDisk` proto messages for the given partition.
-fn create_component_disks(
-    partition: &PartitionInfo,
-    offset: u64,
-    zero_filler_path: &str,
-) -> Result<Vec<ComponentDisk>, Error> {
-    let aligned_size = partition.aligned_size();
-
-    let mut component_disks = vec![ComponentDisk {
-        offset,
-        file_path: partition.path.to_str().context("Invalid partition path")?.to_string(),
-        read_write_capability: if partition.writable {
-            ReadWriteCapability::READ_WRITE
-        } else {
-            ReadWriteCapability::READ_ONLY
-        },
-        ..ComponentDisk::new()
-    }];
-
-    if partition.size != aligned_size {
-        if partition.writable {
-            bail!(
-                "Read-write partition {:?} size is not a multiple of {}.",
-                partition,
-                1 << PARTITION_SIZE_SHIFT
-            );
-        } else {
-            // Fill in the gap by reusing the header file, because we know it is always bigger
-            // than the alignment size (i.e. GPT_BEGINNING_SIZE > 1 << PARTITION_SIZE_SHIFT).
-            warn!(
-                "Read-only partition {:?} size is not a multiple of {}, filling gap.",
-                partition,
-                1 << PARTITION_SIZE_SHIFT
-            );
-            component_disks.push(ComponentDisk {
-                offset: offset + partition.size,
-                file_path: zero_filler_path.to_owned(),
-                read_write_capability: ReadWriteCapability::READ_ONLY,
-                ..ComponentDisk::new()
-            });
-        }
-    }
-
-    Ok(component_disks)
-}
-
-/// Create a new composite disk containing the given partitions, and write it out to the given
-/// 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,
-        offset: 0,
-        read_write_capability: ReadWriteCapability::READ_ONLY,
-        ..ComponentDisk::new()
-    });
-
-    // Write partitions to a temporary buffer so that we can calculate the CRC, and construct the
-    // ComponentDisk proto messages at the same time.
-    let mut partitions_buffer =
-        [0u8; GPT_NUM_PARTITIONS as usize * GPT_PARTITION_ENTRY_SIZE as usize];
-    let mut writer: &mut [u8] = &mut partitions_buffer;
-    let mut next_disk_offset = GPT_BEGINNING_SIZE;
-    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, &zero_filler_path)?
-        {
-            composite_proto.component_disks.push(component_disk);
-        }
-
-        next_disk_offset += partition.aligned_size();
-    }
-    let secondary_table_offset = next_disk_offset;
-    let disk_size = align_to_power_of_2(secondary_table_offset + GPT_END_SIZE, DISK_SIZE_SHIFT);
-    trace!("Partitions: {:#?}", partitions);
-    trace!("Secondary table offset: {} disk size: {}", secondary_table_offset, disk_size);
-
-    composite_proto.component_disks.push(ComponentDisk {
-        file_path: footer_path,
-        offset: secondary_table_offset,
-        read_write_capability: ReadWriteCapability::READ_ONLY,
-        ..ComponentDisk::new()
-    });
-
-    // Calculate CRC32 of partition entries.
-    let mut hasher = Hasher::new();
-    hasher.update(&partitions_buffer);
-    let partition_entries_crc32 = hasher.finalize();
-
-    let disk_guid = Uuid::new_v4();
-    write_beginning(
-        header_file,
-        disk_guid,
-        &partitions_buffer,
-        partition_entries_crc32,
-        secondary_table_offset,
-        disk_size,
-    )?;
-    write_end(
-        footer_file,
-        disk_guid,
-        &partitions_buffer,
-        partition_entries_crc32,
-        secondary_table_offset,
-        disk_size,
-    )?;
-
-    composite_proto.length = disk_size;
-    output_composite.write_all(CDISK_MAGIC.as_bytes())?;
-    composite_proto.write_to_writer(output_composite)?;
-
-    Ok(())
-}
 
 /// Constructs a composite disk image for the given list of partitions, and opens it ready to use.
 ///
-/// Returns the composite disk image file, and a list of FD mappings which must be applied to any
-/// process which wants to use it. This is necessary because the composite image contains paths of
-/// the form `/proc/self/fd/N` for the partition images.
+/// Returns the composite disk image file, and a list of files whose file descriptors must be passed
+/// to any process which wants to use it. This is necessary because the composite image contains
+/// paths of the form `/proc/self/fd/N` for the partition images.
 pub fn make_composite_image(
     partitions: &[Partition],
     zero_filler_path: &Path,
@@ -300,7 +33,7 @@
     header_path: &Path,
     footer_path: &Path,
 ) -> Result<(File, Vec<File>), Error> {
-    let (partitions, files) = convert_partitions(partitions)?;
+    let (partitions, mut files) = convert_partitions(partitions)?;
 
     let mut composite_image = OpenOptions::new()
         .create_new(true)
@@ -316,13 +49,16 @@
         OpenOptions::new().create_new(true).read(true).write(true).open(footer_path).with_context(
             || format!("Failed to create composite image header {:?}", footer_path),
         )?;
+    let zero_filler_file = File::open(&zero_filler_path).with_context(|| {
+        format!("Failed to open composite image zero filler {:?}", zero_filler_path)
+    })?;
 
     create_composite_disk(
         &partitions,
-        zero_filler_path,
-        header_path,
+        &fd_path_for_file(&zero_filler_file),
+        &fd_path_for_file(&header_file),
         &mut header_file,
-        footer_path,
+        &fd_path_for_file(&footer_file),
         &mut footer_file,
         &mut composite_image,
     )?;
@@ -331,12 +67,16 @@
     let composite_image = File::open(&output_path)
         .with_context(|| format!("Failed to open composite image {:?}", output_path))?;
 
+    files.push(header_file);
+    files.push(footer_file);
+    files.push(zero_filler_file);
+
     Ok((composite_image, files))
 }
 
 /// Given the AIDL config containing a list of partitions, with a [`ParcelFileDescriptor`] for each
-/// partition, return the list of file descriptors which must be passed to the composite disk image
-/// partition configuration for it.
+/// partition, returns the corresponding list of PartitionInfo and the list of files whose file
+/// descriptors must be passed to any process using the composite image.
 fn convert_partitions(partitions: &[Partition]) -> Result<(Vec<PartitionInfo>, Vec<File>), Error> {
     // File descriptors to pass to child process.
     let mut files = vec![];
@@ -353,12 +93,12 @@
                 .try_clone()
                 .context("Failed to clone partition image file descriptor")?;
             let size = get_partition_size(&file)?;
-            let fd = file.as_raw_fd();
+            let path = fd_path_for_file(&file);
             files.push(file);
 
             Ok(PartitionInfo {
                 label: partition.label.to_owned(),
-                path: format!("/proc/self/fd/{}", fd).into(),
+                path,
                 partition_type: ImagePartitionType::LinuxFilesystem,
                 writable: partition.writable,
                 size,
@@ -369,6 +109,11 @@
     Ok((partitions, files))
 }
 
+fn fd_path_for_file(file: &File) -> PathBuf {
+    let fd = file.as_raw_fd();
+    format!("/proc/self/fd/{}", fd).into()
+}
+
 /// 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.
@@ -378,63 +123,3 @@
         .map_err(|e| anyhow!("Failed to open partition image: {}", e))?
         .get_len()?)
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn beginning_size() {
-        let mut buffer = vec![];
-        let partitions = [0u8; GPT_NUM_PARTITIONS as usize * GPT_PARTITION_ENTRY_SIZE as usize];
-        let disk_size = 1000 * SECTOR_SIZE;
-        write_beginning(
-            &mut buffer,
-            Uuid::from_u128(0x12345678_1234_5678_abcd_12345678abcd),
-            &partitions,
-            42,
-            disk_size - GPT_END_SIZE,
-            disk_size,
-        )
-        .unwrap();
-
-        assert_eq!(buffer.len(), GPT_BEGINNING_SIZE as usize);
-    }
-
-    #[test]
-    fn end_size() {
-        let mut buffer = vec![];
-        let partitions = [0u8; GPT_NUM_PARTITIONS as usize * GPT_PARTITION_ENTRY_SIZE as usize];
-        let disk_size = 1000 * SECTOR_SIZE;
-        write_end(
-            &mut buffer,
-            Uuid::from_u128(0x12345678_1234_5678_abcd_12345678abcd),
-            &partitions,
-            42,
-            disk_size - GPT_END_SIZE,
-            disk_size,
-        )
-        .unwrap();
-
-        assert_eq!(buffer.len(), GPT_END_SIZE as usize);
-    }
-
-    #[test]
-    fn end_size_with_padding() {
-        let mut buffer = vec![];
-        let partitions = [0u8; GPT_NUM_PARTITIONS as usize * GPT_PARTITION_ENTRY_SIZE as usize];
-        let disk_size = 1000 * SECTOR_SIZE;
-        let padding = 3 * SECTOR_SIZE;
-        write_end(
-            &mut buffer,
-            Uuid::from_u128(0x12345678_1234_5678_abcd_12345678abcd),
-            &partitions,
-            42,
-            disk_size - GPT_END_SIZE - padding,
-            disk_size,
-        )
-        .unwrap();
-
-        assert_eq!(buffer.len(), GPT_END_SIZE as usize + padding as usize);
-    }
-}
diff --git a/virtualizationservice/src/gpt.rs b/virtualizationservice/src/gpt.rs
deleted file mode 100644
index 346a40a..0000000
--- a/virtualizationservice/src/gpt.rs
+++ /dev/null
@@ -1,240 +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.
-
-//! Functions for writing GUID Partition Tables for use in a composite disk image.
-
-use anyhow::Error;
-use crc32fast::Hasher;
-use std::convert::TryInto;
-use std::io::Write;
-use uuid::Uuid;
-
-/// The size in bytes of a disk sector (also called a block).
-pub const SECTOR_SIZE: u64 = 1 << 9;
-/// The size in bytes on an MBR partition entry.
-const MBR_PARTITION_ENTRY_SIZE: usize = 16;
-/// The size in bytes of a GPT header.
-pub const GPT_HEADER_SIZE: u32 = 92;
-/// The number of partition entries in the GPT, which is the maximum number of partitions which are
-/// supported.
-pub const GPT_NUM_PARTITIONS: u32 = 128;
-/// The size in bytes of a single GPT partition entry.
-pub const GPT_PARTITION_ENTRY_SIZE: u32 = 128;
-/// The size in bytes of everything before the first partition: i.e. the MBR, GPT header and GPT
-/// partition entries.
-pub const GPT_BEGINNING_SIZE: u64 = SECTOR_SIZE * 40;
-/// The size in bytes of everything after the last partition: i.e. the GPT partition entries and GPT
-/// footer.
-pub const GPT_END_SIZE: u64 = SECTOR_SIZE * 33;
-
-/// Write a protective MBR for a disk of the given total size (in bytes).
-///
-/// This should be written at the start of the disk, before the GPT header. It
-/// is one `SECTOR_SIZE` long.
-pub fn write_protective_mbr(file: &mut impl Write, disk_size: u64) -> Result<(), Error> {
-    // Bootstrap code
-    file.write_all(&[0; 446])?;
-
-    // Partition status
-    file.write_all(&[0x00])?;
-    // Begin CHS
-    file.write_all(&[0; 3])?;
-    // Partition type
-    file.write_all(&[0xEE])?;
-    // End CHS
-    file.write_all(&[0; 3])?;
-    let first_lba: u32 = 1;
-    file.write_all(&first_lba.to_le_bytes())?;
-    let number_of_sectors: u32 = (disk_size / SECTOR_SIZE).try_into()?;
-    file.write_all(&number_of_sectors.to_le_bytes())?;
-
-    // Three more empty partitions
-    file.write_all(&[0; MBR_PARTITION_ENTRY_SIZE * 3])?;
-
-    // Boot signature
-    file.write_all(&[0x55, 0xAA])?;
-
-    Ok(())
-}
-
-#[derive(Clone, Debug, Default, Eq, PartialEq)]
-struct GptHeader {
-    signature: [u8; 8],
-    revision: [u8; 4],
-    header_size: u32,
-    header_crc32: u32,
-    current_lba: u64,
-    backup_lba: u64,
-    first_usable_lba: u64,
-    last_usable_lba: u64,
-    disk_guid: Uuid,
-    partition_entries_lba: u64,
-    num_partition_entries: u32,
-    partition_entry_size: u32,
-    partition_entries_crc32: u32,
-}
-
-impl GptHeader {
-    fn write_bytes(&self, out: &mut impl Write) -> Result<(), Error> {
-        out.write_all(&self.signature)?;
-        out.write_all(&self.revision)?;
-        out.write_all(&self.header_size.to_le_bytes())?;
-        out.write_all(&self.header_crc32.to_le_bytes())?;
-        // Reserved
-        out.write_all(&[0; 4])?;
-        out.write_all(&self.current_lba.to_le_bytes())?;
-        out.write_all(&self.backup_lba.to_le_bytes())?;
-        out.write_all(&self.first_usable_lba.to_le_bytes())?;
-        out.write_all(&self.last_usable_lba.to_le_bytes())?;
-
-        // GUID is mixed-endian for some reason, so we can't just use `Uuid::as_bytes()`.
-        write_guid(out, self.disk_guid)?;
-
-        out.write_all(&self.partition_entries_lba.to_le_bytes())?;
-        out.write_all(&self.num_partition_entries.to_le_bytes())?;
-        out.write_all(&self.partition_entry_size.to_le_bytes())?;
-        out.write_all(&self.partition_entries_crc32.to_le_bytes())?;
-        Ok(())
-    }
-}
-
-/// Write a GPT header for the disk.
-///
-/// It may either be a primary header (which should go at LBA 1) or a secondary header (which should
-/// go at the end of the disk).
-pub fn write_gpt_header(
-    out: &mut impl Write,
-    disk_guid: Uuid,
-    partition_entries_crc32: u32,
-    secondary_table_offset: u64,
-    secondary: bool,
-) -> Result<(), Error> {
-    let primary_header_lba = 1;
-    let secondary_header_lba = (secondary_table_offset + GPT_END_SIZE) / SECTOR_SIZE - 1;
-    let mut gpt_header = GptHeader {
-        signature: *b"EFI PART",
-        revision: [0, 0, 1, 0],
-        header_size: GPT_HEADER_SIZE,
-        current_lba: if secondary { secondary_header_lba } else { primary_header_lba },
-        backup_lba: if secondary { primary_header_lba } else { secondary_header_lba },
-        first_usable_lba: GPT_BEGINNING_SIZE / SECTOR_SIZE,
-        last_usable_lba: secondary_table_offset / SECTOR_SIZE - 1,
-        disk_guid,
-        partition_entries_lba: 2,
-        num_partition_entries: GPT_NUM_PARTITIONS,
-        partition_entry_size: GPT_PARTITION_ENTRY_SIZE,
-        partition_entries_crc32,
-        header_crc32: 0,
-    };
-
-    // Write once to a temporary buffer to calculate the CRC.
-    let mut header_without_crc = [0u8; GPT_HEADER_SIZE as usize];
-    gpt_header.write_bytes(&mut &mut header_without_crc[..])?;
-    let mut hasher = Hasher::new();
-    hasher.update(&header_without_crc);
-    gpt_header.header_crc32 = hasher.finalize();
-
-    gpt_header.write_bytes(out)?;
-
-    Ok(())
-}
-
-/// A GPT entry for a particular partition.
-#[derive(Clone, Debug, Eq, PartialEq)]
-pub struct GptPartitionEntry {
-    pub partition_type_guid: Uuid,
-    pub unique_partition_guid: Uuid,
-    pub first_lba: u64,
-    pub last_lba: u64,
-    pub attributes: u64,
-    /// UTF-16LE
-    pub partition_name: [u16; 36],
-}
-
-// TODO: Derive this once arrays of more than 32 elements have default values.
-impl Default for GptPartitionEntry {
-    fn default() -> Self {
-        Self {
-            partition_type_guid: Default::default(),
-            unique_partition_guid: Default::default(),
-            first_lba: 0,
-            last_lba: 0,
-            attributes: 0,
-            partition_name: [0; 36],
-        }
-    }
-}
-
-impl GptPartitionEntry {
-    /// Write out the partition table entry. It will take
-    /// `GPT_PARTITION_ENTRY_SIZE` bytes.
-    pub fn write_bytes(&self, out: &mut impl Write) -> Result<(), Error> {
-        write_guid(out, self.partition_type_guid)?;
-        write_guid(out, self.unique_partition_guid)?;
-        out.write_all(&self.first_lba.to_le_bytes())?;
-        out.write_all(&self.last_lba.to_le_bytes())?;
-        out.write_all(&self.attributes.to_le_bytes())?;
-        for code_unit in &self.partition_name {
-            out.write_all(&code_unit.to_le_bytes())?;
-        }
-        Ok(())
-    }
-}
-
-/// Write a UUID in the mixed-endian format which GPT uses for GUIDs.
-fn write_guid(out: &mut impl Write, guid: Uuid) -> Result<(), Error> {
-    let guid_fields = guid.as_fields();
-    out.write_all(&guid_fields.0.to_le_bytes())?;
-    out.write_all(&guid_fields.1.to_le_bytes())?;
-    out.write_all(&guid_fields.2.to_le_bytes())?;
-    out.write_all(guid_fields.3)?;
-
-    Ok(())
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn protective_mbr_size() {
-        let mut buffer = vec![];
-        write_protective_mbr(&mut buffer, 1000 * SECTOR_SIZE).unwrap();
-
-        assert_eq!(buffer.len(), SECTOR_SIZE as usize);
-    }
-
-    #[test]
-    fn header_size() {
-        let mut buffer = vec![];
-        write_gpt_header(
-            &mut buffer,
-            Uuid::from_u128(0x12345678_1234_5678_abcd_12345678abcd),
-            42,
-            1000 * SECTOR_SIZE,
-            false,
-        )
-        .unwrap();
-
-        assert_eq!(buffer.len(), GPT_HEADER_SIZE as usize);
-    }
-
-    #[test]
-    fn partition_entry_size() {
-        let mut buffer = vec![];
-        GptPartitionEntry::default().write_bytes(&mut buffer).unwrap();
-
-        assert_eq!(buffer.len(), GPT_PARTITION_ENTRY_SIZE as usize);
-    }
-}
diff --git a/virtualizationservice/src/main.rs b/virtualizationservice/src/main.rs
index c9cc029..018be7b 100644
--- a/virtualizationservice/src/main.rs
+++ b/virtualizationservice/src/main.rs
@@ -17,7 +17,6 @@
 mod aidl;
 mod composite;
 mod crosvm;
-mod gpt;
 mod payload;
 
 use crate::aidl::{VirtualizationService, BINDER_SERVICE_IDENTIFIER, TEMPORARY_DIRECTORY};