update_engine: Do not cancel external powerwash in invalidations
Do not cancel powerwash initiated with reason != update_engine during
update invalidations.
Also keep the powerwash marker file in case if there is an error when
fetching the powerwash reason.
BUG=None
TEST=cros_run_unit_tests --board=BOARD --packages update_engine
TEST=tast run -var=tlwAddress=<DUT> autoupdate.OmahaInvalidation.*
Change-Id: I6d1d5c5a68476643674bebf7d9a4f0fd9cde7ede
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/4957016
Commit-Queue: Artyom Chen <artyomchen@google.com>
Reviewed-by: John Admanski <jadmanski@chromium.org>
Reviewed-by: Jae Hoon Kim <kimjae@chromium.org>
Tested-by: Artyom Chen <artyomchen@google.com>
diff --git a/common/fake_hardware.h b/common/fake_hardware.h
index 65a6897..64b7c11 100644
--- a/common/fake_hardware.h
+++ b/common/fake_hardware.h
@@ -19,6 +19,7 @@
#include <map>
#include <memory>
+#include <optional>
#include <string>
#include <utility>
@@ -299,6 +300,17 @@
}
bool IsFWTryNextSlotReset() const { return reset_fw_try_next_slot_; }
+ std::optional<bool> IsPowerwashScheduledByUpdateEngine() const override {
+ return is_powerwash_scheduled_by_update_engine_;
+ }
+ void SetIsPowerwashScheduledByUpdateEngine(std::optional<bool> value) {
+ is_powerwash_scheduled_by_update_engine_ = value;
+ }
+
+ base::FilePath GetPowerwashMarkerFullPath() const override {
+ return base::FilePath();
+ };
+
private:
bool is_official_build_{true};
bool is_normal_boot_mode_{true};
@@ -318,6 +330,7 @@
int kernel_max_rollforward_{kKernelMaxRollforward};
int firmware_max_rollforward_{kFirmwareMaxRollforward};
int powerwash_count_{kPowerwashCountNotSet};
+ std::optional<bool> is_powerwash_scheduled_by_update_engine_{true};
bool powerwash_scheduled_{false};
bool save_rollback_data_{false};
int64_t build_timestamp_{0};
diff --git a/common/hardware_interface.h b/common/hardware_interface.h
index 9d7be40..ebae384 100644
--- a/common/hardware_interface.h
+++ b/common/hardware_interface.h
@@ -20,6 +20,7 @@
#include <stdint.h>
#include <memory>
+#include <optional>
#include <string>
#include <vector>
@@ -115,6 +116,12 @@
// recovery don't have this value set.
virtual int GetPowerwashCount() const = 0;
+ // Parses the powerwash marker file and returns if powerwash is
+ // initiated by update engine. If the file is not found, returns
+ // std::nullopt. If parsing failed, returns false.
+ // Otherwise returns if a marker file contains reason=update_engine.
+ virtual std::optional<bool> IsPowerwashScheduledByUpdateEngine() const = 0;
+
// Signals that a powerwash (stateful partition wipe) should be performed
// after reboot. If |save_rollback_data| is true additional state is
// preserved during shutdown that can be restored after the powerwash.
@@ -199,6 +206,9 @@
// Resets a RW firmware partition slot to try on next boot to a current slot.
// Returns false on failure, true on success.
virtual bool ResetFWTryNextSlot() = 0;
+
+ // Returns an absolute path to a powerwash marker file.
+ virtual base::FilePath GetPowerwashMarkerFullPath() const = 0;
};
} // namespace chromeos_update_engine
diff --git a/cros/hardware_chromeos.cc b/cros/hardware_chromeos.cc
index c7f72d8..24f4bed 100644
--- a/cros/hardware_chromeos.cc
+++ b/cros/hardware_chromeos.cc
@@ -16,6 +16,7 @@
#include "update_engine/cros/hardware_chromeos.h"
+#include <optional>
#include <utility>
#include <base/files/file_path.h>
@@ -64,10 +65,14 @@
// a powerwash is performed.
const char kPowerwashCountMarker[] = "powerwash_count";
-// The name of the marker file used to trigger powerwash when post-install
+// The path of the marker file used to trigger powerwash when post-install
// completes successfully so that the device is powerwashed on next reboot.
-const char kPowerwashMarkerFile[] =
- "/mnt/stateful_partition/factory_install_reset";
+constexpr char kPowerwashMarkerPath[] =
+ "mnt/stateful_partition/factory_install_reset";
+
+// Expected tag in the powerwash marker file that indicates that
+// powerwash is initiated by the update engine.
+constexpr char kPowerwashReasonUpdateEngineTag[] = "reason=update_engine";
// The name of the marker file used to trigger a save of rollback data
// during the next shutdown.
@@ -326,29 +331,52 @@
}
auto powerwash_command = GeneratePowerwashCommand(save_rollback_data);
- bool result = utils::WriteFile(
- kPowerwashMarkerFile, powerwash_command.data(), powerwash_command.size());
+ const std::string powerwash_marker_full_path =
+ GetPowerwashMarkerFullPath().value();
+ bool result = utils::WriteFile(powerwash_marker_full_path.c_str(),
+ powerwash_command.data(),
+ powerwash_command.size());
if (result) {
- LOG(INFO) << "Created " << kPowerwashMarkerFile
+ LOG(INFO) << "Created " << powerwash_marker_full_path
<< " to powerwash on next reboot ("
<< "save_rollback_data=" << save_rollback_data << ")";
} else {
PLOG(ERROR) << "Error in creating powerwash marker file: "
- << kPowerwashMarkerFile;
+ << powerwash_marker_full_path;
}
return result;
}
+std::optional<bool> HardwareChromeOS::IsPowerwashScheduledByUpdateEngine()
+ const {
+ const std::string powerwash_marker_full_path =
+ GetPowerwashMarkerFullPath().value();
+
+ if (!utils::FileExists(powerwash_marker_full_path.c_str())) {
+ return std::nullopt;
+ }
+
+ std::string contents;
+ if (!utils::ReadFile(powerwash_marker_full_path, &contents)) {
+ LOG(ERROR) << "Failed to read the powerwash marker file.";
+ return false;
+ }
+
+ return contents.find(kPowerwashReasonUpdateEngineTag) != std::string::npos;
+}
+
bool HardwareChromeOS::CancelPowerwash() {
- bool result = base::DeleteFile(base::FilePath(kPowerwashMarkerFile));
+ const base::FilePath powerwash_marker_full_path =
+ GetPowerwashMarkerFullPath();
+ bool result = base::DeleteFile(powerwash_marker_full_path);
if (result) {
LOG(INFO) << "Successfully deleted the powerwash marker file : "
- << kPowerwashMarkerFile;
+ << powerwash_marker_full_path;
} else {
PLOG(ERROR) << "Could not delete the powerwash marker file : "
- << kPowerwashMarkerFile;
+ << powerwash_marker_full_path;
}
// Delete the rollback save marker file if it existed.
@@ -661,4 +689,8 @@
return true;
}
+base::FilePath HardwareChromeOS::GetPowerwashMarkerFullPath() const {
+ return root_.Append(kPowerwashMarkerPath);
+}
+
} // namespace chromeos_update_engine
diff --git a/cros/hardware_chromeos.h b/cros/hardware_chromeos.h
index f68431e..bce8bc2 100644
--- a/cros/hardware_chromeos.h
+++ b/cros/hardware_chromeos.h
@@ -18,6 +18,7 @@
#define UPDATE_ENGINE_CROS_HARDWARE_CHROMEOS_H_
#include <memory>
+#include <optional>
#include <string>
#include <vector>
@@ -62,6 +63,7 @@
bool SetMaxFirmwareKeyRollforward(int firmware_max_rollforward) override;
bool SetMaxKernelKeyRollforward(int kernel_max_rollforward) override;
int GetPowerwashCount() const override;
+ std::optional<bool> IsPowerwashScheduledByUpdateEngine() const override;
// Must not be called prior to boot control initialization.
bool SchedulePowerwash(bool save_rollback_data) override;
bool CancelPowerwash() override;
@@ -91,6 +93,8 @@
bool ResetFWTryNextSlot() override;
+ base::FilePath GetPowerwashMarkerFullPath() const override;
+
private:
friend class HardwareChromeOSTest;
FRIEND_TEST(HardwareChromeOSTest, GeneratePowerwashCommandCheck);
diff --git a/cros/hardware_chromeos_unittest.cc b/cros/hardware_chromeos_unittest.cc
index f60337c..1cd58ee 100644
--- a/cros/hardware_chromeos_unittest.cc
+++ b/cros/hardware_chromeos_unittest.cc
@@ -17,6 +17,7 @@
#include "update_engine/cros/hardware_chromeos.h"
#include <memory>
+#include <optional>
#include <utility>
#include <base/files/file_util.h>
@@ -467,4 +468,57 @@
ASSERT_FALSE(result);
}
+TEST_F(HardwareChromeOSTest, IsPowerwashScheduledByUpdateEngineValidReason) {
+ base::FilePath root_path = root_dir_.GetPath();
+ hardware_.SetRootForTest(root_path);
+ base::FilePath marker_path = hardware_.GetPowerwashMarkerFullPath();
+ std::string marker_contents = "safe reason=update_engine\n test_key";
+ ASSERT_TRUE(brillo::TouchFile(marker_path));
+ ASSERT_TRUE(base::WriteFile(marker_path, marker_contents));
+
+ std::optional<bool> result = hardware_.IsPowerwashScheduledByUpdateEngine();
+
+ EXPECT_TRUE(result);
+ EXPECT_TRUE(result.value());
+}
+
+TEST_F(HardwareChromeOSTest, IsPowerwashScheduledByUpdateEngineNoMarker) {
+ base::FilePath root_path = root_dir_.GetPath();
+ hardware_.SetRootForTest(root_path);
+ base::FilePath marker_path = hardware_.GetPowerwashMarkerFullPath();
+ ASSERT_FALSE(base::PathExists(marker_path));
+
+ std::optional<bool> result = hardware_.IsPowerwashScheduledByUpdateEngine();
+
+ EXPECT_FALSE(result);
+}
+
+TEST_F(HardwareChromeOSTest, IsPowerwashScheduledByUpdateEngineNoReasonKey) {
+ base::FilePath root_path = root_dir_.GetPath();
+ hardware_.SetRootForTest(root_path);
+ base::FilePath marker_path = hardware_.GetPowerwashMarkerFullPath();
+ std::string marker_contents = "safe reason test_key";
+ ASSERT_TRUE(brillo::TouchFile(marker_path));
+ ASSERT_TRUE(base::WriteFile(marker_path, marker_contents));
+
+ std::optional<bool> result = hardware_.IsPowerwashScheduledByUpdateEngine();
+
+ EXPECT_TRUE(result);
+ EXPECT_FALSE(result.value());
+}
+
+TEST_F(HardwareChromeOSTest, IsPowerwashScheduledByUpdateEngineEmptyReason) {
+ base::FilePath root_path = root_dir_.GetPath();
+ hardware_.SetRootForTest(root_path);
+ base::FilePath marker_path = hardware_.GetPowerwashMarkerFullPath();
+ std::string marker_contents = "safe reason=\n test_key";
+ ASSERT_TRUE(brillo::TouchFile(marker_path));
+ ASSERT_TRUE(base::WriteFile(marker_path, marker_contents));
+
+ std::optional<bool> result = hardware_.IsPowerwashScheduledByUpdateEngine();
+
+ EXPECT_TRUE(result);
+ EXPECT_FALSE(result.value());
+}
+
} // namespace chromeos_update_engine
diff --git a/cros/update_attempter.cc b/cros/update_attempter.cc
index d78eef9..d0eb4a0 100644
--- a/cros/update_attempter.cc
+++ b/cros/update_attempter.cc
@@ -1779,9 +1779,19 @@
}
LOG(INFO) << "Clearing powerwash and rollback flags, if any.";
- if (!SystemState::Get()->hardware()->CancelPowerwash()) {
- LOG(WARNING) << "Failed to cancel powerwash. Continuing anyway.";
- success = false;
+ const std::optional<bool> is_powerwash_scheduled_by_update_engine =
+ SystemState::Get()->hardware()->IsPowerwashScheduledByUpdateEngine();
+ if (!is_powerwash_scheduled_by_update_engine) {
+ LOG(INFO) << "Powerwash is not scheduled, continuing.";
+ } else if (!is_powerwash_scheduled_by_update_engine.value()) {
+ LOG(INFO) << "Not cancelling powerwash. Either not initiated by update "
+ "engine or there was a parsing error.";
+ } else {
+ LOG(INFO) << "Cancelling powerwash that was initiated by update engine.";
+ if (!SystemState::Get()->hardware()->CancelPowerwash()) {
+ LOG(WARNING) << "Failed to cancel powerwash. Continuing anyway.";
+ success = false;
+ }
}
SystemState::Get()->payload_state()->SetRollbackHappened(false);
diff --git a/cros/update_attempter.h b/cros/update_attempter.h
index 5da200e..7df9d81 100644
--- a/cros/update_attempter.h
+++ b/cros/update_attempter.h
@@ -374,6 +374,8 @@
FRIEND_TEST(UpdateAttempterTest, FirstUpdateBeforeReboot);
FRIEND_TEST(UpdateAttempterTest, InvalidateLastUpdate);
FRIEND_TEST(UpdateAttempterTest, InvalidateLastPowerwashUpdate);
+ FRIEND_TEST(UpdateAttempterTest, InvalidateLastUpdateNoPowerwashFile);
+ FRIEND_TEST(UpdateAttempterTest, InvalidateLastUpdateExternalPowerwash);
FRIEND_TEST(UpdateAttempterTest, ConsecutiveUpdateBeforeRebootSuccess);
FRIEND_TEST(UpdateAttempterTest, ConsecutiveUpdateBeforeRebootLimited);
FRIEND_TEST(UpdateAttempterTest, ConsecutiveUpdateFailureMetric);
diff --git a/cros/update_attempter_unittest.cc b/cros/update_attempter_unittest.cc
index 41da85d..6d0a186 100644
--- a/cros/update_attempter_unittest.cc
+++ b/cros/update_attempter_unittest.cc
@@ -22,6 +22,7 @@
#include <limits>
#include <map>
#include <memory>
+#include <optional>
#include <string>
#include <unordered_set>
@@ -2597,6 +2598,9 @@
bool save_rollback_data = true;
FakeSystemState::Get()->fake_hardware()->SchedulePowerwash(
save_rollback_data);
+ FakeSystemState::Get()
+ ->fake_hardware()
+ ->SetIsPowerwashScheduledByUpdateEngine(true);
EXPECT_TRUE(FakeSystemState::Get()->fake_hardware()->IsPowerwashScheduled());
FakeSystemState::Get()->fake_clock()->SetBootTime(Time::FromTimeT(42));
attempter_.WriteUpdateCompletedMarker();
@@ -2616,6 +2620,60 @@
EXPECT_TRUE(FakeSystemState::Get()->fake_hardware()->IsFWTryNextSlotReset());
}
+TEST_F(UpdateAttempterTest, InvalidateLastUpdateNoPowerwashFile) {
+ // Mock a previous update.
+ bool save_rollback_data = true;
+ FakeSystemState::Get()->fake_hardware()->SchedulePowerwash(
+ save_rollback_data);
+ EXPECT_TRUE(FakeSystemState::Get()->fake_hardware()->IsPowerwashScheduled());
+ FakeSystemState::Get()
+ ->fake_hardware()
+ ->SetIsPowerwashScheduledByUpdateEngine(std::nullopt);
+ FakeSystemState::Get()->fake_clock()->SetBootTime(Time::FromTimeT(42));
+ attempter_.WriteUpdateCompletedMarker();
+ EXPECT_TRUE(attempter_.GetBootTimeAtUpdate(nullptr));
+ attempter_.status_ = UpdateStatus::UPDATED_NEED_REBOOT;
+ EXPECT_CALL(*FakeSystemState::Get()->mock_payload_state(),
+ SetRollbackHappened(false))
+ .Times(1);
+
+ // Invalidate an update.
+ MockAction action;
+ attempter_.ActionCompleted(
+ nullptr, &action, ErrorCode::kInvalidateLastUpdate);
+
+ EXPECT_FALSE(attempter_.GetBootTimeAtUpdate(nullptr));
+ EXPECT_TRUE(FakeSystemState::Get()->fake_hardware()->IsPowerwashScheduled());
+ EXPECT_TRUE(FakeSystemState::Get()->fake_hardware()->IsFWTryNextSlotReset());
+}
+
+TEST_F(UpdateAttempterTest, InvalidateLastUpdateExternalPowerwash) {
+ // Mock a previous update.
+ bool save_rollback_data = true;
+ FakeSystemState::Get()->fake_hardware()->SchedulePowerwash(
+ save_rollback_data);
+ EXPECT_TRUE(FakeSystemState::Get()->fake_hardware()->IsPowerwashScheduled());
+ FakeSystemState::Get()
+ ->fake_hardware()
+ ->SetIsPowerwashScheduledByUpdateEngine(false);
+ FakeSystemState::Get()->fake_clock()->SetBootTime(Time::FromTimeT(42));
+ attempter_.WriteUpdateCompletedMarker();
+ EXPECT_TRUE(attempter_.GetBootTimeAtUpdate(nullptr));
+ attempter_.status_ = UpdateStatus::UPDATED_NEED_REBOOT;
+ EXPECT_CALL(*FakeSystemState::Get()->mock_payload_state(),
+ SetRollbackHappened(false))
+ .Times(1);
+
+ // Invalidate an update.
+ MockAction action;
+ attempter_.ActionCompleted(
+ nullptr, &action, ErrorCode::kInvalidateLastUpdate);
+
+ EXPECT_FALSE(attempter_.GetBootTimeAtUpdate(nullptr));
+ EXPECT_TRUE(FakeSystemState::Get()->fake_hardware()->IsPowerwashScheduled());
+ EXPECT_TRUE(FakeSystemState::Get()->fake_hardware()->IsFWTryNextSlotReset());
+}
+
TEST_F(UpdateAttempterTest, CalculateDlcParamsRemoveStaleMetadata) {
string dlc_id = "dlc0";
auto active_key =
