Merge remote-tracking branch 'goog/upstream-master'.
The following commits were reverted:
840703a Fix update over cellular network on guest account
eaad5d0 Do not merge to AOSP: Fixes the link to brillo-clang-format in CrOS
740efad Reboot even if a system update is not available.
Fixed a few sign compare warnings.
Had to ifdef out 2 SquashfsFilesystemTest because it depends on unsquashfs -m.
Test: update_engine_unittests
Change-Id: I6f4ca5003e78c76064ec60d0797505d8c18d00bf
Merged-In: I6f4ca5003e78c76064ec60d0797505d8c18d00bf
diff --git a/payload_consumer/bzip_extent_writer.cc b/payload_consumer/bzip_extent_writer.cc
index 0fcc8ba..39d9d67 100644
--- a/payload_consumer/bzip_extent_writer.cc
+++ b/payload_consumer/bzip_extent_writer.cc
@@ -16,7 +16,7 @@
#include "update_engine/payload_consumer/bzip_extent_writer.h"
-using std::vector;
+using google::protobuf::RepeatedPtrField;
namespace chromeos_update_engine {
@@ -25,7 +25,7 @@
}
bool BzipExtentWriter::Init(FileDescriptorPtr fd,
- const vector<Extent>& extents,
+ const RepeatedPtrField<Extent>& extents,
uint32_t block_size) {
// Init bzip2 stream
int rc = BZ2_bzDecompressInit(&stream_,
diff --git a/payload_consumer/bzip_extent_writer.h b/payload_consumer/bzip_extent_writer.h
index 0ad542e..86b346a 100644
--- a/payload_consumer/bzip_extent_writer.h
+++ b/payload_consumer/bzip_extent_writer.h
@@ -19,7 +19,7 @@
#include <bzlib.h>
#include <memory>
-#include <vector>
+#include <utility>
#include <brillo/secure_blob.h>
@@ -41,7 +41,7 @@
~BzipExtentWriter() override = default;
bool Init(FileDescriptorPtr fd,
- const std::vector<Extent>& extents,
+ const google::protobuf::RepeatedPtrField<Extent>& extents,
uint32_t block_size) override;
bool Write(const void* bytes, size_t count) override;
bool EndImpl() override;
diff --git a/payload_consumer/bzip_extent_writer_unittest.cc b/payload_consumer/bzip_extent_writer_unittest.cc
index 8ac3e59..bf050ef 100644
--- a/payload_consumer/bzip_extent_writer_unittest.cc
+++ b/payload_consumer/bzip_extent_writer_unittest.cc
@@ -19,15 +19,17 @@
#include <fcntl.h>
#include <algorithm>
+#include <memory>
#include <string>
#include <vector>
-#include <brillo/make_unique_ptr.h>
#include <gtest/gtest.h>
#include "update_engine/common/test_utils.h"
#include "update_engine/common/utils.h"
+#include "update_engine/payload_generator/extent_ranges.h"
+using google::protobuf::RepeatedPtrField;
using std::min;
using std::string;
using std::vector;
@@ -55,11 +57,7 @@
};
TEST_F(BzipExtentWriterTest, SimpleTest) {
- vector<Extent> extents;
- Extent extent;
- extent.set_start_block(0);
- extent.set_num_blocks(1);
- extents.push_back(extent);
+ vector<Extent> extents = {ExtentForRange(0, 1)};
// 'echo test | bzip2 | hexdump' yields:
static const char test_uncompressed[] = "test\n";
@@ -70,9 +68,9 @@
0x22, 0x9c, 0x28, 0x48, 0x66, 0x61, 0xb8, 0xea, 0x00,
};
- BzipExtentWriter bzip_writer(
- brillo::make_unique_ptr(new DirectExtentWriter()));
- EXPECT_TRUE(bzip_writer.Init(fd_, extents, kBlockSize));
+ BzipExtentWriter bzip_writer(std::make_unique<DirectExtentWriter>());
+ EXPECT_TRUE(
+ bzip_writer.Init(fd_, {extents.begin(), extents.end()}, kBlockSize));
EXPECT_TRUE(bzip_writer.Write(test, sizeof(test)));
EXPECT_TRUE(bzip_writer.End());
@@ -102,15 +100,12 @@
for (size_t i = 0; i < decompressed_data.size(); ++i)
decompressed_data[i] = static_cast<uint8_t>("ABC\n"[i % 4]);
- vector<Extent> extents;
- Extent extent;
- extent.set_start_block(0);
- extent.set_num_blocks((kDecompressedLength + kBlockSize - 1) / kBlockSize);
- extents.push_back(extent);
+ vector<Extent> extents = {
+ ExtentForRange(0, (kDecompressedLength + kBlockSize - 1) / kBlockSize)};
- BzipExtentWriter bzip_writer(
- brillo::make_unique_ptr(new DirectExtentWriter()));
- EXPECT_TRUE(bzip_writer.Init(fd_, extents, kBlockSize));
+ BzipExtentWriter bzip_writer(std::make_unique<DirectExtentWriter>());
+ EXPECT_TRUE(
+ bzip_writer.Init(fd_, {extents.begin(), extents.end()}, kBlockSize));
brillo::Blob original_compressed_data = compressed_data;
for (brillo::Blob::size_type i = 0; i < compressed_data.size();
diff --git a/payload_consumer/cached_file_descriptor.cc b/payload_consumer/cached_file_descriptor.cc
new file mode 100644
index 0000000..7f2515e
--- /dev/null
+++ b/payload_consumer/cached_file_descriptor.cc
@@ -0,0 +1,98 @@
+//
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#include "update_engine/payload_consumer/cached_file_descriptor.h"
+
+#include <unistd.h>
+
+#include <algorithm>
+
+#include <base/logging.h>
+
+#include "update_engine/common/utils.h"
+
+namespace chromeos_update_engine {
+
+off64_t CachedFileDescriptor::Seek(off64_t offset, int whence) {
+ // Only support SEEK_SET and SEEK_CUR. I think these two would be enough. If
+ // we want to support SEEK_END then we have to figure out the size of the
+ // underlying file descriptor each time and it may not be a very good idea.
+ CHECK(whence == SEEK_SET || whence == SEEK_CUR);
+ off64_t next_offset = whence == SEEK_SET ? offset : offset_ + offset;
+
+ if (next_offset != offset_) {
+ // We sought somewhere other than what we are now. So we have to flush and
+ // move to the new offset.
+ if (!FlushCache()) {
+ return -1;
+ }
+ // Then we have to seek there.
+ if (fd_->Seek(next_offset, SEEK_SET) < 0) {
+ return -1;
+ }
+ offset_ = next_offset;
+ }
+ return offset_;
+}
+
+ssize_t CachedFileDescriptor::Write(const void* buf, size_t count) {
+ auto bytes = static_cast<const uint8_t*>(buf);
+ size_t total_bytes_wrote = 0;
+ while (total_bytes_wrote < count) {
+ auto bytes_to_cache =
+ std::min(count - total_bytes_wrote, cache_.size() - bytes_cached_);
+ if (bytes_to_cache > 0) { // Which means |cache_| is still have some space.
+ memcpy(cache_.data() + bytes_cached_,
+ bytes + total_bytes_wrote,
+ bytes_to_cache);
+ total_bytes_wrote += bytes_to_cache;
+ bytes_cached_ += bytes_to_cache;
+ }
+ if (bytes_cached_ == cache_.size()) {
+ // Cache is full; write it to the |fd_| as long as you can.
+ if (!FlushCache()) {
+ return -1;
+ }
+ }
+ }
+ offset_ += total_bytes_wrote;
+ return total_bytes_wrote;
+}
+
+bool CachedFileDescriptor::Flush() {
+ return FlushCache() && fd_->Flush();
+}
+
+bool CachedFileDescriptor::Close() {
+ offset_ = 0;
+ return FlushCache() && fd_->Close();
+}
+
+bool CachedFileDescriptor::FlushCache() {
+ size_t begin = 0;
+ while (begin < bytes_cached_) {
+ auto bytes_wrote = fd_->Write(cache_.data() + begin, bytes_cached_ - begin);
+ if (bytes_wrote < 0) {
+ PLOG(ERROR) << "Failed to flush cached data!";
+ return false;
+ }
+ begin += bytes_wrote;
+ }
+ bytes_cached_ = 0;
+ return true;
+}
+
+} // namespace chromeos_update_engine
diff --git a/payload_consumer/cached_file_descriptor.h b/payload_consumer/cached_file_descriptor.h
new file mode 100644
index 0000000..28c48f7
--- /dev/null
+++ b/payload_consumer/cached_file_descriptor.h
@@ -0,0 +1,76 @@
+//
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#ifndef UPDATE_ENGINE_PAYLOAD_CONSUMER_CACHED_FILE_DESCRIPTOR_H_
+#define UPDATE_ENGINE_PAYLOAD_CONSUMER_CACHED_FILE_DESCRIPTOR_H_
+
+#include <errno.h>
+#include <sys/types.h>
+
+#include <memory>
+#include <vector>
+
+#include <brillo/secure_blob.h>
+
+#include "update_engine/payload_consumer/file_descriptor.h"
+
+namespace chromeos_update_engine {
+
+class CachedFileDescriptor : public FileDescriptor {
+ public:
+ CachedFileDescriptor(FileDescriptorPtr fd, size_t cache_size) : fd_(fd) {
+ cache_.resize(cache_size);
+ }
+ ~CachedFileDescriptor() override = default;
+
+ bool Open(const char* path, int flags, mode_t mode) override {
+ return fd_->Open(path, flags, mode);
+ }
+ bool Open(const char* path, int flags) override {
+ return fd_->Open(path, flags);
+ }
+ ssize_t Read(void* buf, size_t count) override {
+ return fd_->Read(buf, count);
+ }
+ ssize_t Write(const void* buf, size_t count) override;
+ off64_t Seek(off64_t offset, int whence) override;
+ uint64_t BlockDevSize() override { return fd_->BlockDevSize(); }
+ bool BlkIoctl(int request,
+ uint64_t start,
+ uint64_t length,
+ int* result) override {
+ return fd_->BlkIoctl(request, start, length, result);
+ }
+ bool Flush() override;
+ bool Close() override;
+ bool IsSettingErrno() override { return fd_->IsSettingErrno(); }
+ bool IsOpen() override { return fd_->IsOpen(); }
+
+ private:
+ // Internal flush without the need to call |fd_->Flush()|.
+ bool FlushCache();
+
+ FileDescriptorPtr fd_;
+ brillo::Blob cache_;
+ size_t bytes_cached_{0};
+ off64_t offset_{0};
+
+ DISALLOW_COPY_AND_ASSIGN(CachedFileDescriptor);
+};
+
+} // namespace chromeos_update_engine
+
+#endif // UPDATE_ENGINE_PAYLOAD_CONSUMER_CACHED_FILE_DESCRIPTOR_H_
diff --git a/payload_consumer/cached_file_descriptor_unittest.cc b/payload_consumer/cached_file_descriptor_unittest.cc
new file mode 100644
index 0000000..6a6302a
--- /dev/null
+++ b/payload_consumer/cached_file_descriptor_unittest.cc
@@ -0,0 +1,204 @@
+//
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#include "update_engine/payload_consumer/cached_file_descriptor.h"
+
+#include <fcntl.h>
+
+#include <algorithm>
+#include <string>
+#include <vector>
+
+#include <gtest/gtest.h>
+
+#include "update_engine/common/test_utils.h"
+#include "update_engine/common/utils.h"
+
+using chromeos_update_engine::test_utils::ExpectVectorsEq;
+using std::min;
+using std::string;
+using std::vector;
+
+namespace chromeos_update_engine {
+
+namespace {
+const size_t kCacheSize = 100;
+const size_t kFileSize = 1024;
+const size_t kRandomIterations = 1000;
+} // namespace
+
+class CachedFileDescriptorTest : public ::testing::Test {
+ public:
+ void Open() {
+ cfd_.reset(new CachedFileDescriptor(fd_, kCacheSize));
+ EXPECT_TRUE(cfd_->Open(temp_file_.path().c_str(), O_RDWR, 0600));
+ }
+
+ void Write(uint8_t* buffer, size_t count) {
+ size_t total_bytes_wrote = 0;
+ while (total_bytes_wrote < count) {
+ auto bytes_wrote =
+ cfd_->Write(buffer + total_bytes_wrote, count - total_bytes_wrote);
+ ASSERT_NE(bytes_wrote, -1);
+ total_bytes_wrote += bytes_wrote;
+ }
+ }
+
+ void Close() { EXPECT_TRUE(cfd_->Close()); }
+
+ void SetUp() override {
+ brillo::Blob zero_blob(kFileSize, 0);
+ EXPECT_TRUE(utils::WriteFile(
+ temp_file_.path().c_str(), zero_blob.data(), zero_blob.size()));
+ Open();
+ }
+
+ void TearDown() override {
+ Close();
+ EXPECT_FALSE(cfd_->IsOpen());
+ }
+
+ protected:
+ FileDescriptorPtr fd_{new EintrSafeFileDescriptor};
+ test_utils::ScopedTempFile temp_file_{"CachedFileDescriptor-file.XXXXXX"};
+ int value_{1};
+ FileDescriptorPtr cfd_;
+};
+
+TEST_F(CachedFileDescriptorTest, IsOpenTest) {
+ EXPECT_TRUE(cfd_->IsOpen());
+}
+
+TEST_F(CachedFileDescriptorTest, SimpleWriteTest) {
+ EXPECT_EQ(cfd_->Seek(0, SEEK_SET), 0);
+ brillo::Blob blob_in(kFileSize, value_);
+ Write(blob_in.data(), blob_in.size());
+ EXPECT_TRUE(cfd_->Flush());
+
+ brillo::Blob blob_out;
+ EXPECT_TRUE(utils::ReadFile(temp_file_.path(), &blob_out));
+ EXPECT_EQ(blob_in, blob_out);
+}
+
+TEST_F(CachedFileDescriptorTest, OneBytePerWriteTest) {
+ EXPECT_EQ(cfd_->Seek(0, SEEK_SET), 0);
+ brillo::Blob blob_in(kFileSize, value_);
+ for (size_t idx = 0; idx < blob_in.size(); idx++) {
+ Write(&blob_in[idx], 1);
+ }
+ EXPECT_TRUE(cfd_->Flush());
+
+ brillo::Blob blob_out;
+ EXPECT_TRUE(utils::ReadFile(temp_file_.path(), &blob_out));
+ EXPECT_EQ(blob_in, blob_out);
+}
+
+TEST_F(CachedFileDescriptorTest, RandomWriteTest) {
+ EXPECT_EQ(cfd_->Seek(0, SEEK_SET), 0);
+
+ brillo::Blob blob_in(kFileSize, 0);
+ srand(time(nullptr));
+ uint32_t rand_seed;
+ for (size_t idx = 0; idx < kRandomIterations; idx++) {
+ // zero to full size available.
+ size_t start = rand_r(&rand_seed) % blob_in.size();
+ size_t size = rand_r(&rand_seed) % (blob_in.size() - start);
+ std::fill_n(&blob_in[start], size, idx % 256);
+ EXPECT_EQ(cfd_->Seek(start, SEEK_SET), static_cast<off64_t>(start));
+ Write(&blob_in[start], size);
+ }
+ EXPECT_TRUE(cfd_->Flush());
+
+ brillo::Blob blob_out;
+ EXPECT_TRUE(utils::ReadFile(temp_file_.path(), &blob_out));
+ EXPECT_EQ(blob_in, blob_out);
+}
+
+TEST_F(CachedFileDescriptorTest, SeekTest) {
+ EXPECT_EQ(cfd_->Seek(0, SEEK_SET), 0);
+ EXPECT_EQ(cfd_->Seek(1, SEEK_SET), 1);
+ EXPECT_EQ(cfd_->Seek(kFileSize - 1, SEEK_SET),
+ static_cast<off64_t>(kFileSize - 1));
+ EXPECT_EQ(cfd_->Seek(kFileSize, SEEK_SET), static_cast<off64_t>(kFileSize));
+ EXPECT_EQ(cfd_->Seek(kFileSize + 1, SEEK_SET),
+ static_cast<off64_t>(kFileSize + 1));
+
+ EXPECT_EQ(cfd_->Seek(0, SEEK_SET), 0);
+ EXPECT_EQ(cfd_->Seek(1, SEEK_CUR), 1);
+ EXPECT_EQ(cfd_->Seek(1, SEEK_CUR), 2);
+ EXPECT_EQ(cfd_->Seek(kFileSize - 1, SEEK_SET),
+ static_cast<off64_t>(kFileSize - 1));
+ EXPECT_EQ(cfd_->Seek(1, SEEK_CUR), static_cast<off64_t>(kFileSize));
+ EXPECT_EQ(cfd_->Seek(1, SEEK_CUR), static_cast<off64_t>(kFileSize + 1));
+}
+
+TEST_F(CachedFileDescriptorTest, NoFlushTest) {
+ EXPECT_EQ(cfd_->Seek(0, SEEK_SET), 0);
+ brillo::Blob blob_in(kFileSize, value_);
+ Write(blob_in.data(), blob_in.size());
+
+ brillo::Blob blob_out;
+ EXPECT_TRUE(utils::ReadFile(temp_file_.path(), &blob_out));
+ EXPECT_NE(blob_in, blob_out);
+}
+
+TEST_F(CachedFileDescriptorTest, CacheSizeWriteTest) {
+ off64_t seek = 10;
+ brillo::Blob blob_in(kFileSize, 0);
+ std::fill_n(&blob_in[seek], kCacheSize, value_);
+ // We are writing exactly one cache size; Then it should be commited.
+ EXPECT_EQ(cfd_->Seek(seek, SEEK_SET), seek);
+ Write(&blob_in[seek], kCacheSize);
+
+ brillo::Blob blob_out;
+ EXPECT_TRUE(utils::ReadFile(temp_file_.path(), &blob_out));
+ EXPECT_EQ(blob_in, blob_out);
+}
+
+TEST_F(CachedFileDescriptorTest, UnderCacheSizeWriteTest) {
+ off64_t seek = 100;
+ size_t less_than_cache_size = kCacheSize - 1;
+ EXPECT_EQ(cfd_->Seek(seek, SEEK_SET), seek);
+ brillo::Blob blob_in(kFileSize, 0);
+ std::fill_n(&blob_in[seek], less_than_cache_size, value_);
+ // We are writing less than one cache size; then it should not be commited.
+ Write(&blob_in[seek], less_than_cache_size);
+
+ // Revert the changes in |blob_in|.
+ std::fill_n(&blob_in[seek], less_than_cache_size, 0);
+ brillo::Blob blob_out;
+ EXPECT_TRUE(utils::ReadFile(temp_file_.path(), &blob_out));
+ EXPECT_EQ(blob_in, blob_out);
+}
+
+TEST_F(CachedFileDescriptorTest, SeekAfterWriteTest) {
+ off64_t seek = 100;
+ size_t less_than_cache_size = kCacheSize - 3;
+ EXPECT_EQ(cfd_->Seek(seek, SEEK_SET), seek);
+ brillo::Blob blob_in(kFileSize, 0);
+ std::fill_n(&blob_in[seek], less_than_cache_size, value_);
+ // We are writing less than one cache size; then it should not be commited.
+ Write(&blob_in[seek], less_than_cache_size);
+
+ // Then we seek, it should've written the cache after seek.
+ EXPECT_EQ(cfd_->Seek(200, SEEK_SET), 200);
+
+ brillo::Blob blob_out;
+ EXPECT_TRUE(utils::ReadFile(temp_file_.path(), &blob_out));
+ EXPECT_EQ(blob_in, blob_out);
+}
+
+} // namespace chromeos_update_engine
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index 4ac0c49..d5b0d58 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -29,13 +29,15 @@
#include <base/files/file_util.h>
#include <base/format_macros.h>
+#include <base/metrics/histogram_macros.h>
#include <base/strings/string_number_conversions.h>
#include <base/strings/string_util.h>
#include <base/strings/stringprintf.h>
+#include <base/time/time.h>
#include <brillo/data_encoding.h>
-#include <brillo/make_unique_ptr.h>
#include <bsdiff/bspatch.h>
#include <google/protobuf/repeated_field.h>
+#include <puffin/puffpatch.h>
#include "update_engine/common/constants.h"
#include "update_engine/common/hardware_interface.h"
@@ -43,7 +45,9 @@
#include "update_engine/common/subprocess.h"
#include "update_engine/common/terminator.h"
#include "update_engine/payload_consumer/bzip_extent_writer.h"
+#include "update_engine/payload_consumer/cached_file_descriptor.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
@@ -68,7 +72,7 @@
const uint64_t DeltaPerformer::kDeltaMetadataSignatureSizeSize = 4;
const uint64_t DeltaPerformer::kMaxPayloadHeaderSize = 24;
const uint64_t DeltaPerformer::kSupportedMajorPayloadVersion = 2;
-const uint32_t DeltaPerformer::kSupportedMinorPayloadVersion = 3;
+const uint32_t DeltaPerformer::kSupportedMinorPayloadVersion = 4;
const unsigned DeltaPerformer::kProgressLogMaxChunks = 10;
const unsigned DeltaPerformer::kProgressLogTimeoutSeconds = 30;
@@ -82,6 +86,8 @@
const int kUbiVolumeAttachTimeout = 5 * 60;
#endif
+const uint64_t kCacheSize = 1024 * 1024; // 1MB
+
FileDescriptorPtr CreateFileDescriptor(const char* path) {
FileDescriptorPtr ret;
#if USE_MTD
@@ -112,12 +118,20 @@
// Opens path for read/write. On success returns an open FileDescriptor
// and sets *err to 0. On failure, sets *err to errno and returns nullptr.
-FileDescriptorPtr OpenFile(const char* path, int mode, int* err) {
+FileDescriptorPtr OpenFile(const char* path,
+ int mode,
+ bool cache_writes,
+ int* err) {
// Try to mark the block device read-only based on the mode. Ignore any
// failure since this won't work when passing regular files.
- utils::SetBlockDeviceReadOnly(path, (mode & O_ACCMODE) == O_RDONLY);
+ bool read_only = (mode & O_ACCMODE) == O_RDONLY;
+ utils::SetBlockDeviceReadOnly(path, read_only);
FileDescriptorPtr fd = CreateFileDescriptor(path);
+ if (cache_writes && !read_only) {
+ fd = FileDescriptorPtr(new CachedFileDescriptor(fd, kCacheSize));
+ LOG(INFO) << "Caching writes.";
+ }
#if USE_MTD
// On NAND devices, we can either read, or write, but not both. So here we
// use O_WRONLY.
@@ -347,7 +361,7 @@
GetMinorVersion() != kInPlaceMinorPayloadVersion) {
source_path_ = install_part.source_path;
int err;
- source_fd_ = OpenFile(source_path_.c_str(), O_RDONLY, &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 "
@@ -359,7 +373,15 @@
target_path_ = install_part.target_path;
int err;
- target_fd_ = OpenFile(target_path_.c_str(), O_RDWR, &err);
+
+ int flags = O_RDWR;
+ if (!is_interactive_)
+ flags |= O_DSYNC;
+
+ LOG(INFO) << "Opening " << target_path_ << " partition with"
+ << (is_interactive_ ? "out" : "") << " O_DSYNC";
+
+ target_fd_ = OpenFile(target_path_.c_str(), flags, true, &err);
if (!target_fd_) {
LOG(ERROR) << "Unable to open target partition "
<< partition.partition_name() << " on slot "
@@ -585,6 +607,15 @@
return kMetadataParseSuccess;
}
+#define OP_DURATION_HISTOGRAM(_op_name, _start_time) \
+ LOCAL_HISTOGRAM_CUSTOM_TIMES( \
+ "UpdateEngine.DownloadAction.InstallOperation::" \
+ _op_name ".Duration", \
+ base::TimeTicks::Now() - _start_time, \
+ base::TimeDelta::FromMilliseconds(10), \
+ base::TimeDelta::FromMinutes(5), \
+ 20);
+
// Wrapper around write. Returns true if all requested bytes
// were written, or false on any error, regardless of progress
// and stores an action exit code in |error|.
@@ -719,32 +750,41 @@
ScopedTerminatorExitUnblocker exit_unblocker =
ScopedTerminatorExitUnblocker(); // Avoids a compiler unused var bug.
+ base::TimeTicks op_start_time = base::TimeTicks::Now();
+
bool op_result;
switch (op.type()) {
case InstallOperation::REPLACE:
case InstallOperation::REPLACE_BZ:
case InstallOperation::REPLACE_XZ:
op_result = PerformReplaceOperation(op);
+ OP_DURATION_HISTOGRAM("REPLACE", op_start_time);
break;
case InstallOperation::ZERO:
case InstallOperation::DISCARD:
op_result = PerformZeroOrDiscardOperation(op);
+ OP_DURATION_HISTOGRAM("ZERO_OR_DISCARD", op_start_time);
break;
case InstallOperation::MOVE:
op_result = PerformMoveOperation(op);
+ OP_DURATION_HISTOGRAM("MOVE", op_start_time);
break;
case InstallOperation::BSDIFF:
op_result = PerformBsdiffOperation(op);
+ OP_DURATION_HISTOGRAM("BSDIFF", op_start_time);
break;
case InstallOperation::SOURCE_COPY:
op_result = PerformSourceCopyOperation(op, error);
+ OP_DURATION_HISTOGRAM("SOURCE_COPY", op_start_time);
break;
case InstallOperation::SOURCE_BSDIFF:
+ case InstallOperation::BROTLI_BSDIFF:
op_result = PerformSourceBsdiffOperation(op, error);
+ OP_DURATION_HISTOGRAM("SOURCE_BSDIFF", op_start_time);
break;
case InstallOperation::PUFFDIFF:
- // TODO(ahassani): Later add PerformPuffdiffOperation(op, error);
- op_result = false;
+ op_result = PerformPuffDiffOperation(op, error);
+ OP_DURATION_HISTOGRAM("PUFFDIFF", op_start_time);
break;
default:
op_result = false;
@@ -752,6 +792,10 @@
if (!HandleOpResult(op_result, InstallOperationTypeName(op.type()), error))
return false;
+ if (!target_fd_->Flush()) {
+ return false;
+ }
+
next_operation_num_++;
UpdateOverallProgress(false, "Completed ");
CheckpointUpdateProgress();
@@ -919,9 +963,8 @@
}
// Setup the ExtentWriter stack based on the operation type.
- std::unique_ptr<ExtentWriter> writer =
- brillo::make_unique_ptr(new ZeroPadExtentWriter(
- brillo::make_unique_ptr(new DirectExtentWriter())));
+ std::unique_ptr<ExtentWriter> writer = std::make_unique<ZeroPadExtentWriter>(
+ std::make_unique<DirectExtentWriter>());
if (operation.type() == InstallOperation::REPLACE_BZ) {
writer.reset(new BzipExtentWriter(std::move(writer)));
@@ -929,13 +972,8 @@
writer.reset(new XzExtentWriter(std::move(writer)));
}
- // Create a vector of extents to pass to the ExtentWriter.
- vector<Extent> extents;
- for (int i = 0; i < operation.dst_extents_size(); i++) {
- extents.push_back(operation.dst_extents(i));
- }
-
- TEST_AND_RETURN_FALSE(writer->Init(target_fd_, extents, block_size_));
+ TEST_AND_RETURN_FALSE(
+ writer->Init(target_fd_, operation.dst_extents(), block_size_));
TEST_AND_RETURN_FALSE(writer->Write(buffer_.data(), operation.data_length()));
TEST_AND_RETURN_FALSE(writer->End());
@@ -1160,6 +1198,96 @@
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;
+}
+
+namespace {
+
+class BsdiffExtentFile : public bsdiff::FileInterface {
+ public:
+ BsdiffExtentFile(std::unique_ptr<ExtentReader> reader, size_t size)
+ : BsdiffExtentFile(std::move(reader), nullptr, size) {}
+ BsdiffExtentFile(std::unique_ptr<ExtentWriter> writer, size_t size)
+ : BsdiffExtentFile(nullptr, std::move(writer), size) {}
+
+ ~BsdiffExtentFile() override = default;
+
+ bool Read(void* buf, size_t count, size_t* bytes_read) override {
+ TEST_AND_RETURN_FALSE(reader_->Read(buf, count));
+ *bytes_read = count;
+ offset_ += count;
+ return true;
+ }
+
+ bool Write(const void* buf, size_t count, size_t* bytes_written) override {
+ TEST_AND_RETURN_FALSE(writer_->Write(buf, count));
+ *bytes_written = count;
+ offset_ += count;
+ return true;
+ }
+
+ bool Seek(off_t pos) override {
+ if (reader_ != nullptr) {
+ TEST_AND_RETURN_FALSE(reader_->Seek(pos));
+ offset_ = pos;
+ } else {
+ // For writes technically there should be no change of position, or it
+ // should be equivalent of current offset.
+ TEST_AND_RETURN_FALSE(offset_ == static_cast<uint64_t>(pos));
+ }
+ return true;
+ }
+
+ bool Close() override {
+ if (writer_ != nullptr) {
+ TEST_AND_RETURN_FALSE(writer_->End());
+ }
+ return true;
+ }
+
+ bool GetSize(uint64_t* size) override {
+ *size = size_;
+ return true;
+ }
+
+ private:
+ BsdiffExtentFile(std::unique_ptr<ExtentReader> reader,
+ std::unique_ptr<ExtentWriter> writer,
+ size_t size)
+ : reader_(std::move(reader)),
+ writer_(std::move(writer)),
+ size_(size),
+ offset_(0) {}
+
+ std::unique_ptr<ExtentReader> reader_;
+ std::unique_ptr<ExtentWriter> writer_;
+ uint64_t size_;
+ uint64_t offset_;
+
+ DISALLOW_COPY_AND_ASSIGN(BsdiffExtentFile);
+};
+
+} // namespace
+
bool DeltaPerformer::PerformSourceBsdiffOperation(
const InstallOperation& operation, ErrorCode* error) {
// Since we delete data off the beginning of the buffer as we use it,
@@ -1172,45 +1300,142 @@
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;
- TEST_AND_RETURN_FALSE(ExtentsToBsdiffPositionsString(operation.src_extents(),
- block_size_,
- operation.src_length(),
- &input_positions));
- string output_positions;
- TEST_AND_RETURN_FALSE(ExtentsToBsdiffPositionsString(operation.dst_extents(),
- block_size_,
- operation.dst_length(),
- &output_positions));
+ auto reader = std::make_unique<DirectExtentReader>();
+ TEST_AND_RETURN_FALSE(
+ reader->Init(source_fd_, operation.src_extents(), block_size_));
+ auto src_file = std::make_unique<BsdiffExtentFile>(
+ std::move(reader),
+ utils::BlocksInExtents(operation.src_extents()) * block_size_);
- TEST_AND_RETURN_FALSE(bsdiff::bspatch(source_path_.c_str(),
- target_path_.c_str(),
+ auto writer = std::make_unique<DirectExtentWriter>();
+ TEST_AND_RETURN_FALSE(
+ writer->Init(target_fd_, operation.dst_extents(), block_size_));
+ auto dst_file = std::make_unique<BsdiffExtentFile>(
+ std::move(writer),
+ utils::BlocksInExtents(operation.dst_extents()) * block_size_);
+
+ TEST_AND_RETURN_FALSE(bsdiff::bspatch(std::move(src_file),
+ std::move(dst_file),
buffer_.data(),
- buffer_.size(),
- input_positions.c_str(),
- output_positions.c_str()) == 0);
+ buffer_.size()) == 0);
+ DiscardBuffer(true, buffer_.size());
+ return true;
+}
+
+namespace {
+
+// A class to be passed to |puffpatch| for reading from |source_fd_| and writing
+// into |target_fd_|.
+class PuffinExtentStream : public puffin::StreamInterface {
+ public:
+ // Constructor for creating a stream for reading from an |ExtentReader|.
+ PuffinExtentStream(std::unique_ptr<ExtentReader> reader, size_t size)
+ : PuffinExtentStream(std::move(reader), nullptr, size) {}
+
+ // Constructor for creating a stream for writing to an |ExtentWriter|.
+ PuffinExtentStream(std::unique_ptr<ExtentWriter> writer, size_t size)
+ : PuffinExtentStream(nullptr, std::move(writer), size) {}
+
+ ~PuffinExtentStream() override = default;
+
+ bool GetSize(size_t* size) const override {
+ *size = size_;
+ return true;
+ }
+
+ bool GetOffset(size_t* offset) const override {
+ *offset = offset_;
+ return true;
+ }
+
+ bool Seek(size_t offset) override {
+ if (is_read_) {
+ TEST_AND_RETURN_FALSE(reader_->Seek(offset));
+ offset_ = offset;
+ } else {
+ // For writes technically there should be no change of position, or it
+ // should equivalent of current offset.
+ TEST_AND_RETURN_FALSE(offset_ == offset);
+ }
+ return true;
+ }
+
+ bool Read(void* buffer, size_t count) override {
+ TEST_AND_RETURN_FALSE(is_read_);
+ TEST_AND_RETURN_FALSE(reader_->Read(buffer, count));
+ offset_ += count;
+ return true;
+ }
+
+ bool Write(const void* buffer, size_t count) override {
+ TEST_AND_RETURN_FALSE(!is_read_);
+ TEST_AND_RETURN_FALSE(writer_->Write(buffer, count));
+ offset_ += count;
+ return true;
+ }
+
+ bool Close() override {
+ if (!is_read_) {
+ TEST_AND_RETURN_FALSE(writer_->End());
+ }
+ return true;
+ }
+
+ private:
+ PuffinExtentStream(std::unique_ptr<ExtentReader> reader,
+ std::unique_ptr<ExtentWriter> writer,
+ size_t size)
+ : reader_(std::move(reader)),
+ writer_(std::move(writer)),
+ size_(size),
+ offset_(0),
+ is_read_(reader_ ? true : false) {}
+
+ std::unique_ptr<ExtentReader> reader_;
+ std::unique_ptr<ExtentWriter> writer_;
+ uint64_t size_;
+ uint64_t offset_;
+ bool is_read_;
+
+ DISALLOW_COPY_AND_ASSIGN(PuffinExtentStream);
+};
+
+} // namespace
+
+bool DeltaPerformer::PerformPuffDiffOperation(const InstallOperation& operation,
+ ErrorCode* error) {
+ // Since we delete data off the beginning of the buffer as we use it,
+ // the data we need should be exactly at the beginning of the buffer.
+ TEST_AND_RETURN_FALSE(buffer_offset_ == operation.data_offset());
+ TEST_AND_RETURN_FALSE(buffer_.size() >= operation.data_length());
+
+ if (operation.has_src_sha256_hash()) {
+ TEST_AND_RETURN_FALSE(CalculateAndValidateSourceHash(operation, error));
+ }
+
+ auto reader = std::make_unique<DirectExtentReader>();
+ TEST_AND_RETURN_FALSE(
+ reader->Init(source_fd_, operation.src_extents(), block_size_));
+ puffin::UniqueStreamPtr src_stream(new PuffinExtentStream(
+ std::move(reader),
+ utils::BlocksInExtents(operation.src_extents()) * block_size_));
+
+ auto writer = std::make_unique<DirectExtentWriter>();
+ TEST_AND_RETURN_FALSE(
+ writer->Init(target_fd_, operation.dst_extents(), block_size_));
+ puffin::UniqueStreamPtr dst_stream(new PuffinExtentStream(
+ std::move(writer),
+ utils::BlocksInExtents(operation.dst_extents()) * block_size_));
+
+ const size_t kMaxCacheSize = 5 * 1024 * 1024; // Total 5MB cache.
+ TEST_AND_RETURN_FALSE(puffin::PuffPatch(std::move(src_stream),
+ std::move(dst_stream),
+ buffer_.data(),
+ buffer_.size(),
+ kMaxCacheSize));
DiscardBuffer(true, buffer_.size());
return true;
}
diff --git a/payload_consumer/delta_performer.h b/payload_consumer/delta_performer.h
index f363a4c..731e7f1 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>
@@ -79,13 +80,15 @@
HardwareInterface* hardware,
DownloadActionDelegate* download_delegate,
InstallPlan* install_plan,
- InstallPlan::Payload* payload)
+ InstallPlan::Payload* payload,
+ bool is_interactive)
: prefs_(prefs),
boot_control_(boot_control),
hardware_(hardware),
download_delegate_(download_delegate),
install_plan_(install_plan),
- payload_(payload) {}
+ payload_(payload),
+ is_interactive_(is_interactive) {}
// FileWriter's Write implementation where caller doesn't care about
// error codes.
@@ -162,9 +165,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 +245,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);
@@ -256,6 +263,8 @@
ErrorCode* error);
bool PerformSourceBsdiffOperation(const InstallOperation& operation,
ErrorCode* error);
+ bool PerformPuffDiffOperation(const InstallOperation& operation,
+ ErrorCode* error);
// Extracts the payload signature message from the blob on the |operation| if
// the offset matches the one specified by the manifest. Returns whether the
@@ -390,6 +399,9 @@
// The last progress chunk recorded.
unsigned last_progress_chunk_{0};
+ // If |true|, the update is user initiated (vs. periodic update checks).
+ bool is_interactive_{false};
+
// The timeout after which we should force emitting a progress log (constant),
// and the actual point in time for the next forced log to be emitted.
const base::TimeDelta forced_progress_log_wait_{
diff --git a/payload_consumer/delta_performer_integration_test.cc b/payload_consumer/delta_performer_integration_test.cc
index bc67d93..3572a6d 100644
--- a/payload_consumer/delta_performer_integration_test.cc
+++ b/payload_consumer/delta_performer_integration_test.cc
@@ -747,7 +747,8 @@
&state->fake_hardware_,
&state->mock_delegate_,
install_plan,
- &install_plan->payloads[0]);
+ &install_plan->payloads[0],
+ false /* is_interactive */);
string public_key_path = GetBuildArtifactsPath(kUnittestPublicKeyPath);
EXPECT_TRUE(utils::FileExists(public_key_path.c_str()));
(*performer)->set_public_key_path(public_key_path);
diff --git a/payload_consumer/delta_performer_unittest.cc b/payload_consumer/delta_performer_unittest.cc
index 3d967a8..092327b 100644
--- a/payload_consumer/delta_performer_unittest.cc
+++ b/payload_consumer/delta_performer_unittest.cc
@@ -87,6 +87,61 @@
0x00, 0x00, 0x59, 0x5a,
};
+const uint8_t src_deflates[] = {
+ /* raw 0 */ 0x11, 0x22,
+ /* deflate 2 */ 0x63, 0x64, 0x62, 0x66, 0x61, 0x05, 0x00,
+ /* raw 9 */ 0x33,
+ /* deflate 10 */ 0x03, 0x00,
+ /* raw 12 */
+ /* deflate 12 */ 0x63, 0x04, 0x00,
+ /* raw 15 */ 0x44, 0x55
+};
+
+const uint8_t dst_deflates[] = {
+ /* deflate 0 */ 0x63, 0x64, 0x62, 0x66, 0x61, 0x05, 0x00,
+ /* raw 7 */ 0x33, 0x66,
+ /* deflate 9 */ 0x01, 0x05, 0x00, 0xFA, 0xFF, 0x01, 0x02, 0x03, 0x04, 0x05,
+ /* deflate 19 */ 0x63, 0x04, 0x00
+};
+
+// To generate this patch either:
+// - Use puffin/src/patching_unittest.cc:TestPatching
+// Or
+// - Use the following approach:
+// * Make src_deflate a string of hex with only spaces. (e.g. "0XTE 0xST")
+// * echo "0XTE 0xST" | xxd -r -p > src.bin
+// * Find the location of deflates in src_deflates (in bytes) in the format of
+// "offset:length,...". (e.g. "2:7,10:2,12:3")
+// * Do previous three steps for dst_deflates.
+// * puffin --operation=puffdiff --src_file=src.bin --dst_file=dst.bin \
+// --src_deflates_byte="2:7,10:2,12:3" --dst_deflates_byte="0:7,9:10,19:3" \
+// --patch_file=patch.bin
+// * hexdump -ve '" " 12/1 "0x%02x, " "\n"' patch.bin
+const uint8_t puffdiff_patch[] = {
+ 0x50, 0x55, 0x46, 0x31, 0x00, 0x00, 0x00, 0x51, 0x08, 0x01, 0x12, 0x27,
+ 0x0A, 0x04, 0x08, 0x10, 0x10, 0x32, 0x0A, 0x04, 0x08, 0x50, 0x10, 0x0A,
+ 0x0A, 0x04, 0x08, 0x60, 0x10, 0x12, 0x12, 0x04, 0x08, 0x10, 0x10, 0x58,
+ 0x12, 0x04, 0x08, 0x78, 0x10, 0x28, 0x12, 0x05, 0x08, 0xA8, 0x01, 0x10,
+ 0x38, 0x18, 0x1F, 0x1A, 0x24, 0x0A, 0x02, 0x10, 0x32, 0x0A, 0x04, 0x08,
+ 0x48, 0x10, 0x50, 0x0A, 0x05, 0x08, 0x98, 0x01, 0x10, 0x12, 0x12, 0x02,
+ 0x10, 0x58, 0x12, 0x04, 0x08, 0x70, 0x10, 0x58, 0x12, 0x05, 0x08, 0xC8,
+ 0x01, 0x10, 0x38, 0x18, 0x21, 0x42, 0x53, 0x44, 0x49, 0x46, 0x46, 0x34,
+ 0x30, 0x38, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x34, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x21, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x42, 0x5A, 0x68, 0x39, 0x31, 0x41, 0x59, 0x26, 0x53, 0x59, 0x65,
+ 0x29, 0x8C, 0x9B, 0x00, 0x00, 0x03, 0x60, 0x40, 0x7A, 0x0E, 0x08, 0x00,
+ 0x40, 0x00, 0x20, 0x00, 0x21, 0x22, 0x9A, 0x3D, 0x4F, 0x50, 0x40, 0x0C,
+ 0x3B, 0xC7, 0x9B, 0xB2, 0x21, 0x0E, 0xE9, 0x15, 0x98, 0x7A, 0x7C, 0x5D,
+ 0xC9, 0x14, 0xE1, 0x42, 0x41, 0x94, 0xA6, 0x32, 0x6C, 0x42, 0x5A, 0x68,
+ 0x39, 0x31, 0x41, 0x59, 0x26, 0x53, 0x59, 0xF1, 0x20, 0x5F, 0x0D, 0x00,
+ 0x00, 0x02, 0x41, 0x15, 0x42, 0x08, 0x20, 0x00, 0x40, 0x00, 0x00, 0x02,
+ 0x40, 0x00, 0x20, 0x00, 0x22, 0x3D, 0x23, 0x10, 0x86, 0x03, 0x96, 0x54,
+ 0x11, 0x16, 0x5F, 0x17, 0x72, 0x45, 0x38, 0x50, 0x90, 0xF1, 0x20, 0x5F,
+ 0x0D, 0x42, 0x5A, 0x68, 0x39, 0x31, 0x41, 0x59, 0x26, 0x53, 0x59, 0x07,
+ 0xD4, 0xCB, 0x6E, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x20, 0x00,
+ 0x21, 0x18, 0x46, 0x82, 0xEE, 0x48, 0xA7, 0x0A, 0x12, 0x00, 0xFA, 0x99,
+ 0x6D, 0xC0};
+
} // namespace
class DeltaPerformerTest : public ::testing::Test {
@@ -334,7 +389,8 @@
&fake_hardware_,
&mock_delegate_,
&install_plan_,
- &payload_};
+ &payload_,
+ false /* is_interactive*/};
};
TEST_F(DeltaPerformerTest, FullPayloadWriteTest) {
@@ -375,7 +431,7 @@
testing::Mock::VerifyAndClearExpectations(&mock_delegate_);
EXPECT_CALL(mock_delegate_, ShouldCancel(_))
.WillOnce(
- testing::DoAll(testing::SetArgumentPointee<0>(ErrorCode::kError),
+ testing::DoAll(testing::SetArgPointee<0>(ErrorCode::kError),
testing::Return(true)));
ApplyPayload(payload_data, "/dev/null", false);
@@ -485,6 +541,32 @@
EXPECT_EQ(expected_data, ApplyPayload(payload_data, source_path, true));
}
+TEST_F(DeltaPerformerTest, PuffdiffOperationTest) {
+ AnnotatedOperation aop;
+ *(aop.op.add_src_extents()) = ExtentForRange(0, 1);
+ *(aop.op.add_dst_extents()) = ExtentForRange(0, 1);
+ brillo::Blob puffdiff_payload(std::begin(puffdiff_patch),
+ std::end(puffdiff_patch));
+ aop.op.set_data_offset(0);
+ aop.op.set_data_length(puffdiff_payload.size());
+ aop.op.set_type(InstallOperation::PUFFDIFF);
+ brillo::Blob src(std::begin(src_deflates), std::end(src_deflates));
+ src.resize(4096); // block size
+ brillo::Blob src_hash;
+ EXPECT_TRUE(HashCalculator::RawHashOfData(src, &src_hash));
+ aop.op.set_src_sha256_hash(src_hash.data(), src_hash.size());
+
+ brillo::Blob payload_data = GeneratePayload(puffdiff_payload, {aop}, false);
+
+ string source_path;
+ EXPECT_TRUE(utils::MakeTempFile("Source-XXXXXX", &source_path, nullptr));
+ ScopedPathUnlinker path_unlinker(source_path);
+ EXPECT_TRUE(utils::WriteFile(source_path.c_str(), src.data(), src.size()));
+
+ brillo::Blob dst(std::begin(dst_deflates), std::end(dst_deflates));
+ EXPECT_EQ(dst, ApplyPayload(payload_data, source_path, true));
+}
+
TEST_F(DeltaPerformerTest, SourceHashMismatchTest) {
brillo::Blob expected_data = {'f', 'o', 'o'};
brillo::Blob actual_data = {'b', 'a', 'r'};
diff --git a/payload_consumer/download_action.cc b/payload_consumer/download_action.cc
index cd25962..f1b6e33 100644
--- a/payload_consumer/download_action.cc
+++ b/payload_consumer/download_action.cc
@@ -20,9 +20,10 @@
#include <algorithm>
#include <string>
-#include <vector>
#include <base/files/file_path.h>
+#include <base/metrics/statistics_recorder.h>
+#include <base/strings/stringprintf.h>
#include "update_engine/common/action_pipe.h"
#include "update_engine/common/boot_control_interface.h"
@@ -35,7 +36,6 @@
using base::FilePath;
using std::string;
-using std::vector;
namespace chromeos_update_engine {
@@ -43,17 +43,21 @@
BootControlInterface* boot_control,
HardwareInterface* hardware,
SystemState* system_state,
- HttpFetcher* http_fetcher)
+ HttpFetcher* http_fetcher,
+ bool is_interactive)
: prefs_(prefs),
boot_control_(boot_control),
hardware_(hardware),
system_state_(system_state),
http_fetcher_(new MultiRangeHttpFetcher(http_fetcher)),
+ is_interactive_(is_interactive),
writer_(nullptr),
code_(ErrorCode::kSuccess),
delegate_(nullptr),
p2p_sharing_fd_(-1),
- p2p_visible_(true) {}
+ p2p_visible_(true) {
+ base::StatisticsRecorder::Initialize();
+}
DownloadAction::~DownloadAction() {}
@@ -241,8 +245,13 @@
if (writer_ && writer_ != delta_performer_.get()) {
LOG(INFO) << "Using writer for test.";
} else {
- delta_performer_.reset(new DeltaPerformer(
- prefs_, boot_control_, hardware_, delegate_, &install_plan_, payload_));
+ delta_performer_.reset(new DeltaPerformer(prefs_,
+ boot_control_,
+ hardware_,
+ delegate_,
+ &install_plan_,
+ payload_,
+ is_interactive_));
writer_ = delta_performer_.get();
}
if (system_state_ != nullptr) {
@@ -363,25 +372,33 @@
if (code == ErrorCode::kSuccess) {
if (delta_performer_ && !payload_->already_applied)
code = delta_performer_->VerifyPayload(payload_->hash, payload_->size);
- if (code != ErrorCode::kSuccess) {
+ if (code == ErrorCode::kSuccess) {
+ if (payload_ < &install_plan_.payloads.back() &&
+ system_state_->payload_state()->NextPayload()) {
+ LOG(INFO) << "Incrementing to next payload";
+ // No need to reset if this payload was already applied.
+ if (delta_performer_ && !payload_->already_applied)
+ DeltaPerformer::ResetUpdateProgress(prefs_, false);
+ // Start downloading next payload.
+ bytes_received_previous_payloads_ += payload_->size;
+ payload_++;
+ install_plan_.download_url =
+ system_state_->payload_state()->GetCurrentUrl();
+ StartDownloading();
+ return;
+ }
+ // Log UpdateEngine.DownloadAction.* histograms to help diagnose
+ // long-blocking oeprations.
+ std::string histogram_output;
+ base::StatisticsRecorder::WriteGraph(
+ "UpdateEngine.DownloadAction.", &histogram_output);
+ LOG(INFO) << histogram_output;
+ } else {
LOG(ERROR) << "Download of " << install_plan_.download_url
<< " failed due to payload verification error.";
// Delete p2p file, if applicable.
if (!p2p_file_id_.empty())
CloseP2PSharingFd(true);
- } else if (payload_ < &install_plan_.payloads.back() &&
- system_state_->payload_state()->NextPayload()) {
- LOG(INFO) << "Incrementing to next payload";
- // No need to reset if this payload was already applied.
- if (delta_performer_ && !payload_->already_applied)
- DeltaPerformer::ResetUpdateProgress(prefs_, false);
- // Start downloading next payload.
- bytes_received_previous_payloads_ += payload_->size;
- payload_++;
- install_plan_.download_url =
- system_state_->payload_state()->GetCurrentUrl();
- StartDownloading();
- return;
}
}
diff --git a/payload_consumer/download_action.h b/payload_consumer/download_action.h
index 786db20..81d7333 100644
--- a/payload_consumer/download_action.h
+++ b/payload_consumer/download_action.h
@@ -73,12 +73,13 @@
// Takes ownership of the passed in HttpFetcher. Useful for testing.
// A good calling pattern is:
// DownloadAction(prefs, boot_contol, hardware, system_state,
- // new WhateverHttpFetcher);
+ // new WhateverHttpFetcher, false);
DownloadAction(PrefsInterface* prefs,
BootControlInterface* boot_control,
HardwareInterface* hardware,
SystemState* system_state,
- HttpFetcher* http_fetcher);
+ HttpFetcher* http_fetcher,
+ bool is_interactive);
~DownloadAction() override;
// InstallPlanAction overrides.
@@ -154,6 +155,11 @@
// Pointer to the MultiRangeHttpFetcher that does the http work.
std::unique_ptr<MultiRangeHttpFetcher> http_fetcher_;
+ // If |true|, the update is user initiated (vs. periodic update checks). Hence
+ // the |delta_performer_| can decide not to use O_DSYNC flag for faster
+ // update.
+ bool is_interactive_;
+
// The FileWriter that downloaded data should be written to. It will
// either point to *decompressing_file_writer_ or *delta_performer_.
FileWriter* writer_;
diff --git a/payload_consumer/download_action_unittest.cc b/payload_consumer/download_action_unittest.cc
index b04db49..21ce461 100644
--- a/payload_consumer/download_action_unittest.cc
+++ b/payload_consumer/download_action_unittest.cc
@@ -22,7 +22,6 @@
#include <memory>
#include <string>
#include <utility>
-#include <vector>
#include <base/bind.h>
#include <base/files/file_path.h>
@@ -51,7 +50,6 @@
using base::WriteFile;
using std::string;
using std::unique_ptr;
-using std::vector;
using test_utils::ScopedTempFile;
using testing::AtLeast;
using testing::InSequence;
@@ -166,7 +164,8 @@
fake_system_state.boot_control(),
fake_system_state.hardware(),
&fake_system_state,
- http_fetcher);
+ http_fetcher,
+ false /* is_interactive */);
download_action.SetTestFileWriter(&writer);
BondActions(&feeder_action, &download_action);
MockDownloadActionDelegate download_delegate;
@@ -279,7 +278,8 @@
fake_system_state.boot_control(),
fake_system_state.hardware(),
&fake_system_state,
- http_fetcher);
+ http_fetcher,
+ false /* is_interactive */);
download_action.SetTestFileWriter(&mock_file_writer);
BondActions(&feeder_action, &download_action);
MockDownloadActionDelegate download_delegate;
@@ -368,7 +368,8 @@
fake_system_state_.boot_control(),
fake_system_state_.hardware(),
&fake_system_state_,
- new MockHttpFetcher(data.data(), data.size(), nullptr));
+ new MockHttpFetcher(data.data(), data.size(), nullptr),
+ false /* is_interactive */);
download_action.SetTestFileWriter(&writer);
MockDownloadActionDelegate download_delegate;
if (use_download_delegate) {
@@ -469,7 +470,8 @@
fake_system_state_.boot_control(),
fake_system_state_.hardware(),
&fake_system_state_,
- new MockHttpFetcher("x", 1, nullptr));
+ new MockHttpFetcher("x", 1, nullptr),
+ false /* is_interactive */);
download_action.SetTestFileWriter(&writer);
DownloadActionTestAction test_action;
@@ -558,7 +560,8 @@
fake_system_state_.boot_control(),
fake_system_state_.hardware(),
&fake_system_state_,
- http_fetcher_));
+ http_fetcher_,
+ false /* is_interactive */));
download_action_->SetTestFileWriter(&writer);
BondActions(&feeder_action, download_action_.get());
DownloadActionTestProcessorDelegate delegate(ErrorCode::kSuccess);
diff --git a/payload_consumer/extent_reader.cc b/payload_consumer/extent_reader.cc
new file mode 100644
index 0000000..96ea918
--- /dev/null
+++ b/payload_consumer/extent_reader.cc
@@ -0,0 +1,98 @@
+//
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#include "update_engine/payload_consumer/extent_reader.h"
+
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "update_engine/common/utils.h"
+#include "update_engine/payload_consumer/payload_constants.h"
+
+using google::protobuf::RepeatedPtrField;
+
+namespace chromeos_update_engine {
+
+bool DirectExtentReader::Init(FileDescriptorPtr fd,
+ const RepeatedPtrField<Extent>& extents,
+ uint32_t block_size) {
+ fd_ = fd;
+ extents_ = extents;
+ block_size_ = block_size;
+ cur_extent_ = extents_.begin();
+
+ extents_upper_bounds_.reserve(extents_.size() + 1);
+ // We add this pad as the first element to not bother with boundary checks
+ // later.
+ extents_upper_bounds_.emplace_back(0);
+ for (const auto& extent : extents_) {
+ total_size_ += extent.num_blocks() * block_size_;
+ extents_upper_bounds_.emplace_back(total_size_);
+ }
+ return true;
+}
+
+bool DirectExtentReader::Seek(uint64_t offset) {
+ TEST_AND_RETURN_FALSE(offset <= total_size_);
+ if (offset_ == offset) {
+ return true;
+ }
+ // The first item is zero and upper_bound never returns it because it always
+ // return the item which is greater than the given value.
+ auto extent_idx = std::upper_bound(
+ extents_upper_bounds_.begin(), extents_upper_bounds_.end(), offset) -
+ extents_upper_bounds_.begin() - 1;
+ cur_extent_ = std::next(extents_.begin(), extent_idx);
+ offset_ = offset;
+ cur_extent_bytes_read_ = offset_ - extents_upper_bounds_[extent_idx];
+ return true;
+}
+
+bool DirectExtentReader::Read(void* buffer, size_t count) {
+ auto bytes = reinterpret_cast<uint8_t*>(buffer);
+ uint64_t bytes_read = 0;
+ while (bytes_read < count) {
+ if (cur_extent_ == extents_.end()) {
+ TEST_AND_RETURN_FALSE(bytes_read == count);
+ }
+ uint64_t cur_extent_bytes_left =
+ cur_extent_->num_blocks() * block_size_ - cur_extent_bytes_read_;
+ uint64_t bytes_to_read =
+ std::min(count - bytes_read, cur_extent_bytes_left);
+
+ ssize_t out_bytes_read;
+ TEST_AND_RETURN_FALSE(utils::PReadAll(
+ fd_,
+ bytes + bytes_read,
+ bytes_to_read,
+ cur_extent_->start_block() * block_size_ + cur_extent_bytes_read_,
+ &out_bytes_read));
+ TEST_AND_RETURN_FALSE(out_bytes_read ==
+ static_cast<ssize_t>(bytes_to_read));
+
+ bytes_read += bytes_to_read;
+ cur_extent_bytes_read_ += bytes_to_read;
+ offset_ += bytes_to_read;
+ if (cur_extent_bytes_read_ == cur_extent_->num_blocks() * block_size_) {
+ // We have to advance the cur_extent_;
+ cur_extent_++;
+ cur_extent_bytes_read_ = 0;
+ }
+ }
+ return true;
+}
+
+} // namespace chromeos_update_engine
diff --git a/payload_consumer/extent_reader.h b/payload_consumer/extent_reader.h
new file mode 100644
index 0000000..3f9e4c8
--- /dev/null
+++ b/payload_consumer/extent_reader.h
@@ -0,0 +1,82 @@
+//
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#ifndef UPDATE_ENGINE_PAYLOAD_CONSUMER_EXTENT_READER_H_
+#define UPDATE_ENGINE_PAYLOAD_CONSUMER_EXTENT_READER_H_
+
+#include <vector>
+
+#include "update_engine/payload_consumer/file_descriptor.h"
+#include "update_engine/update_metadata.pb.h"
+
+namespace chromeos_update_engine {
+
+// ExtentReader is an abstract class with reads from a given file descriptor at
+// the extents given.
+class ExtentReader {
+ public:
+ virtual ~ExtentReader() = default;
+
+ // Initializes |ExtentReader|
+ virtual bool Init(FileDescriptorPtr fd,
+ const google::protobuf::RepeatedPtrField<Extent>& extents,
+ uint32_t block_size) = 0;
+
+ // Seeks to the given |offset| assuming all extents are concatenated together.
+ virtual bool Seek(uint64_t offset) = 0;
+
+ // Returns true on success.
+ virtual bool Read(void* buffer, size_t count) = 0;
+};
+
+// DirectExtentReader is probably the simplest ExtentReader implementation.
+// It reads the data directly from the extents.
+class DirectExtentReader : public ExtentReader {
+ public:
+ DirectExtentReader() = default;
+ ~DirectExtentReader() override = default;
+
+ bool Init(FileDescriptorPtr fd,
+ const google::protobuf::RepeatedPtrField<Extent>& extents,
+ uint32_t block_size) override;
+ bool Seek(uint64_t offset) override;
+ bool Read(void* bytes, size_t count) override;
+
+ private:
+ FileDescriptorPtr fd_{nullptr};
+ google::protobuf::RepeatedPtrField<Extent> extents_;
+ size_t block_size_{0};
+
+ // Current extent being read from |fd_|.
+ google::protobuf::RepeatedPtrField<Extent>::iterator cur_extent_;
+
+ // Bytes read from |cur_extent_| thus far.
+ uint64_t cur_extent_bytes_read_{0};
+
+ // Offset assuming all extents are concatenated.
+ uint64_t offset_{0};
+
+ // The accelaring upper bounds for |extents_| if we assume all extents are
+ // concatenated.
+ std::vector<uint64_t> extents_upper_bounds_;
+ uint64_t total_size_{0};
+
+ DISALLOW_COPY_AND_ASSIGN(DirectExtentReader);
+};
+
+} // namespace chromeos_update_engine
+
+#endif // UPDATE_ENGINE_PAYLOAD_CONSUMER_EXTENT_READER_H_
diff --git a/payload_consumer/extent_reader_unittest.cc b/payload_consumer/extent_reader_unittest.cc
new file mode 100644
index 0000000..b7059bc
--- /dev/null
+++ b/payload_consumer/extent_reader_unittest.cc
@@ -0,0 +1,170 @@
+//
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#include "update_engine/payload_consumer/extent_reader.h"
+
+#include <fcntl.h>
+
+#include <algorithm>
+#include <string>
+#include <vector>
+
+#include <brillo/secure_blob.h>
+#include <gtest/gtest.h>
+
+#include "update_engine/common/test_utils.h"
+#include "update_engine/common/utils.h"
+#include "update_engine/payload_consumer/file_descriptor.h"
+#include "update_engine/payload_consumer/payload_constants.h"
+#include "update_engine/payload_generator/extent_ranges.h"
+#include "update_engine/payload_generator/extent_utils.h"
+
+using chromeos_update_engine::test_utils::ExpectVectorsEq;
+using std::min;
+using std::string;
+using std::vector;
+
+namespace chromeos_update_engine {
+
+namespace {
+const size_t kBlockSize = 8;
+const size_t kRandomIterations = 1000;
+} // namespace
+
+class ExtentReaderTest : public ::testing::Test {
+ protected:
+ void SetUp() override {
+ sample_.resize(4096 * 10);
+ srand(time(nullptr));
+ unsigned int rand_seed;
+ for (size_t i = 0; i < sample_.size(); i++) {
+ sample_[i] = rand_r(&rand_seed) % 256;
+ }
+ ASSERT_TRUE(utils::WriteFile(
+ temp_file_.path().c_str(), sample_.data(), sample_.size()));
+
+ fd_.reset(new EintrSafeFileDescriptor());
+ ASSERT_TRUE(fd_->Open(temp_file_.path().c_str(), O_RDONLY, 0600));
+ }
+ void TearDown() override { fd_->Close(); }
+
+ void ReadExtents(vector<Extent> extents, brillo::Blob* blob) {
+ blob->clear();
+ for (const auto& extent : extents) {
+ blob->insert(
+ blob->end(),
+ &sample_[extent.start_block() * kBlockSize],
+ &sample_[(extent.start_block() + extent.num_blocks()) * kBlockSize]);
+ }
+ }
+
+ FileDescriptorPtr fd_;
+ test_utils::ScopedTempFile temp_file_{"ExtentReaderTest-file.XXXXXX"};
+ brillo::Blob sample_;
+};
+
+TEST_F(ExtentReaderTest, SimpleTest) {
+ vector<Extent> extents = {ExtentForRange(1, 1)};
+ DirectExtentReader reader;
+ EXPECT_TRUE(reader.Init(fd_, {extents.begin(), extents.end()}, kBlockSize));
+ EXPECT_TRUE(reader.Seek(0));
+ brillo::Blob blob1(utils::BlocksInExtents(extents) * kBlockSize);
+ EXPECT_TRUE(reader.Read(blob1.data(), blob1.size()));
+ brillo::Blob blob2;
+ ReadExtents(extents, &blob2);
+ ExpectVectorsEq(blob1, blob2);
+}
+
+TEST_F(ExtentReaderTest, ZeroExtentLengthTest) {
+ vector<Extent> extents = {ExtentForRange(1, 0)};
+ DirectExtentReader reader;
+ EXPECT_TRUE(reader.Init(fd_, {extents.begin(), extents.end()}, kBlockSize));
+ EXPECT_TRUE(reader.Seek(0));
+ brillo::Blob blob(1);
+ EXPECT_TRUE(reader.Read(blob.data(), 0));
+ EXPECT_FALSE(reader.Read(blob.data(), 1));
+}
+
+TEST_F(ExtentReaderTest, NoExtentTest) {
+ DirectExtentReader reader;
+ EXPECT_TRUE(reader.Init(fd_, {}, kBlockSize));
+ EXPECT_TRUE(reader.Seek(0));
+ brillo::Blob blob(1);
+ EXPECT_TRUE(reader.Read(blob.data(), 0));
+ EXPECT_FALSE(reader.Read(blob.data(), 1));
+}
+
+TEST_F(ExtentReaderTest, OverflowExtentTest) {
+ vector<Extent> extents = {ExtentForRange(1, 1)};
+ DirectExtentReader reader;
+ EXPECT_TRUE(reader.Init(fd_, {extents.begin(), extents.end()}, kBlockSize));
+ EXPECT_TRUE(reader.Seek(0));
+ brillo::Blob blob(utils::BlocksInExtents(extents) * kBlockSize + 1);
+ EXPECT_FALSE(reader.Read(blob.data(), blob.size()));
+}
+
+TEST_F(ExtentReaderTest, SeekOverflow1Test) {
+ vector<Extent> extents = {ExtentForRange(1, 0)};
+ DirectExtentReader reader;
+ EXPECT_TRUE(reader.Init(fd_, {extents.begin(), extents.end()}, kBlockSize));
+ EXPECT_TRUE(reader.Seek(0));
+ EXPECT_FALSE(reader.Seek(1));
+}
+
+TEST_F(ExtentReaderTest, SeekOverflow2Test) {
+ DirectExtentReader reader;
+ reader.Init(fd_, {}, kBlockSize);
+ EXPECT_TRUE(reader.Seek(0));
+ EXPECT_FALSE(reader.Seek(1));
+}
+
+TEST_F(ExtentReaderTest, SeekOverflow3Test) {
+ vector<Extent> extents = {ExtentForRange(1, 1)};
+ DirectExtentReader reader;
+ EXPECT_TRUE(reader.Init(fd_, {extents.begin(), extents.end()}, kBlockSize));
+ // Seek to the end of the extents should be fine as long as nothing is read.
+ EXPECT_TRUE(reader.Seek(kBlockSize));
+ EXPECT_FALSE(reader.Seek(kBlockSize + 1));
+}
+
+TEST_F(ExtentReaderTest, RandomReadTest) {
+ vector<Extent> extents = {ExtentForRange(0, 0),
+ ExtentForRange(1, 1),
+ ExtentForRange(3, 0),
+ ExtentForRange(4, 2),
+ ExtentForRange(7, 1)};
+ DirectExtentReader reader;
+ EXPECT_TRUE(reader.Init(fd_, {extents.begin(), extents.end()}, kBlockSize));
+
+ brillo::Blob result;
+ ReadExtents(extents, &result);
+
+ brillo::Blob blob(utils::BlocksInExtents(extents) * kBlockSize);
+ srand(time(nullptr));
+ uint32_t rand_seed;
+ for (size_t idx = 0; idx < kRandomIterations; idx++) {
+ // zero to full size available.
+ size_t start = rand_r(&rand_seed) % blob.size();
+ size_t size = rand_r(&rand_seed) % (blob.size() - start);
+ EXPECT_TRUE(reader.Seek(start));
+ EXPECT_TRUE(reader.Read(blob.data(), size));
+ for (size_t i = 0; i < size; i++) {
+ ASSERT_EQ(blob[i], result[start + i]);
+ }
+ }
+}
+
+} // namespace chromeos_update_engine
diff --git a/payload_consumer/extent_writer.cc b/payload_consumer/extent_writer.cc
index 5501e22..c5776ec 100644
--- a/payload_consumer/extent_writer.cc
+++ b/payload_consumer/extent_writer.cc
@@ -34,21 +34,19 @@
return true;
const char* c_bytes = reinterpret_cast<const char*>(bytes);
size_t bytes_written = 0;
- while (count - bytes_written > 0) {
- TEST_AND_RETURN_FALSE(next_extent_index_ < extents_.size());
- uint64_t bytes_remaining_next_extent =
- extents_[next_extent_index_].num_blocks() * block_size_ -
- extent_bytes_written_;
- CHECK_NE(bytes_remaining_next_extent, static_cast<uint64_t>(0));
+ while (bytes_written < count) {
+ TEST_AND_RETURN_FALSE(cur_extent_ != extents_.end());
+ uint64_t bytes_remaining_cur_extent =
+ cur_extent_->num_blocks() * block_size_ - extent_bytes_written_;
+ CHECK_NE(bytes_remaining_cur_extent, static_cast<uint64_t>(0));
size_t bytes_to_write =
static_cast<size_t>(min(static_cast<uint64_t>(count - bytes_written),
- bytes_remaining_next_extent));
+ bytes_remaining_cur_extent));
TEST_AND_RETURN_FALSE(bytes_to_write > 0);
- if (extents_[next_extent_index_].start_block() != kSparseHole) {
+ if (cur_extent_->start_block() != kSparseHole) {
const off64_t offset =
- extents_[next_extent_index_].start_block() * block_size_ +
- extent_bytes_written_;
+ cur_extent_->start_block() * block_size_ + extent_bytes_written_;
TEST_AND_RETURN_FALSE_ERRNO(fd_->Seek(offset, SEEK_SET) !=
static_cast<off64_t>(-1));
TEST_AND_RETURN_FALSE(
@@ -56,13 +54,12 @@
}
bytes_written += bytes_to_write;
extent_bytes_written_ += bytes_to_write;
- if (bytes_remaining_next_extent == bytes_to_write) {
+ if (bytes_remaining_cur_extent == bytes_to_write) {
// We filled this extent
- CHECK_EQ(extent_bytes_written_,
- extents_[next_extent_index_].num_blocks() * block_size_);
+ CHECK_EQ(extent_bytes_written_, cur_extent_->num_blocks() * block_size_);
// move to next extent
extent_bytes_written_ = 0;
- next_extent_index_++;
+ cur_extent_++;
}
}
return true;
diff --git a/payload_consumer/extent_writer.h b/payload_consumer/extent_writer.h
index 6484ebf..2c15861 100644
--- a/payload_consumer/extent_writer.h
+++ b/payload_consumer/extent_writer.h
@@ -17,7 +17,8 @@
#ifndef UPDATE_ENGINE_PAYLOAD_CONSUMER_EXTENT_WRITER_H_
#define UPDATE_ENGINE_PAYLOAD_CONSUMER_EXTENT_WRITER_H_
-#include <vector>
+#include <memory>
+#include <utility>
#include <base/logging.h>
#include <brillo/secure_blob.h>
@@ -40,7 +41,7 @@
// Returns true on success.
virtual bool Init(FileDescriptorPtr fd,
- const std::vector<Extent>& extents,
+ const google::protobuf::RepeatedPtrField<Extent>& extents,
uint32_t block_size) = 0;
// Returns true on success.
@@ -66,11 +67,12 @@
~DirectExtentWriter() override = default;
bool Init(FileDescriptorPtr fd,
- const std::vector<Extent>& extents,
+ const google::protobuf::RepeatedPtrField<Extent>& extents,
uint32_t block_size) override {
fd_ = fd;
block_size_ = block_size;
extents_ = extents;
+ cur_extent_ = extents_.begin();
return true;
}
bool Write(const void* bytes, size_t count) override;
@@ -80,11 +82,11 @@
FileDescriptorPtr fd_{nullptr};
size_t block_size_{0};
- // Bytes written into next_extent_index_ thus far
+ // Bytes written into |cur_extent_| thus far.
uint64_t extent_bytes_written_{0};
- std::vector<Extent> extents_;
- // The next call to write should correspond to extents_[next_extent_index_]
- std::vector<Extent>::size_type next_extent_index_{0};
+ google::protobuf::RepeatedPtrField<Extent> extents_;
+ // The next call to write should correspond to |cur_extents_|.
+ google::protobuf::RepeatedPtrField<Extent>::iterator cur_extent_;
};
// Takes an underlying ExtentWriter to which all operations are delegated.
@@ -100,7 +102,7 @@
~ZeroPadExtentWriter() override = default;
bool Init(FileDescriptorPtr fd,
- const std::vector<Extent>& extents,
+ const google::protobuf::RepeatedPtrField<Extent>& extents,
uint32_t block_size) override {
block_size_ = block_size;
return underlying_extent_writer_->Init(fd, extents, block_size);
diff --git a/payload_consumer/extent_writer_unittest.cc b/payload_consumer/extent_writer_unittest.cc
index 24d238e..48b27cb 100644
--- a/payload_consumer/extent_writer_unittest.cc
+++ b/payload_consumer/extent_writer_unittest.cc
@@ -19,16 +19,17 @@
#include <fcntl.h>
#include <algorithm>
+#include <memory>
#include <string>
#include <vector>
-#include <brillo/make_unique_ptr.h>
#include <brillo/secure_blob.h>
#include <gtest/gtest.h>
#include "update_engine/common/test_utils.h"
#include "update_engine/common/utils.h"
#include "update_engine/payload_consumer/payload_constants.h"
+#include "update_engine/payload_generator/extent_ranges.h"
using chromeos_update_engine::test_utils::ExpectVectorsEq;
using std::min;
@@ -65,16 +66,11 @@
};
TEST_F(ExtentWriterTest, SimpleTest) {
- vector<Extent> extents;
- Extent extent;
- extent.set_start_block(1);
- extent.set_num_blocks(1);
- extents.push_back(extent);
-
+ vector<Extent> extents = {ExtentForRange(1, 1)};
const string bytes = "1234";
-
DirectExtentWriter direct_writer;
- EXPECT_TRUE(direct_writer.Init(fd_, extents, kBlockSize));
+ EXPECT_TRUE(
+ direct_writer.Init(fd_, {extents.begin(), extents.end()}, kBlockSize));
EXPECT_TRUE(direct_writer.Write(bytes.data(), bytes.size()));
EXPECT_TRUE(direct_writer.End());
@@ -91,14 +87,10 @@
}
TEST_F(ExtentWriterTest, ZeroLengthTest) {
- vector<Extent> extents;
- Extent extent;
- extent.set_start_block(1);
- extent.set_num_blocks(1);
- extents.push_back(extent);
-
+ vector<Extent> extents = {ExtentForRange(1, 1)};
DirectExtentWriter direct_writer;
- EXPECT_TRUE(direct_writer.Init(fd_, extents, kBlockSize));
+ EXPECT_TRUE(
+ direct_writer.Init(fd_, {extents.begin(), extents.end()}, kBlockSize));
EXPECT_TRUE(direct_writer.Write(nullptr, 0));
EXPECT_TRUE(direct_writer.End());
}
@@ -117,23 +109,14 @@
void ExtentWriterTest::WriteAlignedExtents(size_t chunk_size,
size_t first_chunk_size) {
- vector<Extent> extents;
- Extent extent;
- extent.set_start_block(1);
- extent.set_num_blocks(1);
- extents.push_back(extent);
- extent.set_start_block(0);
- extent.set_num_blocks(1);
- extents.push_back(extent);
- extent.set_start_block(2);
- extent.set_num_blocks(1);
- extents.push_back(extent);
-
+ vector<Extent> extents = {
+ ExtentForRange(1, 1), ExtentForRange(0, 1), ExtentForRange(2, 1)};
brillo::Blob data(kBlockSize * 3);
test_utils::FillWithData(&data);
DirectExtentWriter direct_writer;
- EXPECT_TRUE(direct_writer.Init(fd_, extents, kBlockSize));
+ EXPECT_TRUE(
+ direct_writer.Init(fd_, {extents.begin(), extents.end()}, kBlockSize));
size_t bytes_written = 0;
while (bytes_written < data.size()) {
@@ -172,22 +155,14 @@
}
void ExtentWriterTest::TestZeroPad(bool aligned_size) {
- vector<Extent> extents;
- Extent extent;
- extent.set_start_block(1);
- extent.set_num_blocks(1);
- extents.push_back(extent);
- extent.set_start_block(0);
- extent.set_num_blocks(1);
- extents.push_back(extent);
-
+ vector<Extent> extents = {ExtentForRange(1, 1), ExtentForRange(0, 1)};
brillo::Blob data(kBlockSize * 2);
test_utils::FillWithData(&data);
- ZeroPadExtentWriter zero_pad_writer(
- brillo::make_unique_ptr(new DirectExtentWriter()));
+ ZeroPadExtentWriter zero_pad_writer(std::make_unique<DirectExtentWriter>());
- EXPECT_TRUE(zero_pad_writer.Init(fd_, extents, kBlockSize));
+ EXPECT_TRUE(
+ zero_pad_writer.Init(fd_, {extents.begin(), extents.end()}, kBlockSize));
size_t bytes_to_write = data.size();
const size_t missing_bytes = (aligned_size ? 0 : 9);
bytes_to_write -= missing_bytes;
@@ -216,17 +191,9 @@
}
TEST_F(ExtentWriterTest, SparseFileTest) {
- vector<Extent> extents;
- Extent extent;
- extent.set_start_block(1);
- extent.set_num_blocks(1);
- extents.push_back(extent);
- extent.set_start_block(kSparseHole);
- extent.set_num_blocks(2);
- extents.push_back(extent);
- extent.set_start_block(0);
- extent.set_num_blocks(1);
- extents.push_back(extent);
+ vector<Extent> extents = {ExtentForRange(1, 1),
+ ExtentForRange(kSparseHole, 2),
+ ExtentForRange(0, 1)};
const int block_count = 4;
const int on_disk_count = 2;
@@ -234,7 +201,8 @@
test_utils::FillWithData(&data);
DirectExtentWriter direct_writer;
- EXPECT_TRUE(direct_writer.Init(fd_, extents, kBlockSize));
+ EXPECT_TRUE(
+ direct_writer.Init(fd_, {extents.begin(), extents.end()}, kBlockSize));
size_t bytes_written = 0;
while (bytes_written < (block_count * kBlockSize)) {
diff --git a/payload_consumer/fake_extent_writer.h b/payload_consumer/fake_extent_writer.h
index 762c6d5..4418a9e 100644
--- a/payload_consumer/fake_extent_writer.h
+++ b/payload_consumer/fake_extent_writer.h
@@ -18,7 +18,6 @@
#define UPDATE_ENGINE_PAYLOAD_CONSUMER_FAKE_EXTENT_WRITER_H_
#include <memory>
-#include <vector>
#include <brillo/secure_blob.h>
@@ -35,7 +34,7 @@
// ExtentWriter overrides.
bool Init(FileDescriptorPtr /* fd */,
- const std::vector<Extent>& /* extents */,
+ const google::protobuf::RepeatedPtrField<Extent>& /* extents */,
uint32_t /* block_size */) override {
init_called_ = true;
return true;
diff --git a/payload_consumer/fake_file_descriptor.cc b/payload_consumer/fake_file_descriptor.cc
index 09bd2c9..d54856b 100644
--- a/payload_consumer/fake_file_descriptor.cc
+++ b/payload_consumer/fake_file_descriptor.cc
@@ -23,7 +23,7 @@
read_ops_.emplace_back(offset_, count);
// Check for the EOF condition first to avoid reporting it as a failure.
- if (offset_ >= static_cast<uint64_t>(size_) || !count)
+ if (offset_ >= static_cast<uint64_t>(size_) || count == 0)
return 0;
// Find the first offset greater or equal than the current position where a
// failure will occur. This will mark the end of the read chunk.
diff --git a/payload_consumer/fake_file_descriptor.h b/payload_consumer/fake_file_descriptor.h
index ad49373..f17820b 100644
--- a/payload_consumer/fake_file_descriptor.h
+++ b/payload_consumer/fake_file_descriptor.h
@@ -27,14 +27,14 @@
namespace chromeos_update_engine {
// A fake file descriptor with configurable errors. The file descriptor always
-// reads a fixed sequence of bytes, consisting on the concatenation of the
+// reads a fixed sequence of bytes, consisting of the concatenation of the
// numbers 0, 1, 2... each one encoded in 4 bytes as the big-endian 16-bit
// number encoded in hexadecimal. For example, the beginning of the stream in
// ASCII is 0000000100020003... which corresponds to the numbers 0, 1, 2 and 3.
class FakeFileDescriptor : public FileDescriptor {
public:
FakeFileDescriptor() = default;
- ~FakeFileDescriptor() = default;
+ ~FakeFileDescriptor() override = default;
// FileDescriptor override methods.
bool Open(const char* path, int flags, mode_t mode) override {
@@ -67,6 +67,10 @@
return false;
}
+ bool Flush() override {
+ return open_;
+ }
+
bool Close() override {
if (!open_)
return false;
@@ -86,7 +90,7 @@
// Marks the range starting from |offset| bytes into the file and |length|
// size as a failure range. Reads from this range will always fail.
void AddFailureRange(uint64_t offset, uint64_t length) {
- if (!length)
+ if (length == 0)
return;
failure_ranges_.emplace_back(offset, length);
}
diff --git a/payload_consumer/file_descriptor.cc b/payload_consumer/file_descriptor.cc
index ebe4428..4eabb8f 100644
--- a/payload_consumer/file_descriptor.cc
+++ b/payload_consumer/file_descriptor.cc
@@ -123,6 +123,11 @@
#endif // defined(BLKZEROOUT)
}
+bool EintrSafeFileDescriptor::Flush() {
+ CHECK_GE(fd_, 0);
+ return true;
+}
+
bool EintrSafeFileDescriptor::Close() {
CHECK_GE(fd_, 0);
if (IGNORE_EINTR(close(fd_)))
diff --git a/payload_consumer/file_descriptor.h b/payload_consumer/file_descriptor.h
index c8a5b15..5e524d9 100644
--- a/payload_consumer/file_descriptor.h
+++ b/payload_consumer/file_descriptor.h
@@ -87,6 +87,11 @@
uint64_t length,
int* result) = 0;
+ // Flushes any cached data. The descriptor must be opened prior to this
+ // call. Returns false if it fails to write data. Implementations may set
+ // errno accrodingly.
+ virtual bool Flush() = 0;
+
// Closes a file descriptor. The descriptor must be open prior to this call.
// Returns true on success, false otherwise. Specific implementations may set
// errno accordingly.
@@ -118,6 +123,7 @@
uint64_t start,
uint64_t length,
int* result) override;
+ bool Flush() override;
bool Close() override;
bool IsSettingErrno() override {
return true;
diff --git a/payload_consumer/file_descriptor_utils.cc b/payload_consumer/file_descriptor_utils.cc
index f7f61a5..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,53 +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;
- std::vector<Extent> vec_tgt_extents;
- vec_tgt_extents.reserve(tgt_extents.size());
- for (const auto& ext : tgt_extents) {
- vec_tgt_extents.push_back(ext);
- }
- TEST_AND_RETURN_FALSE(writer.Init(target, vec_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.h b/payload_consumer/file_descriptor_utils.h
index b73defb..d1289d6 100644
--- a/payload_consumer/file_descriptor_utils.h
+++ b/payload_consumer/file_descriptor_utils.h
@@ -17,8 +17,6 @@
#ifndef UPDATE_ENGINE_PAYLOAD_CONSUMER_FILE_DESCRIPTOR_UTILS_H_
#define UPDATE_ENGINE_PAYLOAD_CONSUMER_FILE_DESCRIPTOR_UTILS_H_
-#include <vector>
-
#include <brillo/secure_blob.h>
#include "update_engine/payload_consumer/file_descriptor.h"
@@ -27,7 +25,7 @@
namespace chromeos_update_engine {
namespace fd_utils {
-// Copy a blocks from the |source| file to the |target| file and hash the
+// Copy blocks from the |source| file to the |target| file and hashes the
// contents. The blocks to copy from the |source| to the |target| files are
// specified by the |src_extents| and |tgt_extents| list of Extents, which
// must have the same length in number of blocks. Stores the hash of the
diff --git a/payload_consumer/file_descriptor_utils_unittest.cc b/payload_consumer/file_descriptor_utils_unittest.cc
index 9910239..8ba8ce6 100644
--- a/payload_consumer/file_descriptor_utils_unittest.cc
+++ b/payload_consumer/file_descriptor_utils_unittest.cc
@@ -16,13 +16,14 @@
#include "update_engine/payload_consumer/file_descriptor_utils.h"
+#include <fcntl.h>
+
#include <string>
#include <utility>
#include <vector>
-#include <gtest/gtest.h>
-
#include <brillo/data_encoding.h>
+#include <gtest/gtest.h>
#include "update_engine/common/hash_calculator.h"
#include "update_engine/common/test_utils.h"
@@ -31,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);
}
diff --git a/payload_consumer/file_writer_unittest.cc b/payload_consumer/file_writer_unittest.cc
index debb4c3..92837c8 100644
--- a/payload_consumer/file_writer_unittest.cc
+++ b/payload_consumer/file_writer_unittest.cc
@@ -21,7 +21,6 @@
#include <unistd.h>
#include <string>
-#include <vector>
#include <brillo/secure_blob.h>
#include <gtest/gtest.h>
@@ -30,7 +29,6 @@
#include "update_engine/common/utils.h"
using std::string;
-using std::vector;
namespace chromeos_update_engine {
diff --git a/payload_consumer/mtd_file_descriptor.cc b/payload_consumer/mtd_file_descriptor.cc
index 3f0a33f..5d7758a 100644
--- a/payload_consumer/mtd_file_descriptor.cc
+++ b/payload_consumer/mtd_file_descriptor.cc
@@ -18,11 +18,12 @@
#include <fcntl.h>
#include <mtd/ubi-user.h>
-#include <string>
#include <sys/ioctl.h>
#include <sys/stat.h>
#include <sys/types.h>
-#include <vector>
+
+#include <memory>
+#include <string>
#include <base/files/file_path.h>
#include <base/strings/string_number_conversions.h>
@@ -33,7 +34,6 @@
#include "update_engine/common/utils.h"
using std::string;
-using std::vector;
namespace {
diff --git a/payload_consumer/payload_constants.cc b/payload_consumer/payload_constants.cc
index 7d396b6..ad193a0 100644
--- a/payload_consumer/payload_constants.cc
+++ b/payload_consumer/payload_constants.cc
@@ -54,6 +54,8 @@
return "REPLACE_XZ";
case InstallOperation::PUFFDIFF:
return "PUFFDIFF";
+ case InstallOperation::BROTLI_BSDIFF:
+ return "BROTLI_BSDIFF";
}
return "<unknown_op>";
}
diff --git a/payload_consumer/payload_verifier.h b/payload_consumer/payload_verifier.h
index 22ced40..8caef35 100644
--- a/payload_consumer/payload_verifier.h
+++ b/payload_consumer/payload_verifier.h
@@ -18,7 +18,6 @@
#define UPDATE_ENGINE_PAYLOAD_CONSUMER_PAYLOAD_VERIFIER_H_
#include <string>
-#include <vector>
#include <base/macros.h>
#include <brillo/secure_blob.h>
diff --git a/payload_consumer/postinstall_runner_action_unittest.cc b/payload_consumer/postinstall_runner_action_unittest.cc
index 772270c..f15171b 100644
--- a/payload_consumer/postinstall_runner_action_unittest.cc
+++ b/payload_consumer/postinstall_runner_action_unittest.cc
@@ -22,7 +22,6 @@
#include <memory>
#include <string>
-#include <vector>
#include <base/bind.h>
#include <base/files/file_util.h>
@@ -43,7 +42,6 @@
using brillo::MessageLoop;
using chromeos_update_engine::test_utils::ScopedLoopbackDeviceBinder;
using std::string;
-using std::vector;
namespace chromeos_update_engine {
diff --git a/payload_consumer/xz_extent_writer.cc b/payload_consumer/xz_extent_writer.cc
index 4bd893d..343ed80 100644
--- a/payload_consumer/xz_extent_writer.cc
+++ b/payload_consumer/xz_extent_writer.cc
@@ -16,7 +16,7 @@
#include "update_engine/payload_consumer/xz_extent_writer.h"
-using std::vector;
+using google::protobuf::RepeatedPtrField;
namespace chromeos_update_engine {
@@ -47,7 +47,7 @@
return "<unknown xz error>";
}
#undef __XZ_ERROR_STRING_CASE
-};
+}
} // namespace
XzExtentWriter::~XzExtentWriter() {
@@ -55,7 +55,7 @@
}
bool XzExtentWriter::Init(FileDescriptorPtr fd,
- const vector<Extent>& extents,
+ const RepeatedPtrField<Extent>& extents,
uint32_t block_size) {
stream_ = xz_dec_init(XZ_DYNALLOC, kXzMaxDictSize);
TEST_AND_RETURN_FALSE(stream_ != nullptr);
diff --git a/payload_consumer/xz_extent_writer.h b/payload_consumer/xz_extent_writer.h
index a6b3257..5e50256 100644
--- a/payload_consumer/xz_extent_writer.h
+++ b/payload_consumer/xz_extent_writer.h
@@ -20,7 +20,7 @@
#include <xz.h>
#include <memory>
-#include <vector>
+#include <utility>
#include <brillo/secure_blob.h>
@@ -40,7 +40,7 @@
~XzExtentWriter() override;
bool Init(FileDescriptorPtr fd,
- const std::vector<Extent>& extents,
+ const google::protobuf::RepeatedPtrField<Extent>& extents,
uint32_t block_size) override;
bool Write(const void* bytes, size_t count) override;
bool EndImpl() override;
diff --git a/payload_consumer/xz_extent_writer_unittest.cc b/payload_consumer/xz_extent_writer_unittest.cc
index fb8bb40..c8bcdf9 100644
--- a/payload_consumer/xz_extent_writer_unittest.cc
+++ b/payload_consumer/xz_extent_writer_unittest.cc
@@ -22,19 +22,14 @@
#include <unistd.h>
#include <algorithm>
-#include <string>
-#include <vector>
-#include <brillo/make_unique_ptr.h>
+#include <base/memory/ptr_util.h>
#include <gtest/gtest.h>
#include "update_engine/common/test_utils.h"
#include "update_engine/common/utils.h"
#include "update_engine/payload_consumer/fake_extent_writer.h"
-using std::string;
-using std::vector;
-
namespace chromeos_update_engine {
namespace {
@@ -88,8 +83,7 @@
protected:
void SetUp() override {
fake_extent_writer_ = new FakeExtentWriter();
- xz_writer_.reset(
- new XzExtentWriter(brillo::make_unique_ptr(fake_extent_writer_)));
+ xz_writer_.reset(new XzExtentWriter(base::WrapUnique(fake_extent_writer_)));
}
void WriteAll(const brillo::Blob& compressed) {