Merge "snapuserd: Fix merging of overlapping copy blocks" am: 0efd6c2a4c am: 64b0b1a54e am: 9cfe067b2b

Original change: https://android-review.googlesource.com/c/platform/system/core/+/1700825

Change-Id: Ic577a48ace9c1cbdbdf4eca45cba8e1bb86a9773

fs_mgr/libsnapshot/cow_snapuserd_test.cpp

@@ -96,7 +96,8 @@ class TempDevice {
 class CowSnapuserdTest final {
   public:
     bool Setup();
-    bool SetupCopyOverlap();
+    bool SetupCopyOverlap_1();
+    bool SetupCopyOverlap_2();
     bool Merge();
     void ValidateMerge();
     void ReadSnapshotDeviceAndValidate();
@@ -115,7 +116,9 @@ class CowSnapuserdTest final {
     void StartMerge();
 
     void CreateCowDevice();
-    void CreateCowDeviceWithCopyOverlap();
+    void CreateCowDeviceWithCopyOverlap_1();
+    void CreateCowDeviceWithCopyOverlap_2();
+    bool SetupDaemon();
     void CreateBaseDevice();
     void InitCowDevice();
     void SetDeviceControlName();
@@ -193,10 +196,19 @@ bool CowSnapuserdTest::Setup() {
     return setup_ok_;
 }
 
-bool CowSnapuserdTest::SetupCopyOverlap() {
+bool CowSnapuserdTest::SetupCopyOverlap_1() {
     CreateBaseDevice();
-    CreateCowDeviceWithCopyOverlap();
+    CreateCowDeviceWithCopyOverlap_1();
+    return SetupDaemon();
+}
 
+bool CowSnapuserdTest::SetupCopyOverlap_2() {
+    CreateBaseDevice();
+    CreateCowDeviceWithCopyOverlap_2();
+    return SetupDaemon();
+}
+
+bool CowSnapuserdTest::SetupDaemon() {
     SetDeviceControlName();
 
     StartSnapuserdDaemon();
@@ -275,7 +287,59 @@ void CowSnapuserdTest::ReadSnapshotDeviceAndValidate() {
     ASSERT_EQ(memcmp(snapuserd_buffer.get(), (char*)orig_buffer_.get() + (size_ * 3), size_), 0);
 }
 
-void CowSnapuserdTest::CreateCowDeviceWithCopyOverlap() {
+void CowSnapuserdTest::CreateCowDeviceWithCopyOverlap_2() {
+    std::string path = android::base::GetExecutableDirectory();
+    cow_system_ = std::make_unique<TemporaryFile>(path);
+
+    CowOptions options;
+    options.compression = "gz";
+    CowWriter writer(options);
+
+    ASSERT_TRUE(writer.Initialize(cow_system_->fd));
+
+    size_t num_blocks = size_ / options.block_size;
+    size_t x = num_blocks;
+    size_t blk_src_copy = 0;
+
+    // Create overlapping copy operations
+    while (1) {
+        ASSERT_TRUE(writer.AddCopy(blk_src_copy, blk_src_copy + 1));
+        x -= 1;
+        if (x == 1) {
+            break;
+        }
+        blk_src_copy += 1;
+    }
+
+    // Flush operations
+    ASSERT_TRUE(writer.Finalize());
+
+    // Construct the buffer required for validation
+    orig_buffer_ = std::make_unique<uint8_t[]>(total_base_size_);
+
+    // Read the entire base device
+    ASSERT_EQ(android::base::ReadFullyAtOffset(base_fd_, orig_buffer_.get(), total_base_size_, 0),
+              true);
+
+    // Merged operations required for validation
+    int block_size = 4096;
+    x = num_blocks;
+    loff_t src_offset = block_size;
+    loff_t dest_offset = 0;
+
+    while (1) {
+        memmove((char*)orig_buffer_.get() + dest_offset, (char*)orig_buffer_.get() + src_offset,
+                block_size);
+        x -= 1;
+        if (x == 1) {
+            break;
+        }
+        src_offset += block_size;
+        dest_offset += block_size;
+    }
+}
+
+void CowSnapuserdTest::CreateCowDeviceWithCopyOverlap_1() {
     std::string path = android::base::GetExecutableDirectory();
     cow_system_ = std::make_unique<TemporaryFile>(path);
 
@@ -770,19 +834,19 @@ void CowSnapuserdMetadataTest::ValidateMetadata() {
 
             de = reinterpret_cast<struct disk_exception*>((char*)buffer + offset);
             ASSERT_EQ(de->old_chunk, 21);
-            ASSERT_EQ(de->new_chunk, 536);
-            offset += sizeof(struct disk_exception);
-
-            de = reinterpret_cast<struct disk_exception*>((char*)buffer + offset);
-            ASSERT_EQ(de->old_chunk, 22);
             ASSERT_EQ(de->new_chunk, 537);
             offset += sizeof(struct disk_exception);
 
             de = reinterpret_cast<struct disk_exception*>((char*)buffer + offset);
-            ASSERT_EQ(de->old_chunk, 23);
+            ASSERT_EQ(de->old_chunk, 22);
             ASSERT_EQ(de->new_chunk, 538);
             offset += sizeof(struct disk_exception);
 
+            de = reinterpret_cast<struct disk_exception*>((char*)buffer + offset);
+            ASSERT_EQ(de->old_chunk, 23);
+            ASSERT_EQ(de->new_chunk, 539);
+            offset += sizeof(struct disk_exception);
+
             // End of metadata
             de = reinterpret_cast<struct disk_exception*>((char*)buffer + offset);
             ASSERT_EQ(de->old_chunk, 0);
@@ -821,9 +885,17 @@ TEST(Snapuserd_Test, Snapshot_IO_TEST) {
     harness.Shutdown();
 }
 
-TEST(Snapuserd_Test, Snapshot_COPY_Overlap_TEST) {
+TEST(Snapuserd_Test, Snapshot_COPY_Overlap_TEST_1) {
     CowSnapuserdTest harness;
-    ASSERT_TRUE(harness.SetupCopyOverlap());
+    ASSERT_TRUE(harness.SetupCopyOverlap_1());
+    ASSERT_TRUE(harness.Merge());
+    harness.ValidateMerge();
+    harness.Shutdown();
+}
+
+TEST(Snapuserd_Test, Snapshot_COPY_Overlap_TEST_2) {
+    CowSnapuserdTest harness;
+    ASSERT_TRUE(harness.SetupCopyOverlap_2());
     ASSERT_TRUE(harness.Merge());
     harness.ValidateMerge();
     harness.Shutdown();
@@ -831,7 +903,7 @@ TEST(Snapuserd_Test, Snapshot_COPY_Overlap_TEST) {
 
 TEST(Snapuserd_Test, Snapshot_COPY_Overlap_Merge_Resume_TEST) {
     CowSnapuserdTest harness;
-    ASSERT_TRUE(harness.SetupCopyOverlap());
+    ASSERT_TRUE(harness.SetupCopyOverlap_1());
     harness.MergeInterrupt();
     harness.ValidateMerge();
     harness.Shutdown();

fs_mgr/libsnapshot/snapuserd.cpp

@@ -437,6 +437,7 @@ bool Snapuserd::ReadMetadata() {
     int num_ra_ops_per_iter = ((GetBufferDataSize()) / BLOCK_SZ);
     std::optional<chunk_t> prev_id = {};
     std::map<uint64_t, const CowOperation*> map;
+    std::set<uint64_t> dest_blocks;
     size_t pending_copy_ops = exceptions_per_area_ - num_ops;
     uint64_t total_copy_ops = reader_->total_copy_ops();
 
@@ -555,10 +556,15 @@ bool Snapuserd::ReadMetadata() {
                 if (diff != 1) {
                     break;
                 }
+
+                if (dest_blocks.count(cow_op->new_block) || map.count(cow_op->source) > 0) {
+                    break;
+                }
             }
             metadata_found = true;
             pending_copy_ops -= 1;
             map[cow_op->new_block] = cow_op;
+            dest_blocks.insert(cow_op->source);
             prev_id = cow_op->new_block;
             cowop_riter_->Next();
         } while (!cowop_riter_->Done() && pending_copy_ops);
@@ -620,6 +626,7 @@ bool Snapuserd::ReadMetadata() {
             }
         }
         map.clear();
+        dest_blocks.clear();
         prev_id.reset();
     }
 

fs_mgr/libsnapshot/snapuserd_server.cpp

@@ -191,7 +191,7 @@ bool SnapuserdServer::Receivemsg(android::base::borrowed_fd fd, const std::strin
         }
         case DaemonOperations::DETACH: {
             terminating_ = true;
-            return Sendmsg(fd, "success");
+            return true;
         }
         default: {
             LOG(ERROR) << "Received unknown message type from client";