Merge "snapuserd: Fix merging of overlapping copy blocks" am: 0efd6c2a4c
Original change: https://android-review.googlesource.com/c/platform/system/core/+/1700825
Change-Id: I63bcfb9878600baa696a03dfe0475d13334be9a7
diff --git a/fs_mgr/libsnapshot/cow_snapuserd_test.cpp b/fs_mgr/libsnapshot/cow_snapuserd_test.cpp
index 313eb64..3888eb1 100644
--- a/fs_mgr/libsnapshot/cow_snapuserd_test.cpp
+++ b/fs_mgr/libsnapshot/cow_snapuserd_test.cpp
@@ -96,7 +96,8 @@
class CowSnapuserdTest final {
public:
bool Setup();
- bool SetupCopyOverlap();
+ bool SetupCopyOverlap_1();
+ bool SetupCopyOverlap_2();
bool Merge();
void ValidateMerge();
void ReadSnapshotDeviceAndValidate();
@@ -115,7 +116,9 @@
void StartMerge();
void CreateCowDevice();
- void CreateCowDeviceWithCopyOverlap();
+ void CreateCowDeviceWithCopyOverlap_1();
+ void CreateCowDeviceWithCopyOverlap_2();
+ bool SetupDaemon();
void CreateBaseDevice();
void InitCowDevice();
void SetDeviceControlName();
@@ -193,10 +196,19 @@
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 @@
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 @@
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 @@
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_Merge_Resume_TEST) {
CowSnapuserdTest harness;
- ASSERT_TRUE(harness.SetupCopyOverlap());
+ ASSERT_TRUE(harness.SetupCopyOverlap_1());
harness.MergeInterrupt();
harness.ValidateMerge();
harness.Shutdown();
diff --git a/fs_mgr/libsnapshot/snapuserd.cpp b/fs_mgr/libsnapshot/snapuserd.cpp
index 2ccc750..3210983 100644
--- a/fs_mgr/libsnapshot/snapuserd.cpp
+++ b/fs_mgr/libsnapshot/snapuserd.cpp
@@ -437,6 +437,7 @@
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 @@
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 @@
}
}
map.clear();
+ dest_blocks.clear();
prev_id.reset();
}
diff --git a/fs_mgr/libsnapshot/snapuserd_server.cpp b/fs_mgr/libsnapshot/snapuserd_server.cpp
index ff8a259..3b0af3e 100644
--- a/fs_mgr/libsnapshot/snapuserd_server.cpp
+++ b/fs_mgr/libsnapshot/snapuserd_server.cpp
@@ -191,7 +191,7 @@
}
case DaemonOperations::DETACH: {
terminating_ = true;
- return Sendmsg(fd, "success");
+ return true;
}
default: {
LOG(ERROR) << "Received unknown message type from client";