AU: Handle firmware update failure when booted from FW slot B.
The firmware updater will fail if we are booted into FW slot B and we
try to update firmware. We shouldn't treat this like a usual update
failure because trying again won't help until we reboot. Thus, with
this CL, we detect this case and request a reboot from the
user. Technically this request is a lie, b/c we are reusing the reboot
request mechanism which tells the user an update has been applied and
thus must be rebooted. We accept this fib since this situation is very
rare: use would have to have 2 FW update updates in a row w/o any
extra boots between.
Also, fix error code in subprocess.
Also, remove execute permissions on a bunch of source files.
BUG=chromium-os:14343
TEST=unittests, tested updates on machine that success, fail, have FW B failure
Review URL: http://codereview.chromium.org/6880077
Change-Id: I2509c6e1c9c9da3ff1ea27da4861c4850bd4d000
diff --git a/action_processor.h b/action_processor.h
index 7f7862a..2ed5f78 100644
--- a/action_processor.h
+++ b/action_processor.h
@@ -42,6 +42,7 @@
kActionCodeNewKernelVerificationError = 16,
kActionCodeSignedDeltaPayloadExpectedError = 17,
kActionCodeDownloadPayloadPubKeyVerificationError = 18,
+ kActionCodePostinstallBootedFromFirmwareB = 19,
kActionCodeOmahaRequestEmptyResponseError = 200,
kActionCodeOmahaRequestXMLParseError = 201,
kActionCodeOmahaRequestNoUpdateCheckNode = 202,
diff --git a/decompressing_file_writer.cc b/decompressing_file_writer.cc
old mode 100755
new mode 100644
diff --git a/delta_performer_unittest.cc b/delta_performer_unittest.cc
old mode 100755
new mode 100644
diff --git a/extent_mapper.cc b/extent_mapper.cc
old mode 100755
new mode 100644
diff --git a/extent_mapper.h b/extent_mapper.h
old mode 100755
new mode 100644
diff --git a/extent_ranges.cc b/extent_ranges.cc
old mode 100755
new mode 100644
diff --git a/extent_ranges_unittest.cc b/extent_ranges_unittest.cc
old mode 100755
new mode 100644
diff --git a/filesystem_copier_action.cc b/filesystem_copier_action.cc
old mode 100755
new mode 100644
diff --git a/http_fetcher.h b/http_fetcher.h
old mode 100755
new mode 100644
diff --git a/main.cc b/main.cc
old mode 100755
new mode 100644
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
old mode 100755
new mode 100644
diff --git a/postinstall_runner_action.cc b/postinstall_runner_action.cc
index 80c28c9..2c8b943 100644
--- a/postinstall_runner_action.cc
+++ b/postinstall_runner_action.cc
@@ -65,6 +65,12 @@
ScopedTempUnmounter temp_unmounter(temp_rootfs_dir_);
if (return_code != 0) {
LOG(ERROR) << "Postinst command failed with code: " << return_code;
+ if (return_code == 2) {
+ // This special return code means that we tried to update firmware,
+ // but couldn't because we booted from FW B, and we need to reboot
+ // to get back to FW A.
+ completer.set_code(kActionCodePostinstallBootedFromFirmwareB);
+ }
return;
}
if (HasOutputPipe()) {
diff --git a/postinstall_runner_action_unittest.cc b/postinstall_runner_action_unittest.cc
index 05acb4c..2db4edc 100644
--- a/postinstall_runner_action_unittest.cc
+++ b/postinstall_runner_action_unittest.cc
@@ -31,7 +31,7 @@
class PostinstallRunnerActionTest : public ::testing::Test {
public:
- void DoTest(bool do_losetup, bool do_err_script);
+ void DoTest(bool do_losetup, int err_code);
};
class PostinstActionProcessorDelegate : public ActionProcessorDelegate {
@@ -60,20 +60,25 @@
TEST_F(PostinstallRunnerActionTest, RunAsRootSimpleTest) {
ASSERT_EQ(0, getuid());
- DoTest(true, false);
+ DoTest(true, 0);
}
TEST_F(PostinstallRunnerActionTest, RunAsRootCantMountTest) {
ASSERT_EQ(0, getuid());
- DoTest(false, false);
+ DoTest(false, 0);
}
TEST_F(PostinstallRunnerActionTest, RunAsRootErrScriptTest) {
ASSERT_EQ(0, getuid());
- DoTest(true, true);
+ DoTest(true, 1);
}
-void PostinstallRunnerActionTest::DoTest(bool do_losetup, bool do_err_script) {
+TEST_F(PostinstallRunnerActionTest, RunAsRootFirmwareBErrScriptTest) {
+ ASSERT_EQ(0, getuid());
+ DoTest(true, 2);
+}
+
+void PostinstallRunnerActionTest::DoTest(bool do_losetup, int err_code) {
ASSERT_EQ(0, getuid()) << "Run me as root. Ideally don't run other tests "
<< "as root, tho.";
@@ -107,8 +112,8 @@
" touch %s/postinst_called\n"
"fi\n",
cwd.c_str());
- if (do_err_script) {
- script = "#!/bin/bash\nexit 1";
+ if (err_code) {
+ script = StringPrintf("#!/bin/bash\nexit %d", err_code);
}
ASSERT_TRUE(WriteFileString(mountpoint + "/postinst", script));
ASSERT_EQ(0, System(string("chmod a+x ") + mountpoint + "/postinst"));
@@ -161,16 +166,18 @@
ASSERT_FALSE(processor.IsRunning());
EXPECT_TRUE(delegate.code_set_);
- EXPECT_EQ(do_losetup && !do_err_script, delegate.code_ == kActionCodeSuccess);
- EXPECT_EQ(do_losetup && !do_err_script,
+ EXPECT_EQ(do_losetup && !err_code, delegate.code_ == kActionCodeSuccess);
+ EXPECT_EQ(do_losetup && !err_code,
!collector_action.object().install_path.empty());
- if (do_losetup && !do_err_script) {
+ if (do_losetup && !err_code) {
EXPECT_TRUE(install_plan == collector_action.object());
}
+ if (err_code == 2)
+ EXPECT_EQ(kActionCodePostinstallBootedFromFirmwareB, delegate.code_);
struct stat stbuf;
int rc = lstat((string(cwd) + "/postinst_called").c_str(), &stbuf);
- if (do_losetup && !do_err_script)
+ if (do_losetup && !err_code)
ASSERT_EQ(0, rc);
else
ASSERT_LT(rc, 0);
diff --git a/subprocess.cc b/subprocess.cc
old mode 100755
new mode 100644
index 9c19367..f6c1e3d
--- a/subprocess.cc
+++ b/subprocess.cc
@@ -29,14 +29,18 @@
close(fd);
g_spawn_close_pid(pid);
+ gint use_status = status;
+ if (WIFEXITED(status))
+ use_status = WEXITSTATUS(status);
+
if (status) {
- LOG(INFO) << "Subprocess status: " << status;
+ LOG(INFO) << "Subprocess status: " << use_status;
}
if (!record->stdout.empty()) {
LOG(INFO) << "Subprocess output:\n" << record->stdout;
}
if (record->callback) {
- record->callback(status, record->stdout, record->callback_data);
+ record->callback(use_status, record->stdout, record->callback_data);
}
Get().subprocess_records_.erase(record->tag);
}
diff --git a/subprocess_unittest.cc b/subprocess_unittest.cc
index 314e0ae..37deba9 100644
--- a/subprocess_unittest.cc
+++ b/subprocess_unittest.cc
@@ -28,7 +28,7 @@
const int kLocalHttpPort = 8088;
void Callback(int return_code, const string& output, void *p) {
- EXPECT_EQ(256, return_code);
+ EXPECT_EQ(1, return_code);
GMainLoop* loop = reinterpret_cast<GMainLoop*>(p);
g_main_loop_quit(loop);
}
diff --git a/update_attempter.cc b/update_attempter.cc
index 61c9947..1314086 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -103,6 +103,7 @@
prefs_(prefs),
metrics_lib_(metrics_lib),
update_check_scheduler_(NULL),
+ fake_update_success_(false),
http_response_code_(0),
priority_(utils::kProcessPriorityNormal),
manage_priority_source_(NULL),
@@ -129,6 +130,7 @@
const std::string& omaha_url,
bool obey_proxies) {
chrome_proxy_resolver_.Init();
+ fake_update_success_ = false;
UpdateBootFlags(); // Just in case we didn't do this yet.
if (status_ == UPDATE_STATUS_UPDATED_NEED_REBOOT) {
// Although we have applied an update, we still want to ping Omaha
@@ -314,7 +316,11 @@
if (status_ == UPDATE_STATUS_REPORTING_ERROR_EVENT) {
LOG(INFO) << "Error event sent.";
SetStatusAndNotify(UPDATE_STATUS_IDLE);
- return;
+ if (!fake_update_success_) {
+ return;
+ }
+ LOG(INFO) << "Booted from FW B and tried to install new firmware, "
+ "so requesting reboot from user.";
}
if (code == kActionCodeSuccess) {
@@ -326,11 +332,12 @@
// Report the time it took to update the system.
int64_t update_time = time(NULL) - last_checked_time_;
- metrics_lib_->SendToUMA("Installer.UpdateTime",
- static_cast<int>(update_time), // sample
- 1, // min = 1 second
- 20 * 60, // max = 20 minutes
- 50); // buckets
+ if (!fake_update_success_)
+ metrics_lib_->SendToUMA("Installer.UpdateTime",
+ static_cast<int>(update_time), // sample
+ 1, // min = 1 second
+ 20 * 60, // max = 20 minutes
+ 50); // buckets
return;
}
@@ -511,6 +518,7 @@
}
code = GetErrorCodeForAction(action, code);
+ fake_update_success_ = code == kActionCodePostinstallBootedFromFirmwareB;
error_event_.reset(new OmahaEvent(OmahaEvent::kTypeUpdateComplete,
OmahaEvent::kResultError,
code));
diff --git a/update_attempter.h b/update_attempter.h
index d92ff97..02e7162 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -217,6 +217,9 @@
// Pending error event, if any.
scoped_ptr<OmahaEvent> error_event_;
+ // If we should request a reboot even tho we failed the update
+ bool fake_update_success_;
+
// HTTP server response code from the last HTTP request action.
int http_response_code_;