update_engine: Use ExtentReader
Updates CopyAndHashExtents to use the new ExtentReader.
Adds CalculateAndValidatffeSourceHash() to DeltaPerformer to be used in both
SOURCE_BSDIFF and PUFFDIFF.
BUG=chromium:761138
TEST=FEATURES="test" emerge-amd64-generic update_engine; brillo_update_payload verify
Change-Id: I2e0c10fe0078c5a1ab4cd646a91d42893b6b691b
Reviewed-on: https://chromium-review.googlesource.com/653478
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Sen Jiang <senj@chromium.org>
diff --git a/common/utils.h b/common/utils.h
index 184e72b..262a403 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -88,8 +88,8 @@
bool PReadAll(int fd, void* buf, size_t count, off_t offset,
ssize_t* out_bytes_read);
-bool PReadAll(const FileDescriptorPtr& fd, void* buf, size_t count, off_t offset,
- ssize_t* out_bytes_read);
+bool PReadAll(const FileDescriptorPtr& fd, void* buf, size_t count,
+ off_t offset, ssize_t* out_bytes_read);
// Opens |path| for reading and appends its entire content to the container
// pointed to by |out_p|. Returns true upon successfully reading all of the
@@ -256,6 +256,16 @@
}
}
+// Return the total number of blocks in the passed |extents| collection.
+template <class T>
+uint64_t BlocksInExtents(const T& extents) {
+ uint64_t sum = 0;
+ for (const auto& ext : extents) {
+ sum += ext.num_blocks();
+ }
+ return sum;
+}
+
// Converts seconds into human readable notation including days, hours, minutes
// and seconds. For example, 185 will yield 3m5s, 4300 will yield 1h11m40s, and
// 360000 will yield 4d4h0m0s. Zero padding not applied. Seconds are always
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index 9299dcd..e6aa6d0 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -46,6 +46,7 @@
#include "update_engine/common/terminator.h"
#include "update_engine/payload_consumer/bzip_extent_writer.h"
#include "update_engine/payload_consumer/download_action.h"
+#include "update_engine/payload_consumer/extent_reader.h"
#include "update_engine/payload_consumer/extent_writer.h"
#include "update_engine/payload_consumer/file_descriptor_utils.h"
#if USE_MTD
@@ -1172,6 +1173,28 @@
return true;
}
+bool DeltaPerformer::CalculateAndValidateSourceHash(
+ const InstallOperation& operation, ErrorCode* error) {
+ const uint64_t kMaxBlocksToRead = 256; // 1MB if block size is 4KB
+ auto total_blocks = utils::BlocksInExtents(operation.src_extents());
+ brillo::Blob buf(std::min(kMaxBlocksToRead, total_blocks) * block_size_);
+ DirectExtentReader reader;
+ TEST_AND_RETURN_FALSE(
+ reader.Init(source_fd_, operation.src_extents(), block_size_));
+ HashCalculator source_hasher;
+ while (total_blocks > 0) {
+ auto read_blocks = std::min(total_blocks, kMaxBlocksToRead);
+ TEST_AND_RETURN_FALSE(reader.Read(buf.data(), read_blocks * block_size_));
+ TEST_AND_RETURN_FALSE(
+ source_hasher.Update(buf.data(), read_blocks * block_size_));
+ total_blocks -= read_blocks;
+ }
+ TEST_AND_RETURN_FALSE(source_hasher.Finalize());
+ TEST_AND_RETURN_FALSE(
+ ValidateSourceHash(source_hasher.raw_hash(), operation, error));
+ return true;
+}
+
bool DeltaPerformer::PerformSourceBsdiffOperation(
const InstallOperation& operation, ErrorCode* error) {
// Since we delete data off the beginning of the buffer as we use it,
@@ -1184,26 +1207,7 @@
TEST_AND_RETURN_FALSE(operation.dst_length() % block_size_ == 0);
if (operation.has_src_sha256_hash()) {
- HashCalculator source_hasher;
- const uint64_t kMaxBlocksToRead = 512; // 2MB if block size is 4KB
- brillo::Blob buf(kMaxBlocksToRead * block_size_);
- for (const Extent& extent : operation.src_extents()) {
- for (uint64_t i = 0; i < extent.num_blocks(); i += kMaxBlocksToRead) {
- uint64_t blocks_to_read = min(
- kMaxBlocksToRead, static_cast<uint64_t>(extent.num_blocks()) - i);
- ssize_t bytes_to_read = blocks_to_read * block_size_;
- ssize_t bytes_read_this_iteration = 0;
- TEST_AND_RETURN_FALSE(
- utils::PReadAll(source_fd_, buf.data(), bytes_to_read,
- (extent.start_block() + i) * block_size_,
- &bytes_read_this_iteration));
- TEST_AND_RETURN_FALSE(bytes_read_this_iteration == bytes_to_read);
- TEST_AND_RETURN_FALSE(source_hasher.Update(buf.data(), bytes_to_read));
- }
- }
- TEST_AND_RETURN_FALSE(source_hasher.Finalize());
- TEST_AND_RETURN_FALSE(
- ValidateSourceHash(source_hasher.raw_hash(), operation, error));
+ TEST_AND_RETURN_FALSE(CalculateAndValidateSourceHash(operation, error));
}
string input_positions;
diff --git a/payload_consumer/delta_performer.h b/payload_consumer/delta_performer.h
index 038b260..5fc972b 100644
--- a/payload_consumer/delta_performer.h
+++ b/payload_consumer/delta_performer.h
@@ -19,6 +19,7 @@
#include <inttypes.h>
+#include <limits>
#include <string>
#include <vector>
@@ -162,9 +163,9 @@
public_key_path_ = public_key_path;
}
- // Set |*out_offset| to the byte offset where the size of the metadata signature
- // is stored in a payload. Return true on success, if this field is not
- // present in the payload, return false.
+ // Set |*out_offset| to the byte offset where the size of the metadata
+ // signature is stored in a payload. Return true on success, if this field is
+ // not present in the payload, return false.
bool GetMetadataSignatureSizeOffset(uint64_t* out_offset) const;
// Set |*out_offset| to the byte offset at which the manifest protobuf begins
@@ -242,6 +243,10 @@
// buffer.
ErrorCode ValidateMetadataSignature(const brillo::Blob& payload);
+ // Calculates and validates the source hash of the operation |operation|.
+ bool CalculateAndValidateSourceHash(const InstallOperation& operation,
+ ErrorCode* error);
+
// Returns true on success.
bool PerformInstallOperation(const InstallOperation& operation);
diff --git a/payload_consumer/file_descriptor_utils.cc b/payload_consumer/file_descriptor_utils.cc
index b092149..73f86df 100644
--- a/payload_consumer/file_descriptor_utils.cc
+++ b/payload_consumer/file_descriptor_utils.cc
@@ -22,6 +22,7 @@
#include "update_engine/common/hash_calculator.h"
#include "update_engine/common/utils.h"
+#include "update_engine/payload_consumer/extent_reader.h"
#include "update_engine/payload_consumer/extent_writer.h"
using google::protobuf::RepeatedPtrField;
@@ -34,15 +35,6 @@
// Size of the buffer used to copy blocks.
const int kMaxCopyBufferSize = 1024 * 1024;
-// Return the total number of blocks in the passed |extents| list.
-uint64_t GetBlockCount(const RepeatedPtrField<Extent>& extents) {
- uint64_t sum = 0;
- for (const Extent& ext : extents) {
- sum += ext.num_blocks();
- }
- return sum;
-}
-
} // namespace
namespace fd_utils {
@@ -53,48 +45,31 @@
const RepeatedPtrField<Extent>& tgt_extents,
uint32_t block_size,
brillo::Blob* hash_out) {
- HashCalculator source_hasher;
+ uint64_t total_blocks = utils::BlocksInExtents(src_extents);
+ TEST_AND_RETURN_FALSE(total_blocks == utils::BlocksInExtents(tgt_extents));
+
+ DirectExtentReader reader;
+ TEST_AND_RETURN_FALSE(reader.Init(source, src_extents, block_size));
+ DirectExtentWriter writer;
+ TEST_AND_RETURN_FALSE(writer.Init(target, tgt_extents, block_size));
uint64_t buffer_blocks = kMaxCopyBufferSize / block_size;
// Ensure we copy at least one block at a time.
if (buffer_blocks < 1)
buffer_blocks = 1;
-
- uint64_t total_blocks = GetBlockCount(src_extents);
- TEST_AND_RETURN_FALSE(total_blocks == GetBlockCount(tgt_extents));
-
brillo::Blob buf(buffer_blocks * block_size);
- DirectExtentWriter writer;
- TEST_AND_RETURN_FALSE(writer.Init(target, tgt_extents, block_size));
-
- for (const Extent& src_ext : src_extents) {
- for (uint64_t src_ext_block = 0; src_ext_block < src_ext.num_blocks();
- src_ext_block += buffer_blocks) {
- uint64_t iteration_blocks =
- min(buffer_blocks,
- static_cast<uint64_t>(src_ext.num_blocks() - src_ext_block));
- uint64_t src_start_block = src_ext.start_block() + src_ext_block;
-
- ssize_t bytes_read_this_iteration;
- TEST_AND_RETURN_FALSE(utils::PReadAll(source,
- buf.data(),
- iteration_blocks * block_size,
- src_start_block * block_size,
- &bytes_read_this_iteration));
-
+ HashCalculator source_hasher;
+ uint64_t blocks_left = total_blocks;
+ while (blocks_left > 0) {
+ uint64_t read_blocks = std::min(blocks_left, buffer_blocks);
+ TEST_AND_RETURN_FALSE(reader.Read(buf.data(), read_blocks * block_size));
+ if (hash_out) {
TEST_AND_RETURN_FALSE(
- bytes_read_this_iteration ==
- static_cast<ssize_t>(iteration_blocks * block_size));
-
- TEST_AND_RETURN_FALSE(
- writer.Write(buf.data(), iteration_blocks * block_size));
-
- if (hash_out) {
- TEST_AND_RETURN_FALSE(
- source_hasher.Update(buf.data(), iteration_blocks * block_size));
- }
+ source_hasher.Update(buf.data(), read_blocks * block_size));
}
+ TEST_AND_RETURN_FALSE(writer.Write(buf.data(), read_blocks * block_size));
+ blocks_left -= read_blocks;
}
TEST_AND_RETURN_FALSE(writer.End());
diff --git a/payload_consumer/file_descriptor_utils_unittest.cc b/payload_consumer/file_descriptor_utils_unittest.cc
index 42bdf17..8ba8ce6 100644
--- a/payload_consumer/file_descriptor_utils_unittest.cc
+++ b/payload_consumer/file_descriptor_utils_unittest.cc
@@ -32,13 +32,15 @@
#include "update_engine/payload_consumer/file_descriptor.h"
#include "update_engine/payload_generator/extent_ranges.h"
+using google::protobuf::RepeatedPtrField;
+
namespace chromeos_update_engine {
namespace {
-::google::protobuf::RepeatedPtrField<Extent> CreateExtentList(
+RepeatedPtrField<Extent> CreateExtentList(
const std::vector<std::pair<uint64_t, uint64_t>>& lst) {
- ::google::protobuf::RepeatedPtrField<Extent> result;
+ RepeatedPtrField<Extent> result;
for (const auto& item : lst) {
*result.Add() = ExtentForRange(item.first, item.second);
}
