From b03e4cdccdc201856c7b8c08d111c68d7474fd9e Mon Sep 17 00:00:00 2001 From: Akilesh Kailash Date: Tue, 13 Oct 2020 23:22:52 +0000 Subject: [PATCH] Integrate snapuserd with dm-user ABI changes. Create loopback device to simulate system_a/product_a partitions to test IO path. Bug: 168259959 Test: cow_snapuserd_test Signed-off-by: Akilesh Kailash Change-Id: I9f2a311d3eccfa20c82d0ebdb3e028ea3323a48d --- fs_mgr/libsnapshot/Android.bp | 2 + fs_mgr/libsnapshot/cow_snapuserd_test.cpp | 134 ++++++++++++++---- .../include/libsnapshot/snapuserd.h | 7 +- .../include/libsnapshot/snapuserd_client.h | 5 +- .../include/libsnapshot/snapuserd_kernel.h | 1 - .../include/libsnapshot/snapuserd_server.h | 6 +- fs_mgr/libsnapshot/snapuserd.cpp | 35 +---- fs_mgr/libsnapshot/snapuserd_client.cpp | 40 ++---- fs_mgr/libsnapshot/snapuserd_server.cpp | 14 +- 9 files changed, 149 insertions(+), 95 deletions(-) diff --git a/fs_mgr/libsnapshot/Android.bp b/fs_mgr/libsnapshot/Android.bp index ad592e9d0..0c5f3baa9 100644 --- a/fs_mgr/libsnapshot/Android.bp +++ b/fs_mgr/libsnapshot/Android.bp @@ -544,9 +544,11 @@ cc_test { "libsnapshot_snapuserd", "libcutils_sockets", "libz", + "libdm", ], header_libs: [ "libstorage_literals_headers", + "libfiemap_headers", ], test_min_api_level: 30, auto_gen_config: true, diff --git a/fs_mgr/libsnapshot/cow_snapuserd_test.cpp b/fs_mgr/libsnapshot/cow_snapuserd_test.cpp index 75e54f706..1d6c10405 100644 --- a/fs_mgr/libsnapshot/cow_snapuserd_test.cpp +++ b/fs_mgr/libsnapshot/cow_snapuserd_test.cpp @@ -12,12 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include #include #include #include #include +#include #include #include #include @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -34,6 +35,8 @@ namespace snapshot { using namespace android::storage_literals; using android::base::unique_fd; +using LoopDevice = android::dm::LoopDevice; +using namespace std::chrono_literals; class SnapuserdTest : public ::testing::Test { protected: @@ -50,6 +53,18 @@ class SnapuserdTest : public ::testing::Test { cow_product_1_ = std::make_unique(); ASSERT_GE(cow_product_1_->fd, 0) << strerror(errno); + // Create temp files in the PWD as selinux + // allows kernel domin to read from that directory only + // on userdebug/eng builds. Creating files under /data/local/tmp + // will have selinux denials. + std::string path = android::base::GetExecutableDirectory(); + + system_a_ = std::make_unique(path); + ASSERT_GE(system_a_->fd, 0) << strerror(errno); + + product_a_ = std::make_unique(path); + ASSERT_GE(product_a_->fd, 0) << strerror(errno); + size_ = 100_MiB; } @@ -61,6 +76,12 @@ class SnapuserdTest : public ::testing::Test { cow_product_1_ = nullptr; } + std::unique_ptr system_a_; + std::unique_ptr product_a_; + + std::unique_ptr system_a_loop_; + std::unique_ptr product_a_loop_; + std::unique_ptr cow_system_; std::unique_ptr cow_product_; @@ -76,6 +97,9 @@ class SnapuserdTest : public ::testing::Test { std::string system_device_name_; std::string product_device_name_; + std::string system_device_ctrl_name_; + std::string product_device_ctrl_name_; + std::unique_ptr random_buffer_1_; std::unique_ptr random_buffer_2_; std::unique_ptr zero_buffer_; @@ -86,10 +110,18 @@ class SnapuserdTest : public ::testing::Test { void CreateCowDevice(std::unique_ptr& cow); void CreateSystemDmUser(std::unique_ptr& cow); void CreateProductDmUser(std::unique_ptr& cow); + void DeleteDmUser(std::unique_ptr& cow, std::string snapshot_device); void StartSnapuserdDaemon(); void CreateSnapshotDevices(); void SwitchSnapshotDevices(); + std::string GetSystemControlPath() { + return std::string("/dev/dm-user-") + system_device_ctrl_name_; + } + std::string GetProductControlPath() { + return std::string("/dev/dm-user-") + product_device_ctrl_name_; + } + void TestIO(unique_fd& snapshot_fd, std::unique_ptr& buffer); SnapuserdClient client_; }; @@ -97,6 +129,7 @@ class SnapuserdTest : public ::testing::Test { void SnapuserdTest::Init() { unique_fd rnd_fd; loff_t offset = 0; + std::unique_ptr random_buffer = std::make_unique(1_MiB); rnd_fd.reset(open("/dev/random", O_RDONLY)); ASSERT_TRUE(rnd_fd > 0); @@ -118,10 +151,27 @@ void SnapuserdTest::Init() { offset += 1_MiB; } - sys_fd_.reset(open("/dev/block/mapper/system_a", O_RDONLY)); + for (size_t j = 0; j < (800_MiB / 1_MiB); j++) { + ASSERT_EQ(ReadFullyAtOffset(rnd_fd, (char*)random_buffer.get(), 1_MiB, 0), true); + ASSERT_EQ(android::base::WriteFully(system_a_->fd, random_buffer.get(), 1_MiB), true); + } + + for (size_t j = 0; j < (800_MiB / 1_MiB); j++) { + ASSERT_EQ(ReadFullyAtOffset(rnd_fd, (char*)random_buffer.get(), 1_MiB, 0), true); + ASSERT_EQ(android::base::WriteFully(product_a_->fd, random_buffer.get(), 1_MiB), true); + } + + // Create loopback devices + system_a_loop_ = std::make_unique(std::string(system_a_->path), 10s); + ASSERT_TRUE(system_a_loop_->valid()); + + product_a_loop_ = std::make_unique(std::string(product_a_->path), 10s); + ASSERT_TRUE(product_a_loop_->valid()); + + sys_fd_.reset(open(system_a_loop_->device().c_str(), O_RDONLY)); ASSERT_TRUE(sys_fd_ > 0); - product_fd_.reset(open("/dev/block/mapper/product_a", O_RDONLY)); + product_fd_.reset(open(product_a_loop_->device().c_str(), O_RDONLY)); ASSERT_TRUE(product_fd_ > 0); // Read from system partition from offset 0 of size 100MB @@ -183,47 +233,65 @@ void SnapuserdTest::CreateCowDevice(std::unique_ptr& cow) { } void SnapuserdTest::CreateSystemDmUser(std::unique_ptr& cow) { - unique_fd system_a_fd; std::string cmd; system_device_name_.clear(); + system_device_ctrl_name_.clear(); // Create a COW device. Number of sectors is chosen random which can // hold at least 400MB of data - system_a_fd.reset(open("/dev/block/mapper/system_a", O_RDONLY)); - ASSERT_TRUE(system_a_fd > 0); - - int err = ioctl(system_a_fd.get(), BLKGETSIZE, &system_blksize_); + int err = ioctl(sys_fd_.get(), BLKGETSIZE, &system_blksize_); ASSERT_GE(err, 0); std::string str(cow->path); std::size_t found = str.find_last_of("/\\"); ASSERT_NE(found, std::string::npos); system_device_name_ = str.substr(found + 1); + + // Create a control device + system_device_ctrl_name_ = system_device_name_ + "-ctrl"; cmd = "dmctl create " + system_device_name_ + " user 0 " + std::to_string(system_blksize_); + cmd += " " + system_device_ctrl_name_; + + system(cmd.c_str()); +} + +void SnapuserdTest::DeleteDmUser(std::unique_ptr& cow, std::string snapshot_device) { + std::string cmd; + + cmd = "dmctl delete " + snapshot_device; + system(cmd.c_str()); + + cmd.clear(); + + std::string str(cow->path); + std::size_t found = str.find_last_of("/\\"); + ASSERT_NE(found, std::string::npos); + std::string device_name = str.substr(found + 1); + + cmd = "dmctl delete " + device_name; system(cmd.c_str()); } void SnapuserdTest::CreateProductDmUser(std::unique_ptr& cow) { - unique_fd product_a_fd; std::string cmd; product_device_name_.clear(); + product_device_ctrl_name_.clear(); // Create a COW device. Number of sectors is chosen random which can // hold at least 400MB of data - product_a_fd.reset(open("/dev/block/mapper/product_a", O_RDONLY)); - ASSERT_TRUE(product_a_fd > 0); - - int err = ioctl(product_a_fd.get(), BLKGETSIZE, &product_blksize_); + int err = ioctl(product_fd_.get(), BLKGETSIZE, &product_blksize_); ASSERT_GE(err, 0); std::string str(cow->path); std::size_t found = str.find_last_of("/\\"); ASSERT_NE(found, std::string::npos); product_device_name_ = str.substr(found + 1); + product_device_ctrl_name_ = product_device_name_ + "-ctrl"; cmd = "dmctl create " + product_device_name_ + " user 0 " + std::to_string(product_blksize_); + cmd += " " + product_device_ctrl_name_; system(cmd.c_str()); } @@ -234,10 +302,12 @@ void SnapuserdTest::StartSnapuserdDaemon() { ret = client_.StartSnapuserd(); ASSERT_EQ(ret, 0); - ret = client_.InitializeSnapuserd(cow_system_->path, "/dev/block/mapper/system_a"); + ret = client_.InitializeSnapuserd(cow_system_->path, system_a_loop_->device(), + GetSystemControlPath()); ASSERT_EQ(ret, 0); - ret = client_.InitializeSnapuserd(cow_product_->path, "/dev/block/mapper/product_a"); + ret = client_.InitializeSnapuserd(cow_product_->path, product_a_loop_->device(), + GetProductControlPath()); ASSERT_EQ(ret, 0); } @@ -245,7 +315,7 @@ void SnapuserdTest::CreateSnapshotDevices() { std::string cmd; cmd = "dmctl create system-snapshot -ro snapshot 0 " + std::to_string(system_blksize_); - cmd += " /dev/block/mapper/system_a"; + cmd += " " + system_a_loop_->device(); cmd += " /dev/block/mapper/" + system_device_name_; cmd += " P 8"; @@ -254,7 +324,7 @@ void SnapuserdTest::CreateSnapshotDevices() { cmd.clear(); cmd = "dmctl create product-snapshot -ro snapshot 0 " + std::to_string(product_blksize_); - cmd += " /dev/block/mapper/product_a"; + cmd += " " + product_a_loop_->device(); cmd += " /dev/block/mapper/" + product_device_name_; cmd += " P 8"; @@ -265,7 +335,7 @@ void SnapuserdTest::SwitchSnapshotDevices() { std::string cmd; cmd = "dmctl create system-snapshot-1 -ro snapshot 0 " + std::to_string(system_blksize_); - cmd += " /dev/block/mapper/system_a"; + cmd += " " + system_a_loop_->device(); cmd += " /dev/block/mapper/" + system_device_name_; cmd += " P 8"; @@ -274,7 +344,7 @@ void SnapuserdTest::SwitchSnapshotDevices() { cmd.clear(); cmd = "dmctl create product-snapshot-1 -ro snapshot 0 " + std::to_string(product_blksize_); - cmd += " /dev/block/mapper/product_a"; + cmd += " " + product_a_loop_->device(); cmd += " /dev/block/mapper/" + product_device_name_; cmd += " P 8"; @@ -318,7 +388,7 @@ void SnapuserdTest::TestIO(unique_fd& snapshot_fd, std::unique_ptr& b // // IO path: // - // dm-snap->dm-snap-persistent->dm-user->snapuserd->read_from_system_a_partition + // dm-snap->dm-snap-persistent->dm-user->snapuserd->read_from_(system_a/product_a) partition // (copy op) -> return ASSERT_EQ(ReadFullyAtOffset(snapshot_fd, snapuserd_buffer.get(), size_, offset), true); @@ -379,24 +449,33 @@ TEST_F(SnapuserdTest, ReadWrite) { ASSERT_TRUE(snapshot_fd > 0); TestIO(snapshot_fd, product_buffer_); + snapshot_fd.reset(-1); + // Sequence of operations for transition CreateCowDevice(cow_system_1_); CreateCowDevice(cow_product_1_); + // Create dm-user which creates new control devices CreateSystemDmUser(cow_system_1_); CreateProductDmUser(cow_product_1_); - std::vector> vec; - vec.push_back(std::make_pair(cow_system_1_->path, "/dev/block/mapper/system_a")); - vec.push_back(std::make_pair(cow_product_1_->path, "/dev/block/mapper/product_a")); + // Send the path information to second stage daemon through vector + std::vector> vec{ + {cow_system_1_->path, system_a_loop_->device(), GetSystemControlPath()}, + {cow_product_1_->path, product_a_loop_->device(), GetProductControlPath()}}; - // Start the second stage deamon and send the devices + // Start the second stage deamon and send the devices information through + // vector. ASSERT_EQ(client_.RestartSnapuserd(vec), 0); // TODO: This is not switching snapshot device but creates a new table; - // however, it should serve the testing purpose. + // Second stage daemon will be ready to serve the IO request. From now + // onwards, we can go ahead and shutdown the first stage daemon SwitchSnapshotDevices(); + DeleteDmUser(cow_system_, "system-snapshot"); + DeleteDmUser(cow_product_, "product-snapshot"); + // Stop the first stage daemon ASSERT_EQ(client_.StopSnapuserd(true), 0); @@ -409,6 +488,11 @@ TEST_F(SnapuserdTest, ReadWrite) { ASSERT_TRUE(snapshot_fd > 0); TestIO(snapshot_fd, product_buffer_); + snapshot_fd.reset(-1); + + DeleteDmUser(cow_system_1_, "system-snapshot-1"); + DeleteDmUser(cow_product_1_, "product-snapshot-1"); + // Stop the second stage daemon ASSERT_EQ(client_.StopSnapuserd(false), 0); } diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapuserd.h b/fs_mgr/libsnapshot/include/libsnapshot/snapuserd.h index d495014f9..2f727d6ee 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapuserd.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapuserd.h @@ -60,9 +60,11 @@ class BufferSink : public IByteSink { class Snapuserd final { public: - Snapuserd(const std::string& in_cow_device, const std::string& in_backing_store_device) + Snapuserd(const std::string& in_cow_device, const std::string& in_backing_store_device, + const std::string& in_control_device) : cow_device_(in_cow_device), backing_store_device_(in_backing_store_device), + control_device_(in_control_device), metadata_read_done_(false) {} bool Init(); @@ -75,6 +77,8 @@ class Snapuserd final { int ReadDiskExceptions(chunk_t chunk, size_t size); int ReadData(chunk_t chunk, size_t size); + std::string GetControlDevicePath() { return control_device_; } + private: int ProcessReplaceOp(const CowOperation* cow_op); int ProcessCopyOp(const CowOperation* cow_op); @@ -82,6 +86,7 @@ class Snapuserd final { std::string cow_device_; std::string backing_store_device_; + std::string control_device_; unique_fd cow_fd_; unique_fd backing_store_fd_; diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_client.h b/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_client.h index 535e9235e..ab2149e04 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_client.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_client.h @@ -51,8 +51,9 @@ class SnapuserdClient { public: int StartSnapuserd(); int StopSnapuserd(bool firstStageDaemon); - int RestartSnapuserd(std::vector>& vec); - int InitializeSnapuserd(std::string cow_device, std::string backing_device); + int RestartSnapuserd(std::vector>& vec); + int InitializeSnapuserd(std::string cow_device, std::string backing_device, + std::string control_device); }; } // namespace snapshot diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_kernel.h b/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_kernel.h index 1a6ba8fe4..1037c126b 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_kernel.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_kernel.h @@ -80,7 +80,6 @@ struct dm_user_header { __u64 flags; __u64 sector; __u64 len; - __u64 io_in_progress; } __attribute__((packed)); struct dm_user_payload { diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_server.h b/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_server.h index 584fe7154..a1ebd3af4 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_server.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapuserd_server.h @@ -60,7 +60,8 @@ class Stoppable { virtual ~Stoppable() {} - virtual void ThreadStart(std::string cow_device, std::string backing_device) = 0; + virtual void ThreadStart(std::string cow_device, std::string backing_device, + std::string control_device) = 0; bool StopRequested() { // checks if value in future object is available @@ -78,7 +79,8 @@ class SnapuserdServer : public Stoppable { bool terminating_; std::vector> clients_vec_; - void ThreadStart(std::string cow_device, std::string backing_device) override; + void ThreadStart(std::string cow_device, std::string backing_device, + std::string control_device) override; void ShutdownThreads(); DaemonOperations Resolveop(std::string& input); std::string GetDaemonStatus(); diff --git a/fs_mgr/libsnapshot/snapuserd.cpp b/fs_mgr/libsnapshot/snapuserd.cpp index 3ed853f29..62ef1b063 100644 --- a/fs_mgr/libsnapshot/snapuserd.cpp +++ b/fs_mgr/libsnapshot/snapuserd.cpp @@ -34,18 +34,6 @@ static constexpr size_t PAYLOAD_SIZE = (1UL << 16); static_assert(PAYLOAD_SIZE >= BLOCK_SIZE); -class Target { - public: - // Represents an already-created Target, which is referenced by UUID. - Target(std::string uuid) : uuid_(uuid) {} - - const auto& uuid() { return uuid_; } - std::string control_path() { return std::string("/dev/dm-user-") + uuid(); } - - private: - const std::string uuid_; -}; - void BufferSink::Initialize(size_t size) { buffer_size_ = size; buffer_offset_ = 0; @@ -498,26 +486,13 @@ bool Snapuserd::Init() { return false; } - std::string str(cow_device_); - std::size_t found = str.find_last_of("/\\"); - CHECK(found != std::string::npos); - std::string device_name = str.substr(found + 1); + std::string control_path = GetControlDevicePath(); - LOG(DEBUG) << "Fetching UUID for: " << device_name; + LOG(DEBUG) << "Opening control device " << control_path; - auto& dm = dm::DeviceMapper::Instance(); - std::string uuid; - if (!dm.GetDmDeviceUuidByName(device_name, &uuid)) { - LOG(ERROR) << "Unable to find UUID for " << cow_device_; - return false; - } - - LOG(DEBUG) << "UUID: " << uuid; - Target t(uuid); - - ctrl_fd_.reset(open(t.control_path().c_str(), O_RDWR)); + ctrl_fd_.reset(open(control_path.c_str(), O_RDWR)); if (ctrl_fd_ < 0) { - LOG(ERROR) << "Unable to open " << t.control_path(); + LOG(ERROR) << "Unable to open " << control_path; return false; } @@ -553,7 +528,6 @@ int Snapuserd::Run() { case DM_USER_MAP_READ: { size_t remaining_size = header->len; loff_t offset = 0; - header->io_in_progress = 0; ret = 0; do { size_t read_size = std::min(PAYLOAD_SIZE, remaining_size); @@ -619,7 +593,6 @@ int Snapuserd::Run() { if (remaining_size) { LOG(DEBUG) << "Write done ret: " << ret << " remaining size: " << remaining_size; - bufsink_.GetHeaderPtr()->io_in_progress = 1; } } while (remaining_size); diff --git a/fs_mgr/libsnapshot/snapuserd_client.cpp b/fs_mgr/libsnapshot/snapuserd_client.cpp index bef8f5c8b..78dbadabb 100644 --- a/fs_mgr/libsnapshot/snapuserd_client.cpp +++ b/fs_mgr/libsnapshot/snapuserd_client.cpp @@ -127,24 +127,6 @@ std::string SnapuserdClient::Receivemsg() { return msgStr; } -#if 0 -std::string SnapuserdClient::Receivemsg() { - char msg[PACKET_SIZE]; - std::string msgStr("fail"); - int ret; - - ret = TEMP_FAILURE_RETRY(recv(sockfd_, msg, PACKET_SIZE, 0)); - if (ret <= 0) { - LOG(ERROR) << "recv failed " << strerror(errno); - return msgStr; - } - - msgStr.clear(); - msgStr = msg; - return msgStr; -} -#endif - int SnapuserdClient::StopSnapuserd(bool firstStageDaemon) { if (firstStageDaemon) { sockfd_ = socket_local_client(GetSocketNameFirstStage().c_str(), @@ -209,7 +191,8 @@ int SnapuserdClient::StartSnapuserd() { return 0; } -int SnapuserdClient::InitializeSnapuserd(std::string cow_device, std::string backing_device) { +int SnapuserdClient::InitializeSnapuserd(std::string cow_device, std::string backing_device, + std::string control_device) { int ret = 0; if (!ConnectToServer()) { @@ -217,7 +200,7 @@ int SnapuserdClient::InitializeSnapuserd(std::string cow_device, std::string bac return -1; } - std::string msg = "start," + cow_device + "," + backing_device; + std::string msg = "start," + cow_device + "," + backing_device + "," + control_device; ret = Sendmsg(msg.c_str(), msg.size()); if (ret < 0) { @@ -270,7 +253,7 @@ int SnapuserdClient::InitializeSnapuserd(std::string cow_device, std::string bac * completely active to serve the IO and merging process. * */ -int SnapuserdClient::RestartSnapuserd(std::vector>& vec) { +int SnapuserdClient::RestartSnapuserd(std::vector>& vec) { // Connect to first-stage daemon and send a terminate-request control // message. This will not terminate the daemon but will mark the daemon as // passive. @@ -306,14 +289,19 @@ int SnapuserdClient::RestartSnapuserd(std::vector, + // start,,, // // Start the new thread which binds to dm-user misc device newClient = std::make_unique(); newClient->SetThreadHandler( - std::bind(&SnapuserdServer::ThreadStart, this, out[1], out[2])); + std::bind(&SnapuserdServer::ThreadStart, this, out[1], out[2], out[3])); clients_vec_.push_back(std::move(newClient)); sprintf(msg, "success"); Sendmsg(fd, msg, MAX_PACKET_SIZE);