Revert "update_engine: Merge contiguous operations."
This reverts commit 6a3ea94766bb4972abbd629779914d80dada1245.
The CL is suspected to close the tree due to paygen failures.
See comments inline (delta_diff_generator.cc) for the culprit.
BUG=chromium:486497
TEST=trybot
Change-Id: Iaed694a79371e7de208b0de90f8e9cefd61dd7e6
Reviewed-on: https://chromium-review.googlesource.com/270183
Trybot-Ready: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Cheng-Yi Chiang <cychiang@chromium.org>
Commit-Queue: Cheng-Yi Chiang <cychiang@chromium.org>
Tested-by: Cheng-Yi Chiang <cychiang@chromium.org>
diff --git a/delta_performer.cc b/delta_performer.cc
index 87241c0..5f94e44 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -59,7 +59,6 @@
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 b36c6ec..a57788d 100644
--- a/delta_performer.h
+++ b/delta_performer.h
@@ -30,9 +30,6 @@
// 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 381f777..a72061b 100644
--- a/delta_performer_unittest.cc
+++ b/delta_performer_unittest.cc
@@ -111,6 +111,9 @@
kValidOperationData,
};
+// Chuck size used for full payloads during test.
+size_t kDefaultFullChunkSize = 1024 * 1024;
+
} // namespace
class DeltaPerformerTest : public ::testing::Test {
@@ -540,7 +543,7 @@
} else {
if (payload_config.chunk_size == -1)
- payload_config.chunk_size = kDefaultChunkSize;
+ payload_config.chunk_size = kDefaultFullChunkSize;
}
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 7f8a36c..0e73479 100644
--- a/payload_generator/annotated_operation.cc
+++ b/payload_generator/annotated_operation.cc
@@ -8,8 +8,6 @@
#include <base/strings/string_number_conversions.h>
#include <base/strings/stringprintf.h>
-#include "update_engine/utils.h"
-
using std::string;
namespace chromeos_update_engine {
@@ -38,18 +36,6 @@
}
}
-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 d07b6ae..e370cfe 100644
--- a/payload_generator/annotated_operation.h
+++ b/payload_generator/annotated_operation.h
@@ -8,7 +8,6 @@
#include <ostream> // NOLINT(readability/streams)
#include <string>
-#include <chromeos/secure_blob.h>
#include "update_engine/update_metadata.pb.h"
namespace chromeos_update_engine {
@@ -24,12 +23,6 @@
// 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 053d750..ff759f6 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,16 +933,6 @@
}
}
-// 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(
@@ -1079,23 +1069,12 @@
data_file_fd,
data_file_size));
TEST_AND_RETURN_FALSE(FragmentOperations(kernel_ops,
- config.target.kernel_part,
+ config.target.rootfs_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;
}
@@ -1385,7 +1364,7 @@
bool DeltaDiffGenerator::FragmentOperations(
vector<AnnotatedOperation>* aops,
- const string& target_part_path,
+ const string& target_rootfs_part,
int data_fd,
off_t* data_file_size) {
vector<AnnotatedOperation> fragmented_aops;
@@ -1400,7 +1379,7 @@
DeltaArchiveManifest_InstallOperation_Type_REPLACE_BZ) {
TEST_AND_RETURN_FALSE(SplitReplaceBz(aop,
&fragmented_aops,
- target_part_path,
+ target_rootfs_part,
data_fd,
data_file_size));
} else {
@@ -1504,16 +1483,16 @@
bool DeltaDiffGenerator::SplitReplaceBz(
const AnnotatedOperation& original_aop,
vector<AnnotatedOperation>* result_aops,
- const string& target_part_path,
+ const string& target_rootfs_part,
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_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);
+ 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);
for (int i = 0; i < original_op.dst_extents_size(); i++) {
Extent dst_ext = original_op.dst_extents(i);
@@ -1527,7 +1506,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_part_fd,
+ TEST_AND_RETURN_FALSE(utils::PReadAll(target_rootfs_fd,
uncompressed_data.data(),
uncompressed_data_size,
kBlockSize * dst_ext.start_block(),
@@ -1538,126 +1517,20 @@
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());
- 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());
- 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 db72e70..5aa506c 100644
--- a/payload_generator/delta_diff_generator.h
+++ b/payload_generator/delta_diff_generator.h
@@ -161,11 +161,6 @@
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
@@ -275,28 +270,10 @@
// one dst extent each.
static bool SplitReplaceBz(const AnnotatedOperation& original_aop,
std::vector<AnnotatedOperation>* result_aops,
- const std::string& target_part,
+ const std::string& target_rootfs_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 5f2c8ed..d62a4de 100644
--- a/payload_generator/delta_diff_generator_unittest.cc
+++ b/payload_generator/delta_diff_generator_unittest.cc
@@ -110,10 +110,6 @@
}
}
-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 {
@@ -959,7 +955,8 @@
first_op.type());
EXPECT_EQ(2 * kBlockSize, first_op.dst_length());
EXPECT_EQ(1, first_op.dst_extents().size());
- EXPECT_TRUE(ExtentEquals(first_op.dst_extents(0), 2, 2));
+ EXPECT_EQ(2, first_op.dst_extents(0).start_block());
+ EXPECT_EQ(2, first_op.dst_extents(0).num_blocks());
// Get the blob corresponding to this extent and compress it.
chromeos::Blob first_blob(data.begin() + (kBlockSize * 2),
data.begin() + (kBlockSize * 4));
@@ -984,7 +981,8 @@
second_op.type());
EXPECT_EQ(kBlockSize, second_op.dst_length());
EXPECT_EQ(1, second_op.dst_extents().size());
- EXPECT_TRUE(ExtentEquals(second_op.dst_extents(0), 6, 1));
+ EXPECT_EQ(6, second_op.dst_extents(0).start_block());
+ EXPECT_EQ(1, second_op.dst_extents(0).num_blocks());
chromeos::Blob second_blob(data.begin() + (kBlockSize * 6),
data.begin() + (kBlockSize * 7));
chromeos::Blob second_blob_bz;
@@ -1036,276 +1034,5 @@
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_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_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 1e94ffa..b25b602 100644
--- a/utils.cc
+++ b/utils.cc
@@ -1676,7 +1676,7 @@
return false;
}
-bool ReadExtents(const std::string& path, const vector<Extent>& extents,
+bool ReadExtents(const std::string& path, 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 8f30ebd..3344a78 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, const std::vector<Extent>& extents,
+bool ReadExtents(const std::string& path, std::vector<Extent>* extents,
chromeos::Blob* out_data, ssize_t out_data_size,
size_t block_size);