update_engine: Deprecate minor version 1
Minor version 1 was for the old days where we rewrite the signle
partition with an update (no A/B partitions). But those days are long
over and we don't think there is any device out that has this capability
anymore. Even if there is, we can always serve full payloads along with
the stepping stone we have in M53. So this is safe to go.
BUG=chromium:1008553
TEST=sudo FEATURES=test emerge update_engine
TEST=ran cros flash two times.
Change-Id: Ib928ade36af5136cd4013a30dfb39ee7fd5b07b1
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/1829160
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Sen Jiang <senj@chromium.org>
Commit-Queue: Amin Hassani <ahassani@chromium.org>
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index 53acc11..cc39943 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -315,9 +315,8 @@
install_plan_->partitions.size() - partitions_.size();
const InstallPlan::Partition& install_part =
install_plan_->partitions[num_previous_partitions + current_partition_];
- // Open source fds if we have a delta payload with minor version >= 2.
- if (payload_->type == InstallPayloadType::kDelta &&
- GetMinorVersion() != kInPlaceMinorPayloadVersion) {
+ // Open source fds if we have a delta payload.
+ if (payload_->type == InstallPayloadType::kDelta) {
source_path_ = install_part.source_path;
int err;
source_fd_ = OpenFile(source_path_.c_str(), O_RDONLY, false, &err);
@@ -369,9 +368,8 @@
if (current_partition_ >= partitions_.size())
return false;
- // No support for ECC in minor version 1 or full payloads.
- if (payload_->type == InstallPayloadType::kFull ||
- GetMinorVersion() == kInPlaceMinorPayloadVersion)
+ // No support for ECC for full payloads.
+ if (payload_->type == InstallPayloadType::kFull)
return false;
#if USE_FEC
@@ -685,14 +683,6 @@
op_result = PerformZeroOrDiscardOperation(op);
OP_DURATION_HISTOGRAM("ZERO_OR_DISCARD", op_start_time);
break;
- case InstallOperation::MOVE:
- op_result = PerformMoveOperation(op);
- OP_DURATION_HISTOGRAM("MOVE", op_start_time);
- break;
- case InstallOperation::BSDIFF:
- op_result = PerformBsdiffOperation(op);
- OP_DURATION_HISTOGRAM("BSDIFF", op_start_time);
- break;
case InstallOperation::SOURCE_COPY:
op_result = PerformSourceCopyOperation(op, error);
OP_DURATION_HISTOGRAM("SOURCE_COPY", op_start_time);
@@ -1030,57 +1020,6 @@
return true;
}
-bool DeltaPerformer::PerformMoveOperation(const InstallOperation& operation) {
- // Calculate buffer size. Note, this function doesn't do a sliding
- // window to copy in case the source and destination blocks overlap.
- // If we wanted to do a sliding window, we could program the server
- // to generate deltas that effectively did a sliding window.
-
- uint64_t blocks_to_read = 0;
- for (int i = 0; i < operation.src_extents_size(); i++)
- blocks_to_read += operation.src_extents(i).num_blocks();
-
- uint64_t blocks_to_write = 0;
- for (int i = 0; i < operation.dst_extents_size(); i++)
- blocks_to_write += operation.dst_extents(i).num_blocks();
-
- DCHECK_EQ(blocks_to_write, blocks_to_read);
- brillo::Blob buf(blocks_to_write * block_size_);
-
- // Read in bytes.
- ssize_t bytes_read = 0;
- for (int i = 0; i < operation.src_extents_size(); i++) {
- ssize_t bytes_read_this_iteration = 0;
- const Extent& extent = operation.src_extents(i);
- const size_t bytes = extent.num_blocks() * block_size_;
- TEST_AND_RETURN_FALSE(extent.start_block() != kSparseHole);
- TEST_AND_RETURN_FALSE(utils::PReadAll(target_fd_,
- &buf[bytes_read],
- bytes,
- extent.start_block() * block_size_,
- &bytes_read_this_iteration));
- TEST_AND_RETURN_FALSE(bytes_read_this_iteration ==
- static_cast<ssize_t>(bytes));
- bytes_read += bytes_read_this_iteration;
- }
-
- // Write bytes out.
- ssize_t bytes_written = 0;
- for (int i = 0; i < operation.dst_extents_size(); i++) {
- const Extent& extent = operation.dst_extents(i);
- const size_t bytes = extent.num_blocks() * block_size_;
- TEST_AND_RETURN_FALSE(extent.start_block() != kSparseHole);
- TEST_AND_RETURN_FALSE(utils::PWriteAll(target_fd_,
- &buf[bytes_written],
- bytes,
- extent.start_block() * block_size_));
- bytes_written += bytes;
- }
- DCHECK_EQ(bytes_written, bytes_read);
- DCHECK_EQ(bytes_written, static_cast<ssize_t>(buf.size()));
- return true;
-}
-
bool DeltaPerformer::ValidateSourceHash(const brillo::Blob& calculated_hash,
const InstallOperation& operation,
const FileDescriptorPtr source_fd,
@@ -1265,47 +1204,6 @@
return true;
}
-bool DeltaPerformer::PerformBsdiffOperation(const InstallOperation& operation) {
- // Since we delete data off the beginning of the buffer as we use it,
- // the data we need should be exactly at the beginning of the buffer.
- TEST_AND_RETURN_FALSE(buffer_offset_ == operation.data_offset());
- TEST_AND_RETURN_FALSE(buffer_.size() >= operation.data_length());
-
- string input_positions;
- TEST_AND_RETURN_FALSE(ExtentsToBsdiffPositionsString(operation.src_extents(),
- block_size_,
- operation.src_length(),
- &input_positions));
- string output_positions;
- TEST_AND_RETURN_FALSE(ExtentsToBsdiffPositionsString(operation.dst_extents(),
- block_size_,
- operation.dst_length(),
- &output_positions));
-
- TEST_AND_RETURN_FALSE(bsdiff::bspatch(target_path_.c_str(),
- target_path_.c_str(),
- buffer_.data(),
- buffer_.size(),
- input_positions.c_str(),
- output_positions.c_str()) == 0);
- DiscardBuffer(true, buffer_.size());
-
- if (operation.dst_length() % block_size_) {
- // Zero out rest of final block.
- // TODO(adlr): build this into bspatch; it's more efficient that way.
- const Extent& last_extent =
- operation.dst_extents(operation.dst_extents_size() - 1);
- const uint64_t end_byte =
- (last_extent.start_block() + last_extent.num_blocks()) * block_size_;
- const uint64_t begin_byte =
- end_byte - (block_size_ - operation.dst_length() % block_size_);
- brillo::Blob zeros(end_byte - begin_byte);
- TEST_AND_RETURN_FALSE(utils::PWriteAll(
- target_fd_, zeros.data(), end_byte - begin_byte, begin_byte));
- }
- return true;
-}
-
namespace {
class BsdiffExtentFile : public bsdiff::FileInterface {
diff --git a/payload_consumer/delta_performer.h b/payload_consumer/delta_performer.h
index 55cb2a4..4493c2a 100644
--- a/payload_consumer/delta_performer.h
+++ b/payload_consumer/delta_performer.h
@@ -223,8 +223,6 @@
// set even if it fails.
bool PerformReplaceOperation(const InstallOperation& operation);
bool PerformZeroOrDiscardOperation(const InstallOperation& operation);
- bool PerformMoveOperation(const InstallOperation& operation);
- bool PerformBsdiffOperation(const InstallOperation& operation);
bool PerformSourceCopyOperation(const InstallOperation& operation,
ErrorCode* error);
bool PerformSourceBsdiffOperation(const InstallOperation& operation,
diff --git a/payload_consumer/delta_performer_integration_test.cc b/payload_consumer/delta_performer_integration_test.cc
index e064077..904ea5a 100644
--- a/payload_consumer/delta_performer_integration_test.cc
+++ b/payload_consumer/delta_performer_integration_test.cc
@@ -291,7 +291,6 @@
static void GenerateDeltaFile(bool full_kernel,
bool full_rootfs,
- bool noop,
ssize_t chunk_size,
SignatureTest signature_test,
DeltaState* state,
@@ -368,24 +367,16 @@
ones.size()));
}
- if (noop) {
- EXPECT_TRUE(base::CopyFile(base::FilePath(state->a_img),
- base::FilePath(state->b_img)));
- old_image_info = new_image_info;
- } else {
- if (minor_version == kSourceMinorPayloadVersion) {
- // Create a result image with image_size bytes of garbage.
- brillo::Blob ones(state->image_size, 0xff);
- EXPECT_TRUE(utils::WriteFile(
- state->result_img.c_str(), ones.data(), ones.size()));
- EXPECT_EQ(utils::FileSize(state->a_img),
- utils::FileSize(state->result_img));
- }
+ // Create a result image with image_size bytes of garbage.
+ brillo::Blob ones(state->image_size, 0xff);
+ EXPECT_TRUE(
+ utils::WriteFile(state->result_img.c_str(), ones.data(), ones.size()));
+ EXPECT_EQ(utils::FileSize(state->a_img), utils::FileSize(state->result_img));
- EXPECT_TRUE(
- base::CopyFile(GetBuildArtifactsPath().Append("gen/disk_ext2_4k.img"),
- base::FilePath(state->b_img)));
-
+ EXPECT_TRUE(
+ base::CopyFile(GetBuildArtifactsPath().Append("gen/disk_ext2_4k.img"),
+ base::FilePath(state->b_img)));
+ {
// Make some changes to the B image.
string b_mnt;
ScopedLoopMounter b_mounter(state->b_img, &b_mnt, 0);
@@ -460,10 +451,6 @@
std::copy(
std::begin(kNewData), std::end(kNewData), state->new_kernel_data.begin());
- if (noop) {
- state->old_kernel_data = state->new_kernel_data;
- }
-
// Write kernels to disk
EXPECT_TRUE(utils::WriteFile(state->old_kernel.c_str(),
state->old_kernel_data.data(),
@@ -564,7 +551,6 @@
static void ApplyDeltaFile(bool full_kernel,
bool full_rootfs,
- bool noop,
SignatureTest signature_test,
DeltaState* state,
bool hash_checks_mandatory,
@@ -611,11 +597,6 @@
EXPECT_FALSE(signature.data().empty());
}
- if (noop) {
- EXPECT_EQ(0, manifest.install_operations_size());
- EXPECT_EQ(1, manifest.kernel_install_operations_size());
- }
-
if (full_kernel) {
EXPECT_FALSE(manifest.has_old_kernel_info());
} else {
@@ -632,25 +613,12 @@
EXPECT_EQ(manifest.new_image_info().build_version(), "test-build-version");
if (!full_rootfs) {
- if (noop) {
- EXPECT_EQ(manifest.old_image_info().channel(), "test-channel");
- EXPECT_EQ(manifest.old_image_info().board(), "test-board");
- EXPECT_EQ(manifest.old_image_info().version(), "test-version");
- EXPECT_EQ(manifest.old_image_info().key(), "test-key");
- EXPECT_EQ(manifest.old_image_info().build_channel(),
- "test-build-channel");
- EXPECT_EQ(manifest.old_image_info().build_version(),
- "test-build-version");
- } else {
- EXPECT_EQ(manifest.old_image_info().channel(), "src-channel");
- EXPECT_EQ(manifest.old_image_info().board(), "src-board");
- EXPECT_EQ(manifest.old_image_info().version(), "src-version");
- EXPECT_EQ(manifest.old_image_info().key(), "src-key");
- EXPECT_EQ(manifest.old_image_info().build_channel(),
- "src-build-channel");
- EXPECT_EQ(manifest.old_image_info().build_version(),
- "src-build-version");
- }
+ EXPECT_EQ(manifest.old_image_info().channel(), "src-channel");
+ EXPECT_EQ(manifest.old_image_info().board(), "src-board");
+ EXPECT_EQ(manifest.old_image_info().version(), "src-version");
+ EXPECT_EQ(manifest.old_image_info().key(), "src-key");
+ EXPECT_EQ(manifest.old_image_info().build_channel(), "src-build-channel");
+ EXPECT_EQ(manifest.old_image_info().build_version(), "src-build-version");
}
if (full_rootfs) {
@@ -741,25 +709,14 @@
// The partitions should be empty before DeltaPerformer.
install_plan->partitions.clear();
- // With minor version 2, we want the target to be the new image, result_img,
- // but with version 1, we want to update A in place.
- string target_root, target_kernel;
- if (minor_version == kSourceMinorPayloadVersion) {
- target_root = state->result_img;
- target_kernel = state->result_kernel;
- } else {
- target_root = state->a_img;
- target_kernel = state->old_kernel;
- }
-
state->fake_boot_control_.SetPartitionDevice(
kPartitionNameRoot, install_plan->source_slot, state->a_img);
state->fake_boot_control_.SetPartitionDevice(
kPartitionNameKernel, install_plan->source_slot, state->old_kernel);
state->fake_boot_control_.SetPartitionDevice(
- kPartitionNameRoot, install_plan->target_slot, target_root);
+ kPartitionNameRoot, install_plan->target_slot, state->result_img);
state->fake_boot_control_.SetPartitionDevice(
- kPartitionNameKernel, install_plan->target_slot, target_kernel);
+ kPartitionNameKernel, install_plan->target_slot, state->result_kernel);
ErrorCode expected_error, actual_error;
bool continue_writing;
@@ -838,20 +795,12 @@
return;
}
- brillo::Blob updated_kernel_partition;
- if (minor_version == kSourceMinorPayloadVersion) {
- CompareFilesByBlock(
- state->result_kernel, state->new_kernel, state->kernel_size);
- CompareFilesByBlock(state->result_img, state->b_img, state->image_size);
- EXPECT_TRUE(
- utils::ReadFile(state->result_kernel, &updated_kernel_partition));
- } else {
- CompareFilesByBlock(
- state->old_kernel, state->new_kernel, state->kernel_size);
- CompareFilesByBlock(state->a_img, state->b_img, state->image_size);
- EXPECT_TRUE(utils::ReadFile(state->old_kernel, &updated_kernel_partition));
- }
+ CompareFilesByBlock(
+ state->result_kernel, state->new_kernel, state->kernel_size);
+ CompareFilesByBlock(state->result_img, state->b_img, state->image_size);
+ brillo::Blob updated_kernel_partition;
+ EXPECT_TRUE(utils::ReadFile(state->result_kernel, &updated_kernel_partition));
ASSERT_GE(updated_kernel_partition.size(), arraysize(kNewData));
EXPECT_TRUE(std::equal(std::begin(kNewData),
std::end(kNewData),
@@ -897,7 +846,6 @@
void DoSmallImageTest(bool full_kernel,
bool full_rootfs,
- bool noop,
ssize_t chunk_size,
SignatureTest signature_test,
bool hash_checks_mandatory,
@@ -906,7 +854,6 @@
DeltaPerformer* performer = nullptr;
GenerateDeltaFile(full_kernel,
full_rootfs,
- noop,
chunk_size,
signature_test,
&state,
@@ -921,7 +868,6 @@
ScopedPathUnlinker result_kernel_unlinker(state.result_kernel);
ApplyDeltaFile(full_kernel,
full_rootfs,
- noop,
signature_test,
&state,
hash_checks_mandatory,
@@ -936,8 +882,7 @@
bool hash_checks_mandatory) {
DeltaState state;
uint64_t minor_version = kFullPayloadMinorVersion;
- GenerateDeltaFile(
- true, true, false, -1, kSignatureGenerated, &state, minor_version);
+ GenerateDeltaFile(true, true, -1, kSignatureGenerated, &state, minor_version);
ScopedPathUnlinker a_img_unlinker(state.a_img);
ScopedPathUnlinker b_img_unlinker(state.b_img);
ScopedPathUnlinker delta_unlinker(state.delta_path);
@@ -946,7 +891,6 @@
DeltaPerformer* performer = nullptr;
ApplyDeltaFile(true,
true,
- false,
kSignatureGenerated,
&state,
hash_checks_mandatory,
@@ -957,24 +901,18 @@
}
TEST(DeltaPerformerIntegrationTest, RunAsRootSmallImageTest) {
- DoSmallImageTest(false,
- false,
- false,
- -1,
- kSignatureGenerator,
- false,
- kInPlaceMinorPayloadVersion);
+ DoSmallImageTest(
+ false, false, -1, kSignatureGenerator, false, kSourceMinorPayloadVersion);
}
TEST(DeltaPerformerIntegrationTest,
RunAsRootSmallImageSignaturePlaceholderTest) {
DoSmallImageTest(false,
false,
- false,
-1,
kSignatureGeneratedPlaceholder,
false,
- kInPlaceMinorPayloadVersion);
+ kSourceMinorPayloadVersion);
}
TEST(DeltaPerformerIntegrationTest,
@@ -982,120 +920,87 @@
DeltaState state;
GenerateDeltaFile(false,
false,
- false,
-1,
kSignatureGeneratedPlaceholderMismatch,
&state,
- kInPlaceMinorPayloadVersion);
+ kSourceMinorPayloadVersion);
}
TEST(DeltaPerformerIntegrationTest, RunAsRootSmallImageChunksTest) {
DoSmallImageTest(false,
false,
- false,
kBlockSize,
kSignatureGenerator,
false,
- kInPlaceMinorPayloadVersion);
+ kSourceMinorPayloadVersion);
}
TEST(DeltaPerformerIntegrationTest, RunAsRootFullKernelSmallImageTest) {
- DoSmallImageTest(true,
- false,
- false,
- -1,
- kSignatureGenerator,
- false,
- kInPlaceMinorPayloadVersion);
+ DoSmallImageTest(
+ true, false, -1, kSignatureGenerator, false, kSourceMinorPayloadVersion);
}
TEST(DeltaPerformerIntegrationTest, RunAsRootFullSmallImageTest) {
DoSmallImageTest(true,
true,
- false,
-1,
kSignatureGenerator,
true,
kFullPayloadMinorVersion);
}
-TEST(DeltaPerformerIntegrationTest, RunAsRootNoopSmallImageTest) {
- DoSmallImageTest(false,
- false,
- true,
- -1,
- kSignatureGenerator,
- false,
- kInPlaceMinorPayloadVersion);
-}
-
TEST(DeltaPerformerIntegrationTest, RunAsRootSmallImageSignNoneTest) {
- DoSmallImageTest(false,
- false,
- false,
- -1,
- kSignatureNone,
- false,
- kInPlaceMinorPayloadVersion);
+ DoSmallImageTest(
+ false, false, -1, kSignatureNone, false, kSourceMinorPayloadVersion);
}
TEST(DeltaPerformerIntegrationTest, RunAsRootSmallImageSignGeneratedTest) {
- DoSmallImageTest(false,
- false,
- false,
- -1,
- kSignatureGenerated,
- true,
- kInPlaceMinorPayloadVersion);
+ DoSmallImageTest(
+ false, false, -1, kSignatureGenerated, true, kSourceMinorPayloadVersion);
}
TEST(DeltaPerformerIntegrationTest, RunAsRootSmallImageSignGeneratedShellTest) {
DoSmallImageTest(false,
false,
- false,
-1,
kSignatureGeneratedShell,
false,
- kInPlaceMinorPayloadVersion);
+ kSourceMinorPayloadVersion);
}
TEST(DeltaPerformerIntegrationTest,
RunAsRootSmallImageSignGeneratedShellBadKeyTest) {
DoSmallImageTest(false,
false,
- false,
-1,
kSignatureGeneratedShellBadKey,
false,
- kInPlaceMinorPayloadVersion);
+ kSourceMinorPayloadVersion);
}
TEST(DeltaPerformerIntegrationTest,
RunAsRootSmallImageSignGeneratedShellRotateCl1Test) {
DoSmallImageTest(false,
false,
- false,
-1,
kSignatureGeneratedShellRotateCl1,
false,
- kInPlaceMinorPayloadVersion);
+ kSourceMinorPayloadVersion);
}
TEST(DeltaPerformerIntegrationTest,
RunAsRootSmallImageSignGeneratedShellRotateCl2Test) {
DoSmallImageTest(false,
false,
- false,
-1,
kSignatureGeneratedShellRotateCl2,
false,
- kInPlaceMinorPayloadVersion);
+ kSourceMinorPayloadVersion);
}
TEST(DeltaPerformerIntegrationTest, RunAsRootSmallImageSourceOpsTest) {
DoSmallImageTest(false,
false,
- false,
-1,
kSignatureGenerator,
false,
diff --git a/payload_consumer/payload_constants.cc b/payload_consumer/payload_constants.cc
index 213d798..9e684d7 100644
--- a/payload_consumer/payload_constants.cc
+++ b/payload_consumer/payload_constants.cc
@@ -16,22 +16,24 @@
#include "update_engine/payload_consumer/payload_constants.h"
+#include <base/logging.h>
+
namespace chromeos_update_engine {
const uint64_t kChromeOSMajorPayloadVersion = 1;
const uint64_t kBrilloMajorPayloadVersion = 2;
-const uint32_t kMinSupportedMinorPayloadVersion = 1;
-const uint32_t kMaxSupportedMinorPayloadVersion = 6;
-
const uint32_t kFullPayloadMinorVersion = 0;
-const uint32_t kInPlaceMinorPayloadVersion = 1;
+// const uint32_t kInPlaceMinorPayloadVersion = 1; DEPRECATED
const uint32_t kSourceMinorPayloadVersion = 2;
const uint32_t kOpSrcHashMinorPayloadVersion = 3;
const uint32_t kBrotliBsdiffMinorPayloadVersion = 4;
const uint32_t kPuffdiffMinorPayloadVersion = 5;
const uint32_t kVerityMinorPayloadVersion = 6;
+const uint32_t kMinSupportedMinorPayloadVersion = kSourceMinorPayloadVersion;
+const uint32_t kMaxSupportedMinorPayloadVersion = kVerityMinorPayloadVersion;
+
const uint64_t kMinSupportedMajorPayloadVersion = 1;
const uint64_t kMaxSupportedMajorPayloadVersion = 2;
@@ -44,10 +46,6 @@
const char* InstallOperationTypeName(InstallOperation_Type op_type) {
switch (op_type) {
- case InstallOperation::BSDIFF:
- return "BSDIFF";
- case InstallOperation::MOVE:
- return "MOVE";
case InstallOperation::REPLACE:
return "REPLACE";
case InstallOperation::REPLACE_BZ:
@@ -66,6 +64,10 @@
return "PUFFDIFF";
case InstallOperation::BROTLI_BSDIFF:
return "BROTLI_BSDIFF";
+
+ case InstallOperation::BSDIFF:
+ case InstallOperation::MOVE:
+ NOTREACHED();
}
return "<unknown_op>";
}
diff --git a/payload_consumer/payload_constants.h b/payload_consumer/payload_constants.h
index 7f76898..fe823f4 100644
--- a/payload_consumer/payload_constants.h
+++ b/payload_consumer/payload_constants.h
@@ -39,7 +39,7 @@
extern const uint32_t kFullPayloadMinorVersion;
// The minor version used by the in-place delta generator algorithm.
-extern const uint32_t kInPlaceMinorPayloadVersion;
+// extern const uint32_t kInPlaceMinorPayloadVersion; DEPRECATED
// The minor version used by the A to B delta generator algorithm.
extern const uint32_t kSourceMinorPayloadVersion;