diff options
author | David Anderson <dvander@google.com> | 2022-12-08 09:53:03 -0800 |
---|---|---|
committer | lakshmi k <quic_ksoundar@quicinc.com> | 2023-01-19 15:47:31 +0530 |
commit | 2c2146daddb85ae90789d2be311e8eee52822d98 (patch) | |
tree | 7715ea6f795d85b53ea180ac32ed324d5f445262 | |
parent | 275205155f0388056cd1eaa9e768e022cbb07042 (diff) |
Support sysfs changes in the Linux 5.15 kernel.
DM_DEV_CREATE no longer creates sysfs nodes. Note this in ueventd and
add some helper APIs to libdm, so devices can be created with a
placeholder table.
This also fixes a bug in dmctl where the detailed info on suspended
devices was wrong.
Bug: 259328366
CRs-Fixed: 3312045
Test: dmctl with "uevents" tool
Change-Id: I822f8010e48d32841aa0ee508822f76d03a3dd85
(cherry picked from commit d6bf86b8cf12ac50c9bc8ddaa6a33d6ee67ab91f)
-rw-r--r-- | fs_mgr/libdm/dm.cpp | 14 | ||||
-rw-r--r-- | fs_mgr/libdm/dm_test.cpp | 20 | ||||
-rw-r--r-- | fs_mgr/libdm/include/libdm/dm.h | 9 | ||||
-rw-r--r-- | fs_mgr/libdm/include/libdm/dm_table.h | 3 | ||||
-rw-r--r-- | fs_mgr/libdm/include/libdm/dm_target.h | 8 | ||||
-rw-r--r-- | fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h | 3 | ||||
-rw-r--r-- | fs_mgr/tools/dmctl.cpp | 62 | ||||
-rw-r--r-- | init/devices.cpp | 6 |
8 files changed, 100 insertions, 25 deletions
diff --git a/fs_mgr/libdm/dm.cpp b/fs_mgr/libdm/dm.cpp index 4b906a22e..deffae1ad 100644 --- a/fs_mgr/libdm/dm.cpp +++ b/fs_mgr/libdm/dm.cpp @@ -290,7 +290,7 @@ bool DeviceMapper::CreateDevice(const std::string& name, const DmTable& table) { return true; } -bool DeviceMapper::LoadTableAndActivate(const std::string& name, const DmTable& table) { +bool DeviceMapper::LoadTable(const std::string& name, const DmTable& table) { std::string ioctl_buffer(sizeof(struct dm_ioctl), 0); ioctl_buffer += table.Serialize(); @@ -306,9 +306,17 @@ bool DeviceMapper::LoadTableAndActivate(const std::string& name, const DmTable& PLOG(ERROR) << "DM_TABLE_LOAD failed"; return false; } + return true; +} - InitIo(io, name); - if (ioctl(fd_, DM_DEV_SUSPEND, io)) { +bool DeviceMapper::LoadTableAndActivate(const std::string& name, const DmTable& table) { + if (!LoadTable(name, table)) { + return false; + } + + struct dm_ioctl io; + InitIo(&io, name); + if (ioctl(fd_, DM_DEV_SUSPEND, &io)) { PLOG(ERROR) << "DM_TABLE_SUSPEND resume failed"; return false; } diff --git a/fs_mgr/libdm/dm_test.cpp b/fs_mgr/libdm/dm_test.cpp index 541f254cb..4448d35c5 100644 --- a/fs_mgr/libdm/dm_test.cpp +++ b/fs_mgr/libdm/dm_test.cpp @@ -690,3 +690,23 @@ TEST(libdm, CreateEmptyDevice) { // Empty device should be in suspended state. ASSERT_EQ(DmDeviceState::SUSPENDED, dm.GetState("empty-device")); } + +TEST(libdm, UeventAfterLoadTable) { + static const char* kDeviceName = "libmd-test-uevent-load-table"; + + DeviceMapper& dm = DeviceMapper::Instance(); + ASSERT_TRUE(dm.CreateEmptyDevice(kDeviceName)); + + DmTable table; + table.Emplace<DmTargetError>(0, 1); + ASSERT_TRUE(dm.LoadTable(kDeviceName, table)); + + std::string ignore_path; + ASSERT_TRUE(dm.WaitForDevice(kDeviceName, 5s, &ignore_path)); + + auto info = dm.GetDetailedInfo(kDeviceName); + ASSERT_TRUE(info.has_value()); + ASSERT_TRUE(info->IsSuspended()); + + ASSERT_TRUE(dm.DeleteDevice(kDeviceName)); +} diff --git a/fs_mgr/libdm/include/libdm/dm.h b/fs_mgr/libdm/include/libdm/dm.h index 0a3e8d2bd..dbef8f902 100644 --- a/fs_mgr/libdm/include/libdm/dm.h +++ b/fs_mgr/libdm/include/libdm/dm.h @@ -75,6 +75,7 @@ class IDeviceMapper { const std::chrono::milliseconds& timeout_ms) = 0; virtual DmDeviceState GetState(const std::string& name) const = 0; virtual bool LoadTableAndActivate(const std::string& name, const DmTable& table) = 0; + virtual bool LoadTable(const std::string& name, const DmTable& table) = 0; virtual bool GetTableInfo(const std::string& name, std::vector<TargetInfo>* table) = 0; virtual bool GetTableStatus(const std::string& name, std::vector<TargetInfo>* table) = 0; virtual bool GetDmDevicePathByName(const std::string& name, std::string* path) = 0; @@ -116,7 +117,7 @@ class DeviceMapper final : public IDeviceMapper { bool IsBufferFull() const { return flags_ & DM_BUFFER_FULL_FLAG; } bool IsInactiveTablePresent() const { return flags_ & DM_INACTIVE_PRESENT_FLAG; } bool IsReadOnly() const { return flags_ & DM_READONLY_FLAG; } - bool IsSuspended() const { return flags_ & DM_SUSPEND_FLAG; } + bool IsSuspended() const { return !IsActiveTablePresent() || (flags_ & DM_SUSPEND_FLAG); } }; // Removes a device mapper device with the given name. @@ -199,6 +200,12 @@ class DeviceMapper final : public IDeviceMapper { // Returns 'true' on success, false otherwise. bool LoadTableAndActivate(const std::string& name, const DmTable& table) override; + // Same as LoadTableAndActivate, but there is no resume step. This puts the + // new table in the inactive slot. + // + // Returns 'true' on success, false otherwise. + bool LoadTable(const std::string& name, const DmTable& table) override; + // Returns true if a list of available device mapper targets registered in the kernel was // successfully read and stored in 'targets'. Returns 'false' otherwise. bool GetAvailableTargets(std::vector<DmTargetTypeInfo>* targets); diff --git a/fs_mgr/libdm/include/libdm/dm_table.h b/fs_mgr/libdm/include/libdm/dm_table.h index ee66653a5..427f34dea 100644 --- a/fs_mgr/libdm/include/libdm/dm_table.h +++ b/fs_mgr/libdm/include/libdm/dm_table.h @@ -31,6 +31,7 @@ namespace dm { class DmTable { public: DmTable() : num_sectors_(0), readonly_(false) {} + DmTable(DmTable&& other) = default; // Adds a target to the device mapper table for a range specified in the target object. // The function will return 'true' if the target was successfully added and doesn't overlap with @@ -70,6 +71,8 @@ class DmTable { void set_readonly(bool readonly) { readonly_ = readonly; } bool readonly() const { return readonly_; } + DmTable& operator=(DmTable&& other) = default; + ~DmTable() = default; private: diff --git a/fs_mgr/libdm/include/libdm/dm_target.h b/fs_mgr/libdm/include/libdm/dm_target.h index 954305816..09fe20012 100644 --- a/fs_mgr/libdm/include/libdm/dm_target.h +++ b/fs_mgr/libdm/include/libdm/dm_target.h @@ -323,6 +323,14 @@ class DmTargetUser final : public DmTarget { std::string control_device_; }; +class DmTargetError final : public DmTarget { + public: + DmTargetError(uint64_t start, uint64_t length) : DmTarget(start, length) {} + + std::string name() const override { return "error"; } + std::string GetParameterString() const override { return ""; } +}; + } // namespace dm } // namespace android diff --git a/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h b/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h index c3b40dca4..1de6d9835 100644 --- a/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h +++ b/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h @@ -143,6 +143,9 @@ class DeviceMapperWrapper : public android::dm::IDeviceMapper { virtual DmDeviceState GetState(const std::string& name) const override { return impl_.GetState(name); } + virtual bool LoadTable(const std::string& name, const DmTable& table) { + return impl_.LoadTable(name, table); + } virtual bool LoadTableAndActivate(const std::string& name, const DmTable& table) { return impl_.LoadTableAndActivate(name, table); } diff --git a/fs_mgr/tools/dmctl.cpp b/fs_mgr/tools/dmctl.cpp index 62ca1620b..10efd0cab 100644 --- a/fs_mgr/tools/dmctl.cpp +++ b/fs_mgr/tools/dmctl.cpp @@ -33,6 +33,7 @@ #include <ios> #include <iostream> #include <map> +#include <optional> #include <sstream> #include <string> #include <vector> @@ -183,6 +184,8 @@ class TargetParser final { } std::string control_device = NextArg(); return std::make_unique<DmTargetUser>(start_sector, num_sectors, control_device); + } else if (target_type == "error") { + return std::make_unique<DmTargetError>(start_sector, num_sectors); } else { std::cerr << "Unrecognized target type: " << target_type << std::endl; return nullptr; @@ -206,16 +209,26 @@ class TargetParser final { char** argv_; }; -static bool parse_table_args(DmTable* table, int argc, char** argv) { +struct TableArgs { + DmTable table; + bool suspended = false; +}; + +static std::optional<TableArgs> parse_table_args(int argc, char** argv) { + TableArgs out; + // Parse extended options first. int arg_index = 1; while (arg_index < argc && argv[arg_index][0] == '-') { if (strcmp(argv[arg_index], "-ro") == 0) { - table->set_readonly(true); + out.table.set_readonly(true); + arg_index++; + } else if (strcmp(argv[arg_index], "-suspended") == 0) { + out.suspended = true; arg_index++; } else { std::cerr << "Unrecognized option: " << argv[arg_index] << std::endl; - return -EINVAL; + return {}; } } @@ -223,37 +236,44 @@ static bool parse_table_args(DmTable* table, int argc, char** argv) { TargetParser parser(argc - arg_index, argv + arg_index); while (parser.More()) { std::unique_ptr<DmTarget> target = parser.Next(); - if (!target || !table->AddTarget(std::move(target))) { - return -EINVAL; + if (!target || !out.table.AddTarget(std::move(target))) { + return {}; } } - if (table->num_targets() == 0) { + if (out.table.num_targets() == 0) { std::cerr << "Must define at least one target." << std::endl; - return -EINVAL; + return {}; } - return 0; + return {std::move(out)}; } static int DmCreateCmdHandler(int argc, char** argv) { if (argc < 1) { - std::cerr << "Usage: dmctl create <dm-name> [-ro] <targets...>" << std::endl; + std::cerr << "Usage: dmctl create <dm-name> [--suspended] [-ro] <targets...>" << std::endl; return -EINVAL; } std::string name = argv[0]; - DmTable table; - int ret = parse_table_args(&table, argc, argv); - if (ret) { - return ret; + auto table_args = parse_table_args(argc, argv); + if (!table_args) { + return -EINVAL; } std::string ignore_path; DeviceMapper& dm = DeviceMapper::Instance(); - if (!dm.CreateDevice(name, table, &ignore_path, 5s)) { + if (!dm.CreateEmptyDevice(name)) { std::cerr << "Failed to create device-mapper device with name: " << name << std::endl; return -EIO; } + if (!dm.LoadTable(name, table_args->table)) { + std::cerr << "Failed to load table for dm device: " << name << std::endl; + return -EIO; + } + if (!table_args->suspended && !dm.ChangeState(name, DmDeviceState::ACTIVE)) { + std::cerr << "Failed to activate table for " << name << std::endl; + return -EIO; + } return 0; } @@ -269,7 +289,6 @@ static int DmDeleteCmdHandler(int argc, char** argv) { std::cerr << "Failed to delete [" << name << "]" << std::endl; return -EIO; } - return 0; } @@ -280,17 +299,20 @@ static int DmReplaceCmdHandler(int argc, char** argv) { } std::string name = argv[0]; - DmTable table; - int ret = parse_table_args(&table, argc, argv); - if (ret) { - return ret; + auto table_args = parse_table_args(argc, argv); + if (!table_args) { + return -EINVAL; } DeviceMapper& dm = DeviceMapper::Instance(); - if (!dm.LoadTableAndActivate(name, table)) { + if (!dm.LoadTable(name, table_args->table)) { std::cerr << "Failed to replace device-mapper table to: " << name << std::endl; return -EIO; } + if (!table_args->suspended && !dm.ChangeState(name, DmDeviceState::ACTIVE)) { + std::cerr << "Failed to activate table for " << name << std::endl; + return -EIO; + } return 0; } diff --git a/init/devices.cpp b/init/devices.cpp index d4a3cb9d3..ee8738c2a 100644 --- a/init/devices.cpp +++ b/init/devices.cpp @@ -465,7 +465,11 @@ void DeviceHandler::HandleDevice(const std::string& action, const std::string& d MakeDevice(devpath, block, major, minor, links); } - // We don't have full device-mapper information until a change event is fired. + // Handle device-mapper nodes. + // On kernels <= 5.10, the "add" event is fired on DM_DEV_CREATE, but does not contain name + // information until DM_TABLE_LOAD - thus, we wait for a "change" event. + // On kernels >= 5.15, the "add" event is fired on DM_TABLE_LOAD, followed by a "change" + // event. if (action == "add" || (action == "change" && StartsWith(devpath, "/dev/block/dm-"))) { for (const auto& link : links) { if (!mkdir_recursive(Dirname(link), 0755)) { |