diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index 86ff5f710..51389a0db 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -729,6 +729,14 @@ bool SnapshotManager::DeleteSnapshot(LockedFile* lock, const std::string& name) LOG(ERROR) << "Failed to remove status file " << file_path << ": " << error; return false; } + + // This path may never exist. If it is present, then it's a stale + // snapshot status file. Just remove the file and log the message. + const std::string tmp_path = file_path + ".tmp"; + if (!android::base::RemoveFileIfExists(tmp_path, &error)) { + LOG(ERROR) << "Failed to remove stale snapshot file " << tmp_path; + } + return true; } @@ -754,10 +762,10 @@ bool SnapshotManager::InitiateMerge() { return false; } - auto other_suffix = device_->GetOtherSlotSuffix(); + auto current_slot_suffix = device_->GetSlotSuffix(); for (const auto& snapshot : snapshots) { - if (android::base::EndsWith(snapshot, other_suffix)) { + if (!android::base::EndsWith(snapshot, current_slot_suffix)) { // Allow the merge to continue, but log this unexpected case. LOG(ERROR) << "Unexpected snapshot found during merge: " << snapshot; continue; @@ -1123,7 +1131,7 @@ auto SnapshotManager::CheckMergeState(LockedFile* lock, const std::functionGetOtherSlotSuffix(); + auto current_slot_suffix = device_->GetSlotSuffix(); bool cancelled = false; bool merging = false; @@ -1131,9 +1139,9 @@ auto SnapshotManager::CheckMergeState(LockedFile* lock, const std::functionInitiateMerge()); + // Create stale files in snapshot directory. Merge should skip these files + // as the suffix doesn't match the current slot. + auto tmp_path = test_device->GetMetadataDir() + "/snapshots/test_partition_b.tmp"; + auto other_slot = test_device->GetMetadataDir() + "/snapshots/test_partition_a"; + + unique_fd fd(open(tmp_path.c_str(), O_RDWR | O_CLOEXEC | O_CREAT, 0644)); + ASSERT_GE(fd, 0); + + fd.reset(open(other_slot.c_str(), O_RDWR | O_CLOEXEC | O_CREAT, 0644)); + ASSERT_GE(fd, 0); + // The device should have been switched to a snapshot-merge target. DeviceMapper::TargetInfo target; ASSERT_TRUE(sm->IsSnapshotDevice("test_partition_b", &target)); @@ -700,13 +711,23 @@ TEST_F(SnapshotTest, Merge) { ASSERT_EQ(sm->ProcessUpdateState(), UpdateState::MergeCompleted); ASSERT_EQ(sm->GetUpdateState(), UpdateState::None); + // Make sure that snapshot states are cleared and all stale files + // are deleted + { + ASSERT_TRUE(AcquireLock()); + auto local_lock = std::move(lock_); + std::vector snapshots; + ASSERT_TRUE(sm->ListSnapshots(local_lock.get(), &snapshots)); + ASSERT_TRUE(snapshots.empty()); + } + // The device should no longer be a snapshot or snapshot-merge. ASSERT_FALSE(sm->IsSnapshotDevice("test_partition_b")); // Test that we can read back the string we wrote to the snapshot. Note // that the base device is gone now. |snap_device| contains the correct // partition. - unique_fd fd(open("/dev/block/mapper/test_partition_b", O_RDONLY | O_CLOEXEC)); + fd.reset(open("/dev/block/mapper/test_partition_b", O_RDONLY | O_CLOEXEC)); ASSERT_GE(fd, 0); std::string buffer(test_string.size(), '\0');