From a7573c7b9065aa5d5b98a13d937431eb996c58e9 Mon Sep 17 00:00:00 2001 From: Daniel Rosenberg Date: Fri, 30 Jul 2021 20:39:20 -0700 Subject: [PATCH] libsnapshot: Don't PrepMergeOps on resume If we're reading up to a label, we're resuming setting up the file, and there is no reason to expect the ops we require for sequence ops to be present. In that case, skip prepping for merge, and return an empty merge iterator if it is mistakenly requested. Test: cow_api_test CowTest.ResumeSeqOp Change-Id: Idd93bd4c4209197b9728fcb21a7191aae971b62d --- fs_mgr/libsnapshot/cow_api_test.cpp | 48 +++++++++++++++++++++++++++++ fs_mgr/libsnapshot/cow_reader.cpp | 8 ++++- 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/fs_mgr/libsnapshot/cow_api_test.cpp b/fs_mgr/libsnapshot/cow_api_test.cpp index 7e9097e3f..60663098a 100644 --- a/fs_mgr/libsnapshot/cow_api_test.cpp +++ b/fs_mgr/libsnapshot/cow_api_test.cpp @@ -1113,6 +1113,54 @@ TEST_F(CowTest, MissingSeqOp) { ASSERT_FALSE(reader.Parse(cow_->fd)); } +TEST_F(CowTest, ResumeSeqOp) { + CowOptions options; + auto writer = std::make_unique(options); + const int seq_len = 10; + uint32_t sequence[seq_len]; + for (int i = 0; i < seq_len; i++) { + sequence[i] = i + 1; + } + + ASSERT_TRUE(writer->Initialize(cow_->fd)); + + ASSERT_TRUE(writer->AddSequenceData(seq_len, sequence)); + ASSERT_TRUE(writer->AddZeroBlocks(1, seq_len / 2)); + ASSERT_TRUE(writer->AddLabel(1)); + ASSERT_TRUE(writer->AddZeroBlocks(1 + seq_len / 2, 1)); + + ASSERT_EQ(lseek(cow_->fd, 0, SEEK_SET), 0); + auto reader = std::make_unique(); + ASSERT_TRUE(reader->Parse(cow_->fd, 1)); + auto itr = reader->GetRevMergeOpIter(); + ASSERT_TRUE(itr->Done()); + + writer = std::make_unique(options); + ASSERT_TRUE(writer->InitializeAppend(cow_->fd, 1)); + ASSERT_TRUE(writer->AddZeroBlocks(1 + seq_len / 2, seq_len / 2)); + ASSERT_TRUE(writer->Finalize()); + + ASSERT_EQ(lseek(cow_->fd, 0, SEEK_SET), 0); + + reader = std::make_unique(); + ASSERT_TRUE(reader->Parse(cow_->fd)); + + auto iter = reader->GetRevMergeOpIter(); + + uint64_t expected_block = 10; + while (!iter->Done() && expected_block > 0) { + ASSERT_FALSE(iter->Done()); + const auto& op = iter->Get(); + + ASSERT_EQ(op.new_block, expected_block); + + iter->Next(); + expected_block--; + } + ASSERT_EQ(expected_block, 0); + ASSERT_TRUE(iter->Done()); +} + TEST_F(CowTest, RevMergeOpItrTest) { CowOptions options; options.cluster_ops = 5; diff --git a/fs_mgr/libsnapshot/cow_reader.cpp b/fs_mgr/libsnapshot/cow_reader.cpp index 3f59001b3..773d978ad 100644 --- a/fs_mgr/libsnapshot/cow_reader.cpp +++ b/fs_mgr/libsnapshot/cow_reader.cpp @@ -34,7 +34,11 @@ namespace android { namespace snapshot { -CowReader::CowReader() : fd_(-1), header_(), fd_size_(0) {} +CowReader::CowReader() + : fd_(-1), + header_(), + fd_size_(0), + merge_op_blocks_(std::make_shared>()) {} static void SHA256(const void*, size_t, uint8_t[]) { #if 0 @@ -150,6 +154,8 @@ bool CowReader::Parse(android::base::borrowed_fd fd, std::optional lab if (!ParseOps(label)) { return false; } + // If we're resuming a write, we're not ready to merge + if (label.has_value()) return true; return PrepMergeOps(); }