update_engine: Respond to forced update requests when updates are disabled.
With the new UpdateCheckAllowed policy, we stopped changing the reported
state of the updater when update is permanently disabled, for example
when running from a USB image. This caused a problem whereas client code
(e.g. OOBE) expected the state of the updater to change as an indicator
for the fact that the update request is being processed.
This change fixes that by doing the following:
- Adds a new state (disabled) that is being signaled from the
UpdateAttempter through the system API to DBus clients. This state is
being used when UpdateCheckAllowed indicates that updates are
permanently disabled; the state is immediately set the idle again, so
that subsequent requests can also witness the state change.
- The code that rescheduled checks when the updater state was switched
back to idle was pulled out of that method and placed at the callsites
instead. It is now safe to call SetStatusAndNotify() from the callback
of ScheduleUpdate() without incurring an infinite call chain.
- When an update request is received via DBus, we now make sure to call
ScheduleUpdate() prior to broadcasting the forced update pending
status. This ensures that the policy will be called on each DBus
request, even if updates were formerly disabled.
BUG=chromium:417751
TEST=Unit tests.
TEST=Booting from USB on link, OOBE passes as expected.
TEST=update_engine_client yields the correct sequence of status readings.
CQ-DEPEND=CL:220374
Change-Id: I71eee1674d60956e39f953d44df86b61effd7800
Reviewed-on: https://chromium-review.googlesource.com/220404
Tested-by: Gilad Arnold <garnold@chromium.org>
Reviewed-by: Alex Deymo <deymo@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
diff --git a/update_attempter.cc b/update_attempter.cc
index 2dc1b42..bc18945 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -98,6 +98,8 @@
return update_engine::kUpdateStatusReportingErrorEvent;
case UPDATE_STATUS_ATTEMPTING_ROLLBACK:
return update_engine::kUpdateStatusAttemptingRollback;
+ case UPDATE_STATUS_DISABLED:
+ return update_engine::kUpdateStatusDisabled;
default:
return "unknown status";
}
@@ -158,6 +160,9 @@
}
void UpdateAttempter::ScheduleUpdates() {
+ if (IsUpdateRunningOrScheduled())
+ return;
+
chromeos_update_manager::UpdateManager* const update_manager =
system_state_->update_manager();
CHECK(update_manager);
@@ -808,8 +813,12 @@
LOG(INFO) << "Forced update check requested.";
forced_app_version_ = app_version;
forced_omaha_url_ = omaha_url;
- if (forced_update_pending_callback_.get())
+ if (forced_update_pending_callback_.get()) {
+ // Make sure that a scheduling request is made prior to calling the forced
+ // update pending callback.
+ ScheduleUpdates();
forced_update_pending_callback_->Run(true, interactive);
+ }
}
bool UpdateAttempter::RebootIfNeeded() {
@@ -887,6 +896,13 @@
if (status == EvalStatus::kSucceeded) {
if (!params.updates_enabled) {
LOG(WARNING) << "Updates permanently disabled.";
+ // Signal disabled status, then switch right back to idle. This is
+ // necessary for ensuring that observers waiting for a signal change will
+ // actually notice one on subsequent calls. Note that we don't need to
+ // re-schedule a check in this case as updates are permanently disabled;
+ // further (forced) checks may still initiate a scheduling call.
+ SetStatusAndNotify(UPDATE_STATUS_DISABLED);
+ SetStatusAndNotify(UPDATE_STATUS_IDLE);
return;
}
@@ -916,9 +932,7 @@
// a bug that will most likely prevent further automatic update checks. It
// seems better to crash in such cases and restart the update_engine daemon
// into, hopefully, a known good state.
- CHECK((this->status() != UPDATE_STATUS_IDLE &&
- this->status() != UPDATE_STATUS_UPDATED_NEED_REBOOT) ||
- waiting_for_scheduled_check_);
+ CHECK(IsUpdateRunningOrScheduled());
}
void UpdateAttempter::UpdateLastCheckedTime() {
@@ -939,6 +953,7 @@
// Inform scheduler of new status;
SetStatusAndNotify(UPDATE_STATUS_IDLE);
+ ScheduleUpdates();
if (!fake_update_success_) {
return;
@@ -970,6 +985,7 @@
prefs_->Delete(kPrefsUpdateFirstSeenAt);
SetStatusAndNotify(UPDATE_STATUS_UPDATED_NEED_REBOOT);
+ ScheduleUpdates();
LOG(INFO) << "Update successfully applied, waiting to reboot.";
// This pointer is null during rollback operations, and the stats
@@ -1006,6 +1022,7 @@
}
LOG(INFO) << "No update.";
SetStatusAndNotify(UPDATE_STATUS_IDLE);
+ ScheduleUpdates();
}
void UpdateAttempter::ProcessingStopped(const ActionProcessor* processor) {
@@ -1013,6 +1030,7 @@
CleanupCpuSharesManagement();
download_progress_ = 0.0;
SetStatusAndNotify(UPDATE_STATUS_IDLE);
+ ScheduleUpdates();
actions_.clear();
error_event_.reset(nullptr);
}
@@ -1255,11 +1273,6 @@
void UpdateAttempter::SetStatusAndNotify(UpdateStatus status) {
status_ = status;
- // If not updating, schedule subsequent update checks.
- if (status_ == UPDATE_STATUS_IDLE ||
- status_ == UPDATE_STATUS_UPDATED_NEED_REBOOT) {
- ScheduleUpdates();
- }
BroadcastStatus();
}
@@ -1465,6 +1478,7 @@
// Update the status which will schedule the next update check
SetStatusAndNotify(UPDATE_STATUS_UPDATED_NEED_REBOOT);
+ ScheduleUpdates();
}
@@ -1597,4 +1611,10 @@
return true;
}
+bool UpdateAttempter::IsUpdateRunningOrScheduled() {
+ return ((status_ != UPDATE_STATUS_IDLE &&
+ status_ != UPDATE_STATUS_UPDATED_NEED_REBOOT) ||
+ waiting_for_scheduled_check_);
+}
+
} // namespace chromeos_update_engine