ACPR/android_system_core
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

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

Browse source 
am: c9b797ac25

Change-Id: I30a3bf89f71269b0072dad4f73e6b74273ea031a
This commit is contained in:
David Anderson 2019-07-12 16:24:39 -07:00 • committed by android-build-merger
commit 92c53170f3
10 changed files with 197 additions and 101 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
fs_mgr/Android.bp
View file

@ -73,6 +73,7 @@ cc_library {
whole_static_libs: [
"liblogwrap",
"libdm",
"libext2_uuid",
"libfstab",
],
cppflags: [

12
fs_mgr/fs_mgr_dm_linear.cpp
View file

@ -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
fs_mgr/libdm/Android.bp
View file

@ -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",
],

110
fs_mgr/libdm/dm.cpp
View file

@ -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;

32
fs_mgr/libdm/dm_test.cpp
View file

@ -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;
}

53
fs_mgr/libdm/include/libdm/dm.h
View file

@ -25,6 +25,7 @@
#include <sys/sysmacros.h>
#include <unistd.h>
#include <chrono>
#include <memory>
#include <string>
#include <utility>
@ -73,10 +74,6 @@ class DeviceMapper final {
// Returns 'true' on success, false otherwise.
bool DeleteDevice(const std::string& name);
// Reads the device mapper table from the device with given anme and
// returns it in a DmTable object.
const std::unique_ptr<DmTable> table(const std::string& name) const;
// Returns the current state of the underlying device mapper device
// with given name.
// One of INVALID, SUSPENDED or ACTIVE.
@ -84,6 +81,33 @@ class DeviceMapper final {
// Creates a device, loads the given table, and activates it. If the device
// is not able to be activated, it is destroyed, and false is returned.
// After creation, |path| contains the result of calling
// GetDmDevicePathByName, and the path is guaranteed to exist. If after
// |timeout_ms| the path is not available, the device will be deleted and
// this function will return false.
//
// This variant must be used when depending on the device path. The
// following manual sequence should not be used:
//
// 1. CreateDevice(name, table)
// 2. GetDmDevicePathByName(name, &path)
// 3. fs_mgr::WaitForFile(path, <timeout>)
//
// This sequence has a race condition where, if another process deletes a
// device, CreateDevice may acquire the same path. When this happens, the
// WaitForFile() may early-return since ueventd has not yet processed all
// of the outstanding udev events. The caller may unexpectedly get an
// ENOENT on a system call using the affected path.
//
// If |timeout_ms| is 0ms, then this function will return true whether or
// not |path| is available. It is the caller's responsibility to ensure
// there are no races.
bool CreateDevice(const std::string& name, const DmTable& table, std::string* path,
const std::chrono::milliseconds& timeout_ms);
// Create a device and activate the given table, without waiting to acquire
// a valid path. If the caller will use GetDmDevicePathByName(), it should
// use the timeout variant above.
bool CreateDevice(const std::string& name, const DmTable& table);
// Loads the device mapper table from parameter into the underlying device
@ -110,8 +134,21 @@ class DeviceMapper final {
// Returns the path to the device mapper device node in '/dev' corresponding to
// 'name'. If the device does not exist, false is returned, and the path
// parameter is not set.
//
// This returns a path in the format "/dev/block/dm-N" that can be easily
// re-used with sysfs.
//
// WaitForFile() should not be used in conjunction with this call, since it
// could race with ueventd.
bool GetDmDevicePathByName(const std::string& name, std::string* path);
// Returns a device's unique path as generated by ueventd. This will return
// true as long as the device has been created, even if ueventd has not
// processed it yet.
//
// The formatting of this path is /dev/block/mapper/by-uuid/<uuid>.
bool GetDeviceUniquePath(const std::string& name, std::string* path);
// Returns the dev_t for the named device-mapper node.
bool GetDeviceNumber(const std::string& name, dev_t* dev);
@ -158,18 +195,12 @@ class DeviceMapper final {
// limit we are imposing here of 256.
static constexpr uint32_t kMaxPossibleDmDevices = 256;
bool CreateDevice(const std::string& name, const std::string& uuid = {});
bool GetTable(const std::string& name, uint32_t flags, std::vector<TargetInfo>* table);
void InitIo(struct dm_ioctl* io, const std::string& name = std::string()) const;
DeviceMapper();
// Creates a device mapper device with given name.
// Return 'true' on success and 'false' on failure to
// create OR if a device mapper device with the same name already
// exists.
bool CreateDevice(const std::string& name);
int fd_;
// Non-copyable & Non-movable
DeviceMapper(const DeviceMapper&) = delete;

1
fs_mgr/libfs_avb/Android.bp
View file

@ -61,6 +61,7 @@ cc_defaults {
"libavb",
"libavb_host_sysdeps",
"libdm",
"libext2_uuid",
"libfs_avb",
"libfstab",
"libgtest_host",

18
fs_mgr/libfs_avb/avb_util.cpp
View file

@ -104,31 +104,23 @@ bool HashtreeDmVeritySetup(FstabEntry* fstab_entry, const FsAvbHashtreeDescripto
}
table.set_readonly(true);
std::chrono::milliseconds timeout = {};
if (wait_for_verity_dev) timeout = 1s;
std::string dev_path;
const std::string mount_point(Basename(fstab_entry->mount_point));
const std::string device_name(GetVerityDeviceName(*fstab_entry));
android::dm::DeviceMapper& dm = android::dm::DeviceMapper::Instance();
if (!dm.CreateDevice(device_name, table)) {
if (!dm.CreateDevice(device_name, table, &dev_path, timeout)) {
LERROR << "Couldn't create verity device!";
return false;
}
std::string dev_path;
if (!dm.GetDmDevicePathByName(device_name, &dev_path)) {
LERROR << "Couldn't get verity device path!";
return false;
}
// Marks the underlying block device as read-only.
SetBlockDeviceReadOnly(fstab_entry->blk_device);
// Updates fstab_rec->blk_device to verity device name.
fstab_entry->blk_device = dev_path;
// Makes sure we've set everything up properly.
if (wait_for_verity_dev && !WaitForFile(dev_path, 1s)) {
return false;
}
return true;
}

1
init/Android.mk
View file

@ -113,6 +113,7 @@ LOCAL_STATIC_LIBRARIES := \
libunwindstack \
libbacktrace \
libmodprobe \
libext2_uuid \
LOCAL_SANITIZE := signed-integer-overflow
# First stage init is weird: it may start without stdout/stderr, and no /proc.

66
init/devices.cpp
View file

@ -22,7 +22,6 @@
#include <unistd.h>
#include <chrono>
#include <map>
#include <memory>
#include <string>
#include <thread>
@ -111,24 +110,16 @@ static bool FindVbdDevicePrefix(const std::string& path, std::string* result) {
// the supplied buffer with the dm module's instantiated name.
// If it doesn't start with a virtual block device, or there is some
// error, return false.
static bool FindDmDevicePartition(const std::string& path, std::string* result) {
result->clear();
static bool FindDmDevice(const std::string& path, std::string* name, std::string* uuid) {
if (!StartsWith(path, "/devices/virtual/block/dm-")) return false;
if (getpid() == 1) return false; // first_stage_init has no sepolicy needs
static std::map<std::string, std::string> cache;
// wait_for_file will not work, the content is also delayed ...
for (android::base::Timer t; t.duration() < 200ms; std::this_thread::sleep_for(10ms)) {
if (ReadFileToString("/sys" + path + "/dm/name", result) && !result->empty()) {
// Got it, set cache with result, when node arrives
cache[path] = *result = Trim(*result);
return true;
}
if (!ReadFileToString("/sys" + path + "/dm/name", name)) {
return false;
}
auto it = cache.find(path);
if ((it == cache.end()) || (it->second.empty())) return false;
// Return cached results, when node goes away
*result = it->second;
ReadFileToString("/sys" + path + "/dm/uuid", uuid);
*name = android::base::Trim(*name);
*uuid = android::base::Trim(*uuid);
return true;
}
@ -325,6 +316,7 @@ std::vector<std::string> DeviceHandler::GetBlockDeviceSymlinks(const Uevent& uev
std::string device;
std::string type;
std::string partition;
std::string uuid;
if (FindPlatformDevice(uevent.path, &device)) {
// Skip /devices/platform or /devices/ if present
@ -342,8 +334,12 @@ std::vector<std::string> DeviceHandler::GetBlockDeviceSymlinks(const Uevent& uev
type = "pci";
} else if (FindVbdDevicePrefix(uevent.path, &device)) {
type = "vbd";
} else if (FindDmDevicePartition(uevent.path, &partition)) {
return {"/dev/block/mapper/" + partition};
} else if (FindDmDevice(uevent.path, &partition, &uuid)) {
std::vector<std::string> symlinks = {"/dev/block/mapper/" + partition};
if (!uuid.empty()) {
symlinks.emplace_back("/dev/block/mapper/by-uuid/" + uuid);
}
return symlinks;
} else {
return {};
}
@ -379,10 +375,41 @@ std::vector<std::string> DeviceHandler::GetBlockDeviceSymlinks(const Uevent& uev
return links;
}
static void RemoveDeviceMapperLinks(const std::string& devpath) {
std::vector<std::string> dirs = {
"/dev/block/mapper",
"/dev/block/mapper/by-uuid",
};
for (const auto& dir : dirs) {
if (access(dir.c_str(), F_OK) != 0) continue;
std::unique_ptr<DIR, decltype(&closedir)> dh(opendir(dir.c_str()), closedir);
if (!dh) {
PLOG(ERROR) << "Failed to open directory " << dir;
continue;
}
struct dirent* dp;
std::string link_path;
while ((dp = readdir(dh.get())) != nullptr) {
if (dp->d_type != DT_LNK) continue;
auto path = dir + "/" + dp->d_name;
if (Readlink(path, &link_path) && link_path == devpath) {
unlink(path.c_str());
}
}
}
}
void DeviceHandler::HandleDevice(const std::string& action, const std::string& devpath, bool block,
int major, int minor, const std::vector<std::string>& links) const {
if (action == "add") {
MakeDevice(devpath, block, major, minor, links);
}
// We don't have full device-mapper information until a change event is fired.
if (action == "add" || (action == "change" && StartsWith(devpath, "/dev/block/dm-"))) {
for (const auto& link : links) {
if (!mkdir_recursive(Dirname(link), 0755)) {
PLOG(ERROR) << "Failed to create directory " << Dirname(link);
@ -401,6 +428,9 @@ void DeviceHandler::HandleDevice(const std::string& action, const std::string& d
}
if (action == "remove") {
if (StartsWith(devpath, "/dev/block/dm-")) {
RemoveDeviceMapperLinks(devpath);
}
for (const auto& link : links) {
std::string link_path;
if (Readlink(link, &link_path) && link_path == devpath) {