Move FinishedSnapshotWrites call to filesystem verifier
FinishedSnapshotWrites() must be called before slot switch and after
finish writing data to the target partition. It also requires all target
partitions to be mapped when calling. Priori to this CL,
PostinstallRunnerAction is responsible for calling
FinishedSnapshotWrites(). However, postinstall action has complicated
logic to decide if partitions should be mapped/unmapped. So it's
difficult to ensure that partitions are mapped before calling
FinishedSnapshotWrites(). To make things simpler, move the
FinishedSnapshotWrites() call to FilesystemVerifierAction.
Since FilesystemVerifierAction is responsible for performing the last
write to partitions(verity writes), this make placement make sense. We
are also guaranteed that partitions will be mapped, as
FilesystemVerifierAction need to map these partitions in order to do
verification.
Test: install VAB test w/o slot switch, call setShouldSwitchSlotOnReboot
Bug: 269397263
(cherry picked from https://android-review.googlesource.com/q/commit:5b00dc5386e67db5609e79f9e5c603d4ef1098a9)
Merged-In: I900b529f8b4a0b604bab3a334f61ff9a51139916
Change-Id: I900b529f8b4a0b604bab3a334f61ff9a51139916
diff --git a/aosp/hardware_android.cc b/aosp/hardware_android.cc
index 624cfc9..f8732ab 100644
--- a/aosp/hardware_android.cc
+++ b/aosp/hardware_android.cc
@@ -85,7 +85,7 @@
}
std::string CalculateVbmetaDigestForInactiveSlot() {
- AvbSlotVerifyData* avb_slot_data;
+ AvbSlotVerifyData* avb_slot_data{};
auto suffix = fs_mgr_get_other_slot_suffix();
const char* requested_partitions[] = {nullptr};
@@ -105,8 +105,9 @@
avb_slot_verify_data_calculate_vbmeta_digest(
avb_slot_data, AVB_DIGEST_TYPE_SHA256, vbmeta_digest);
- std::string encoded_digest =
+ const std::string encoded_digest =
base::HexEncode(vbmeta_digest, AVB_SHA256_DIGEST_SIZE);
+ LOG(INFO) << "vbmeta digest for target slot: " << encoded_digest;
return base::ToLowerASCII(encoded_digest);
}
diff --git a/payload_consumer/filesystem_verifier_action.cc b/payload_consumer/filesystem_verifier_action.cc
index dfbe513..abfae72 100644
--- a/payload_consumer/filesystem_verifier_action.cc
+++ b/payload_consumer/filesystem_verifier_action.cc
@@ -136,6 +136,13 @@
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 b2ed158..3c3997d 100644
--- a/payload_consumer/filesystem_verifier_action_unittest.cc
+++ b/payload_consumer/filesystem_verifier_action_unittest.cc
@@ -32,6 +32,7 @@
#include <libsnapshot/snapshot_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"
@@ -556,12 +557,13 @@
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>();
@@ -661,6 +663,7 @@
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 a6fa4b3..fa4654d 100644
--- a/payload_consumer/postinstall_runner_action.cc
+++ b/payload_consumer/postinstall_runner_action.cc
@@ -109,7 +109,7 @@
auto dynamic_control = boot_control_->GetDynamicPartitionControl();
CHECK(dynamic_control);
- // Mount snapshot partitions for Virtual AB Compression Compression.
+ // Mount snapshot partitions for Virtual AB Compression
if (dynamic_control->UpdateUsesSnapshotCompression()) {
// Before calling MapAllPartitions to map snapshot devices, all CowWriters
// must be closed, and MapAllPartitions() should be called.
@@ -434,9 +434,8 @@
// steps succeeded.
if (error_code == ErrorCode::kSuccess) {
if (install_plan_.switch_slot_on_reboot) {
- if (!boot_control_->GetDynamicPartitionControl()->FinishUpdate(
- install_plan_.powerwash_required) ||
- !boot_control_->SetActiveBootSlot(install_plan_.target_slot)) {
+ if (!boot_control_->SetActiveBootSlot(install_plan_.target_slot)) {
+ LOG(ERROR) << "Failed to switch slot to " << install_plan_.target_slot;
error_code = ErrorCode::kPostinstallRunnerError;
} else {
// Schedules warm reset on next reboot, ignores the error.
