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

Commit 2c28919d authored by Nikita Ioffe's avatar Nikita Ioffe
Browse files

libdm: add an overload of DeleteDevice accepting a timeout_ms 

In some scenarios (e.g. apexd or userspace reboot), dm-devices are
getting deleted and re-created. Since this operation can be racy (newly
created device can get the same path as the previously deleted one,
resulting in the unexpected ENOENT errors on a system call to the path),
it will be nice to have an API that blocks until ueventd processes
corresponding udev events.

Test: libdm_test
Bug: 143970043
Bug: 122059364
Change-Id: I31a19afd9e245bf5e3554011bdde1c3cc4878f1c
parent 028e1d44

fs_mgr/libdm/dm.cpp

+28 −3
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ 
#include <sys/sysmacros.h>

#include <sys/types.h>



#include <chrono>

#include <functional>

#include <thread>
@@ -79,14 +80,24 @@ bool DeviceMapper::CreateDevice(const std::string& name, const std::string& uuid 
    return true;

}



bool DeviceMapper::DeleteDeviceIfExists(const std::string& name) {

bool DeviceMapper::DeleteDeviceIfExists(const std::string& name,

                                        const std::chrono::milliseconds& timeout_ms) {

    if (GetState(name) == DmDeviceState::INVALID) {

        return true;

    }

    return DeleteDevice(name);

    return DeleteDevice(name, timeout_ms);

}



bool DeviceMapper::DeleteDevice(const std::string& name) {

bool DeviceMapper::DeleteDeviceIfExists(const std::string& name) {

    return DeleteDeviceIfExists(name, 0ms);

}



bool DeviceMapper::DeleteDevice(const std::string& name,

                                const std::chrono::milliseconds& timeout_ms) {

    std::string unique_path;

    if (!GetDeviceUniquePath(name, &unique_path)) {

        LOG(ERROR) << "Failed to get unique path for device " << name;

    }

    struct dm_ioctl io;

    InitIo(&io, name);
@@ -100,8 +111,22 @@ bool DeviceMapper::DeleteDevice(const std::string& name) { 
    CHECK(io.flags & DM_UEVENT_GENERATED_FLAG)

            << "Didn't generate uevent for [" << name << "] removal";



    if (timeout_ms <= std::chrono::milliseconds::zero()) {

        return true;

    }

    if (unique_path.empty()) {

        return false;

    }

    if (!WaitForFileDeleted(unique_path, timeout_ms)) {

        LOG(ERROR) << "Timeout out waiting for " << unique_path << " to be deleted";

        return false;

    }

    return true;

}



bool DeviceMapper::DeleteDevice(const std::string& name) {

    return DeleteDevice(name, 0ms);

}



static std::string GenerateUuid() {

    uuid_t uuid_bytes;

fs_mgr/libdm/dm_test.cpp

+24 −0
Original line number Diff line number Diff line
@@ -520,3 +520,27 @@ TEST(libdm, DefaultKeyArgs) { 
    ASSERT_TRUE(target.Valid());

    ASSERT_EQ(target.GetParameterString(), "AES-256-XTS abcdef0123456789 /dev/loop0 0");

}



TEST(libdm, DeleteDeviceWithTimeout) {

    unique_fd tmp(CreateTempFile("file_1", 4096));

    ASSERT_GE(tmp, 0);

    LoopDevice loop(tmp, 10s);

    ASSERT_TRUE(loop.valid());



    DmTable table;

    ASSERT_TRUE(table.Emplace<DmTargetLinear>(0, 1, loop.device(), 0));

    ASSERT_TRUE(table.valid());

    TempDevice dev("libdm-test-dm-linear", table);

    ASSERT_TRUE(dev.valid());



    DeviceMapper& dm = DeviceMapper::Instance();



    std::string path;

    ASSERT_TRUE(dm.GetDmDevicePathByName("libdm-test-dm-linear", &path));

    ASSERT_EQ(0, access(path.c_str(), F_OK));



    ASSERT_TRUE(dm.DeleteDevice("libdm-test-dm-linear", 5s));

    ASSERT_EQ(DmDeviceState::INVALID, dm.GetState("libdm-test-dm-linear"));

    ASSERT_NE(0, access(path.c_str(), F_OK));

    ASSERT_EQ(ENOENT, errno);

}

fs_mgr/libdm/include/libdm/dm.h

+4 −0
Original line number Diff line number Diff line
@@ -90,6 +90,10 @@ class DeviceMapper final { 
    // Returns 'true' on success, false otherwise.

    bool DeleteDevice(const std::string& name);

    bool DeleteDeviceIfExists(const std::string& name);

    // Removes a device mapper device with the given name and waits for |timeout_ms| milliseconds

    // for the corresponding block device to be deleted.

    bool DeleteDevice(const std::string& name, const std::chrono::milliseconds& timeout_ms);

    bool DeleteDeviceIfExists(const std::string& name, const std::chrono::milliseconds& timeout_ms);



    // Fetches and returns the complete state of the underlying device mapper

    // device with given name.

fs_mgr/libdm/utility.cpp

+10 −0
Original line number Diff line number Diff line
@@ -52,5 +52,15 @@ bool WaitForFile(const std::string& path, const std::chrono::milliseconds& timeo 
    return WaitForCondition(condition, timeout_ms);

}



bool WaitForFileDeleted(const std::string& path, const std::chrono::milliseconds& timeout_ms) {

    auto condition = [&]() -> WaitResult {

        if (access(path.c_str(), F_OK) == 0 || errno != ENOENT) {

            return WaitResult::Wait;

        }

        return WaitResult::Done;

    };

    return WaitForCondition(condition, timeout_ms);

}



}  // namespace dm

}  // namespace android

fs_mgr/libdm/utility.h

+1 −0
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ namespace dm { 
enum class WaitResult { Wait, Done, Fail };



bool WaitForFile(const std::string& path, const std::chrono::milliseconds& timeout_ms);

bool WaitForFileDeleted(const std::string& path, const std::chrono::milliseconds& timeout_ms);

bool WaitForCondition(const std::function<WaitResult()>& condition,

                      const std::chrono::milliseconds& timeout_ms);