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

Commit 3cb682e3 authored by David Anderson's avatar David Anderson
Browse files

libsnapshot: Improve first test-run and test cleanup. 

This CL fixes a bug where libsnapshot_test failed on the first run. It
also fixes bugs where it could not run if it died in the middle of a
test.

Previously, libsnapshot_test relied on CancelUpdate() to perform
cleanup, which cannot run in certain states. Instead, manually delete
dm devices and COW image files, and forcefully erase any lingering data.

Bug: 136678799
Test: libsnapshot_test gtest
Change-Id: I7b2399a403b387eb47184626e71dcf8674f6ab89
parent d97c946d

fs_mgr/libsnapshot/include/libsnapshot/snapshot.h

+2 −0
Original line number Diff line number Diff line
@@ -189,6 +189,7 @@ class SnapshotManager final { 
    std::unique_ptr<LockedFile> LockExclusive();

    UpdateState ReadUpdateState(LockedFile* file);

    bool WriteUpdateState(LockedFile* file, UpdateState state);

    std::string GetStateFilePath() const;



    // This state is persisted per-snapshot in /metadata/ota/snapshots/.

    struct SnapshotStatus {
@@ -205,6 +206,7 @@ class SnapshotManager final { 
                                                       int lock_flags);

    bool WriteSnapshotStatus(LockedFile* file, const SnapshotStatus& status);

    bool ReadSnapshotStatus(LockedFile* file, SnapshotStatus* status);

    std::string GetSnapshotStatusFilePath(const std::string& name);



    // Return the name of the device holding the "snapshot" or "snapshot-merge"

    // target. This may not be the final device presented via MapSnapshot(), if

fs_mgr/libsnapshot/snapshot.cpp

+17 −2
Original line number Diff line number Diff line
@@ -342,6 +342,12 @@ bool SnapshotManager::RemoveAllSnapshots(LockedFile* lock) { 
}



UpdateState SnapshotManager::GetUpdateState(double* progress) {

    // If we've never started an update, the state file won't exist.

    auto state_file = GetStateFilePath();

    if (access(state_file.c_str(), F_OK) != 0 && errno == ENOENT) {

        return UpdateState::None;

    }



    auto file = LockShared();

    if (!file) {

        return UpdateState::None;
@@ -397,9 +403,13 @@ SnapshotManager::LockedFile::~LockedFile() { 
    }

}



std::string SnapshotManager::GetStateFilePath() const {

    return metadata_dir_ + "/state"s;

}



std::unique_ptr<SnapshotManager::LockedFile> SnapshotManager::OpenStateFile(int open_flags,

                                                                            int lock_flags) {

    auto state_file = metadata_dir_ + "/state"s;

    auto state_file = GetStateFilePath();

    return OpenFile(state_file, open_flags, lock_flags);

}
@@ -471,9 +481,14 @@ bool SnapshotManager::WriteUpdateState(LockedFile* file, UpdateState state) { 
    return true;

}



std::string SnapshotManager::GetSnapshotStatusFilePath(const std::string& name) {

    auto file = metadata_dir_ + "/snapshots/"s + name;

    return file;

}



auto SnapshotManager::OpenSnapshotStatusFile(const std::string& name, int open_flags,

                                             int lock_flags) -> std::unique_ptr<LockedFile> {

    auto file = metadata_dir_ + "/snapshots/"s + name;

    auto file = GetSnapshotStatusFilePath(name);

    return OpenFile(file, open_flags, lock_flags);

}

fs_mgr/libsnapshot/snapshot_test.cpp

+37 −3
Original line number Diff line number Diff line
@@ -22,13 +22,19 @@ 
#include <chrono>

#include <iostream>



#include <android-base/file.h>

#include <android-base/strings.h>

#include <android-base/unique_fd.h>

#include <gtest/gtest.h>

#include <libdm/dm.h>

#include <libfiemap/image_manager.h>



namespace android {

namespace snapshot {



using android::base::unique_fd;

using android::dm::DeviceMapper;

using android::dm::DmDeviceState;

using namespace std::chrono_literals;

using namespace std::string_literals;
@@ -48,12 +54,15 @@ std::unique_ptr<SnapshotManager> sm; 
TestDeviceInfo* test_device = nullptr;



class SnapshotTest : public ::testing::Test {

  public:

    SnapshotTest() : dm_(DeviceMapper::Instance()) {}



  protected:

    void SetUp() override {

        test_device->set_is_running_snapshot(false);



        if (sm->GetUpdateState() != UpdateState::None) {

            ASSERT_TRUE(sm->CancelUpdate());

            CleanupTestArtifacts();

        }

        ASSERT_TRUE(sm->BeginUpdate());

        ASSERT_TRUE(sm->EnsureImageManager());
@@ -65,13 +74,37 @@ class SnapshotTest : public ::testing::Test { 
    void TearDown() override {

        lock_ = nullptr;



        if (sm->GetUpdateState() != UpdateState::None) {

            ASSERT_TRUE(sm->CancelUpdate());

        CleanupTestArtifacts();

    }



    void CleanupTestArtifacts() {

        // Normally cancelling inside a merge is not allowed. Since these

        // are tests, we don't care, destroy everything that might exist.

        std::vector<std::string> snapshots = {"test-snapshot"};

        for (const auto& snapshot : snapshots) {

            if (dm_.GetState(snapshot) != DmDeviceState::INVALID) {

                dm_.DeleteDevice(snapshot);

            }

            if (dm_.GetState(snapshot + "-inner") != DmDeviceState::INVALID) {

                dm_.DeleteDevice(snapshot + "-inner");

            }

            temp_images_.emplace_back(snapshot + "-cow");



            auto status_file = sm->GetSnapshotStatusFilePath(snapshot);

            android::base::RemoveFileIfExists(status_file);

        }



        // Remove all images.

        temp_images_.emplace_back("test-snapshot-cow");

        for (const auto& temp_image : temp_images_) {

            image_manager_->UnmapImageDevice(temp_image);

            image_manager_->DeleteBackingImage(temp_image);

        }



        if (sm->GetUpdateState() != UpdateState::None) {

            auto state_file = sm->GetStateFilePath();

            unlink(state_file.c_str());

        }

    }



    bool AcquireLock() {
@@ -87,6 +120,7 @@ class SnapshotTest : public ::testing::Test { 
        return image_manager_->MapImageDevice(name, 10s, path);

    }



    DeviceMapper& dm_;

    std::unique_ptr<SnapshotManager::LockedFile> lock_;

    std::vector<std::string> temp_images_;

    android::fiemap::IImageManager* image_manager_ = nullptr;