Cache OTA manifest in update_engine
When resuming download of an OTA package, update_engine needs to
redownload the manifest of OTA package, located at beginning of file.
However, the manifest blob can be as large as ~150K for some updates.
This CL caches manifest on disk, so that update engine no longer has to
redownload the manifest for every resume.
Test: Perform OTA, pause and resume, verify that update succeeds
Bug: 70736331
Change-Id: Iaf157ef57e68e4842d5867dea5467a3ab34f8286
diff --git a/common/constants.cc b/common/constants.cc
index fa13a38..c85ba54 100644
--- a/common/constants.cc
+++ b/common/constants.cc
@@ -108,6 +108,7 @@
const char kPrefsWallClockScatteringWaitPeriod[] = "wall-clock-wait-period";
const char kPrefsWallClockStagingWaitPeriod[] =
"wall-clock-staging-wait-period";
+const char kPrefsManifestBytes[] = "manifest-bytes";
// These four fields are generated by scripts/brillo_update_payload.
const char kPayloadPropertyFileSize[] = "FILE_SIZE";
diff --git a/common/constants.h b/common/constants.h
index eb489fc..7170201 100644
--- a/common/constants.h
+++ b/common/constants.h
@@ -104,6 +104,7 @@
extern const char kPrefsVerityWritten[];
extern const char kPrefsWallClockScatteringWaitPeriod[];
extern const char kPrefsWallClockStagingWaitPeriod[];
+extern const char kPrefsManifestBytes[];
// Keys used when storing and loading payload properties.
extern const char kPayloadPropertyFileSize[];
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index 7d837db..68f38df 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -585,6 +585,9 @@
if ((*error = ValidateManifest()) != ErrorCode::kSuccess)
return false;
manifest_valid_ = true;
+ if (!install_plan_->is_resume) {
+ prefs_->SetString(kPrefsManifestBytes, {buffer_.begin(), buffer_.end()});
+ }
// Clear the download buffer.
DiscardBuffer(false, metadata_size_);
diff --git a/payload_consumer/delta_performer.h b/payload_consumer/delta_performer.h
index 7b30a83..2d1768d 100644
--- a/payload_consumer/delta_performer.h
+++ b/payload_consumer/delta_performer.h
@@ -78,7 +78,9 @@
download_delegate_(download_delegate),
install_plan_(install_plan),
payload_(payload),
- interactive_(interactive) {}
+ interactive_(interactive) {
+ CHECK(install_plan_);
+ }
// FileWriter's Write implementation where caller doesn't care about
// error codes.
diff --git a/payload_consumer/delta_performer_integration_test.cc b/payload_consumer/delta_performer_integration_test.cc
index 16641c6..acbecad 100644
--- a/payload_consumer/delta_performer_integration_test.cc
+++ b/payload_consumer/delta_performer_integration_test.cc
@@ -28,6 +28,7 @@
#include <base/stl_util.h>
#include <base/strings/string_util.h>
#include <base/strings/stringprintf.h>
+#include <gmock/gmock-matchers.h>
#include <google/protobuf/repeated_field.h>
#include <gtest/gtest.h>
#include <openssl/pem.h>
@@ -188,7 +189,7 @@
string private_key_path = GetBuildArtifactsPath(kUnittestPrivateKeyPath);
size_t signature_size;
ASSERT_TRUE(PayloadSigner::GetMaximumSignatureSize(private_key_path,
- &signature_size));
+ &signature_size));
brillo::Blob metadata_hash, payload_hash;
ASSERT_TRUE(PayloadSigner::HashPayloadForSigning(
payload_path, {signature_size}, &payload_hash, &metadata_hash));
@@ -226,12 +227,13 @@
string delta_generator_path = GetBuildArtifactsPath("delta_generator");
ASSERT_EQ(0,
System(base::StringPrintf(
- "%s -in_file=%s -signature_size=%s -out_hash_file=%s "
- "-out_metadata_hash_file=%s",
+ "%s -in_file=%s -signature_size=%s -out_hash_file=%s "
+ "-out_metadata_hash_file=%s",
delta_generator_path.c_str(),
payload_path.c_str(),
signature_size_string.c_str(),
- hash_file.path().c_str(), metadata_hash_file.path().c_str())));
+ hash_file.path().c_str(),
+ metadata_hash_file.path().c_str())));
// Sign the hash with all private keys.
vector<test_utils::ScopedTempFile> sig_files, metadata_sig_files;
@@ -248,16 +250,19 @@
brillo::Blob metadata_hash, metadata_signature;
ASSERT_TRUE(utils::ReadFile(metadata_hash_file.path(), &metadata_hash));
- ASSERT_TRUE(PayloadSigner::SignHash(metadata_hash, key_path, &metadata_signature));
+ ASSERT_TRUE(
+ PayloadSigner::SignHash(metadata_hash, key_path, &metadata_signature));
test_utils::ScopedTempFile metadata_sig_file("signature.XXXXXX");
- ASSERT_TRUE(test_utils::WriteFileVector(metadata_sig_file.path(), metadata_signature));
+ ASSERT_TRUE(test_utils::WriteFileVector(metadata_sig_file.path(),
+ metadata_signature));
metadata_sig_file_paths.push_back(metadata_sig_file.path());
metadata_sig_files.push_back(std::move(metadata_sig_file));
}
string sig_files_string = base::JoinString(sig_file_paths, ":");
- string metadata_sig_files_string = base::JoinString(metadata_sig_file_paths, ":");
+ string metadata_sig_files_string =
+ base::JoinString(metadata_sig_file_paths, ":");
// Add the signature to the payload.
ASSERT_EQ(0,
@@ -735,6 +740,11 @@
.WillRepeatedly(Return(true));
EXPECT_CALL(prefs, SetString(kPrefsDynamicPartitionMetadataUpdated, _))
.WillRepeatedly(Return(true));
+ EXPECT_CALL(prefs,
+ SetString(kPrefsManifestBytes,
+ testing::SizeIs(state->metadata_signature_size +
+ state->metadata_size)))
+ .WillRepeatedly(Return(true));
if (op_hash_test == kValidOperationData && signature_test != kSignatureNone) {
EXPECT_CALL(prefs, SetString(kPrefsUpdateStateSignatureBlob, _))
.WillOnce(Return(true));
@@ -1026,12 +1036,8 @@
}
TEST(DeltaPerformerIntegrationTest, RunAsRootFullSmallImageTest) {
- DoSmallImageTest(true,
- true,
- -1,
- kSignatureGenerator,
- true,
- kFullPayloadMinorVersion);
+ DoSmallImageTest(
+ true, true, -1, kSignatureGenerator, true, kFullPayloadMinorVersion);
}
TEST(DeltaPerformerIntegrationTest, RunAsRootSmallImageSignNoneTest) {
@@ -1094,12 +1100,8 @@
}
TEST(DeltaPerformerIntegrationTest, RunAsRootSmallImageSourceOpsTest) {
- DoSmallImageTest(false,
- false,
- -1,
- kSignatureGenerator,
- false,
- kSourceMinorPayloadVersion);
+ DoSmallImageTest(
+ false, false, -1, kSignatureGenerator, false, kSourceMinorPayloadVersion);
}
TEST(DeltaPerformerIntegrationTest,
diff --git a/payload_consumer/download_action.cc b/payload_consumer/download_action.cc
index 45df5a9..ea99892 100644
--- a/payload_consumer/download_action.cc
+++ b/payload_consumer/download_action.cc
@@ -55,8 +55,7 @@
code_(ErrorCode::kSuccess),
delegate_(nullptr),
p2p_sharing_fd_(-1),
- p2p_visible_(true) {
-}
+ p2p_visible_(true) {}
DownloadAction::~DownloadAction() {}
@@ -203,18 +202,76 @@
StartDownloading();
}
+bool DownloadAction::LoadCachedManifest(int64_t manifest_size) {
+ std::string cached_manifest_bytes;
+ if (!prefs_->GetString(kPrefsManifestBytes, &cached_manifest_bytes) ||
+ cached_manifest_bytes.size() <= 0) {
+ LOG(INFO) << "Cached Manifest data not found";
+ return false;
+ }
+ if (static_cast<int64_t>(cached_manifest_bytes.size()) != manifest_size) {
+ LOG(WARNING) << "Cached metadata has unexpected size: "
+ << cached_manifest_bytes.size() << " vs. " << manifest_size;
+ return false;
+ }
+
+ ErrorCode error;
+ const bool success =
+ delta_performer_->Write(
+ cached_manifest_bytes.data(), cached_manifest_bytes.size(), &error) &&
+ delta_performer_->IsManifestValid();
+ if (success) {
+ LOG(INFO) << "Successfully parsed cached manifest";
+ } else {
+ // If parsing of cached data failed, fall back to fetch them using HTTP
+ LOG(WARNING) << "Cached manifest data fails to load, error code:"
+ << static_cast<int>(error) << "," << error;
+ }
+ return success;
+}
+
void DownloadAction::StartDownloading() {
download_active_ = true;
http_fetcher_->ClearRanges();
+
+ 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_,
+ interactive_));
+ writer_ = delta_performer_.get();
+ }
+
if (install_plan_.is_resume &&
payload_ == &install_plan_.payloads[resume_payload_index_]) {
- // Resuming an update so fetch the update manifest metadata first.
+ // Resuming an update so parse the cached manifest first
int64_t manifest_metadata_size = 0;
int64_t manifest_signature_size = 0;
prefs_->GetInt64(kPrefsManifestMetadataSize, &manifest_metadata_size);
prefs_->GetInt64(kPrefsManifestSignatureSize, &manifest_signature_size);
- http_fetcher_->AddRange(base_offset_,
- manifest_metadata_size + manifest_signature_size);
+
+ // TODO(zhangkelvin) Add unittest for success and fallback route
+ if (!LoadCachedManifest(manifest_metadata_size + manifest_signature_size)) {
+ if (delta_performer_) {
+ // Create a new DeltaPerformer to reset all its state
+ delta_performer_ = std::make_unique<DeltaPerformer>(prefs_,
+ boot_control_,
+ hardware_,
+ delegate_,
+ &install_plan_,
+ payload_,
+ interactive_);
+ writer_ = delta_performer_.get();
+ }
+ http_fetcher_->AddRange(base_offset_,
+ manifest_metadata_size + manifest_signature_size);
+ }
+
// If there're remaining unprocessed data blobs, fetch them. Be careful not
// to request data beyond the end of the payload to avoid 416 HTTP response
// error codes.
@@ -238,18 +295,6 @@
}
}
- 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_,
- interactive_));
- writer_ = delta_performer_.get();
- }
if (system_state_ != nullptr) {
const PayloadStateInterface* payload_state = system_state_->payload_state();
string file_id = utils::CalculateP2PFileId(payload_->hash, payload_->size);
diff --git a/payload_consumer/download_action.h b/payload_consumer/download_action.h
index 61c93d2..740416d 100644
--- a/payload_consumer/download_action.h
+++ b/payload_consumer/download_action.h
@@ -131,6 +131,10 @@
// called or if CloseP2PSharingFd() has been called.
void WriteToP2PFile(const void* data, size_t length, off_t file_offset);
+ // Attempt to load cached manifest data from prefs
+ // return true on success, false otherwise.
+ bool LoadCachedManifest(int64_t manifest_size);
+
// Start downloading the current payload using delta_performer.
void StartDownloading();