libsnapshot: Only sync after labels
This changes labels to belong at the end of the set of ops that they
refer to. We only sync after writing a label, or the footer, saving the
cost of syncing after ever op.
Change-Id: Iee9dd69132b8e3321eccfe1e43fa0c072a94d3bd
Bug: 172026020
Test: cow_api_test
diff --git a/fs_mgr/libsnapshot/cow_api_test.cpp b/fs_mgr/libsnapshot/cow_api_test.cpp
index 1408244..76d69ac 100644
--- a/fs_mgr/libsnapshot/cow_api_test.cpp
+++ b/fs_mgr/libsnapshot/cow_api_test.cpp
@@ -334,9 +334,8 @@
std::string data = "This is some data, believe it";
data.resize(options.block_size, '\0');
- ASSERT_TRUE(writer->AddLabel(0));
ASSERT_TRUE(writer->AddRawBlocks(50, data.data(), data.size()));
- ASSERT_TRUE(writer->AddLabel(1));
+ ASSERT_TRUE(writer->AddLabel(0));
ASSERT_TRUE(writer->AddZeroBlocks(50, 1));
ASSERT_TRUE(writer->Finalize());
// Drop the tail end of the header. Last entry may be corrupted.
@@ -347,13 +346,14 @@
writer = std::make_unique<CowWriter>(options);
ASSERT_TRUE(writer->Initialize(cow_->fd, CowWriter::OpenMode::APPEND));
- ASSERT_TRUE(writer->AddLabel(2));
- ASSERT_TRUE(writer->AddZeroBlocks(50, 1));
+ ASSERT_TRUE(writer->AddZeroBlocks(51, 1));
+ ASSERT_TRUE(writer->AddLabel(1));
std::string data2 = "More data!";
data2.resize(options.block_size, '\0');
- ASSERT_TRUE(writer->AddLabel(3));
- ASSERT_TRUE(writer->AddRawBlocks(51, data2.data(), data2.size()));
+ ASSERT_TRUE(writer->AddRawBlocks(52, data2.data(), data2.size()));
+ ASSERT_TRUE(writer->AddLabel(2));
+
ASSERT_TRUE(writer->Finalize());
ASSERT_EQ(lseek(cow_->fd, 0, SEEK_SET), 0);
@@ -373,13 +373,6 @@
ASSERT_FALSE(iter->Done());
auto op = &iter->Get();
- ASSERT_EQ(op->type, kCowLabelOp);
- ASSERT_EQ(op->source, 0);
-
- iter->Next();
-
- ASSERT_FALSE(iter->Done());
- op = &iter->Get();
ASSERT_EQ(op->type, kCowReplaceOp);
ASSERT_TRUE(reader.ReadData(*op, &sink));
ASSERT_EQ(sink.stream(), data);
@@ -390,7 +383,7 @@
ASSERT_FALSE(iter->Done());
op = &iter->Get();
ASSERT_EQ(op->type, kCowLabelOp);
- ASSERT_EQ(op->source, 2);
+ ASSERT_EQ(op->source, 0);
iter->Next();
@@ -403,7 +396,7 @@
ASSERT_FALSE(iter->Done());
op = &iter->Get();
ASSERT_EQ(op->type, kCowLabelOp);
- ASSERT_EQ(op->source, 3);
+ ASSERT_EQ(op->source, 1);
iter->Next();
@@ -414,6 +407,13 @@
ASSERT_EQ(sink.stream(), data2);
iter->Next();
+
+ ASSERT_FALSE(iter->Done());
+ op = &iter->Get();
+ ASSERT_EQ(op->type, kCowLabelOp);
+ ASSERT_EQ(op->source, 2);
+ iter->Next();
+
ASSERT_TRUE(iter->Done());
}
@@ -423,11 +423,11 @@
ASSERT_TRUE(writer->Initialize(cow_->fd));
ASSERT_TRUE(writer->AddLabel(5));
- ASSERT_TRUE(writer->AddLabel(6));
std::string data = "This is some data, believe it";
data.resize(options.block_size * 2, '\0');
ASSERT_TRUE(writer->AddRawBlocks(50, data.data(), data.size()));
+ ASSERT_TRUE(writer->AddLabel(6));
// fail to write the footer
@@ -451,7 +451,7 @@
ASSERT_EQ(fstat(cow_->fd, &buf), 0);
ASSERT_EQ(buf.st_size, writer->GetCowSize());
- // Read back all three operations.
+ // Read back all valid operations
CowReader reader;
ASSERT_TRUE(reader.Parse(cow_->fd));
@@ -474,16 +474,20 @@
auto writer = std::make_unique<CowWriter>(options);
ASSERT_TRUE(writer->Initialize(cow_->fd));
- ASSERT_TRUE(writer->AddLabel(4));
-
- ASSERT_TRUE(writer->AddLabel(5));
std::string data = "This is some data, believe it";
data.resize(options.block_size * 2, '\0');
ASSERT_TRUE(writer->AddRawBlocks(50, data.data(), data.size()));
- ASSERT_TRUE(writer->AddLabel(6));
+ ASSERT_TRUE(writer->AddLabel(4));
+
ASSERT_TRUE(writer->AddZeroBlocks(50, 2));
+ ASSERT_TRUE(writer->AddLabel(5));
+
+ ASSERT_TRUE(writer->AddCopy(5, 6));
+
+ ASSERT_TRUE(writer->AddLabel(6));
+
ASSERT_EQ(lseek(cow_->fd, 0, SEEK_SET), 0);
writer = std::make_unique<CowWriter>(options);
@@ -508,20 +512,6 @@
ASSERT_FALSE(iter->Done());
auto op = &iter->Get();
- ASSERT_EQ(op->type, kCowLabelOp);
- ASSERT_EQ(op->source, 4);
-
- iter->Next();
-
- ASSERT_FALSE(iter->Done());
- op = &iter->Get();
- ASSERT_EQ(op->type, kCowLabelOp);
- ASSERT_EQ(op->source, 5);
-
- iter->Next();
-
- ASSERT_FALSE(iter->Done());
- op = &iter->Get();
ASSERT_EQ(op->type, kCowReplaceOp);
ASSERT_TRUE(reader.ReadData(*op, &sink));
ASSERT_EQ(sink.stream(), data.substr(0, options.block_size));
@@ -538,6 +528,31 @@
iter->Next();
sink.Reset();
+ ASSERT_FALSE(iter->Done());
+ op = &iter->Get();
+ ASSERT_EQ(op->type, kCowLabelOp);
+ ASSERT_EQ(op->source, 4);
+
+ iter->Next();
+
+ ASSERT_FALSE(iter->Done());
+ op = &iter->Get();
+ ASSERT_EQ(op->type, kCowZeroOp);
+
+ iter->Next();
+
+ ASSERT_FALSE(iter->Done());
+ op = &iter->Get();
+ ASSERT_EQ(op->type, kCowZeroOp);
+
+ iter->Next();
+ ASSERT_FALSE(iter->Done());
+ op = &iter->Get();
+ ASSERT_EQ(op->type, kCowLabelOp);
+ ASSERT_EQ(op->source, 5);
+
+ iter->Next();
+
ASSERT_TRUE(iter->Done());
}
diff --git a/fs_mgr/libsnapshot/cow_reader.cpp b/fs_mgr/libsnapshot/cow_reader.cpp
index 452a5f3..517cd9c 100644
--- a/fs_mgr/libsnapshot/cow_reader.cpp
+++ b/fs_mgr/libsnapshot/cow_reader.cpp
@@ -140,16 +140,17 @@
return false;
}
current_op_num++;
+ if (next_last_label) {
+ last_label_ = next_last_label.value();
+ has_last_label_ = true;
+ }
if (current_op.type == kCowLabelOp) {
- // If we don't have a footer, the last label may be incomplete
+ // If we don't have a footer, the last label may be incomplete.
+ // If we see any operation after it, we can infer the flush finished.
if (has_footer_) {
has_last_label_ = true;
last_label_ = current_op.source;
} else {
- if (next_last_label) {
- last_label_ = next_last_label.value();
- has_last_label_ = true;
- }
next_last_label = {current_op.source};
}
} else if (current_op.type == kCowFooterOp) {
diff --git a/fs_mgr/libsnapshot/cow_writer.cpp b/fs_mgr/libsnapshot/cow_writer.cpp
index b3e75a0..8704eb9 100644
--- a/fs_mgr/libsnapshot/cow_writer.cpp
+++ b/fs_mgr/libsnapshot/cow_writer.cpp
@@ -171,10 +171,13 @@
return true;
}
-bool CowWriter::OpenForAppend() {
+bool CowWriter::OpenForAppend(std::optional<uint64_t> label) {
auto reader = std::make_unique<CowReader>();
bool incomplete = false;
+ bool add_next = false;
std::queue<CowOperation> toAdd;
+ bool found_label = false;
+
if (!reader->Parse(fd_) || !reader->GetHeader(&header_)) {
return false;
}
@@ -188,67 +191,37 @@
ops_.resize(0);
auto iter = reader->GetOpIter();
- while (!iter->Done()) {
+ while (!iter->Done() && !found_label) {
CowOperation op = iter->Get();
if (op.type == kCowFooterOp) break;
- if (incomplete) {
- // Last operation translation may be corrupt. Wait to add it.
+ if (label.has_value()) {
+ if (op.type == kCowFooterOp) break;
if (op.type == kCowLabelOp) {
- while (!toAdd.empty()) {
- AddOperation(toAdd.front());
- toAdd.pop();
- }
+ if (op.source == label) found_label = true;
}
- toAdd.push(op);
- } else {
AddOperation(op);
+ } else {
+ if (incomplete) {
+ // Last set of labeled operations may be corrupt. Wait to add it.
+ // We always sync after a label. If we see ops after a label, we
+ // can infer that sync must have completed.
+ if (add_next) {
+ add_next = false;
+ while (!toAdd.empty()) {
+ AddOperation(toAdd.front());
+ toAdd.pop();
+ }
+ }
+ toAdd.push(op);
+ if (op.type == kCowLabelOp) add_next = true;
+ } else {
+ AddOperation(op);
+ }
}
iter->Next();
}
- // Free reader so we own the descriptor position again.
- reader = nullptr;
-
- // Position for new writing
- if (ftruncate(fd_.get(), next_op_pos_) != 0) {
- PLOG(ERROR) << "Failed to trim file";
- return false;
- }
- if (lseek(fd_.get(), 0, SEEK_END) < 0) {
- PLOG(ERROR) << "lseek failed";
- return false;
- }
- return true;
-}
-
-bool CowWriter::OpenForAppend(uint64_t label) {
- auto reader = std::make_unique<CowReader>();
- std::queue<CowOperation> toAdd;
- if (!reader->Parse(fd_) || !reader->GetHeader(&header_)) {
- return false;
- }
-
- options_.block_size = header_.block_size;
- bool found_label = false;
-
- // Reset this, since we're going to reimport all operations.
- footer_.op.num_ops = 0;
- next_op_pos_ = sizeof(header_);
- ops_.resize(0);
-
- auto iter = reader->GetOpIter();
- while (!iter->Done()) {
- CowOperation op = iter->Get();
- if (op.type == kCowFooterOp) break;
- if (op.type == kCowLabelOp) {
- if (found_label) break;
- if (op.source == label) found_label = true;
- }
- AddOperation(op);
- iter->Next();
- }
-
- if (!found_label) {
+ if (label.has_value() && !found_label) {
LOG(ERROR) << "Failed to find last label";
return false;
}
@@ -331,7 +304,7 @@
CowOperation op = {};
op.type = kCowLabelOp;
op.source = label;
- return WriteOperation(op);
+ return WriteOperation(op) && !fsync(fd_.get());
}
std::basic_string<uint8_t> CowWriter::Compress(const void* data, size_t length) {
@@ -410,7 +383,7 @@
PLOG(ERROR) << "lseek ops failed";
return false;
}
- return true;
+ return !fsync(fd_.get());
}
uint64_t CowWriter::GetCowSize() {
@@ -431,10 +404,11 @@
if (!android::base::WriteFully(fd_, reinterpret_cast<const uint8_t*>(&op), sizeof(op))) {
return false;
}
- if (data != NULL && size > 0)
+ if (data != nullptr && size > 0) {
if (!WriteRawData(data, size)) return false;
+ }
AddOperation(op);
- return !fsync(fd_.get());
+ return true;
}
void CowWriter::AddOperation(const CowOperation& op) {
diff --git a/fs_mgr/libsnapshot/include/libsnapshot/cow_writer.h b/fs_mgr/libsnapshot/include/libsnapshot/cow_writer.h
index 0f47622..50ce5bc 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/cow_writer.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/cow_writer.h
@@ -112,8 +112,7 @@
void SetupHeaders();
bool ParseOptions();
bool OpenForWrite();
- bool OpenForAppend();
- bool OpenForAppend(uint64_t label);
+ bool OpenForAppend(std::optional<uint64_t> label = std::nullopt);
bool GetDataPos(uint64_t* pos);
bool WriteRawData(const void* data, size_t size);
bool WriteOperation(const CowOperation& op, const void* data = nullptr, size_t size = 0);