AU: Optimize checkpointing a bit and decide on new update vs. resume.
BUG=7390
TEST=unit tests, gmerged on device
Change-Id: Ibed6082fe697e6b28b03fb1cc39d700826bf2bfe
Review URL: http://codereview.chromium.org/3541016
diff --git a/delta_performer.cc b/delta_performer.cc
index 6143ec5..92d4733 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -222,7 +222,7 @@
// that if the operation gets interrupted, we don't try to resume the
// update.
if (!IsIdempotentOperation(op)) {
- ResetUpdateProgress();
+ ResetUpdateProgress(prefs_);
}
if (op.type() == DeltaArchiveManifest_InstallOperation_Type_REPLACE ||
op.type() == DeltaArchiveManifest_InstallOperation_Type_REPLACE_BZ) {
@@ -512,19 +512,57 @@
buffer_.erase(buffer_.begin(), buffer_.begin() + count);
}
-bool DeltaPerformer::ResetUpdateProgress() {
- TEST_AND_RETURN_FALSE(prefs_->SetInt64(kPrefsUpdateStateNextOperation,
- kUpdateStateOperationInvalid));
+bool DeltaPerformer::CanResumeUpdate(PrefsInterface* prefs,
+ string update_check_response_hash) {
+ int64_t next_operation = kUpdateStateOperationInvalid;
+ TEST_AND_RETURN_FALSE(prefs->GetInt64(kPrefsUpdateStateNextOperation,
+ &next_operation) &&
+ next_operation != kUpdateStateOperationInvalid &&
+ next_operation > 0);
+
+ string interrupted_hash;
+ TEST_AND_RETURN_FALSE(prefs->GetString(kPrefsUpdateCheckResponseHash,
+ &interrupted_hash) &&
+ !interrupted_hash.empty() &&
+ interrupted_hash == update_check_response_hash);
+
+ // Sanity check the rest.
+ int64_t next_data_offset = -1;
+ TEST_AND_RETURN_FALSE(prefs->GetInt64(kPrefsUpdateStateNextDataOffset,
+ &next_data_offset) &&
+ next_data_offset >= 0);
+
+ string signed_sha256_context;
+ TEST_AND_RETURN_FALSE(
+ prefs->GetString(kPrefsUpdateStateSignedSHA256Context,
+ &signed_sha256_context) &&
+ !signed_sha256_context.empty());
+
+ int64_t manifest_metadata_size = 0;
+ TEST_AND_RETURN_FALSE(prefs->GetInt64(kPrefsManifestMetadataSize,
+ &manifest_metadata_size) &&
+ manifest_metadata_size > 0);
+
+ return true;
+}
+
+bool DeltaPerformer::ResetUpdateProgress(PrefsInterface* prefs) {
+ TEST_AND_RETURN_FALSE(prefs->SetInt64(kPrefsUpdateStateNextOperation,
+ kUpdateStateOperationInvalid));
return true;
}
bool DeltaPerformer::CheckpointUpdateProgress() {
// First reset the progress in case we die in the middle of the state update.
- ResetUpdateProgress();
- TEST_AND_RETURN_FALSE(prefs_->SetString(kPrefsUpdateStateSignedSHA256Context,
- hash_calculator_.GetContext()));
- TEST_AND_RETURN_FALSE(prefs_->SetInt64(kPrefsUpdateStateNextDataOffset,
- buffer_offset_));
+ ResetUpdateProgress(prefs_);
+ if (last_updated_buffer_offset_ != buffer_offset_) {
+ TEST_AND_RETURN_FALSE(
+ prefs_->SetString(kPrefsUpdateStateSignedSHA256Context,
+ hash_calculator_.GetContext()));
+ TEST_AND_RETURN_FALSE(prefs_->SetInt64(kPrefsUpdateStateNextDataOffset,
+ buffer_offset_));
+ last_updated_buffer_offset_ = buffer_offset_;
+ }
TEST_AND_RETURN_FALSE(prefs_->SetInt64(kPrefsUpdateStateNextOperation,
next_operation_num_));
return true;
diff --git a/delta_performer.h b/delta_performer.h
index 2f9fd8a..87a27dd 100644
--- a/delta_performer.h
+++ b/delta_performer.h
@@ -31,6 +31,7 @@
manifest_valid_(false),
next_operation_num_(0),
buffer_offset_(0),
+ last_updated_buffer_offset_(kuint64max),
block_size_(0) {}
// Opens the kernel. Should be called before or after Open(), but before
@@ -72,6 +73,15 @@
uint64_t full_length,
std::string* positions_string);
+ // Returns true if a previous update attempt can be continued based on the
+ // persistent preferences and the new update check response hash.
+ static bool CanResumeUpdate(PrefsInterface* prefs,
+ std::string update_check_response_hash);
+
+ // Resets the persistent update progress state to indicate that an update
+ // can't be resumed. Returns true on success, false otherwise.
+ static bool ResetUpdateProgress(PrefsInterface* prefs);
+
private:
// Returns true if enough of the delta file has been passed via Write()
// to be able to perform a given install operation.
@@ -102,7 +112,8 @@
// updates the hash calculator with these bytes before discarding them.
void DiscardBufferHeadBytes(size_t count, bool do_hash);
- bool ResetUpdateProgress();
+ // Checkpoints the update progress into persistent storage to allow this
+ // update attempt to be resumed after reboot.
bool CheckpointUpdateProgress();
// Update Engine preference store.
@@ -131,6 +142,9 @@
// Offset of buffer_ in the binary blobs section of the update.
uint64_t buffer_offset_;
+ // Last |buffer_offset_| value updated as part of the progress update.
+ uint64_t last_updated_buffer_offset_;
+
// The block size (parsed from the manifest).
uint32_t block_size_;
diff --git a/download_action_unittest.cc b/download_action_unittest.cc
index 3e8aabd..7571fb4 100644
--- a/download_action_unittest.cc
+++ b/download_action_unittest.cc
@@ -102,6 +102,7 @@
OmahaHashCalculator::OmahaHashOfData(data);
uint64_t size = data.size() + (size_test ? 1 : 0);
InstallPlan install_plan(true,
+ false,
"",
size,
hash,
@@ -227,7 +228,7 @@
// takes ownership of passed in HttpFetcher
ObjectFeederAction<InstallPlan> feeder_action;
- InstallPlan install_plan(true, "", 0, "", temp_file.GetPath(), "");
+ InstallPlan install_plan(true, false, "", 0, "", temp_file.GetPath(), "");
feeder_action.set_obj(install_plan);
PrefsMock prefs;
DownloadAction download_action(&prefs,
@@ -326,6 +327,7 @@
// takes ownership of passed in HttpFetcher
InstallPlan install_plan(true,
+ false,
"",
1,
OmahaHashCalculator::OmahaHashOfString("x"),
@@ -364,7 +366,7 @@
DirectFileWriter writer;
// takes ownership of passed in HttpFetcher
- InstallPlan install_plan(true, "", 0, "", path, "");
+ InstallPlan install_plan(true, false, "", 0, "", path, "");
ObjectFeederAction<InstallPlan> feeder_action;
feeder_action.set_obj(install_plan);
PrefsMock prefs;
diff --git a/filesystem_copier_action_unittest.cc b/filesystem_copier_action_unittest.cc
index 03aa86a..f82097e 100644
--- a/filesystem_copier_action_unittest.cc
+++ b/filesystem_copier_action_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -231,7 +231,7 @@
ObjectFeederAction<InstallPlan> feeder_action;
const char* kUrl = "http://some/url";
- InstallPlan install_plan(true, kUrl, 0, "", "", "");
+ InstallPlan install_plan(true, false, kUrl, 0, "", "", "");
feeder_action.set_obj(install_plan);
FilesystemCopierAction copier_action(false);
ObjectCollectorAction<InstallPlan> collector_action;
@@ -256,7 +256,13 @@
processor.set_delegate(&delegate);
ObjectFeederAction<InstallPlan> feeder_action;
- InstallPlan install_plan(false, "", 0, "", "/no/such/file", "/no/such/file");
+ InstallPlan install_plan(false,
+ false,
+ "",
+ 0,
+ "",
+ "/no/such/file",
+ "/no/such/file");
feeder_action.set_obj(install_plan);
FilesystemCopierAction copier_action(false);
ObjectCollectorAction<InstallPlan> collector_action;
diff --git a/install_plan.h b/install_plan.h
index 19fee8a..8dc42aa 100644
--- a/install_plan.h
+++ b/install_plan.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -15,20 +15,23 @@
struct InstallPlan {
InstallPlan(bool is_full,
+ bool is_resume,
const std::string& url,
uint64_t size,
const std::string& hash,
const std::string& install_path,
const std::string& kernel_install_path)
: is_full_update(is_full),
+ is_resume(is_resume),
download_url(url),
size(size),
download_hash(hash),
install_path(install_path),
kernel_install_path(kernel_install_path) {}
- InstallPlan() : is_full_update(false) {}
+ InstallPlan() : is_full_update(false), is_resume(false), size(0) {}
bool is_full_update;
+ bool is_resume;
std::string download_url; // url to download from
uint64_t size; // size of the download url's data
std::string download_hash; // hash of the data at the url
@@ -37,11 +40,12 @@
bool operator==(const InstallPlan& that) const {
return (is_full_update == that.is_full_update) &&
- (download_url == that.download_url) &&
- (size == that.size) &&
- (download_hash == that.download_hash) &&
- (install_path == that.install_path) &&
- (kernel_install_path == that.kernel_install_path);
+ (is_resume == that.is_resume) &&
+ (download_url == that.download_url) &&
+ (size == that.size) &&
+ (download_hash == that.download_hash) &&
+ (install_path == that.install_path) &&
+ (kernel_install_path == that.kernel_install_path);
}
bool operator!=(const InstallPlan& that) const {
return !((*this) == that);
@@ -49,6 +53,7 @@
void Dump() const {
LOG(INFO) << "InstallPlan: "
<< (is_full_update ? "full_update" : "delta_update")
+ << (is_resume ? ", resume" : ", new_update")
<< ", url: " << download_url
<< ", size: " << size
<< ", hash: " << download_hash
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index cdcf735..d3142a7 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -8,6 +8,7 @@
#include <base/logging.h>
+#include "update_engine/delta_performer.h"
#include "update_engine/prefs_interface.h"
#include "update_engine/utils.h"
@@ -27,12 +28,17 @@
install_plan_.download_url = response.codebase;
install_plan_.size = response.size;
install_plan_.download_hash = response.hash;
- // TODO(petkov): Decide here if this is going to be a regular update or
- // resume-after-boot. This should also set the number of ops performed so far
- // to invalid if no need to resume.
- LOG_IF(WARNING, !prefs_->SetString(kPrefsUpdateCheckResponseHash,
- response.hash))
- << "Unable to save the update check response hash.";
+
+ install_plan_.is_resume =
+ DeltaPerformer::CanResumeUpdate(prefs_, response.hash);
+ if (!install_plan_.is_resume) {
+ LOG_IF(WARNING, !DeltaPerformer::ResetUpdateProgress(prefs_))
+ << "Unable to reset the update progress.";
+ LOG_IF(WARNING, !prefs_->SetString(kPrefsUpdateCheckResponseHash,
+ response.hash))
+ << "Unable to save the update check response hash.";
+ }
+
TEST_AND_RETURN(GetInstallDev(
(!boot_device_.empty() ? boot_device_ : utils::BootDevice()),
&install_plan_.install_path));