libsnapshot: Add a source_partition parameter to OpenSnapshotWriter.
Reading from the base device is incorrect, because if the partition
shrinks, we may still have copy operations from the removed area in the
original partition. Ask the caller to explicitly name the source device
for AddCopy() operations.
Bug: 168554689
Test: vts_libsnapshot_test
Test: full OTA with update_device.py
Test: incremental OTA with update_device.py
Change-Id: If388e37c2a2f9288a43d2849312c921bf59d4918
diff --git a/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot.h
index 13f19aa..6dee3d4 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot.h
@@ -39,7 +39,9 @@
std::string* snapshot_path),
(override));
MOCK_METHOD(std::unique_ptr<ISnapshotWriter>, OpenSnapshotWriter,
- (const android::fs_mgr::CreateLogicalPartitionParams& params), (override));
+ (const android::fs_mgr::CreateLogicalPartitionParams& params,
+ const std::optional<std::string>&),
+ (override));
MOCK_METHOD(bool, UnmapUpdateSnapshot, (const std::string& target_partition_name), (override));
MOCK_METHOD(bool, NeedSnapshotsInFirstStageMount, (), (override));
MOCK_METHOD(bool, CreateLogicalAndSnapshotPartitions,
diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
index 403e350..4bbdca3 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
@@ -182,9 +182,12 @@
std::string* snapshot_path) = 0;
// Create an ISnapshotWriter to build a snapshot against a target partition. The partition name
- // must be suffixed.
+ // must be suffixed. If a source partition exists, it must be specified as well. The source
+ // partition will only be used if raw bytes are needed. The source partition should be an
+ // absolute path to the device, not a partition name.
virtual std::unique_ptr<ISnapshotWriter> OpenSnapshotWriter(
- const android::fs_mgr::CreateLogicalPartitionParams& params) = 0;
+ const android::fs_mgr::CreateLogicalPartitionParams& params,
+ const std::optional<std::string>& source_device) = 0;
// Unmap a snapshot device or CowWriter that was previously opened with MapUpdateSnapshot,
// OpenSnapshotWriter. All outstanding open descriptors, writers, or
@@ -300,7 +303,8 @@
bool MapUpdateSnapshot(const CreateLogicalPartitionParams& params,
std::string* snapshot_path) override;
std::unique_ptr<ISnapshotWriter> OpenSnapshotWriter(
- const android::fs_mgr::CreateLogicalPartitionParams& params) override;
+ const android::fs_mgr::CreateLogicalPartitionParams& params,
+ const std::optional<std::string>& source_device) override;
bool UnmapUpdateSnapshot(const std::string& target_partition_name) override;
bool NeedSnapshotsInFirstStageMount() override;
bool CreateLogicalAndSnapshotPartitions(
@@ -540,14 +544,14 @@
};
// Helpers for OpenSnapshotWriter.
- std::unique_ptr<ISnapshotWriter> OpenCompressedSnapshotWriter(LockedFile* lock,
- const std::string& partition_name,
- const SnapshotStatus& status,
- const SnapshotPaths& paths);
- std::unique_ptr<ISnapshotWriter> OpenKernelSnapshotWriter(LockedFile* lock,
- const std::string& partition_name,
- const SnapshotStatus& status,
- const SnapshotPaths& paths);
+ std::unique_ptr<ISnapshotWriter> OpenCompressedSnapshotWriter(
+ 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_stub.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stub.h
index cda2bee..ed790a0 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stub.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stub.h
@@ -37,7 +37,8 @@
bool MapUpdateSnapshot(const android::fs_mgr::CreateLogicalPartitionParams& params,
std::string* snapshot_path) override;
std::unique_ptr<ISnapshotWriter> OpenSnapshotWriter(
- const android::fs_mgr::CreateLogicalPartitionParams& params) override;
+ const android::fs_mgr::CreateLogicalPartitionParams& params,
+ const std::optional<std::string>& source_device) override;
bool UnmapUpdateSnapshot(const std::string& target_partition_name) override;
bool NeedSnapshotsInFirstStageMount() override;
bool CreateLogicalAndSnapshotPartitions(
diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot_writer.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot_writer.h
index da058f9..e25ec07 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot_writer.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot_writer.h
@@ -33,13 +33,18 @@
// Set the source device. This is used for AddCopy() operations, if the
// underlying writer needs the original bytes (for example if backed by
- // dm-snapshot or if writing directly to an unsnapshotted region).
- void SetSourceDevice(android::base::unique_fd&& source_fd);
+ // dm-snapshot or if writing directly to an unsnapshotted region). The
+ // device is only opened on the first operation that requires it.
+ void SetSourceDevice(const std::string& source_device);
virtual std::unique_ptr<FileDescriptor> OpenReader() = 0;
protected:
+ android::base::borrowed_fd GetSourceFd();
+
+ private:
android::base::unique_fd source_fd_;
+ std::optional<std::string> source_device_;
};
// Send writes to a COW or a raw device directly, based on a threshold.
diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp
index 3541ef6..6574457 100644
--- a/fs_mgr/libsnapshot/snapshot.cpp
+++ b/fs_mgr/libsnapshot/snapshot.cpp
@@ -2471,9 +2471,11 @@
}
std::unique_ptr<ISnapshotWriter> SnapshotManager::OpenSnapshotWriter(
- const android::fs_mgr::CreateLogicalPartitionParams& params) {
+ const android::fs_mgr::CreateLogicalPartitionParams& params,
+ const std::optional<std::string>& source_device) {
#if defined(LIBSNAPSHOT_NO_COW_WRITE)
(void)params;
+ (void)source_device;
LOG(ERROR) << "Snapshots cannot be written in first-stage init or recovery";
return nullptr;
@@ -2508,16 +2510,19 @@
}
if (IsCompressionEnabled()) {
- return OpenCompressedSnapshotWriter(lock.get(), params.GetPartitionName(), status, paths);
+ return OpenCompressedSnapshotWriter(lock.get(), source_device, params.GetPartitionName(),
+ status, paths);
}
- return OpenKernelSnapshotWriter(lock.get(), params.GetPartitionName(), status, paths);
+ return OpenKernelSnapshotWriter(lock.get(), source_device, params.GetPartitionName(), status,
+ paths);
#endif
}
#if !defined(LIBSNAPSHOT_NO_COW_WRITE)
std::unique_ptr<ISnapshotWriter> SnapshotManager::OpenCompressedSnapshotWriter(
- LockedFile* lock, [[maybe_unused]] const std::string& partition_name,
- const SnapshotStatus& status, const SnapshotPaths& paths) {
+ 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;
@@ -2529,13 +2534,9 @@
CHECK(status.snapshot_size() == status.device_size());
auto writer = std::make_unique<CompressedSnapshotWriter>(cow_options);
-
- unique_fd base_fd(open(paths.target_device.c_str(), O_RDWR | O_CLOEXEC));
- if (base_fd < 0) {
- PLOG(ERROR) << "OpenCompressedSnapshotWriter: open " << paths.target_device;
- return nullptr;
+ if (source_device) {
+ writer->SetSourceDevice(*source_device);
}
- writer->SetSourceDevice(std::move(base_fd));
std::string cow_path;
if (!GetMappedImageDevicePath(paths.cow_device_name, &cow_path)) {
@@ -2557,8 +2558,9 @@
}
std::unique_ptr<ISnapshotWriter> SnapshotManager::OpenKernelSnapshotWriter(
- LockedFile* lock, [[maybe_unused]] const std::string& partition_name,
- const SnapshotStatus& status, const SnapshotPaths& paths) {
+ 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;
@@ -2573,6 +2575,10 @@
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);
diff --git a/fs_mgr/libsnapshot/snapshot_stub.cpp b/fs_mgr/libsnapshot/snapshot_stub.cpp
index 41f5da4..5be3c10 100644
--- a/fs_mgr/libsnapshot/snapshot_stub.cpp
+++ b/fs_mgr/libsnapshot/snapshot_stub.cpp
@@ -131,7 +131,7 @@
}
std::unique_ptr<ISnapshotWriter> SnapshotManagerStub::OpenSnapshotWriter(
- const CreateLogicalPartitionParams&) {
+ const CreateLogicalPartitionParams&, const std::optional<std::string>&) {
LOG(ERROR) << __FUNCTION__ << " should never be called.";
return nullptr;
}
diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp
index 77226ad..7327629 100644
--- a/fs_mgr/libsnapshot/snapshot_test.cpp
+++ b/fs_mgr/libsnapshot/snapshot_test.cpp
@@ -903,7 +903,7 @@
.partition_opener = opener_.get(),
};
- auto result = sm->OpenSnapshotWriter(params);
+ auto result = sm->OpenSnapshotWriter(params, {});
if (!result) {
return AssertionFailure() << "Cannot open snapshot for writing: " << name;
}
diff --git a/fs_mgr/libsnapshot/snapshot_writer.cpp b/fs_mgr/libsnapshot/snapshot_writer.cpp
index aa49ab1..19aa80e 100644
--- a/fs_mgr/libsnapshot/snapshot_writer.cpp
+++ b/fs_mgr/libsnapshot/snapshot_writer.cpp
@@ -24,13 +24,30 @@
namespace android {
namespace snapshot {
+using android::base::borrowed_fd;
using android::base::unique_fd;
using chromeos_update_engine::FileDescriptor;
ISnapshotWriter::ISnapshotWriter(const CowOptions& options) : ICowWriter(options) {}
-void ISnapshotWriter::SetSourceDevice(android::base::unique_fd&& source_fd) {
- source_fd_ = std::move(source_fd);
+void ISnapshotWriter::SetSourceDevice(const std::string& source_device) {
+ source_device_ = {source_device};
+}
+
+borrowed_fd ISnapshotWriter::GetSourceFd() {
+ if (!source_device_) {
+ LOG(ERROR) << "Attempted to read from source device but none was set";
+ return borrowed_fd{-1};
+ }
+
+ if (source_fd_ < 0) {
+ source_fd_.reset(open(source_device_->c_str(), O_RDONLY | O_CLOEXEC));
+ if (source_fd_ < 0) {
+ PLOG(ERROR) << "open " << *source_device_;
+ return borrowed_fd{-1};
+ }
+ }
+ return source_fd_;
}
CompressedSnapshotWriter::CompressedSnapshotWriter(const CowOptions& options)
@@ -109,9 +126,14 @@
}
bool OnlineKernelSnapshotWriter::EmitCopy(uint64_t new_block, uint64_t old_block) {
+ auto source_fd = GetSourceFd();
+ if (source_fd < 0) {
+ return false;
+ }
+
std::string buffer(options_.block_size, 0);
uint64_t offset = old_block * options_.block_size;
- if (!android::base::ReadFullyAtOffset(source_fd_, buffer.data(), buffer.size(), offset)) {
+ if (!android::base::ReadFullyAtOffset(source_fd, buffer.data(), buffer.size(), offset)) {
PLOG(ERROR) << "EmitCopy read";
return false;
}