update_engine: Merge contiguous operations (reprise).
This is reapplying CL:269604 with a small fix that prevents it from
adding spurious src_length fields when merging payload operations. This
also revises relevant unit tests to catch the said error. The original
change is described below.
Merges operations of type SOURCE_COPY, REPLACE, or REPLACE_BZ if they
have the same type, are contiguous, have only one destination extent,
and their combined block count does not exceed the chunk size.
BUG=chromium:461651
BUG=chromium:486497
TEST=Unit test (fails before fix, passes after)
TEST=Trybot
Change-Id: I9f3d7f653454e27177428f2d7c0e6485184a7f8b
Reviewed-on: https://chromium-review.googlesource.com/270268
Trybot-Ready: Gilad Arnold <garnold@chromium.org>
Tested-by: Gilad Arnold <garnold@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
diff --git a/delta_performer.cc b/delta_performer.cc
index 5f94e44..87241c0 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -59,6 +59,7 @@
const uint32_t kInPlaceMinorPayloadVersion = 1;
const uint32_t kSourceMinorPayloadVersion = 2;
+const size_t kDefaultChunkSize = 1024 * 1024;
namespace {
const int kUpdateStateOperationInvalid = -1;
diff --git a/delta_performer.h b/delta_performer.h
index a57788d..b36c6ec 100644
--- a/delta_performer.h
+++ b/delta_performer.h
@@ -30,6 +30,9 @@
// The minor version used by the A to B delta generator algorithm.
extern const uint32_t kSourceMinorPayloadVersion;
+// Chunk size used for payloads during test.
+extern const size_t kDefaultChunkSize;
+
class PrefsInterface;
// This class performs the actions in a delta update synchronously. The delta
diff --git a/delta_performer_unittest.cc b/delta_performer_unittest.cc
index a72061b..381f777 100644
--- a/delta_performer_unittest.cc
+++ b/delta_performer_unittest.cc
@@ -111,9 +111,6 @@
kValidOperationData,
};
-// Chuck size used for full payloads during test.
-size_t kDefaultFullChunkSize = 1024 * 1024;
-
} // namespace
class DeltaPerformerTest : public ::testing::Test {
@@ -543,7 +540,7 @@
} else {
if (payload_config.chunk_size == -1)
- payload_config.chunk_size = kDefaultFullChunkSize;
+ payload_config.chunk_size = kDefaultChunkSize;
}
payload_config.target.rootfs_part = state->b_img;
payload_config.target.rootfs_mountpt = b_mnt;
diff --git a/payload_generator/annotated_operation.cc b/payload_generator/annotated_operation.cc
index 0e73479..7f8a36c 100644
--- a/payload_generator/annotated_operation.cc
+++ b/payload_generator/annotated_operation.cc
@@ -8,6 +8,8 @@
#include <base/strings/string_number_conversions.h>
#include <base/strings/stringprintf.h>
+#include "update_engine/utils.h"
+
using std::string;
namespace chromeos_update_engine {
@@ -36,6 +38,18 @@
}
}
+bool AnnotatedOperation::SetOperationBlob(chromeos::Blob* blob, int data_fd,
+ off_t* data_file_size) {
+ TEST_AND_RETURN_FALSE(utils::PWriteAll(data_fd,
+ blob->data(),
+ blob->size(),
+ *data_file_size));
+ op.set_data_length(blob->size());
+ op.set_data_offset(*data_file_size);
+ *data_file_size += blob->size();
+ return true;
+}
+
string InstallOperationTypeName(
DeltaArchiveManifest_InstallOperation_Type op_type) {
switch (op_type) {
diff --git a/payload_generator/annotated_operation.h b/payload_generator/annotated_operation.h
index e370cfe..d07b6ae 100644
--- a/payload_generator/annotated_operation.h
+++ b/payload_generator/annotated_operation.h
@@ -8,6 +8,7 @@
#include <ostream> // NOLINT(readability/streams)
#include <string>
+#include <chromeos/secure_blob.h>
#include "update_engine/update_metadata.pb.h"
namespace chromeos_update_engine {
@@ -23,6 +24,12 @@
// Sets |name| to a human readable representation of a chunk in a file.
void SetNameFromFileAndChunk(const std::string& filename,
off_t chunk_offset, off_t chunk_size);
+
+ // Writes |blob| to the end of |data_fd|, and updates |data_file_size| to
+ // match the new size of |data_fd|. It sets the data_offset and data_length
+ // in AnnotatedOperation to match the offset and size of |blob| in |data_fd|.
+ bool SetOperationBlob(chromeos::Blob* blob, int data_fd,
+ off_t* data_file_size);
};
// For logging purposes.
diff --git a/payload_generator/delta_diff_generator.cc b/payload_generator/delta_diff_generator.cc
index ff759f6..14586f7 100644
--- a/payload_generator/delta_diff_generator.cc
+++ b/payload_generator/delta_diff_generator.cc
@@ -554,7 +554,7 @@
// Read in bytes from new data.
chromeos::Blob new_data;
TEST_AND_RETURN_FALSE(utils::ReadExtents(new_part,
- &dst_extents,
+ dst_extents,
&new_data,
kBlockSize * blocks_to_write,
kBlockSize));
@@ -582,7 +582,7 @@
if (original) {
// Read old data.
TEST_AND_RETURN_FALSE(
- utils::ReadExtents(old_part, &src_extents, &old_data,
+ utils::ReadExtents(old_part, src_extents, &old_data,
kBlockSize * blocks_to_read, kBlockSize));
if (old_data == new_data) {
// No change in data.
@@ -933,6 +933,16 @@
}
}
+// Stores all extents in |extents| into |out_vector|.
+void DeltaDiffGenerator::ExtentsToVector(
+ const google::protobuf::RepeatedPtrField<Extent>& extents,
+ vector<Extent>* out_vector) {
+ out_vector->clear();
+ for (int i = 0; i < extents.size(); i++) {
+ out_vector->push_back(extents.Get(i));
+ }
+}
+
// Returns true if |op| is a no-op operation that doesn't do any useful work
// (e.g., a move operation that copies blocks onto themselves).
bool DeltaDiffGenerator::IsNoopOperation(
@@ -1069,12 +1079,23 @@
data_file_fd,
data_file_size));
TEST_AND_RETURN_FALSE(FragmentOperations(kernel_ops,
- config.target.rootfs_part,
+ config.target.kernel_part,
data_file_fd,
data_file_size));
SortOperationsByDestination(rootfs_ops);
SortOperationsByDestination(kernel_ops);
-
+ // TODO(alliewood): Change merge operations to use config.chunk_size once
+ // specifying chunk_size on the command line works. crbug/485397.
+ TEST_AND_RETURN_FALSE(MergeOperations(rootfs_ops,
+ kDefaultChunkSize,
+ config.target.rootfs_part,
+ data_file_fd,
+ data_file_size));
+ TEST_AND_RETURN_FALSE(MergeOperations(kernel_ops,
+ kDefaultChunkSize,
+ config.target.kernel_part,
+ data_file_fd,
+ data_file_size));
return true;
}
@@ -1364,7 +1385,7 @@
bool DeltaDiffGenerator::FragmentOperations(
vector<AnnotatedOperation>* aops,
- const string& target_rootfs_part,
+ const string& target_part_path,
int data_fd,
off_t* data_file_size) {
vector<AnnotatedOperation> fragmented_aops;
@@ -1379,7 +1400,7 @@
DeltaArchiveManifest_InstallOperation_Type_REPLACE_BZ) {
TEST_AND_RETURN_FALSE(SplitReplaceBz(aop,
&fragmented_aops,
- target_rootfs_part,
+ target_part_path,
data_fd,
data_file_size));
} else {
@@ -1483,16 +1504,16 @@
bool DeltaDiffGenerator::SplitReplaceBz(
const AnnotatedOperation& original_aop,
vector<AnnotatedOperation>* result_aops,
- const string& target_rootfs_part,
+ const string& target_part_path,
int data_fd,
off_t* data_file_size) {
DeltaArchiveManifest_InstallOperation original_op = original_aop.op;
TEST_AND_RETURN_FALSE(original_op.type() ==
DeltaArchiveManifest_InstallOperation_Type_REPLACE_BZ);
- int target_rootfs_fd = open(target_rootfs_part.c_str(), O_RDONLY, 000);
- TEST_AND_RETURN_FALSE_ERRNO(target_rootfs_fd >= 0);
- ScopedFdCloser target_rootfs_fd_closer(&target_rootfs_fd);
+ int target_part_fd = open(target_part_path.c_str(), O_RDONLY, 000);
+ TEST_AND_RETURN_FALSE_ERRNO(target_part_fd >= 0);
+ ScopedFdCloser target_part_fd_closer(&target_part_fd);
for (int i = 0; i < original_op.dst_extents_size(); i++) {
Extent dst_ext = original_op.dst_extents(i);
@@ -1506,7 +1527,7 @@
// Get the original uncompressed data for this extent.
ssize_t bytes_read;
chromeos::Blob uncompressed_data(uncompressed_data_size);
- TEST_AND_RETURN_FALSE(utils::PReadAll(target_rootfs_fd,
+ TEST_AND_RETURN_FALSE(utils::PReadAll(target_part_fd,
uncompressed_data.data(),
uncompressed_data_size,
kBlockSize * dst_ext.start_block(),
@@ -1517,20 +1538,128 @@
chromeos::Blob new_data_bz;
TEST_AND_RETURN_FALSE(BzipCompress(uncompressed_data, &new_data_bz));
CHECK(!new_data_bz.empty());
- new_op.set_data_length(new_data_bz.size());
- new_op.set_data_offset(*data_file_size);
- TEST_AND_RETURN_FALSE(utils::PWriteAll(data_fd,
- new_data_bz.data(),
- new_data_bz.size(),
- *data_file_size));
- *data_file_size += new_data_bz.size();
AnnotatedOperation new_aop;
new_aop.op = new_op;
+ new_aop.SetOperationBlob(&new_data_bz, data_fd, data_file_size);
new_aop.name = base::StringPrintf("%s:%d", original_aop.name.c_str(), i);
result_aops->push_back(new_aop);
}
return true;
}
+bool DeltaDiffGenerator::MergeOperations(vector<AnnotatedOperation>* aops,
+ off_t chunk_size,
+ const string& target_part_path,
+ int data_fd,
+ off_t* data_file_size) {
+ vector<AnnotatedOperation> new_aops;
+ for (const AnnotatedOperation& curr_aop : *aops) {
+ if (new_aops.empty()) {
+ new_aops.push_back(curr_aop);
+ continue;
+ }
+ AnnotatedOperation& last_aop = new_aops.back();
+
+ if (last_aop.op.dst_extents_size() <= 0 ||
+ curr_aop.op.dst_extents_size() <= 0) {
+ new_aops.push_back(curr_aop);
+ continue;
+ }
+ uint32_t last_dst_idx = last_aop.op.dst_extents_size() - 1;
+ uint32_t last_end_block =
+ last_aop.op.dst_extents(last_dst_idx).start_block() +
+ last_aop.op.dst_extents(last_dst_idx).num_blocks();
+ uint32_t curr_start_block = curr_aop.op.dst_extents(0).start_block();
+ uint32_t combined_block_count =
+ last_aop.op.dst_extents(last_dst_idx).num_blocks() +
+ curr_aop.op.dst_extents(0).num_blocks();
+ bool good_op_type =
+ curr_aop.op.type() ==
+ DeltaArchiveManifest_InstallOperation_Type_SOURCE_COPY ||
+ curr_aop.op.type() ==
+ DeltaArchiveManifest_InstallOperation_Type_REPLACE ||
+ curr_aop.op.type() ==
+ DeltaArchiveManifest_InstallOperation_Type_REPLACE_BZ;
+ if (good_op_type &&
+ last_aop.op.type() == curr_aop.op.type() &&
+ last_end_block == curr_start_block &&
+ static_cast<off_t>(combined_block_count * kBlockSize) <= chunk_size) {
+ // If the operations have the same type (which is a type that we can
+ // merge), are contiguous, are fragmented to have one destination extent,
+ // and their combined block count would be less than chunk size, merge
+ // them.
+ last_aop.name = base::StringPrintf("%s,%s",
+ last_aop.name.c_str(),
+ curr_aop.name.c_str());
+
+ ExtendExtents(last_aop.op.mutable_src_extents(),
+ curr_aop.op.src_extents());
+ if (curr_aop.op.src_length() > 0)
+ last_aop.op.set_src_length(last_aop.op.src_length() +
+ curr_aop.op.src_length());
+ ExtendExtents(last_aop.op.mutable_dst_extents(),
+ curr_aop.op.dst_extents());
+ if (curr_aop.op.dst_length() > 0)
+ last_aop.op.set_dst_length(last_aop.op.dst_length() +
+ curr_aop.op.dst_length());
+ // Set the data length to zero so we know to add the blob later.
+ if (curr_aop.op.type() ==
+ DeltaArchiveManifest_InstallOperation_Type_REPLACE ||
+ curr_aop.op.type() ==
+ DeltaArchiveManifest_InstallOperation_Type_REPLACE_BZ) {
+ last_aop.op.set_data_length(0);
+ }
+ } else {
+ // Otherwise just include the extent as is.
+ new_aops.push_back(curr_aop);
+ }
+ }
+
+ // Set the blobs for REPLACE/REPLACE_BZ operations that have been merged.
+ for (AnnotatedOperation& curr_aop : new_aops) {
+ if (curr_aop.op.data_length() == 0 &&
+ (curr_aop.op.type() ==
+ DeltaArchiveManifest_InstallOperation_Type_REPLACE ||
+ curr_aop.op.type() ==
+ DeltaArchiveManifest_InstallOperation_Type_REPLACE_BZ)) {
+ chromeos::Blob data(curr_aop.op.dst_length());
+ vector<Extent> dst_extents;
+ ExtentsToVector(curr_aop.op.dst_extents(), &dst_extents);
+ TEST_AND_RETURN_FALSE(utils::ReadExtents(target_part_path,
+ dst_extents,
+ &data,
+ data.size(),
+ kBlockSize));
+ if (curr_aop.op.type() ==
+ DeltaArchiveManifest_InstallOperation_Type_REPLACE) {
+ curr_aop.SetOperationBlob(&data, data_fd, data_file_size);
+ } else if (curr_aop.op.type() ==
+ DeltaArchiveManifest_InstallOperation_Type_REPLACE_BZ) {
+ chromeos::Blob data_bz;
+ TEST_AND_RETURN_FALSE(BzipCompress(data, &data_bz));
+ curr_aop.SetOperationBlob(&data_bz, data_fd, data_file_size);
+ }
+ }
+ }
+
+ *aops = new_aops;
+ return true;
+}
+
+void DeltaDiffGenerator::ExtendExtents(
+ google::protobuf::RepeatedPtrField<Extent>* extents,
+ const google::protobuf::RepeatedPtrField<Extent>& extents_to_add) {
+ vector<Extent> extents_vector;
+ vector<Extent> extents_to_add_vector;
+ ExtentsToVector(*extents, &extents_vector);
+ ExtentsToVector(extents_to_add, &extents_to_add_vector);
+ extents_vector.insert(extents_vector.end(),
+ extents_to_add_vector.begin(),
+ extents_to_add_vector.end());
+ NormalizeExtents(&extents_vector);
+ extents->Clear();
+ StoreExtents(extents_vector, extents);
+}
+
}; // namespace chromeos_update_engine
diff --git a/payload_generator/delta_diff_generator.h b/payload_generator/delta_diff_generator.h
index 5aa506c..db72e70 100644
--- a/payload_generator/delta_diff_generator.h
+++ b/payload_generator/delta_diff_generator.h
@@ -161,6 +161,11 @@
static void StoreExtents(const std::vector<Extent>& extents,
google::protobuf::RepeatedPtrField<Extent>* out);
+ // Stores all extents in |extents| into |out_vector|.
+ static void ExtentsToVector(
+ const google::protobuf::RepeatedPtrField<Extent>& extents,
+ std::vector<Extent>* out_vector);
+
// Install operations in the manifest may reference data blobs, which
// are in data_blobs_path. This function creates a new data blobs file
// with the data blobs in the same order as the referencing install
@@ -270,10 +275,28 @@
// one dst extent each.
static bool SplitReplaceBz(const AnnotatedOperation& original_aop,
std::vector<AnnotatedOperation>* result_aops,
- const std::string& target_rootfs_part,
+ const std::string& target_part,
int data_fd,
off_t* data_file_size);
+ // Takes a sorted (by first destination extent) vector of operations |aops|
+ // and merges SOURCE_COPY, REPLACE, and REPLACE_BZ operations in that vector.
+ // It will merge two operations if:
+ // - They are of the same type.
+ // - They are contiguous.
+ // - Their combined blocks do not exceed |chunk_size|.
+ static bool MergeOperations(std::vector<AnnotatedOperation>* aops,
+ off_t chunk_size,
+ const std::string& target_part,
+ int data_fd,
+ off_t* data_file_size);
+
+ // Takes a pointer to extents |extents| and extents |extents_to_add|, and
+ // merges them by adding |extents_to_add| to |extents| and normalizing.
+ static void ExtendExtents(
+ google::protobuf::RepeatedPtrField<Extent>* extents,
+ const google::protobuf::RepeatedPtrField<Extent>& extents_to_add);
+
private:
DISALLOW_COPY_AND_ASSIGN(DeltaDiffGenerator);
};
diff --git a/payload_generator/delta_diff_generator_unittest.cc b/payload_generator/delta_diff_generator_unittest.cc
index d62a4de..77e0193 100644
--- a/payload_generator/delta_diff_generator_unittest.cc
+++ b/payload_generator/delta_diff_generator_unittest.cc
@@ -110,6 +110,10 @@
}
}
+bool ExtentEquals(Extent ext, uint64_t start_block, uint64_t num_blocks) {
+ return ext.start_block() == start_block && ext.num_blocks() == num_blocks;
+}
+
} // namespace
class DeltaDiffGeneratorTest : public ::testing::Test {
@@ -955,8 +959,7 @@
first_op.type());
EXPECT_EQ(2 * kBlockSize, first_op.dst_length());
EXPECT_EQ(1, first_op.dst_extents().size());
- EXPECT_EQ(2, first_op.dst_extents(0).start_block());
- EXPECT_EQ(2, first_op.dst_extents(0).num_blocks());
+ EXPECT_TRUE(ExtentEquals(first_op.dst_extents(0), 2, 2));
// Get the blob corresponding to this extent and compress it.
chromeos::Blob first_blob(data.begin() + (kBlockSize * 2),
data.begin() + (kBlockSize * 4));
@@ -981,8 +984,7 @@
second_op.type());
EXPECT_EQ(kBlockSize, second_op.dst_length());
EXPECT_EQ(1, second_op.dst_extents().size());
- EXPECT_EQ(6, second_op.dst_extents(0).start_block());
- EXPECT_EQ(1, second_op.dst_extents(0).num_blocks());
+ EXPECT_TRUE(ExtentEquals(second_op.dst_extents(0), 6, 1));
chromeos::Blob second_blob(data.begin() + (kBlockSize * 6),
data.begin() + (kBlockSize * 7));
chromeos::Blob second_blob_bz;
@@ -1034,5 +1036,278 @@
EXPECT_EQ(second_aop.name, aops[2].name);
}
+TEST_F(DeltaDiffGeneratorTest, MergeSourceCopyOperationsTest) {
+ vector<AnnotatedOperation> aops;
+ DeltaArchiveManifest_InstallOperation first_op;
+ first_op.set_type(DeltaArchiveManifest_InstallOperation_Type_SOURCE_COPY);
+ first_op.set_src_length(kBlockSize);
+ first_op.set_dst_length(kBlockSize);
+ *(first_op.add_src_extents()) = ExtentForRange(1, 1);
+ *(first_op.add_dst_extents()) = ExtentForRange(6, 1);
+ AnnotatedOperation first_aop;
+ first_aop.op = first_op;
+ first_aop.name = "1";
+ aops.push_back(first_aop);
+
+ DeltaArchiveManifest_InstallOperation second_op;
+ second_op.set_type(DeltaArchiveManifest_InstallOperation_Type_SOURCE_COPY);
+ second_op.set_src_length(3 * kBlockSize);
+ second_op.set_dst_length(3 * kBlockSize);
+ *(second_op.add_src_extents()) = ExtentForRange(2, 2);
+ *(second_op.add_src_extents()) = ExtentForRange(8, 2);
+ *(second_op.add_dst_extents()) = ExtentForRange(7, 3);
+ *(second_op.add_dst_extents()) = ExtentForRange(11, 1);
+ AnnotatedOperation second_aop;
+ second_aop.op = second_op;
+ second_aop.name = "2";
+ aops.push_back(second_aop);
+
+ DeltaArchiveManifest_InstallOperation third_op;
+ third_op.set_type(DeltaArchiveManifest_InstallOperation_Type_SOURCE_COPY);
+ third_op.set_src_length(kBlockSize);
+ third_op.set_dst_length(kBlockSize);
+ *(third_op.add_src_extents()) = ExtentForRange(11, 1);
+ *(third_op.add_dst_extents()) = ExtentForRange(12, 1);
+ AnnotatedOperation third_aop;
+ third_aop.op = third_op;
+ third_aop.name = "3";
+ aops.push_back(third_aop);
+
+ EXPECT_TRUE(DeltaDiffGenerator::MergeOperations(
+ &aops, 5 * kBlockSize, "", 0, nullptr));
+
+ EXPECT_EQ(aops.size(), 1);
+ DeltaArchiveManifest_InstallOperation first_result_op = aops[0].op;
+ EXPECT_EQ(DeltaArchiveManifest_InstallOperation_Type_SOURCE_COPY,
+ first_result_op.type());
+ EXPECT_EQ(kBlockSize * 5, first_result_op.src_length());
+ EXPECT_EQ(3, first_result_op.src_extents().size());
+ EXPECT_TRUE(ExtentEquals(first_result_op.src_extents(0), 1, 3));
+ EXPECT_TRUE(ExtentEquals(first_result_op.src_extents(1), 8, 2));
+ EXPECT_TRUE(ExtentEquals(first_result_op.src_extents(2), 11, 1));
+ EXPECT_EQ(kBlockSize * 5, first_result_op.dst_length());
+ EXPECT_EQ(2, first_result_op.dst_extents().size());
+ EXPECT_TRUE(ExtentEquals(first_result_op.dst_extents(0), 6, 4));
+ EXPECT_TRUE(ExtentEquals(first_result_op.dst_extents(1), 11, 2));
+ EXPECT_EQ(aops[0].name, "1,2,3");
+}
+
+TEST_F(DeltaDiffGeneratorTest, MergeReplaceOperationsTest) {
+ string data_path;
+ EXPECT_TRUE(utils::MakeTempFile(
+ "MergeReplaceTest_data.XXXXXX", &data_path, nullptr));
+ int data_fd = open(data_path.c_str(), O_RDWR, 000);
+ EXPECT_GE(data_fd, 0);
+ ScopedFdCloser data_fd_closer(&data_fd);
+ chromeos::Blob original_data(4 * kBlockSize);
+ test_utils::FillWithData(&original_data);
+ EXPECT_TRUE(utils::WriteFile(
+ data_path.c_str(), original_data.data(), original_data.size()));
+ off_t data_file_size = 4 * kBlockSize;
+
+ string part_path;
+ EXPECT_TRUE(utils::MakeTempFile(
+ "MergeReplaceBzTest_part.XXXXXX", &part_path, nullptr));
+ chromeos::Blob part_data(4 * kBlockSize);
+ test_utils::FillWithData(&part_data);
+ EXPECT_TRUE(utils::WriteFile(
+ part_path.c_str(), part_data.data(), part_data.size()));
+
+ vector<AnnotatedOperation> aops;
+ DeltaArchiveManifest_InstallOperation first_op;
+ first_op.set_type(DeltaArchiveManifest_InstallOperation_Type_REPLACE);
+ first_op.set_dst_length(kBlockSize);
+ first_op.set_data_length(kBlockSize);
+ first_op.set_data_offset(0);
+ *(first_op.add_dst_extents()) = ExtentForRange(0, 1);
+ AnnotatedOperation first_aop;
+ first_aop.op = first_op;
+ first_aop.name = "first";
+ aops.push_back(first_aop);
+
+ DeltaArchiveManifest_InstallOperation second_op;
+ second_op.set_type(DeltaArchiveManifest_InstallOperation_Type_REPLACE);
+ second_op.set_dst_length(3 * kBlockSize);
+ second_op.set_data_length(3 * kBlockSize);
+ second_op.set_data_offset(1 * kBlockSize);
+ *(second_op.add_dst_extents()) = ExtentForRange(1, 3);
+ AnnotatedOperation second_aop;
+ second_aop.op = second_op;
+ second_aop.name = "second";
+ aops.push_back(second_aop);
+
+ EXPECT_TRUE(DeltaDiffGenerator::MergeOperations(
+ &aops, 5 * kBlockSize, part_path, data_fd, &data_file_size));
+
+ EXPECT_EQ(aops.size(), 1);
+ DeltaArchiveManifest_InstallOperation first_result_op = aops[0].op;
+ EXPECT_EQ(DeltaArchiveManifest_InstallOperation_Type_REPLACE,
+ first_result_op.type());
+ EXPECT_FALSE(first_result_op.has_src_length());
+ EXPECT_EQ(kBlockSize * 4, first_result_op.dst_length());
+ EXPECT_EQ(1, first_result_op.dst_extents().size());
+ EXPECT_TRUE(ExtentEquals(first_result_op.dst_extents(0), 0, 4));
+ EXPECT_EQ(4 * kBlockSize, first_result_op.data_length());
+ EXPECT_EQ(4 * kBlockSize, first_result_op.data_offset());
+ EXPECT_EQ(aops[0].name, "first,second");
+
+ // Check to see if the blob in the new extent has what we expect.
+ chromeos::Blob target_data(first_result_op.data_length());
+ ssize_t bytes_read;
+ EXPECT_TRUE(utils::PReadAll(data_fd,
+ target_data.data(),
+ first_result_op.data_length(),
+ first_result_op.data_offset(),
+ &bytes_read));
+ chromeos::Blob first_original_blob(part_data.begin(),
+ part_data.begin() + kBlockSize);
+ chromeos::Blob first_new_blob(target_data.begin(),
+ target_data.begin() + kBlockSize);
+ EXPECT_EQ(first_original_blob, first_new_blob);
+
+ chromeos::Blob second_original_blob(part_data.begin() + kBlockSize,
+ part_data.begin() + (4 * kBlockSize));
+ chromeos::Blob second_new_blob(target_data.begin() + kBlockSize,
+ target_data.end());
+ EXPECT_EQ(second_original_blob, second_new_blob);
+ EXPECT_EQ(data_file_size, 8 * kBlockSize);
+}
+
+TEST_F(DeltaDiffGeneratorTest, MergeReplaceBzOperationsTest) {
+ string data_path;
+ EXPECT_TRUE(utils::MakeTempFile(
+ "MergeReplaceBzTest_data.XXXXXX", &data_path, nullptr));
+ int data_fd = open(data_path.c_str(), O_RDWR, 000);
+ EXPECT_GE(data_fd, 0);
+ ScopedFdCloser data_fd_closer(&data_fd);
+ off_t data_file_size = 0;
+
+ string part_path;
+ EXPECT_TRUE(utils::MakeTempFile(
+ "MergeReplaceBzTest_part.XXXXXX", &part_path, nullptr));
+ chromeos::Blob part_data(3 * kBlockSize);
+ test_utils::FillWithData(&part_data);
+ EXPECT_TRUE(utils::WriteFile(
+ part_path.c_str(), part_data.data(), part_data.size()));
+
+ vector<AnnotatedOperation> aops;
+ DeltaArchiveManifest_InstallOperation first_op;
+ first_op.set_type(DeltaArchiveManifest_InstallOperation_Type_REPLACE_BZ);
+ first_op.set_dst_length(kBlockSize);
+ *(first_op.add_dst_extents()) = ExtentForRange(0, 1);
+ chromeos::Blob first_op_blob(part_data.begin(),
+ part_data.begin() + kBlockSize);
+ chromeos::Blob first_op_bz;
+ EXPECT_TRUE(BzipCompress(first_op_blob, &first_op_bz));
+ first_op.set_data_length(first_op_bz.size());
+ AnnotatedOperation first_aop;
+ first_aop.op = first_op;
+ first_aop.name = "first";
+ aops.push_back(first_aop);
+
+ DeltaArchiveManifest_InstallOperation second_op;
+ second_op.set_type(DeltaArchiveManifest_InstallOperation_Type_REPLACE_BZ);
+ second_op.set_dst_length(2 * kBlockSize);
+ *(second_op.add_dst_extents()) = ExtentForRange(1, 2);
+ chromeos::Blob second_op_blob(part_data.begin() + kBlockSize,
+ part_data.end());
+ chromeos::Blob second_op_bz;
+ EXPECT_TRUE(BzipCompress(second_op_blob, &second_op_bz));
+ second_op.set_data_length(second_op_bz.size());
+ AnnotatedOperation second_aop;
+ second_aop.op = second_op;
+ second_aop.name = "second";
+ aops.push_back(second_aop);
+
+ EXPECT_TRUE(DeltaDiffGenerator::MergeOperations(
+ &aops, 5 * kBlockSize, part_path, data_fd, &data_file_size));
+
+ EXPECT_EQ(aops.size(), 1);
+ DeltaArchiveManifest_InstallOperation result_op = aops[0].op;
+ EXPECT_EQ(DeltaArchiveManifest_InstallOperation_Type_REPLACE_BZ,
+ result_op.type());
+ EXPECT_FALSE(result_op.has_src_length());
+ EXPECT_EQ(kBlockSize * 3, result_op.dst_length());
+ EXPECT_EQ(1, result_op.dst_extents().size());
+ EXPECT_TRUE(ExtentEquals(result_op.dst_extents(0), 0, 3));
+ EXPECT_EQ(aops[0].name, "first,second");
+
+ // Check to see if the blob pointed to in the new extent has what we expect.
+ chromeos::Blob part_data_bz;
+ EXPECT_TRUE(BzipCompress(part_data, &part_data_bz));
+ chromeos::Blob result_data_bz(part_data_bz.size());
+ ssize_t bytes_read;
+ EXPECT_TRUE(utils::PReadAll(data_fd,
+ result_data_bz.data(),
+ result_op.data_length(),
+ result_op.data_offset(),
+ &bytes_read));
+ EXPECT_EQ(part_data_bz, result_data_bz);
+ EXPECT_EQ(result_data_bz.size(), result_op.data_length());
+ EXPECT_EQ(0, result_op.data_offset());
+ EXPECT_EQ(data_file_size, part_data_bz.size());
+}
+
+TEST_F(DeltaDiffGeneratorTest, NoMergeOperationsTest) {
+ // Test to make sure we don't merge operations that shouldn't be merged.
+ vector<AnnotatedOperation> aops;
+ DeltaArchiveManifest_InstallOperation first_op;
+ first_op.set_type(DeltaArchiveManifest_InstallOperation_Type_REPLACE_BZ);
+ *(first_op.add_dst_extents()) = ExtentForRange(0, 1);
+ first_op.set_data_length(kBlockSize);
+ AnnotatedOperation first_aop;
+ first_aop.op = first_op;
+ aops.push_back(first_aop);
+
+ // Should merge with first, except op types don't match...
+ DeltaArchiveManifest_InstallOperation second_op;
+ second_op.set_type(DeltaArchiveManifest_InstallOperation_Type_REPLACE);
+ *(second_op.add_dst_extents()) = ExtentForRange(1, 2);
+ second_op.set_data_length(2 * kBlockSize);
+ AnnotatedOperation second_aop;
+ second_aop.op = second_op;
+ aops.push_back(second_aop);
+
+ // Should merge with second, except it would exceed chunk size...
+ DeltaArchiveManifest_InstallOperation third_op;
+ third_op.set_type(DeltaArchiveManifest_InstallOperation_Type_REPLACE);
+ *(third_op.add_dst_extents()) = ExtentForRange(3, 3);
+ third_op.set_data_length(3 * kBlockSize);
+ AnnotatedOperation third_aop;
+ third_aop.op = third_op;
+ aops.push_back(third_aop);
+
+ // Should merge with third, except they aren't contiguous...
+ DeltaArchiveManifest_InstallOperation fourth_op;
+ fourth_op.set_type(DeltaArchiveManifest_InstallOperation_Type_REPLACE);
+ *(fourth_op.add_dst_extents()) = ExtentForRange(7, 2);
+ fourth_op.set_data_length(2 * kBlockSize);
+ AnnotatedOperation fourth_aop;
+ fourth_aop.op = fourth_op;
+ aops.push_back(fourth_aop);
+
+ EXPECT_TRUE(DeltaDiffGenerator::MergeOperations(&aops, 4 * kBlockSize,
+ "", 0, nullptr));
+
+ // No operations were merged, the number of ops is the same.
+ EXPECT_EQ(aops.size(), 4);
+}
+
+TEST_F(DeltaDiffGeneratorTest, ExtendExtentsTest) {
+ DeltaArchiveManifest_InstallOperation first_op;
+ *(first_op.add_src_extents()) = ExtentForRange(1, 1);
+ *(first_op.add_src_extents()) = ExtentForRange(3, 1);
+
+ DeltaArchiveManifest_InstallOperation second_op;
+ *(second_op.add_src_extents()) = ExtentForRange(4, 2);
+ *(second_op.add_src_extents()) = ExtentForRange(8, 2);
+
+ DeltaDiffGenerator::ExtendExtents(first_op.mutable_src_extents(),
+ second_op.src_extents());
+ EXPECT_EQ(first_op.src_extents_size(), 3);
+ EXPECT_TRUE(ExtentEquals(first_op.src_extents(0), 1, 1));
+ EXPECT_TRUE(ExtentEquals(first_op.src_extents(1), 3, 3));
+ EXPECT_TRUE(ExtentEquals(first_op.src_extents(2), 8, 2));
+}
} // namespace chromeos_update_engine
diff --git a/utils.cc b/utils.cc
index b25b602..1e94ffa 100644
--- a/utils.cc
+++ b/utils.cc
@@ -1676,7 +1676,7 @@
return false;
}
-bool ReadExtents(const std::string& path, vector<Extent>* extents,
+bool ReadExtents(const std::string& path, const vector<Extent>& extents,
chromeos::Blob* out_data, ssize_t out_data_size,
size_t block_size) {
chromeos::Blob data(out_data_size);
@@ -1685,7 +1685,7 @@
TEST_AND_RETURN_FALSE_ERRNO(fd >= 0);
ScopedFdCloser fd_closer(&fd);
- for (const Extent& extent : *extents) {
+ for (const Extent& extent : extents) {
ssize_t bytes_read_this_iteration = 0;
ssize_t bytes = extent.num_blocks() * block_size;
TEST_AND_RETURN_FALSE(bytes_read + bytes <= out_data_size);
diff --git a/utils.h b/utils.h
index 3344a78..8f30ebd 100644
--- a/utils.h
+++ b/utils.h
@@ -468,7 +468,7 @@
// extents are read from the file at |path|. |out_data_size| is the size of
// |out_data|. Returns false if the number of bytes to read given in
// |extents| does not equal |out_data_size|.
-bool ReadExtents(const std::string& path, std::vector<Extent>* extents,
+bool ReadExtents(const std::string& path, const std::vector<Extent>& extents,
chromeos::Blob* out_data, ssize_t out_data_size,
size_t block_size);