Introduce PayloadVersion struct for version information.
This new little struct encapsulates the version information (major and
minor version numbers) and the zlib fingerprint information. Now,
instead of querying throughout if the version number is certain value,
we ask the PayloadVersion struct whether certain operation is allowed in
that version or not. This moves the logic of what's supported and
what's not to a single place and eliminates the need to pass several
booleans to the helper functions.
Bug: None
TEST=Unittest still pass.
Change-Id: Iaa6e7bc032db6479fdeab72255d7062fca1c07e5
diff --git a/payload_generator/ab_generator.cc b/payload_generator/ab_generator.cc
index 740659b..c9619ba 100644
--- a/payload_generator/ab_generator.cc
+++ b/payload_generator/ab_generator.cc
@@ -46,15 +46,13 @@
size_t soft_chunk_blocks = config.soft_chunk_size / config.block_size;
aops->clear();
- TEST_AND_RETURN_FALSE(diff_utils::DeltaReadPartition(
- aops,
- old_part,
- new_part,
- hard_chunk_blocks,
- soft_chunk_blocks,
- blob_file,
- config.imgdiff_allowed,
- true)); // src_ops_allowed
+ TEST_AND_RETURN_FALSE(diff_utils::DeltaReadPartition(aops,
+ old_part,
+ new_part,
+ hard_chunk_blocks,
+ soft_chunk_blocks,
+ config.version,
+ blob_file));
LOG(INFO) << "done reading " << new_part.name;
TEST_AND_RETURN_FALSE(FragmentOperations(aops,
@@ -75,7 +73,7 @@
new_part.path,
blob_file));
- if (config.minor_version >= kOpSrcHashMinorPayloadVersion)
+ if (config.version.minor >= kOpSrcHashMinorPayloadVersion)
TEST_AND_RETURN_FALSE(AddSourceHash(aops, old_part.path));
return true;
diff --git a/payload_generator/delta_diff_generator.cc b/payload_generator/delta_diff_generator.cc
index d8c4275..91d212e 100644
--- a/payload_generator/delta_diff_generator.cc
+++ b/payload_generator/delta_diff_generator.cc
@@ -55,6 +55,11 @@
const string& output_path,
const string& private_key_path,
uint64_t* metadata_size) {
+ if (!config.version.Validate()) {
+ LOG(ERROR) << "Unsupported major.minor version: " << config.version.major
+ << "." << config.version.minor;
+ return false;
+ }
// Create empty payload file object.
PayloadFile payload;
@@ -101,18 +106,12 @@
LOG_IF(FATAL, old_part.size > new_part.size)
<< "Shirking the filesystem size is not supported at the moment.";
}
- if (config.minor_version == kInPlaceMinorPayloadVersion) {
+ if (config.version.minor == kInPlaceMinorPayloadVersion) {
LOG(INFO) << "Using generator InplaceGenerator().";
strategy.reset(new InplaceGenerator());
- } else if (config.minor_version == kSourceMinorPayloadVersion ||
- config.minor_version == kOpSrcHashMinorPayloadVersion ||
- config.minor_version == kImgdiffMinorPayloadVersion) {
+ } else {
LOG(INFO) << "Using generator ABGenerator().";
strategy.reset(new ABGenerator());
- } else {
- LOG(ERROR) << "Unsupported minor version given for delta payload: "
- << config.minor_version;
- return false;
}
} else {
LOG(INFO) << "Using generator FullUpdateGenerator().";
diff --git a/payload_generator/delta_diff_utils.cc b/payload_generator/delta_diff_utils.cc
index 9206c50..7199f7e 100644
--- a/payload_generator/delta_diff_utils.cc
+++ b/payload_generator/delta_diff_utils.cc
@@ -52,6 +52,11 @@
// Chrome binary in ASan builders.
const uint64_t kMaxBsdiffDestinationSize = 200 * 1024 * 1024; // bytes
+// The maximum destination size allowed for imgdiff. In general, imgdiff should
+// work for arbitrary big files, but the payload application is quite memory
+// intensive, so we limit these operations to 50 MiB.
+const uint64_t kMaxImgdiffDestinationSize = 50 * 1024 * 1024; // bytes
+
// Process a range of blocks from |range_start| to |range_end| in the extent at
// position |*idx_p| of |extents|. If |do_remove| is true, this range will be
// removed, which may cause the extent to be trimmed, split or removed entirely.
@@ -163,15 +168,13 @@
namespace diff_utils {
-bool DeltaReadPartition(
- vector<AnnotatedOperation>* aops,
- const PartitionConfig& old_part,
- const PartitionConfig& new_part,
- ssize_t hard_chunk_blocks,
- size_t soft_chunk_blocks,
- BlobFileWriter* blob_file,
- bool imgdiff_allowed,
- bool src_ops_allowed) {
+bool DeltaReadPartition(vector<AnnotatedOperation>* aops,
+ const PartitionConfig& old_part,
+ const PartitionConfig& new_part,
+ ssize_t hard_chunk_blocks,
+ size_t soft_chunk_blocks,
+ const PayloadVersion& version,
+ BlobFileWriter* blob_file) {
ExtentRanges old_visited_blocks;
ExtentRanges new_visited_blocks;
@@ -182,7 +185,7 @@
old_part.fs_interface ? old_part.fs_interface->GetBlockCount() : 0,
new_part.fs_interface->GetBlockCount(),
soft_chunk_blocks,
- src_ops_allowed,
+ version,
blob_file,
&old_visited_blocks,
&new_visited_blocks));
@@ -233,17 +236,15 @@
old_files_map[new_file.name], old_visited_blocks);
old_visited_blocks.AddExtents(old_file_extents);
- TEST_AND_RETURN_FALSE(DeltaReadFile(
- aops,
- old_part.path,
- new_part.path,
- old_file_extents,
- new_file_extents,
- new_file.name, // operation name
- hard_chunk_blocks,
- blob_file,
- imgdiff_allowed,
- src_ops_allowed));
+ TEST_AND_RETURN_FALSE(DeltaReadFile(aops,
+ old_part.path,
+ new_part.path,
+ old_file_extents,
+ new_file_extents,
+ new_file.name, // operation name
+ hard_chunk_blocks,
+ version,
+ blob_file));
}
// Process all the blocks not included in any file. We provided all the unused
// blocks in the old partition as available data.
@@ -265,32 +266,29 @@
// We use the soft_chunk_blocks limit for the <non-file-data> as we don't
// really know the structure of this data and we should not expect it to have
// redundancy between partitions.
- TEST_AND_RETURN_FALSE(DeltaReadFile(
- aops,
- old_part.path,
- new_part.path,
- old_unvisited,
- new_unvisited,
- "<non-file-data>", // operation name
- soft_chunk_blocks,
- blob_file,
- imgdiff_allowed,
- src_ops_allowed));
+ TEST_AND_RETURN_FALSE(DeltaReadFile(aops,
+ old_part.path,
+ new_part.path,
+ old_unvisited,
+ new_unvisited,
+ "<non-file-data>", // operation name
+ soft_chunk_blocks,
+ version,
+ blob_file));
return true;
}
-bool DeltaMovedAndZeroBlocks(
- vector<AnnotatedOperation>* aops,
- const string& old_part,
- const string& new_part,
- size_t old_num_blocks,
- size_t new_num_blocks,
- ssize_t chunk_blocks,
- bool src_ops_allowed,
- BlobFileWriter* blob_file,
- ExtentRanges* old_visited_blocks,
- ExtentRanges* new_visited_blocks) {
+bool DeltaMovedAndZeroBlocks(vector<AnnotatedOperation>* aops,
+ const string& old_part,
+ const string& new_part,
+ size_t old_num_blocks,
+ size_t new_num_blocks,
+ ssize_t chunk_blocks,
+ const PayloadVersion& version,
+ BlobFileWriter* blob_file,
+ ExtentRanges* old_visited_blocks,
+ ExtentRanges* new_visited_blocks) {
vector<BlockMapping::BlockId> old_block_ids;
vector<BlockMapping::BlockId> new_block_ids;
TEST_AND_RETURN_FALSE(MapPartitionBlocks(old_part,
@@ -301,9 +299,10 @@
&old_block_ids,
&new_block_ids));
- // For minor-version=1, we map all the blocks that didn't move, regardless of
- // the contents since they are already copied and no operation is required.
- if (!src_ops_allowed) {
+ // If the update is inplace, we map all the blocks that didn't move,
+ // regardless of the contents since they are already copied and no operation
+ // is required.
+ if (version.InplaceUpdate()) {
uint64_t num_blocks = std::min(old_num_blocks, new_num_blocks);
for (uint64_t block = 0; block < num_blocks; block++) {
if (old_block_ids[block] == new_block_ids[block] &&
@@ -354,26 +353,25 @@
old_blocks_map_it->second.back());
AppendBlockToExtents(&new_identical_blocks, block);
// We can't reuse source blocks in minor version 1 because the cycle
- // breaking algorithm doesn't support that.
- if (!src_ops_allowed)
+ // breaking algorithm used in the in-place update doesn't support that.
+ if (version.InplaceUpdate())
old_blocks_map_it->second.pop_back();
}
// Produce operations for the zero blocks split per output extent.
+ // TODO(deymo): Produce ZERO operations instead of calling DeltaReadFile().
size_t num_ops = aops->size();
new_visited_blocks->AddExtents(new_zeros);
for (Extent extent : new_zeros) {
- TEST_AND_RETURN_FALSE(DeltaReadFile(
- aops,
- "",
- new_part,
- vector<Extent>(), // old_extents
- vector<Extent>{extent}, // new_extents
- "<zeros>",
- chunk_blocks,
- blob_file,
- false, // imgdiff_allowed
- src_ops_allowed));
+ TEST_AND_RETURN_FALSE(DeltaReadFile(aops,
+ "",
+ new_part,
+ vector<Extent>(), // old_extents
+ vector<Extent>{extent}, // new_extents
+ "<zeros>",
+ chunk_blocks,
+ version,
+ blob_file));
}
LOG(INFO) << "Produced " << (aops->size() - num_ops) << " operations for "
<< BlocksInExtents(new_zeros) << " zeroed blocks";
@@ -393,8 +391,9 @@
aops->emplace_back();
AnnotatedOperation* aop = &aops->back();
aop->name = "<identical-blocks>";
- aop->op.set_type(src_ops_allowed ? InstallOperation::SOURCE_COPY
- : InstallOperation::MOVE);
+ aop->op.set_type(version.OperationAllowed(InstallOperation::SOURCE_COPY)
+ ? InstallOperation::SOURCE_COPY
+ : InstallOperation::MOVE);
uint64_t chunk_num_blocks =
std::min(extent.num_blocks() - op_block_offset,
@@ -424,17 +423,15 @@
return true;
}
-bool DeltaReadFile(
- vector<AnnotatedOperation>* aops,
- const string& old_part,
- const string& new_part,
- const vector<Extent>& old_extents,
- const vector<Extent>& new_extents,
- const string& name,
- ssize_t chunk_blocks,
- BlobFileWriter* blob_file,
- bool imgdiff_allowed,
- bool src_ops_allowed) {
+bool DeltaReadFile(vector<AnnotatedOperation>* aops,
+ const string& old_part,
+ const string& new_part,
+ const vector<Extent>& old_extents,
+ const vector<Extent>& new_extents,
+ const string& name,
+ ssize_t chunk_blocks,
+ const PayloadVersion& version,
+ BlobFileWriter* blob_file) {
brillo::Blob data;
InstallOperation operation;
@@ -442,21 +439,6 @@
if (chunk_blocks == -1)
chunk_blocks = total_blocks;
- // If bsdiff breaks again, blacklist the problem file by using:
- // bsdiff_allowed = (name != "/foo/bar")
- //
- // TODO(dgarrett): chromium-os:15274 connect this test to the command line.
- bool bsdiff_allowed = true;
- if (static_cast<uint64_t>(chunk_blocks) * kBlockSize >
- kMaxBsdiffDestinationSize) {
- bsdiff_allowed = false;
- imgdiff_allowed = false;
- }
-
- if (!bsdiff_allowed) {
- LOG(INFO) << "bsdiff blacklisting: " << name;
- }
-
for (uint64_t block_offset = 0; block_offset < total_blocks;
block_offset += chunk_blocks) {
// Split the old/new file in the same chunks. Note that this could drop
@@ -474,11 +456,9 @@
new_part,
old_extents_chunk,
new_extents_chunk,
- bsdiff_allowed,
- imgdiff_allowed,
+ version,
&data,
- &operation,
- src_ops_allowed));
+ &operation));
// Check if the operation writes nothing.
if (operation.dst_extents_size() == 0) {
@@ -517,11 +497,9 @@
const string& new_part,
const vector<Extent>& old_extents,
const vector<Extent>& new_extents,
- bool bsdiff_allowed,
- bool imgdiff_allowed,
+ const PayloadVersion& version,
brillo::Blob* out_data,
- InstallOperation* out_op,
- bool src_ops_allowed) {
+ InstallOperation* out_op) {
InstallOperation operation;
// Data blob that will be written to delta file.
const brillo::Blob* data_blob = nullptr;
@@ -530,6 +508,25 @@
uint64_t blocks_to_read = BlocksInExtents(old_extents);
uint64_t blocks_to_write = BlocksInExtents(new_extents);
+ // Disable bsdiff and imgdiff when the data is too big.
+ bool bsdiff_allowed =
+ version.OperationAllowed(InstallOperation::SOURCE_BSDIFF) ||
+ version.OperationAllowed(InstallOperation::BSDIFF);
+ if (bsdiff_allowed &&
+ blocks_to_read * kBlockSize > kMaxBsdiffDestinationSize) {
+ LOG(INFO) << "bsdiff blacklisted, data too big: "
+ << blocks_to_read * kBlockSize << " bytes";
+ bsdiff_allowed = false;
+ }
+
+ bool imgdiff_allowed = version.OperationAllowed(InstallOperation::IMGDIFF);
+ if (imgdiff_allowed &&
+ blocks_to_read * kBlockSize > kMaxImgdiffDestinationSize) {
+ LOG(INFO) << "imgdiff blacklisted, data too big: "
+ << blocks_to_read * kBlockSize << " bytes";
+ imgdiff_allowed = false;
+ }
+
// Make copies of the extents so we can modify them.
vector<Extent> src_extents = old_extents;
vector<Extent> dst_extents = new_extents;
@@ -569,11 +566,9 @@
kBlockSize * blocks_to_read, kBlockSize));
if (old_data == new_data) {
// No change in data.
- if (src_ops_allowed) {
- operation.set_type(InstallOperation::SOURCE_COPY);
- } else {
- operation.set_type(InstallOperation::MOVE);
- }
+ operation.set_type(version.OperationAllowed(InstallOperation::SOURCE_COPY)
+ ? InstallOperation::SOURCE_COPY
+ : InstallOperation::MOVE);
data_blob = &empty_blob;
} else if (bsdiff_allowed || imgdiff_allowed) {
// If the source file is considered bsdiff safe (no bsdiff bugs
@@ -594,11 +589,10 @@
kBsdiffPath, old_chunk.value(), new_chunk.value(), &bsdiff_delta));
CHECK_GT(bsdiff_delta.size(), static_cast<brillo::Blob::size_type>(0));
if (bsdiff_delta.size() < data_blob->size()) {
- if (src_ops_allowed) {
- operation.set_type(InstallOperation::SOURCE_BSDIFF);
- } else {
- operation.set_type(InstallOperation::BSDIFF);
- }
+ operation.set_type(
+ version.OperationAllowed(InstallOperation::SOURCE_BSDIFF)
+ ? InstallOperation::SOURCE_BSDIFF
+ : InstallOperation::BSDIFF);
data_blob = &bsdiff_delta;
}
}
diff --git a/payload_generator/delta_diff_utils.h b/payload_generator/delta_diff_utils.h
index b22ff63..db88e6c 100644
--- a/payload_generator/delta_diff_utils.h
+++ b/payload_generator/delta_diff_utils.h
@@ -46,9 +46,8 @@
const PartitionConfig& new_part,
ssize_t hard_chunk_blocks,
size_t soft_chunk_blocks,
- BlobFileWriter* blob_file,
- bool imgdiff_allowed,
- bool src_ops_allowed);
+ const PayloadVersion& version,
+ BlobFileWriter* blob_file);
// Create operations in |aops| for identical blocks that moved around in the old
// and new partition and also handle zeroed blocks. The old and new partition
@@ -66,7 +65,7 @@
size_t old_num_blocks,
size_t new_num_blocks,
ssize_t chunk_blocks,
- bool src_ops_allowed,
+ const PayloadVersion& version,
BlobFileWriter* blob_file,
ExtentRanges* old_visited_blocks,
ExtentRanges* new_visited_blocks);
@@ -85,29 +84,24 @@
const std::vector<Extent>& new_extents,
const std::string& name,
ssize_t chunk_blocks,
- BlobFileWriter* blob_file,
- bool imgdiff_allowed,
- bool src_ops_allowed);
+ const PayloadVersion& version,
+ BlobFileWriter* blob_file);
// Reads the blocks |old_extents| from |old_part| (if it exists) and the
// |new_extents| from |new_part| and determines the smallest way to encode
// this |new_extents| for the diff. It stores necessary data in |out_data| and
// fills in |out_op|. If there's no change in old and new files, it creates a
-// MOVE operation. If there is a change, the smallest of REPLACE, REPLACE_BZ,
-// BSDIFF (if |bsdiff_allowed|) or IMGDIFF (if |imgdiff_allowed|) wins.
-// |new_extents| must not be empty.
-// If |src_ops_allowed| is true, it will emit SOURCE_COPY and SOURCE_BSDIFF
-// operations instead of MOVE and BSDIFF, respectively.
-// Returns true on success.
+// MOVE or SOURCE_COPY operation. If there is a change, the smallest of the
+// operations allowed in the given |version| (REPLACE, REPLACE_BZ, BSDIFF,
+// SOURCE_BSDIFF or IMGDIFF) wins.
+// |new_extents| must not be empty. Returns true on success.
bool ReadExtentsToDiff(const std::string& old_part,
const std::string& new_part,
const std::vector<Extent>& old_extents,
const std::vector<Extent>& new_extents,
- bool bsdiff_allowed,
- bool imgdiff_allowed,
+ const PayloadVersion& version,
brillo::Blob* out_data,
- InstallOperation* out_op,
- bool src_ops_allowed);
+ InstallOperation* out_op);
// Runs the bsdiff or imgdiff tool in |diff_path| on two files and returns the
// resulting delta in |out|. Returns true on success.
diff --git a/payload_generator/delta_diff_utils_unittest.cc b/payload_generator/delta_diff_utils_unittest.cc
index 7364965..ce02a54 100644
--- a/payload_generator/delta_diff_utils_unittest.cc
+++ b/payload_generator/delta_diff_utils_unittest.cc
@@ -162,21 +162,22 @@
}
// Helper function to call DeltaMovedAndZeroBlocks() using this class' data
- // members. This simply avoid repeating all the arguments that never change.
+ // members. This simply avoids repeating all the arguments that never change.
bool RunDeltaMovedAndZeroBlocks(ssize_t chunk_blocks,
- bool src_ops_allowed) {
+ uint32_t minor_version) {
BlobFileWriter blob_file(blob_fd_, &blob_size_);
- return diff_utils::DeltaMovedAndZeroBlocks(
- &aops_,
- old_part_.path,
- new_part_.path,
- old_part_.size / block_size_,
- new_part_.size / block_size_,
- chunk_blocks,
- src_ops_allowed,
- &blob_file,
- &old_visited_blocks_,
- &new_visited_blocks_);
+ PayloadVersion version(kChromeOSMajorPayloadVersion, minor_version);
+ version.imgdiff_allowed = true; // Assume no fingerprint mismatch.
+ return diff_utils::DeltaMovedAndZeroBlocks(&aops_,
+ old_part_.path,
+ new_part_.path,
+ old_part_.size / block_size_,
+ new_part_.size / block_size_,
+ chunk_blocks,
+ version,
+ &blob_file,
+ &old_visited_blocks_,
+ &new_visited_blocks_);
}
// Old and new temporary partitions used in the tests. These are initialized
@@ -215,11 +216,9 @@
new_part_.path,
old_extents,
new_extents,
- true, // bsdiff_allowed
- true, // imgdiff_allowed
+ PayloadVersion(kChromeOSMajorPayloadVersion, kInPlaceMinorPayloadVersion),
&data,
- &op,
- false)); // src_ops_allowed
+ &op));
EXPECT_TRUE(data.empty());
EXPECT_TRUE(op.has_type());
@@ -276,11 +275,9 @@
new_part_.path,
old_extents,
new_extents,
- true, // bsdiff_allowed
- true, // imgdiff_allowed
+ PayloadVersion(kChromeOSMajorPayloadVersion, kInPlaceMinorPayloadVersion),
&data,
- &op,
- false)); // src_ops_allowed
+ &op));
EXPECT_TRUE(data.empty());
@@ -341,11 +338,9 @@
new_part_.path,
old_extents,
new_extents,
- true, // bsdiff_allowed
- true, // imgdiff_allowed
+ PayloadVersion(kChromeOSMajorPayloadVersion, kInPlaceMinorPayloadVersion),
&data,
- &op,
- false)); // src_ops_allowed
+ &op));
EXPECT_FALSE(data.empty());
@@ -362,74 +357,6 @@
EXPECT_EQ(1U, BlocksInExtents(op.dst_extents()));
}
-TEST_F(DeltaDiffUtilsTest, BsdiffNotAllowedTest) {
- // Same setup as the previous test, but this time BSDIFF operations are not
- // allowed.
- brillo::Blob data_blob(kBlockSize);
- test_utils::FillWithData(&data_blob);
-
- // The old file is on a different block than the new one.
- vector<Extent> old_extents = { ExtentForRange(1, 1) };
- vector<Extent> new_extents = { ExtentForRange(2, 1) };
-
- EXPECT_TRUE(WriteExtents(old_part_.path, old_extents, kBlockSize, data_blob));
- // Modify one byte in the new file.
- data_blob[0]++;
- EXPECT_TRUE(WriteExtents(new_part_.path, new_extents, kBlockSize, data_blob));
-
- brillo::Blob data;
- InstallOperation op;
- EXPECT_TRUE(diff_utils::ReadExtentsToDiff(
- old_part_.path,
- new_part_.path,
- old_extents,
- new_extents,
- false, // bsdiff_allowed
- false, // imgdiff_allowed
- &data,
- &op,
- false)); // src_ops_allowed
-
- EXPECT_FALSE(data.empty());
-
- // The point of this test is that we don't use BSDIFF the way the above
- // did. The rest of the details are to be caught in other tests.
- EXPECT_TRUE(op.has_type());
- EXPECT_NE(InstallOperation::BSDIFF, op.type());
-}
-
-TEST_F(DeltaDiffUtilsTest, BsdiffNotAllowedMoveTest) {
- brillo::Blob data_blob(kBlockSize);
- test_utils::FillWithData(&data_blob);
-
- // The old file is on a different block than the new one.
- vector<Extent> old_extents = { ExtentForRange(1, 1) };
- vector<Extent> new_extents = { ExtentForRange(2, 1) };
-
- EXPECT_TRUE(WriteExtents(old_part_.path, old_extents, kBlockSize, data_blob));
- EXPECT_TRUE(WriteExtents(new_part_.path, new_extents, kBlockSize, data_blob));
-
- brillo::Blob data;
- InstallOperation op;
- EXPECT_TRUE(diff_utils::ReadExtentsToDiff(
- old_part_.path,
- new_part_.path,
- old_extents,
- new_extents,
- false, // bsdiff_allowed
- false, // imgdiff_allowed
- &data,
- &op,
- false)); // src_ops_allowed
-
- EXPECT_TRUE(data.empty());
-
- // The point of this test is that we can still use a MOVE for a file
- // that is blacklisted.
- EXPECT_TRUE(op.has_type());
- EXPECT_EQ(InstallOperation::MOVE, op.type());
-}
-
TEST_F(DeltaDiffUtilsTest, ReplaceSmallTest) {
// The old file is on a different block than the new one.
vector<Extent> old_extents = { ExtentForRange(1, 1) };
@@ -459,11 +386,10 @@
new_part_.path,
old_extents,
new_extents,
- true, // bsdiff_allowed
- true, // imgdiff_allowed
+ PayloadVersion(kChromeOSMajorPayloadVersion,
+ kInPlaceMinorPayloadVersion),
&data,
- &op,
- false)); // src_ops_allowed
+ &op));
EXPECT_FALSE(data.empty());
EXPECT_TRUE(op.has_type());
@@ -501,11 +427,9 @@
new_part_.path,
old_extents,
new_extents,
- true, // bsdiff_allowed
- true, // imgdiff_allowed
+ PayloadVersion(kChromeOSMajorPayloadVersion, kSourceMinorPayloadVersion),
&data,
- &op,
- true)); // src_ops_allowed
+ &op));
EXPECT_TRUE(data.empty());
EXPECT_TRUE(op.has_type());
@@ -535,11 +459,9 @@
new_part_.path,
old_extents,
new_extents,
- true, // bsdiff_allowed
- true, // imgdiff_allowed
+ PayloadVersion(kChromeOSMajorPayloadVersion, kSourceMinorPayloadVersion),
&data,
- &op,
- true)); // src_ops_allowed
+ &op));
EXPECT_FALSE(data.empty());
EXPECT_TRUE(op.has_type());
@@ -596,7 +518,7 @@
InitializePartitionWithUniqueBlocks(new_part_, block_size_, 42);
EXPECT_TRUE(RunDeltaMovedAndZeroBlocks(-1, // chunk_blocks
- false)); // src_ops_allowed
+ kInPlaceMinorPayloadVersion));
EXPECT_EQ(0U, old_visited_blocks_.blocks());
EXPECT_EQ(0U, new_visited_blocks_.blocks());
@@ -619,7 +541,7 @@
// Most of the blocks rest in the same place, but there's no need for MOVE
// operations on those blocks.
EXPECT_TRUE(RunDeltaMovedAndZeroBlocks(-1, // chunk_blocks
- false)); // src_ops_allowed
+ kInPlaceMinorPayloadVersion));
EXPECT_EQ(kDefaultBlockCount, old_visited_blocks_.blocks());
EXPECT_EQ(kDefaultBlockCount, new_visited_blocks_.blocks());
@@ -649,7 +571,7 @@
brillo::Blob(5 * kBlockSize, 'a')));
EXPECT_TRUE(RunDeltaMovedAndZeroBlocks(10, // chunk_blocks
- true)); // src_ops_allowed
+ kSourceMinorPayloadVersion));
ExtentRanges expected_ranges;
expected_ranges.AddExtent(ExtentForRange(0, 50));
@@ -702,7 +624,7 @@
EXPECT_TRUE(test_utils::WriteFileVector(new_part_.path, partition_data));
EXPECT_TRUE(RunDeltaMovedAndZeroBlocks(-1, // chunk_blocks
- true)); // src_ops_allowed
+ kSourceMinorPayloadVersion));
// There should be only one SOURCE_COPY, for the whole partition and the
// source extents should cover only the first copy of the source file since
@@ -744,7 +666,7 @@
EXPECT_TRUE(WriteExtents(old_part_.path, old_zeros, block_size_, zeros_data));
EXPECT_TRUE(RunDeltaMovedAndZeroBlocks(5, // chunk_blocks
- false)); // src_ops_allowed
+ kInPlaceMinorPayloadVersion));
// Zeroed blocks from old_visited_blocks_ were copied over, so me actually
// use them regardless of the trivial MOVE operation not being emitted.
@@ -799,7 +721,7 @@
new_contents));
EXPECT_TRUE(RunDeltaMovedAndZeroBlocks(-1, // chunk_blocks
- true)); // src_ops_allowed
+ kSourceMinorPayloadVersion));
EXPECT_EQ(permutation.size(), old_visited_blocks_.blocks());
EXPECT_EQ(permutation.size(), new_visited_blocks_.blocks());
diff --git a/payload_generator/full_update_generator_unittest.cc b/payload_generator/full_update_generator_unittest.cc
index d5af9d0..9e62de2 100644
--- a/payload_generator/full_update_generator_unittest.cc
+++ b/payload_generator/full_update_generator_unittest.cc
@@ -35,7 +35,7 @@
protected:
void SetUp() override {
config_.is_delta = false;
- config_.minor_version = kFullPayloadMinorVersion;
+ config_.version.minor = kFullPayloadMinorVersion;
config_.hard_chunk_size = 128 * 1024;
config_.block_size = 4096;
@@ -86,8 +86,10 @@
EXPECT_EQ(new_part_chunks, static_cast<int64_t>(aops.size()));
for (off_t i = 0; i < new_part_chunks; ++i) {
EXPECT_EQ(1, aops[i].op.dst_extents_size());
- EXPECT_EQ(static_cast<uint64_t>(i * config_.hard_chunk_size / config_.block_size),
- aops[i].op.dst_extents(0).start_block()) << "i = " << i;
+ EXPECT_EQ(
+ static_cast<uint64_t>(i * config_.hard_chunk_size / config_.block_size),
+ aops[i].op.dst_extents(0).start_block())
+ << "i = " << i;
EXPECT_EQ(config_.hard_chunk_size / config_.block_size,
aops[i].op.dst_extents(0).num_blocks());
if (aops[i].op.type() != InstallOperation::REPLACE) {
diff --git a/payload_generator/generate_delta_main.cc b/payload_generator/generate_delta_main.cc
index f1fcad5..99af679 100644
--- a/payload_generator/generate_delta_main.cc
+++ b/payload_generator/generate_delta_main.cc
@@ -516,36 +516,35 @@
CHECK(part.OpenFilesystem());
}
- payload_config.major_version = FLAGS_major_version;
+ payload_config.version.major = FLAGS_major_version;
LOG(INFO) << "Using provided major_version=" << FLAGS_major_version;
if (FLAGS_minor_version == -1) {
// Autodetect minor_version by looking at the update_engine.conf in the old
// image.
if (payload_config.is_delta) {
- payload_config.minor_version = kInPlaceMinorPayloadVersion;
+ payload_config.version.minor = kInPlaceMinorPayloadVersion;
brillo::KeyValueStore store;
uint32_t minor_version;
for (const PartitionConfig& part : payload_config.source.partitions) {
if (part.fs_interface && part.fs_interface->LoadSettings(&store) &&
utils::GetMinorVersion(store, &minor_version)) {
- payload_config.minor_version = minor_version;
+ payload_config.version.minor = minor_version;
break;
}
}
} else {
- payload_config.minor_version = kFullPayloadMinorVersion;
+ payload_config.version.minor = kFullPayloadMinorVersion;
}
- LOG(INFO) << "Auto-detected minor_version=" << payload_config.minor_version;
+ LOG(INFO) << "Auto-detected minor_version=" << payload_config.version.minor;
} else {
- payload_config.minor_version = FLAGS_minor_version;
+ payload_config.version.minor = FLAGS_minor_version;
LOG(INFO) << "Using provided minor_version=" << FLAGS_minor_version;
}
- if (payload_config.minor_version >= kImgdiffMinorPayloadVersion &&
- !FLAGS_zlib_fingerprint.empty()) {
+ if (!FLAGS_zlib_fingerprint.empty()) {
if (utils::IsZlibCompatible(FLAGS_zlib_fingerprint)) {
- payload_config.imgdiff_allowed = true;
+ payload_config.version.imgdiff_allowed = true;
} else {
LOG(INFO) << "IMGDIFF operation disabled due to fingerprint mismatch.";
}
diff --git a/payload_generator/inplace_generator.cc b/payload_generator/inplace_generator.cc
index dcc8e01..30dbafc 100644
--- a/payload_generator/inplace_generator.cc
+++ b/payload_generator/inplace_generator.cc
@@ -47,6 +47,10 @@
namespace {
+// The only PayloadVersion supported by this implementation.
+const PayloadVersion kInPlacePayloadVersion{kChromeOSMajorPayloadVersion,
+ kInPlaceMinorPayloadVersion};
+
// This class allocates non-existent temp blocks, starting from
// kTempBlockStart. Other code is responsible for converting these
// temp blocks into real blocks, as the client can't read or write to
@@ -571,9 +575,8 @@
new_extents,
(*graph)[cut.old_dst].aop.name,
-1, // chunk_blocks, forces to have a single operation.
- blob_file,
- false,
- false)); // src_ops_allowed
+ kInPlacePayloadVersion,
+ blob_file));
TEST_AND_RETURN_FALSE(new_aop.size() == 1);
TEST_AND_RETURN_FALSE(AddInstallOpToGraph(
graph, cut.old_dst, nullptr, new_aop.front().op, new_aop.front().name));
@@ -792,6 +795,8 @@
BlobFileWriter* blob_file,
vector<AnnotatedOperation>* aops) {
TEST_AND_RETURN_FALSE(old_part.name == new_part.name);
+ TEST_AND_RETURN_FALSE(config.version.major == kInPlacePayloadVersion.major);
+ TEST_AND_RETURN_FALSE(config.version.minor == kInPlacePayloadVersion.minor);
ssize_t hard_chunk_blocks = (config.hard_chunk_size == -1 ? -1 :
config.hard_chunk_size / config.block_size);
@@ -801,15 +806,13 @@
partition_size = config.rootfs_partition_size;
LOG(INFO) << "Delta compressing " << new_part.name << " partition...";
- TEST_AND_RETURN_FALSE(
- diff_utils::DeltaReadPartition(aops,
- old_part,
- new_part,
- hard_chunk_blocks,
- soft_chunk_blocks,
- blob_file,
- false, // imgdiff_allowed
- false)); // src_ops_allowed
+ TEST_AND_RETURN_FALSE(diff_utils::DeltaReadPartition(aops,
+ old_part,
+ new_part,
+ hard_chunk_blocks,
+ soft_chunk_blocks,
+ config.version,
+ blob_file));
LOG(INFO) << "Done reading " << new_part.name;
TEST_AND_RETURN_FALSE(
diff --git a/payload_generator/payload_file.cc b/payload_generator/payload_file.cc
index d268ab8..de81a39 100644
--- a/payload_generator/payload_file.cc
+++ b/payload_generator/payload_file.cc
@@ -59,10 +59,9 @@
} // namespace
bool PayloadFile::Init(const PayloadGenerationConfig& config) {
- major_version_ = config.major_version;
- TEST_AND_RETURN_FALSE(major_version_ == kChromeOSMajorPayloadVersion ||
- major_version_ == kBrilloMajorPayloadVersion);
- manifest_.set_minor_version(config.minor_version);
+ TEST_AND_RETURN_FALSE(config.version.Validate());
+ major_version_ = config.version.major;
+ manifest_.set_minor_version(config.version.minor);
if (!config.source.ImageInfoIsEmpty())
*(manifest_.mutable_old_image_info()) = config.source.image_info;
@@ -288,7 +287,7 @@
ScopedFileWriterCloser writer_closer(&writer);
uint64_t out_file_size = 0;
- for (auto& part: part_vec_) {
+ for (auto& part : part_vec_) {
for (AnnotatedOperation& aop : part.aops) {
if (!aop.op.has_data_offset())
continue;
diff --git a/payload_generator/payload_generation_config.cc b/payload_generator/payload_generation_config.cc
index 9ecc682..07cf07e 100644
--- a/payload_generator/payload_generation_config.cc
+++ b/payload_generator/payload_generation_config.cc
@@ -107,7 +107,67 @@
&& image_info.build_version().empty();
}
+PayloadVersion::PayloadVersion(uint64_t major_version, uint32_t minor_version) {
+ major = major_version;
+ minor = minor_version;
+}
+
+bool PayloadVersion::Validate() const {
+ TEST_AND_RETURN_FALSE(major == kChromeOSMajorPayloadVersion ||
+ major == kBrilloMajorPayloadVersion);
+ TEST_AND_RETURN_FALSE(minor == kFullPayloadMinorVersion ||
+ minor == kInPlaceMinorPayloadVersion ||
+ minor == kSourceMinorPayloadVersion ||
+ minor == kOpSrcHashMinorPayloadVersion ||
+ minor == kImgdiffMinorPayloadVersion);
+ return true;
+}
+
+bool PayloadVersion::OperationAllowed(InstallOperation_Type operation) const {
+ switch (operation) {
+ // Full operations:
+ case InstallOperation::REPLACE:
+ case InstallOperation::REPLACE_BZ:
+ // These operations were included in the original payload format.
+ return true;
+
+ case InstallOperation::ZERO:
+ case InstallOperation::DISCARD:
+ case InstallOperation::REPLACE_XZ:
+ // These operations are included in the major version used in Brillo, but
+ // can also be used with minor version 3 or newer.
+ return major == kBrilloMajorPayloadVersion ||
+ minor >= kOpSrcHashMinorPayloadVersion;
+
+ // Delta operations:
+ case InstallOperation::MOVE:
+ case InstallOperation::BSDIFF:
+ // MOVE and BSDIFF were replaced by SOURCE_COPY and SOURCE_BSDIFF and
+ // should not be used in newer delta versions, since the idempotent checks
+ // were removed.
+ return minor == kInPlaceMinorPayloadVersion;
+
+ case InstallOperation::SOURCE_COPY:
+ case InstallOperation::SOURCE_BSDIFF:
+ return minor >= kSourceMinorPayloadVersion;
+
+ case InstallOperation::IMGDIFF:
+ return minor >= kImgdiffMinorPayloadVersion && imgdiff_allowed;
+ }
+ return false;
+}
+
+bool PayloadVersion::IsDelta() const {
+ return minor != kFullPayloadMinorVersion;
+}
+
+bool PayloadVersion::InplaceUpdate() const {
+ return minor == kInPlaceMinorPayloadVersion;
+}
+
bool PayloadGenerationConfig::Validate() const {
+ TEST_AND_RETURN_FALSE(version.Validate());
+ TEST_AND_RETURN_FALSE(version.IsDelta() == is_delta);
if (is_delta) {
for (const PartitionConfig& part : source.partitions) {
if (!part.path.empty()) {
@@ -118,32 +178,22 @@
TEST_AND_RETURN_FALSE(part.postinstall.IsEmpty());
}
- // Check for the supported minor_version values.
- TEST_AND_RETURN_FALSE(minor_version == kInPlaceMinorPayloadVersion ||
- minor_version == kSourceMinorPayloadVersion ||
- minor_version == kOpSrcHashMinorPayloadVersion ||
- minor_version == kImgdiffMinorPayloadVersion);
-
- if (imgdiff_allowed)
- TEST_AND_RETURN_FALSE(minor_version >= kImgdiffMinorPayloadVersion);
-
// If new_image_info is present, old_image_info must be present.
TEST_AND_RETURN_FALSE(source.ImageInfoIsEmpty() ==
target.ImageInfoIsEmpty());
} else {
// All the "source" image fields must be empty for full payloads.
TEST_AND_RETURN_FALSE(source.ValidateIsEmpty());
- TEST_AND_RETURN_FALSE(minor_version == kFullPayloadMinorVersion);
}
// In all cases, the target image must exists.
for (const PartitionConfig& part : target.partitions) {
TEST_AND_RETURN_FALSE(part.ValidateExists());
TEST_AND_RETURN_FALSE(part.size % block_size == 0);
- if (minor_version == kInPlaceMinorPayloadVersion &&
+ if (version.minor == kInPlaceMinorPayloadVersion &&
part.name == kLegacyPartitionNameRoot)
TEST_AND_RETURN_FALSE(rootfs_partition_size >= part.size);
- if (major_version == kChromeOSMajorPayloadVersion)
+ if (version.major == kChromeOSMajorPayloadVersion)
TEST_AND_RETURN_FALSE(part.postinstall.IsEmpty());
}
diff --git a/payload_generator/payload_generation_config.h b/payload_generator/payload_generation_config.h
index 92ebbe5..2601821 100644
--- a/payload_generator/payload_generation_config.h
+++ b/payload_generator/payload_generation_config.h
@@ -107,6 +107,34 @@
std::vector<PartitionConfig> partitions;
};
+struct PayloadVersion {
+ PayloadVersion() : PayloadVersion(0, 0) {}
+ PayloadVersion(uint64_t major_version, uint32_t minor_version);
+
+ // Returns whether the PayloadVersion is valid.
+ bool Validate() const;
+
+ // Return whether the passed |operation| is allowed by this payload.
+ bool OperationAllowed(InstallOperation_Type operation) const;
+
+ // Whether this payload version is a delta payload.
+ bool IsDelta() const;
+
+ // Tells whether the update is done in-place, that is, whether the operations
+ // read and write from the same partition.
+ bool InplaceUpdate() const;
+
+ // The major version of the payload.
+ uint64_t major;
+
+ // The minor version of the payload.
+ uint32_t minor;
+
+ // Wheter the IMGDIFF operation is allowed based on the available compressor
+ // in the delta_generator and the one supported by the target.
+ bool imgdiff_allowed = false;
+};
+
// The PayloadGenerationConfig struct encapsulates all the configuration to
// build the requested payload. This includes information about the old and new
// image as well as the restrictions applied to the payload (like minor-version
@@ -125,14 +153,8 @@
// Wheter the requested payload is a delta payload.
bool is_delta = false;
- // Wheter the IMGDIFF operation is allowed.
- bool imgdiff_allowed = false;
-
- // The major_version of the requested payload.
- uint64_t major_version;
-
- // The minor_version of the requested payload.
- uint32_t minor_version;
+ // The major/minor version of the payload.
+ PayloadVersion version;
// The size of the rootfs partition, that not necessarily is the same as the
// filesystem in either source or target version, since there is some space
diff --git a/payload_generator/payload_signer_unittest.cc b/payload_generator/payload_signer_unittest.cc
index f6319d4..e159f08 100644
--- a/payload_generator/payload_signer_unittest.cc
+++ b/payload_generator/payload_signer_unittest.cc
@@ -143,7 +143,7 @@
&load_metadata_size,
nullptr));
EXPECT_EQ(metadata_size, payload_metadata_blob.size());
- EXPECT_EQ(config.major_version, load_major_version);
+ EXPECT_EQ(config.version.major, load_major_version);
EXPECT_EQ(metadata_size, load_metadata_size);
}
@@ -152,13 +152,13 @@
TEST_F(PayloadSignerTest, LoadPayloadV1Test) {
PayloadGenerationConfig config;
- config.major_version = kChromeOSMajorPayloadVersion;
+ config.version.major = kChromeOSMajorPayloadVersion;
DoWriteAndLoadPayloadTest(config);
}
TEST_F(PayloadSignerTest, LoadPayloadV2Test) {
PayloadGenerationConfig config;
- config.major_version = kBrilloMajorPayloadVersion;
+ config.version.major = kBrilloMajorPayloadVersion;
DoWriteAndLoadPayloadTest(config);
}
@@ -220,7 +220,7 @@
ScopedPathUnlinker payload_path_unlinker(payload_path);
PayloadGenerationConfig config;
- config.major_version = kBrilloMajorPayloadVersion;
+ config.version.major = kBrilloMajorPayloadVersion;
PayloadFile payload;
EXPECT_TRUE(payload.Init(config));
uint64_t metadata_size;
@@ -248,7 +248,7 @@
ScopedPathUnlinker payload_path_unlinker(payload_path);
PayloadGenerationConfig config;
- config.major_version = kBrilloMajorPayloadVersion;
+ config.version.major = kBrilloMajorPayloadVersion;
PayloadFile payload;
EXPECT_TRUE(payload.Init(config));
uint64_t metadata_size;