Create a VmManagerFlag type

This expresses the dependency in choosing a default VMM based on the
guest architecture.

It's currently instantiated in two places to avoid changing type
signatures on the `flags.cc` members.

Bug: b/435031349

Author: Databean

base/cvd/cuttlefish/common/libs/utils/architecture.cpp

@@ -20,6 +20,7 @@
 #include <sys/utsname.h>
 
 #include <cstdlib>
+#include <ostream>
 #include <string>
 
 #include <android-base/logging.h>
@@ -63,4 +64,19 @@ bool IsHostCompatible(Arch arch) {
          (arch == Arch::X86 && host_arch == Arch::X86_64);
 }
 
+std::ostream& operator<<(std::ostream& out, Arch arch) {
+  switch (arch) {
+    case Arch::Arm:
+      return out << "arm";
+    case Arch::Arm64:
+      return out << "arm64";
+    case Arch::RiscV64:
+      return out << "riscv64";
+    case Arch::X86:
+      return out << "x86";
+    case Arch::X86_64:
+      return out << "x86_64";
+  }
+}
+
 }  // namespace cuttlefish

base/cvd/cuttlefish/common/libs/utils/architecture.h

@@ -15,6 +15,7 @@
  */
 #pragma once
 
+#include <ostream>
 #include <string>
 
 namespace cuttlefish {
@@ -31,4 +32,6 @@ const std::string& HostArchStr();
 Arch HostArch();
 bool IsHostCompatible(Arch arch);
 
+std::ostream& operator<<(std::ostream&, Arch);
+
 }  // namespace cuttlefish

base/cvd/cuttlefish/host/commands/assemble_cvd/BUILD.bazel

@@ -258,6 +258,7 @@ cf_cc_library(
         "//cuttlefish/host/commands/assemble_cvd/flags:initramfs_path",
         "//cuttlefish/host/commands/assemble_cvd/flags:kernel_path",
         "//cuttlefish/host/commands/assemble_cvd/flags:system_image_dir",
+        "//cuttlefish/host/commands/assemble_cvd/flags:vm_manager",
         "//cuttlefish/host/libs/config:ap_boot_flow",
         "//cuttlefish/host/libs/config:config_constants",
         "//cuttlefish/host/libs/config:cuttlefish_config",

base/cvd/cuttlefish/host/commands/assemble_cvd/assemble_cvd_flags.cpp

@@ -23,7 +23,6 @@
 
 #include "cuttlefish/common/libs/utils/known_paths.h"
 #include "cuttlefish/host/commands/assemble_cvd/flags_defaults.h"
-#include "cuttlefish/host/libs/config/config_constants.h"
 #include "cuttlefish/host/libs/config/display.h"
 
 #define DEFINE_vec DEFINE_string
@@ -73,8 +72,6 @@ DEFINE_vec(serial_number, CF_DEFAULTS_SERIAL_NUMBER,
               "Serial number to use for the device");
 DEFINE_vec(use_random_serial, fmt::format("{}", CF_DEFAULTS_USE_RANDOM_SERIAL),
            "Whether to use random serial for the device.");
-DEFINE_vec(vm_manager, CF_DEFAULTS_VM_MANAGER,
-              "What virtual machine manager to use, one of {qemu_cli, crosvm}");
 DEFINE_vec(gpu_mode, CF_DEFAULTS_GPU_MODE,
            "What gpu configuration to use, one of {auto, custom, drm_virgl, "
            "gfxstream, gfxstream_guest_angle, "

base/cvd/cuttlefish/host/commands/assemble_cvd/assemble_cvd_flags.h

@@ -45,7 +45,6 @@ DECLARE_vec(guest_enforce_security);
 DECLARE_vec(memory_mb);
 DECLARE_vec(serial_number);
 DECLARE_vec(use_random_serial);
-DECLARE_vec(vm_manager);
 DECLARE_vec(gpu_mode);
 DECLARE_vec(gpu_vhost_user_mode);
 DECLARE_vec(hwcomposer);

base/cvd/cuttlefish/host/commands/assemble_cvd/flags.cc

@@ -54,6 +54,7 @@
 #include "cuttlefish/host/commands/assemble_cvd/flags/initramfs_path.h"
 #include "cuttlefish/host/commands/assemble_cvd/flags/kernel_path.h"
 #include "cuttlefish/host/commands/assemble_cvd/flags/system_image_dir.h"
+#include "cuttlefish/host/commands/assemble_cvd/flags/vm_manager.h"
 #include "cuttlefish/host/commands/assemble_cvd/graphics_flags.h"
 #include "cuttlefish/host/commands/assemble_cvd/guest_config.h"
 #include "cuttlefish/host/commands/assemble_cvd/network_flags.h"
@@ -410,31 +411,16 @@ Result<CuttlefishConfig> InitializeCuttlefishConfiguration(
   auto instance_nums =
       CF_EXPECT(InstanceNumsCalculator().FromGlobalGflags().Calculate());
 
-  // TODO(weihsu), b/250988697:
-  // FLAGS_vm_manager used too early, have to handle this vectorized string early
-  // Currently, all instances should use same vmm, added checking here
-  std::vector<std::string> vm_manager_vec =
-      android::base::Split(FLAGS_vm_manager, ",");
-  for (int i=1; i<vm_manager_vec.size(); i++) {
-    CF_EXPECT(
-        vm_manager_vec[0] == vm_manager_vec[i],
-        "All instances should have same vm_manager, " << FLAGS_vm_manager);
-  }
-  CF_EXPECT_GT(vm_manager_vec.size(), 0);
-  while (vm_manager_vec.size() < instance_nums.size()) {
-    vm_manager_vec.emplace_back(vm_manager_vec[0]);
-  }
-
   // TODO(weihsu), b/250988697: moved bootconfig_supported and hctr2_supported
   // into each instance, but target_arch is still in todo
   // target_arch should be in instance later
-  auto vmm_mode = CF_EXPECT(ParseVmm(vm_manager_vec[0]));
-  auto vmm = GetVmManager(vmm_mode, guest_configs[0].target_arch);
-  if (!vmm) {
-    LOG(FATAL) << "Invalid vm_manager: " << vm_manager_vec[0];
-  }
-  tmp_config_obj.set_vm_manager(vmm_mode);
-  tmp_config_obj.set_ap_vm_manager(vm_manager_vec[0] + "_openwrt");
+  VmManagerFlag vm_manager_flag =
+      CF_EXPECT(VmManagerFlag::FromGlobalGflags(guest_configs));
+  std::unique_ptr<vm_manager::VmManager> vmm =
+      GetVmManager(vm_manager_flag.Mode(), guest_configs[0].target_arch);
+  tmp_config_obj.set_vm_manager(vm_manager_flag.Mode());
+  tmp_config_obj.set_ap_vm_manager(ToString(vm_manager_flag.Mode()) +
+                                   "_openwrt");
 
   // TODO: schuffelen - fix behavior on riscv64
   if (guest_configs[0].target_arch == Arch::RiscV64) {
@@ -1092,7 +1078,7 @@ Result<CuttlefishConfig> InitializeCuttlefishConfiguration(
         gpu_renderer_features_vec[instance_index],
         gpu_context_types_vec[instance_index],
         guest_hwui_renderer_vec[instance_index],
-        guest_renderer_preload_vec[instance_index], vmm_mode,
+        guest_renderer_preload_vec[instance_index], vm_manager_flag.Mode(),
         guest_configs[instance_index], instance));
     calculated_gpu_mode_vec[instance_index] = gpu_mode_vec[instance_index];
 
@@ -1175,7 +1161,7 @@ Result<CuttlefishConfig> InitializeCuttlefishConfiguration(
 
     bool os_overlay = true;
     // Gem5 already uses CoW wrappers around disk images
-    os_overlay &= vmm_mode != VmmMode::kGem5;
+    os_overlay &= vm_manager_flag.Mode() != VmmMode::kGem5;
     os_overlay &= FLAGS_use_overlay;
     if (os_overlay) {
       auto path = const_instance.PerInstancePath("overlay.img");
@@ -1184,7 +1170,7 @@ Result<CuttlefishConfig> InitializeCuttlefishConfiguration(
       virtual_disk_paths.push_back(const_instance.os_composite_disk_path());
     }
 
-    bool persistent_disk = vmm_mode != VmmMode::kGem5;
+    bool persistent_disk = vm_manager_flag.Mode() != VmmMode::kGem5;
     if (persistent_disk) {
 #ifdef __APPLE__
       const std::string persistent_composite_img_base =
@@ -1305,7 +1291,7 @@ Result<CuttlefishConfig> InitializeCuttlefishConfiguration(
     auto external_network_mode = CF_EXPECT(
         ParseExternalNetworkMode(device_external_network_vec[instance_index]));
     CF_EXPECT(external_network_mode == ExternalNetworkMode::kTap ||
-                  vmm_mode == VmmMode::kQemu,
+                  vm_manager_flag.Mode() == VmmMode::kQemu,
               "TODO(b/286284441): slirp only works on QEMU");
     instance.set_external_network_mode(external_network_mode);
 
@@ -1601,50 +1587,30 @@ Result<std::vector<GuestConfig>> GetGuestConfigAndSetDefaults(
   std::vector<GuestConfig> guest_configs =
       CF_EXPECT(ReadGuestConfig(boot_image, kernel_path, system_image_dir));
 
-  // TODO(weihsu), b/250988697:
-  // assume all instances are using same VM manager/app/arch,
-  // later that multiple instances may use different VM manager/app/arch
-
-  // Temporary add this checking to make sure all instances have same target_arch.
-  // This checking should be removed later.
-  for (int instance_index = 1; instance_index < guest_configs.size(); instance_index++) {
-    CF_EXPECT(guest_configs[0].target_arch == guest_configs[instance_index].target_arch,
-              "all instance target_arch should be same");
-  }
-  if (FLAGS_vm_manager.empty()) {
-    if (IsHostCompatible(guest_configs[0].target_arch)) {
-      FLAGS_vm_manager = ToString(VmmMode::kCrosvm);
-    } else {
-      FLAGS_vm_manager = ToString(VmmMode::kQemu);
-    }
-  }
-
-  std::vector<std::string> vm_manager_vec =
-      android::base::Split(FLAGS_vm_manager, ",");
-
-  // TODO(weihsu), b/250988697:
-  // Currently, all instances should use same vmm
-  auto vmm = CF_EXPECT(ParseVmm(vm_manager_vec[0]));
+  VmManagerFlag vm_manager_flag =
+      CF_EXPECT(VmManagerFlag::FromGlobalGflags(guest_configs));
 
   // get flag default values and store into map
   auto name_to_default_value = CurrentFlagsToDefaultValue();
 
-  if (vmm == VmmMode::kQemu) {
-    CF_EXPECT(SetDefaultFlagsForQemu(system_image_dir, guest_configs,
-                                     name_to_default_value));
-  } else if (vmm == VmmMode::kCrosvm) {
-    CF_EXPECT(SetDefaultFlagsForCrosvm(system_image_dir, guest_configs,
+  switch (vm_manager_flag.Mode()) {
+    case VmmMode::kQemu:
+      CF_EXPECT(SetDefaultFlagsForQemu(system_image_dir, guest_configs,
                                        name_to_default_value));
-  } else if (vmm == VmmMode::kGem5) {
-    // TODO: Get the other architectures working
-    if (guest_configs[0].target_arch != Arch::Arm64) {
-      return CF_ERR("Gem5 only supports ARM64");
-    }
-    SetDefaultFlagsForGem5();
-  } else {
-    return CF_ERR("Unknown Virtual Machine Manager: " << FLAGS_vm_manager);
+      break;
+    case VmmMode::kCrosvm:
+      CF_EXPECT(SetDefaultFlagsForCrosvm(system_image_dir, guest_configs,
+                                         name_to_default_value));
+      break;
+    case VmmMode::kGem5:
+      CF_EXPECT_EQ(guest_configs[0].target_arch, Arch::Arm64,
+                   "Gem5 only supports ARM64");
+      SetDefaultFlagsForGem5();
+      break;
+    case VmmMode::kUnknown:
+      return CF_ERR("Unknown VM manager");
   }
-  if (vmm != VmmMode::kGem5) {
+  if (vm_manager_flag.Mode() != VmmMode::kGem5) {
     // After SetCommandLineOptionWithMode in SetDefaultFlagsForCrosvm/Qemu,
     // default flag values changed, need recalculate name_to_default_value
     name_to_default_value = CurrentFlagsToDefaultValue();

base/cvd/cuttlefish/host/commands/assemble_cvd/flags/BUILD.bazel

@@ -81,3 +81,17 @@ cf_cc_library(
         "@gflags",
     ],
 )
+
+cf_cc_library(
+    name = "vm_manager",
+    srcs = ["vm_manager.cc"],
+    hdrs = ["vm_manager.h"],
+    deps = [
+        "//cuttlefish/common/libs/utils:result",
+        "//cuttlefish/host/commands/assemble_cvd:flags_defaults",
+        "//cuttlefish/host/commands/assemble_cvd:guest_config",
+        "//cuttlefish/host/libs/config:cuttlefish_config",
+        "//libbase",
+        "@gflags",
+    ],
+)

base/cvd/cuttlefish/host/commands/assemble_cvd/flags/vm_manager.cc

@@ -0,0 +1,69 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "cuttlefish/host/commands/assemble_cvd/flags/vm_manager.h"
+
+#include <string>
+#include <string_view>
+#include <vector>
+
+#include <android-base/strings.h>
+#include <gflags/gflags.h>
+
+#include "cuttlefish/common/libs/utils/result.h"
+#include "cuttlefish/host/commands/assemble_cvd/flags_defaults.h"
+#include "cuttlefish/host/commands/assemble_cvd/guest_config.h"
+#include "cuttlefish/host/libs/config/cuttlefish_config.h"
+
+DEFINE_string(vm_manager, CF_DEFAULTS_VM_MANAGER,
+              "What virtual machine manager to use, one of {qemu_cli, crosvm}");
+
+namespace cuttlefish {
+
+Result<VmManagerFlag> VmManagerFlag::FromGlobalGflags(
+    const std::vector<GuestConfig>& guest_configs) {
+  // TODO: b/250988697 - Support multiple multiple VM managers in one group
+  CF_EXPECT(!guest_configs.empty());
+  for (const GuestConfig& guest_config : guest_configs) {
+    CF_EXPECT_EQ(guest_config.target_arch, guest_configs[0].target_arch,
+                 "All instance target architectures should be the same");
+  }
+
+  std::vector<std::string> vm_manager_str_vec =
+      android::base::Split(FLAGS_vm_manager, ",");
+
+  VmmMode default_vmm = IsHostCompatible(guest_configs[0].target_arch)
+                            ? VmmMode::kCrosvm
+                            : VmmMode::kQemu;
+
+  std::vector<VmmMode> vmm_vec;
+  for (std::string_view vmm_str : vm_manager_str_vec) {
+    vmm_vec.emplace_back(vmm_str.empty() ? default_vmm
+                                         : CF_EXPECT(ParseVmm(vmm_str)));
+  }
+  if (vmm_vec.empty()) {
+    vmm_vec.emplace_back(default_vmm);
+  }
+  for (VmmMode mode : vmm_vec) {
+    CF_EXPECT_EQ(mode, vmm_vec[0], "All VMMs must be the same");
+  }
+  return VmManagerFlag(vmm_vec[0]);
+}
+
+VmManagerFlag::VmManagerFlag(VmmMode mode) : mode_(mode) {}
+
+VmmMode VmManagerFlag::Mode() const { return mode_; }
+
+}  // namespace cuttlefish

base/cvd/cuttlefish/host/commands/assemble_cvd/flags/vm_manager.h

@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#pragma once
+
+#include <vector>
+
+#include "cuttlefish/common/libs/utils/result.h"
+#include "cuttlefish/host/commands/assemble_cvd/guest_config.h"
+#include "cuttlefish/host/libs/config/cuttlefish_config.h"
+
+namespace cuttlefish {
+
+class VmManagerFlag {
+ public:
+  static Result<VmManagerFlag> FromGlobalGflags(
+      const std::vector<GuestConfig>&);
+
+  VmmMode Mode() const;
+
+ private:
+  explicit VmManagerFlag(VmmMode);
+
+  VmmMode mode_;
+};
+
+}  // namespace cuttlefish