Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3983f9aa authored by Akilesh Kailash's avatar Akilesh Kailash
Browse files

libsnapshot: Check for valid snapshots based on current slot 



We may have snapshot files in /metadata/ota/snapshot/ which ends with
.tmp such as system_a.tmp - This happens if the device
reboots just before `rename` in `WriteStringToFileAtomic`. This
can lead to spurious merge failures.

Log the error and skip these snapshot files. It is ok to skip
as we will still have original snapshot status files since
we are already in the merge path. Additionally, try to remove
these files when snapshot is deleted.

Bug: 292198189
Test: OTA
Change-Id: I5db3dbd5a919b263ae577185de3e7f79a5e9b89a
Signed-off-by: default avatarAkilesh Kailash <akailash@google.com>
parent a981d589

fs_mgr/libsnapshot/snapshot.cpp

+13 −5
Original line number Diff line number Diff line
@@ -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::function<bool 
        return MergeResult(UpdateState::MergeFailed, MergeFailureCode::ListSnapshots);

    }



    auto other_suffix = device_->GetOtherSlotSuffix();

    auto current_slot_suffix = device_->GetSlotSuffix();



    bool cancelled = false;

    bool merging = false;
@@ -1131,9 +1139,9 @@ auto SnapshotManager::CheckMergeState(LockedFile* lock, const std::function<bool 
    bool wrong_phase = false;

    MergeFailureCode failure_code = MergeFailureCode::Ok;

    for (const auto& snapshot : snapshots) {

        if (android::base::EndsWith(snapshot, other_suffix)) {

        if (!android::base::EndsWith(snapshot, current_slot_suffix)) {

            // This will have triggered an error message in InitiateMerge already.

            LOG(INFO) << "Skipping merge validation of unexpected snapshot: " << snapshot;

            LOG(ERROR) << "Skipping merge validation of unexpected snapshot: " << snapshot;

            continue;

        }

fs_mgr/libsnapshot/snapshot_test.cpp

+22 −1
Original line number Diff line number Diff line
@@ -685,6 +685,17 @@ TEST_F(SnapshotTest, Merge) { 
    }

    ASSERT_TRUE(sm->InitiateMerge());



    // 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<std::string> 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');