From 8f50cfc28d2d91f91f45370bd9859a328ffc6c16 Mon Sep 17 00:00:00 2001 From: Nikita Ioffe Date: Fri, 6 Mar 2020 15:55:20 +0000 Subject: [PATCH] Move GetMountedEntryForUserdata to fs_mgr.h Logic of unwinding dm-device stack to figure out what entry was used to mount userdata turned out to be a little bit more involved, and it shouldn't be part of libfstab This CL just moves code around and cleans API a little bit, actual fix will be in the follow-up CL. Test: atest CtsFsMgrTest Test: atest CtsUserspaceRebootHostSideTestCases Bug: 148612148 Change-Id: If0f8a765dba63adb0e6a711eb81fffdcabea3480 --- fs_mgr/fs_mgr.cpp | 85 +++++++++++++++++++++++++++--- fs_mgr/fs_mgr_fstab.cpp | 83 ----------------------------- fs_mgr/include/fs_mgr.h | 4 ++ fs_mgr/include_fstab/fstab/fstab.h | 1 - fs_mgr/tests/fs_mgr_test.cpp | 7 ++- 5 files changed, 88 insertions(+), 92 deletions(-) diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index 59cae61d7..2e46b4f87 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -96,6 +96,7 @@ using android::base::Basename; using android::base::GetBoolProperty; +using android::base::Readlink; using android::base::Realpath; using android::base::SetProperty; using android::base::StartsWith; @@ -1588,6 +1589,79 @@ static bool fs_mgr_unmount_all_data_mounts(const std::string& block_device) { } } +static std::string ResolveBlockDevice(const std::string& block_device) { + if (!StartsWith(block_device, "/dev/block/")) { + LWARNING << block_device << " is not a block device"; + return block_device; + } + std::string name = block_device.substr(5); + if (!StartsWith(name, "block/dm-")) { + // Not a dm-device, but might be a symlink. Optimistically try to readlink. + std::string result; + if (Readlink(block_device, &result)) { + return result; + } else if (errno == EINVAL) { + // After all, it wasn't a symlink. + return block_device; + } else { + LERROR << "Failed to readlink " << block_device; + return ""; + } + } + // It's a dm-device, let's find what's inside! + std::string sys_dir = "/sys/" + name; + while (true) { + std::string slaves_dir = sys_dir + "/slaves"; + std::unique_ptr dir(opendir(slaves_dir.c_str()), closedir); + if (!dir) { + LERROR << "Failed to open " << slaves_dir; + return ""; + } + std::string sub_device_name = ""; + for (auto entry = readdir(dir.get()); entry; entry = readdir(dir.get())) { + if (entry->d_type != DT_LNK) continue; + if (!sub_device_name.empty()) { + LERROR << "Too many slaves in " << slaves_dir; + return ""; + } + sub_device_name = entry->d_name; + } + if (sub_device_name.empty()) { + LERROR << "No slaves in " << slaves_dir; + return ""; + } + if (!StartsWith(sub_device_name, "dm-")) { + // Not a dm-device! We can stop now. + return "/dev/block/" + sub_device_name; + } + // Still a dm-device, keep digging. + sys_dir = "/sys/block/" + sub_device_name; + } +} + +FstabEntry* fs_mgr_get_mounted_entry_for_userdata(Fstab* fstab, const FstabEntry& mounted_entry) { + std::string resolved_block_device = ResolveBlockDevice(mounted_entry.blk_device); + if (resolved_block_device.empty()) { + return nullptr; + } + LINFO << "/data is mounted on " << resolved_block_device; + for (auto& entry : *fstab) { + if (entry.mount_point != "/data") { + continue; + } + std::string block_device; + if (!Readlink(entry.blk_device, &block_device)) { + LWARNING << "Failed to readlink " << entry.blk_device; + block_device = entry.blk_device; + } + if (block_device == resolved_block_device) { + return &entry; + } + } + LERROR << "Didn't find entry that was used to mount /data"; + return nullptr; +} + // TODO(b/143970043): return different error codes based on which step failed. int fs_mgr_remount_userdata_into_checkpointing(Fstab* fstab) { Fstab proc_mounts; @@ -1596,16 +1670,13 @@ int fs_mgr_remount_userdata_into_checkpointing(Fstab* fstab) { return -1; } std::string block_device; - if (auto entry = GetEntryForMountPoint(&proc_mounts, "/data"); entry != nullptr) { - // Note: we don't care about a userdata wrapper here, since it's safe - // to remount on top of the bow device instead, there will be no - // conflicts. - block_device = entry->blk_device; - } else { + auto mounted_entry = GetEntryForMountPoint(&proc_mounts, "/data"); + if (mounted_entry == nullptr) { LERROR << "/data is not mounted"; return -1; } - auto fstab_entry = GetMountedEntryForUserdata(fstab); + block_device = mounted_entry->blk_device; + auto fstab_entry = fs_mgr_get_mounted_entry_for_userdata(fstab, *mounted_entry); if (fstab_entry == nullptr) { LERROR << "Can't find /data in fstab"; return -1; diff --git a/fs_mgr/fs_mgr_fstab.cpp b/fs_mgr/fs_mgr_fstab.cpp index a36934a3a..f3f1cb724 100644 --- a/fs_mgr/fs_mgr_fstab.cpp +++ b/fs_mgr/fs_mgr_fstab.cpp @@ -814,89 +814,6 @@ std::vector GetEntriesForMountPoint(Fstab* fstab, const std::string return entries; } -static std::string ResolveBlockDevice(const std::string& block_device) { - if (!StartsWith(block_device, "/dev/block/")) { - LWARNING << block_device << " is not a block device"; - return block_device; - } - std::string name = block_device.substr(5); - if (!StartsWith(name, "block/dm-")) { - // Not a dm-device, but might be a symlink. Optimistically try to readlink. - std::string result; - if (Readlink(block_device, &result)) { - return result; - } else if (errno == EINVAL) { - // After all, it wasn't a symlink. - return block_device; - } else { - LERROR << "Failed to readlink " << block_device; - return ""; - } - } - // It's a dm-device, let's find what's inside! - std::string sys_dir = "/sys/" + name; - while (true) { - std::string slaves_dir = sys_dir + "/slaves"; - std::unique_ptr dir(opendir(slaves_dir.c_str()), closedir); - if (!dir) { - LERROR << "Failed to open " << slaves_dir; - return ""; - } - std::string sub_device_name = ""; - for (auto entry = readdir(dir.get()); entry; entry = readdir(dir.get())) { - if (entry->d_type != DT_LNK) continue; - if (!sub_device_name.empty()) { - LERROR << "Too many slaves in " << slaves_dir; - return ""; - } - sub_device_name = entry->d_name; - } - if (sub_device_name.empty()) { - LERROR << "No slaves in " << slaves_dir; - return ""; - } - if (!StartsWith(sub_device_name, "dm-")) { - // Not a dm-device! We can stop now. - return "/dev/block/" + sub_device_name; - } - // Still a dm-device, keep digging. - sys_dir = "/sys/block/" + sub_device_name; - } -} - -FstabEntry* GetMountedEntryForUserdata(Fstab* fstab) { - Fstab mounts; - if (!ReadFstabFromFile("/proc/mounts", &mounts)) { - LERROR << "Failed to read /proc/mounts"; - return nullptr; - } - auto mounted_entry = GetEntryForMountPoint(&mounts, "/data"); - if (mounted_entry == nullptr) { - LWARNING << "/data is not mounted"; - return nullptr; - } - std::string resolved_block_device = ResolveBlockDevice(mounted_entry->blk_device); - if (resolved_block_device.empty()) { - return nullptr; - } - LINFO << "/data is mounted on " << resolved_block_device; - for (auto& entry : *fstab) { - if (entry.mount_point != "/data") { - continue; - } - std::string block_device; - if (!Readlink(entry.blk_device, &block_device)) { - LWARNING << "Failed to readlink " << entry.blk_device; - block_device = entry.blk_device; - } - if (block_device == resolved_block_device) { - return &entry; - } - } - LERROR << "Didn't find entry that was used to mount /data"; - return nullptr; -} - std::set GetBootDevices() { // First check the kernel commandline, then try the device tree otherwise std::string dt_file_name = get_android_dt_dir() + "/boot_devices"; diff --git a/fs_mgr/include/fs_mgr.h b/fs_mgr/include/fs_mgr.h index 9bc38f97b..3d556c9e5 100644 --- a/fs_mgr/include/fs_mgr.h +++ b/fs_mgr/include/fs_mgr.h @@ -107,6 +107,10 @@ enum FsMgrUmountStatus : int { // it destroys verity devices from device mapper after the device is unmounted. int fs_mgr_umount_all(android::fs_mgr::Fstab* fstab); +// Finds a entry in |fstab| that was used to mount a /data |mounted_entry| from +// /proc/mounts. +android::fs_mgr::FstabEntry* fs_mgr_get_mounted_entry_for_userdata( + android::fs_mgr::Fstab* fstab, const android::fs_mgr::FstabEntry& mounted_entry); int fs_mgr_remount_userdata_into_checkpointing(android::fs_mgr::Fstab* fstab); // Finds the dm_bow device on which this block device is stacked, or returns diff --git a/fs_mgr/include_fstab/fstab/fstab.h b/fs_mgr/include_fstab/fstab/fstab.h index c94d7acd1..009c04c19 100644 --- a/fs_mgr/include_fstab/fstab/fstab.h +++ b/fs_mgr/include_fstab/fstab/fstab.h @@ -102,7 +102,6 @@ bool SkipMountingPartitions(Fstab* fstab); FstabEntry* GetEntryForMountPoint(Fstab* fstab, const std::string& path); // The Fstab can contain multiple entries for the same mount point with different configurations. std::vector GetEntriesForMountPoint(Fstab* fstab, const std::string& path); -FstabEntry* GetMountedEntryForUserdata(Fstab* fstab); // This method builds DSU fstab entries and transfer the fstab. // diff --git a/fs_mgr/tests/fs_mgr_test.cpp b/fs_mgr/tests/fs_mgr_test.cpp index 9caae3597..16e38f157 100644 --- a/fs_mgr/tests/fs_mgr_test.cpp +++ b/fs_mgr/tests/fs_mgr_test.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -1001,6 +1002,10 @@ TEST(fs_mgr, UserdataMountedFromDefaultFstab) { } Fstab fstab; ASSERT_TRUE(ReadDefaultFstab(&fstab)) << "Failed to read default fstab"; - ASSERT_NE(nullptr, GetMountedEntryForUserdata(&fstab)) + Fstab proc_mounts; + ASSERT_TRUE(ReadFstabFromFile("/proc/mounts", &proc_mounts)) << "Failed to read /proc/mounts"; + auto mounted_entry = GetEntryForMountPoint(&proc_mounts, "/data"); + ASSERT_NE(mounted_entry, nullptr) << "/data is not mounted"; + ASSERT_NE(nullptr, fs_mgr_get_mounted_entry_for_userdata(&fstab, *mounted_entry)) << "/data wasn't mounted from default fstab"; }