Merge "Use AutoSetup for InitBootloaderEnvPartition" into main am: c5b80706b1

Original change: https://android-review.googlesource.com/c/device/google/cuttlefish/+/2978397

Change-Id: I7200b67d44a95e96414127f102fbe8e898bcc20f
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

5 files changed, 118 insertions, 142 deletions

diff --git a/host/commands/assemble_cvd/boot_config.cc b/host/commands/assemble_cvd/boot_config.cc
index b1215173c..9604653df 100644
--- a/host/commands/assemble_cvd/boot_config.cc
+++ b/host/commands/assemble_cvd/boot_config.cc
@@ -133,138 +133,118 @@ size_t WriteEnvironment(const CuttlefishConfig::InstanceSpecific& instance,
   return env_str.length();
 }
 
-}  // namespace
+std::unordered_map<std::string, std::string> ReplaceKernelBootArgs(
+    const std::unordered_map<std::string, std::string>& args) {
+  std::unordered_map<std::string, std::string> ret;
+  std::transform(std::begin(args), std::end(args),
+                 std::inserter(ret, ret.end()), [](const auto& kv) {
+                   const auto& k = kv.first;
+                   const auto& v = kv.second;
+                   return std::make_pair(
+                       android::base::StringReplace(k, " kernel.", " ", true),
+                       v);
+                 });
+  return ret;
+}
 
-class InitBootloaderEnvPartitionImpl : public InitBootloaderEnvPartition {
- public:
-  INJECT(InitBootloaderEnvPartitionImpl(
-      const CuttlefishConfig& config,
-      const CuttlefishConfig::InstanceSpecific& instance))
-      : config_(config), instance_(instance) {}
-
-  // SetupFeature
-  std::string Name() const override { return "InitBootloaderEnvPartitionImpl"; }
-  bool Enabled() const override { return !instance_.protected_vm(); }
-
- private:
-  std::unordered_set<SetupFeature*> Dependencies() const override { return {}; }
-  Result<void> ResultSetup() override {
-    if (instance_.ap_boot_flow() == CuttlefishConfig::InstanceSpecific::APBootFlow::Grub) {
-      CF_EXPECT(PrepareBootEnvImage(
-          instance_.ap_uboot_env_image_path(),
-          CuttlefishConfig::InstanceSpecific::BootFlow::Linux));
-    }
-    CF_EXPECT(PrepareBootEnvImage(instance_.uboot_env_image_path(),
-                                  instance_.boot_flow()));
+Result<void> PrepareBootEnvImage(
+    const CuttlefishConfig& config,
+    const CuttlefishConfig::InstanceSpecific& instance,
+    const std::string& image_path,
+    const CuttlefishConfig::InstanceSpecific::BootFlow& flow) {
+  if (instance.protected_vm()) {
     return {};
   }
+  auto tmp_boot_env_image_path = image_path + ".tmp";
+  auto uboot_env_path = instance.PerInstancePath("mkenvimg_input");
+  auto kernel_cmdline =
+      android::base::Join(KernelCommandLineFromConfig(config, instance), " ");
+  // If the bootconfig isn't supported in the guest kernel, the bootconfig
+  // args need to be passed in via the uboot env. This won't be an issue for
+  // protect kvm which is running a kernel with bootconfig support.
+  if (!instance.bootconfig_supported()) {
+    auto bootconfig_args =
+        CF_EXPECT(BootconfigArgsFromConfig(config, instance));
+
+    // "androidboot.hardware" kernel parameter has changed to "hardware" in
+    // bootconfig and needs to be replaced before being used in the kernel
+    // cmdline.
+    auto bootconfig_hardware_it = bootconfig_args.find("hardware");
+    if (bootconfig_hardware_it != bootconfig_args.end()) {
+      bootconfig_args["androidboot.hardware"] = bootconfig_hardware_it->second;
+      bootconfig_args.erase(bootconfig_hardware_it);
+    }
 
-  std::unordered_map<std::string, std::string> ReplaceKernelBootArgs(
-      const std::unordered_map<std::string, std::string>& args) {
-    std::unordered_map<std::string, std::string> ret;
-    std::transform(std::begin(args), std::end(args),
-                   std::inserter(ret, ret.end()), [](const auto& kv) {
-                     const auto& k = kv.first;
-                     const auto& v = kv.second;
-                     return std::make_pair(
-                         android::base::StringReplace(k, " kernel.", " ", true),
-                         v);
-                   });
-    return ret;
+    // TODO(b/182417593): Until we pass the module parameters through
+    // modules.options, we pass them through bootconfig using
+    // 'kernel.<key>=<value>' But if we don't support bootconfig, we need to
+    // rename them back to the old cmdline version
+    bootconfig_args = ReplaceKernelBootArgs(bootconfig_args);
+
+    kernel_cmdline +=
+        " " + CF_EXPECT(BootconfigArgsString(bootconfig_args, " "));
   }
 
-  Result<void> PrepareBootEnvImage(
-      const std::string& image_path,
-      const CuttlefishConfig::InstanceSpecific::BootFlow& flow) {
-    auto tmp_boot_env_image_path = image_path + ".tmp";
-    auto uboot_env_path = instance_.PerInstancePath("mkenvimg_input");
-    auto kernel_cmdline = android::base::Join(
-        KernelCommandLineFromConfig(config_, instance_), " ");
-    // If the bootconfig isn't supported in the guest kernel, the bootconfig
-    // args need to be passed in via the uboot env. This won't be an issue for
-    // protect kvm which is running a kernel with bootconfig support.
-    if (!instance_.bootconfig_supported()) {
-      auto bootconfig_args =
-          CF_EXPECT(BootconfigArgsFromConfig(config_, instance_));
-
-      // "androidboot.hardware" kernel parameter has changed to "hardware" in
-      // bootconfig and needs to be replaced before being used in the kernel
-      // cmdline.
-      auto bootconfig_hardware_it = bootconfig_args.find("hardware");
-      if (bootconfig_hardware_it != bootconfig_args.end()) {
-        bootconfig_args["androidboot.hardware"] =
-            bootconfig_hardware_it->second;
-        bootconfig_args.erase(bootconfig_hardware_it);
-      }
-
-      // TODO(b/182417593): Until we pass the module parameters through
-      // modules.options, we pass them through bootconfig using
-      // 'kernel.<key>=<value>' But if we don't support bootconfig, we need to
-      // rename them back to the old cmdline version
-      bootconfig_args = ReplaceKernelBootArgs(bootconfig_args);
-
-      kernel_cmdline +=
-          " " + CF_EXPECT(BootconfigArgsString(bootconfig_args, " "));
-    }
+  CF_EXPECTF(WriteEnvironment(instance, flow, kernel_cmdline, uboot_env_path),
+             "Unable to write out plaintext env '{}'", uboot_env_path);
+
+  auto mkimage_path = HostBinaryPath("mkenvimage_slim");
+  Command cmd(mkimage_path);
+  cmd.AddParameter("-output_path");
+  cmd.AddParameter(tmp_boot_env_image_path);
+  cmd.AddParameter("-input_path");
+  cmd.AddParameter(uboot_env_path);
+  int success = cmd.Start().Wait();
+  CF_EXPECTF(success == 0,
+             "Unable to run mkenvimage_slim. Exited with status {}", success);
+
+  const off_t boot_env_size_bytes =
+      AlignToPowerOf2(MAX_AVB_METADATA_SIZE + 4096, PARTITION_SIZE_SHIFT);
+
+  auto avbtool_path = HostBinaryPath("avbtool");
+  Command boot_env_hash_footer_cmd(avbtool_path);
+  boot_env_hash_footer_cmd.AddParameter("add_hash_footer");
+  boot_env_hash_footer_cmd.AddParameter("--image");
+  boot_env_hash_footer_cmd.AddParameter(tmp_boot_env_image_path);
+  boot_env_hash_footer_cmd.AddParameter("--partition_size");
+  boot_env_hash_footer_cmd.AddParameter(boot_env_size_bytes);
+  boot_env_hash_footer_cmd.AddParameter("--partition_name");
+  boot_env_hash_footer_cmd.AddParameter("uboot_env");
+  boot_env_hash_footer_cmd.AddParameter("--key");
+  boot_env_hash_footer_cmd.AddParameter(
+      DefaultHostArtifactsPath("etc/cvd_avb_testkey.pem"));
+  boot_env_hash_footer_cmd.AddParameter("--algorithm");
+  boot_env_hash_footer_cmd.AddParameter("SHA256_RSA4096");
+  success = boot_env_hash_footer_cmd.Start().Wait();
+  CF_EXPECTF(success == 0,
+             "Unable to append hash footer. Exited with status {}", success);
+
+  if (!FileExists(image_path) ||
+      ReadFile(image_path) != ReadFile(tmp_boot_env_image_path)) {
+    CF_EXPECT(RenameFile(tmp_boot_env_image_path, image_path),
+              "Unable to delete the old env image");
+    LOG(DEBUG) << "Updated bootloader environment image.";
+  } else {
+    RemoveFile(tmp_boot_env_image_path);
+  }
 
-    CF_EXPECTF(
-        WriteEnvironment(instance_, flow, kernel_cmdline, uboot_env_path),
-        "Unable to write out plaintext env '{}'", uboot_env_path);
-
-    auto mkimage_path = HostBinaryPath("mkenvimage_slim");
-    Command cmd(mkimage_path);
-    cmd.AddParameter("-output_path");
-    cmd.AddParameter(tmp_boot_env_image_path);
-    cmd.AddParameter("-input_path");
-    cmd.AddParameter(uboot_env_path);
-    int success = cmd.Start().Wait();
-    CF_EXPECTF(success == 0,
-               "Unable to run mkenvimage_slim. Exited with status {}", success);
-
-    const off_t boot_env_size_bytes = AlignToPowerOf2(
-        MAX_AVB_METADATA_SIZE + 4096, PARTITION_SIZE_SHIFT);
-
-    auto avbtool_path = HostBinaryPath("avbtool");
-    Command boot_env_hash_footer_cmd(avbtool_path);
-    boot_env_hash_footer_cmd.AddParameter("add_hash_footer");
-    boot_env_hash_footer_cmd.AddParameter("--image");
-    boot_env_hash_footer_cmd.AddParameter(tmp_boot_env_image_path);
-    boot_env_hash_footer_cmd.AddParameter("--partition_size");
-    boot_env_hash_footer_cmd.AddParameter(boot_env_size_bytes);
-    boot_env_hash_footer_cmd.AddParameter("--partition_name");
-    boot_env_hash_footer_cmd.AddParameter("uboot_env");
-    boot_env_hash_footer_cmd.AddParameter("--key");
-    boot_env_hash_footer_cmd.AddParameter(
-        DefaultHostArtifactsPath("etc/cvd_avb_testkey.pem"));
-    boot_env_hash_footer_cmd.AddParameter("--algorithm");
-    boot_env_hash_footer_cmd.AddParameter("SHA256_RSA4096");
-    success = boot_env_hash_footer_cmd.Start().Wait();
-    CF_EXPECTF(success == 0,
-               "Unable to append hash footer. Exited with status {}", success);
-
-    if (!FileExists(image_path) ||
-        ReadFile(image_path) != ReadFile(tmp_boot_env_image_path)) {
-      CF_EXPECT(RenameFile(tmp_boot_env_image_path, image_path),
-                "Unable to delete the old env image");
-      LOG(DEBUG) << "Updated bootloader environment image.";
-    } else {
-      RemoveFile(tmp_boot_env_image_path);
-    }
+  return {};
+}
 
-    return {};
-  }
+}  // namespace
 
-  const CuttlefishConfig& config_;
-  const CuttlefishConfig::InstanceSpecific& instance_;
-};
-
-fruit::Component<fruit::Required<const CuttlefishConfig,
-                                 const CuttlefishConfig::InstanceSpecific>,
-                 InitBootloaderEnvPartition>
-InitBootloaderEnvPartitionComponent() {
-  return fruit::createComponent()
-      .bind<InitBootloaderEnvPartition, InitBootloaderEnvPartitionImpl>()
-      .addMultibinding<SetupFeature, InitBootloaderEnvPartition>();
+Result<void> InitBootloaderEnvPartition(
+    const CuttlefishConfig& config,
+    const CuttlefishConfig::InstanceSpecific& instance) {
+  if (instance.ap_boot_flow() ==
+      CuttlefishConfig::InstanceSpecific::APBootFlow::Grub) {
+    CF_EXPECT(PrepareBootEnvImage(
+        config, instance, instance.ap_uboot_env_image_path(),
+        CuttlefishConfig::InstanceSpecific::BootFlow::Linux));
+  }
+  CF_EXPECT(PrepareBootEnvImage(
+      config, instance, instance.uboot_env_image_path(), instance.boot_flow()));
+  return {};
 }
 
 } // namespace cuttlefish
diff --git a/host/commands/assemble_cvd/boot_config.h b/host/commands/assemble_cvd/boot_config.h
index 87318495e..dc5328ff2 100644
--- a/host/commands/assemble_cvd/boot_config.h
+++ b/host/commands/assemble_cvd/boot_config.h
@@ -17,16 +17,12 @@
 
 #include <fruit/fruit.h>
 
+#include "common/libs/utils/result.h"
 #include "host/libs/config/cuttlefish_config.h"
-#include "host/libs/config/feature.h"
 
 namespace cuttlefish {
 
-class InitBootloaderEnvPartition : public SetupFeature {};
-
-fruit::Component<fruit::Required<const CuttlefishConfig,
-                                 const CuttlefishConfig::InstanceSpecific>,
-                 InitBootloaderEnvPartition>
-InitBootloaderEnvPartitionComponent();
+Result<void> InitBootloaderEnvPartition(
+    const CuttlefishConfig&, const CuttlefishConfig::InstanceSpecific&);
 
 } // namespace cuttlefish
diff --git a/host/commands/assemble_cvd/disk/disk.h b/host/commands/assemble_cvd/disk/disk.h
index 1265d8121..8a94bc2da 100644
--- a/host/commands/assemble_cvd/disk/disk.h
+++ b/host/commands/assemble_cvd/disk/disk.h
@@ -45,10 +45,10 @@ Gem5ImageUnpackerComponent();
 
 class GeneratePersistentVbmeta : public SetupFeature {};
 
-fruit::Component<
-    fruit::Required<const CuttlefishConfig::InstanceSpecific,
-                    InitBootloaderEnvPartition, GeneratePersistentBootconfig>,
-    GeneratePersistentVbmeta>
+fruit::Component<fruit::Required<const CuttlefishConfig::InstanceSpecific,
+                                 AutoSetup<InitBootloaderEnvPartition>::Type,
+                                 GeneratePersistentBootconfig>,
+                 GeneratePersistentVbmeta>
 GeneratePersistentVbmetaComponent();
 
 class InitializeFactoryResetProtected : public SetupFeature {};
diff --git a/host/commands/assemble_cvd/disk/generate_persistent_vbmeta.cpp b/host/commands/assemble_cvd/disk/generate_persistent_vbmeta.cpp
index 1f95b9b1c..a1c4cf2b3 100644
--- a/host/commands/assemble_cvd/disk/generate_persistent_vbmeta.cpp
+++ b/host/commands/assemble_cvd/disk/generate_persistent_vbmeta.cpp
@@ -34,7 +34,7 @@ class GeneratePersistentVbmetaImpl : public GeneratePersistentVbmeta {
  public:
   INJECT(GeneratePersistentVbmetaImpl(
       const CuttlefishConfig::InstanceSpecific& instance,
-      InitBootloaderEnvPartition& bootloader_env,
+      AutoSetup<InitBootloaderEnvPartition>::Type& bootloader_env,
       GeneratePersistentBootconfig& bootconfig))
       : instance_(instance),
         bootloader_env_(bootloader_env),
@@ -111,14 +111,14 @@ class GeneratePersistentVbmetaImpl : public GeneratePersistentVbmeta {
   }
 
   const CuttlefishConfig::InstanceSpecific& instance_;
-  InitBootloaderEnvPartition& bootloader_env_;
+  AutoSetup<InitBootloaderEnvPartition>::Type& bootloader_env_;
   GeneratePersistentBootconfig& bootconfig_;
 };
 
-fruit::Component<
-    fruit::Required<const CuttlefishConfig::InstanceSpecific,
-                    InitBootloaderEnvPartition, GeneratePersistentBootconfig>,
-    GeneratePersistentVbmeta>
+fruit::Component<fruit::Required<const CuttlefishConfig::InstanceSpecific,
+                                 AutoSetup<InitBootloaderEnvPartition>::Type,
+                                 GeneratePersistentBootconfig>,
+                 GeneratePersistentVbmeta>
 GeneratePersistentVbmetaComponent() {
   return fruit::createComponent()
       .addMultibinding<SetupFeature, GeneratePersistentVbmetaImpl>()
diff --git a/host/commands/assemble_cvd/disk_flags.cc b/host/commands/assemble_cvd/disk_flags.cc
index f321def87..3c5c0b151 100644
--- a/host/commands/assemble_cvd/disk_flags.cc
+++ b/host/commands/assemble_cvd/disk_flags.cc
@@ -684,6 +684,7 @@ static fruit::Component<> DiskChangesPerInstanceComponent(
       .bindInstance(*config)
       .bindInstance(*instance)
       .install(AutoSetup<InitializeAccessKregistryImage>::Component)
+      .install(AutoSetup<InitBootloaderEnvPartition>::Component)
       .install(AutoSetup<InitializeHwcomposerPmemImage>::Component)
       .install(AutoSetup<InitializePstore>::Component)
       .install(AutoSetup<InitializeSdCard>::Component)
@@ -691,8 +692,7 @@ static fruit::Component<> DiskChangesPerInstanceComponent(
       .install(GeneratePersistentBootconfigComponent)
       .install(GeneratePersistentVbmetaComponent)
       .install(AutoSetup<InitializeInstanceCompositeDisk>::Component)
-      .install(InitializeDataImageComponent)
-      .install(InitBootloaderEnvPartitionComponent);
+      .install(InitializeDataImageComponent);
 }
 
 Result<void> DiskImageFlagsVectorization(CuttlefishConfig& config, const FetcherConfig& fetcher_config) {