Merge "Speed up AuthFsHostTest by reusing the VM"
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachine.java b/javalib/src/android/system/virtualmachine/VirtualMachine.java
index 522651b..c46bb2b 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachine.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachine.java
@@ -18,6 +18,8 @@
 
 import static android.os.ParcelFileDescriptor.MODE_READ_WRITE;
 
+import android.annotation.NonNull;
+import android.annotation.Nullable;
 import android.content.Context;
 import android.os.IBinder;
 import android.os.ParcelFileDescriptor;
@@ -71,35 +73,36 @@
     }
 
     /** The package which owns this VM. */
-    private final String mPackageName;
+    private final @NonNull String mPackageName;
 
     /** Name of this VM within the package. The name should be unique in the package. */
-    private final String mName;
+    private final @NonNull String mName;
 
     /**
      * Path to the config file for this VM. The config file is where the configuration is persisted.
      */
-    private final File mConfigFilePath;
+    private final @NonNull File mConfigFilePath;
 
     /** Path to the instance image file for this VM. */
-    private final File mInstanceFilePath;
+    private final @NonNull File mInstanceFilePath;
 
     /** Size of the instance image. 10 MB. */
     private static final long INSTANCE_FILE_SIZE = 10 * 1024 * 1024;
 
     /** The configuration that is currently associated with this VM. */
-    private VirtualMachineConfig mConfig;
+    private @NonNull VirtualMachineConfig mConfig;
 
     /** Handle to the "running" VM. */
-    private IVirtualMachine mVirtualMachine;
+    private @Nullable IVirtualMachine mVirtualMachine;
 
     /** The registered callback */
-    private VirtualMachineCallback mCallback;
+    private @Nullable VirtualMachineCallback mCallback;
 
-    private ParcelFileDescriptor mConsoleReader;
-    private ParcelFileDescriptor mConsoleWriter;
+    private @Nullable ParcelFileDescriptor mConsoleReader;
+    private @Nullable ParcelFileDescriptor mConsoleWriter;
 
-    private VirtualMachine(Context context, String name, VirtualMachineConfig config) {
+    private VirtualMachine(
+            @NonNull Context context, @NonNull String name, @NonNull VirtualMachineConfig config) {
         mPackageName = context.getPackageName();
         mName = name;
         mConfig = config;
@@ -115,8 +118,8 @@
      * it is persisted until it is deleted by calling {@link #delete()}. The created virtual machine
      * is in {@link #STOPPED} state. To run the VM, call {@link #run()}.
      */
-    /* package */ static VirtualMachine create(
-            Context context, String name, VirtualMachineConfig config)
+    /* package */ static @NonNull VirtualMachine create(
+            @NonNull Context context, @NonNull String name, @NonNull VirtualMachineConfig config)
             throws VirtualMachineException {
         if (config == null) {
             throw new VirtualMachineException("null config");
@@ -165,8 +168,8 @@
     }
 
     /** Loads a virtual machine that is already created before. */
-    /* package */ static VirtualMachine load(Context context, String name)
-            throws VirtualMachineException {
+    /* package */ static @NonNull VirtualMachine load(
+            @NonNull Context context, @NonNull String name) throws VirtualMachineException {
         VirtualMachine vm = new VirtualMachine(context, name, /* config */ null);
 
         try (FileInputStream input = new FileInputStream(vm.mConfigFilePath)) {
@@ -193,7 +196,7 @@
      * Returns the name of this virtual machine. The name is unique in the package and can't be
      * changed.
      */
-    public String getName() {
+    public @NonNull String getName() {
         return mName;
     }
 
@@ -204,12 +207,12 @@
      * share the same config. It is also possible that a virtual machine can switch its config,
      * which can be done by calling {@link #setConfig(VirtualMachineCOnfig)}.
      */
-    public VirtualMachineConfig getConfig() {
+    public @NonNull VirtualMachineConfig getConfig() {
         return mConfig;
     }
 
     /** Returns the current status of this virtual machine. */
-    public Status getStatus() throws VirtualMachineException {
+    public @NonNull Status getStatus() throws VirtualMachineException {
         try {
             if (mVirtualMachine != null && mVirtualMachine.isRunning()) {
                 return Status.RUNNING;
@@ -227,12 +230,12 @@
      * Registers the callback object to get events from the virtual machine. If a callback was
      * already registered, it is replaced with the new one.
      */
-    public void setCallback(VirtualMachineCallback callback) {
+    public void setCallback(@Nullable VirtualMachineCallback callback) {
         mCallback = callback;
     }
 
     /** Returns the currently registered callback. */
-    public VirtualMachineCallback getCallback() {
+    public @Nullable VirtualMachineCallback getCallback() {
         return mCallback;
     }
 
@@ -304,7 +307,7 @@
     }
 
     /** Returns the stream object representing the console output from the virtual machine. */
-    public InputStream getConsoleOutputStream() throws VirtualMachineException {
+    public @NonNull InputStream getConsoleOutputStream() throws VirtualMachineException {
         if (mConsoleReader == null) {
             throw new VirtualMachineException("Console output not available");
         }
@@ -339,7 +342,7 @@
     }
 
     /** Returns the CID of this virtual machine, if it is running. */
-    public Optional<Integer> getCid() throws VirtualMachineException {
+    public @NonNull Optional<Integer> getCid() throws VirtualMachineException {
         if (getStatus() != Status.RUNNING) {
             return Optional.empty();
         }
@@ -361,7 +364,7 @@
      *
      * @return the old config
      */
-    public VirtualMachineConfig setConfig(VirtualMachineConfig newConfig)
+    public @NonNull VirtualMachineConfig setConfig(@NonNull VirtualMachineConfig newConfig)
             throws VirtualMachineException {
         final VirtualMachineConfig oldConfig = getConfig();
         if (!oldConfig.isCompatibleWith(newConfig)) {
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java b/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java
index 0267de8..07af4a1 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java
@@ -16,6 +16,7 @@
 
 package android.system.virtualmachine;
 
+import android.annotation.NonNull;
 import android.os.ParcelFileDescriptor;
 
 /**
@@ -27,8 +28,8 @@
 public interface VirtualMachineCallback {
 
     /** Called when the payload starts in the VM. */
-    void onPayloadStarted(VirtualMachine vm, ParcelFileDescriptor stdout);
+    void onPayloadStarted(@NonNull VirtualMachine vm, @NonNull ParcelFileDescriptor stdout);
 
     /** Called when the VM died. */
-    void onDied(VirtualMachine vm);
+    void onDied(@NonNull VirtualMachine vm);
 }
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java b/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
index f0e1ce6..21e1a46 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
@@ -18,6 +18,7 @@
 
 import static android.os.ParcelFileDescriptor.MODE_READ_ONLY;
 
+import android.annotation.NonNull;
 import android.content.Context;
 import android.content.pm.PackageManager;
 import android.content.pm.Signature; // This actually is certificate!
@@ -52,23 +53,23 @@
     private static final String KEY_DEBUGMODE = "debugMode";
 
     // Paths to the APK and its idsig file of this application.
-    private final String mApkPath;
-    private final Signature[] mCerts;
-    private final String mIdsigPath;
+    private final @NonNull String mApkPath;
+    private final @NonNull Signature[] mCerts;
+    private final @NonNull String mIdsigPath;
     private final boolean mDebugMode;
 
     /**
      * Path within the APK to the payload config file that defines software aspects of this config.
      */
-    private final String mPayloadConfigPath;
+    private final @NonNull String mPayloadConfigPath;
 
     // TODO(jiyong): add more items like # of cpu, size of ram, debuggability, etc.
 
     private VirtualMachineConfig(
-            String apkPath,
-            Signature[] certs,
-            String idsigPath,
-            String payloadConfigPath,
+            @NonNull String apkPath,
+            @NonNull Signature[] certs,
+            @NonNull String idsigPath,
+            @NonNull String payloadConfigPath,
             boolean debugMode) {
         mApkPath = apkPath;
         mCerts = certs;
@@ -78,7 +79,7 @@
     }
 
     /** Loads a config from a stream, for example a file. */
-    /* package */ static VirtualMachineConfig from(InputStream input)
+    /* package */ static @NonNull VirtualMachineConfig from(@NonNull InputStream input)
             throws IOException, VirtualMachineException {
         PersistableBundle b = PersistableBundle.readFromStream(input);
         final int version = b.getInt(KEY_VERSION);
@@ -111,7 +112,7 @@
     }
 
     /** Persists this config to a stream, for example a file. */
-    /* package */ void serialize(OutputStream output) throws IOException {
+    /* package */ void serialize(@NonNull OutputStream output) throws IOException {
         PersistableBundle b = new PersistableBundle();
         b.putInt(KEY_VERSION, VERSION);
         b.putString(KEY_APKPATH, mApkPath);
@@ -128,7 +129,7 @@
     }
 
     /** Returns the path to the payload config within the owning application. */
-    public String getPayloadConfigPath() {
+    public @NonNull String getPayloadConfigPath() {
         return mPayloadConfigPath;
     }
 
@@ -139,7 +140,7 @@
      * signed by the same signer. All other changes (e.g. using a payload from a different signer,
      * change of the debug mode, etc.) are considered as incompatible.
      */
-    public boolean isCompatibleWith(VirtualMachineConfig other) {
+    public boolean isCompatibleWith(@NonNull VirtualMachineConfig other) {
         if (!Arrays.equals(this.mCerts, other.mCerts)) {
             return false;
         }
@@ -173,7 +174,7 @@
         // TODO(jiyong): add more items like # of cpu, size of ram, debuggability, etc.
 
         /** Creates a builder for the given context (APK), and the payload config file in APK. */
-        public Builder(Context context, String payloadConfigPath) {
+        public Builder(@NonNull Context context, @NonNull String payloadConfigPath) {
             mContext = context;
             mPayloadConfigPath = payloadConfigPath;
             mDebugMode = false;
@@ -188,13 +189,13 @@
         // TODO(jiyong): remove this. Apps shouldn't need to set the path to the idsig file. It
         // should be automatically found or created on demand.
         /** Set the path to the idsig file for the current application. */
-        public Builder idsigPath(String idsigPath) {
+        public Builder idsigPath(@NonNull String idsigPath) {
             mIdsigPath = idsigPath;
             return this;
         }
 
         /** Builds an immutable {@link VirtualMachineConfig} */
-        public VirtualMachineConfig build() {
+        public @NonNull VirtualMachineConfig build() {
             final String apkPath = mContext.getPackageCodePath();
             final String packageName = mContext.getPackageName();
             Signature[] certs;
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineManager.java b/javalib/src/android/system/virtualmachine/VirtualMachineManager.java
index 317caee..3654886 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineManager.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineManager.java
@@ -16,6 +16,7 @@
 
 package android.system.virtualmachine;
 
+import android.annotation.NonNull;
 import android.content.Context;
 
 import java.lang.ref.WeakReference;
@@ -28,16 +29,16 @@
  * @hide
  */
 public class VirtualMachineManager {
-    private final Context mContext;
+    private final @NonNull Context mContext;
 
-    private VirtualMachineManager(Context context) {
+    private VirtualMachineManager(@NonNull Context context) {
         mContext = context;
     }
 
     static Map<Context, WeakReference<VirtualMachineManager>> sInstances = new WeakHashMap<>();
 
     /** Returns the per-context instance. */
-    public static VirtualMachineManager getInstance(Context context) {
+    public static @NonNull VirtualMachineManager getInstance(@NonNull Context context) {
         synchronized (sInstances) {
             VirtualMachineManager vmm =
                     sInstances.containsKey(context) ? sInstances.get(context).get() : null;
@@ -59,7 +60,8 @@
      * new (and different) virtual machine even if the name and the config are the same as the
      * deleted one.
      */
-    public VirtualMachine create(String name, VirtualMachineConfig config)
+    public @NonNull VirtualMachine create(
+            @NonNull String name, @NonNull VirtualMachineConfig config)
             throws VirtualMachineException {
         synchronized (sCreateLock) {
             return VirtualMachine.create(mContext, name, config);
@@ -70,31 +72,24 @@
      * Returns an existing {@link VirtualMachine} with the given name. Returns null if there is no
      * such virtual machine.
      */
-    public VirtualMachine get(String name) throws VirtualMachineException {
+    public @NonNull VirtualMachine get(@NonNull String name) throws VirtualMachineException {
         return VirtualMachine.load(mContext, name);
     }
 
     /**
-     * Returns an existing {@link VirtualMachine} if it exists, or create a new one. If the virtual
-     * machine exists, and config is not null, the virtual machine is re-configured with the new
-     * config. However, if the config is not compatible with the original config of the virtual
-     * machine, exception is thrown.
+     * Returns an existing {@link VirtualMachine} if it exists, or create a new one. The config
+     * parameter is used only when a new virtual machine is created.
      */
-    public VirtualMachine getOrCreate(String name, VirtualMachineConfig config)
+    public @NonNull VirtualMachine getOrCreate(
+            @NonNull String name, @NonNull VirtualMachineConfig config)
             throws VirtualMachineException {
         VirtualMachine vm;
         synchronized (sCreateLock) {
             vm = get(name);
             if (vm == null) {
-                return create(name, config);
+                vm = create(name, config);
             }
         }
-
-        if (config != null) {
-            // Can throw VirtualMachineException is the new config is not compatible with the
-            // old config.
-            vm.setConfig(config);
-        }
         return vm;
     }
 }
diff --git a/virtualizationservice/Android.bp b/virtualizationservice/Android.bp
index a1dba43..239d729 100644
--- a/virtualizationservice/Android.bp
+++ b/virtualizationservice/Android.bp
@@ -33,8 +33,8 @@
         "libonce_cell",
         "libprotobuf",
         "libprotos",
-        "libregex",
         "libserde_json",
+        "libserde_xml_rs",
         "libserde",
         "libshared_child",
         "libuuid",
diff --git a/virtualizationservice/src/payload.rs b/virtualizationservice/src/payload.rs
index 76c55de..1df537c 100644
--- a/virtualizationservice/src/payload.rs
+++ b/virtualizationservice/src/payload.rs
@@ -16,27 +16,32 @@
 
 use crate::composite::align_to_partition_size;
 
-use anyhow::{anyhow, bail, Result};
+use anyhow::{anyhow, Context, Result};
 use microdroid_metadata::{ApexPayload, ApkPayload, Metadata};
 use microdroid_payload_config::ApexConfig;
 use once_cell::sync::OnceCell;
-use regex::Regex;
+use serde::Deserialize;
+use serde_xml_rs::from_reader;
 use std::fs;
-use std::fs::OpenOptions;
-use std::io::{BufRead, BufReader, Seek, SeekFrom, Write};
+use std::fs::{File, OpenOptions};
+use std::io::{Seek, SeekFrom, Write};
 use std::path::{Path, PathBuf};
-use std::process::Command;
 use vmconfig::{DiskImage, Partition};
 
+const APEX_INFO_LIST_PATH: &str = "/apex/apex-info-list.xml";
+
 /// Represents the list of APEXes
-#[derive(Debug)]
+#[derive(Debug, Deserialize)]
 struct ApexInfoList {
+    #[serde(rename = "apex-info")]
     list: Vec<ApexInfo>,
 }
 
-#[derive(Debug)]
+#[derive(Debug, Deserialize)]
 struct ApexInfo {
+    #[serde(rename = "moduleName")]
     name: String,
+    #[serde(rename = "modulePath")]
     path: PathBuf,
 }
 
@@ -45,34 +50,11 @@
     fn load() -> Result<&'static ApexInfoList> {
         static INSTANCE: OnceCell<ApexInfoList> = OnceCell::new();
         INSTANCE.get_or_try_init(|| {
-            // TODO(b/191601801): look up /apex/apex-info-list.xml instead of apexservice
-            // Each APEX prints the line:
-            //   Module: <...> Version: <...> VersionName: <...> Path: <...> IsActive: <...> IsFactory: <...>
-            // We only care about "Module:" and "Path:" tagged values for now.
-            let info_pattern =
-                Regex::new(r"^Module: (?P<name>[^ ]*) .* Path: (?P<path>[^ ]*) .*$")?;
-            let output = Command::new("cmd")
-                .arg("-w")
-                .arg("apexservice")
-                .arg("getActivePackages")
-                .output()
-                .expect("failed to execute apexservice cmd");
-            let list = BufReader::new(output.stdout.as_slice())
-                .lines()
-                .map(|line| -> Result<ApexInfo> {
-                    let line = line?;
-                    let captures = info_pattern
-                        .captures(&line)
-                        .ok_or_else(|| anyhow!("can't parse: {}", line))?;
-                    let name = captures.name("name").unwrap();
-                    let path = captures.name("path").unwrap();
-                    Ok(ApexInfo { name: name.as_str().to_owned(), path: path.as_str().into() })
-                })
-                .collect::<Result<Vec<ApexInfo>>>()?;
-            if list.is_empty() {
-                bail!("failed to load apex info: empty");
-            }
-            Ok(ApexInfoList { list })
+            let apex_info_list = File::open(APEX_INFO_LIST_PATH)
+                .context(format!("Failed to open {}", APEX_INFO_LIST_PATH))?;
+            let apex_info_list: ApexInfoList = from_reader(apex_info_list)
+                .context(format!("Failed to parse {}", APEX_INFO_LIST_PATH))?;
+            Ok(apex_info_list)
         })
     }