Merge "libsnapshot:snapuserd: Fix cow_snapuserd_test."
diff --git a/fs_mgr/libdm/dm.cpp b/fs_mgr/libdm/dm.cpp
index bb7d8b3..d5b8a61 100644
--- a/fs_mgr/libdm/dm.cpp
+++ b/fs_mgr/libdm/dm.cpp
@@ -111,8 +111,10 @@
// Check to make sure appropriate uevent is generated so ueventd will
// do the right thing and remove the corresponding device node and symlinks.
- CHECK(io.flags & DM_UEVENT_GENERATED_FLAG)
- << "Didn't generate uevent for [" << name << "] removal";
+ if ((io.flags & DM_UEVENT_GENERATED_FLAG) == 0) {
+ LOG(ERROR) << "Didn't generate uevent for [" << name << "] removal";
+ return false;
+ }
if (timeout_ms <= std::chrono::milliseconds::zero()) {
return true;
diff --git a/fs_mgr/libsnapshot/cow_api_test.cpp b/fs_mgr/libsnapshot/cow_api_test.cpp
index 76d69ac..9a44020 100644
--- a/fs_mgr/libsnapshot/cow_api_test.cpp
+++ b/fs_mgr/libsnapshot/cow_api_test.cpp
@@ -271,7 +271,7 @@
ASSERT_EQ(buf.st_size, writer.GetCowSize());
}
-TEST_F(CowTest, Append) {
+TEST_F(CowTest, AppendLabelSmall) {
CowOptions options;
auto writer = std::make_unique<CowWriter>(options);
ASSERT_TRUE(writer->Initialize(cow_->fd));
@@ -279,12 +279,13 @@
std::string data = "This is some data, believe it";
data.resize(options.block_size, '\0');
ASSERT_TRUE(writer->AddRawBlocks(50, data.data(), data.size()));
+ ASSERT_TRUE(writer->AddLabel(3));
ASSERT_TRUE(writer->Finalize());
ASSERT_EQ(lseek(cow_->fd, 0, SEEK_SET), 0);
writer = std::make_unique<CowWriter>(options);
- ASSERT_TRUE(writer->Initialize(cow_->fd, CowWriter::OpenMode::APPEND));
+ ASSERT_TRUE(writer->InitializeAppend(cow_->fd, 3));
std::string data2 = "More data!";
data2.resize(options.block_size, '\0');
@@ -297,11 +298,12 @@
ASSERT_EQ(fstat(cow_->fd, &buf), 0);
ASSERT_EQ(buf.st_size, writer->GetCowSize());
- // Read back both operations.
+ // Read back both operations, and label.
CowReader reader;
uint64_t label;
ASSERT_TRUE(reader.Parse(cow_->fd));
- ASSERT_FALSE(reader.GetLastLabel(&label));
+ ASSERT_TRUE(reader.GetLastLabel(&label));
+ ASSERT_EQ(label, 3);
StringSink sink;
@@ -319,6 +321,13 @@
ASSERT_FALSE(iter->Done());
op = &iter->Get();
+ ASSERT_EQ(op->type, kCowLabelOp);
+ ASSERT_EQ(op->source, 3);
+
+ iter->Next();
+
+ ASSERT_FALSE(iter->Done());
+ op = &iter->Get();
ASSERT_EQ(op->type, kCowReplaceOp);
ASSERT_TRUE(reader.ReadData(*op, &sink));
ASSERT_EQ(sink.stream(), data2);
@@ -327,33 +336,26 @@
ASSERT_TRUE(iter->Done());
}
-TEST_F(CowTest, AppendCorrupted) {
+TEST_F(CowTest, AppendLabelMissing) {
CowOptions options;
auto writer = std::make_unique<CowWriter>(options);
ASSERT_TRUE(writer->Initialize(cow_->fd));
+ ASSERT_TRUE(writer->AddLabel(0));
std::string data = "This is some data, believe it";
data.resize(options.block_size, '\0');
ASSERT_TRUE(writer->AddRawBlocks(50, data.data(), data.size()));
- 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.
- ftruncate(cow_->fd, writer->GetCowSize() - 5);
+ ASSERT_TRUE(writer->AddLabel(1));
+ // Drop the tail end of the last op header, corrupting it.
+ ftruncate(cow_->fd, writer->GetCowSize() - sizeof(CowFooter) - 3);
ASSERT_EQ(lseek(cow_->fd, 0, SEEK_SET), 0);
writer = std::make_unique<CowWriter>(options);
- ASSERT_TRUE(writer->Initialize(cow_->fd, CowWriter::OpenMode::APPEND));
+ ASSERT_FALSE(writer->InitializeAppend(cow_->fd, 1));
+ ASSERT_TRUE(writer->InitializeAppend(cow_->fd, 0));
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->AddRawBlocks(52, data2.data(), data2.size()));
- ASSERT_TRUE(writer->AddLabel(2));
-
ASSERT_TRUE(writer->Finalize());
ASSERT_EQ(lseek(cow_->fd, 0, SEEK_SET), 0);
@@ -362,7 +364,7 @@
ASSERT_EQ(fstat(cow_->fd, &buf), 0);
ASSERT_EQ(buf.st_size, writer->GetCowSize());
- // Read back all three operations.
+ // Read back both operations.
CowReader reader;
ASSERT_TRUE(reader.Parse(cow_->fd));
@@ -373,15 +375,6 @@
ASSERT_FALSE(iter->Done());
auto op = &iter->Get();
- ASSERT_EQ(op->type, kCowReplaceOp);
- ASSERT_TRUE(reader.ReadData(*op, &sink));
- ASSERT_EQ(sink.stream(), data);
-
- iter->Next();
- sink.Reset();
-
- ASSERT_FALSE(iter->Done());
- op = &iter->Get();
ASSERT_EQ(op->type, kCowLabelOp);
ASSERT_EQ(op->source, 0);
@@ -393,27 +386,6 @@
iter->Next();
- ASSERT_FALSE(iter->Done());
- op = &iter->Get();
- ASSERT_EQ(op->type, kCowLabelOp);
- ASSERT_EQ(op->source, 1);
-
- iter->Next();
-
- ASSERT_FALSE(iter->Done());
- op = &iter->Get();
- ASSERT_EQ(op->type, kCowReplaceOp);
- ASSERT_TRUE(reader.ReadData(*op, &sink));
- 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());
}
@@ -429,7 +401,7 @@
ASSERT_TRUE(writer->AddRawBlocks(50, data.data(), data.size()));
ASSERT_TRUE(writer->AddLabel(6));
- // fail to write the footer
+ // fail to write the footer. Cow Format does not know if Label 6 is valid
ASSERT_EQ(lseek(cow_->fd, 0, SEEK_SET), 0);
@@ -443,7 +415,7 @@
ASSERT_EQ(lseek(cow_->fd, 0, SEEK_SET), 0);
writer = std::make_unique<CowWriter>(options);
- ASSERT_TRUE(writer->Initialize(cow_->fd, CowWriter::OpenMode::APPEND));
+ ASSERT_TRUE(writer->InitializeAppend(cow_->fd, 5));
ASSERT_TRUE(writer->Finalize());
diff --git a/fs_mgr/libsnapshot/cow_writer.cpp b/fs_mgr/libsnapshot/cow_writer.cpp
index 632c866..f550e54 100644
--- a/fs_mgr/libsnapshot/cow_writer.cpp
+++ b/fs_mgr/libsnapshot/cow_writer.cpp
@@ -125,25 +125,17 @@
return true;
}
-bool CowWriter::Initialize(unique_fd&& fd, OpenMode mode) {
+bool CowWriter::Initialize(unique_fd&& fd) {
owned_fd_ = std::move(fd);
- return Initialize(borrowed_fd{owned_fd_}, mode);
+ return Initialize(borrowed_fd{owned_fd_});
}
-bool CowWriter::Initialize(borrowed_fd fd, OpenMode mode) {
+bool CowWriter::Initialize(borrowed_fd fd) {
if (!SetFd(fd) || !ParseOptions()) {
return false;
}
- switch (mode) {
- case OpenMode::WRITE:
- return OpenForWrite();
- case OpenMode::APPEND:
- return OpenForAppend();
- default:
- LOG(ERROR) << "Unknown open mode in CowWriter";
- return false;
- }
+ return OpenForWrite();
}
bool CowWriter::InitializeAppend(android::base::unique_fd&& fd, uint64_t label) {
@@ -182,17 +174,15 @@
return true;
}
-bool CowWriter::OpenForAppend(std::optional<uint64_t> label) {
+bool CowWriter::OpenForAppend(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;
}
- incomplete = !reader->GetFooter(&footer_);
+ reader->GetFooter(&footer_);
options_.block_size = header_.block_size;
@@ -203,36 +193,16 @@
auto iter = reader->GetOpIter();
while (!iter->Done() && !found_label) {
- CowOperation op = iter->Get();
+ const CowOperation& op = iter->Get();
+
if (op.type == kCowFooterOp) break;
- if (label.has_value()) {
- if (op.type == kCowFooterOp) break;
- if (op.type == kCowLabelOp) {
- if (op.source == label) found_label = true;
- }
- 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);
- }
- }
+ if (op.type == kCowLabelOp && op.source == label) found_label = true;
+ AddOperation(op);
+
iter->Next();
}
- if (label.has_value() && !found_label) {
+ if (!found_label) {
LOG(ERROR) << "Failed to find last label";
return false;
}
diff --git a/fs_mgr/libsnapshot/include/libsnapshot/cow_writer.h b/fs_mgr/libsnapshot/include/libsnapshot/cow_writer.h
index 35690d4..95c735d 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/cow_writer.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/cow_writer.h
@@ -82,19 +82,15 @@
class CowWriter : public ICowWriter {
public:
- enum class OpenMode { WRITE, APPEND };
-
explicit CowWriter(const CowOptions& options);
// Set up the writer.
- // If opening for write, the file starts from the beginning.
- // If opening for append, if the file has a footer, we start appending to the last op.
- // If the footer isn't found, the last label is considered corrupt, and dropped.
+ // The file starts from the beginning.
//
// If fd is < 0, the CowWriter will be opened against /dev/null. This is for
// computing COW sizes without using storage space.
- bool Initialize(android::base::unique_fd&& fd, OpenMode mode = OpenMode::WRITE);
- bool Initialize(android::base::borrowed_fd fd, OpenMode mode = OpenMode::WRITE);
+ bool Initialize(android::base::unique_fd&& fd);
+ bool Initialize(android::base::borrowed_fd fd);
// Set up a writer, assuming that the given label is the last valid label.
// This will result in dropping any labels that occur after the given on, and will fail
// if the given label does not appear.
@@ -115,7 +111,7 @@
void SetupHeaders();
bool ParseOptions();
bool OpenForWrite();
- bool OpenForAppend(std::optional<uint64_t> label = std::nullopt);
+ bool OpenForAppend(uint64_t label);
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);
diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot_writer.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot_writer.h
index 4732c2d..bf5ce8b 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot_writer.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot_writer.h
@@ -42,9 +42,9 @@
// Open the writer in write mode (no append).
virtual bool Initialize() = 0;
- // Open the writer in append mode, optionally with the last label to resume
+ // Open the writer in append mode, with the last label to resume
// from. See CowWriter::InitializeAppend.
- virtual bool InitializeAppend(std::optional<uint64_t> label = {}) = 0;
+ virtual bool InitializeAppend(uint64_t label) = 0;
virtual std::unique_ptr<FileDescriptor> OpenReader() = 0;
@@ -66,7 +66,7 @@
bool SetCowDevice(android::base::unique_fd&& cow_device);
bool Initialize() override;
- bool InitializeAppend(std::optional<uint64_t> label = {}) override;
+ bool InitializeAppend(uint64_t label) override;
bool Finalize() override;
uint64_t GetCowSize() override;
std::unique_ptr<FileDescriptor> OpenReader() override;
@@ -92,7 +92,7 @@
void SetSnapshotDevice(android::base::unique_fd&& snapshot_fd, uint64_t cow_size);
bool Initialize() override { return true; }
- bool InitializeAppend(std::optional<uint64_t>) override { return true; }
+ bool InitializeAppend(uint64_t) override { return true; }
bool Finalize() override;
uint64_t GetCowSize() override { return cow_size_; }
diff --git a/fs_mgr/libsnapshot/snapshot_reader.cpp b/fs_mgr/libsnapshot/snapshot_reader.cpp
index 0ac79a1..b56d879 100644
--- a/fs_mgr/libsnapshot/snapshot_reader.cpp
+++ b/fs_mgr/libsnapshot/snapshot_reader.cpp
@@ -91,6 +91,7 @@
while (!op_iter_->Done()) {
const CowOperation* op = &op_iter_->Get();
if (op->type == kCowLabelOp || op->type == kCowFooterOp) {
+ op_iter_->Next();
continue;
}
if (op->new_block >= ops_.size()) {
diff --git a/fs_mgr/libsnapshot/snapshot_writer.cpp b/fs_mgr/libsnapshot/snapshot_writer.cpp
index 85ed156..8e379e4 100644
--- a/fs_mgr/libsnapshot/snapshot_writer.cpp
+++ b/fs_mgr/libsnapshot/snapshot_writer.cpp
@@ -113,14 +113,11 @@
}
bool CompressedSnapshotWriter::Initialize() {
- return cow_->Initialize(cow_device_, CowWriter::OpenMode::WRITE);
+ return cow_->Initialize(cow_device_);
}
-bool CompressedSnapshotWriter::InitializeAppend(std::optional<uint64_t> label) {
- if (label) {
- return cow_->InitializeAppend(cow_device_, *label);
- }
- return cow_->Initialize(cow_device_, CowWriter::OpenMode::APPEND);
+bool CompressedSnapshotWriter::InitializeAppend(uint64_t label) {
+ return cow_->InitializeAppend(cow_device_, label);
}
OnlineKernelSnapshotWriter::OnlineKernelSnapshotWriter(const CowOptions& options)