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