From 5d30009a7e12e7a066fb1d9e3b1b16267fe905e2 Mon Sep 17 00:00:00 2001 From: Daniel Zheng Date: Mon, 20 Nov 2023 11:40:36 -0800 Subject: [PATCH 1/3] libsnapshot: update variable name updating name to count rather than buffer size Test: cow_api_test Change-Id: I9e44330e7a230b5ab5f5e914ef74a63cc4ebaa61 --- fs_mgr/libsnapshot/include/libsnapshot/cow_format.h | 4 ++-- fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/fs_mgr/libsnapshot/include/libsnapshot/cow_format.h b/fs_mgr/libsnapshot/include/libsnapshot/cow_format.h index 75467cb79..9e6cfea33 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/cow_format.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/cow_format.h @@ -105,8 +105,8 @@ struct ResumePoint { static constexpr uint8_t kNumResumePoints = 4; struct CowHeaderV3 : public CowHeader { - // Location of sequence buffer in COW. - uint64_t sequence_buffer_offset; + // Number of sequence data stored (each of which is a 32 byte integer) + uint64_t sequence_data_count; // number of currently written resume points uint32_t resume_point_count; // Size, in bytes, of the CowResumePoint buffer. diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.cpp b/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.cpp index 6883c5e93..b3c8a2074 100644 --- a/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.cpp +++ b/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.cpp @@ -77,7 +77,7 @@ void CowWriterV3::SetupHeaders() { // v3 specific fields // WIP: not quite sure how some of these are calculated yet, assuming buffer_size is determined // during COW size estimation - header_.sequence_buffer_offset = 0; + header_.sequence_data_count = 0; header_.resume_point_count = 0; header_.resume_point_max = kNumResumePoints; header_.op_count = 0; From 763776435d848bb781ac89656658be1101065617 Mon Sep 17 00:00:00 2001 From: Daniel Zheng Date: Mon, 20 Nov 2023 03:01:03 -0800 Subject: [PATCH 2/3] libsnapshot: sync header metadata After we write emit a label, we need to update the number of resume points + sequence data and op_count. Realistically we could just call Finalize, but maybe synching these specific fields could prevent unexpected outcomes. Test: cow_api_test Change-Id: I1585601a134221689ce8d5675a2a3e32f1e8a0e6 --- .../libsnapshot/libsnapshot_cow/test_v3.cpp | 38 +++++++++++++++++++ .../libsnapshot/libsnapshot_cow/writer_v3.cpp | 2 +- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/test_v3.cpp b/fs_mgr/libsnapshot/libsnapshot_cow/test_v3.cpp index c41e07cc2..9f857eeb4 100644 --- a/fs_mgr/libsnapshot/libsnapshot_cow/test_v3.cpp +++ b/fs_mgr/libsnapshot/libsnapshot_cow/test_v3.cpp @@ -482,5 +482,43 @@ TEST_F(CowTestV3, ResumePointTest) { header = reader.header_v3(); ASSERT_EQ(header.op_count, 15); } + +TEST_F(CowTestV3, BufferMetadataSyncTest) { + CowOptions options; + options.op_count_max = 100; + auto writer = CreateCowWriter(3, options, GetCowFd()); + /* + Header metadafields + sequence_data_count = 0; + resume_point_count = 0; + resume_point_max = 4; + */ + ASSERT_TRUE(writer->Finalize()); + + CowReader reader; + ASSERT_TRUE(reader.Parse(cow_->fd)); + + auto header = reader.header_v3(); + ASSERT_EQ(header.sequence_data_count, 0); + ASSERT_EQ(header.resume_point_count, 0); + ASSERT_EQ(header.resume_point_max, 4); + + writer->AddLabel(0); + ASSERT_TRUE(reader.Parse(cow_->fd)); + header = reader.header_v3(); + ASSERT_EQ(header.sequence_data_count, 0); + ASSERT_EQ(header.resume_point_count, 1); + ASSERT_EQ(header.resume_point_max, 4); + + ASSERT_TRUE(reader.Parse(cow_->fd)); + header = reader.header_v3(); + + /* + Header metadafields + sequence_data_count = 1; + resume_point_count = 0; + resume_point_max = 4; + */ +} } // namespace snapshot } // namespace android diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.cpp b/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.cpp index b3c8a2074..e9e05b088 100644 --- a/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.cpp +++ b/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.cpp @@ -311,7 +311,7 @@ bool CowWriterV3::EmitLabel(uint64_t label) { PLOG(ERROR) << "writing resume buffer failed"; return false; } - return Sync(); + return Finalize(); } bool CowWriterV3::EmitSequenceData(size_t num_ops, const uint32_t* data) { From 209fda3562108d83fe109345990092b314722559 Mon Sep 17 00:00:00 2001 From: Daniel Zheng Date: Mon, 20 Nov 2023 03:48:07 -0800 Subject: [PATCH 3/3] libsnapshot: move header op count setup Op count should be set before we sync the header. This way subsequence writers can initialize with the correct op buffer size Test: cow_api_test Change-Id: I56a0d747b3f2a1d9d582d8f9d643b81cbdd9b8d7 --- fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.cpp b/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.cpp index e9e05b088..81ccea9c1 100644 --- a/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.cpp +++ b/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.cpp @@ -100,6 +100,7 @@ bool CowWriterV3::ParseOptions() { return false; } header_.compression_algorithm = *algorithm; + header_.op_count_max = options_.op_count_max; if (parts.size() > 1) { if (!android::base::ParseUint(parts[1], &compression_.compression_level)) { @@ -163,7 +164,7 @@ bool CowWriterV3::OpenForWrite() { return false; } } - header_.op_count_max = options_.op_count_max; + resume_points_ = std::make_shared>(); if (!Sync()) {