More API changes

- Add a comment on stopping a VM more gracefully.
- Switched to @linkplain in a few places, simplified various link
targets.
- Specify caller must close the connectVsock FD.
- Separate out and improve isCompatibleWIth tests.

Bug: 261037705
Test: atest MicrodroidTests

Change-Id: Ia06350928395c1bb0500a4fd2734b01bf1f9af7d
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachine.java b/javalib/src/android/system/virtualmachine/VirtualMachine.java
index 3295681..beaac18 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachine.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachine.java
@@ -162,7 +162,7 @@
     })
     public @interface Status {}
 
-     /** The virtual machine has just been created, or {@link #stop()} was called on it. */
+    /** The virtual machine has just been created, or {@link #stop} was called on it. */
     public static final int STATUS_STOPPED = 0;
 
     /** The virtual machine is running. */
@@ -588,8 +588,7 @@
      * Returns the currently selected config of this virtual machine. There can be multiple virtual
      * machines sharing the same config. Even in that case, the virtual machines are completely
      * isolated from each other; they have different secrets. It is also possible that a virtual
-     * machine can change its config, which can be done by calling {@link
-     * #setConfig(VirtualMachineConfig)}.
+     * machine can change its config, which can be done by calling {@link #setConfig}.
      *
      * <p>NOTE: This method may block and should not be called on the main thread.
      *
@@ -748,8 +747,7 @@
     /**
      * Runs this virtual machine. The returning of this method however doesn't mean that the VM has
      * actually started running or the OS has booted there. Such events can be notified by
-     * registering a callback using {@link #setCallback(Executor, VirtualMachineCallback)} before
-     * calling {@code run()}.
+     * registering a callback using {@link #setCallback} before calling {@code run()}.
      *
      * <p>NOTE: This method may block and should not be called on the main thread.
      *
@@ -933,7 +931,15 @@
     /**
      * Stops this virtual machine. Stopping a virtual machine is like pulling the plug on a real
      * computer; the machine halts immediately. Software running on the virtual machine is not
-     * notified of the event. A stopped virtual machine can be re-started by calling {@link #run()}.
+     * notified of the event. Writes to {@linkplain
+     * VirtualMachineConfig.Builder#setEncryptedStorageKib encrypted storage} might not be
+     * persisted, and the instance might be left in an inconsistent state.
+     *
+     * <p>For a graceful shutdown, you could request the payload to call {@code exit()}, e.g. via a
+     * {@linkplain #connectToVsockServer binder request}, and wait for {@link
+     * VirtualMachineCallback#onPayloadFinished} to be called.
+     *
+     * <p>A stopped virtual machine can be re-started by calling {@link #run()}.
      *
      * <p>NOTE: This method may block and should not be called on the main thread.
      *
@@ -1016,8 +1022,8 @@
      * like the number of CPU and size of the RAM, depending on the situation (e.g. the size of the
      * application to run on the virtual machine, etc.)
      *
-     * <p>The new config must be {@link VirtualMachineConfig#isCompatibleWith compatible with} the
-     * existing config.
+     * <p>The new config must be {@linkplain VirtualMachineConfig#isCompatibleWith compatible with}
+     * the existing config.
      *
      * <p>NOTE: This method may block and should not be called on the main thread.
      *
@@ -1053,8 +1059,8 @@
     /**
      * Connect to a VM's binder service via vsock and return the root IBinder object. Guest VMs are
      * expected to set up vsock servers in their payload. After the host app receives the {@link
-     * VirtualMachineCallback#onPayloadReady(VirtualMachine)}, it can use this method to establish a
-     * connection to the guest VM.
+     * VirtualMachineCallback#onPayloadReady}, it can use this method to establish a connection to
+     * the guest VM.
      *
      * <p>NOTE: This method may block and should not be called on the main thread.
      *
@@ -1082,6 +1088,8 @@
     /**
      * Opens a vsock connection to the VM on the given port.
      *
+     * <p>The caller is responsible for closing the returned {@code ParcelFileDescriptor}.
+     *
      * <p>NOTE: This method may block and should not be called on the main thread.
      *
      * @throws VirtualMachineException if connecting fails.
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java b/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java
index 9aaecf0..d72ba14 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineCallback.java
@@ -141,8 +141,8 @@
     void onPayloadStarted(@NonNull VirtualMachine vm);
 
     /**
-     * Called when the payload in the VM is ready to serve. See
-     * {@link VirtualMachine#connectToVsockServer(int)}.
+     * Called when the payload in the VM is ready to serve. See {@link
+     * VirtualMachine#connectToVsockServer}.
      */
     void onPayloadReady(@NonNull VirtualMachine vm);
 
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java b/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
index 7555dec..7c16798 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
@@ -587,7 +587,7 @@
 
         /**
          * 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()}).
+         * CPUs (as returned by {@link Runtime#availableProcessors}).
          *
          * @hide
          */
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineDescriptor.java b/javalib/src/android/system/virtualmachine/VirtualMachineDescriptor.java
index c9718aa..483779a 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineDescriptor.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineDescriptor.java
@@ -29,7 +29,7 @@
  * A VM descriptor that captures the state of a Virtual Machine.
  *
  * <p>You can capture the current state of VM by creating an instance of this class with {@link
- * VirtualMachine#toDescriptor()}, optionally pass it to another App, and then build an identical VM
+ * VirtualMachine#toDescriptor}, optionally pass it to another App, and then build an identical VM
  * with the descriptor received.
  *
  * @hide
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineManager.java b/javalib/src/android/system/virtualmachine/VirtualMachineManager.java
index 6aa8133..7773cb5 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineManager.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineManager.java
@@ -38,9 +38,9 @@
 import java.util.Map;
 
 /**
- * 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.
+ * Manages {@linkplain VirtualMachine virtual machine} instances created by an app. Each instance is
+ * created from a {@linkplain 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.
diff --git a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
index 897879b..2656c3b 100644
--- a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
+++ b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
@@ -20,6 +20,8 @@
 import static android.system.virtualmachine.VirtualMachine.STATUS_STOPPED;
 import static android.system.virtualmachine.VirtualMachineConfig.DEBUG_LEVEL_FULL;
 import static android.system.virtualmachine.VirtualMachineConfig.DEBUG_LEVEL_NONE;
+import static android.system.virtualmachine.VirtualMachineManager.CAPABILITY_NON_PROTECTED_VM;
+import static android.system.virtualmachine.VirtualMachineManager.CAPABILITY_PROTECTED_VM;
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.TruthJUnit.assume;
@@ -48,6 +50,8 @@
 import com.android.microdroid.test.device.MicrodroidDeviceTestBase;
 import com.android.microdroid.testservice.ITestService;
 
+import com.google.common.truth.BooleanSubject;
+
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Ignore;
@@ -314,8 +318,8 @@
 
     @Test
     @CddTest(requirements = {"9.17/C-1-1"})
-    public void vmConfigUnitTests() {
-
+    public void vmConfigGetAndSetTests() {
+        // Minimal has as little as specified as possible; everything that can be is defaulted.
         VirtualMachineConfig.Builder minimalBuilder = newVmConfigBuilder();
         VirtualMachineConfig minimal = minimalBuilder.setPayloadBinaryPath("binary/path").build();
 
@@ -329,6 +333,8 @@
         assertThat(minimal.isEncryptedStorageEnabled()).isFalse();
         assertThat(minimal.getEncryptedStorageKib()).isEqualTo(0);
 
+        // Maximal has everything that can be set to some non-default value. (And has different
+        // values than minimal for the required fields.)
         int maxCpus = Runtime.getRuntime().availableProcessors();
         VirtualMachineConfig.Builder maximalBuilder =
                 newVmConfigBuilder()
@@ -353,18 +359,11 @@
         assertThat(minimal.isCompatibleWith(maximal)).isFalse();
         assertThat(minimal.isCompatibleWith(minimal)).isTrue();
         assertThat(maximal.isCompatibleWith(maximal)).isTrue();
-
-        VirtualMachineConfig compatible = maximalBuilder.setNumCpus(1).setMemoryMib(99).build();
-        assertThat(compatible.isCompatibleWith(maximal)).isTrue();
-
-        // Assert that different encrypted storage size would imply the configs are incompatible
-        VirtualMachineConfig incompatible = minimalBuilder.setEncryptedStorageKib(1048).build();
-        assertThat(incompatible.isCompatibleWith(minimal)).isFalse();
     }
 
     @Test
     @CddTest(requirements = {"9.17/C-1-1"})
-    public void vmConfigBuilderUnitTests() {
+    public void vmConfigBuilderValidationTests() {
         VirtualMachineConfig.Builder builder = newVmConfigBuilder();
 
         // All your null are belong to me.
@@ -395,6 +394,44 @@
 
     @Test
     @CddTest(requirements = {"9.17/C-1-1"})
+    public void compatibleConfigTests() throws Exception {
+        int maxCpus = Runtime.getRuntime().availableProcessors();
+
+        VirtualMachineConfig.Builder builder =
+                newVmConfigBuilder().setPayloadBinaryPath("binary/path").setApkPath("/apk/path");
+        VirtualMachineConfig baseline = builder.build();
+
+        // A config must be compatible with itself
+        assertConfigCompatible(baseline, builder).isTrue();
+
+        // Changes that must always be compatible
+        assertConfigCompatible(baseline, builder.setMemoryMib(99)).isTrue();
+        if (maxCpus > 1) {
+            assertConfigCompatible(baseline, builder.setNumCpus(2)).isTrue();
+        }
+
+        // Changes that must be incompatible, since they must change the VM identity.
+        assertConfigCompatible(baseline, builder.setDebugLevel(DEBUG_LEVEL_FULL)).isFalse();
+        assertConfigCompatible(baseline, builder.setPayloadBinaryPath("different")).isFalse();
+        int capabilities = getVirtualMachineManager().getCapabilities();
+        if ((capabilities & CAPABILITY_PROTECTED_VM) != 0
+                && (capabilities & CAPABILITY_NON_PROTECTED_VM) != 0) {
+            assertConfigCompatible(baseline, builder.setProtectedVm(!isProtectedVm())).isFalse();
+        }
+
+        // Changes that are currently incompatible for ease of implementation, but this might change
+        // in the future.
+        assertConfigCompatible(baseline, builder.setApkPath("/different")).isFalse();
+        assertConfigCompatible(baseline, builder.setEncryptedStorageKib(100)).isFalse();
+    }
+
+    private BooleanSubject assertConfigCompatible(
+            VirtualMachineConfig baseline, VirtualMachineConfig.Builder builder) {
+        return assertThat(builder.build().isCompatibleWith(baseline));
+    }
+
+    @Test
+    @CddTest(requirements = {"9.17/C-1-1"})
     public void vmUnitTests() throws Exception {
         VirtualMachineConfig.Builder builder =
                 newVmConfigBuilder().setPayloadBinaryPath("binary/path");