Merge changes Idcb30151,I0f746870 am: 04502383f6
Original change: https://android-review.googlesource.com/c/platform/system/core/+/1611274
MUST ONLY BE SUBMITTED BY AUTOMERGER
Change-Id: I0c8906d9523ab7d1217c40959954365c6e098883
diff --git a/fs_mgr/libfiemap/image_manager.cpp b/fs_mgr/libfiemap/image_manager.cpp
index 841f215..44f659b 100644
--- a/fs_mgr/libfiemap/image_manager.cpp
+++ b/fs_mgr/libfiemap/image_manager.cpp
@@ -16,6 +16,8 @@
#include <libfiemap/image_manager.h>
+#include <optional>
+
#include <android-base/file.h>
#include <android-base/logging.h>
#include <android-base/properties.h>
@@ -574,7 +576,7 @@
return false;
}
auto& dm = DeviceMapper::Instance();
- LoopControl loop;
+ std::optional<LoopControl> loop;
std::string status;
auto status_file = GetStatusFilePath(name);
@@ -598,9 +600,14 @@
return false;
}
} else if (pieces[0] == "loop") {
+ // Lazily connect to loop-control to avoid spurious errors in recovery.
+ if (!loop.has_value()) {
+ loop.emplace();
+ }
+
// Failure to remove a loop device is not fatal, since we can still
// remove the backing file if we want.
- loop.Detach(pieces[1]);
+ loop->Detach(pieces[1]);
} else {
LOG(ERROR) << "Unknown status: " << pieces[0];
}
diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
index 0d90f6c..a79a86d 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
@@ -694,8 +694,8 @@
// 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);
+ UpdateState ProcessUpdateStateOnDataWipe(bool allow_forward_merge,
+ const std::function<bool()>& callback);
// Return device string of a mapped image, or if it is not available, the mapped image path.
bool GetMappedImageDeviceStringOrPath(const std::string& device_name,
diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp
index eb3a501..cc2599d 100644
--- a/fs_mgr/libsnapshot/snapshot.cpp
+++ b/fs_mgr/libsnapshot/snapshot.cpp
@@ -894,6 +894,8 @@
const std::function<bool()>& before_cancel) {
while (true) {
UpdateState state = CheckMergeState(before_cancel);
+ LOG(INFO) << "ProcessUpdateState handling state: " << state;
+
if (state == UpdateState::MergeFailed) {
AcknowledgeMergeFailure();
}
@@ -920,13 +922,15 @@
}
UpdateState state = CheckMergeState(lock.get(), before_cancel);
+ LOG(INFO) << "CheckMergeState for snapshots returned: " << state;
+
if (state == UpdateState::MergeCompleted) {
// Do this inside the same lock. Failures get acknowledged without the
// lock, because flock() might have failed.
AcknowledgeMergeSuccess(lock.get());
} else if (state == UpdateState::Cancelled) {
- if (!RemoveAllUpdateState(lock.get(), before_cancel)) {
- return ReadSnapshotUpdateStatus(lock.get()).state();
+ if (!device_->IsRecovery() && !RemoveAllUpdateState(lock.get(), before_cancel)) {
+ LOG(ERROR) << "Failed to remove all update state after acknowleding cancelled update.";
}
}
return state;
@@ -968,13 +972,23 @@
return UpdateState::MergeFailed;
}
+ auto other_suffix = device_->GetOtherSlotSuffix();
+
bool cancelled = false;
bool failed = false;
bool merging = false;
bool needs_reboot = false;
bool wrong_phase = false;
for (const auto& snapshot : snapshots) {
+ if (android::base::EndsWith(snapshot, other_suffix)) {
+ // This will have triggered an error message in InitiateMerge already.
+ LOG(INFO) << "Skipping merge validation of unexpected snapshot: " << snapshot;
+ continue;
+ }
+
UpdateState snapshot_state = CheckTargetMergeState(lock, snapshot, update_status);
+ LOG(INFO) << "CheckTargetMergeState for " << snapshot << " returned: " << snapshot_state;
+
switch (snapshot_state) {
case UpdateState::MergeFailed:
failed = true;
@@ -1173,7 +1187,7 @@
// indicator that cleanup is needed on reboot. If a factory data reset
// was requested, it doesn't matter, everything will get wiped anyway.
// To make testing easier we consider a /data wipe as cleaned up.
- if (device_->IsRecovery() && !in_factory_data_reset_) {
+ if (device_->IsRecovery()) {
WriteUpdateState(lock, UpdateState::MergeCompleted);
return;
}
@@ -1692,6 +1706,7 @@
for (const auto& snapshot : snapshots) {
DmTargetSnapshot::Status current_status;
+ if (!IsSnapshotDevice(snapshot)) continue;
if (!QuerySnapshotStatus(snapshot, nullptr, ¤t_status)) continue;
fake_snapshots_status.sectors_allocated += current_status.sectors_allocated;
@@ -3212,10 +3227,11 @@
};
in_factory_data_reset_ = true;
- bool ok = ProcessUpdateStateOnDataWipe(true /* allow_forward_merge */, process_callback);
+ UpdateState state =
+ ProcessUpdateStateOnDataWipe(true /* allow_forward_merge */, process_callback);
in_factory_data_reset_ = false;
- if (!ok) {
+ if (state == UpdateState::MergeFailed) {
return false;
}
@@ -3223,6 +3239,16 @@
if (!UnmapAllPartitionsInRecovery()) {
LOG(ERROR) << "Unable to unmap all partitions; fastboot may fail to flash.";
}
+
+ if (state != UpdateState::None) {
+ auto lock = LockExclusive();
+ if (!lock) return false;
+
+ // Zap the update state so the bootloader doesn't think we're still
+ // merging. It's okay if this fails, it's informative only at this
+ // point.
+ WriteUpdateState(lock.get(), UpdateState::None);
+ }
return true;
}
@@ -3257,15 +3283,15 @@
return true;
}
-bool SnapshotManager::ProcessUpdateStateOnDataWipe(bool allow_forward_merge,
- const std::function<bool()>& callback) {
+UpdateState 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;
+ return state;
case UpdateState::Unverified: {
// If an OTA was just applied but has not yet started merging:
//
@@ -3285,8 +3311,12 @@
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);
+
+ if (!InitiateMerge()) {
+ LOG(ERROR) << "Failed to initiate merge on data wipe.";
+ return UpdateState::MergeFailed;
+ }
+ return ProcessUpdateStateOnDataWipe(false /* allow_forward_merge */, callback);
}
LOG(ERROR) << "Reverting to old slot since update will be deleted.";
@@ -3304,7 +3334,7 @@
default:
break;
}
- return true;
+ return state;
}
bool SnapshotManager::EnsureNoOverflowSnapshot(LockedFile* lock) {
diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp
index d57aa6c..bde4cca 100644
--- a/fs_mgr/libsnapshot/snapshot_test.cpp
+++ b/fs_mgr/libsnapshot/snapshot_test.cpp
@@ -636,8 +636,8 @@
// Because the status is Merging, we must call ProcessUpdateState, which should
// detect a cancelled update.
- ASSERT_EQ(sm->ProcessUpdateState(), UpdateState::Cancelled);
- ASSERT_EQ(sm->GetUpdateState(), UpdateState::None);
+ ASSERT_EQ(init->ProcessUpdateState(), UpdateState::Cancelled);
+ ASSERT_EQ(init->GetUpdateState(), UpdateState::None);
}
TEST_F(SnapshotTest, UpdateBootControlHal) {
@@ -1767,7 +1767,7 @@
ASSERT_TRUE(new_sm->HandleImminentDataWipe());
// Manually mount metadata so that we can call GetUpdateState() below.
MountMetadata();
- EXPECT_EQ(new_sm->GetUpdateState(), UpdateState::Unverified);
+ EXPECT_EQ(new_sm->GetUpdateState(), UpdateState::None);
EXPECT_TRUE(test_device->IsSlotUnbootable(1));
EXPECT_FALSE(test_device->IsSlotUnbootable(0));
}
@@ -2105,8 +2105,12 @@
// There should be no snapshot to merge.
auto new_sm = SnapshotManager::New(new TestDeviceInfo(fake_super, flashed_slot_suffix));
- // update_enigne calls ProcessUpdateState first -- should see Cancelled.
- ASSERT_EQ(UpdateState::Cancelled, new_sm->ProcessUpdateState());
+ if (flashed_slot == 0 && after_merge) {
+ ASSERT_EQ(UpdateState::MergeCompleted, new_sm->ProcessUpdateState());
+ } else {
+ // update_engine calls ProcessUpdateState first -- should see Cancelled.
+ ASSERT_EQ(UpdateState::Cancelled, new_sm->ProcessUpdateState());
+ }
// Next OTA calls CancelUpdate no matter what.
ASSERT_TRUE(new_sm->CancelUpdate());