Add progress updates to FilesystemVerificationAction
An attempt to fix b/142525610. This CL adds progress reports to
FilesystemVerificationAction. So that clients(e.x. GMSCore) could
properly display a progress bar instead of hanging there for a
few minutes. GMSCore support is already done, see
https://critique-ng.corp.google.com/cl/317362455
Test: Did an OTA update and observed that verification progress
is ported from android OTA client
Bug: 142525610
Change-Id: I1cdb1970cb65fc767b49cd38650894eed4d90960
diff --git a/payload_consumer/filesystem_verifier_action.cc b/payload_consumer/filesystem_verifier_action.cc
index 36e5a35..b6affc3 100644
--- a/payload_consumer/filesystem_verifier_action.cc
+++ b/payload_consumer/filesystem_verifier_action.cc
@@ -76,9 +76,16 @@
return;
if (code == ErrorCode::kSuccess && HasOutputPipe())
SetOutputObject(install_plan_);
+ UpdateProgress(1.0);
processor_->ActionComplete(this, code);
}
+void FilesystemVerifierAction::UpdateProgress(double progress) {
+ if (delegate_ != nullptr) {
+ delegate_->OnVerifyProgressUpdate(progress);
+ }
+}
+
void FilesystemVerifierAction::StartPartitionHashing() {
if (partition_index_ == install_plan_.partitions.size()) {
Cleanup(ErrorCode::kSuccess);
@@ -187,7 +194,6 @@
Cleanup(ErrorCode::kError);
return;
}
-
if (bytes_read == 0) {
LOG(ERROR) << "Failed to read the remaining " << partition_size_ - offset_
<< " bytes from partition "
@@ -202,6 +208,13 @@
return;
}
+ // WE don't consider sizes of each partition. Every partition
+ // has the same length on progress bar.
+ // TODO(zhangkelvin) Take sizes of each partition into account
+
+ UpdateProgress(
+ (static_cast<double>(offset_) / partition_size_ + partition_index_) /
+ install_plan_.partitions.size());
if (verifier_step_ == VerifierStep::kVerifyTargetHash &&
install_plan_.write_verity) {
if (!verity_writer_->Update(offset_, buffer_.data(), bytes_read)) {
diff --git a/payload_consumer/filesystem_verifier_action.h b/payload_consumer/filesystem_verifier_action.h
index 83d6668..6ef3d16 100644
--- a/payload_consumer/filesystem_verifier_action.h
+++ b/payload_consumer/filesystem_verifier_action.h
@@ -49,6 +49,12 @@
kVerifySourceHash,
};
+class FilesystemVerifyDelegate {
+ public:
+ virtual ~FilesystemVerifyDelegate() = default;
+ virtual void OnVerifyProgressUpdate(double progress) = 0;
+};
+
class FilesystemVerifierAction : public InstallPlanAction {
public:
FilesystemVerifierAction()
@@ -58,6 +64,14 @@
void PerformAction() override;
void TerminateProcessing() override;
+ // Used for listening to progress updates
+ void set_delegate(FilesystemVerifyDelegate* delegate) {
+ this->delegate_ = delegate;
+ }
+ [[nodiscard]] FilesystemVerifyDelegate* get_delegate() const {
+ return this->delegate_;
+ }
+
// Debugging/logging
static std::string StaticType() { return "FilesystemVerifierAction"; }
std::string Type() const override { return StaticType(); }
@@ -85,6 +99,9 @@
// true if TerminateProcessing() was called.
void Cleanup(ErrorCode code);
+ // Invoke delegate callback to report progress, if delegate is not null
+ void UpdateProgress(double progress);
+
// The type of the partition that we are verifying.
VerifierStep verifier_step_ = VerifierStep::kVerifyTargetHash;
@@ -119,6 +136,9 @@
// The byte offset that we are reading in the current partition.
uint64_t offset_{0};
+ // An observer that observes progress updates of this action.
+ FilesystemVerifyDelegate* delegate_{};
+
DISALLOW_COPY_AND_ASSIGN(FilesystemVerifierAction);
};
diff --git a/update_attempter_android.cc b/update_attempter_android.cc
index b7d119f..4d74379 100644
--- a/update_attempter_android.cc
+++ b/update_attempter_android.cc
@@ -576,9 +576,9 @@
cleanup_previous_update_code_ = code;
NotifyCleanupPreviousUpdateCallbacksAndClear();
}
- if (type == DownloadAction::StaticType()) {
- download_progress_ = 0;
- }
+ // download_progress_ is actually used by other actions, such as
+ // filesystem_verify_action. Therefore we always clear it.
+ download_progress_ = 0;
if (type == PostinstallRunnerAction::StaticType()) {
bool succeeded =
code == ErrorCode::kSuccess || code == ErrorCode::kUpdatedButNotActive;
@@ -592,8 +592,9 @@
SetStatusAndNotify(UpdateStatus::CLEANUP_PREVIOUS_UPDATE);
}
if (type == DownloadAction::StaticType()) {
- SetStatusAndNotify(UpdateStatus::FINALIZING);
+ SetStatusAndNotify(UpdateStatus::VERIFYING);
} else if (type == FilesystemVerifierAction::StaticType()) {
+ SetStatusAndNotify(UpdateStatus::FINALIZING);
prefs_->SetBoolean(kPrefsVerityWritten, true);
}
}
@@ -644,6 +645,11 @@
}
}
+void UpdateAttempterAndroid::OnVerifyProgressUpdate(double progress) {
+ assert(status_ == UpdateStatus::VERIFYING);
+ ProgressUpdate(progress);
+}
+
void UpdateAttempterAndroid::ScheduleProcessingStart() {
LOG(INFO) << "Scheduling an action processor start.";
brillo::MessageLoop::current()->PostTask(
@@ -734,6 +740,7 @@
std::make_unique<FilesystemVerifierAction>();
auto postinstall_runner_action =
std::make_unique<PostinstallRunnerAction>(boot_control_, hardware_);
+ filesystem_verifier_action->set_delegate(this);
postinstall_runner_action->set_delegate(this);
// Bond them together. We have to use the leaf-types when calling
diff --git a/update_attempter_android.h b/update_attempter_android.h
index f8c78de..55003a0 100644
--- a/update_attempter_android.h
+++ b/update_attempter_android.h
@@ -37,6 +37,7 @@
#include "update_engine/metrics_utils.h"
#include "update_engine/network_selector_interface.h"
#include "update_engine/payload_consumer/download_action.h"
+#include "update_engine/payload_consumer/filesystem_verifier_action.h"
#include "update_engine/payload_consumer/postinstall_runner_action.h"
#include "update_engine/service_delegate_android_interface.h"
#include "update_engine/service_observer_interface.h"
@@ -47,6 +48,7 @@
: public ServiceDelegateAndroidInterface,
public ActionProcessorDelegate,
public DownloadActionDelegate,
+ public FilesystemVerifyDelegate,
public PostinstallRunnerAction::DelegateInterface,
public CleanupPreviousUpdateActionDelegateInterface {
public:
@@ -101,6 +103,9 @@
bool ShouldCancel(ErrorCode* cancel_reason) override;
void DownloadComplete() override;
+ // FilesystemVerifyDelegate overrides
+ void OnVerifyProgressUpdate(double progress) override;
+
// PostinstallRunnerAction::DelegateInterface
void ProgressUpdate(double progress) override;