More API improvements

This attempts to address more of the comments on aosp/2192077.
- Add a message to some of our exceptions.
- Move error checking from build() to the VirtualMachineConfig
  constructor, so we catch any errors when deserializing config too.
- Make negative memory an error. Add @IntRange to memory & number of
  CPUs.
- Check debugLevel is valid.
- Improve various javadoc comments.
- Rename toParcel() to toVsConfig() because it was confusing me.
- Remove the no-args construction for our exception because we don't
  use it and probably never should.

Bug: 243512115
Test: atest MicrodroidTests MicrodroidHostTests
Change-Id: I64c6b9e9f9620e9b93702cd93aedc72d754ba406
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachine.java b/javalib/src/android/system/virtualmachine/VirtualMachine.java
index e2fc33e..9a29218 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachine.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachine.java
@@ -90,8 +90,13 @@
 import java.util.zip.ZipFile;
 
 /**
- * A handle to the virtual machine. The virtual machine is local to the app which created the
- * virtual machine.
+ * Represents an VM instance, with its own configuration and state. Instances are persistent and are
+ * created or retrieved via {@link VirtualMachineManager}.
+ * <p>
+ * The {@link #run} method actually starts up the VM and allows the payload code to execute. It
+ * will continue until it exits or {@link #stop} is called. Updates on the state of the VM can
+ * be received using {@link #setCallback}. The app can communicate with the VM using
+ * {@link #connectToVsockServer} or {@link #connectVsock}.
  *
  * @hide
  */
@@ -483,7 +488,7 @@
         try {
             createVmPipes();
 
-            VirtualMachineAppConfig appConfig = getConfig().toParcel();
+            VirtualMachineAppConfig appConfig = getConfig().toVsConfig();
             appConfig.name = mName;
 
             // Fill the idsig file by hashing the apk
@@ -587,7 +592,7 @@
                 mLogWriter = pipe[1];
             }
         } catch (IOException e) {
-            throw new VirtualMachineException(e);
+            throw new VirtualMachineException("Failed to create stream for VM", e);
         }
     }
 
@@ -735,7 +740,6 @@
      * @throws VirtualMachineException if the virtual machine is not running.
      * @hide
      */
-    @NonNull
     public int getCid() throws VirtualMachineException {
         if (getStatus() != STATUS_RUNNING) {
             throw new VirtualMachineException("VM is not running");
@@ -777,7 +781,7 @@
             newConfig.serialize(output);
             output.close();
         } catch (IOException e) {
-            throw new VirtualMachineException(e);
+            throw new VirtualMachineException("Failed to persist config", e);
         }
         mConfig = newConfig;
 
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java b/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
index 2dff9bb..90b09c8 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
@@ -21,6 +21,7 @@
 import static java.util.Objects.requireNonNull;
 
 import android.annotation.IntDef;
+import android.annotation.IntRange;
 import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.annotation.RequiresPermission;
@@ -39,15 +40,16 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.util.Objects;
+
 /**
  * Represents a configuration of a virtual machine. A configuration consists of hardware
  * configurations like the number of CPUs and the size of RAM, and software configurations like the
- * OS and application to run on the virtual machine.
+ * payload to run on the virtual machine.
  *
  * @hide
  */
 public final class VirtualMachineConfig {
-    // These defines the schema of the config file persisted on disk.
+    // These define the schema of the config file persisted on disk.
     private static final int VERSION = 2;
     private static final String KEY_VERSION = "version";
     private static final String KEY_APKPATH = "apkPath";
@@ -133,6 +135,43 @@
         if (!apkPath.startsWith("/")) {
             throw new IllegalArgumentException("APK path must be an absolute path");
         }
+
+        if (memoryMib < 0) {
+            throw new IllegalArgumentException("Memory size cannot be negative");
+        }
+
+        int availableCpus = Runtime.getRuntime().availableProcessors();
+        if (numCpus < 1 || numCpus > availableCpus) {
+            throw new IllegalArgumentException("Number of vCPUs (" + numCpus + ") is out of "
+                    + "range [1, " + availableCpus + "]");
+        }
+
+        if (debugLevel != DEBUG_LEVEL_NONE && debugLevel != DEBUG_LEVEL_APP_ONLY
+                && debugLevel != DEBUG_LEVEL_FULL) {
+            throw new IllegalArgumentException("Invalid debugLevel: " + debugLevel);
+        }
+
+        if (payloadBinaryPath == null) {
+            if (payloadConfigPath == null) {
+                throw new IllegalStateException("setPayloadBinaryPath must be called");
+            }
+        } else {
+            if (payloadConfigPath != null) {
+                throw new IllegalStateException(
+                        "setPayloadBinaryPath and setPayloadConfigPath may not both be called");
+            }
+        }
+
+        if (protectedVm
+                && !HypervisorProperties.hypervisor_protected_vm_supported().orElse(false)) {
+            throw new UnsupportedOperationException(
+                    "Protected VMs are not supported on this device.");
+        }
+        if (!protectedVm && !HypervisorProperties.hypervisor_vm_supported().orElse(false)) {
+            throw new UnsupportedOperationException(
+                    "Unprotected VMs are not supported on this device.");
+        }
+
         mApkPath = apkPath;
         mPayloadConfigPath = payloadConfigPath;
         mPayloadBinaryPath = payloadBinaryPath;
@@ -177,7 +216,7 @@
     }
 
     /** Persists this config to a stream, for example a file. */
-    /* package */ void serialize(@NonNull OutputStream output) throws IOException {
+    void serialize(@NonNull OutputStream output) throws IOException {
         PersistableBundle b = new PersistableBundle();
         b.putInt(KEY_VERSION, VERSION);
         b.putString(KEY_APKPATH, mApkPath);
@@ -204,7 +243,8 @@
     }
 
     /**
-     * Returns the path to the payload config within the owning application.
+     * Returns the path within the APK to the payload config file that defines software aspects
+     * of the VM.
      *
      * @hide
      */
@@ -214,8 +254,8 @@
     }
 
     /**
-     * Returns the path within the APK to the payload binary file that will be executed within the
-     * VM.
+     * Returns the path within the {@code lib/<ABI>} directory of the APK to the payload binary
+     * file that will be executed within the VM.
      *
      * @hide
      */
@@ -249,6 +289,7 @@
      *
      * @hide
      */
+    @IntRange(from = 0)
     public int getMemoryMib() {
         return mMemoryMib;
     }
@@ -258,6 +299,7 @@
      *
      * @hide
      */
+    @IntRange(from = 1)
     public int getNumCpus() {
         return mNumCpus;
     }
@@ -279,41 +321,42 @@
     }
 
     /**
-     * Converts this config object into a parcel. Used when creating a VM via the virtualization
-     * service. Notice that the files are not passed as paths, but as file descriptors because the
-     * service doesn't accept paths as it might not have permission to open app-owned files and that
-     * could be abused to run a VM with software that the calling application doesn't own.
+     * Converts this config object into the parcelable type used when creating a VM via the
+     * virtualization service. Notice that the files are not passed as paths, but as file
+     * descriptors because the service doesn't accept paths as it might not have permission to open
+     * app-owned files and that could be abused to run a VM with software that the calling
+     * application doesn't own.
      */
-    VirtualMachineAppConfig toParcel() throws FileNotFoundException {
-        VirtualMachineAppConfig parcel = new VirtualMachineAppConfig();
-        parcel.apk = ParcelFileDescriptor.open(new File(mApkPath), MODE_READ_ONLY);
+    VirtualMachineAppConfig toVsConfig() throws FileNotFoundException {
+        VirtualMachineAppConfig vsConfig = new VirtualMachineAppConfig();
+        vsConfig.apk = ParcelFileDescriptor.open(new File(mApkPath), MODE_READ_ONLY);
         if (mPayloadBinaryPath != null) {
             VirtualMachinePayloadConfig payloadConfig = new VirtualMachinePayloadConfig();
             payloadConfig.payloadPath = mPayloadBinaryPath;
-            parcel.payload =
+            vsConfig.payload =
                     VirtualMachineAppConfig.Payload.payloadConfig(payloadConfig);
         } else {
-            parcel.payload =
+            vsConfig.payload =
                     VirtualMachineAppConfig.Payload.configPath(mPayloadConfigPath);
         }
         switch (mDebugLevel) {
             case DEBUG_LEVEL_APP_ONLY:
-                parcel.debugLevel = VirtualMachineAppConfig.DebugLevel.APP_ONLY;
+                vsConfig.debugLevel = VirtualMachineAppConfig.DebugLevel.APP_ONLY;
                 break;
             case DEBUG_LEVEL_FULL:
-                parcel.debugLevel = VirtualMachineAppConfig.DebugLevel.FULL;
+                vsConfig.debugLevel = VirtualMachineAppConfig.DebugLevel.FULL;
                 break;
             default:
-                parcel.debugLevel = VirtualMachineAppConfig.DebugLevel.NONE;
+                vsConfig.debugLevel = VirtualMachineAppConfig.DebugLevel.NONE;
                 break;
         }
-        parcel.protectedVm = mProtectedVm;
-        parcel.memoryMib = mMemoryMib;
-        parcel.numCpus = mNumCpus;
+        vsConfig.protectedVm = mProtectedVm;
+        vsConfig.memoryMib = mMemoryMib;
+        vsConfig.numCpus = mNumCpus;
         // Don't allow apps to set task profiles ... at last for now. Also, don't forget to
         // validate the string because these are appended to the cmdline argument.
-        parcel.taskProfiles = new String[0];
-        return parcel;
+        vsConfig.taskProfiles = new String[0];
+        return vsConfig;
     }
 
     /**
@@ -333,7 +376,7 @@
         private int mNumCpus;
 
         /**
-         * Creates a builder for the given context (APK).
+         * Creates a builder for the given context.
          *
          * @hide
          */
@@ -352,37 +395,10 @@
         public VirtualMachineConfig build() {
             String apkPath = (mApkPath == null) ? mContext.getPackageCodePath() : mApkPath;
 
-            int availableCpus = Runtime.getRuntime().availableProcessors();
-            if (mNumCpus < 1 || mNumCpus > availableCpus) {
-                throw new IllegalArgumentException("Number of vCPUs (" + mNumCpus + ") is out of "
-                        + "range [1, " + availableCpus + "]");
-            }
-
-            if (mPayloadBinaryPath == null) {
-                if (mPayloadConfigPath == null) {
-                    throw new IllegalStateException("payloadBinaryPath must be set");
-                }
-            } else {
-                if (mPayloadConfigPath != null) {
-                    throw new IllegalStateException(
-                            "payloadBinaryPath and payloadConfigPath may not both be set");
-                }
-            }
-
             if (!mProtectedVmSet) {
                 throw new IllegalStateException("setProtectedVm must be called explicitly");
             }
 
-            if (mProtectedVm
-                    && !HypervisorProperties.hypervisor_protected_vm_supported().orElse(false)) {
-                throw new UnsupportedOperationException(
-                        "Protected VMs are not supported on this device.");
-            }
-            if (!mProtectedVm && !HypervisorProperties.hypervisor_vm_supported().orElse(false)) {
-                throw new UnsupportedOperationException(
-                        "Unprotected VMs are not supported on this device.");
-            }
-
             return new VirtualMachineConfig(
                     apkPath, mPayloadConfigPath, mPayloadBinaryPath, mDebugLevel, mProtectedVm,
                     mMemoryMib, mNumCpus);
@@ -402,7 +418,8 @@
 
         /**
          * Sets the path within the APK to the payload config file that defines software aspects
-         * of the VM.
+         * of the VM. The file is a JSON file; see
+         * packages/modules/Virtualization/microdroid/payload/config/src/lib.rs for the format.
          *
          * @hide
          */
@@ -426,7 +443,7 @@
         }
 
         /**
-         * Sets the debug level
+         * Sets the debug level. Defaults to {@link #DEBUG_LEVEL_NONE}.
          *
          * @hide
          */
@@ -451,24 +468,25 @@
         }
 
         /**
-         * Sets the amount of RAM to give the VM. If this is zero or negative then the default will
-         * be used.
+         * Sets the amount of RAM to give the VM, in mebibytes. If zero or not explicitly set
+         * than a default size will be used.
          *
          * @hide
          */
         @NonNull
-        public Builder setMemoryMib(int memoryMib) {
+        public Builder setMemoryMib(@IntRange(from = 0) int memoryMib) {
             mMemoryMib = memoryMib;
             return this;
         }
 
         /**
-         * Sets the number of vCPUs in the VM. Defaults to 1.
+         * Sets the number of vCPUs in the VM. Defaults to 1. Cannot be more than the number of
+         * real CPUs (as returned by {@link Runtime#availableProcessors()}).
          *
          * @hide
          */
         @NonNull
-        public Builder setNumCpus(int num) {
+        public Builder setNumCpus(@IntRange(from = 1) int num) {
             mNumCpus = num;
             return this;
         }
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineException.java b/javalib/src/android/system/virtualmachine/VirtualMachineException.java
index 88b5ea3..828775a 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineException.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineException.java
@@ -24,10 +24,6 @@
  * @hide
  */
 public class VirtualMachineException extends Exception {
-    public VirtualMachineException() {
-        super();
-    }
-
     public VirtualMachineException(@Nullable String message) {
         super(message);
     }
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineManager.java b/javalib/src/android/system/virtualmachine/VirtualMachineManager.java
index 3f904be..a153e26 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineManager.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineManager.java
@@ -33,7 +33,14 @@
 import java.util.WeakHashMap;
 
 /**
- * Manages {@link VirtualMachine} objects created for an application.
+ * Manages {@link VirtualMachine virtual machine} instances created by an app. Each instance is
+ * created from a {@link VirtualMachineConfig configuration} that defines the shape of the VM
+ * (RAM, CPUs), the code to execute within it, etc.
+ * <p>
+ * Each virtual machine instance is named; the configuration and related state of each is
+ * persisted in the app's private data directory and an instance can be retrieved given the name.
+ * <p>
+ * The app can then start, stop and otherwise interact with the VM.
  *
  * @hide
  */