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

Commit 943dd5cf authored by David Anderson's avatar David Anderson
Browse files

libsnapshot: Get DaemonTransition test passing again. 

This fixes a number of small bugs in libsnapshot. It also refactors the
handler list a bit. Previously, it was a list of unique_ptrs. Now it is
a list of shared_ptrs to simplify ownership.

Additionally, Snapuserd is now keyed solely on the misc device name.
This allows two identical snapshots to run in the same daemon, with
different control names (a scenario that comes up in the
DaemonTransition test). As part of this change, the two-stage
initialization process has been refactored slightly. The "init" message
sets all the device paths, and the "start" message needs only the misc
name.

Both the init and start messages now validate that no duplicate handlers
exist, and that we're not overwriting any previous thread.

This cleanup also fixes a bug in DmUserHandler cleanup - if a control
device shut down raced with WaitForDelete(), the std::thread object
would delete without a call to detach() or join(). In the new
RemoveHandler(), we now correctly detach() in this scenario.

This also fixes a bug where, if a COW had no partition component (it
only resided on /data), the second-stage transition would fail because
it used the wrong device-mapper name.

Bug: N/A
Test: vts_libsnapshot_test
Change-Id: Ib4a281a3b5fe665c727c7077672e3c6b0b3abdba
parent 3e3159c8

fs_mgr/libsnapshot/cow_snapuserd_test.cpp

+6 −6
Original line number Original line Diff line number Diff line
@@ -288,20 +288,20 @@ void SnapuserdTest::CreateProductDmUser(std::unique_ptr<TemporaryFile>& cow) { 
}

}





void SnapuserdTest::InitCowDevices() {

void SnapuserdTest::InitCowDevices() {

    system_blksize_ = client_->InitDmUserCow(cow_system_->path);

    system_blksize_ = client_->InitDmUserCow(system_device_ctrl_name_, cow_system_->path,

                                             system_a_loop_->device());

    ASSERT_NE(system_blksize_, 0);

    ASSERT_NE(system_blksize_, 0);





    product_blksize_ = client_->InitDmUserCow(cow_product_->path);

    product_blksize_ = client_->InitDmUserCow(product_device_ctrl_name_, cow_product_->path,

                                              product_a_loop_->device());

    ASSERT_NE(product_blksize_, 0);

    ASSERT_NE(product_blksize_, 0);

}

}





void SnapuserdTest::InitDaemon() {

void SnapuserdTest::InitDaemon() {

    bool ok = client_->InitializeSnapuserd(cow_system_->path, system_a_loop_->device(),

    bool ok = client_->AttachDmUser(system_device_ctrl_name_);

                                           GetSystemControlPath());

    ASSERT_TRUE(ok);

    ASSERT_TRUE(ok);





    ok = client_->InitializeSnapuserd(cow_product_->path, product_a_loop_->device(),

    ok = client_->AttachDmUser(product_device_ctrl_name_);

                                      GetProductControlPath());

    ASSERT_TRUE(ok);

    ASSERT_TRUE(ok);

}

}

fs_mgr/libsnapshot/include/libsnapshot/snapuserd.h

+7 −3
Original line number Original line Diff line number Diff line
@@ -61,12 +61,15 @@ class BufferSink : public IByteSink { 




class Snapuserd final {

class Snapuserd final {

  public:

  public:

    bool InitBackingAndControlDevice(std::string& backing_device, std::string& control_device);

    Snapuserd(const std::string& misc_name, const std::string& cow_device,

    bool InitCowDevice(std::string& cow_device);

              const std::string& backing_device);

    bool InitBackingAndControlDevice();

    bool InitCowDevice();

    int Run();

    int Run();

    const std::string& GetControlDevicePath() { return control_device_; }

    const std::string& GetControlDevicePath() { return control_device_; }

    const std::string& GetCowDevice() { return cow_device_; }

    const std::string& GetMiscName() { return misc_name_; }

    uint64_t GetNumSectors() { return num_sectors_; }

    uint64_t GetNumSectors() { return num_sectors_; }

    bool IsAttached() const { return ctrl_fd_ >= 0; }





  private:

  private:

    int ReadDmUserHeader();

    int ReadDmUserHeader();
@@ -96,6 +99,7 @@ class Snapuserd final { 
    std::string cow_device_;

    std::string cow_device_;

    std::string backing_store_device_;

    std::string backing_store_device_;

    std::string control_device_;

    std::string control_device_;

    std::string misc_name_;





    unique_fd cow_fd_;

    unique_fd cow_fd_;

    unique_fd backing_store_fd_;

    unique_fd backing_store_fd_;

fs_mgr/libsnapshot/include/libsnapshot/snapuserd_client.h

+13 −3
Original line number Original line Diff line number Diff line
@@ -58,9 +58,19 @@ class SnapuserdClient { 
                                                    std::chrono::milliseconds timeout_ms);

                                                    std::chrono::milliseconds timeout_ms);





    bool StopSnapuserd();

    bool StopSnapuserd();

    uint64_t InitDmUserCow(const std::string& cow_device);



    bool InitializeSnapuserd(const std::string& cow_device, const std::string& backing_device,

    // Initializing a snapuserd handler is a three-step process:

                             const std::string& control_device);

    //

    //  1. Client invokes InitDmUserCow. This creates the snapuserd handler and validates the

    //     COW. The number of sectors required for the dm-user target is returned.

    //  2. Client creates the device-mapper device with the dm-user target.

    //  3. Client calls AttachControlDevice.

    //

    // The misc_name must be the "misc_name" given to dm-user in step 2.

    //

    uint64_t InitDmUserCow(const std::string& misc_name, const std::string& cow_device,

                           const std::string& backing_device);

    bool AttachDmUser(const std::string& misc_name);





    // Wait for snapuserd to disassociate with a dm-user control device. This

    // Wait for snapuserd to disassociate with a dm-user control device. This

    // must ONLY be called if the control device has already been deleted.

    // must ONLY be called if the control device has already been deleted.

fs_mgr/libsnapshot/include/libsnapshot/snapuserd_server.h

+9 −7
Original line number Original line Diff line number Diff line
@@ -56,7 +56,7 @@ class DmUserHandler { 
    const std::unique_ptr<Snapuserd>& snapuserd() const { return snapuserd_; }

    const std::unique_ptr<Snapuserd>& snapuserd() const { return snapuserd_; }

    std::thread& thread() { return thread_; }

    std::thread& thread() { return thread_; }





    const std::string& GetControlDevice() const;

    const std::string& GetMiscName() const;

};

};





class Stoppable {

class Stoppable {
@@ -85,7 +85,9 @@ class SnapuserdServer : public Stoppable { 
    std::vector<struct pollfd> watched_fds_;

    std::vector<struct pollfd> watched_fds_;





    std::mutex lock_;

    std::mutex lock_;

    std::vector<std::unique_ptr<DmUserHandler>> dm_users_;



    using HandlerList = std::vector<std::shared_ptr<DmUserHandler>>;

    HandlerList dm_users_;





    void AddWatchedFd(android::base::borrowed_fd fd);

    void AddWatchedFd(android::base::borrowed_fd fd);

    void AcceptClient();

    void AcceptClient();
@@ -95,7 +97,7 @@ class SnapuserdServer : public Stoppable { 
    bool Receivemsg(android::base::borrowed_fd fd, const std::string& str);

    bool Receivemsg(android::base::borrowed_fd fd, const std::string& str);





    void ShutdownThreads();

    void ShutdownThreads();

    bool WaitForDelete(const std::string& control_device);

    bool RemoveHandler(const std::string& control_device, bool wait);

    DaemonOperations Resolveop(std::string& input);

    DaemonOperations Resolveop(std::string& input);

    std::string GetDaemonStatus();

    std::string GetDaemonStatus();

    void Parsemsg(std::string const& msg, const char delim, std::vector<std::string>& out);

    void Parsemsg(std::string const& msg, const char delim, std::vector<std::string>& out);
@@ -103,11 +105,11 @@ class SnapuserdServer : public Stoppable { 
    void SetTerminating() { terminating_ = true; }

    void SetTerminating() { terminating_ = true; }

    bool IsTerminating() { return terminating_; }

    bool IsTerminating() { return terminating_; }





    void RunThread(DmUserHandler* handler);

    void RunThread(std::shared_ptr<DmUserHandler> handler);





    // Remove a DmUserHandler from dm_users_, searching by its control device.

    // Find a DmUserHandler within a lock.

    // If none is found, return nullptr.

    HandlerList::iterator FindHandler(std::lock_guard<std::mutex>* proof_of_lock,

    std::unique_ptr<DmUserHandler> RemoveHandler(const std::string& control_device);

                                      const std::string& misc_name);





  public:

  public:

    SnapuserdServer() { terminating_ = false; }

    SnapuserdServer() { terminating_ = false; }

fs_mgr/libsnapshot/snapshot.cpp

+39 −18
Original line number Original line Diff line number Diff line
@@ -397,7 +397,7 @@ bool SnapshotManager::MapDmUserCow(LockedFile* lock, const std::string& name, 
        return false;

        return false;

    }

    }





    uint64_t base_sectors = snapuserd_client_->InitDmUserCow(cow_file);

    uint64_t base_sectors = snapuserd_client_->InitDmUserCow(misc_name, cow_file, base_device);

    if (base_sectors == 0) {

    if (base_sectors == 0) {

        LOG(ERROR) << "Failed to retrieve base_sectors from Snapuserd";

        LOG(ERROR) << "Failed to retrieve base_sectors from Snapuserd";

        return false;

        return false;
@@ -410,7 +410,12 @@ bool SnapshotManager::MapDmUserCow(LockedFile* lock, const std::string& name, 
    }

    }





    auto control_device = "/dev/dm-user/" + misc_name;

    auto control_device = "/dev/dm-user/" + misc_name;

    return snapuserd_client_->InitializeSnapuserd(cow_file, base_device, control_device);

    if (!android::fs_mgr::WaitForFile(control_device, timeout_ms)) {

        LOG(ERROR) << "Timed out waiting for dm-user misc device: " << control_device;

        return false;

    }



    return snapuserd_client_->AttachDmUser(misc_name);

}

}





bool SnapshotManager::MapSnapshot(LockedFile* lock, const std::string& name,

bool SnapshotManager::MapSnapshot(LockedFile* lock, const std::string& name,
@@ -1310,28 +1315,34 @@ bool SnapshotManager::PerformSecondStageTransition() { 
    size_t num_cows = 0;

    size_t num_cows = 0;

    size_t ok_cows = 0;

    size_t ok_cows = 0;

    for (const auto& snapshot : snapshots) {

    for (const auto& snapshot : snapshots) {

        std::string cow_name = GetDmUserCowName(snapshot);

        std::string user_cow_name = GetDmUserCowName(snapshot);

        if (dm.GetState(cow_name) == DmDeviceState::INVALID) {

        if (dm.GetState(user_cow_name) == DmDeviceState::INVALID) {

            continue;

            continue;

        }

        }





        DeviceMapper::TargetInfo target;

        DeviceMapper::TargetInfo target;

        if (!GetSingleTarget(cow_name, TableQuery::Table, &target)) {

        if (!GetSingleTarget(user_cow_name, TableQuery::Table, &target)) {

            continue;

            continue;

        }

        }





        auto target_type = DeviceMapper::GetTargetType(target.spec);

        auto target_type = DeviceMapper::GetTargetType(target.spec);

        if (target_type != "user") {

        if (target_type != "user") {

            LOG(ERROR) << "Unexpected target type for " << cow_name << ": " << target_type;

            LOG(ERROR) << "Unexpected target type for " << user_cow_name << ": " << target_type;

            continue;

            continue;

        }

        }





        num_cows++;

        num_cows++;





        SnapshotStatus snapshot_status;

        if (!ReadSnapshotStatus(lock.get(), snapshot, &snapshot_status)) {

            LOG(ERROR) << "Unable to read snapshot status: " << snapshot;

            continue;

        }



        DmTable table;

        DmTable table;

        table.Emplace<DmTargetUser>(0, target.spec.length, cow_name);

        table.Emplace<DmTargetUser>(0, target.spec.length, user_cow_name);

        if (!dm.LoadTableAndActivate(cow_name, table)) {

        if (!dm.LoadTableAndActivate(user_cow_name, table)) {

            LOG(ERROR) << "Unable to swap tables for " << cow_name;

            LOG(ERROR) << "Unable to swap tables for " << user_cow_name;

            continue;

            continue;

        }

        }
@@ -1341,20 +1352,30 @@ bool SnapshotManager::PerformSecondStageTransition() { 
            continue;

            continue;

        }

        }





        std::string cow_device;

        // If no partition was created (the COW exists entirely on /data), the

        if (!dm.GetDmDevicePathByName(GetCowName(snapshot), &cow_device)) {

        // device-mapper layering is different than if we had a partition.

            LOG(ERROR) << "Could not get device path for " << GetCowName(snapshot);

        std::string cow_image_name;

        if (snapshot_status.cow_partition_size() == 0) {

            cow_image_name = GetCowImageDeviceName(snapshot);

        } else {

            cow_image_name = GetCowName(snapshot);

        }



        std::string cow_image_device;

        if (!dm.GetDmDevicePathByName(cow_image_name, &cow_image_device)) {

            LOG(ERROR) << "Could not get device path for " << cow_image_name;

            continue;

            continue;

        }

        }





        // Wait for ueventd to acknowledge and create the control device node.

        // Wait for ueventd to acknowledge and create the control device node.

        std::string control_device = "/dev/dm-user/" + cow_name;

        std::string control_device = "/dev/dm-user/" + user_cow_name;

        if (!android::fs_mgr::WaitForFile(control_device, 10s)) {

        if (!android::fs_mgr::WaitForFile(control_device, 10s)) {

            LOG(ERROR) << "Could not find control device: " << control_device;

            LOG(ERROR) << "Could not find control device: " << control_device;

            continue;

            continue;

        }

        }





        uint64_t base_sectors = snapuserd_client_->InitDmUserCow(cow_device);

        uint64_t base_sectors =

                snapuserd_client_->InitDmUserCow(user_cow_name, cow_image_device, backing_device);

        if (base_sectors == 0) {

        if (base_sectors == 0) {

            // Unrecoverable as metadata reads from cow device failed

            // Unrecoverable as metadata reads from cow device failed

            LOG(FATAL) << "Failed to retrieve base_sectors from Snapuserd";

            LOG(FATAL) << "Failed to retrieve base_sectors from Snapuserd";
@@ -1363,10 +1384,10 @@ bool SnapshotManager::PerformSecondStageTransition() { 




        CHECK(base_sectors == target.spec.length);

        CHECK(base_sectors == target.spec.length);





        if (!snapuserd_client_->InitializeSnapuserd(cow_device, backing_device, control_device)) {

        if (!snapuserd_client_->AttachDmUser(user_cow_name)) {

            // This error is unrecoverable. We cannot proceed because reads to

            // This error is unrecoverable. We cannot proceed because reads to

            // the underlying device will fail.

            // the underlying device will fail.

            LOG(FATAL) << "Could not initialize snapuserd for " << cow_name;

            LOG(FATAL) << "Could not initialize snapuserd for " << user_cow_name;

            return false;

            return false;

        }

        }
@@ -2053,13 +2074,13 @@ bool SnapshotManager::UnmapCowDevices(LockedFile* lock, const std::string& name) 
            return false;

            return false;

        }

        }





        auto control_device = "/dev/dm-user/" + dm_user_name;

        if (!snapuserd_client_->WaitForDeviceDelete(dm_user_name)) {

        if (!snapuserd_client_->WaitForDeviceDelete(control_device)) {

            LOG(ERROR) << "Failed to wait for " << dm_user_name << " control device to delete";

            LOG(ERROR) << "Failed to wait for " << dm_user_name << " control device to delete";

            return false;

            return false;

        }

        }





        // Ensure the control device is gone so we don't run into ABA problems.

        // Ensure the control device is gone so we don't run into ABA problems.

        auto control_device = "/dev/dm-user/" + dm_user_name;

        if (!android::fs_mgr::WaitForFileDeleted(control_device, 10s)) {

        if (!android::fs_mgr::WaitForFileDeleted(control_device, 10s)) {

            LOG(ERROR) << "Timed out waiting for " << control_device << " to unlink";

            LOG(ERROR) << "Timed out waiting for " << control_device << " to unlink";

            return false;

            return false;