diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h index aeeb4aabb..6130a10fe 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -84,6 +85,7 @@ enum class UpdateState : unsigned int { // operation via fastboot. This state can only be returned by WaitForMerge. Cancelled }; +std::ostream& operator<<(std::ostream& os, UpdateState state); class SnapshotManager final { using CreateLogicalPartitionParams = android::fs_mgr::CreateLogicalPartitionParams; @@ -125,8 +127,9 @@ class SnapshotManager final { // will fail if GetUpdateState() != None. bool BeginUpdate(); - // Cancel an update; any snapshots will be deleted. This will fail if the - // state != Initiated or None. + // Cancel an update; any snapshots will be deleted. This is allowed if the + // state == Initiated, None, or Unverified (before rebooting to the new + // slot). bool CancelUpdate(); // Mark snapshot writes as having completed. After this, new snapshots cannot @@ -393,6 +396,13 @@ class SnapshotManager final { // The reverse of MapPartitionWithSnapshot. bool UnmapPartitionWithSnapshot(LockedFile* lock, const std::string& target_partition_name); + // If there isn't a previous update, return true. |needs_merge| is set to false. + // If there is a previous update but the device has not boot into it, tries to cancel the + // update and delete any snapshots. Return true if successful. |needs_merge| is set to false. + // If there is a previous update and the device has boot into it, do nothing and return true. + // |needs_merge| is set to true. + bool TryCancelUpdate(bool* needs_merge); + std::string gsid_dir_; std::string metadata_dir_; std::unique_ptr device_; diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index b5b3af353..803bdb550 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -131,6 +131,16 @@ static std::string GetSnapshotExtraDeviceName(const std::string& snapshot_name) } bool SnapshotManager::BeginUpdate() { + bool needs_merge = false; + if (!TryCancelUpdate(&needs_merge)) { + return false; + } + if (needs_merge) { + LOG(INFO) << "Wait for merge (if any) before beginning a new update."; + auto state = ProcessUpdateState(); + LOG(INFO) << "Merged with state = " << state; + } + auto file = LockExclusive(); if (!file) return false; @@ -143,6 +153,19 @@ bool SnapshotManager::BeginUpdate() { } bool SnapshotManager::CancelUpdate() { + bool needs_merge = false; + if (!TryCancelUpdate(&needs_merge)) { + return false; + } + if (needs_merge) { + LOG(ERROR) << "Cannot cancel update after it has completed or started merging"; + } + return !needs_merge; +} + +bool SnapshotManager::TryCancelUpdate(bool* needs_merge) { + *needs_merge = false; + auto file = LockExclusive(); if (!file) return false; @@ -167,8 +190,8 @@ bool SnapshotManager::CancelUpdate() { return RemoveAllUpdateState(file.get()); } } - LOG(ERROR) << "Cannot cancel update after it has completed or started merging"; - return false; + *needs_merge = true; + return true; } bool SnapshotManager::RemoveAllUpdateState(LockedFile* lock) { @@ -1526,34 +1549,33 @@ UpdateState SnapshotManager::ReadUpdateState(LockedFile* file) { } } -bool SnapshotManager::WriteUpdateState(LockedFile* file, UpdateState state) { - std::string contents; +std::ostream& operator<<(std::ostream& os, UpdateState state) { switch (state) { case UpdateState::None: - contents = "none"; - break; + return os << "none"; case UpdateState::Initiated: - contents = "initiated"; - break; + return os << "initiated"; case UpdateState::Unverified: - contents = "unverified"; - break; + return os << "unverified"; case UpdateState::Merging: - contents = "merging"; - break; + return os << "merging"; case UpdateState::MergeCompleted: - contents = "merge-completed"; - break; + return os << "merge-completed"; case UpdateState::MergeNeedsReboot: - contents = "merge-needs-reboot"; - break; + return os << "merge-needs-reboot"; case UpdateState::MergeFailed: - contents = "merge-failed"; - break; + return os << "merge-failed"; default: LOG(ERROR) << "Unknown update state"; - return false; + return os; } +} + +bool SnapshotManager::WriteUpdateState(LockedFile* file, UpdateState state) { + std::stringstream ss; + ss << state; + std::string contents = ss.str(); + if (contents.empty()) return false; if (!Truncate(file)) return false; if (!android::base::WriteStringToFd(contents, file->fd())) { diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp index 36982a9ca..bc764fd2f 100644 --- a/fs_mgr/libsnapshot/snapshot_test.cpp +++ b/fs_mgr/libsnapshot/snapshot_test.cpp @@ -761,8 +761,7 @@ class SnapshotUpdateTest : public SnapshotTest { // Also test UnmapUpdateSnapshot unmaps everything. // Also test first stage mount and merge after this. TEST_F(SnapshotUpdateTest, FullUpdateFlow) { - // OTA client calls CancelUpdate then BeginUpdate before doing anything. - ASSERT_TRUE(sm->CancelUpdate()); + // OTA client calls BeginUpdate before doing anything. ASSERT_TRUE(sm->BeginUpdate()); // OTA client blindly unmaps all partitions that are possibly mapped.