Resume multiple payloads.
am: 5ae865b431
Change-Id: I90b2f4b60f26cb7c9d810fa93f1345c20e816420
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index 84048eb..ab5e275 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -626,6 +626,12 @@
if (!ParseManifestPartitions(error))
return false;
+ // |install_plan.partitions| was filled in, nothing need to be done here if
+ // the payload was already applied, returns false to terminate http fetcher,
+ // but keep |error| as ErrorCode::kSuccess.
+ if (payload_->already_applied)
+ return false;
+
num_total_operations_ = 0;
for (const auto& partition : partitions_) {
num_total_operations_ += partition.operations_size();
@@ -1673,7 +1679,6 @@
TEST_AND_RETURN_FALSE(prefs->SetInt64(kPrefsUpdateStateNextOperation,
kUpdateStateOperationInvalid));
if (!quick) {
- prefs->SetString(kPrefsUpdateCheckResponseHash, "");
prefs->SetInt64(kPrefsUpdateStateNextDataOffset, -1);
prefs->SetInt64(kPrefsUpdateStateNextDataLength, 0);
prefs->SetString(kPrefsUpdateStateSHA256Context, "");
diff --git a/payload_consumer/download_action.cc b/payload_consumer/download_action.cc
index 451c49e..c3a5016 100644
--- a/payload_consumer/download_action.cc
+++ b/payload_consumer/download_action.cc
@@ -27,6 +27,7 @@
#include "update_engine/common/action_pipe.h"
#include "update_engine/common/boot_control_interface.h"
#include "update_engine/common/error_code_utils.h"
+#include "update_engine/common/multi_range_http_fetcher.h"
#include "update_engine/common/utils.h"
#include "update_engine/omaha_request_params.h"
#include "update_engine/p2p_manager.h"
@@ -47,14 +48,12 @@
boot_control_(boot_control),
hardware_(hardware),
system_state_(system_state),
- http_fetcher_(http_fetcher),
+ http_fetcher_(new MultiRangeHttpFetcher(http_fetcher)),
writer_(nullptr),
code_(ErrorCode::kSuccess),
delegate_(nullptr),
- bytes_received_(0),
p2p_sharing_fd_(-1),
- p2p_visible_(true) {
-}
+ p2p_visible_(true) {}
DownloadAction::~DownloadAction() {}
@@ -171,9 +170,24 @@
// Get the InstallPlan and read it
CHECK(HasInputObject());
install_plan_ = GetInputObject();
- bytes_received_ = 0;
-
install_plan_.Dump();
+
+ bytes_received_ = 0;
+ bytes_total_ = 0;
+ for (const auto& payload : install_plan_.payloads)
+ bytes_total_ += payload.size;
+
+ if (install_plan_.is_resume) {
+ int64_t payload_index = 0;
+ if (prefs_->GetInt64(kPrefsUpdateStatePayloadIndex, &payload_index) &&
+ static_cast<size_t>(payload_index) < install_plan_.payloads.size()) {
+ // Save the index for the resume payload before downloading any previous
+ // payload, otherwise it will be overwritten.
+ resume_payload_index_ = payload_index;
+ for (int i = 0; i < payload_index; i++)
+ install_plan_.payloads[i].already_applied = true;
+ }
+ }
// TODO(senj): check that install plan has at least one payload.
if (!payload_)
payload_ = &install_plan_.payloads[0];
@@ -185,11 +199,44 @@
<< ". Proceeding with the update anyway.";
}
- download_active_ = true;
StartDownloading();
}
void DownloadAction::StartDownloading() {
+ download_active_ = true;
+ http_fetcher_->ClearRanges();
+ if (install_plan_.is_resume &&
+ payload_ == &install_plan_.payloads[resume_payload_index_]) {
+ // Resuming an update so fetch the update manifest metadata 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);
+ // 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.
+ int64_t next_data_offset = 0;
+ prefs_->GetInt64(kPrefsUpdateStateNextDataOffset, &next_data_offset);
+ uint64_t resume_offset =
+ manifest_metadata_size + manifest_signature_size + next_data_offset;
+ if (!payload_->size) {
+ http_fetcher_->AddRange(base_offset_ + resume_offset);
+ } else if (resume_offset < payload_->size) {
+ http_fetcher_->AddRange(base_offset_ + resume_offset,
+ payload_->size - resume_offset);
+ }
+ } else {
+ if (payload_->size) {
+ http_fetcher_->AddRange(base_offset_, payload_->size);
+ } else {
+ // If no payload size is passed we assume we read until the end of the
+ // stream.
+ http_fetcher_->AddRange(base_offset_);
+ }
+ }
+
if (writer_ && writer_ != delta_performer_.get()) {
LOG(INFO) << "Using writer for test.";
} else {
@@ -271,12 +318,14 @@
bytes_received_ += length;
if (delegate_ && download_active_) {
- delegate_->BytesReceived(length, bytes_received_, payload_->size);
+ delegate_->BytesReceived(length, bytes_received_, bytes_total_);
}
if (writer_ && !writer_->Write(bytes, length, &code_)) {
- LOG(ERROR) << "Error " << utils::ErrorCodeToString(code_) << " (" << code_
- << ") in DeltaPerformer's Write method when "
- << "processing the received payload -- Terminating processing";
+ if (code_ != ErrorCode::kSuccess) {
+ LOG(ERROR) << "Error " << utils::ErrorCodeToString(code_) << " (" << code_
+ << ") in DeltaPerformer's Write method when "
+ << "processing the received payload -- Terminating processing";
+ }
// Delete p2p file, if applicable.
if (!p2p_file_id_.empty())
CloseP2PSharingFd(true);
@@ -306,7 +355,8 @@
ErrorCode code =
successful ? ErrorCode::kSuccess : ErrorCode::kDownloadTransferError;
if (code == ErrorCode::kSuccess && delta_performer_.get()) {
- code = delta_performer_->VerifyPayload(payload_->hash, payload_->size);
+ if (!payload_->already_applied)
+ code = delta_performer_->VerifyPayload(payload_->hash, payload_->size);
if (code != ErrorCode::kSuccess) {
LOG(ERROR) << "Download of " << install_plan_.download_url
<< " failed due to payload verification error.";
@@ -315,9 +365,11 @@
CloseP2PSharingFd(true);
} else if (payload_ < &install_plan_.payloads.back() &&
system_state_->payload_state()->NextPayload()) {
+ // No need to reset if this payload was already applied.
+ if (!payload_->already_applied)
+ DeltaPerformer::ResetUpdateProgress(prefs_, false);
// Start downloading next payload.
payload_++;
- DeltaPerformer::ResetUpdateProgress(prefs_, false);
install_plan_.download_url =
system_state_->payload_state()->GetCurrentUrl();
StartDownloading();
@@ -331,9 +383,13 @@
processor_->ActionComplete(this, code);
}
-void DownloadAction::TransferTerminated(HttpFetcher *fetcher) {
+void DownloadAction::TransferTerminated(HttpFetcher* fetcher) {
if (code_ != ErrorCode::kSuccess) {
processor_->ActionComplete(this, code_);
+ } else if (payload_->already_applied) {
+ LOG(INFO) << "TransferTerminated with ErrorCode::kSuccess when the current "
+ "payload has already applied, treating as TransferComplete.";
+ TransferComplete(fetcher, true);
}
}
diff --git a/payload_consumer/download_action.h b/payload_consumer/download_action.h
index 48d6292..d0e6000 100644
--- a/payload_consumer/download_action.h
+++ b/payload_consumer/download_action.h
@@ -27,6 +27,7 @@
#include "update_engine/common/action.h"
#include "update_engine/common/boot_control_interface.h"
#include "update_engine/common/http_fetcher.h"
+#include "update_engine/common/multi_range_http_fetcher.h"
#include "update_engine/payload_consumer/delta_performer.h"
#include "update_engine/payload_consumer/install_plan.h"
#include "update_engine/system_state.h"
@@ -106,6 +107,8 @@
delegate_ = delegate;
}
+ void set_base_offset(int64_t base_offset) { base_offset_ = base_offset; }
+
HttpFetcher* http_fetcher() { return http_fetcher_.get(); }
// Returns the p2p file id for the file being written or the empty
@@ -148,8 +151,8 @@
// Global context for the system.
SystemState* system_state_;
- // Pointer to the HttpFetcher that does the http work.
- std::unique_ptr<HttpFetcher> http_fetcher_;
+ // Pointer to the MultiRangeHttpFetcher that does the http work.
+ std::unique_ptr<MultiRangeHttpFetcher> http_fetcher_;
// The FileWriter that downloaded data should be written to. It will
// either point to *decompressing_file_writer_ or *delta_performer_.
@@ -163,7 +166,8 @@
// For reporting status to outsiders
DownloadActionDelegate* delegate_;
- uint64_t bytes_received_;
+ uint64_t bytes_received_{0};
+ uint64_t bytes_total_{0};
bool download_active_{false};
// The file-id for the file we're sharing or the empty string
@@ -177,6 +181,12 @@
// Set to |false| if p2p file is not visible.
bool p2p_visible_;
+ // Loaded from prefs before downloading any payload.
+ size_t resume_payload_index_{0};
+
+ // Offset of the payload in the download URL, used by UpdateAttempterAndroid.
+ int64_t base_offset_{0};
+
DISALLOW_COPY_AND_ASSIGN(DownloadAction);
};
diff --git a/payload_consumer/download_action_unittest.cc b/payload_consumer/download_action_unittest.cc
index 57910cc..9d85550 100644
--- a/payload_consumer/download_action_unittest.cc
+++ b/payload_consumer/download_action_unittest.cc
@@ -139,7 +139,7 @@
0, writer.Open(output_temp_file.path().c_str(), O_WRONLY | O_CREAT, 0));
writer.set_fail_write(fail_write);
- uint64_t size = data.size();
+ uint64_t size = data.size() - 1;
InstallPlan install_plan;
install_plan.payload_type = InstallPayloadType::kDelta;
install_plan.payloads.push_back({.size = size});
@@ -174,7 +174,7 @@
download_action.set_delegate(&download_delegate);
if (data.size() > kMockHttpFetcherChunkSize)
EXPECT_CALL(download_delegate,
- BytesReceived(_, 1 + kMockHttpFetcherChunkSize, _));
+ BytesReceived(_, kMockHttpFetcherChunkSize, _));
EXPECT_CALL(download_delegate, BytesReceived(_, _, _)).Times(AtLeast(1));
}
ErrorCode expected_code = ErrorCode::kSuccess;
diff --git a/payload_consumer/install_plan.h b/payload_consumer/install_plan.h
index db471da..d8d9f57 100644
--- a/payload_consumer/install_plan.h
+++ b/payload_consumer/install_plan.h
@@ -61,10 +61,15 @@
uint64_t metadata_size = 0; // size of the metadata
std::string metadata_signature; // signature of the metadata in base64
brillo::Blob hash; // SHA256 hash of the payload
+ // Only download manifest and fill in partitions in install plan without
+ // apply the payload if true. Will be set by DownloadAction when resuming
+ // multi-payload.
+ bool already_applied = false;
bool operator==(const Payload& that) const {
return size == that.size && metadata_size == that.metadata_size &&
- metadata_signature == that.metadata_signature && hash == that.hash;
+ metadata_signature == that.metadata_signature &&
+ hash == that.hash && already_applied == that.already_applied;
}
};
std::vector<Payload> payloads;
diff --git a/payload_state.cc b/payload_state.cc
index 287e24c..2a7030f 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -881,6 +881,20 @@
full_payload_attempt_number_);
}
+void PayloadState::SetPayloadIndex(size_t payload_index) {
+ CHECK(prefs_);
+ payload_index_ = payload_index;
+ LOG(INFO) << "Payload Index = " << payload_index_;
+ prefs_->SetInt64(kPrefsUpdateStatePayloadIndex, payload_index_);
+}
+
+bool PayloadState::NextPayload() {
+ if (payload_index_ + 1 >= candidate_urls_.size())
+ return false;
+ SetPayloadIndex(payload_index_ + 1);
+ return true;
+}
+
void PayloadState::LoadUrlIndex() {
SetUrlIndex(GetPersistedValue(kPrefsCurrentUrlIndex));
}
diff --git a/payload_state.h b/payload_state.h
index 56e32fd..699fc74 100644
--- a/payload_state.h
+++ b/payload_state.h
@@ -153,13 +153,7 @@
return attempt_error_code_;
}
- bool NextPayload() override {
- if (payload_index_ + 1 >= candidate_urls_.size())
- return false;
- payload_index_++;
- url_index_ = 0;
- return true;
- }
+ bool NextPayload() override;
private:
enum class AttemptType {
@@ -280,6 +274,11 @@
// of a process restart.
void SetFullPayloadAttemptNumber(int payload_attempt_number);
+ // Sets the current payload index to the given value. Also persists the value
+ // being set so that we resume from the same value in case of a process
+ // restart.
+ void SetPayloadIndex(size_t payload_index);
+
// Initializes the current URL index from the persisted state.
void LoadUrlIndex();
diff --git a/update_attempter.cc b/update_attempter.cc
index 448e29c..ff3b046 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -45,7 +45,6 @@
#include "update_engine/common/clock_interface.h"
#include "update_engine/common/constants.h"
#include "update_engine/common/hardware_interface.h"
-#include "update_engine/common/multi_range_http_fetcher.h"
#include "update_engine/common/platform_constants.h"
#include "update_engine/common/prefs_interface.h"
#include "update_engine/common/subprocess.h"
@@ -619,12 +618,12 @@
LibcurlHttpFetcher* download_fetcher =
new LibcurlHttpFetcher(GetProxyResolver(), system_state_->hardware());
download_fetcher->set_server_to_check(ServerToCheck::kDownload);
- shared_ptr<DownloadAction> download_action(new DownloadAction(
- prefs_,
- system_state_->boot_control(),
- system_state_->hardware(),
- system_state_,
- new MultiRangeHttpFetcher(download_fetcher))); // passes ownership
+ shared_ptr<DownloadAction> download_action(
+ new DownloadAction(prefs_,
+ system_state_->boot_control(),
+ system_state_->hardware(),
+ system_state_,
+ download_fetcher)); // passes ownership
shared_ptr<OmahaRequestAction> download_finished_action(
new OmahaRequestAction(
system_state_,
@@ -1038,7 +1037,6 @@
new_payload_size_ = 0;
for (const auto& payload : plan.payloads)
new_payload_size_ += payload.size;
- SetupDownload();
cpu_limiter_.StartLimiter();
SetStatusAndNotify(UpdateStatus::UPDATE_AVAILABLE);
} else if (type == DownloadAction::StaticType()) {
@@ -1326,35 +1324,6 @@
prefs_->SetInt64(kPrefsDeltaUpdateFailures, ++delta_failures);
}
-void UpdateAttempter::SetupDownload() {
- MultiRangeHttpFetcher* fetcher =
- static_cast<MultiRangeHttpFetcher*>(download_action_->http_fetcher());
- fetcher->ClearRanges();
- if (response_handler_action_->install_plan().is_resume) {
- // Resuming an update so fetch the update manifest metadata 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);
- fetcher->AddRange(0, 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.
- int64_t next_data_offset = 0;
- prefs_->GetInt64(kPrefsUpdateStateNextDataOffset, &next_data_offset);
- uint64_t resume_offset =
- manifest_metadata_size + manifest_signature_size + next_data_offset;
- int64_t payload_index = 0;
- prefs_->GetInt64(kPrefsUpdateStatePayloadIndex, &payload_index);
- if (resume_offset <
- response_handler_action_->install_plan().payloads[payload_index].size) {
- fetcher->AddRange(resume_offset);
- }
- } else {
- fetcher->AddRange(0);
- }
-}
-
void UpdateAttempter::PingOmaha() {
if (!processor_->IsRunning()) {
shared_ptr<OmahaRequestAction> ping_action(new OmahaRequestAction(
diff --git a/update_attempter.h b/update_attempter.h
index 193e172..7780357 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -281,9 +281,6 @@
// Sets the status to the given status and notifies a status update over dbus.
void SetStatusAndNotify(UpdateStatus status);
- // Sets up the download parameters after receiving the update check response.
- void SetupDownload();
-
// Creates an error event object in |error_event_| to be included in an
// OmahaRequestAction once the current action processor is done.
void CreatePendingErrorEvent(AbstractAction* action, ErrorCode code);
diff --git a/update_attempter_android.cc b/update_attempter_android.cc
index a39379f..09549f8 100644
--- a/update_attempter_android.cc
+++ b/update_attempter_android.cc
@@ -30,7 +30,6 @@
#include "update_engine/common/constants.h"
#include "update_engine/common/file_fetcher.h"
-#include "update_engine/common/multi_range_http_fetcher.h"
#include "update_engine/common/utils.h"
#include "update_engine/daemon_state_interface.h"
#include "update_engine/network_selector.h"
@@ -207,7 +206,6 @@
install_plan_.Dump();
BuildUpdateActions(payload_url);
- SetupDownload();
// Setup extra headers.
HttpFetcher* fetcher = download_action_->http_fetcher();
if (!headers[kPayloadPropertyAuthorization].empty())
@@ -459,12 +457,12 @@
download_fetcher = libcurl_fetcher;
#endif // _UE_SIDELOAD
}
- shared_ptr<DownloadAction> download_action(new DownloadAction(
- prefs_,
- boot_control_,
- hardware_,
- nullptr, // system_state, not used.
- new MultiRangeHttpFetcher(download_fetcher))); // passes ownership
+ shared_ptr<DownloadAction> download_action(
+ new DownloadAction(prefs_,
+ boot_control_,
+ hardware_,
+ nullptr, // system_state, not used.
+ download_fetcher)); // passes ownership
shared_ptr<FilesystemVerifierAction> filesystem_verifier_action(
new FilesystemVerifierAction());
@@ -472,6 +470,7 @@
new PostinstallRunnerAction(boot_control_, hardware_));
download_action->set_delegate(this);
+ download_action->set_base_offset(base_offset_);
download_action_ = download_action;
postinstall_runner_action->set_delegate(this);
@@ -492,42 +491,6 @@
processor_->EnqueueAction(action.get());
}
-void UpdateAttempterAndroid::SetupDownload() {
- MultiRangeHttpFetcher* fetcher =
- static_cast<MultiRangeHttpFetcher*>(download_action_->http_fetcher());
- fetcher->ClearRanges();
- if (install_plan_.is_resume) {
- // Resuming an update so fetch the update manifest metadata 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);
- 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.
- int64_t next_data_offset = 0;
- prefs_->GetInt64(kPrefsUpdateStateNextDataOffset, &next_data_offset);
- uint64_t resume_offset =
- manifest_metadata_size + manifest_signature_size + next_data_offset;
- if (!install_plan_.payloads[0].size) {
- fetcher->AddRange(base_offset_ + resume_offset);
- } else if (resume_offset < install_plan_.payloads[0].size) {
- fetcher->AddRange(base_offset_ + resume_offset,
- install_plan_.payloads[0].size - resume_offset);
- }
- } else {
- if (install_plan_.payloads[0].size) {
- fetcher->AddRange(base_offset_, install_plan_.payloads[0].size);
- } else {
- // If no payload size is passed we assume we read until the end of the
- // stream.
- fetcher->AddRange(base_offset_);
- }
- }
-}
-
bool UpdateAttempterAndroid::WriteUpdateCompletedMarker() {
string boot_id;
TEST_AND_RETURN_FALSE(utils::GetBootId(&boot_id));
diff --git a/update_attempter_android.h b/update_attempter_android.h
index 6a5c227..167191e 100644
--- a/update_attempter_android.h
+++ b/update_attempter_android.h
@@ -110,10 +110,6 @@
// applying an update from the given |url|.
void BuildUpdateActions(const std::string& url);
- // Sets up the download parameters based on the update requested on the
- // |install_plan_|.
- void SetupDownload();
-
// Writes to the processing completed marker. Does nothing if
// |update_completed_marker_| is empty.
bool WriteUpdateCompletedMarker();
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 17baaa0..4928477 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -218,7 +218,6 @@
DownloadAction action(prefs_, nullptr, nullptr, nullptr, fetcher.release());
EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _)).Times(0);
attempter_.ActionCompleted(nullptr, &action, ErrorCode::kSuccess);
- EXPECT_EQ(503, attempter_.http_response_code());
EXPECT_EQ(UpdateStatus::FINALIZING, attempter_.status());
ASSERT_EQ(nullptr, attempter_.error_event_.get());
}