Refactor the vm app

Flags are organized into structs while flattened at the cmdline level.

Bug: 295174105
Test: watch TH
Change-Id: I4125c5ca26d3dc53e64cfedf1468e19ebe03968d
diff --git a/vm/src/run.rs b/vm/src/run.rs
index 250c56c..fc8d7e0 100644
--- a/vm/src/run.rs
+++ b/vm/src/run.rs
@@ -15,8 +15,8 @@
 //! Command to run a VM.
 
 use crate::create_partition::command_create_partition;
+use crate::{get_service, RunAppConfig, RunCustomVmConfig, RunMicrodroidConfig};
 use android_system_virtualizationservice::aidl::android::system::virtualizationservice::{
-    CpuTopology::CpuTopology,
     IVirtualizationService::IVirtualizationService,
     PartitionType::PartitionType,
     VirtualMachineAppConfig::{
@@ -35,7 +35,6 @@
 use std::fs;
 use std::fs::File;
 use std::io;
-use std::num::NonZeroU16;
 use std::os::unix::io::{AsRawFd, FromRawFd};
 use std::path::{Path, PathBuf};
 use vmclient::{ErrorCode, VmInstance};
@@ -43,98 +42,78 @@
 use zip::ZipArchive;
 
 /// Run a VM from the given APK, idsig, and config.
-#[allow(clippy::too_many_arguments)]
-pub fn command_run_app(
-    name: Option<String>,
-    service: &dyn IVirtualizationService,
-    apk: &Path,
-    idsig: &Path,
-    instance: &Path,
-    storage: Option<&Path>,
-    storage_size: Option<u64>,
-    config_path: Option<String>,
-    payload_binary_name: Option<String>,
-    console_out_path: Option<&Path>,
-    console_in_path: Option<&Path>,
-    log_path: Option<&Path>,
-    debug_level: DebugLevel,
-    protected: bool,
-    mem: Option<u32>,
-    cpu_topology: CpuTopology,
-    task_profiles: Vec<String>,
-    extra_idsigs: &[PathBuf],
-    gdb: Option<NonZeroU16>,
-    kernel: Option<&Path>,
-    vendor: Option<&Path>,
-    devices: Vec<PathBuf>,
-) -> Result<(), Error> {
-    let apk_file = File::open(apk).context("Failed to open APK file")?;
+pub fn command_run_app(config: RunAppConfig) -> Result<(), Error> {
+    let service = get_service()?;
+    let apk = File::open(&config.apk).context("Failed to open APK file")?;
 
-    let extra_apks = match config_path.as_deref() {
-        Some(path) => parse_extra_apk_list(apk, path)?,
+    let extra_apks = match config.config_path.as_deref() {
+        Some(path) => parse_extra_apk_list(&config.apk, path)?,
         None => vec![],
     };
 
-    if extra_apks.len() != extra_idsigs.len() {
+    if extra_apks.len() != config.extra_idsigs.len() {
         bail!(
             "Found {} extra apks, but there are {} extra idsigs",
             extra_apks.len(),
-            extra_idsigs.len()
+            config.extra_idsigs.len()
         )
     }
 
-    for i in 0..extra_apks.len() {
-        let extra_apk_fd = ParcelFileDescriptor::new(File::open(&extra_apks[i])?);
-        let extra_idsig_fd = ParcelFileDescriptor::new(File::create(&extra_idsigs[i])?);
+    for (i, extra_apk) in extra_apks.iter().enumerate() {
+        let extra_apk_fd = ParcelFileDescriptor::new(File::open(extra_apk)?);
+        let extra_idsig_fd = ParcelFileDescriptor::new(File::create(&config.extra_idsigs[i])?);
         service.createOrUpdateIdsigFile(&extra_apk_fd, &extra_idsig_fd)?;
     }
 
-    let idsig_file = File::create(idsig).context("Failed to create idsig file")?;
+    let idsig = File::create(&config.idsig).context("Failed to create idsig file")?;
 
-    let apk_fd = ParcelFileDescriptor::new(apk_file);
-    let idsig_fd = ParcelFileDescriptor::new(idsig_file);
+    let apk_fd = ParcelFileDescriptor::new(apk);
+    let idsig_fd = ParcelFileDescriptor::new(idsig);
     service.createOrUpdateIdsigFile(&apk_fd, &idsig_fd)?;
 
-    let idsig_file = File::open(idsig).context("Failed to open idsig file")?;
-    let idsig_fd = ParcelFileDescriptor::new(idsig_file);
+    let idsig = File::open(&config.idsig).context("Failed to open idsig file")?;
+    let idsig_fd = ParcelFileDescriptor::new(idsig);
 
-    if !instance.exists() {
+    if !config.instance.exists() {
         const INSTANCE_FILE_SIZE: u64 = 10 * 1024 * 1024;
         command_create_partition(
-            service,
-            instance,
+            service.as_ref(),
+            &config.instance,
             INSTANCE_FILE_SIZE,
             PartitionType::ANDROID_VM_INSTANCE,
         )?;
     }
 
-    let storage = if let Some(path) = storage {
+    let storage = if let Some(path) = config.microdroid.storage {
         if !path.exists() {
             command_create_partition(
-                service,
-                path,
-                storage_size.unwrap_or(10 * 1024 * 1024),
+                service.as_ref(),
+                &path,
+                config.microdroid.storage_size.unwrap_or(10 * 1024 * 1024),
                 PartitionType::ENCRYPTEDSTORE,
             )?;
         }
-        Some(open_parcel_file(path, true)?)
+        Some(open_parcel_file(&path, true)?)
     } else {
         None
     };
 
-    let kernel = kernel.map(|p| open_parcel_file(p, false)).transpose()?;
+    let kernel =
+        config.microdroid.kernel.as_ref().map(|p| open_parcel_file(p, false)).transpose()?;
 
-    let vendor = vendor.map(|p| open_parcel_file(p, false)).transpose()?;
+    let vendor =
+        config.microdroid.vendor.as_ref().map(|p| open_parcel_file(p, false)).transpose()?;
 
-    let extra_idsig_files: Result<Vec<File>, _> = extra_idsigs.iter().map(File::open).collect();
+    let extra_idsig_files: Result<Vec<File>, _> =
+        config.extra_idsigs.iter().map(File::open).collect();
     let extra_idsig_fds = extra_idsig_files?.into_iter().map(ParcelFileDescriptor::new).collect();
 
-    let payload = if let Some(config_path) = config_path {
-        if payload_binary_name.is_some() {
+    let payload = if let Some(config_path) = config.config_path {
+        if config.payload_binary_name.is_some() {
             bail!("Only one of --config-path or --payload-binary-name can be defined")
         }
         Payload::ConfigPath(config_path)
-    } else if let Some(payload_binary_name) = payload_binary_name {
+    } else if let Some(payload_binary_name) = config.payload_binary_name {
         Payload::PayloadConfig(VirtualMachinePayloadConfig {
             payloadBinaryName: payload_binary_name,
         })
@@ -142,14 +121,16 @@
         bail!("Either --config-path or --payload-binary-name must be defined")
     };
 
-    let payload_config_str = format!("{:?}!{:?}", apk, payload);
+    let payload_config_str = format!("{:?}!{:?}", config.apk, payload);
 
     let custom_config = CustomConfig {
         customKernelImage: kernel,
-        gdbPort: gdb.map(u16::from).unwrap_or(0) as i32, // 0 means no gdb
-        taskProfiles: task_profiles,
+        gdbPort: config.debug.gdb.map(u16::from).unwrap_or(0) as i32, // 0 means no gdb
+        taskProfiles: config.common.task_profiles,
         vendorImage: vendor,
-        devices: devices
+        devices: config
+            .microdroid
+            .devices
             .iter()
             .map(|x| {
                 x.to_str().map(String::from).ok_or(anyhow!("Failed to convert {x:?} to String"))
@@ -157,21 +138,28 @@
             .collect::<Result<_, _>>()?,
     };
 
-    let config = VirtualMachineConfig::AppConfig(VirtualMachineAppConfig {
-        name: name.unwrap_or_else(|| String::from("VmRunApp")),
+    let vm_config = VirtualMachineConfig::AppConfig(VirtualMachineAppConfig {
+        name: config.common.name.unwrap_or_else(|| String::from("VmRunApp")),
         apk: apk_fd.into(),
         idsig: idsig_fd.into(),
         extraIdsigs: extra_idsig_fds,
-        instanceImage: open_parcel_file(instance, true /* writable */)?.into(),
+        instanceImage: open_parcel_file(&config.instance, true /* writable */)?.into(),
         encryptedStorageImage: storage,
         payload,
-        debugLevel: debug_level,
-        protectedVm: protected,
-        memoryMib: mem.unwrap_or(0) as i32, // 0 means use the VM default
-        cpuTopology: cpu_topology,
+        debugLevel: config.debug.debug,
+        protectedVm: config.common.protected,
+        memoryMib: config.common.mem.unwrap_or(0) as i32, // 0 means use the VM default
+        cpuTopology: config.common.cpu_topology,
         customConfig: Some(custom_config),
     });
-    run(service, &config, &payload_config_str, console_out_path, console_in_path, log_path)
+    run(
+        service.as_ref(),
+        &vm_config,
+        &payload_config_str,
+        config.debug.console.as_ref().map(|p| p.as_ref()),
+        config.debug.console_in.as_ref().map(|p| p.as_ref()),
+        config.debug.log.as_ref().map(|p| p.as_ref()),
+    )
 }
 
 fn find_empty_payload_apk_path() -> Result<PathBuf, Error> {
@@ -197,100 +185,55 @@
 }
 
 /// Run a VM with Microdroid
-#[allow(clippy::too_many_arguments)]
-pub fn command_run_microdroid(
-    name: Option<String>,
-    service: &dyn IVirtualizationService,
-    work_dir: Option<PathBuf>,
-    storage: Option<&Path>,
-    storage_size: Option<u64>,
-    console_out_path: Option<&Path>,
-    console_in_path: Option<&Path>,
-    log_path: Option<&Path>,
-    debug_level: DebugLevel,
-    protected: bool,
-    mem: Option<u32>,
-    cpu_topology: CpuTopology,
-    task_profiles: Vec<String>,
-    gdb: Option<NonZeroU16>,
-    kernel: Option<&Path>,
-    vendor: Option<&Path>,
-    devices: Vec<PathBuf>,
-) -> Result<(), Error> {
+pub fn command_run_microdroid(config: RunMicrodroidConfig) -> Result<(), Error> {
     let apk = find_empty_payload_apk_path()?;
     println!("found path {}", apk.display());
 
-    let work_dir = work_dir.unwrap_or(create_work_dir()?);
+    let work_dir = config.work_dir.unwrap_or(create_work_dir()?);
     let idsig = work_dir.join("apk.idsig");
     println!("apk.idsig path: {}", idsig.display());
     let instance_img = work_dir.join("instance.img");
     println!("instance.img path: {}", instance_img.display());
 
-    let payload_binary_name = "MicrodroidEmptyPayloadJniLib.so";
-    let extra_sig = [];
-    command_run_app(
-        name,
-        service,
-        &apk,
-        &idsig,
-        &instance_img,
-        storage,
-        storage_size,
-        /* config_path= */ None,
-        Some(payload_binary_name.to_owned()),
-        console_out_path,
-        console_in_path,
-        log_path,
-        debug_level,
-        protected,
-        mem,
-        cpu_topology,
-        task_profiles,
-        &extra_sig,
-        gdb,
-        kernel,
-        vendor,
-        devices,
-    )
+    let app_config = RunAppConfig {
+        common: config.common,
+        debug: config.debug,
+        microdroid: config.microdroid,
+        apk,
+        idsig,
+        instance: instance_img,
+        config_path: None,
+        payload_binary_name: Some("MicrodroidEmptyPayloadJniLib.so".to_owned()),
+        extra_idsigs: [].to_vec(),
+    };
+    command_run_app(app_config)
 }
 
 /// Run a VM from the given configuration file.
-#[allow(clippy::too_many_arguments)]
-pub fn command_run(
-    name: Option<String>,
-    service: &dyn IVirtualizationService,
-    config_path: &Path,
-    console_out_path: Option<&Path>,
-    console_in_path: Option<&Path>,
-    log_path: Option<&Path>,
-    mem: Option<u32>,
-    cpu_topology: CpuTopology,
-    task_profiles: Vec<String>,
-    gdb: Option<NonZeroU16>,
-) -> Result<(), Error> {
-    let config_file = File::open(config_path).context("Failed to open config file")?;
-    let mut config =
+pub fn command_run(config: RunCustomVmConfig) -> Result<(), Error> {
+    let config_file = File::open(&config.config).context("Failed to open config file")?;
+    let mut vm_config =
         VmConfig::load(&config_file).context("Failed to parse config file")?.to_parcelable()?;
-    if let Some(mem) = mem {
-        config.memoryMib = mem as i32;
+    if let Some(mem) = config.common.mem {
+        vm_config.memoryMib = mem as i32;
     }
-    if let Some(name) = name {
-        config.name = name;
+    if let Some(name) = config.common.name {
+        vm_config.name = name;
     } else {
-        config.name = String::from("VmRun");
+        vm_config.name = String::from("VmRun");
     }
-    if let Some(gdb) = gdb {
-        config.gdbPort = gdb.get() as i32;
+    if let Some(gdb) = config.debug.gdb {
+        vm_config.gdbPort = gdb.get() as i32;
     }
-    config.cpuTopology = cpu_topology;
-    config.taskProfiles = task_profiles;
+    vm_config.cpuTopology = config.common.cpu_topology;
+    vm_config.taskProfiles = config.common.task_profiles;
     run(
-        service,
-        &VirtualMachineConfig::RawConfig(config),
-        &format!("{:?}", config_path),
-        console_out_path,
-        console_in_path,
-        log_path,
+        get_service()?.as_ref(),
+        &VirtualMachineConfig::RawConfig(vm_config),
+        &format!("{:?}", &config.config),
+        config.debug.console.as_ref().map(|p| p.as_ref()),
+        config.debug.console_in.as_ref().map(|p| p.as_ref()),
+        config.debug.log.as_ref().map(|p| p.as_ref()),
     )
 }