Move FinishedSnapshotWrites call back to PostinstallRunnerAction am: 19acf4fe19

Original change: https://android-review.googlesource.com/c/platform/system/update_engine/+/2901827

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

5 files changed, 39 insertions, 40 deletions

diff --git a/aosp/update_attempter_android.cc b/aosp/update_attempter_android.cc
index 1adaabc3..e41531cb 100644
--- a/aosp/update_attempter_android.cc
+++ b/aosp/update_attempter_android.cc
@@ -1325,21 +1325,22 @@ bool UpdateAttempterAndroid::setShouldSwitchSlotOnReboot(
   CHECK_NE(install_plan_.source_slot, UINT32_MAX);
   CHECK_NE(install_plan_.target_slot, UINT32_MAX);
 
-  auto install_plan_action = std::make_unique<InstallPlanAction>(install_plan_);
   auto postinstall_runner_action =
       std::make_unique<PostinstallRunnerAction>(boot_control_, hardware_);
-  SetStatusAndNotify(UpdateStatus::VERIFYING);
   postinstall_runner_action->set_delegate(this);
-  ErrorCode error_code{};
 
   // If last error code is kUpdatedButNotActive, we know that we reached this
   // state by calling applyPayload() with switch_slot=false. That applyPayload()
   // call would have already performed filesystem verification, therefore, we
   // can safely skip the verification to save time.
   if (last_error_ == ErrorCode::kUpdatedButNotActive) {
+    auto install_plan_action =
+        std::make_unique<InstallPlanAction>(install_plan_);
     BondActions(install_plan_action.get(), postinstall_runner_action.get());
     processor_->EnqueueAction(std::move(install_plan_action));
+    SetStatusAndNotify(UpdateStatus::FINALIZING);
   } else {
+    ErrorCode error_code{};
     if (!boot_control_->GetDynamicPartitionControl()
              ->PreparePartitionsForUpdate(GetCurrentSlot(),
                                           GetTargetSlot(),
@@ -1361,7 +1362,8 @@ bool UpdateAttempterAndroid::setShouldSwitchSlotOnReboot(
                                 utils::ErrorCodeToString(error_code),
                             error_code);
     }
-
+    auto install_plan_action =
+        std::make_unique<InstallPlanAction>(install_plan_);
     auto filesystem_verifier_action =
         std::make_unique<FilesystemVerifierAction>(
             boot_control_->GetDynamicPartitionControl());
@@ -1371,6 +1373,7 @@ bool UpdateAttempterAndroid::setShouldSwitchSlotOnReboot(
                 postinstall_runner_action.get());
     processor_->EnqueueAction(std::move(install_plan_action));
     processor_->EnqueueAction(std::move(filesystem_verifier_action));
+    SetStatusAndNotify(UpdateStatus::VERIFYING);
   }
 
   processor_->EnqueueAction(std::move(postinstall_runner_action));
diff --git a/payload_consumer/filesystem_verifier_action.cc b/payload_consumer/filesystem_verifier_action.cc
index 5342e6ff..5345085c 100644
--- a/payload_consumer/filesystem_verifier_action.cc
+++ b/payload_consumer/filesystem_verifier_action.cc
@@ -136,13 +136,6 @@ void FilesystemVerifierAction::Cleanup(ErrorCode code) {
   partition_fd_.reset();
   // This memory is not used anymore.
   buffer_.clear();
-  if (code == ErrorCode::kSuccess && !cancelled_) {
-    if (!dynamic_control_->FinishUpdate(install_plan_.powerwash_required)) {
-      LOG(ERROR) << "Failed to FinishUpdate("
-                 << install_plan_.powerwash_required << ")";
-      code = ErrorCode::kFilesystemVerifierError;
-    }
-  }
 
   // If we didn't write verity, partitions were maped. Releaase resource now.
   if (!install_plan_.write_verity &&
diff --git a/payload_consumer/filesystem_verifier_action_unittest.cc b/payload_consumer/filesystem_verifier_action_unittest.cc
index 565976ad..f2967dbd 100644
--- a/payload_consumer/filesystem_verifier_action_unittest.cc
+++ b/payload_consumer/filesystem_verifier_action_unittest.cc
@@ -32,7 +32,6 @@
 #include <libsnapshot/cow_writer.h>
 #include <sys/stat.h>
 
-#include "gmock/gmock-spec-builders.h"
 #include "update_engine/common/dynamic_partition_control_stub.h"
 #include "update_engine/common/hash_calculator.h"
 #include "update_engine/common/mock_dynamic_partition_control.h"
@@ -557,13 +556,12 @@ void FilesystemVerifierActionTest::DoTestVABC(bool clear_target_hash,
     install_plan_.write_verity = true;
     ASSERT_NO_FATAL_FAILURE(SetHashWithVerity(&part));
   }
-  NiceMock<MockDynamicPartitionControl> dynamic_control;
   if (clear_target_hash) {
     part.target_hash.clear();
-  } else {
-    EXPECT_CALL(dynamic_control, FinishUpdate(0)).WillOnce(Return(true));
   }
 
+  NiceMock<MockDynamicPartitionControl> dynamic_control;
+
   EnableVABC(&dynamic_control, part.name);
   auto open_cow = [part]() {
     auto cow_fd = std::make_unique<EintrSafeFileDescriptor>();
@@ -663,7 +661,6 @@ TEST_F(FilesystemVerifierActionTest, VABC_Verity_ReadAfterWrite) {
   part.readonly_target_path = target_part_.path();
   NiceMock<MockDynamicPartitionControl> dynamic_control;
   EnableVABC(&dynamic_control, part.name);
-  ON_CALL(dynamic_control, FinishUpdate(_)).WillByDefault(Return(true));
 
   // b/186196758 is only visible if we repeatedely run FS verification w/o
   // writing verity
diff --git a/payload_consumer/postinstall_runner_action.cc b/payload_consumer/postinstall_runner_action.cc
index fa4654df..2cfd3c6b 100644
--- a/payload_consumer/postinstall_runner_action.cc
+++ b/payload_consumer/postinstall_runner_action.cc
@@ -109,12 +109,11 @@ void PostinstallRunnerAction::PerformAction() {
   auto dynamic_control = boot_control_->GetDynamicPartitionControl();
   CHECK(dynamic_control);
 
-  // Mount snapshot partitions for Virtual AB Compression
+  // Mount snapshot partitions for Virtual AB Compression Compression.
   if (dynamic_control->UpdateUsesSnapshotCompression()) {
     // Before calling MapAllPartitions to map snapshot devices, all CowWriters
     // must be closed, and MapAllPartitions() should be called.
     if (!install_plan_.partitions.empty()) {
-      dynamic_control->UnmapAllPartitions();
       if (!dynamic_control->MapAllPartitions()) {
         return CompletePostinstall(ErrorCode::kPostInstallMountError);
       }
@@ -429,13 +428,39 @@ void PostinstallRunnerAction::CompletePartitionPostinstall(
   PerformPartitionPostinstall();
 }
 
+PostinstallRunnerAction::~PostinstallRunnerAction() {
+  if (!install_plan_.partitions.empty()) {
+    auto dynamic_control = boot_control_->GetDynamicPartitionControl();
+    CHECK(dynamic_control);
+    dynamic_control->UnmapAllPartitions();
+    LOG(INFO) << "Unmapped all partitions.";
+  }
+}
+
 void PostinstallRunnerAction::CompletePostinstall(ErrorCode error_code) {
   // We only attempt to mark the new slot as active if all the postinstall
   // steps succeeded.
+  DEFER {
+    if (error_code != ErrorCode::kSuccess &&
+        error_code != ErrorCode::kUpdatedButNotActive) {
+      LOG(ERROR) << "Postinstall action failed.";
+
+      // Undo any changes done to trigger Powerwash.
+      if (powerwash_scheduled_)
+        hardware_->CancelPowerwash();
+    }
+    processor_->ActionComplete(this, error_code);
+  };
   if (error_code == ErrorCode::kSuccess) {
     if (install_plan_.switch_slot_on_reboot) {
-      if (!boot_control_->SetActiveBootSlot(install_plan_.target_slot)) {
-        LOG(ERROR) << "Failed to switch slot to " << install_plan_.target_slot;
+      if (!boot_control_->GetDynamicPartitionControl()->MapAllPartitions()) {
+        LOG(ERROR) << "Failed to map all partitions before marking snapshot as "
+                      "ready for slot switch.";
+        return;
+      }
+      if (!boot_control_->GetDynamicPartitionControl()->FinishUpdate(
+              install_plan_.powerwash_required) ||
+          !boot_control_->SetActiveBootSlot(install_plan_.target_slot)) {
         error_code = ErrorCode::kPostinstallRunnerError;
       } else {
         // Schedules warm reset on next reboot, ignores the error.
@@ -447,26 +472,6 @@ void PostinstallRunnerAction::CompletePostinstall(ErrorCode error_code) {
       error_code = ErrorCode::kUpdatedButNotActive;
     }
   }
-  if (!install_plan_.partitions.empty()) {
-    auto dynamic_control = boot_control_->GetDynamicPartitionControl();
-    CHECK(dynamic_control);
-    dynamic_control->UnmapAllPartitions();
-    LOG(INFO) << "Unmapped all partitions.";
-  }
-
-  ScopedActionCompleter completer(processor_, this);
-  completer.set_code(error_code);
-
-  if (error_code != ErrorCode::kSuccess &&
-      error_code != ErrorCode::kUpdatedButNotActive) {
-    LOG(ERROR) << "Postinstall action failed.";
-
-    // Undo any changes done to trigger Powerwash.
-    if (powerwash_scheduled_)
-      hardware_->CancelPowerwash();
-
-    return;
-  }
 
   LOG(INFO) << "All post-install commands succeeded";
   if (HasOutputPipe()) {
diff --git a/payload_consumer/postinstall_runner_action.h b/payload_consumer/postinstall_runner_action.h
index 66721af9..60170697 100644
--- a/payload_consumer/postinstall_runner_action.h
+++ b/payload_consumer/postinstall_runner_action.h
@@ -42,6 +42,7 @@ class PostinstallRunnerAction : public InstallPlanAction {
  public:
   PostinstallRunnerAction(BootControlInterface* boot_control,
                           HardwareInterface* hardware);
+  ~PostinstallRunnerAction();
 
   // InstallPlanAction overrides.
   void PerformAction() override;