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)