Don't use unix open() syscall on VABC partitions
Move logic dealing with target partitions to a function, such that it
can be overriden by subclasses.
Bug: 168554689
Test: treehugger
Change-Id: I59053a70915e51b0ab1b30922d14f211e1ba0605
diff --git a/payload_consumer/partition_writer.cc b/payload_consumer/partition_writer.cc
index ec36d06..bec2594 100644
--- a/payload_consumer/partition_writer.cc
+++ b/payload_consumer/partition_writer.cc
@@ -253,27 +253,36 @@
Close();
}
+bool PartitionWriter::OpenSourcePartition(uint32_t source_slot,
+ bool source_may_exist) {
+ source_path_.clear();
+ if (!source_may_exist) {
+ return true;
+ }
+ if (install_part_.source_size > 0 && !install_part_.source_path.empty()) {
+ source_path_ = install_part_.source_path;
+ int err;
+ source_fd_ = OpenFile(source_path_.c_str(), O_RDONLY, false, &err);
+ if (source_fd_ == nullptr) {
+ LOG(ERROR) << "Unable to open source partition " << install_part_.name
+ << " on slot " << BootControlInterface::SlotName(source_slot)
+ << ", file " << source_path_;
+ return false;
+ }
+ }
+ return true;
+}
+
bool PartitionWriter::Init(const InstallPlan* install_plan,
bool source_may_exist) {
const PartitionUpdate& partition = partition_update_;
uint32_t source_slot = install_plan->source_slot;
uint32_t target_slot = install_plan->target_slot;
+ TEST_AND_RETURN_FALSE(OpenSourcePartition(source_slot, source_may_exist));
// We shouldn't open the source partition in certain cases, e.g. some dynamic
// partitions in delta payload, partitions included in the full payload for
// partial updates. Use the source size as the indicator.
- if (source_may_exist && install_part_.source_size > 0) {
- source_path_ = install_part_.source_path;
- int err;
- source_fd_ = OpenFile(source_path_.c_str(), O_RDONLY, false, &err);
- if (!source_fd_) {
- LOG(ERROR) << "Unable to open source partition "
- << partition.partition_name() << " on slot "
- << BootControlInterface::SlotName(source_slot) << ", file "
- << source_path_;
- return false;
- }
- }
target_path_ = install_part_.target_path;
int err;
diff --git a/payload_consumer/partition_writer.h b/payload_consumer/partition_writer.h
index 1c8ffbd..79ba665 100644
--- a/payload_consumer/partition_writer.h
+++ b/payload_consumer/partition_writer.h
@@ -84,6 +84,8 @@
friend class PartitionWriterTest;
FRIEND_TEST(PartitionWriterTest, ChooseSourceFDTest);
+ bool OpenSourcePartition(uint32_t source_slot, bool source_may_exist);
+
bool OpenCurrentECCPartition();
// For a given operation, choose the source fd to be used (raw device or error
// correction device) based on the source operation hash.
diff --git a/payload_consumer/vabc_partition_writer.cc b/payload_consumer/vabc_partition_writer.cc
index 980f2ca..a869509 100644
--- a/payload_consumer/vabc_partition_writer.cc
+++ b/payload_consumer/vabc_partition_writer.cc
@@ -17,6 +17,7 @@
#include "update_engine/payload_consumer/vabc_partition_writer.h"
#include <memory>
+#include <string>
#include <vector>
#include <libsnapshot/cow_writer.h>
@@ -33,9 +34,15 @@
bool VABCPartitionWriter::Init(const InstallPlan* install_plan,
bool source_may_exist) {
TEST_AND_RETURN_FALSE(install_plan != nullptr);
- TEST_AND_RETURN_FALSE(PartitionWriter::Init(install_plan, source_may_exist));
+ TEST_AND_RETURN_FALSE(
+ OpenSourcePartition(install_plan->source_slot, source_may_exist));
+ std::optional<std::string> source_path;
+ if (!install_part_.source_path.empty()) {
+ // TODO(zhangkelvin) Make |source_path| a std::optional<std::string>
+ source_path = install_part_.source_path;
+ }
cow_writer_ = dynamic_control_->OpenCowWriter(
- install_part_.name, install_part_.source_path, install_plan->is_resume);
+ install_part_.name, source_path, install_plan->is_resume);
TEST_AND_RETURN_FALSE(cow_writer_ != nullptr);
// TODO(zhangkelvin) Emit a label before writing SOURCE_COPY. When resuming,