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

Commit c9b797ac authored by David Anderson's avatar David Anderson Committed by Gerrit Code Review
Browse files

Merge "libdm: Improve the reliability of dm device paths."

parents e0b729d2 924858cd
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -73,6 +73,7 @@ cc_library {
    whole_static_libs: [
        "liblogwrap",
        "libdm",
        "libext2_uuid",
        "libfstab",
    ],
    cppflags: [
+1 −11
Original line number Diff line number Diff line
@@ -122,19 +122,9 @@ static bool CreateLogicalPartition(const LpMetadata& metadata, const LpMetadataP
        table.set_readonly(false);
    }
    std::string name = GetPartitionName(partition);
    if (!dm.CreateDevice(name, table)) {
    if (!dm.CreateDevice(name, table, path, timeout_ms)) {
        return false;
    }
    if (!dm.GetDmDevicePathByName(name, path)) {
        return false;
    }
    if (timeout_ms > std::chrono::milliseconds::zero()) {
        if (!WaitForFile(*path, timeout_ms)) {
            DestroyLogicalPartition(name, {});
            LERROR << "Timed out waiting for device path: " << *path;
            return false;
        }
    }
    LINFO << "Created logical partition " << name << " on device " << *path;
    return true;
}
+4 −0
Original line number Diff line number Diff line
@@ -29,6 +29,9 @@ cc_library_static {
        "loop_control.cpp",
    ],

    static_libs: [
        "libext2_uuid",
    ],
    header_libs: [
        "libbase_headers",
        "liblog_headers",
@@ -46,6 +49,7 @@ cc_test {
    static_libs: [
        "libdm",
        "libbase",
        "libext2_uuid",
        "libfs_mgr",
        "liblog",
    ],
+90 −20
Original line number Diff line number Diff line
@@ -20,12 +20,20 @@
#include <sys/sysmacros.h>
#include <sys/types.h>

#include <functional>
#include <thread>

#include <android-base/file.h>
#include <android-base/logging.h>
#include <android-base/macros.h>
#include <android-base/strings.h>
#include <uuid/uuid.h>

namespace android {
namespace dm {

using namespace std::literals;

DeviceMapper::DeviceMapper() : fd_(-1) {
    fd_ = TEMP_FAILURE_RETRY(open("/dev/device-mapper", O_RDWR | O_CLOEXEC));
    if (fd_ < 0) {
@@ -37,13 +45,13 @@ DeviceMapper& DeviceMapper::Instance() {
    static DeviceMapper instance;
    return instance;
}

// Creates a new device mapper device
bool DeviceMapper::CreateDevice(const std::string& name) {
bool DeviceMapper::CreateDevice(const std::string& name, const std::string& uuid) {
    if (name.empty()) {
        LOG(ERROR) << "Unnamed device mapper device creation is not supported";
        return false;
    }

    if (name.size() >= DM_NAME_LEN) {
        LOG(ERROR) << "[" << name << "] is too long to be device mapper name";
        return false;
@@ -51,6 +59,9 @@ bool DeviceMapper::CreateDevice(const std::string& name) {

    struct dm_ioctl io;
    InitIo(&io, name);
    if (!uuid.empty()) {
        snprintf(io.uuid, sizeof(io.uuid), "%s", uuid.c_str());
    }

    if (ioctl(fd_, DM_DEV_CREATE, &io)) {
        PLOG(ERROR) << "DM_DEV_CREATE failed for [" << name << "]";
@@ -67,16 +78,6 @@ bool DeviceMapper::CreateDevice(const std::string& name) {
}

bool DeviceMapper::DeleteDevice(const std::string& name) {
    if (name.empty()) {
        LOG(ERROR) << "Unnamed device mapper device creation is not supported";
        return false;
    }

    if (name.size() >= DM_NAME_LEN) {
        LOG(ERROR) << "[" << name << "] is too long to be device mapper name";
        return false;
    }

    struct dm_ioctl io;
    InitIo(&io, name);

@@ -93,9 +94,81 @@ bool DeviceMapper::DeleteDevice(const std::string& name) {
    return true;
}

const std::unique_ptr<DmTable> DeviceMapper::table(const std::string& /* name */) const {
    // TODO(b/110035986): Return the table, as read from the kernel instead
    return nullptr;
bool WaitForCondition(const std::function<bool()>& condition,
                      const std::chrono::milliseconds& timeout_ms) {
    auto start_time = std::chrono::steady_clock::now();
    while (true) {
        if (condition()) return true;

        std::this_thread::sleep_for(20ms);

        auto now = std::chrono::steady_clock::now();
        auto time_elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(now - start_time);
        if (time_elapsed > timeout_ms) return false;
    }
}

static std::string GenerateUuid() {
    uuid_t uuid_bytes;
    uuid_generate(uuid_bytes);

    char uuid_chars[37] = {};
    uuid_unparse_lower(uuid_bytes, uuid_chars);

    return std::string{uuid_chars};
}

bool DeviceMapper::CreateDevice(const std::string& name, const DmTable& table, std::string* path,
                                const std::chrono::milliseconds& timeout_ms) {
    std::string uuid = GenerateUuid();
    if (!CreateDevice(name, uuid)) {
        return false;
    }

    // We use the unique path for testing whether the device is ready. After
    // that, it's safe to use the dm-N path which is compatible with callers
    // that expect it to be formatted as such.
    std::string unique_path;
    if (!LoadTableAndActivate(name, table) || !GetDeviceUniquePath(name, &unique_path) ||
        !GetDmDevicePathByName(name, path)) {
        DeleteDevice(name);
        return false;
    }

    if (timeout_ms <= std::chrono::milliseconds::zero()) {
        return true;
    }

    auto condition = [&]() -> bool {
        // If the file exists but returns EPERM or something, we consider the
        // condition met.
        if (access(unique_path.c_str(), F_OK) != 0) {
            if (errno == ENOENT) return false;
        }
        return true;
    };
    if (!WaitForCondition(condition, timeout_ms)) {
        LOG(ERROR) << "Timed out waiting for device path: " << unique_path;
        DeleteDevice(name);
        return false;
    }
    return true;
}

bool DeviceMapper::GetDeviceUniquePath(const std::string& name, std::string* path) {
    struct dm_ioctl io;
    InitIo(&io, name);
    if (ioctl(fd_, DM_DEV_STATUS, &io) < 0) {
        PLOG(ERROR) << "Failed to get device path: " << name;
        return false;
    }

    if (io.uuid[0] == '\0') {
        LOG(ERROR) << "Device does not have a unique path: " << name;
        return false;
    }
    *path = "/dev/block/mapper/by-uuid/"s + io.uuid;
    return true;
}

DmDeviceState DeviceMapper::GetState(const std::string& name) const {
@@ -111,11 +184,8 @@ DmDeviceState DeviceMapper::GetState(const std::string& name) const {
}

bool DeviceMapper::CreateDevice(const std::string& name, const DmTable& table) {
    if (!CreateDevice(name)) {
        return false;
    }
    if (!LoadTableAndActivate(name, table)) {
        DeleteDevice(name);
    std::string ignore_path;
    if (!CreateDevice(name, table, &ignore_path, 0ms)) {
        return false;
    }
    return true;
+4 −28
Original line number Diff line number Diff line
@@ -52,10 +52,10 @@ class TempDevice {
  public:
    TempDevice(const std::string& name, const DmTable& table)
        : dm_(DeviceMapper::Instance()), name_(name), valid_(false) {
        valid_ = dm_.CreateDevice(name, table);
        valid_ = dm_.CreateDevice(name, table, &path_, 5s);
    }
    TempDevice(TempDevice&& other) noexcept
        : dm_(other.dm_), name_(other.name_), valid_(other.valid_) {
        : dm_(other.dm_), name_(other.name_), path_(other.path_), valid_(other.valid_) {
        other.valid_ = false;
    }
    ~TempDevice() {
@@ -70,29 +70,7 @@ class TempDevice {
        valid_ = false;
        return dm_.DeleteDevice(name_);
    }
    bool WaitForUdev() const {
        auto start_time = std::chrono::steady_clock::now();
        while (true) {
            if (!access(path().c_str(), F_OK)) {
                return true;
            }
            if (errno != ENOENT) {
                return false;
            }
            std::this_thread::sleep_for(50ms);
            std::chrono::duration elapsed = std::chrono::steady_clock::now() - start_time;
            if (elapsed >= 5s) {
                return false;
            }
        }
    }
    std::string path() const {
        std::string device_path;
        if (!dm_.GetDmDevicePathByName(name_, &device_path)) {
            return "";
        }
        return device_path;
    }
    std::string path() const { return path_; }
    const std::string& name() const { return name_; }
    bool valid() const { return valid_; }

@@ -109,6 +87,7 @@ class TempDevice {
  private:
    DeviceMapper& dm_;
    std::string name_;
    std::string path_;
    bool valid_;
};

@@ -139,7 +118,6 @@ TEST(libdm, DmLinear) {
    TempDevice dev("libdm-test-dm-linear", table);
    ASSERT_TRUE(dev.valid());
    ASSERT_FALSE(dev.path().empty());
    ASSERT_TRUE(dev.WaitForUdev());

    auto& dm = DeviceMapper::Instance();

@@ -290,7 +268,6 @@ void SnapshotTestHarness::SetupImpl() {
    origin_dev_ = std::make_unique<TempDevice>("libdm-test-dm-snapshot-origin", origin_table);
    ASSERT_TRUE(origin_dev_->valid());
    ASSERT_FALSE(origin_dev_->path().empty());
    ASSERT_TRUE(origin_dev_->WaitForUdev());

    // chunk size = 4K blocks.
    DmTable snap_table;
@@ -302,7 +279,6 @@ void SnapshotTestHarness::SetupImpl() {
    snapshot_dev_ = std::make_unique<TempDevice>("libdm-test-dm-snapshot", snap_table);
    ASSERT_TRUE(snapshot_dev_->valid());
    ASSERT_FALSE(snapshot_dev_->path().empty());
    ASSERT_TRUE(snapshot_dev_->WaitForUdev());

    setup_ok_ = true;
}
Loading