From 0b32b199ac1775415e4d6094a34ffd57a9e5d202 Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Tue, 17 Sep 2019 15:45:17 -0700 Subject: [PATCH 1/2] libsnapshot: operator<< for UpdateState Test: pass Change-Id: Ifc88666d0b7c76f3ea9b7ec662398084d631c4e6 --- .../include/libsnapshot/snapshot.h | 2 ++ fs_mgr/libsnapshot/snapshot.cpp | 33 +++++++++---------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h index aeeb4aabb..8d29e40c7 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; diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index b5b3af353..0bf0365c8 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -1526,34 +1526,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())) { From 6aa77dd27126d5e58bd75711610edf8c43e2a950 Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Tue, 17 Sep 2019 15:20:34 -0700 Subject: [PATCH 2/2] libsnapshot: Cancel/merge existing update before begin Attempt to cancel or merge an existing update before starting a new one. Test: libsnapshot_test Change-Id: Ic534db846f1fe97bd7ca1f176a9494fbc6a0f8d5 --- .../include/libsnapshot/snapshot.h | 12 +++++++-- fs_mgr/libsnapshot/snapshot.cpp | 27 +++++++++++++++++-- fs_mgr/libsnapshot/snapshot_test.cpp | 3 +-- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h index 8d29e40c7..6130a10fe 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h @@ -127,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 @@ -395,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 0bf0365c8..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) { 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.