Revert "AU: do not copy filesystem during full updates"
This reverts commit 81c90628abb268c9e54f655c2493b4495b5cf91e
This broke all of the Canaries with NPO update tests. It may be because devserver changes require manual staging into the autotest framework.
An example test:
http://cautotest/afe/#tab_id=view_job&object_id=3064976
Test Logs:
https://storage.cloud.google.com/?arg=chromeos-autotest-results/3064976-chromeos-test/chromeos2-row5-rack1-host9/debug#chromeos-autotest-results%2F3064976-chromeos-test%2Fchromeos2-row5-rack1-host9%2Fautoupdate_EndToEndTest.npo_test_delta%2Fsysinfo%2Fvar%2Flog_diff%2Fupdate_engine
UE Logs:
[0522/183224:INFO:chrome_browser_proxy_resolver.cc(168)] dbus_g_proxy_call succeeded!
[0522/183224:INFO:libcurl_http_fetcher.cc(63)] Starting/Resuming transfer
[0522/183224:INFO:libcurl_http_fetcher.cc(74)] Using proxy: no
[0522/183224:INFO:libcurl_http_fetcher.cc(52)] We are connected via ethernet, Updates allowed: Yes
[0522/183224:INFO:libcurl_http_fetcher.cc(180)] Not setting http(s) curl options because we are in test mode or running a dev/test image
[0522/183224:INFO:libcurl_http_fetcher.cc(485)] Setting up timeout source: 1 seconds.
[0522/183224:INFO:delta_performer.cc(104)] Completed 0/? operations, 15677/681963 bytes downloaded (2%), overall progress 1%
[0522/183224:WARNING:delta_performer.cc(327)] Ignoring missing/incorrect metadata size (0) in Omaha response as validation is not mandatory. Trusting metadata size in payload = 1410
[0522/183224:INFO:utils.cc(824)] Sending error code 32 (kErrorCodeDownloadInvalidMetadataSize) to UMA metric: Installer.DevModeErrorCodes. Flags = TestOmahaUrl, TestImage
[0522/183224:WARNING:delta_performer.cc(791)] Cannot validate metadata as the signature is empty
[0522/183224:INFO:utils.cc(824)] Sending error code 39 (kErrorCodeDownloadMetadataSignatureMissingError) to UMA metric: Installer.DevModeErrorCodes. Flags = TestOmahaUrl, TestImage
[0522/183224:ERROR:delta_performer.cc(852)] Purported full payload contains old partition hash(es), aborting update
[0522/183224:ERROR:download_action.cc(102)] Error 6 in DeltaPerformer's Write method when processing the received payload -- Terminating processing
[0522/183224:INFO:delta_performer.cc(216)] Discarding 15677 unused downloaded bytes
[0522/183224:INFO:update_attempter.cc(724)] Download status: inactive
[0522/183224:INFO:multi_range_http_fetcher.cc(155)] Received transfer terminated.
[0522/183224:INFO:multi_range_http_fetcher.cc(107)] TransferEnded w/ code 200
[0522/183224:INFO:multi_range_http_fetcher.cc(109)] Terminating.
[0522/183224:INFO:prefs.cc(27)] delta-update-failures not present in /var/lib/update_engine/prefs
[0522/183224:INFO:action_processor.cc(68)] ActionProcessor::ActionComplete: DownloadAction action failed. Aborting processing.
[0522/183224:INFO:action_processor.cc(73)] ActionProcessor::ActionComplete: finished last action of type DownloadAction
[0522/183224:INFO:update_attempter.cc(578)] Processing Done.
[0522/183224:INFO:utils.cc(673)] Setting cgroup cpu shares to 1024
[0522/183224:INFO:update_attempter.cc(963)] CPU shares = 1024
[0522/183224:ERROR:update_attempter.cc(934)] Update failed.
Change-Id: I69d3b93711707d9429201865346fd50117338718
Reviewed-on: https://gerrit.chromium.org/gerrit/56351
Commit-Queue: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
diff --git a/delta_performer.cc b/delta_performer.cc
index 5615801..360272f 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -383,11 +383,6 @@
if (result == kMetadataParseInsufficientData) {
return true;
}
-
- // Checks the integrity of the payload manifest.
- if ((*error = ValidateManifest()) != kErrorCodeSuccess)
- return false;
-
// Remove protobuf and header info from buffer_, so buffer_ contains
// just data blobs
DiscardBufferHeadBytes(manifest_metadata_size_);
@@ -836,30 +831,6 @@
return kErrorCodeSuccess;
}
-ErrorCode DeltaPerformer::ValidateManifest() {
- // Ensure that a full update does not contain old partition hashes, which is
- // indicative of a delta.
- //
- // TODO(garnold) in general, the presence of an old partition hash should be
- // the sole indicator for a delta update, as we would generally like update
- // payloads to be self contained and not assume an Omaha response to tell us
- // that. However, since this requires some massive reengineering of the update
- // flow (making filesystem copying happen conditionally only *after*
- // downloading and parsing of the update manifest) we'll put it off for now.
- // See chromium-os:7597 for further discussion.
- if (install_plan_->is_full_update &&
- (manifest_.has_old_kernel_info() || manifest_.has_old_rootfs_info())) {
- LOG(ERROR) << "Purported full payload contains old partition "
- "hash(es), aborting update";
- return kErrorCodePayloadMismatchedType;
- }
-
- // TODO(garnold) we should be adding more and more manifest checks, such as
- // partition boundaries etc (see chromium-os:37661).
-
- return kErrorCodeSuccess;
-}
-
ErrorCode DeltaPerformer::ValidateOperationHash(
const DeltaArchiveManifest_InstallOperation& operation) {
diff --git a/delta_performer.h b/delta_performer.h
index 33e39d7..1f8f5cb 100644
--- a/delta_performer.h
+++ b/delta_performer.h
@@ -193,10 +193,6 @@
bool CanPerformInstallOperation(
const DeltaArchiveManifest_InstallOperation& operation);
- // Checks the integrity of the payload manifest. Returns true upon success,
- // false otherwise.
- ErrorCode ValidateManifest();
-
// Validates that the hash of the blobs corresponding to the given |operation|
// matches what's specified in the manifest in the payload.
// Returns kErrorCodeSuccess on match or a suitable error code otherwise.
diff --git a/download_action_unittest.cc b/download_action_unittest.cc
index e937d71..3911d55 100644
--- a/download_action_unittest.cc
+++ b/download_action_unittest.cc
@@ -131,7 +131,6 @@
OmahaHashCalculator::OmahaHashOfBytes(&data[1], data.size() - 1);
uint64_t size = data.size();
InstallPlan install_plan(false,
- false,
"",
size,
hash,
@@ -253,8 +252,7 @@
// takes ownership of passed in HttpFetcher
ObjectFeederAction<InstallPlan> feeder_action;
- InstallPlan install_plan(false, false, "", 0, "", 0, "",
- temp_file.GetPath(), "");
+ InstallPlan install_plan(false, "", 0, "", 0, "", temp_file.GetPath(), "");
feeder_action.set_obj(install_plan);
PrefsMock prefs;
DownloadAction download_action(&prefs, NULL,
@@ -356,7 +354,6 @@
// takes ownership of passed in HttpFetcher
InstallPlan install_plan(false,
- false,
"",
1,
OmahaHashCalculator::OmahaHashOfString("x"),
@@ -398,7 +395,7 @@
DirectFileWriter writer;
// takes ownership of passed in HttpFetcher
- InstallPlan install_plan(false, false, "", 0, "", 0, "", path, "");
+ InstallPlan install_plan(false, "", 0, "", 0, "", path, "");
ObjectFeederAction<InstallPlan> feeder_action;
feeder_action.set_obj(install_plan);
PrefsMock prefs;
diff --git a/error_code.h b/error_code.h
index aab0917..db023f7 100644
--- a/error_code.h
+++ b/error_code.h
@@ -15,7 +15,7 @@
kErrorCodeOmahaResponseHandlerError = 3,
kErrorCodeFilesystemCopierError = 4,
kErrorCodePostinstallRunnerError = 5,
- kErrorCodePayloadMismatchedType = 6,
+ kErrorCodeSetBootableFlagError = 6, // TODO(petkov): Unused. Recycle?
kErrorCodeInstallDeviceOpenError = 7,
kErrorCodeKernelDeviceOpenError = 8,
kErrorCodeDownloadTransferError = 9,
diff --git a/filesystem_copier_action.cc b/filesystem_copier_action.cc
index 3e5160e..7024b67 100644
--- a/filesystem_copier_action.cc
+++ b/filesystem_copier_action.cc
@@ -69,11 +69,8 @@
return;
}
install_plan_ = GetInputObject();
- if (!verify_hash_ &&
- (install_plan_.is_resume || install_plan_.is_full_update)) {
+ if (!verify_hash_ && install_plan_.is_resume) {
// No copy or hash verification needed. Done!
- LOG(INFO) << "filesystem copying skipped: "
- << (install_plan_.is_resume ? "resumed" : "full") << " update";
if (HasOutputPipe())
SetOutputObject(install_plan_);
abort_action_completer.set_code(kErrorCodeSuccess);
diff --git a/filesystem_copier_action_unittest.cc b/filesystem_copier_action_unittest.cc
index 0db1640..3ffed88 100644
--- a/filesystem_copier_action_unittest.cc
+++ b/filesystem_copier_action_unittest.cc
@@ -311,7 +311,7 @@
ObjectFeederAction<InstallPlan> feeder_action;
const char* kUrl = "http://some/url";
- InstallPlan install_plan(false, true, kUrl, 0, "", 0, "", "", "");
+ InstallPlan install_plan(true, kUrl, 0, "", 0, "", "", "");
feeder_action.set_obj(install_plan);
FilesystemCopierAction copier_action(false, false);
ObjectCollectorAction<InstallPlan> collector_action;
@@ -337,7 +337,6 @@
ObjectFeederAction<InstallPlan> feeder_action;
InstallPlan install_plan(false,
- false,
"",
0,
"",
diff --git a/install_plan.cc b/install_plan.cc
index a17cb52..59bd5da 100644
--- a/install_plan.cc
+++ b/install_plan.cc
@@ -13,7 +13,6 @@
namespace chromeos_update_engine {
InstallPlan::InstallPlan(bool is_resume,
- bool is_full_update,
const string& url,
uint64_t payload_size,
const string& payload_hash,
@@ -22,7 +21,6 @@
const string& install_path,
const string& kernel_install_path)
: is_resume(is_resume),
- is_full_update(is_full_update),
download_url(url),
payload_size(payload_size),
payload_hash(payload_hash),
@@ -36,7 +34,6 @@
powerwash_required(false) {}
InstallPlan::InstallPlan() : is_resume(false),
- is_full_update(false), // play it safe.
payload_size(0),
metadata_size(0),
kernel_size(0),
@@ -47,7 +44,6 @@
bool InstallPlan::operator==(const InstallPlan& that) const {
return ((is_resume == that.is_resume) &&
- (is_full_update == that.is_full_update) &&
(download_url == that.download_url) &&
(payload_size == that.payload_size) &&
(payload_hash == that.payload_hash) &&
@@ -63,8 +59,7 @@
void InstallPlan::Dump() const {
LOG(INFO) << "InstallPlan: "
- << (is_resume ? "resume" : "new_update")
- << ", payload type: " << (is_full_update ? "full" : "delta")
+ << (is_resume ? ", resume" : ", new_update")
<< ", url: " << download_url
<< ", payload size: " << payload_size
<< ", payload hash: " << payload_hash
diff --git a/install_plan.h b/install_plan.h
index d6ff9e0..fc33f25 100644
--- a/install_plan.h
+++ b/install_plan.h
@@ -16,7 +16,6 @@
struct InstallPlan {
InstallPlan(bool is_resume,
- bool is_full_update,
const std::string& url,
uint64_t payload_size,
const std::string& payload_hash,
@@ -35,7 +34,6 @@
void Dump() const;
bool is_resume;
- bool is_full_update;
std::string download_url; // url to download from
uint64_t payload_size; // size of the payload
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index c876d94..c85711b 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -70,7 +70,6 @@
kPrefsUpdateCheckResponseHash, response.hash))
<< "Unable to save the update check response hash.";
}
- install_plan_.is_full_update = !response.is_delta_payload;
TEST_AND_RETURN(GetInstallDev(
(!boot_device_.empty() ? boot_device_ : utils::BootDevice()),
diff --git a/payload_state.cc b/payload_state.cc
index 0636f56..3de9e04 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -189,7 +189,6 @@
case kErrorCodeDownloadInvalidMetadataSignature:
case kErrorCodeDownloadOperationHashMissingError:
case kErrorCodeDownloadMetadataSignatureMissingError:
- case kErrorCodePayloadMismatchedType:
IncrementUrlIndex();
break;
@@ -240,6 +239,7 @@
break;
case kErrorCodeSuccess: // success code
+ case kErrorCodeSetBootableFlagError: // unused
case kErrorCodeUmaReportedMax: // not an error code
case kErrorCodeOmahaRequestHTTPResponseBase: // aggregated already
case kErrorCodeDevModeFlag: // not an error code
diff --git a/utils.cc b/utils.cc
index a6d8d91..be98a71 100644
--- a/utils.cc
+++ b/utils.cc
@@ -848,8 +848,8 @@
return "kErrorCodeFilesystemCopierError";
case kErrorCodePostinstallRunnerError:
return "kErrorCodePostinstallRunnerError";
- case kErrorCodePayloadMismatchedType:
- return "kErrorCodePayloadMismatchedType";
+ case kErrorCodeSetBootableFlagError:
+ return "kErrorCodeSetBootableFlagError";
case kErrorCodeInstallDeviceOpenError:
return "kErrorCodeInstallDeviceOpenError";
case kErrorCodeKernelDeviceOpenError: