Merge changes from topic "fdr_merge" into rvc-dev

* changes:
  libsnapshot: Allow forward merge on FDR
  libsnapshot: Place forward merge indicator for wipe
  libsnapshot: Add allow-forward-merge indicator.
  libsnapshot: Fix DataWipeAfterRollback test
  libsnapshot: Fix intermittent test failure due to missing null check.
  libsnapshot: dump rollback indicator
This commit is contained in:
Yifan Hong 2020-03-30 19:12:51 +00:00 committed by Android (Google) Code Review
commit 5d4ed0240a
3 changed files with 149 additions and 30 deletions

View file

@ -142,7 +142,9 @@ class SnapshotManager final {
// be created, and the device must either cancel the OTA (either before
// rebooting or after rolling back), or merge the OTA.
// Before calling this function, all snapshots must be mapped.
bool FinishedSnapshotWrites();
// If |wipe| is set to true, wipe is scheduled after reboot, and snapshots
// may need to be merged before wiping.
bool FinishedSnapshotWrites(bool wipe);
// Initiate a merge on all snapshot devices. This should only be used after an
// update has been marked successful after booting.
@ -452,6 +454,7 @@ class SnapshotManager final {
std::string GetSnapshotBootIndicatorPath();
std::string GetRollbackIndicatorPath();
std::string GetForwardMergeIndicatorPath();
// Return the name of the device holding the "snapshot" or "snapshot-merge"
// target. This may not be the final device presented via MapSnapshot(), if
@ -522,6 +525,17 @@ class SnapshotManager final {
bool ShouldDeleteSnapshot(LockedFile* lock, const std::map<std::string, bool>& flashing_status,
Slot current_slot, const std::string& name);
// Create or delete forward merge indicator given |wipe|. Iff wipe is scheduled,
// allow forward merge on FDR.
bool UpdateForwardMergeIndicator(bool wipe);
// Helper for HandleImminentDataWipe.
// Call ProcessUpdateState and handle states with special rules before data wipe. Specifically,
// if |allow_forward_merge| and allow-forward-merge indicator exists, initiate merge if
// necessary.
bool ProcessUpdateStateOnDataWipe(bool allow_forward_merge,
const std::function<bool()>& callback);
std::string gsid_dir_;
std::string metadata_dir_;
std::unique_ptr<IDeviceInfo> device_;

View file

@ -228,10 +228,17 @@ bool SnapshotManager::RemoveAllUpdateState(LockedFile* lock, const std::function
return false;
}
// It's okay if these fail - first-stage init performs a deeper check after
// It's okay if these fail:
// - For SnapshotBoot and Rollback, first-stage init performs a deeper check after
// reading the indicator file, so it's not a problem if it still exists
// after the update completes.
std::vector<std::string> files = {GetSnapshotBootIndicatorPath(), GetRollbackIndicatorPath()};
// - For ForwardMerge, FinishedSnapshotWrites asserts that the existence of the indicator
// matches the incoming update.
std::vector<std::string> files = {
GetSnapshotBootIndicatorPath(),
GetRollbackIndicatorPath(),
GetForwardMergeIndicatorPath(),
};
for (const auto& file : files) {
RemoveFileIfExists(file);
}
@ -241,7 +248,7 @@ bool SnapshotManager::RemoveAllUpdateState(LockedFile* lock, const std::function
return WriteUpdateState(lock, UpdateState::None);
}
bool SnapshotManager::FinishedSnapshotWrites() {
bool SnapshotManager::FinishedSnapshotWrites(bool wipe) {
auto lock = LockExclusive();
if (!lock) return false;
@ -261,6 +268,10 @@ bool SnapshotManager::FinishedSnapshotWrites() {
return false;
}
if (!UpdateForwardMergeIndicator(wipe)) {
return false;
}
// This file is written on boot to detect whether a rollback occurred. It
// MUST NOT exist before rebooting, otherwise, we're at risk of deleting
// snapshots too early.
@ -992,6 +1003,10 @@ std::string SnapshotManager::GetRollbackIndicatorPath() {
return metadata_dir_ + "/" + android::base::Basename(kRollbackIndicatorPath);
}
std::string SnapshotManager::GetForwardMergeIndicatorPath() {
return metadata_dir_ + "/allow-forward-merge";
}
void SnapshotManager::AcknowledgeMergeSuccess(LockedFile* lock) {
RemoveAllUpdateState(lock);
}
@ -2435,6 +2450,12 @@ bool SnapshotManager::Dump(std::ostream& os) {
ss << "Current slot: " << device_->GetSlotSuffix() << std::endl;
ss << "Boot indicator: booting from " << GetCurrentSlot() << " slot" << std::endl;
ss << "Rollback indicator: "
<< (access(GetRollbackIndicatorPath().c_str(), F_OK) == 0 ? "exists" : strerror(errno))
<< std::endl;
ss << "Forward merge indicator: "
<< (access(GetForwardMergeIndicatorPath().c_str(), F_OK) == 0 ? "exists" : strerror(errno))
<< std::endl;
bool ok = true;
std::vector<std::string> snapshots;
@ -2501,17 +2522,39 @@ bool SnapshotManager::HandleImminentDataWipe(const std::function<void()>& callba
return false;
}
UpdateState state = ProcessUpdateState([&]() -> bool {
callback();
auto process_callback = [&]() -> bool {
if (callback) {
callback();
}
return true;
});
};
if (!ProcessUpdateStateOnDataWipe(true /* allow_forward_merge */, process_callback)) {
return false;
}
// Nothing should be depending on partitions now, so unmap them all.
if (!UnmapAllPartitions()) {
LOG(ERROR) << "Unable to unmap all partitions; fastboot may fail to flash.";
}
return true;
}
bool SnapshotManager::ProcessUpdateStateOnDataWipe(bool allow_forward_merge,
const std::function<bool()>& callback) {
auto slot_number = SlotNumberForSlotSuffix(device_->GetSlotSuffix());
UpdateState state = ProcessUpdateState(callback);
LOG(INFO) << "Update state in recovery: " << state;
switch (state) {
case UpdateState::MergeFailed:
LOG(ERROR) << "Unrecoverable merge failure detected.";
return false;
case UpdateState::Unverified: {
// If an OTA was just applied but has not yet started merging, we
// If an OTA was just applied but has not yet started merging:
//
// - if forward merge is allowed, initiate merge and call
// ProcessUpdateState again.
//
// - if forward merge is not allowed, we
// have no choice but to revert slots, because the current slot will
// immediately become unbootable. Rather than wait for the device
// to reboot N times until a rollback, we proactively disable the
@ -2521,8 +2564,17 @@ bool SnapshotManager::HandleImminentDataWipe(const std::function<void()>& callba
// as an error here.
auto slot = GetCurrentSlot();
if (slot == Slot::Target) {
if (allow_forward_merge &&
access(GetForwardMergeIndicatorPath().c_str(), F_OK) == 0) {
LOG(INFO) << "Forward merge allowed, initiating merge now.";
return InitiateMerge() &&
ProcessUpdateStateOnDataWipe(false /* allow_forward_merge */, callback);
}
LOG(ERROR) << "Reverting to old slot since update will be deleted.";
device_->SetSlotAsUnbootable(slot_number);
} else {
LOG(INFO) << "Booting from " << slot << " slot, no action is taken.";
}
break;
}
@ -2534,11 +2586,6 @@ bool SnapshotManager::HandleImminentDataWipe(const std::function<void()>& callba
default:
break;
}
// Nothing should be depending on partitions now, so unmap them all.
if (!UnmapAllPartitions()) {
LOG(ERROR) << "Unable to unmap all partitions; fastboot may fail to flash.";
}
return true;
}
@ -2619,5 +2666,24 @@ CreateResult SnapshotManager::RecoveryCreateSnapshotDevices(
return CreateResult::CREATED;
}
bool SnapshotManager::UpdateForwardMergeIndicator(bool wipe) {
auto path = GetForwardMergeIndicatorPath();
if (!wipe) {
LOG(INFO) << "Wipe is not scheduled. Deleting forward merge indicator.";
return RemoveFileIfExists(path);
}
// TODO(b/152094219): Don't forward merge if no CoW file is allocated.
LOG(INFO) << "Wipe will be scheduled. Allowing forward merge of snapshots.";
if (!android::base::WriteStringToFile("1", path)) {
PLOG(ERROR) << "Unable to write forward merge indicator: " << path;
return false;
}
return true;
}
} // namespace snapshot
} // namespace android

View file

@ -320,7 +320,7 @@ class SnapshotTest : public ::testing::Test {
// Simulate a reboot into the new slot.
AssertionResult SimulateReboot() {
lock_ = nullptr;
if (!sm->FinishedSnapshotWrites()) {
if (!sm->FinishedSnapshotWrites(false)) {
return AssertionFailure();
}
if (!dm_.DeleteDevice("test_partition_b")) {
@ -424,7 +424,7 @@ TEST_F(SnapshotTest, MapPartialSnapshot) {
}
TEST_F(SnapshotTest, NoMergeBeforeReboot) {
ASSERT_TRUE(sm->FinishedSnapshotWrites());
ASSERT_TRUE(sm->FinishedSnapshotWrites(false));
// Merge should fail, since the slot hasn't changed.
ASSERT_FALSE(sm->InitiateMerge());
@ -440,7 +440,7 @@ TEST_F(SnapshotTest, CleanFirstStageMount) {
}
TEST_F(SnapshotTest, FirstStageMountAfterRollback) {
ASSERT_TRUE(sm->FinishedSnapshotWrites());
ASSERT_TRUE(sm->FinishedSnapshotWrites(false));
// We didn't change the slot, so we shouldn't need snapshots.
TestDeviceInfo* info = new TestDeviceInfo(fake_super);
@ -476,7 +476,7 @@ TEST_F(SnapshotTest, Merge) {
lock_ = nullptr;
// Done updating.
ASSERT_TRUE(sm->FinishedSnapshotWrites());
ASSERT_TRUE(sm->FinishedSnapshotWrites(false));
test_device->set_slot_suffix("_b");
ASSERT_TRUE(sm->InitiateMerge());
@ -1007,7 +1007,7 @@ TEST_F(SnapshotUpdateTest, FullUpdateFlow) {
ASSERT_TRUE(IsPartitionUnchanged(name));
}
ASSERT_TRUE(sm->FinishedSnapshotWrites());
ASSERT_TRUE(sm->FinishedSnapshotWrites(false));
// Simulate shutting down the device.
ASSERT_TRUE(UnmapAll());
@ -1139,7 +1139,7 @@ TEST_F(SnapshotUpdateTest, TestRollback) {
ASSERT_TRUE(IsPartitionUnchanged(name));
}
ASSERT_TRUE(sm->FinishedSnapshotWrites());
ASSERT_TRUE(sm->FinishedSnapshotWrites(false));
// Simulate shutting down the device.
ASSERT_TRUE(UnmapAll());
@ -1171,7 +1171,7 @@ TEST_F(SnapshotUpdateTest, TestRollback) {
// Test that if an update is applied but not booted into, it can be canceled.
TEST_F(SnapshotUpdateTest, CancelAfterApply) {
ASSERT_TRUE(sm->BeginUpdate());
ASSERT_TRUE(sm->FinishedSnapshotWrites());
ASSERT_TRUE(sm->FinishedSnapshotWrites(false));
ASSERT_TRUE(sm->CancelUpdate());
}
@ -1188,7 +1188,7 @@ TEST_F(SnapshotUpdateTest, ReclaimCow) {
ASSERT_TRUE(sm->BeginUpdate());
ASSERT_TRUE(sm->CreateUpdateSnapshots(manifest_));
ASSERT_TRUE(MapUpdateSnapshots());
ASSERT_TRUE(sm->FinishedSnapshotWrites());
ASSERT_TRUE(sm->FinishedSnapshotWrites(false));
// Simulate shutting down the device.
ASSERT_TRUE(UnmapAll());
@ -1295,7 +1295,7 @@ TEST_F(SnapshotUpdateTest, RetrofitAfterRegularAb) {
ASSERT_TRUE(IsPartitionUnchanged(name));
}
ASSERT_TRUE(sm->FinishedSnapshotWrites());
ASSERT_TRUE(sm->FinishedSnapshotWrites(false));
}
TEST_F(SnapshotUpdateTest, MergeCannotRemoveCow) {
@ -1324,7 +1324,7 @@ TEST_F(SnapshotUpdateTest, MergeCannotRemoveCow) {
ASSERT_TRUE(sm->BeginUpdate());
ASSERT_TRUE(sm->CreateUpdateSnapshots(manifest_));
ASSERT_TRUE(MapUpdateSnapshots());
ASSERT_TRUE(sm->FinishedSnapshotWrites());
ASSERT_TRUE(sm->FinishedSnapshotWrites(false));
// Simulate shutting down the device.
ASSERT_TRUE(UnmapAll());
@ -1428,7 +1428,7 @@ TEST_F(SnapshotUpdateTest, MergeInRecovery) {
ASSERT_TRUE(sm->BeginUpdate());
ASSERT_TRUE(sm->CreateUpdateSnapshots(manifest_));
ASSERT_TRUE(MapUpdateSnapshots());
ASSERT_TRUE(sm->FinishedSnapshotWrites());
ASSERT_TRUE(sm->FinishedSnapshotWrites(false));
// Simulate shutting down the device.
ASSERT_TRUE(UnmapAll());
@ -1460,7 +1460,7 @@ TEST_F(SnapshotUpdateTest, DataWipeRollbackInRecovery) {
ASSERT_TRUE(sm->BeginUpdate());
ASSERT_TRUE(sm->CreateUpdateSnapshots(manifest_));
ASSERT_TRUE(MapUpdateSnapshots());
ASSERT_TRUE(sm->FinishedSnapshotWrites());
ASSERT_TRUE(sm->FinishedSnapshotWrites(false));
// Simulate shutting down the device.
ASSERT_TRUE(UnmapAll());
@ -1485,7 +1485,7 @@ TEST_F(SnapshotUpdateTest, DataWipeAfterRollback) {
ASSERT_TRUE(sm->BeginUpdate());
ASSERT_TRUE(sm->CreateUpdateSnapshots(manifest_));
ASSERT_TRUE(MapUpdateSnapshots());
ASSERT_TRUE(sm->FinishedSnapshotWrites());
ASSERT_TRUE(sm->FinishedSnapshotWrites(false));
// Simulate shutting down the device.
ASSERT_TRUE(UnmapAll());
@ -1498,7 +1498,46 @@ TEST_F(SnapshotUpdateTest, DataWipeAfterRollback) {
ASSERT_TRUE(new_sm->HandleImminentDataWipe());
EXPECT_EQ(new_sm->GetUpdateState(), UpdateState::None);
EXPECT_FALSE(test_device->IsSlotUnbootable(0));
EXPECT_FALSE(test_device->IsSlotUnbootable(0));
EXPECT_FALSE(test_device->IsSlotUnbootable(1));
}
// Test update package that requests data wipe.
TEST_F(SnapshotUpdateTest, DataWipeRequiredInPackage) {
AddOperationForPartitions();
// Execute the update.
ASSERT_TRUE(sm->BeginUpdate());
ASSERT_TRUE(sm->CreateUpdateSnapshots(manifest_));
// Write some data to target partitions.
for (const auto& name : {"sys_b", "vnd_b", "prd_b"}) {
ASSERT_TRUE(WriteSnapshotAndHash(name)) << name;
}
ASSERT_TRUE(sm->FinishedSnapshotWrites(true /* wipe */));
// Simulate shutting down the device.
ASSERT_TRUE(UnmapAll());
// Simulate a reboot into recovery.
auto test_device = new TestDeviceInfo(fake_super, "_b");
test_device->set_recovery(true);
auto new_sm = SnapshotManager::NewForFirstStageMount(test_device);
ASSERT_TRUE(new_sm->HandleImminentDataWipe());
// Manually mount metadata so that we can call GetUpdateState() below.
MountMetadata();
EXPECT_EQ(new_sm->GetUpdateState(), UpdateState::None);
ASSERT_FALSE(test_device->IsSlotUnbootable(1));
ASSERT_FALSE(test_device->IsSlotUnbootable(0));
// Now reboot into new slot.
test_device = new TestDeviceInfo(fake_super, "_b");
auto init = SnapshotManager::NewForFirstStageMount(test_device);
ASSERT_TRUE(init->CreateLogicalAndSnapshotPartitions("super", snapshot_timeout_));
// Verify that we are on the downgraded build.
for (const auto& name : {"sys_b", "vnd_b", "prd_b"}) {
ASSERT_TRUE(IsPartitionUnchanged(name)) << name;
}
}
TEST_F(SnapshotUpdateTest, Hashtree) {
@ -1533,7 +1572,7 @@ TEST_F(SnapshotUpdateTest, Hashtree) {
ASSERT_TRUE(WriteSnapshotAndHash("sys_b", partition_size));
// Finish update.
ASSERT_TRUE(sm->FinishedSnapshotWrites());
ASSERT_TRUE(sm->FinishedSnapshotWrites(false));
// Simulate shutting down the device.
ASSERT_TRUE(UnmapAll());
@ -1569,7 +1608,7 @@ TEST_F(SnapshotUpdateTest, Overflow) {
ASSERT_EQ(1u, table.size());
EXPECT_TRUE(table[0].IsOverflowSnapshot());
ASSERT_FALSE(sm->FinishedSnapshotWrites())
ASSERT_FALSE(sm->FinishedSnapshotWrites(false))
<< "FinishedSnapshotWrites should detect overflow of CoW device.";
}
@ -1623,7 +1662,7 @@ TEST_P(FlashAfterUpdateTest, FlashSlotAfterUpdate) {
ASSERT_TRUE(sm->BeginUpdate());
ASSERT_TRUE(sm->CreateUpdateSnapshots(manifest_));
ASSERT_TRUE(MapUpdateSnapshots());
ASSERT_TRUE(sm->FinishedSnapshotWrites());
ASSERT_TRUE(sm->FinishedSnapshotWrites(false));
// Simulate shutting down the device.
ASSERT_TRUE(UnmapAll());