storageproxyd: Add file handle param to debug buffer print am: 3bb483b196
Original change: https://googleplex-android-review.googlesource.com/c/platform/system/core/+/15787946
Change-Id: I4c8edb5e565776803cabc21efbedfcb9786a8172
diff --git a/fs_mgr/libdm/dm.cpp b/fs_mgr/libdm/dm.cpp
index e43c00b..b1d5b39 100644
--- a/fs_mgr/libdm/dm.cpp
+++ b/fs_mgr/libdm/dm.cpp
@@ -170,19 +170,18 @@
return access("/system/bin/recovery", F_OK) == 0;
}
-bool DeviceMapper::CreateDevice(const std::string& name, const DmTable& table, std::string* path,
- const std::chrono::milliseconds& timeout_ms) {
+bool DeviceMapper::CreateEmptyDevice(const std::string& name) {
std::string uuid = GenerateUuid();
- if (!CreateDevice(name, uuid)) {
- return false;
- }
+ return CreateDevice(name, uuid);
+}
+bool DeviceMapper::WaitForDevice(const std::string& name,
+ const std::chrono::milliseconds& timeout_ms, std::string* path) {
// 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)) {
+ if (!GetDeviceUniquePath(name, &unique_path) || !GetDmDevicePathByName(name, path)) {
DeleteDevice(name);
return false;
}
@@ -208,6 +207,25 @@
return true;
}
+bool DeviceMapper::CreateDevice(const std::string& name, const DmTable& table, std::string* path,
+ const std::chrono::milliseconds& timeout_ms) {
+ if (!CreateEmptyDevice(name)) {
+ return false;
+ }
+
+ if (!LoadTableAndActivate(name, table)) {
+ DeleteDevice(name);
+ return false;
+ }
+
+ if (!WaitForDevice(name, timeout_ms, path)) {
+ DeleteDevice(name);
+ return false;
+ }
+
+ return true;
+}
+
bool DeviceMapper::GetDeviceUniquePath(const std::string& name, std::string* path) {
struct dm_ioctl io;
InitIo(&io, name);
diff --git a/fs_mgr/libdm/dm_test.cpp b/fs_mgr/libdm/dm_test.cpp
index 8006db2..8314ec5 100644
--- a/fs_mgr/libdm/dm_test.cpp
+++ b/fs_mgr/libdm/dm_test.cpp
@@ -29,6 +29,7 @@
#include <thread>
#include <android-base/file.h>
+#include <android-base/scopeguard.h>
#include <android-base/strings.h>
#include <android-base/unique_fd.h>
#include <gtest/gtest.h>
@@ -679,3 +680,17 @@
ASSERT_NE(0, access(path.c_str(), F_OK));
ASSERT_EQ(ENOENT, errno);
}
+
+TEST(libdm, CreateEmptyDevice) {
+ DeviceMapper& dm = DeviceMapper::Instance();
+ ASSERT_TRUE(dm.CreateEmptyDevice("empty-device"));
+ auto guard = android::base::make_scope_guard([&]() { dm.DeleteDevice("empty-device", 5s); });
+
+ // Empty device should be in suspended state.
+ ASSERT_EQ(DmDeviceState::SUSPENDED, dm.GetState("empty-device"));
+
+ std::string path;
+ ASSERT_TRUE(dm.WaitForDevice("empty-device", 5s, &path));
+ // Path should exist.
+ ASSERT_EQ(0, access(path.c_str(), F_OK));
+}
diff --git a/fs_mgr/libdm/include/libdm/dm.h b/fs_mgr/libdm/include/libdm/dm.h
index bdbbf91..e6698ea 100644
--- a/fs_mgr/libdm/include/libdm/dm.h
+++ b/fs_mgr/libdm/include/libdm/dm.h
@@ -119,6 +119,19 @@
// - ACTIVE: resumes the device.
bool ChangeState(const std::string& name, DmDeviceState state);
+ // Creates empty device.
+ // This supports a use case when a caller doesn't need a device straight away, but instead
+ // asks kernel to create it beforehand, thus avoiding blocking itself from waiting for ueventd
+ // to create user space paths.
+ // Callers are expected to then activate their device by calling LoadTableAndActivate function.
+ // To avoid race conditions, callers must still synchronize with ueventd by calling
+ // WaitForDevice function.
+ bool CreateEmptyDevice(const std::string& name);
+
+ // Waits for device paths to be created in the user space.
+ bool WaitForDevice(const std::string& name, const std::chrono::milliseconds& timeout_ms,
+ std::string* path);
+
// 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
diff --git a/fs_mgr/libsnapshot/cow_snapuserd_test.cpp b/fs_mgr/libsnapshot/cow_snapuserd_test.cpp
index d09c6e9..bd432bb 100644
--- a/fs_mgr/libsnapshot/cow_snapuserd_test.cpp
+++ b/fs_mgr/libsnapshot/cow_snapuserd_test.cpp
@@ -108,6 +108,7 @@
void MergeInterruptFixed(int duration);
void MergeInterruptRandomly(int max_duration);
void ReadDmUserBlockWithoutDaemon();
+ void ReadLastBlock();
std::string snapshot_dev() const { return snapshot_dev_->path(); }
@@ -256,6 +257,73 @@
}
}
+void CowSnapuserdTest::ReadLastBlock() {
+ unique_fd rnd_fd;
+ total_base_size_ = BLOCK_SZ * 2;
+
+ base_fd_ = CreateTempFile("base_device", total_base_size_);
+ ASSERT_GE(base_fd_, 0);
+
+ rnd_fd.reset(open("/dev/random", O_RDONLY));
+ ASSERT_TRUE(rnd_fd > 0);
+
+ std::unique_ptr<uint8_t[]> random_buffer = std::make_unique<uint8_t[]>(BLOCK_SZ);
+
+ for (size_t j = 0; j < ((total_base_size_) / BLOCK_SZ); j++) {
+ ASSERT_EQ(ReadFullyAtOffset(rnd_fd, (char*)random_buffer.get(), BLOCK_SZ, 0), true);
+ ASSERT_EQ(android::base::WriteFully(base_fd_, random_buffer.get(), BLOCK_SZ), true);
+ }
+
+ ASSERT_EQ(lseek(base_fd_, 0, SEEK_SET), 0);
+
+ base_loop_ = std::make_unique<LoopDevice>(base_fd_, 10s);
+ ASSERT_TRUE(base_loop_->valid());
+
+ std::string path = android::base::GetExecutableDirectory();
+ cow_system_ = std::make_unique<TemporaryFile>(path);
+
+ std::unique_ptr<uint8_t[]> random_buffer_1_ = std::make_unique<uint8_t[]>(total_base_size_);
+ loff_t offset = 0;
+
+ // Fill random data
+ for (size_t j = 0; j < (total_base_size_ / BLOCK_SZ); j++) {
+ ASSERT_EQ(ReadFullyAtOffset(rnd_fd, (char*)random_buffer_1_.get() + offset, BLOCK_SZ, 0),
+ true);
+
+ offset += BLOCK_SZ;
+ }
+
+ CowOptions options;
+ options.compression = "gz";
+ CowWriter writer(options);
+
+ ASSERT_TRUE(writer.Initialize(cow_system_->fd));
+
+ ASSERT_TRUE(writer.AddRawBlocks(0, random_buffer_1_.get(), BLOCK_SZ));
+ ASSERT_TRUE(writer.AddRawBlocks(1, (char*)random_buffer_1_.get() + BLOCK_SZ, BLOCK_SZ));
+
+ ASSERT_TRUE(writer.Finalize());
+
+ SetDeviceControlName();
+
+ StartSnapuserdDaemon();
+ InitCowDevice();
+
+ CreateDmUserDevice();
+ InitDaemon();
+
+ CreateSnapshotDevice();
+
+ unique_fd snapshot_fd(open(snapshot_dev_->path().c_str(), O_RDONLY));
+ ASSERT_TRUE(snapshot_fd > 0);
+
+ std::unique_ptr<uint8_t[]> snapuserd_buffer = std::make_unique<uint8_t[]>(BLOCK_SZ);
+
+ offset = 7680;
+ ASSERT_EQ(ReadFullyAtOffset(snapshot_fd, snapuserd_buffer.get(), 512, offset), true);
+ ASSERT_EQ(memcmp(snapuserd_buffer.get(), (char*)random_buffer_1_.get() + offset, 512), 0);
+}
+
void CowSnapuserdTest::CreateBaseDevice() {
unique_fd rnd_fd;
@@ -1068,6 +1136,12 @@
harness.Shutdown();
}
+TEST(Snapuserd_Test, Snapshot_END_IO_TEST) {
+ CowSnapuserdTest harness;
+ harness.ReadLastBlock();
+ harness.Shutdown();
+}
+
TEST(Snapuserd_Test, Snapshot_COPY_Overlap_TEST_1) {
CowSnapuserdTest harness;
ASSERT_TRUE(harness.SetupCopyOverlap_1());
diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp
index 10cb7b8..4c94da2 100644
--- a/fs_mgr/libsnapshot/snapshot.cpp
+++ b/fs_mgr/libsnapshot/snapshot.cpp
@@ -518,6 +518,13 @@
break;
}
+ if (mode == SnapshotStorageMode::Persistent && status.state() == SnapshotState::MERGING) {
+ LOG(ERROR) << "Snapshot: " << name
+ << " has snapshot status Merging but mode set to Persistent."
+ << " Changing mode to Snapshot-Merge.";
+ mode = SnapshotStorageMode::Merge;
+ }
+
DmTable table;
table.Emplace<DmTargetSnapshot>(0, snapshot_sectors, base_device, cow_device, mode,
kSnapshotChunkSize);
@@ -886,6 +893,10 @@
if (target_type) {
*target_type = DeviceMapper::GetTargetType(target.spec);
}
+ if (!status->error.empty()) {
+ LOG(ERROR) << "Snapshot: " << dm_name << " returned error code: " << status->error;
+ return false;
+ }
return true;
}
diff --git a/fs_mgr/libsnapshot/snapuserd_worker.cpp b/fs_mgr/libsnapshot/snapuserd_worker.cpp
index 682f9da..defb5bb 100644
--- a/fs_mgr/libsnapshot/snapuserd_worker.cpp
+++ b/fs_mgr/libsnapshot/snapuserd_worker.cpp
@@ -287,16 +287,36 @@
it = std::lower_bound(chunk_vec.begin(), chunk_vec.end(), std::make_pair(sector, nullptr),
Snapuserd::compare);
- if (!(it != chunk_vec.end())) {
- SNAP_LOG(ERROR) << "ReadData: Sector " << sector << " not found in chunk_vec";
- return -1;
+ bool read_end_of_device = false;
+ if (it == chunk_vec.end()) {
+ // |-------|-------|-------|
+ // 0 1 2 3
+ //
+ // Block 0 - op 1
+ // Block 1 - op 2
+ // Block 2 - op 3
+ //
+ // chunk_vec will have block 0, 1, 2 which maps to relavant COW ops.
+ //
+ // Each block is 4k bytes. Thus, the last block will span 8 sectors
+ // ranging till block 3 (However, block 3 won't be in chunk_vec as
+ // it doesn't have any mapping to COW ops. Now, if we get an I/O request for a sector
+ // spanning between block 2 and block 3, we need to step back
+ // and get hold of the last element.
+ //
+ // Additionally, dm-snapshot makes sure that I/O request beyond block 3
+ // will not be routed to the daemon. Hence, it is safe to assume that
+ // if a sector is not available in the chunk_vec, the I/O falls in the
+ // end of region.
+ it = std::prev(chunk_vec.end());
+ read_end_of_device = true;
}
// We didn't find the required sector; hence find the previous sector
// as lower_bound will gives us the value greater than
// the requested sector
if (it->first != sector) {
- if (it != chunk_vec.begin()) {
+ if (it != chunk_vec.begin() && !read_end_of_device) {
--it;
}