From 359350250b39a5a5832db2d229f2367172167f2b Mon Sep 17 00:00:00 2001 From: David Anderson Date: Tue, 25 Apr 2023 17:20:36 -0700 Subject: [PATCH 1/3] libsnapshot: Remove fallible ICowReader::GetHeader. This function cannot fail, so replace it with an infallible version. This is also slightly more efficient since there's no copying involved. Bug: 280529365 Test: builds Change-Id: Iccfd6e72bc2192f4e1efda50cf544e1e956a9263 --- .../include/libsnapshot/cow_reader.h | 2 -- .../libsnapshot_cow/cow_api_test.cpp | 30 ++++++++++--------- .../libsnapshot_cow/cow_reader.cpp | 5 ---- .../libsnapshot_cow/cow_writer.cpp | 3 +- .../libsnapshot_cow/inspect_cow.cpp | 6 +--- fs_mgr/libsnapshot/snapshot_reader.cpp | 5 +--- .../snapuserd/dm-snapshot-merge/snapuserd.cpp | 25 +++++----------- .../user-space-merge/snapuserd_core.cpp | 28 +++++------------ 8 files changed, 35 insertions(+), 69 deletions(-) diff --git a/fs_mgr/libsnapshot/include/libsnapshot/cow_reader.h b/fs_mgr/libsnapshot/include/libsnapshot/cow_reader.h index 0450d7f48..f433dd9e8 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/cow_reader.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/cow_reader.h @@ -35,7 +35,6 @@ class ICowReader { virtual ~ICowReader() {} // Return the file header. - virtual bool GetHeader(CowHeader* header) = 0; virtual CowHeader& GetHeader() = 0; // Return the file footer. @@ -107,7 +106,6 @@ class CowReader final : public ICowReader { bool InitForMerge(android::base::unique_fd&& fd); bool VerifyMergeOps() override; - bool GetHeader(CowHeader* header) override; bool GetFooter(CowFooter* footer) override; bool GetLastLabel(uint64_t* label) override; diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/cow_api_test.cpp b/fs_mgr/libsnapshot/libsnapshot_cow/cow_api_test.cpp index f05aeb20f..ad8570952 100644 --- a/fs_mgr/libsnapshot/libsnapshot_cow/cow_api_test.cpp +++ b/fs_mgr/libsnapshot/libsnapshot_cow/cow_api_test.cpp @@ -62,15 +62,16 @@ TEST_F(CowTest, CopyContiguous) { ASSERT_EQ(lseek(cow_->fd, 0, SEEK_SET), 0); CowReader reader; - CowHeader header; - CowFooter footer; ASSERT_TRUE(reader.Parse(cow_->fd)); - ASSERT_TRUE(reader.GetHeader(&header)); - ASSERT_TRUE(reader.GetFooter(&footer)); + + const auto& header = reader.GetHeader(); ASSERT_EQ(header.magic, kCowMagicNumber); ASSERT_EQ(header.major_version, kCowVersionMajor); ASSERT_EQ(header.minor_version, kCowVersionMinor); ASSERT_EQ(header.block_size, options.block_size); + + CowFooter footer; + ASSERT_TRUE(reader.GetFooter(&footer)); ASSERT_EQ(footer.op.num_ops, 100); auto iter = reader.GetOpIter(); @@ -110,15 +111,16 @@ TEST_F(CowTest, ReadWrite) { ASSERT_EQ(lseek(cow_->fd, 0, SEEK_SET), 0); CowReader reader; - CowHeader header; - CowFooter footer; ASSERT_TRUE(reader.Parse(cow_->fd)); - ASSERT_TRUE(reader.GetHeader(&header)); - ASSERT_TRUE(reader.GetFooter(&footer)); + + const auto& header = reader.GetHeader(); ASSERT_EQ(header.magic, kCowMagicNumber); ASSERT_EQ(header.major_version, kCowVersionMajor); ASSERT_EQ(header.minor_version, kCowVersionMinor); ASSERT_EQ(header.block_size, options.block_size); + + CowFooter footer; + ASSERT_TRUE(reader.GetFooter(&footer)); ASSERT_EQ(footer.op.num_ops, 4); auto iter = reader.GetOpIter(); @@ -188,15 +190,16 @@ TEST_F(CowTest, ReadWriteXor) { ASSERT_EQ(lseek(cow_->fd, 0, SEEK_SET), 0); CowReader reader; - CowHeader header; - CowFooter footer; ASSERT_TRUE(reader.Parse(cow_->fd)); - ASSERT_TRUE(reader.GetHeader(&header)); - ASSERT_TRUE(reader.GetFooter(&footer)); + + const auto& header = reader.GetHeader(); ASSERT_EQ(header.magic, kCowMagicNumber); ASSERT_EQ(header.major_version, kCowVersionMajor); ASSERT_EQ(header.minor_version, kCowVersionMinor); ASSERT_EQ(header.block_size, options.block_size); + + CowFooter footer; + ASSERT_TRUE(reader.GetFooter(&footer)); ASSERT_EQ(footer.op.num_ops, 4); auto iter = reader.GetOpIter(); @@ -1065,8 +1068,7 @@ AssertionResult WriteDataBlock(CowWriter* writer, uint64_t new_block, std::strin AssertionResult CompareDataBlock(CowReader* reader, const CowOperation& op, const std::string& data) { - CowHeader header; - reader->GetHeader(&header); + const auto& header = reader->GetHeader(); std::string cmp = data; cmp.resize(header.block_size, '\0'); diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/cow_reader.cpp b/fs_mgr/libsnapshot/libsnapshot_cow/cow_reader.cpp index 31581064a..c388a9ce1 100644 --- a/fs_mgr/libsnapshot/libsnapshot_cow/cow_reader.cpp +++ b/fs_mgr/libsnapshot/libsnapshot_cow/cow_reader.cpp @@ -544,11 +544,6 @@ bool CowReader::VerifyMergeOps() { return true; } -bool CowReader::GetHeader(CowHeader* header) { - *header = header_; - return true; -} - bool CowReader::GetFooter(CowFooter* footer) { if (!footer_) return false; *footer = footer_.value(); diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/cow_writer.cpp b/fs_mgr/libsnapshot/libsnapshot_cow/cow_writer.cpp index 042ffb400..a996b57c9 100644 --- a/fs_mgr/libsnapshot/libsnapshot_cow/cow_writer.cpp +++ b/fs_mgr/libsnapshot/libsnapshot_cow/cow_writer.cpp @@ -392,10 +392,11 @@ bool CowWriter::OpenForAppend(uint64_t label) { auto reader = std::make_unique(); std::queue toAdd; - if (!reader->Parse(fd_, {label}) || !reader->GetHeader(&header_)) { + if (!reader->Parse(fd_, {label})) { return false; } + header_ = reader->GetHeader(); options_.block_size = header_.block_size; options_.cluster_ops = header_.cluster_ops; diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/inspect_cow.cpp b/fs_mgr/libsnapshot/libsnapshot_cow/inspect_cow.cpp index 271615679..f6b323ab8 100644 --- a/fs_mgr/libsnapshot/libsnapshot_cow/inspect_cow.cpp +++ b/fs_mgr/libsnapshot/libsnapshot_cow/inspect_cow.cpp @@ -94,11 +94,7 @@ static bool Inspect(const std::string& path, Options opt) { return false; } - CowHeader header; - if (!reader.GetHeader(&header)) { - LOG(ERROR) << "could not get header: " << path; - return false; - } + const CowHeader& header = reader.GetHeader(); CowFooter footer; bool has_footer = false; if (reader.GetFooter(&footer)) has_footer = true; diff --git a/fs_mgr/libsnapshot/snapshot_reader.cpp b/fs_mgr/libsnapshot/snapshot_reader.cpp index 54e2436c9..6111edd93 100644 --- a/fs_mgr/libsnapshot/snapshot_reader.cpp +++ b/fs_mgr/libsnapshot/snapshot_reader.cpp @@ -80,10 +80,7 @@ bool ReadFdFileDescriptor::Flush() { bool CompressedSnapshotReader::SetCow(std::unique_ptr&& cow) { cow_ = std::move(cow); - CowHeader header; - if (!cow_->GetHeader(&header)) { - return false; - } + const auto& header = cow_->GetHeader(); block_size_ = header.block_size; // Populate the operation map. diff --git a/fs_mgr/libsnapshot/snapuserd/dm-snapshot-merge/snapuserd.cpp b/fs_mgr/libsnapshot/snapuserd/dm-snapshot-merge/snapuserd.cpp index 89260306c..c3f6870af 100644 --- a/fs_mgr/libsnapshot/snapuserd/dm-snapshot-merge/snapuserd.cpp +++ b/fs_mgr/libsnapshot/snapuserd/dm-snapshot-merge/snapuserd.cpp @@ -347,7 +347,6 @@ void Snapuserd::CheckMergeCompletionStatus() { */ bool Snapuserd::ReadMetadata() { reader_ = std::make_unique(); - CowHeader header; CowOptions options; bool metadata_found = false; int replace_ops = 0, zero_ops = 0, copy_ops = 0; @@ -359,11 +358,7 @@ bool Snapuserd::ReadMetadata() { return false; } - if (!reader_->GetHeader(&header)) { - SNAP_LOG(ERROR) << "Failed to get header"; - return false; - } - + const auto& header = reader_->GetHeader(); if (!(header.block_size == BLOCK_SZ)) { SNAP_LOG(ERROR) << "Invalid header block size found: " << header.block_size; return false; @@ -636,8 +631,7 @@ bool Snapuserd::ReadMetadata() { } bool Snapuserd::MmapMetadata() { - CowHeader header; - reader_->GetHeader(&header); + const auto& header = reader_->GetHeader(); if (header.major_version >= 2 && header.buffer_size > 0) { total_mapped_addr_length_ = header.header_size + BUFFER_REGION_DEFAULT_SIZE; @@ -832,8 +826,7 @@ bool Snapuserd::Start() { } uint64_t Snapuserd::GetBufferMetadataOffset() { - CowHeader header; - reader_->GetHeader(&header); + const auto& header = reader_->GetHeader(); size_t size = header.header_size + sizeof(BufferState); return size; @@ -848,16 +841,14 @@ uint64_t Snapuserd::GetBufferMetadataOffset() { * */ size_t Snapuserd::GetBufferMetadataSize() { - CowHeader header; - reader_->GetHeader(&header); + const auto& header = reader_->GetHeader(); size_t metadata_bytes = (header.buffer_size * sizeof(struct ScratchMetadata)) / BLOCK_SZ; return metadata_bytes; } size_t Snapuserd::GetBufferDataOffset() { - CowHeader header; - reader_->GetHeader(&header); + const auto& header = reader_->GetHeader(); return (header.header_size + GetBufferMetadataSize()); } @@ -866,16 +857,14 @@ size_t Snapuserd::GetBufferDataOffset() { * (2MB - 8K = 2088960 bytes) will be the buffer region to hold the data. */ size_t Snapuserd::GetBufferDataSize() { - CowHeader header; - reader_->GetHeader(&header); + const auto& header = reader_->GetHeader(); size_t size = header.buffer_size - GetBufferMetadataSize(); return size; } struct BufferState* Snapuserd::GetBufferState() { - CowHeader header; - reader_->GetHeader(&header); + const auto& header = reader_->GetHeader(); struct BufferState* ra_state = reinterpret_cast((char*)mapped_addr_ + header.header_size); diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp index 25ce0ae30..1e4960020 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp @@ -98,8 +98,7 @@ bool SnapshotHandler::CommitMerge(int num_merge_ops) { } } else { reader_->UpdateMergeOpsCompleted(num_merge_ops); - CowHeader header; - reader_->GetHeader(&header); + const auto& header = reader_->GetHeader(); if (lseek(cow_fd_.get(), 0, SEEK_SET) < 0) { SNAP_PLOG(ERROR) << "lseek failed"; @@ -154,7 +153,6 @@ bool SnapshotHandler::CheckMergeCompletionStatus() { bool SnapshotHandler::ReadMetadata() { reader_ = std::make_unique(CowReader::ReaderFlags::USERSPACE_MERGE, true); - CowHeader header; CowOptions options; SNAP_LOG(DEBUG) << "ReadMetadata: Parsing cow file"; @@ -164,11 +162,7 @@ bool SnapshotHandler::ReadMetadata() { return false; } - if (!reader_->GetHeader(&header)) { - SNAP_LOG(ERROR) << "Failed to get header"; - return false; - } - + const auto& header = reader_->GetHeader(); if (!(header.block_size == BLOCK_SZ)) { SNAP_LOG(ERROR) << "Invalid header block size found: " << header.block_size; return false; @@ -244,8 +238,7 @@ bool SnapshotHandler::ReadMetadata() { } bool SnapshotHandler::MmapMetadata() { - CowHeader header; - reader_->GetHeader(&header); + const auto& header = reader_->GetHeader(); total_mapped_addr_length_ = header.header_size + BUFFER_REGION_DEFAULT_SIZE; @@ -367,8 +360,7 @@ bool SnapshotHandler::Start() { } uint64_t SnapshotHandler::GetBufferMetadataOffset() { - CowHeader header; - reader_->GetHeader(&header); + const auto& header = reader_->GetHeader(); return (header.header_size + sizeof(BufferState)); } @@ -383,8 +375,7 @@ uint64_t SnapshotHandler::GetBufferMetadataOffset() { * */ size_t SnapshotHandler::GetBufferMetadataSize() { - CowHeader header; - reader_->GetHeader(&header); + const auto& header = reader_->GetHeader(); size_t buffer_size = header.buffer_size; // If there is no scratch space, then just use the @@ -397,8 +388,7 @@ size_t SnapshotHandler::GetBufferMetadataSize() { } size_t SnapshotHandler::GetBufferDataOffset() { - CowHeader header; - reader_->GetHeader(&header); + const auto& header = reader_->GetHeader(); return (header.header_size + GetBufferMetadataSize()); } @@ -407,8 +397,7 @@ size_t SnapshotHandler::GetBufferDataOffset() { * (2MB - 8K = 2088960 bytes) will be the buffer region to hold the data. */ size_t SnapshotHandler::GetBufferDataSize() { - CowHeader header; - reader_->GetHeader(&header); + const auto& header = reader_->GetHeader(); size_t buffer_size = header.buffer_size; // If there is no scratch space, then just use the @@ -421,8 +410,7 @@ size_t SnapshotHandler::GetBufferDataSize() { } struct BufferState* SnapshotHandler::GetBufferState() { - CowHeader header; - reader_->GetHeader(&header); + const auto& header = reader_->GetHeader(); struct BufferState* ra_state = reinterpret_cast((char*)mapped_addr_ + header.header_size); From 82356c3f6a8d9b0975fbf8de653055951c0a2b52 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Wed, 26 Apr 2023 13:19:27 -0700 Subject: [PATCH 2/3] libsnapshot: Rename and clarify some methods in ICowOpIter. Done and RDone are not exactly symmetrical, so rename them to match their underlying STL behavior. Done becomes "AtEnd" and RDone becomes "AtBegin". Bug: 280529365 Test: builds Change-Id: I876f858a7bef84d8b37c1c822fb42941fbcded25 --- .../include/libsnapshot/cow_reader.h | 15 +- .../libsnapshot_cow/cow_api_test.cpp | 132 +++++++++--------- .../libsnapshot_cow/cow_reader.cpp | 44 +++--- .../libsnapshot_cow/cow_writer.cpp | 2 +- .../libsnapshot_cow/inspect_cow.cpp | 2 +- fs_mgr/libsnapshot/snapshot_reader.cpp | 2 +- .../snapuserd/dm-snapshot-merge/snapuserd.cpp | 10 +- .../user-space-merge/snapuserd_core.cpp | 2 +- .../user-space-merge/snapuserd_merge.cpp | 14 +- .../user-space-merge/snapuserd_readahead.cpp | 4 +- 10 files changed, 115 insertions(+), 112 deletions(-) diff --git a/fs_mgr/libsnapshot/include/libsnapshot/cow_reader.h b/fs_mgr/libsnapshot/include/libsnapshot/cow_reader.h index f433dd9e8..73429e1c6 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/cow_reader.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/cow_reader.h @@ -67,13 +67,14 @@ class ICowReader { size_t ignore_bytes = 0) = 0; }; -// Iterate over a sequence of COW operations. +// Iterate over a sequence of COW operations. The iterator is bidirectional. class ICowOpIter { public: virtual ~ICowOpIter() {} - // True if there are no more items to read forward, false otherwise. - virtual bool Done() = 0; + // Returns true if the iterator is at the end of the operation list. + // If true, Get() and Next() must not be called. + virtual bool AtEnd() = 0; // Read the current operation. virtual const CowOperation& Get() = 0; @@ -81,11 +82,13 @@ class ICowOpIter { // Advance to the next item. virtual void Next() = 0; + // Returns true if the iterator is at the beginning of the operation list. + // If true, Prev() must not be called; Get() however will be valid if + // AtEnd() is not true. + virtual bool AtBegin() = 0; + // Advance to the previous item. virtual void Prev() = 0; - - // True if there are no more items to read backwards, false otherwise - virtual bool RDone() = 0; }; class CowReader final : public ICowReader { diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/cow_api_test.cpp b/fs_mgr/libsnapshot/libsnapshot_cow/cow_api_test.cpp index ad8570952..7d1b7ba85 100644 --- a/fs_mgr/libsnapshot/libsnapshot_cow/cow_api_test.cpp +++ b/fs_mgr/libsnapshot/libsnapshot_cow/cow_api_test.cpp @@ -76,10 +76,10 @@ TEST_F(CowTest, CopyContiguous) { auto iter = reader.GetOpIter(); ASSERT_NE(iter, nullptr); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); size_t i = 0; - while (!iter->Done()) { + while (!iter->AtEnd()) { auto op = &iter->Get(); ASSERT_EQ(op->type, kCowCopyOp); ASSERT_EQ(op->compression, kCowCompressNone); @@ -125,7 +125,7 @@ TEST_F(CowTest, ReadWrite) { auto iter = reader.GetOpIter(); ASSERT_NE(iter, nullptr); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); auto op = &iter->Get(); ASSERT_EQ(op->type, kCowCopyOp); @@ -137,7 +137,7 @@ TEST_F(CowTest, ReadWrite) { std::string sink(data.size(), '\0'); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowReplaceOp); @@ -148,7 +148,7 @@ TEST_F(CowTest, ReadWrite) { ASSERT_EQ(sink, data); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); // Note: the zero operation gets split into two blocks. @@ -159,7 +159,7 @@ TEST_F(CowTest, ReadWrite) { ASSERT_EQ(op->source, 0); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowZeroOp); @@ -169,7 +169,7 @@ TEST_F(CowTest, ReadWrite) { ASSERT_EQ(op->source, 0); iter->Next(); - ASSERT_TRUE(iter->Done()); + ASSERT_TRUE(iter->AtEnd()); } TEST_F(CowTest, ReadWriteXor) { @@ -204,7 +204,7 @@ TEST_F(CowTest, ReadWriteXor) { auto iter = reader.GetOpIter(); ASSERT_NE(iter, nullptr); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); auto op = &iter->Get(); ASSERT_EQ(op->type, kCowCopyOp); @@ -216,7 +216,7 @@ TEST_F(CowTest, ReadWriteXor) { std::string sink(data.size(), '\0'); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowXorOp); @@ -228,7 +228,7 @@ TEST_F(CowTest, ReadWriteXor) { ASSERT_EQ(sink, data); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); // Note: the zero operation gets split into two blocks. @@ -239,7 +239,7 @@ TEST_F(CowTest, ReadWriteXor) { ASSERT_EQ(op->source, 0); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowZeroOp); @@ -249,7 +249,7 @@ TEST_F(CowTest, ReadWriteXor) { ASSERT_EQ(op->source, 0); iter->Next(); - ASSERT_TRUE(iter->Done()); + ASSERT_TRUE(iter->AtEnd()); } TEST_F(CowTest, CompressGz) { @@ -273,7 +273,7 @@ TEST_F(CowTest, CompressGz) { auto iter = reader.GetOpIter(); ASSERT_NE(iter, nullptr); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); auto op = &iter->Get(); std::string sink(data.size(), '\0'); @@ -286,7 +286,7 @@ TEST_F(CowTest, CompressGz) { ASSERT_EQ(sink, data); iter->Next(); - ASSERT_TRUE(iter->Done()); + ASSERT_TRUE(iter->AtEnd()); } class CompressionTest : public CowTest, public testing::WithParamInterface {}; @@ -328,7 +328,7 @@ TEST_P(CompressionTest, ThreadedBatchWrites) { ASSERT_NE(iter, nullptr); int total_blocks = 0; - while (!iter->Done()) { + while (!iter->AtEnd()) { auto op = &iter->Get(); if (op->type == kCowXorOp) { @@ -402,7 +402,7 @@ TEST_P(CompressionTest, NoBatchWrites) { ASSERT_NE(iter, nullptr); int total_blocks = 0; - while (!iter->Done()) { + while (!iter->AtEnd()) { auto op = &iter->Get(); if (op->type == kCowReplaceOp) { @@ -518,7 +518,7 @@ TEST_F(CowTest, ClusterCompressGz) { auto iter = reader.GetOpIter(); ASSERT_NE(iter, nullptr); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); auto op = &iter->Get(); std::string sink(data.size(), '\0'); @@ -531,13 +531,13 @@ TEST_F(CowTest, ClusterCompressGz) { ASSERT_EQ(sink, data); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowClusterOp); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); sink = {}; @@ -549,13 +549,13 @@ TEST_F(CowTest, ClusterCompressGz) { ASSERT_EQ(sink, data2); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowClusterOp); iter->Next(); - ASSERT_TRUE(iter->Done()); + ASSERT_TRUE(iter->AtEnd()); } TEST_F(CowTest, CompressTwoBlocks) { @@ -579,9 +579,9 @@ TEST_F(CowTest, CompressTwoBlocks) { auto iter = reader.GetOpIter(); ASSERT_NE(iter, nullptr); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); std::string sink(options.block_size, '\0'); @@ -658,7 +658,7 @@ TEST_F(CowTest, AppendLabelSmall) { auto iter = reader.GetOpIter(); ASSERT_NE(iter, nullptr); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); auto op = &iter->Get(); ASSERT_EQ(op->type, kCowReplaceOp); ASSERT_TRUE(ReadData(reader, *op, sink.data(), sink.size())); @@ -668,21 +668,21 @@ TEST_F(CowTest, AppendLabelSmall) { sink = {}; sink.resize(data2.size(), '\0'); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowLabelOp); ASSERT_EQ(op->source, 3); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowReplaceOp); ASSERT_TRUE(ReadData(reader, *op, sink.data(), sink.size())); ASSERT_EQ(sink, data2); iter->Next(); - ASSERT_TRUE(iter->Done()); + ASSERT_TRUE(iter->AtEnd()); } TEST_F(CowTest, AppendLabelMissing) { @@ -721,20 +721,20 @@ TEST_F(CowTest, AppendLabelMissing) { auto iter = reader.GetOpIter(); ASSERT_NE(iter, nullptr); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); auto op = &iter->Get(); ASSERT_EQ(op->type, kCowLabelOp); ASSERT_EQ(op->source, 0); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowZeroOp); iter->Next(); - ASSERT_TRUE(iter->Done()); + ASSERT_TRUE(iter->AtEnd()); } TEST_F(CowTest, AppendExtendedCorrupted) { @@ -779,13 +779,13 @@ TEST_F(CowTest, AppendExtendedCorrupted) { auto iter = reader.GetOpIter(); ASSERT_NE(iter, nullptr); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); auto op = &iter->Get(); ASSERT_EQ(op->type, kCowLabelOp); ASSERT_EQ(op->source, 5); iter->Next(); - ASSERT_TRUE(iter->Done()); + ASSERT_TRUE(iter->AtEnd()); } TEST_F(CowTest, AppendbyLabel) { @@ -830,7 +830,7 @@ TEST_F(CowTest, AppendbyLabel) { auto iter = reader.GetOpIter(); ASSERT_NE(iter, nullptr); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); auto op = &iter->Get(); ASSERT_EQ(op->type, kCowReplaceOp); ASSERT_TRUE(ReadData(reader, *op, sink.data(), sink.size())); @@ -840,7 +840,7 @@ TEST_F(CowTest, AppendbyLabel) { sink = {}; sink.resize(options.block_size, '\0'); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowReplaceOp); ASSERT_TRUE(ReadData(reader, *op, sink.data(), sink.size())); @@ -848,32 +848,32 @@ TEST_F(CowTest, AppendbyLabel) { iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowLabelOp); ASSERT_EQ(op->source, 4); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowZeroOp); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowZeroOp); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowLabelOp); ASSERT_EQ(op->source, 5); iter->Next(); - ASSERT_TRUE(iter->Done()); + ASSERT_TRUE(iter->AtEnd()); } TEST_F(CowTest, ClusterTest) { @@ -911,7 +911,7 @@ TEST_F(CowTest, ClusterTest) { auto iter = reader.GetOpIter(); ASSERT_NE(iter, nullptr); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); auto op = &iter->Get(); ASSERT_EQ(op->type, kCowReplaceOp); ASSERT_TRUE(ReadData(reader, *op, sink.data(), sink.size())); @@ -919,58 +919,58 @@ TEST_F(CowTest, ClusterTest) { iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowLabelOp); ASSERT_EQ(op->source, 4); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowZeroOp); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowClusterOp); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowZeroOp); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowLabelOp); ASSERT_EQ(op->source, 5); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowCopyOp); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowClusterOp); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowLabelOp); ASSERT_EQ(op->source, 6); iter->Next(); - ASSERT_TRUE(iter->Done()); + ASSERT_TRUE(iter->AtEnd()); } TEST_F(CowTest, ClusterAppendTest) { @@ -1010,14 +1010,14 @@ TEST_F(CowTest, ClusterAppendTest) { auto iter = reader.GetOpIter(); ASSERT_NE(iter, nullptr); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); auto op = &iter->Get(); ASSERT_EQ(op->type, kCowLabelOp); ASSERT_EQ(op->source, 50); iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowReplaceOp); ASSERT_TRUE(ReadData(reader, *op, sink.data(), sink.size())); @@ -1025,13 +1025,13 @@ TEST_F(CowTest, ClusterAppendTest) { iter->Next(); - ASSERT_FALSE(iter->Done()); + ASSERT_FALSE(iter->AtEnd()); op = &iter->Get(); ASSERT_EQ(op->type, kCowClusterOp); iter->Next(); - ASSERT_TRUE(iter->Done()); + ASSERT_TRUE(iter->AtEnd()); } TEST_F(CowTest, AppendAfterFinalize) { @@ -1118,7 +1118,7 @@ TEST_F(CowTest, ResumeMidCluster) { size_t max_in_cluster = 0; size_t num_in_cluster = 0; size_t num_clusters = 0; - while (!iter->Done()) { + while (!iter->AtEnd()) { const auto& op = iter->Get(); num_in_cluster++; @@ -1179,7 +1179,7 @@ TEST_F(CowTest, ResumeEndCluster) { size_t max_in_cluster = 0; size_t num_in_cluster = 0; size_t num_clusters = 0; - while (!iter->Done()) { + while (!iter->AtEnd()) { const auto& op = iter->Get(); num_in_cluster++; @@ -1231,7 +1231,7 @@ TEST_F(CowTest, DeleteMidCluster) { size_t max_in_cluster = 0; size_t num_in_cluster = 0; size_t num_clusters = 0; - while (!iter->Done()) { + while (!iter->AtEnd()) { const auto& op = iter->Get(); num_in_cluster++; @@ -1275,14 +1275,14 @@ TEST_F(CowTest, BigSeqOp) { auto iter = reader.GetRevMergeOpIter(); for (int i = 0; i < seq_len; i++) { - ASSERT_TRUE(!iter->Done()); + ASSERT_TRUE(!iter->AtEnd()); const auto& op = iter->Get(); ASSERT_EQ(op.new_block, seq_len - i); iter->Next(); } - ASSERT_TRUE(iter->Done()); + ASSERT_TRUE(iter->AtEnd()); } TEST_F(CowTest, MissingSeqOp) { @@ -1326,7 +1326,7 @@ TEST_F(CowTest, ResumeSeqOp) { auto reader = std::make_unique(); ASSERT_TRUE(reader->Parse(cow_->fd, 1)); auto itr = reader->GetRevMergeOpIter(); - ASSERT_TRUE(itr->Done()); + ASSERT_TRUE(itr->AtEnd()); writer = std::make_unique(options); ASSERT_TRUE(writer->InitializeAppend(cow_->fd, 1)); @@ -1341,8 +1341,8 @@ TEST_F(CowTest, ResumeSeqOp) { auto iter = reader->GetRevMergeOpIter(); uint64_t expected_block = 10; - while (!iter->Done() && expected_block > 0) { - ASSERT_FALSE(iter->Done()); + while (!iter->AtEnd() && expected_block > 0) { + ASSERT_FALSE(iter->AtEnd()); const auto& op = iter->Get(); ASSERT_EQ(op.new_block, expected_block); @@ -1351,7 +1351,7 @@ TEST_F(CowTest, ResumeSeqOp) { expected_block--; } ASSERT_EQ(expected_block, 0); - ASSERT_TRUE(iter->Done()); + ASSERT_TRUE(iter->AtEnd()); } TEST_F(CowTest, RevMergeOpItrTest) { @@ -1392,7 +1392,7 @@ TEST_F(CowTest, RevMergeOpItrTest) { auto iter = reader.GetRevMergeOpIter(); auto expected_new_block = revMergeOpSequence.begin(); - while (!iter->Done() && expected_new_block != revMergeOpSequence.end()) { + while (!iter->AtEnd() && expected_new_block != revMergeOpSequence.end()) { const auto& op = iter->Get(); ASSERT_EQ(op.new_block, *expected_new_block); @@ -1401,7 +1401,7 @@ TEST_F(CowTest, RevMergeOpItrTest) { expected_new_block++; } ASSERT_EQ(expected_new_block, revMergeOpSequence.end()); - ASSERT_TRUE(iter->Done()); + ASSERT_TRUE(iter->AtEnd()); } TEST_F(CowTest, LegacyRevMergeOpItrTest) { @@ -1441,7 +1441,7 @@ TEST_F(CowTest, LegacyRevMergeOpItrTest) { auto iter = reader.GetRevMergeOpIter(); auto expected_new_block = revMergeOpSequence.begin(); - while (!iter->Done() && expected_new_block != revMergeOpSequence.end()) { + while (!iter->AtEnd() && expected_new_block != revMergeOpSequence.end()) { const auto& op = iter->Get(); ASSERT_EQ(op.new_block, *expected_new_block); @@ -1450,7 +1450,7 @@ TEST_F(CowTest, LegacyRevMergeOpItrTest) { expected_new_block++; } ASSERT_EQ(expected_new_block, revMergeOpSequence.end()); - ASSERT_TRUE(iter->Done()); + ASSERT_TRUE(iter->AtEnd()); } TEST_F(CowTest, InvalidMergeOrderTest) { diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/cow_reader.cpp b/fs_mgr/libsnapshot/libsnapshot_cow/cow_reader.cpp index c388a9ce1..893d95fc7 100644 --- a/fs_mgr/libsnapshot/libsnapshot_cow/cow_reader.cpp +++ b/fs_mgr/libsnapshot/libsnapshot_cow/cow_reader.cpp @@ -509,7 +509,7 @@ bool CowReader::PrepMergeOps() { bool CowReader::VerifyMergeOps() { auto itr = GetMergeOpIter(true); std::unordered_map overwritten_blocks; - while (!itr->Done()) { + while (!itr->AtEnd()) { CowOperation op = itr->Get(); uint64_t block; bool offset; @@ -560,12 +560,12 @@ class CowOpIter final : public ICowOpIter { public: CowOpIter(std::shared_ptr>& ops, uint64_t start); - bool Done() override; + bool AtEnd() override; const CowOperation& Get() override; void Next() override; void Prev() override; - bool RDone() override; + bool AtBegin() override; private: std::shared_ptr> ops_; @@ -577,26 +577,26 @@ CowOpIter::CowOpIter(std::shared_ptr>& ops, uint64_t s op_iter_ = ops_->begin() + start; } -bool CowOpIter::RDone() { +bool CowOpIter::AtBegin() { return op_iter_ == ops_->begin(); } void CowOpIter::Prev() { - CHECK(!RDone()); + CHECK(!AtBegin()); op_iter_--; } -bool CowOpIter::Done() { +bool CowOpIter::AtEnd() { return op_iter_ == ops_->end(); } void CowOpIter::Next() { - CHECK(!Done()); + CHECK(!AtEnd()); op_iter_++; } const CowOperation& CowOpIter::Get() { - CHECK(!Done()); + CHECK(!AtEnd()); return (*op_iter_); } @@ -605,12 +605,12 @@ class CowRevMergeOpIter final : public ICowOpIter { explicit CowRevMergeOpIter(std::shared_ptr> ops, std::shared_ptr> block_pos_index, uint64_t start); - bool Done() override; + bool AtEnd() override; const CowOperation& Get() override; void Next() override; void Prev() override; - bool RDone() override; + bool AtBegin() override; private: std::shared_ptr> ops_; @@ -624,12 +624,12 @@ class CowMergeOpIter final : public ICowOpIter { explicit CowMergeOpIter(std::shared_ptr> ops, std::shared_ptr> block_pos_index, uint64_t start); - bool Done() override; + bool AtEnd() override; const CowOperation& Get() override; void Next() override; void Prev() override; - bool RDone() override; + bool AtBegin() override; private: std::shared_ptr> ops_; @@ -646,26 +646,26 @@ CowMergeOpIter::CowMergeOpIter(std::shared_ptr> ops, block_iter_ = cow_op_index_vec_->begin() + start; } -bool CowMergeOpIter::RDone() { +bool CowMergeOpIter::AtBegin() { return block_iter_ == cow_op_index_vec_->begin(); } void CowMergeOpIter::Prev() { - CHECK(!RDone()); + CHECK(!AtBegin()); block_iter_--; } -bool CowMergeOpIter::Done() { +bool CowMergeOpIter::AtEnd() { return block_iter_ == cow_op_index_vec_->end(); } void CowMergeOpIter::Next() { - CHECK(!Done()); + CHECK(!AtEnd()); block_iter_++; } const CowOperation& CowMergeOpIter::Get() { - CHECK(!Done()); + CHECK(!AtEnd()); return ops_->data()[*block_iter_]; } @@ -678,26 +678,26 @@ CowRevMergeOpIter::CowRevMergeOpIter(std::shared_ptr> block_riter_ = cow_op_index_vec_->rbegin(); } -bool CowRevMergeOpIter::RDone() { +bool CowRevMergeOpIter::AtBegin() { return block_riter_ == cow_op_index_vec_->rbegin(); } void CowRevMergeOpIter::Prev() { - CHECK(!RDone()); + CHECK(!AtBegin()); block_riter_--; } -bool CowRevMergeOpIter::Done() { +bool CowRevMergeOpIter::AtEnd() { return block_riter_ == cow_op_index_vec_->rend() - start_; } void CowRevMergeOpIter::Next() { - CHECK(!Done()); + CHECK(!AtEnd()); block_riter_++; } const CowOperation& CowRevMergeOpIter::Get() { - CHECK(!Done()); + CHECK(!AtEnd()); return ops_->data()[*block_riter_]; } diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/cow_writer.cpp b/fs_mgr/libsnapshot/libsnapshot_cow/cow_writer.cpp index a996b57c9..cb20c6f51 100644 --- a/fs_mgr/libsnapshot/libsnapshot_cow/cow_writer.cpp +++ b/fs_mgr/libsnapshot/libsnapshot_cow/cow_writer.cpp @@ -406,7 +406,7 @@ bool CowWriter::OpenForAppend(uint64_t label) { auto iter = reader->GetOpIter(); - while (!iter->Done()) { + while (!iter->AtEnd()) { AddOperation(iter->Get()); iter->Next(); } diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/inspect_cow.cpp b/fs_mgr/libsnapshot/libsnapshot_cow/inspect_cow.cpp index f6b323ab8..3e8d3583b 100644 --- a/fs_mgr/libsnapshot/libsnapshot_cow/inspect_cow.cpp +++ b/fs_mgr/libsnapshot/libsnapshot_cow/inspect_cow.cpp @@ -136,7 +136,7 @@ static bool Inspect(const std::string& path, Options opt) { bool success = true; uint64_t xor_ops = 0, copy_ops = 0, replace_ops = 0, zero_ops = 0; - while (!iter->Done()) { + while (!iter->AtEnd()) { const CowOperation& op = iter->Get(); if (!opt.silent && opt.show_ops) std::cout << op << "\n"; diff --git a/fs_mgr/libsnapshot/snapshot_reader.cpp b/fs_mgr/libsnapshot/snapshot_reader.cpp index 6111edd93..48efae061 100644 --- a/fs_mgr/libsnapshot/snapshot_reader.cpp +++ b/fs_mgr/libsnapshot/snapshot_reader.cpp @@ -85,7 +85,7 @@ bool CompressedSnapshotReader::SetCow(std::unique_ptr&& cow) { // Populate the operation map. op_iter_ = cow_->GetOpIter(); - while (!op_iter_->Done()) { + while (!op_iter_->AtEnd()) { const CowOperation* op = &op_iter_->Get(); if (IsMetadataOp(*op)) { op_iter_->Next(); diff --git a/fs_mgr/libsnapshot/snapuserd/dm-snapshot-merge/snapuserd.cpp b/fs_mgr/libsnapshot/snapuserd/dm-snapshot-merge/snapuserd.cpp index c3f6870af..b6e00ea45 100644 --- a/fs_mgr/libsnapshot/snapuserd/dm-snapshot-merge/snapuserd.cpp +++ b/fs_mgr/libsnapshot/snapuserd/dm-snapshot-merge/snapuserd.cpp @@ -390,7 +390,7 @@ bool Snapuserd::ReadMetadata() { // this memset will ensure that metadata read is completed. memset(de_ptr.get(), 0, (exceptions_per_area_ * sizeof(struct disk_exception))); - while (!cowop_rm_iter->Done()) { + while (!cowop_rm_iter->AtEnd()) { const CowOperation* cow_op = &cowop_rm_iter->Get(); struct disk_exception* de = reinterpret_cast((char*)de_ptr.get() + offset); @@ -437,7 +437,7 @@ bool Snapuserd::ReadMetadata() { sizeof(struct disk_exception)); memset(de_ptr.get(), 0, (exceptions_per_area_ * sizeof(struct disk_exception))); - if (cowop_rm_iter->Done()) { + if (cowop_rm_iter->AtEnd()) { vec_.push_back(std::move(de_ptr)); } } @@ -457,7 +457,7 @@ bool Snapuserd::ReadMetadata() { << " Number of replace/zero ops completed in this area: " << num_ops << " Pending copy ops for this area: " << pending_ordered_ops; - while (!cowop_rm_iter->Done()) { + while (!cowop_rm_iter->AtEnd()) { do { const CowOperation* cow_op = &cowop_rm_iter->Get(); @@ -526,7 +526,7 @@ bool Snapuserd::ReadMetadata() { source_blocks.insert(cow_op->new_block); prev_id = cow_op->new_block; cowop_rm_iter->Next(); - } while (!cowop_rm_iter->Done() && pending_ordered_ops); + } while (!cowop_rm_iter->AtEnd() && pending_ordered_ops); data_chunk_id = GetNextAllocatableChunkId(data_chunk_id); SNAP_LOG(DEBUG) << "Batch Merge copy-ops of size: " << vec.size() @@ -569,7 +569,7 @@ bool Snapuserd::ReadMetadata() { sizeof(struct disk_exception)); memset(de_ptr.get(), 0, (exceptions_per_area_ * sizeof(struct disk_exception))); - if (cowop_rm_iter->Done()) { + if (cowop_rm_iter->AtEnd()) { vec_.push_back(std::move(de_ptr)); SNAP_LOG(DEBUG) << "ReadMetadata() completed; Number of Areas: " << vec_.size(); } diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp index 1e4960020..9df8cf989 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp @@ -185,7 +185,7 @@ bool SnapshotHandler::ReadMetadata() { size_t copy_ops = 0, replace_ops = 0, zero_ops = 0, xor_ops = 0; - while (!cowop_iter->Done()) { + while (!cowop_iter->AtEnd()) { const CowOperation* cow_op = &cowop_iter->Get(); if (cow_op->type == kCowCopyOp) { diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_merge.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_merge.cpp index d57f434be..4d91ff313 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_merge.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_merge.cpp @@ -30,7 +30,7 @@ int Worker::PrepareMerge(uint64_t* source_offset, int* pending_ops, bool checkOrderedOp = (replace_zero_vec == nullptr); do { - if (!cowop_iter_->Done() && num_ops) { + if (!cowop_iter_->AtEnd() && num_ops) { const CowOperation* cow_op = &cowop_iter_->Get(); if (checkOrderedOp && !IsOrderedOp(*cow_op)) { break; @@ -45,7 +45,7 @@ int Worker::PrepareMerge(uint64_t* source_offset, int* pending_ops, num_ops -= 1; nr_consecutive = 1; - while (!cowop_iter_->Done() && num_ops) { + while (!cowop_iter_->AtEnd() && num_ops) { const CowOperation* op = &cowop_iter_->Get(); if (checkOrderedOp && !IsOrderedOp(*op)) { break; @@ -85,7 +85,7 @@ bool Worker::MergeReplaceZeroOps() { SNAP_LOG(INFO) << "MergeReplaceZeroOps started...."; - while (!cowop_iter_->Done()) { + while (!cowop_iter_->AtEnd()) { int num_ops = PAYLOAD_BUFFER_SZ / BLOCK_SZ; std::vector replace_zero_vec; uint64_t source_offset; @@ -93,7 +93,7 @@ bool Worker::MergeReplaceZeroOps() { int linear_blocks = PrepareMerge(&source_offset, &num_ops, &replace_zero_vec); if (linear_blocks == 0) { // Merge complete - CHECK(cowop_iter_->Done()); + CHECK(cowop_iter_->AtEnd()); break; } @@ -180,7 +180,7 @@ bool Worker::MergeOrderedOpsAsync() { SNAP_LOG(INFO) << "MergeOrderedOpsAsync started...."; - while (!cowop_iter_->Done()) { + while (!cowop_iter_->AtEnd()) { const CowOperation* cow_op = &cowop_iter_->Get(); if (!IsOrderedOp(*cow_op)) { break; @@ -361,7 +361,7 @@ bool Worker::MergeOrderedOps() { SNAP_LOG(INFO) << "MergeOrderedOps started...."; - while (!cowop_iter_->Done()) { + while (!cowop_iter_->AtEnd()) { const CowOperation* cow_op = &cowop_iter_->Get(); if (!IsOrderedOp(*cow_op)) { break; @@ -443,7 +443,7 @@ bool Worker::AsyncMerge() { if (!MergeOrderedOpsAsync()) { SNAP_LOG(ERROR) << "MergeOrderedOpsAsync failed - Falling back to synchronous I/O"; // Reset the iter so that we retry the merge - while (blocks_merged_in_group_ && !cowop_iter_->RDone()) { + while (blocks_merged_in_group_ && !cowop_iter_->AtBegin()) { cowop_iter_->Prev(); blocks_merged_in_group_ -= 1; } diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_readahead.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_readahead.cpp index b175fe8bc..4aad7a66b 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_readahead.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_readahead.cpp @@ -809,7 +809,7 @@ void ReadAhead::InitializeRAIter() { } bool ReadAhead::RAIterDone() { - if (cowop_iter_->Done()) { + if (cowop_iter_->AtEnd()) { return true; } @@ -827,7 +827,7 @@ void ReadAhead::RAIterNext() { } void ReadAhead::RAResetIter(uint64_t num_blocks) { - while (num_blocks && !cowop_iter_->RDone()) { + while (num_blocks && !cowop_iter_->AtBegin()) { cowop_iter_->Prev(); num_blocks -= 1; } From de20e57f08ece2d8c6ce5f289ed1790cdd81efaa Mon Sep 17 00:00:00 2001 From: David Anderson Date: Thu, 27 Apr 2023 12:13:36 -0700 Subject: [PATCH 3/3] libsnapshot: Improve inspect_cow output. Clean up the formatting and unit display, and add a line displaying how long it took to parse the COW. Example of new output: Version: 2.0 Header size: 38 Footer size: 84 Block size: 4096 Merge ops: 0 Readahead buffer: 2097152 bytes Footer: ops usage: 1005680 bytes Footer: op count: 50284 Parse time: 6.77921ms Data ops: 50015 Replace ops: 50015 Zero ops: 0 Copy ops: 0 Xor ops: 0 Bug: 280529365 Test: inspect_cow /dev/block/mapper/product_b-cow Change-Id: Ie8ac02adc004289cc3a08ef44aa3048280a2407f --- .../libsnapshot_cow/inspect_cow.cpp | 56 +++++++++++-------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/inspect_cow.cpp b/fs_mgr/libsnapshot/libsnapshot_cow/inspect_cow.cpp index 3e8d3583b..40901621a 100644 --- a/fs_mgr/libsnapshot/libsnapshot_cow/inspect_cow.cpp +++ b/fs_mgr/libsnapshot/libsnapshot_cow/inspect_cow.cpp @@ -16,6 +16,7 @@ #include #include +#include #include #include #include @@ -38,16 +39,16 @@ void MyLogger(android::base::LogId, android::base::LogSeverity severity, const c } static void usage(void) { - LOG(ERROR) << "Usage: inspect_cow [-sd] "; - LOG(ERROR) << "\t -s Run Silent"; - LOG(ERROR) << "\t -d Attempt to decompress"; - LOG(ERROR) << "\t -b Show data for failed decompress"; - LOG(ERROR) << "\t -l Show ops"; - LOG(ERROR) << "\t -m Show ops in reverse merge order"; - LOG(ERROR) << "\t -n Show ops in merge order"; - LOG(ERROR) << "\t -a Include merged ops in any merge order listing"; - LOG(ERROR) << "\t -o Shows sequence op block order"; - LOG(ERROR) << "\t -v Verifies merge order has no conflicts\n"; + std::cerr << "Usage: inspect_cow [-sd] \n"; + std::cerr << "\t -s Run Silent\n"; + std::cerr << "\t -d Attempt to decompress\n"; + std::cerr << "\t -b Show data for failed decompress\n"; + std::cerr << "\t -l Show ops\n"; + std::cerr << "\t -m Show ops in reverse merge order\n"; + std::cerr << "\t -n Show ops in merge order\n"; + std::cerr << "\t -a Include merged ops in any merge order listing\n"; + std::cerr << "\t -o Shows sequence op block order\n"; + std::cerr << "\t -v Verifies merge order has no conflicts\n"; } enum OpIter { Normal, RevMerge, Merge }; @@ -89,10 +90,13 @@ static bool Inspect(const std::string& path, Options opt) { } CowReader reader; + + auto start_time = std::chrono::steady_clock::now(); if (!reader.Parse(fd)) { LOG(ERROR) << "parse failed: " << path; return false; } + std::chrono::duration parse_time = std::chrono::steady_clock::now() - start_time; const CowHeader& header = reader.GetHeader(); CowFooter footer; @@ -100,22 +104,26 @@ static bool Inspect(const std::string& path, Options opt) { if (reader.GetFooter(&footer)) has_footer = true; if (!opt.silent) { - std::cout << "Major version: " << header.major_version << "\n"; - std::cout << "Minor version: " << header.minor_version << "\n"; + std::cout << "Version: " << header.major_version << "." << header.minor_version << "\n"; std::cout << "Header size: " << header.header_size << "\n"; std::cout << "Footer size: " << header.footer_size << "\n"; std::cout << "Block size: " << header.block_size << "\n"; - std::cout << "Num merge ops: " << header.num_merge_ops << "\n"; - std::cout << "RA buffer size: " << header.buffer_size << "\n"; - std::cout << "\n"; + std::cout << "Merge ops: " << header.num_merge_ops << "\n"; + std::cout << "Readahead buffer: " << header.buffer_size << " bytes\n"; if (has_footer) { - std::cout << "Total Ops size: " << footer.op.ops_size << "\n"; - std::cout << "Number of Ops: " << footer.op.num_ops << "\n"; - std::cout << "\n"; + std::cout << "Footer: ops usage: " << footer.op.ops_size << " bytes\n"; + std::cout << "Footer: op count: " << footer.op.num_ops << "\n"; + } else { + std::cout << "Footer: none\n"; } } + if (!opt.silent) { + std::cout << "Parse time: " << (parse_time.count() * 1000) << "ms\n"; + } + if (opt.verify_sequence) { + std::cout << "\n"; if (reader.VerifyMergeOps()) { std::cout << "\nMerge sequence is consistent.\n"; } else { @@ -182,9 +190,11 @@ static bool Inspect(const std::string& path, Options opt) { if (!opt.silent) { auto total_ops = replace_ops + zero_ops + copy_ops + xor_ops; - std::cout << "Total-data-ops: " << total_ops << "Replace-ops: " << replace_ops - << " Zero-ops: " << zero_ops << " Copy-ops: " << copy_ops - << " Xor_ops: " << xor_ops << std::endl; + std::cout << "Data ops: " << total_ops << "\n"; + std::cout << "Replace ops: " << replace_ops << "\n"; + std::cout << "Zero ops: " << zero_ops << "\n"; + std::cout << "Copy ops: " << copy_ops << "\n"; + std::cout << "Xor ops: " << xor_ops << "\n"; } return success; @@ -233,15 +243,17 @@ int main(int argc, char** argv) { break; default: android::snapshot::usage(); + return 1; } } - android::base::InitLogging(argv, android::snapshot::MyLogger); if (argc < optind + 1) { android::snapshot::usage(); return 1; } + android::base::InitLogging(argv, android::snapshot::MyLogger); + if (!android::snapshot::Inspect(argv[optind], opt)) { return 1; }