Move MarkBootSuccessful to BootControlInterface.
Updating the boot flags to mark the current boot as successful is
platform-specific and part of the BootControlInterface's job. This
patch moves this to a new async method in this interface.
Bug: 24077637
Test: FEATURES=test emerge-link update_engine; cros flash in Chrome OS; tested on a dragonboard.
Change-Id: I23c3ed915dd8d2588a90d84b212bb04977957975
diff --git a/boot_control_android.cc b/boot_control_android.cc
index 4bb3b92..43a1c18 100644
--- a/boot_control_android.cc
+++ b/boot_control_android.cc
@@ -16,10 +16,12 @@
#include "update_engine/boot_control_android.h"
-#include <base/logging.h>
+#include <base/bind.h>
#include <base/files/file_util.h>
+#include <base/logging.h>
#include <base/strings/string_util.h>
#include <chromeos/make_unique_ptr.h>
+#include <chromeos/message_loops/message_loop.h>
#include <cutils/properties.h>
#include <fs_mgr.h>
@@ -174,4 +176,15 @@
return ret == 0;
}
+bool BootControlAndroid::MarkBootSuccessfulAsync(
+ base::Callback<void(bool)> callback) {
+ int ret = module_->markBootSuccessful(module_);
+ if (ret < 0) {
+ LOG(ERROR) << "Unable to mark boot successful: " << strerror(-ret);
+ }
+ return chromeos::MessageLoop::current()->PostTask(
+ FROM_HERE, base::Bind(callback, ret == 0)) !=
+ chromeos::MessageLoop::kTaskIdNull;
+}
+
} // namespace chromeos_update_engine
diff --git a/boot_control_android.h b/boot_control_android.h
index 2e218a0..f429d9b 100644
--- a/boot_control_android.h
+++ b/boot_control_android.h
@@ -45,6 +45,7 @@
std::string* device) const override;
bool IsSlotBootable(BootControlInterface::Slot slot) const override;
bool MarkSlotUnbootable(BootControlInterface::Slot slot) override;
+ bool MarkBootSuccessfulAsync(base::Callback<void(bool)> callback) override;
private:
// NOTE: There is no way to release/unload HAL implementations so
diff --git a/boot_control_chromeos.cc b/boot_control_chromeos.cc
index 03e1c26..5f8f74e 100644
--- a/boot_control_chromeos.cc
+++ b/boot_control_chromeos.cc
@@ -18,6 +18,7 @@
#include <string>
+#include <base/bind.h>
#include <base/files/file_path.h>
#include <base/files/file_util.h>
#include <base/strings/string_util.h>
@@ -29,6 +30,7 @@
}
#include "update_engine/boot_control.h"
+#include "update_engine/subprocess.h"
#include "update_engine/utils.h"
using std::string;
@@ -57,6 +59,15 @@
return boot_path;
}
+// ExecCallback called when the execution of setgoodkernel finishes. Notifies
+// the caller of MarkBootSuccessfullAsync() by calling |callback| with the
+// result.
+void OnMarkBootSuccessfulDone(base::Callback<void(bool)> callback,
+ int return_code,
+ const string& output) {
+ callback.Run(return_code == 0);
+}
+
} // namespace
namespace chromeos_update_engine {
@@ -194,6 +205,13 @@
return true;
}
+bool BootControlChromeOS::MarkBootSuccessfulAsync(
+ base::Callback<void(bool)> callback) {
+ return Subprocess::Get().Exec(
+ {"/usr/sbin/chromeos-setgoodkernel"},
+ base::Bind(&OnMarkBootSuccessfulDone, callback)) != 0;
+}
+
// static
string BootControlChromeOS::SysfsBlockDevice(const string& device) {
base::FilePath device_path(device);
diff --git a/boot_control_chromeos.h b/boot_control_chromeos.h
index a7c269e..6f5497b 100644
--- a/boot_control_chromeos.h
+++ b/boot_control_chromeos.h
@@ -19,6 +19,7 @@
#include <string>
+#include <base/callback.h>
#include <gtest/gtest_prod.h> // for FRIEND_TEST
#include "update_engine/boot_control_interface.h"
@@ -47,6 +48,7 @@
std::string* device) const override;
bool IsSlotBootable(BootControlInterface::Slot slot) const override;
bool MarkSlotUnbootable(BootControlInterface::Slot slot) override;
+ bool MarkBootSuccessfulAsync(base::Callback<void(bool)> callback) override;
private:
friend class BootControlChromeOSTest;
diff --git a/boot_control_interface.h b/boot_control_interface.h
index 135d2eb..36f2caf 100644
--- a/boot_control_interface.h
+++ b/boot_control_interface.h
@@ -20,6 +20,7 @@
#include <climits>
#include <string>
+#include <base/callback.h>
#include <base/macros.h>
namespace chromeos_update_engine {
@@ -64,6 +65,12 @@
// Returns true on success.
virtual bool MarkSlotUnbootable(Slot slot) = 0;
+ // Mark the current slot as successfully booted asynchronously. No other slot
+ // flags are modified. Returns false if it was not able to schedule the
+ // operation, otherwise, returns true and calls the |callback| with the result
+ // of the operation.
+ virtual bool MarkBootSuccessfulAsync(base::Callback<void(bool)> callback) = 0;
+
// Return a human-readable slot name used for logging.
static std::string SlotName(Slot slot) {
if (slot == kInvalidSlot)
diff --git a/fake_boot_control.h b/fake_boot_control.h
index 508f578f..9d1b54b 100644
--- a/fake_boot_control.h
+++ b/fake_boot_control.h
@@ -65,6 +65,13 @@
return true;
}
+ bool MarkBootSuccessfulAsync(base::Callback<void(bool)> callback) override {
+ // We run the callback directly from here to avoid having to setup a message
+ // loop in the test environment.
+ callback.Run(true);
+ return true;
+ }
+
// Setters
void SetNumSlots(unsigned int num_slots) {
num_slots_ = num_slots;
diff --git a/update_attempter.cc b/update_attempter.cc
index f1eff95..c5320f7 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -41,6 +41,7 @@
#include <power_manager/dbus-proxies.h>
#include <update_engine/dbus-constants.h>
+#include "update_engine/boot_control_interface.h"
#include "update_engine/certificate_checker.h"
#include "update_engine/clock_interface.h"
#include "update_engine/constants.h"
@@ -1185,22 +1186,15 @@
// the script asynchronously to avoid blocking the event loop regardless of
// the script runtime.
update_boot_flags_running_ = true;
- LOG(INFO) << "Updating boot flags...";
- vector<string> cmd{"/usr/sbin/chromeos-setgoodkernel"};
- if (skip_set_good_kernel_) {
- CompleteUpdateBootFlags(1, "Skipping the call to set");
- } else {
- if (!Subprocess::Get().Exec(cmd,
- Bind(&UpdateAttempter::CompleteUpdateBootFlags,
- base::Unretained(this)))) {
- CompleteUpdateBootFlags(
- 1, "Failed to launch process to mark kernel as good");
- }
+ LOG(INFO) << "Marking booted slot as good.";
+ if (!system_state_->boot_control()->MarkBootSuccessfulAsync(Bind(
+ &UpdateAttempter::CompleteUpdateBootFlags, base::Unretained(this)))) {
+ LOG(ERROR) << "Failed to mark current boot as successful.";
+ CompleteUpdateBootFlags(false);
}
}
-void UpdateAttempter::CompleteUpdateBootFlags(int return_code,
- const string& output) {
+void UpdateAttempter::CompleteUpdateBootFlags(bool successful) {
update_boot_flags_running_ = false;
updated_boot_flags_ = true;
if (start_action_processor_) {
diff --git a/update_attempter.h b/update_attempter.h
index 12da537..06a3dac 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -124,8 +124,8 @@
// attempt goes through this method.
void UpdateBootFlags();
- // Subprocess::Exec callback.
- void CompleteUpdateBootFlags(int return_code, const std::string& output);
+ // Called when the boot flags have been updated.
+ void CompleteUpdateBootFlags(bool success);
UpdateStatus status() const { return status_; }
@@ -473,10 +473,6 @@
// True if UpdateBootFlags is running.
bool update_boot_flags_running_ = false;
- // Whether we should skip the async call to "setgoodkernel" command. Used in
- // unittests.
- bool skip_set_good_kernel_ = false;
-
// True if the action processor needs to be started by the boot flag updater.
bool start_action_processor_ = false;
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 1726585..10ab843 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -125,8 +125,6 @@
// Finish initializing the attempter.
attempter_.Init();
- // Don't run setgoodkernel command.
- attempter_.skip_set_good_kernel_ = true;
}
void SetUp() override {