libsnapshot: Remove OnlineKernelSnapshotWriter.
This was never used or tested. The idea was to unify update_engine's
write paths on ISnapshotWriter. But given that legacy VAB is "legacy",
it doesn't make sense to begin refactoring that code and potentially
introducing bugs. Let's just remove this instead.
Bug: 280529365
Test: builds
Change-Id: Ie2f531bd140e183dfde4b65a144f3b4acc28e78a
diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
index 9eb89b6..ecf1d15 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
@@ -697,10 +697,6 @@
LockedFile* lock, const std::optional<std::string>& source_device,
const std::string& partition_name, const SnapshotStatus& status,
const SnapshotPaths& paths);
- std::unique_ptr<ISnapshotWriter> OpenKernelSnapshotWriter(
- LockedFile* lock, const std::optional<std::string>& source_device,
- const std::string& partition_name, const SnapshotStatus& status,
- const SnapshotPaths& paths);
// Map the base device, COW devices, and snapshot device.
bool MapPartitionWithSnapshot(LockedFile* lock, CreateLogicalPartitionParams params,
diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot_writer.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot_writer.h
index 0e3b1db..8f6344c 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot_writer.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot_writer.h
@@ -89,38 +89,5 @@
std::unique_ptr<CowWriter> cow_;
};
-// Write directly to a dm-snapshot device.
-class OnlineKernelSnapshotWriter final : public ISnapshotWriter {
- public:
- OnlineKernelSnapshotWriter(const CowOptions& options);
-
- // Set the device used for all writes.
- void SetSnapshotDevice(android::base::unique_fd&& snapshot_fd, uint64_t cow_size);
-
- bool Initialize() override { return true; }
- bool InitializeAppend(uint64_t) override { return true; }
-
- bool Finalize() override;
- uint64_t GetCowSize() override { return cow_size_; }
- std::unique_ptr<FileDescriptor> OpenReader() override;
-
- // Online kernel snapshot writer doesn't care about merge sequences.
- // So ignore.
- bool VerifyMergeOps() const noexcept override { return true; }
-
- protected:
- bool EmitRawBlocks(uint64_t new_block_start, const void* data, size_t size) override;
- bool EmitZeroBlocks(uint64_t new_block_start, uint64_t num_blocks) override;
- bool EmitXorBlocks(uint32_t new_block_start, const void* data, size_t size, uint32_t old_block,
- uint16_t offset) override;
- bool EmitCopy(uint64_t new_block, uint64_t old_block, uint64_t num_blocks = 1) override;
- bool EmitLabel(uint64_t label) override;
- bool EmitSequenceData(size_t num_ops, const uint32_t* data) override;
-
- private:
- android::base::unique_fd snapshot_fd_;
- uint64_t cow_size_ = 0;
-};
-
} // namespace snapshot
} // namespace android
diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp
index b0c4a83..e114d25 100644
--- a/fs_mgr/libsnapshot/snapshot.cpp
+++ b/fs_mgr/libsnapshot/snapshot.cpp
@@ -3647,12 +3647,13 @@
return nullptr;
}
- if (status.using_snapuserd()) {
- return OpenCompressedSnapshotWriter(lock.get(), source_device, params.GetPartitionName(),
- status, paths);
+ if (!status.using_snapuserd()) {
+ LOG(ERROR) << "Can only create snapshot writers with userspace or compressed snapshots";
+ return nullptr;
}
- return OpenKernelSnapshotWriter(lock.get(), source_device, params.GetPartitionName(), status,
- paths);
+
+ return OpenCompressedSnapshotWriter(lock.get(), source_device, params.GetPartitionName(),
+ status, paths);
#endif
}
@@ -3700,34 +3701,6 @@
return writer;
}
-
-std::unique_ptr<ISnapshotWriter> SnapshotManager::OpenKernelSnapshotWriter(
- LockedFile* lock, const std::optional<std::string>& source_device,
- [[maybe_unused]] const std::string& partition_name, const SnapshotStatus& status,
- const SnapshotPaths& paths) {
- CHECK(lock);
-
- CowOptions cow_options;
- cow_options.max_blocks = {status.device_size() / cow_options.block_size};
-
- auto writer = std::make_unique<OnlineKernelSnapshotWriter>(cow_options);
-
- std::string path = paths.snapshot_device.empty() ? paths.target_device : paths.snapshot_device;
- unique_fd fd(open(path.c_str(), O_RDWR | O_CLOEXEC));
- if (fd < 0) {
- PLOG(ERROR) << "open failed: " << path;
- return nullptr;
- }
-
- if (source_device) {
- writer->SetSourceDevice(*source_device);
- }
-
- uint64_t cow_size = status.cow_partition_size() + status.cow_file_size();
- writer->SetSnapshotDevice(std::move(fd), cow_size);
-
- return writer;
-}
#endif // !defined(LIBSNAPSHOT_NO_COW_WRITE)
bool SnapshotManager::UnmapUpdateSnapshot(const std::string& target_partition_name) {
diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp
index f08f913..7cdb8ed 100644
--- a/fs_mgr/libsnapshot/snapshot_test.cpp
+++ b/fs_mgr/libsnapshot/snapshot_test.cpp
@@ -643,21 +643,38 @@
TEST_F(SnapshotTest, Merge) {
ASSERT_TRUE(AcquireLock());
- static const uint64_t kDeviceSize = 1024 * 1024;
-
- std::unique_ptr<ISnapshotWriter> writer;
- ASSERT_TRUE(PrepareOneSnapshot(kDeviceSize, &writer));
-
- bool userspace_snapshots = sm->UpdateUsesUserSnapshots(lock_.get());
-
- // Release the lock.
- lock_ = nullptr;
+ static constexpr uint64_t kDeviceSize = 1024 * 1024;
+ static constexpr uint32_t kBlockSize = 4096;
std::string test_string = "This is a test string.";
- test_string.resize(writer->options().block_size);
- ASSERT_TRUE(writer->AddRawBlocks(0, test_string.data(), test_string.size()));
- ASSERT_TRUE(writer->Finalize());
- writer = nullptr;
+ test_string.resize(kBlockSize);
+
+ bool userspace_snapshots = false;
+ if (snapuserd_required_) {
+ std::unique_ptr<ISnapshotWriter> writer;
+ ASSERT_TRUE(PrepareOneSnapshot(kDeviceSize, &writer));
+
+ userspace_snapshots = sm->UpdateUsesUserSnapshots(lock_.get());
+
+ // Release the lock.
+ lock_ = nullptr;
+
+ ASSERT_TRUE(writer->AddRawBlocks(0, test_string.data(), test_string.size()));
+ ASSERT_TRUE(writer->Finalize());
+ writer = nullptr;
+ } else {
+ ASSERT_TRUE(PrepareOneSnapshot(kDeviceSize));
+
+ // Release the lock.
+ lock_ = nullptr;
+
+ std::string path;
+ ASSERT_TRUE(dm_.GetDmDevicePathByName("test_partition_b", &path));
+
+ unique_fd fd(open(path.c_str(), O_WRONLY));
+ ASSERT_GE(fd, 0);
+ ASSERT_TRUE(android::base::WriteFully(fd, test_string.data(), test_string.size()));
+ }
// Done updating.
ASSERT_TRUE(sm->FinishedSnapshotWrites(false));
diff --git a/fs_mgr/libsnapshot/snapshot_writer.cpp b/fs_mgr/libsnapshot/snapshot_writer.cpp
index 82a7fd7..6a3906e 100644
--- a/fs_mgr/libsnapshot/snapshot_writer.cpp
+++ b/fs_mgr/libsnapshot/snapshot_writer.cpp
@@ -149,95 +149,5 @@
return cow_->InitializeAppend(cow_device_, label);
}
-OnlineKernelSnapshotWriter::OnlineKernelSnapshotWriter(const CowOptions& options)
- : ISnapshotWriter(options) {}
-
-void OnlineKernelSnapshotWriter::SetSnapshotDevice(android::base::unique_fd&& snapshot_fd,
- uint64_t cow_size) {
- snapshot_fd_ = std::move(snapshot_fd);
- cow_size_ = cow_size;
-}
-
-bool OnlineKernelSnapshotWriter::Finalize() {
- if (fsync(snapshot_fd_.get()) < 0) {
- PLOG(ERROR) << "fsync";
- return false;
- }
- return true;
-}
-
-bool OnlineKernelSnapshotWriter::EmitRawBlocks(uint64_t new_block_start, const void* data,
- size_t size) {
- uint64_t offset = new_block_start * options_.block_size;
- if (lseek(snapshot_fd_.get(), offset, SEEK_SET) < 0) {
- PLOG(ERROR) << "EmitRawBlocks lseek to offset " << offset;
- return false;
- }
- if (!android::base::WriteFully(snapshot_fd_, data, size)) {
- PLOG(ERROR) << "EmitRawBlocks write";
- return false;
- }
- return true;
-}
-
-bool OnlineKernelSnapshotWriter::EmitXorBlocks(uint32_t, const void*, size_t, uint32_t, uint16_t) {
- LOG(ERROR) << "EmitXorBlocks not implemented.";
- return false;
-}
-
-bool OnlineKernelSnapshotWriter::EmitZeroBlocks(uint64_t new_block_start, uint64_t num_blocks) {
- std::string zeroes(options_.block_size, 0);
- for (uint64_t i = 0; i < num_blocks; i++) {
- if (!EmitRawBlocks(new_block_start + i, zeroes.data(), zeroes.size())) {
- return false;
- }
- }
- return true;
-}
-
-bool OnlineKernelSnapshotWriter::EmitCopy(uint64_t new_block, uint64_t old_block,
- uint64_t num_blocks) {
- auto source_fd = GetSourceFd();
- if (source_fd < 0) {
- return false;
- }
-
- CHECK(num_blocks != 0);
-
- for (size_t i = 0; i < num_blocks; i++) {
- std::string buffer(options_.block_size, 0);
- uint64_t offset = (old_block + i) * options_.block_size;
- if (!android::base::ReadFullyAtOffset(source_fd, buffer.data(), buffer.size(), offset)) {
- PLOG(ERROR) << "EmitCopy read";
- return false;
- }
- if (!EmitRawBlocks(new_block + i, buffer.data(), buffer.size())) {
- PLOG(ERROR) << "EmitRawBlocks failed";
- return false;
- }
- }
-
- return true;
-}
-
-bool OnlineKernelSnapshotWriter::EmitLabel(uint64_t) {
- // Not Needed
- return true;
-}
-
-bool OnlineKernelSnapshotWriter::EmitSequenceData(size_t, const uint32_t*) {
- // Not Needed
- return true;
-}
-
-std::unique_ptr<FileDescriptor> OnlineKernelSnapshotWriter::OpenReader() {
- unique_fd fd(dup(snapshot_fd_.get()));
- if (fd < 0) {
- PLOG(ERROR) << "dup2 failed in OpenReader";
- return nullptr;
- }
- return std::make_unique<ReadFdFileDescriptor>(std::move(fd));
-}
-
} // namespace snapshot
} // namespace android