Don't rollback if the other partition isn't valid.
Before we start a rollback to the other OS slot, validate the GPT flags show
it as bootable. This should prevent us from attempting a rollback if an
update has been attempted and failed, or is currently in progress. Such
a rollback would always fail, since the other partition would be left in
a partially modified state.
Piggyback:
Move sanity test in hardware that was added to the wrong method.
Undid some unittest changes that were decided against after the fact.
BUG=chromium:267054
TEST=Unittests
Manual Update Rollbacks (with/without flags on other partition)
Change-Id: Ide6b0673855ba2e4b05a0db93413a1a9f2ece2a9
Reviewed-on: https://chromium-review.googlesource.com/176755
Reviewed-by: David Zeuthen <zeuthen@chromium.org>
Commit-Queue: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
diff --git a/filesystem_copier_action_unittest.cc b/filesystem_copier_action_unittest.cc
index 8b6e627..3e88b42 100644
--- a/filesystem_copier_action_unittest.cc
+++ b/filesystem_copier_action_unittest.cc
@@ -17,6 +17,7 @@
#include "update_engine/filesystem_copier_action.h"
#include "update_engine/filesystem_iterator.h"
+#include "update_engine/mock_hardware.h"
#include "update_engine/mock_system_state.h"
#include "update_engine/omaha_hash_calculator.h"
#include "update_engine/test_utils.h"
@@ -41,6 +42,8 @@
}
void TearDown() {
}
+
+ MockSystemState mock_system_state_;
};
class FilesystemCopierActionTestDelegate : public ActionProcessorDelegate {
@@ -119,7 +122,10 @@
bool terminate_early,
bool use_kernel_partition,
int verify_hash) {
- MockSystemState mock_system_state;
+ // We need MockHardware to verify MarkUnbootable calls, but don't want
+ // warnings about other usages.
+ testing::NiceMock<MockHardware> mock_hardware;
+ mock_system_state_.set_hardware(&mock_hardware);
GMainLoop *loop = g_main_loop_new(g_main_context_default(), FALSE);
@@ -197,13 +203,13 @@
}
}
- EXPECT_CALL(mock_system_state.get_mock_hardware(),
+ EXPECT_CALL(mock_hardware,
MarkKernelUnbootable(a_dev)).Times(use_kernel_partition ? 1 : 0);
ActionProcessor processor;
ObjectFeederAction<InstallPlan> feeder_action;
- FilesystemCopierAction copier_action(&mock_system_state,
+ FilesystemCopierAction copier_action(&mock_system_state_,
use_kernel_partition,
verify_hash != 0);
ObjectCollectorAction<InstallPlan> collector_action;
@@ -276,8 +282,7 @@
LOG(INFO) << "Verifying bootable flag on: " << a_dev;
bool bootable;
- EXPECT_TRUE(mock_system_state.get_mock_hardware().fake().
- IsKernelBootable(a_dev, &bootable));
+ EXPECT_TRUE(mock_hardware.fake().IsKernelBootable(a_dev, &bootable));
// We should always mark a partition as unbootable if it's a kernel
// partition, but never if it's anything else.
EXPECT_EQ(bootable, !use_kernel_partition);
@@ -302,12 +307,11 @@
TEST_F(FilesystemCopierActionTest, MissingInputObjectTest) {
ActionProcessor processor;
- MockSystemState mock_system_state;
FilesystemCopierActionTest2Delegate delegate;
processor.set_delegate(&delegate);
- FilesystemCopierAction copier_action(&mock_system_state, false, false);
+ FilesystemCopierAction copier_action(&mock_system_state_, false, false);
ObjectCollectorAction<InstallPlan> collector_action;
BondActions(&copier_action, &collector_action);
@@ -322,7 +326,6 @@
TEST_F(FilesystemCopierActionTest, ResumeTest) {
ActionProcessor processor;
- MockSystemState mock_system_state;
FilesystemCopierActionTest2Delegate delegate;
processor.set_delegate(&delegate);
@@ -331,7 +334,7 @@
const char* kUrl = "http://some/url";
InstallPlan install_plan(false, true, kUrl, 0, "", 0, "", "", "", "");
feeder_action.set_obj(install_plan);
- FilesystemCopierAction copier_action(&mock_system_state, false, false);
+ FilesystemCopierAction copier_action(&mock_system_state_, false, false);
ObjectCollectorAction<InstallPlan> collector_action;
BondActions(&feeder_action, &copier_action);
@@ -349,7 +352,6 @@
TEST_F(FilesystemCopierActionTest, NonExistentDriveTest) {
ActionProcessor processor;
- MockSystemState mock_system_state;
FilesystemCopierActionTest2Delegate delegate;
processor.set_delegate(&delegate);
@@ -366,7 +368,7 @@
"/no/such/file",
"");
feeder_action.set_obj(install_plan);
- FilesystemCopierAction copier_action(&mock_system_state, false, false);
+ FilesystemCopierAction copier_action(&mock_system_state_, false, false);
ObjectCollectorAction<InstallPlan> collector_action;
BondActions(&copier_action, &collector_action);
@@ -403,7 +405,6 @@
}
TEST_F(FilesystemCopierActionTest, RunAsRootDetermineFilesystemSizeTest) {
- MockSystemState mock_system_state;
string img;
EXPECT_TRUE(utils::MakeTempFile("/tmp/img.XXXXXX", &img, NULL));
ScopedPathUnlinker img_unlinker(img);
@@ -416,7 +417,7 @@
for (int i = 0; i < 2; ++i) {
bool is_kernel = i == 1;
- FilesystemCopierAction action(&mock_system_state, is_kernel, false);
+ FilesystemCopierAction action(&mock_system_state_, is_kernel, false);
EXPECT_EQ(kint64max, action.filesystem_size_);
{
int fd = HANDLE_EINTR(open(img.c_str(), O_RDONLY));