Revert "Handle resume of VABC updates by emitting labels"
This reverts commit 24599af599acf74b71a555a8eeb827bedcd672b5.
Reason for revert: b/173009837
Test: 1. update_device.py ota.zip
--extra-headers="SWITCH_SLOT_ON_REBOOT=0"
2. update_device.py ota.zip
3. Verity that 2 did not re-start the entire update, only fs
verification and postinstall may re-run.
Bug: 173009837
Change-Id: Ia31025ebc68a5e6a72d7a0919994d614213270d1
diff --git a/common/constants.cc b/common/constants.cc
index 1648596..8883668 100644
--- a/common/constants.cc
+++ b/common/constants.cc
@@ -98,8 +98,6 @@
const char kPrefsUpdateStateNextDataLength[] = "update-state-next-data-length";
const char kPrefsUpdateStateNextDataOffset[] = "update-state-next-data-offset";
const char kPrefsUpdateStateNextOperation[] = "update-state-next-operation";
-const char kPrefsUpdateStatePartitionNextOperation[] =
- "update-state-partition-next-operation";
const char kPrefsUpdateStatePayloadIndex[] = "update-state-payload-index";
const char kPrefsUpdateStateSHA256Context[] = "update-state-sha-256-context";
const char kPrefsUpdateStateSignatureBlob[] = "update-state-signature-blob";
diff --git a/common/constants.h b/common/constants.h
index 2a2a62a..3685102 100644
--- a/common/constants.h
+++ b/common/constants.h
@@ -95,7 +95,6 @@
extern const char kPrefsUpdateStateNextDataLength[];
extern const char kPrefsUpdateStateNextDataOffset[];
extern const char kPrefsUpdateStateNextOperation[];
-extern const char kPrefsUpdateStatePartitionNextOperation[];
extern const char kPrefsUpdateStatePayloadIndex[];
extern const char kPrefsUpdateStateSHA256Context[];
extern const char kPrefsUpdateStateSignatureBlob[];
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index b75c8cf..30bd1ef 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -48,7 +48,6 @@
#include "update_engine/common/prefs_interface.h"
#include "update_engine/common/subprocess.h"
#include "update_engine/common/terminator.h"
-#include "update_engine/common/utils.h"
#include "update_engine/payload_consumer/bzip_extent_writer.h"
#include "update_engine/payload_consumer/cached_file_descriptor.h"
#include "update_engine/payload_consumer/certificate_parser_interface.h"
@@ -248,7 +247,6 @@
install_part,
dynamic_control,
block_size_,
- prefs_,
interactive_,
IsDynamicPartition(install_part.name));
// Open source fds if we have a delta payload, or for partitions in the
@@ -1337,13 +1335,6 @@
next_operation != kUpdateStateOperationInvalid && next_operation > 0))
return false;
- int64_t partition_next_op = -1;
- if (!(prefs->GetInt64(kPrefsUpdateStatePartitionNextOperation,
- &partition_next_op) &&
- partition_next_op >= 0)) {
- return false;
- }
-
string interrupted_hash;
if (!(prefs->GetString(kPrefsUpdateCheckResponseHash, &interrupted_hash) &&
!interrupted_hash.empty() &&
@@ -1396,7 +1387,6 @@
prefs->SetString(kPrefsUpdateStateSignatureBlob, "");
prefs->SetInt64(kPrefsManifestMetadataSize, -1);
prefs->SetInt64(kPrefsManifestSignatureSize, -1);
- prefs->SetInt64(kPrefsUpdateStatePartitionNextOperation, -1);
prefs->SetInt64(kPrefsResumedUpdateFailures, 0);
prefs->Delete(kPrefsPostInstallSucceeded);
prefs->Delete(kPrefsVerityWritten);
@@ -1441,7 +1431,6 @@
partitions_[partition_index].operations(partition_operation_num);
TEST_AND_RETURN_FALSE(
prefs_->SetInt64(kPrefsUpdateStateNextDataLength, op.data_length()));
- partition_writer_->CheckpointUpdateProgress(partition_operation_num);
} else {
TEST_AND_RETURN_FALSE(
prefs_->SetInt64(kPrefsUpdateStateNextDataLength, 0));
diff --git a/payload_consumer/delta_performer_integration_test.cc b/payload_consumer/delta_performer_integration_test.cc
index e83a20d..374131e 100644
--- a/payload_consumer/delta_performer_integration_test.cc
+++ b/payload_consumer/delta_performer_integration_test.cc
@@ -718,8 +718,6 @@
.WillOnce(Return(true));
EXPECT_CALL(prefs, SetInt64(kPrefsUpdateStateNextOperation, _))
.WillRepeatedly(Return(true));
- EXPECT_CALL(prefs, SetInt64(kPrefsUpdateStatePartitionNextOperation, _))
- .WillRepeatedly(Return(true));
EXPECT_CALL(prefs, GetInt64(kPrefsUpdateStateNextOperation, _))
.WillOnce(Return(false));
EXPECT_CALL(prefs, SetInt64(kPrefsUpdateStateNextDataOffset, _))
diff --git a/payload_consumer/partition_writer.cc b/payload_consumer/partition_writer.cc
index 12b206e..ec36d06 100644
--- a/payload_consumer/partition_writer.cc
+++ b/payload_consumer/partition_writer.cc
@@ -242,14 +242,12 @@
const InstallPlan::Partition& install_part,
DynamicPartitionControlInterface* dynamic_control,
size_t block_size,
- PrefsInterface* prefs,
bool is_interactive)
: partition_update_(partition_update),
install_part_(install_part),
dynamic_control_(dynamic_control),
interactive_(is_interactive),
- block_size_(block_size),
- prefs_(prefs) {}
+ block_size_(block_size) {}
PartitionWriter::~PartitionWriter() {
Close();
diff --git a/payload_consumer/partition_writer.h b/payload_consumer/partition_writer.h
index a67339e..1acbddc 100644
--- a/payload_consumer/partition_writer.h
+++ b/payload_consumer/partition_writer.h
@@ -25,7 +25,6 @@
#include <gtest/gtest_prod.h>
#include "update_engine/common/dynamic_partition_control_interface.h"
-#include "update_engine/common/prefs_interface.h"
#include "update_engine/payload_consumer/extent_writer.h"
#include "update_engine/payload_consumer/file_descriptor.h"
#include "update_engine/payload_consumer/install_plan.h"
@@ -37,7 +36,6 @@
const InstallPlan::Partition& install_part,
DynamicPartitionControlInterface* dynamic_control,
size_t block_size,
- PrefsInterface* prefs,
bool is_interactive);
virtual ~PartitionWriter();
static bool ValidateSourceHash(const brillo::Blob& calculated_hash,
@@ -50,11 +48,6 @@
[[nodiscard]] virtual bool Init(const InstallPlan* install_plan,
bool source_may_exist);
- // This will be called by DeltaPerformer after applying an InstallOp.
- // |next_op_index| is index of next operation that should be applied.
- // |next_op_index-1| is the last operation that is already applied.
- virtual void CheckpointUpdateProgress(size_t next_op_index) {}
-
int Close();
// These perform a specific type of operation and return true on success.
@@ -118,8 +111,6 @@
// Used to avoid re-opening the same source partition if it is not actually
// error corrected.
bool source_ecc_open_failure_{false};
-
- PrefsInterface* prefs_;
};
namespace partition_writer {
@@ -130,7 +121,6 @@
const InstallPlan::Partition& install_part,
DynamicPartitionControlInterface* dynamic_control,
size_t block_size,
- PrefsInterface* prefs,
bool is_interactive,
bool is_dynamic_partition);
} // namespace partition_writer
diff --git a/payload_consumer/partition_writer_factory_android.cc b/payload_consumer/partition_writer_factory_android.cc
index 5960d9b..0c9f7ea 100644
--- a/payload_consumer/partition_writer_factory_android.cc
+++ b/payload_consumer/partition_writer_factory_android.cc
@@ -19,7 +19,6 @@
#include <base/logging.h>
-#include "update_engine/common/prefs_interface.h"
#include "update_engine/payload_consumer/vabc_partition_writer.h"
namespace chromeos_update_engine::partition_writer {
@@ -29,7 +28,6 @@
const InstallPlan::Partition& install_part,
DynamicPartitionControlInterface* dynamic_control,
size_t block_size,
- PrefsInterface* prefs,
bool is_interactive,
bool is_dynamic_partition) {
if (dynamic_control &&
@@ -42,7 +40,6 @@
install_part,
dynamic_control,
block_size,
- prefs,
is_interactive);
} else {
LOG(INFO) << "Virtual AB Compression disabled, using Partition Writer for `"
@@ -51,7 +48,6 @@
install_part,
dynamic_control,
block_size,
- prefs,
is_interactive);
}
}
diff --git a/payload_consumer/partition_writer_factory_chromeos.cc b/payload_consumer/partition_writer_factory_chromeos.cc
index d89beb7..609f043 100644
--- a/payload_consumer/partition_writer_factory_chromeos.cc
+++ b/payload_consumer/partition_writer_factory_chromeos.cc
@@ -27,14 +27,12 @@
const InstallPlan::Partition& install_part,
DynamicPartitionControlInterface* dynamic_control,
size_t block_size,
- PrefsInterface* prefs,
bool is_interactive,
bool is_dynamic_partition) {
return std::make_unique<PartitionWriter>(partition_update,
install_part,
dynamic_control,
block_size,
- prefs,
is_interactive);
}
} // namespace chromeos_update_engine::partition_writer
diff --git a/payload_consumer/partition_writer_unittest.cc b/payload_consumer/partition_writer_unittest.cc
index ca2ef41..1ef4783 100644
--- a/payload_consumer/partition_writer_unittest.cc
+++ b/payload_consumer/partition_writer_unittest.cc
@@ -112,12 +112,8 @@
DeltaArchiveManifest manifest_{};
PartitionUpdate partition_update_{};
InstallPlan::Partition install_part_{};
- PartitionWriter writer_{partition_update_,
- install_part_,
- &dynamic_control_,
- kBlockSize,
- &prefs_,
- false};
+ PartitionWriter writer_{
+ partition_update_, install_part_, &dynamic_control_, kBlockSize, false};
};
// Test that the error-corrected file descriptor is used to read a partition
// when no hash is available for SOURCE_COPY but it falls back to the normal
diff --git a/payload_consumer/vabc_partition_writer.cc b/payload_consumer/vabc_partition_writer.cc
index e8994b4..d95103b 100644
--- a/payload_consumer/vabc_partition_writer.cc
+++ b/payload_consumer/vabc_partition_writer.cc
@@ -30,30 +30,6 @@
#include "update_engine/payload_consumer/snapshot_extent_writer.h"
namespace chromeos_update_engine {
-// Expected layout of COW file:
-// === Beginning of Cow Image ===
-// All Source Copy Operations
-// ========== Label 0 ==========
-// Operation 0 in PartitionUpdate
-// ========== Label 1 ==========
-// Operation 1 in PartitionUpdate
-// ========== label 2 ==========
-// Operation 2 in PartitionUpdate
-// ========== label 3 ==========
-// .
-// .
-// .
-
-// When resuming, pass |kPrefsUpdateStatePartitionNextOperation| as label to
-// |InitializeWithAppend|.
-// For example, suppose we finished writing SOURCE_COPY, and we finished writing
-// operation 2 completely. Update is suspended when we are half way through
-// operation 3.
-// |kPrefsUpdateStatePartitionNextOperation| would be 3, so we pass 3 as
-// label to |InitializeWithAppend|. The CowWriter will retain all data before
-// label 3, Which contains all operation 2's data, but none of operation 3's
-// data.
-
bool VABCPartitionWriter::Init(const InstallPlan* install_plan,
bool source_may_exist) {
TEST_AND_RETURN_FALSE(install_plan != nullptr);
@@ -62,41 +38,19 @@
install_part_.name, install_part_.source_path, install_plan->is_resume);
TEST_AND_RETURN_FALSE(cow_writer_ != nullptr);
- // Emit a label before writing SOURCE_COPY. When resuming,
+ // TODO(zhangkelvin) Emit a label before writing SOURCE_COPY. When resuming,
// use pref or CowWriter::GetLastLabel to determine if the SOURCE_COPY ops are
// written. No need to handle SOURCE_COPY operations when resuming.
// ===== Resume case handling code goes here ====
- if (install_plan->is_resume) {
- int64_t next_op = 0;
- if (!prefs_->GetInt64(kPrefsUpdateStatePartitionNextOperation, &next_op)) {
- LOG(ERROR)
- << "Resuming an update but can't fetch |next_op| from saved prefs.";
- return false;
- }
- if (next_op < 0) {
- TEST_AND_RETURN_FALSE(cow_writer_->Initialize());
- } else {
- TEST_AND_RETURN_FALSE(cow_writer_->InitializeAppend(next_op));
- return true;
- }
- } else {
- TEST_AND_RETURN_FALSE(cow_writer_->Initialize());
- }
// ==============================================
- TEST_AND_RETURN_FALSE(
- prefs_->SetInt64(kPrefsUpdateStatePartitionNextOperation, -1));
// TODO(zhangkelvin) Rewrite this in C++20 coroutine once that's available.
auto converted = ConvertToCowOperations(partition_update_.operations(),
partition_update_.merge_operations());
WriteAllCowOps(block_size_, converted, cow_writer_.get(), source_fd_);
- // Emit label 0 to mark end of all SOURCE_COPY operations
- cow_writer_->AddLabel(0);
- TEST_AND_RETURN_FALSE(
- prefs_->SetInt64(kPrefsUpdateStatePartitionNextOperation, 0));
return true;
}
@@ -153,27 +107,11 @@
}
bool VABCPartitionWriter::Flush() {
- // No need to call fsync/sync, as CowWriter flushes after a label is added
- // added.
- int64_t next_op = 0;
- // |kPrefsUpdateStatePartitionNextOperation| will be maintained and set by
- // CheckpointUpdateProgress()
- TEST_AND_RETURN_FALSE(
- prefs_->GetInt64(kPrefsUpdateStatePartitionNextOperation, &next_op));
- // +1 because label 0 is reserved for SOURCE_COPY. See beginning of this
- // file for explanation for cow format.
- cow_writer_->AddLabel(next_op + 1);
+ // No need to do anything, as CowWriter automatically flushes every OP added.
return true;
}
-void VABCPartitionWriter::CheckpointUpdateProgress(size_t next_op_index) {
- prefs_->SetInt64(kPrefsUpdateStatePartitionNextOperation, next_op_index);
-}
-
VABCPartitionWriter::~VABCPartitionWriter() {
- // Reset |kPrefsUpdateStatePartitionNextOperation| once we finished a
- // partition.
- prefs_->SetInt64(kPrefsUpdateStatePartitionNextOperation, -1);
cow_writer_->Finalize();
}
diff --git a/payload_consumer/vabc_partition_writer.h b/payload_consumer/vabc_partition_writer.h
index ddade70..7657cb4 100644
--- a/payload_consumer/vabc_partition_writer.h
+++ b/payload_consumer/vabc_partition_writer.h
@@ -44,7 +44,6 @@
[[nodiscard]] bool PerformSourceCopyOperation(
const InstallOperation& operation, ErrorCode* error) override;
[[nodiscard]] bool Flush() override;
- void CheckpointUpdateProgress(size_t next_op_index) override;
static bool WriteAllCowOps(size_t block_size,
const std::vector<CowOperation>& converted,