Mark the new slot as not ready from the DownloadAction.
The FilesystemVerifierAction used to copy over the old kernel and old
rootfs to the new slot, invalidating the contents of the new slot
during the copy. Since now it only parses the source partitions,
marking the new slot as unbootable can be delayed until the payload
starts to download.
Bug: 24667689
Test: FEATURES=test emerge-link update_engine
Change-Id: I17736a10787cb747d181e8dffcc0bc58f9a0cabb
diff --git a/download_action.cc b/download_action.cc
index b913c97..a31c8a3 100644
--- a/download_action.cc
+++ b/download_action.cc
@@ -25,6 +25,7 @@
#include <base/strings/stringprintf.h>
#include "update_engine/action_pipe.h"
+#include "update_engine/boot_control_interface.h"
#include "update_engine/omaha_request_params.h"
#include "update_engine/p2p_manager.h"
#include "update_engine/payload_state_interface.h"
@@ -168,6 +169,14 @@
install_plan_.Dump();
+ LOG(INFO) << "Marking new slot as unbootable";
+ if (!system_state_->boot_control()->MarkSlotUnbootable(
+ install_plan_.target_slot)) {
+ LOG(WARNING) << "Unable to mark new slot "
+ << BootControlInterface::SlotName(install_plan_.target_slot)
+ << ". Proceeding with the update anyway.";
+ }
+
if (writer_) {
LOG(INFO) << "Using writer for test.";
} else {
diff --git a/download_action_unittest.cc b/download_action_unittest.cc
index 486f2c5..d1f4bc9 100644
--- a/download_action_unittest.cc
+++ b/download_action_unittest.cc
@@ -134,6 +134,7 @@
bool use_download_delegate) {
chromeos::FakeMessageLoop loop(nullptr);
loop.SetAsCurrent();
+ FakeSystemState fake_system_state;
// TODO(adlr): see if we need a different file for build bots
ScopedTempFile output_temp_file;
@@ -157,6 +158,14 @@
"",
"",
"");
+ install_plan.source_slot = 0;
+ install_plan.target_slot = 1;
+ // We mark both slots as bootable. Only the target slot should be unbootable
+ // after the download starts.
+ fake_system_state.fake_boot_control()->SetSlotBootable(
+ install_plan.source_slot, true);
+ fake_system_state.fake_boot_control()->SetSlotBootable(
+ install_plan.target_slot, true);
ObjectFeederAction<InstallPlan> feeder_action;
feeder_action.set_obj(install_plan);
MockPrefs prefs;
@@ -164,7 +173,7 @@
data.size(),
nullptr);
// takes ownership of passed in HttpFetcher
- DownloadAction download_action(&prefs, nullptr, http_fetcher);
+ DownloadAction download_action(&prefs, &fake_system_state, http_fetcher);
download_action.SetTestFileWriter(&writer);
BondActions(&feeder_action, &download_action);
DownloadActionDelegateMock download_delegate;
@@ -193,6 +202,11 @@
base::Bind(&StartProcessorInRunLoop, &processor, http_fetcher));
loop.Run();
EXPECT_FALSE(loop.PendingTasks());
+
+ EXPECT_TRUE(fake_system_state.fake_boot_control()->IsSlotBootable(
+ install_plan.source_slot));
+ EXPECT_FALSE(fake_system_state.fake_boot_control()->IsSlotBootable(
+ install_plan.target_slot));
}
} // namespace
@@ -269,8 +283,9 @@
InstallPlan install_plan(false, false, "", 0, "", 0, "",
temp_file.GetPath(), "", "", "", "");
feeder_action.set_obj(install_plan);
+ FakeSystemState fake_system_state_;
MockPrefs prefs;
- DownloadAction download_action(&prefs, nullptr,
+ DownloadAction download_action(&prefs, &fake_system_state_,
new MockHttpFetcher(data.data(),
data.size(),
nullptr));
@@ -377,7 +392,8 @@
ObjectFeederAction<InstallPlan> feeder_action;
feeder_action.set_obj(install_plan);
MockPrefs prefs;
- DownloadAction download_action(&prefs, nullptr,
+ FakeSystemState fake_system_state_;
+ DownloadAction download_action(&prefs, &fake_system_state_,
new MockHttpFetcher("x", 1, nullptr));
download_action.SetTestFileWriter(&writer);
@@ -414,7 +430,8 @@
ObjectFeederAction<InstallPlan> feeder_action;
feeder_action.set_obj(install_plan);
MockPrefs prefs;
- DownloadAction download_action(&prefs, nullptr,
+ FakeSystemState fake_system_state_;
+ DownloadAction download_action(&prefs, &fake_system_state_,
new MockHttpFetcher("x", 1, nullptr));
download_action.SetTestFileWriter(&writer);
diff --git a/filesystem_verifier_action.cc b/filesystem_verifier_action.cc
index d28526f..2291bc9 100644
--- a/filesystem_verifier_action.cc
+++ b/filesystem_verifier_action.cc
@@ -73,16 +73,6 @@
}
install_plan_ = GetInputObject();
- // TODO(deymo): Remove this from the FileSystemVerifierAction.
- if (partition_type_ == PartitionType::kKernel) {
- LOG(INFO) << "verifying kernel, marking as unbootable";
- if (!system_state_->boot_control()->MarkSlotUnbootable(
- install_plan_.target_slot)) {
- PLOG(ERROR) << "Unable to clear kernel GPT boot flags: " <<
- install_plan_.kernel_install_path;
- }
- }
-
if (install_plan_.is_full_update &&
(partition_type_ == PartitionType::kSourceRootfs ||
partition_type_ == PartitionType::kSourceKernel)) {
diff --git a/filesystem_verifier_action_unittest.cc b/filesystem_verifier_action_unittest.cc
index 32f3c59..8280328 100644
--- a/filesystem_verifier_action_unittest.cc
+++ b/filesystem_verifier_action_unittest.cc
@@ -204,9 +204,6 @@
break;
}
- fake_system_state_.fake_boot_control()->SetSlotBootable(
- install_plan.target_slot, true);
-
ActionProcessor processor;
ObjectFeederAction<InstallPlan> feeder_action;
@@ -263,14 +260,6 @@
bool is_install_plan_eq = (collector_action.object() == install_plan);
EXPECT_TRUE(is_install_plan_eq);
success = success && is_install_plan_eq;
-
- LOG(INFO) << "Verifying bootable flag on: " << a_dev;
-
- // We should always mark a partition as unbootable if it's a kernel
- // partition, but never if it's anything else.
- EXPECT_EQ((partition_type != PartitionType::kKernel),
- fake_system_state_.fake_boot_control()->IsSlotBootable(
- install_plan.target_slot));
return success;
}