Merge "Move config validation to the builder"
diff --git a/javalib/api/system-current.txt b/javalib/api/system-current.txt
index c98c62e..c214f1f 100644
--- a/javalib/api/system-current.txt
+++ b/javalib/api/system-current.txt
@@ -74,8 +74,8 @@
method @NonNull public android.system.virtualmachine.VirtualMachineConfig build();
method @NonNull public android.system.virtualmachine.VirtualMachineConfig.Builder setApkPath(@NonNull String);
method @NonNull public android.system.virtualmachine.VirtualMachineConfig.Builder setDebugLevel(int);
- method @NonNull public android.system.virtualmachine.VirtualMachineConfig.Builder setEncryptedStorageKib(@IntRange(from=0) long);
- method @NonNull public android.system.virtualmachine.VirtualMachineConfig.Builder setMemoryMib(@IntRange(from=0) int);
+ method @NonNull public android.system.virtualmachine.VirtualMachineConfig.Builder setEncryptedStorageKib(@IntRange(from=1) long);
+ method @NonNull public android.system.virtualmachine.VirtualMachineConfig.Builder setMemoryMib(@IntRange(from=1) int);
method @NonNull public android.system.virtualmachine.VirtualMachineConfig.Builder setNumCpus(@IntRange(from=1) int);
method @NonNull public android.system.virtualmachine.VirtualMachineConfig.Builder setPayloadBinaryPath(@NonNull String);
method @NonNull public android.system.virtualmachine.VirtualMachineConfig.Builder setProtectedVm(boolean);
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java b/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
index 459b614..602f8b8 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachineConfig.java
@@ -55,6 +55,8 @@
*/
@SystemApi
public final class VirtualMachineConfig {
+ private static final String[] EMPTY_STRING_ARRAY = {};
+
// These define the schema of the config file persisted on disk.
private static final int VERSION = 2;
private static final String KEY_VERSION = "version";
@@ -133,49 +135,7 @@
int memoryMib,
int numCpus,
long encryptedStorageKib) {
- requireNonNull(apkPath);
- 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_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.");
- }
- if (encryptedStorageKib < 0) {
- throw new IllegalArgumentException("Encrypted Storage size cannot be negative");
- }
-
+ // This is only called from Builder.build(); the builder handles parameter validation.
mApkPath = apkPath;
mPayloadConfigPath = payloadConfigPath;
mPayloadBinaryPath = payloadBinaryPath;
@@ -212,40 +172,48 @@
private static VirtualMachineConfig fromInputStream(@NonNull InputStream input)
throws IOException, VirtualMachineException {
PersistableBundle b = PersistableBundle.readFromStream(input);
+ try {
+ return fromPersistableBundle(b);
+ } catch (NullPointerException | IllegalArgumentException | IllegalStateException e) {
+ throw new VirtualMachineException("Persisted VM config is invalid", e);
+ }
+ }
+
+ @NonNull
+ private static VirtualMachineConfig fromPersistableBundle(PersistableBundle b) {
int version = b.getInt(KEY_VERSION);
if (version > VERSION) {
- throw new VirtualMachineException("Version too high");
+ throw new IllegalArgumentException(
+ "Version " + version + " too high; current is " + VERSION);
}
- String apkPath = b.getString(KEY_APKPATH);
- if (apkPath == null) {
- throw new VirtualMachineException("No apkPath");
+
+ Builder builder = new Builder();
+ builder.setApkPath(b.getString(KEY_APKPATH));
+
+ String payloadConfigPath = b.getString(KEY_PAYLOADCONFIGPATH);
+ if (payloadConfigPath == null) {
+ builder.setPayloadBinaryPath(b.getString(KEY_PAYLOADBINARYPATH));
+ } else {
+ builder.setPayloadConfigPath(payloadConfigPath);
}
- String payloadBinaryPath = b.getString(KEY_PAYLOADBINARYPATH);
- String payloadConfigPath = null;
- if (payloadBinaryPath == null) {
- payloadConfigPath = b.getString(KEY_PAYLOADCONFIGPATH);
- if (payloadConfigPath == null) {
- throw new VirtualMachineException("No payloadBinaryPath");
- }
- }
+
@DebugLevel int debugLevel = b.getInt(KEY_DEBUGLEVEL);
if (debugLevel != DEBUG_LEVEL_NONE && debugLevel != DEBUG_LEVEL_FULL) {
- throw new VirtualMachineException("Invalid debugLevel: " + debugLevel);
+ throw new IllegalArgumentException("Invalid debugLevel: " + debugLevel);
}
- boolean protectedVm = b.getBoolean(KEY_PROTECTED_VM);
+ builder.setDebugLevel(debugLevel);
+ builder.setProtectedVm(b.getBoolean(KEY_PROTECTED_VM));
int memoryMib = b.getInt(KEY_MEMORY_MIB);
- int numCpus = b.getInt(KEY_NUM_CPUS);
+ if (memoryMib != 0) {
+ builder.setMemoryMib(memoryMib);
+ }
+ builder.setNumCpus(b.getInt(KEY_NUM_CPUS));
long encryptedStorageKib = b.getLong(KEY_ENCRYPTED_STORAGE_KIB);
+ if (encryptedStorageKib != 0) {
+ builder.setEncryptedStorageKib(encryptedStorageKib);
+ }
- return new VirtualMachineConfig(
- apkPath,
- payloadConfigPath,
- payloadBinaryPath,
- debugLevel,
- protectedVm,
- memoryMib,
- numCpus,
- encryptedStorageKib);
+ return builder.build();
}
/** Persists this config to a file. */
@@ -335,7 +303,8 @@
}
/**
- * Returns the amount of RAM that will be made available to the VM.
+ * Returns the amount of RAM that will be made available to the VM, or 0 if the default size
+ * will be used.
*
* @hide
*/
@@ -426,9 +395,8 @@
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.
- vsConfig.taskProfiles = new String[0];
+ // Don't allow apps to set task profiles ... at least for now.
+ vsConfig.taskProfiles = EMPTY_STRING_ARRAY;
return vsConfig;
}
@@ -439,15 +407,15 @@
*/
@SystemApi
public static final class Builder {
- private final Context mContext;
+ @Nullable private final Context mContext;
@Nullable private String mApkPath;
@Nullable private String mPayloadConfigPath;
@Nullable private String mPayloadBinaryPath;
- @DebugLevel private int mDebugLevel;
+ @DebugLevel private int mDebugLevel = DEBUG_LEVEL_NONE;
private boolean mProtectedVm;
private boolean mProtectedVmSet;
private int mMemoryMib;
- private int mNumCpus;
+ private int mNumCpus = 1;
private long mEncryptedStorageKib;
/**
@@ -458,9 +426,14 @@
@SystemApi
public Builder(@NonNull Context context) {
mContext = requireNonNull(context, "context must not be null");
- mDebugLevel = DEBUG_LEVEL_NONE;
- mNumCpus = 1;
- mEncryptedStorageKib = 0;
+ }
+
+ /**
+ * Creates a builder with no associated context; {@link #setApkPath} must be called to
+ * specify which APK contains the payload.
+ */
+ private Builder() {
+ mContext = null;
}
/**
@@ -471,7 +444,26 @@
@SystemApi
@NonNull
public VirtualMachineConfig build() {
- String apkPath = (mApkPath == null) ? mContext.getPackageCodePath() : mApkPath;
+ String apkPath;
+ if (mApkPath == null) {
+ if (mContext == null) {
+ throw new IllegalStateException("apkPath must be specified");
+ }
+ apkPath = mContext.getPackageCodePath();
+ } else {
+ apkPath = mApkPath;
+ }
+
+ if (mPayloadBinaryPath == null) {
+ if (mPayloadConfigPath == null) {
+ throw new IllegalStateException("setPayloadBinaryPath must be called");
+ }
+ } else {
+ if (mPayloadConfigPath != null) {
+ throw new IllegalStateException(
+ "setPayloadBinaryPath and setPayloadConfigPath may not both be called");
+ }
+ }
if (!mProtectedVmSet) {
throw new IllegalStateException("setProtectedVm must be called explicitly");
@@ -497,7 +489,11 @@
@SystemApi
@NonNull
public Builder setApkPath(@NonNull String apkPath) {
- mApkPath = requireNonNull(apkPath);
+ requireNonNull(apkPath, "apkPath must not be null");
+ if (!apkPath.startsWith("/")) {
+ throw new IllegalArgumentException("APK path must be an absolute path");
+ }
+ mApkPath = apkPath;
return this;
}
@@ -512,7 +508,8 @@
@TestApi
@NonNull
public Builder setPayloadConfigPath(@NonNull String payloadConfigPath) {
- mPayloadConfigPath = requireNonNull(payloadConfigPath);
+ mPayloadConfigPath =
+ requireNonNull(payloadConfigPath, "payloadConfigPath must not be null");
return this;
}
@@ -525,7 +522,8 @@
@SystemApi
@NonNull
public Builder setPayloadBinaryPath(@NonNull String payloadBinaryPath) {
- mPayloadBinaryPath = requireNonNull(payloadBinaryPath);
+ mPayloadBinaryPath =
+ requireNonNull(payloadBinaryPath, "payloadBinaryPath must not be null");
return this;
}
@@ -537,6 +535,9 @@
@SystemApi
@NonNull
public Builder setDebugLevel(@DebugLevel int debugLevel) {
+ if (debugLevel != DEBUG_LEVEL_NONE && debugLevel != DEBUG_LEVEL_FULL) {
+ throw new IllegalArgumentException("Invalid debugLevel: " + debugLevel);
+ }
mDebugLevel = debugLevel;
return this;
}
@@ -551,20 +552,34 @@
@SystemApi
@NonNull
public Builder setProtectedVm(boolean protectedVm) {
+ if (protectedVm) {
+ if (!HypervisorProperties.hypervisor_protected_vm_supported().orElse(false)) {
+ throw new UnsupportedOperationException(
+ "Protected VMs are not supported on this device.");
+ }
+ } else {
+ if (!HypervisorProperties.hypervisor_vm_supported().orElse(false)) {
+ throw new UnsupportedOperationException(
+ "Unprotected VMs are not supported on this device.");
+ }
+ }
mProtectedVm = protectedVm;
mProtectedVmSet = true;
return this;
}
/**
- * Sets the amount of RAM to give the VM, in mebibytes. If zero or not explicitly set then a
- * default size will be used.
+ * Sets the amount of RAM to give the VM, in mebibytes. If not explicitly set then a default
+ * size will be used.
*
* @hide
*/
@SystemApi
@NonNull
- public Builder setMemoryMib(@IntRange(from = 0) int memoryMib) {
+ public Builder setMemoryMib(@IntRange(from = 1) int memoryMib) {
+ if (memoryMib <= 0) {
+ throw new IllegalArgumentException("Memory size must be positive");
+ }
mMemoryMib = memoryMib;
return this;
}
@@ -577,13 +592,24 @@
*/
@SystemApi
@NonNull
- public Builder setNumCpus(@IntRange(from = 1) int num) {
- mNumCpus = num;
+ public Builder setNumCpus(@IntRange(from = 1) int numCpus) {
+ int availableCpus = Runtime.getRuntime().availableProcessors();
+ if (numCpus < 1 || numCpus > availableCpus) {
+ throw new IllegalArgumentException(
+ "Number of vCPUs ("
+ + numCpus
+ + ") is out of "
+ + "range [1, "
+ + availableCpus
+ + "]");
+ }
+ mNumCpus = numCpus;
return this;
}
/**
- * Sets the size (in KB) of encrypted storage available to the VM.
+ * Sets the size (in KB) of encrypted storage available to the VM. If not set, no encrypted
+ * storage is provided.
*
* <p>The storage is encrypted with a key deterministically derived from the VM identity
*
@@ -599,7 +625,10 @@
*/
@SystemApi
@NonNull
- public Builder setEncryptedStorageKib(@IntRange(from = 0) long encryptedStorageKib) {
+ public Builder setEncryptedStorageKib(@IntRange(from = 1) long encryptedStorageKib) {
+ if (encryptedStorageKib <= 0) {
+ throw new IllegalArgumentException("Encrypted Storage size must be positive");
+ }
mEncryptedStorageKib = encryptedStorageKib;
return this;
}
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 2cd4e51..25f2310 100644
--- a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
+++ b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
@@ -458,10 +458,10 @@
VirtualMachineConfig.Builder builder =
newVmConfigBuilder()
.setPayloadBinaryPath("MicrodroidTestNativeLib.so")
- .setApkPath("relative/path/to.apk")
.setDebugLevel(DEBUG_LEVEL_FULL)
.setMemoryMib(minMemoryRequired());
- assertThrows(IllegalArgumentException.class, () -> builder.build());
+ assertThrows(
+ IllegalArgumentException.class, () -> builder.setApkPath("relative/path/to.apk"));
}
@Test