Verify the extents for untouched dynamic partitions during partial update
For partial updates, the metadata for untouched dynamic partitions
are just copied over to the target slot. So, verifying the extents
of these partitions in the target metadata should be sufficient for
correctness. This saves the work to read & hash the bytes on these
partitions for each resumed update.
Bug: 151088567
Test: unit tests pass, apply a partial update
Change-Id: I9d40ed2643e145a1546ea17b146fcdcfb91f213f
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index af1baa4..7d837db 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -806,15 +806,32 @@
auto generator = partition_update_generator::Create(boot_control_,
manifest_.block_size());
- std::vector<PartitionUpdate> other_partitions;
+ std::vector<PartitionUpdate> untouched_static_partitions;
TEST_AND_RETURN_FALSE(
generator->GenerateOperationsForPartitionsNotInPayload(
install_plan_->source_slot,
install_plan_->target_slot,
touched_partitions,
- &other_partitions));
- partitions_.insert(
- partitions_.end(), other_partitions.begin(), other_partitions.end());
+ &untouched_static_partitions));
+ partitions_.insert(partitions_.end(),
+ untouched_static_partitions.begin(),
+ untouched_static_partitions.end());
+
+ // Save the untouched dynamic partitions in install plan.
+ std::vector<std::string> dynamic_partitions;
+ if (!boot_control_->GetDynamicPartitionControl()
+ ->ListDynamicPartitionsForSlot(install_plan_->source_slot,
+ &dynamic_partitions)) {
+ LOG(ERROR) << "Failed to load dynamic partitions from slot "
+ << install_plan_->source_slot;
+ return false;
+ }
+ install_plan_->untouched_dynamic_partitions.clear();
+ for (const auto& name : dynamic_partitions) {
+ if (touched_partitions.find(name) == touched_partitions.end()) {
+ install_plan_->untouched_dynamic_partitions.push_back(name);
+ }
+ }
}
// Fill in the InstallPlan::partitions based on the partitions from the
diff --git a/payload_consumer/filesystem_verifier_action.cc b/payload_consumer/filesystem_verifier_action.cc
index 5b2b5f4..61917ea 100644
--- a/payload_consumer/filesystem_verifier_action.cc
+++ b/payload_consumer/filesystem_verifier_action.cc
@@ -28,6 +28,7 @@
#include <base/bind.h>
#include <brillo/data_encoding.h>
#include <brillo/streams/file_stream.h>
+#include <base/strings/string_util.h>
#include "update_engine/common/utils.h"
@@ -89,6 +90,20 @@
void FilesystemVerifierAction::StartPartitionHashing() {
if (partition_index_ == install_plan_.partitions.size()) {
+ if (!install_plan_.untouched_dynamic_partitions.empty()) {
+ LOG(INFO) << "Verifying extents of untouched dynamic partitions ["
+ << base::JoinString(install_plan_.untouched_dynamic_partitions,
+ ", ")
+ << "]";
+ if (!dynamic_control_->VerifyExtentsForUntouchedPartitions(
+ install_plan_.source_slot,
+ install_plan_.target_slot,
+ install_plan_.untouched_dynamic_partitions)) {
+ Cleanup(ErrorCode::kFilesystemVerifierError);
+ return;
+ }
+ }
+
Cleanup(ErrorCode::kSuccess);
return;
}
diff --git a/payload_consumer/filesystem_verifier_action.h b/payload_consumer/filesystem_verifier_action.h
index 7d179df..6a8823a 100644
--- a/payload_consumer/filesystem_verifier_action.h
+++ b/payload_consumer/filesystem_verifier_action.h
@@ -57,8 +57,13 @@
class FilesystemVerifierAction : public InstallPlanAction {
public:
- FilesystemVerifierAction()
- : verity_writer_(verity_writer::CreateVerityWriter()) {}
+ explicit FilesystemVerifierAction(
+ DynamicPartitionControlInterface* dynamic_control)
+ : verity_writer_(verity_writer::CreateVerityWriter()),
+ dynamic_control_(dynamic_control) {
+ CHECK(dynamic_control_);
+ }
+
~FilesystemVerifierAction() override = default;
void PerformAction() override;
@@ -123,6 +128,9 @@
// Write verity data of the current partition.
std::unique_ptr<VerityWriterInterface> verity_writer_;
+ // Verifies the untouched dynamic partitions for partial updates.
+ DynamicPartitionControlInterface* dynamic_control_{nullptr};
+
// Reads and hashes this many bytes from the head of the input stream. When
// the partition starts to be hashed, this field is initialized from the
// corresponding InstallPlan::Partition size which is the total size
diff --git a/payload_consumer/filesystem_verifier_action_unittest.cc b/payload_consumer/filesystem_verifier_action_unittest.cc
index cb33404..2971849 100644
--- a/payload_consumer/filesystem_verifier_action_unittest.cc
+++ b/payload_consumer/filesystem_verifier_action_unittest.cc
@@ -27,6 +27,7 @@
#include <brillo/secure_blob.h>
#include <gtest/gtest.h>
+#include "update_engine/common/dynamic_partition_control_stub.h"
#include "update_engine/common/hash_calculator.h"
#include "update_engine/common/test_utils.h"
#include "update_engine/common/utils.h"
@@ -51,6 +52,7 @@
brillo::FakeMessageLoop loop_{nullptr};
ActionProcessor processor_;
+ DynamicPartitionControlStub dynamic_control_stub_;
};
class FilesystemVerifierActionTestDelegate : public ActionProcessorDelegate {
@@ -188,7 +190,8 @@
void FilesystemVerifierActionTest::BuildActions(
const InstallPlan& install_plan) {
auto feeder_action = std::make_unique<ObjectFeederAction<InstallPlan>>();
- auto verifier_action = std::make_unique<FilesystemVerifierAction>();
+ auto verifier_action =
+ std::make_unique<FilesystemVerifierAction>(&dynamic_control_stub_);
auto collector_action =
std::make_unique<ObjectCollectorAction<InstallPlan>>();
@@ -217,7 +220,8 @@
};
TEST_F(FilesystemVerifierActionTest, MissingInputObjectTest) {
- auto copier_action = std::make_unique<FilesystemVerifierAction>();
+ auto copier_action =
+ std::make_unique<FilesystemVerifierAction>(&dynamic_control_stub_);
auto collector_action =
std::make_unique<ObjectCollectorAction<InstallPlan>>();
diff --git a/payload_consumer/install_plan.h b/payload_consumer/install_plan.h
index 63178bd..f04c650 100644
--- a/payload_consumer/install_plan.h
+++ b/payload_consumer/install_plan.h
@@ -158,6 +158,10 @@
// If not blank, a base-64 encoded representation of the PEM-encoded
// public key in the response.
std::string public_key_rsa;
+
+ // The name of dynamic partitions not included in the payload. Only used
+ // for partial updates.
+ std::vector<std::string> untouched_dynamic_partitions;
};
class InstallPlanAction;
diff --git a/payload_consumer/partition_update_generator_android.cc b/payload_consumer/partition_update_generator_android.cc
index aa3f2e5..5768dd6 100644
--- a/payload_consumer/partition_update_generator_android.cc
+++ b/payload_consumer/partition_update_generator_android.cc
@@ -50,25 +50,14 @@
BootControlInterface::Slot target_slot,
const std::set<std::string>& partitions_in_payload,
std::vector<PartitionUpdate>* update_list) {
- auto ret = GetStaticAbPartitionsOnDevice();
- if (!ret.has_value()) {
+ auto ab_partitions = GetStaticAbPartitionsOnDevice();
+ if (!ab_partitions.has_value()) {
LOG(ERROR) << "Failed to load static a/b partitions";
return false;
}
- auto ab_partitions = ret.value();
-
- // Add the dynamic partitions.
- auto dynamic_control = boot_control_->GetDynamicPartitionControl();
- std::vector<std::string> dynamic_partitions;
- if (!dynamic_control->ListDynamicPartitionsForSlot(source_slot,
- &dynamic_partitions)) {
- LOG(ERROR) << "Failed to load dynamic partitions from slot " << source_slot;
- return false;
- }
- ab_partitions.insert(dynamic_partitions.begin(), dynamic_partitions.end());
std::vector<PartitionUpdate> partition_updates;
- for (const auto& partition_name : ab_partitions) {
+ for (const auto& partition_name : ab_partitions.value()) {
if (partitions_in_payload.find(partition_name) !=
partitions_in_payload.end()) {
LOG(INFO) << partition_name << " has included in payload";
@@ -159,13 +148,15 @@
return std::nullopt;
}
- if (is_source_dynamic != is_target_dynamic) {
- LOG(ERROR) << "Source slot " << source_slot << " for partition "
- << partition_name << " is " << (is_source_dynamic ? "" : "not")
- << " dynamic, but target slot " << target_slot << " is "
+ if (is_source_dynamic || is_target_dynamic) {
+ LOG(ERROR) << "Partition " << partition_name << " is expected to be a"
+ << " static partition. source slot is "
+ << (is_source_dynamic ? "" : "not")
+ << " dynamic, and target slot " << target_slot << " is "
<< (is_target_dynamic ? "" : "not") << " dynamic.";
return std::nullopt;
}
+
auto source_size = utils::FileSize(source_device);
auto target_size = utils::FileSize(target_device);
if (source_size == -1 || target_size == -1 || source_size != target_size ||
@@ -175,11 +166,8 @@
return std::nullopt;
}
- return CreatePartitionUpdate(partition_name,
- source_device,
- target_device,
- source_size,
- is_source_dynamic);
+ return CreatePartitionUpdate(
+ partition_name, source_device, target_device, source_size);
}
std::optional<PartitionUpdate>
@@ -187,8 +175,7 @@
const std::string& partition_name,
const std::string& source_device,
const std::string& target_device,
- int64_t partition_size,
- bool is_dynamic) {
+ int64_t partition_size) {
PartitionUpdate partition_update;
partition_update.set_partition_name(partition_name);
auto old_partition_info = partition_update.mutable_old_partition_info();
@@ -202,18 +189,15 @@
auto new_partition_info = partition_update.mutable_new_partition_info();
new_partition_info->set_size(partition_size);
new_partition_info->set_hash(raw_hash->data(), raw_hash->size());
- // TODO(xunchang) TBD, should we skip hashing and verification of the
- // dynamic partitions not in payload?
- if (!is_dynamic) {
- auto copy_operation = partition_update.add_operations();
- copy_operation->set_type(InstallOperation::SOURCE_COPY);
- Extent copy_extent;
- copy_extent.set_start_block(0);
- copy_extent.set_num_blocks(partition_size / block_size_);
- *copy_operation->add_src_extents() = copy_extent;
- *copy_operation->add_dst_extents() = copy_extent;
- }
+ auto copy_operation = partition_update.add_operations();
+ copy_operation->set_type(InstallOperation::SOURCE_COPY);
+ Extent copy_extent;
+ copy_extent.set_start_block(0);
+ copy_extent.set_num_blocks(partition_size / block_size_);
+
+ *copy_operation->add_src_extents() = copy_extent;
+ *copy_operation->add_dst_extents() = copy_extent;
return partition_update;
}
diff --git a/payload_consumer/partition_update_generator_android.h b/payload_consumer/partition_update_generator_android.h
index 8f33077..97b7d83 100644
--- a/payload_consumer/partition_update_generator_android.h
+++ b/payload_consumer/partition_update_generator_android.h
@@ -56,8 +56,7 @@
const std::string& partition_name,
const std::string& source_device,
const std::string& target_device,
- int64_t partition_size,
- bool is_dynamic);
+ int64_t partition_size);
std::optional<PartitionUpdate> CreatePartitionUpdate(
const std::string& partition_name,