Revert "update_engine: Remove sparse hole extents."
Speculatively reverting commit 96b659d794be39762e8e7fd9f72fe6f723e21ec8 due to crbug.com/474497.
Change-Id: I17fd91c8568b30eafea9e70c9f2255ac2dc459a6
Reviewed-on: https://chromium-review.googlesource.com/264306
Reviewed-by: Shawn N <shawnn@chromium.org>
Commit-Queue: Shawn N <shawnn@chromium.org>
Tested-by: Shawn N <shawnn@chromium.org>
diff --git a/delta_performer.cc b/delta_performer.cc
index 0544e12..e6441fe 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -486,6 +486,7 @@
return kMetadataParseSuccess;
}
+
// Wrapper around write. Returns true if all requested bytes
// were written, or false on any error, regardless of progress
// and stores an action exit code in |error|.
@@ -733,12 +734,16 @@
ssize_t bytes_read_this_iteration = 0;
const Extent& extent = operation.src_extents(i);
const size_t bytes = extent.num_blocks() * block_size_;
- TEST_AND_RETURN_FALSE(extent.start_block() != kSparseHole);
- TEST_AND_RETURN_FALSE(utils::PReadAll(fd,
- &buf[bytes_read],
- bytes,
- extent.start_block() * block_size_,
- &bytes_read_this_iteration));
+ if (extent.start_block() == kSparseHole) {
+ bytes_read_this_iteration = bytes;
+ memset(&buf[bytes_read], 0, bytes);
+ } else {
+ TEST_AND_RETURN_FALSE(utils::PReadAll(fd,
+ &buf[bytes_read],
+ bytes,
+ extent.start_block() * block_size_,
+ &bytes_read_this_iteration));
+ }
TEST_AND_RETURN_FALSE(
bytes_read_this_iteration == static_cast<ssize_t>(bytes));
bytes_read += bytes_read_this_iteration;
@@ -757,11 +762,18 @@
for (int i = 0; i < operation.dst_extents_size(); i++) {
const Extent& extent = operation.dst_extents(i);
const size_t bytes = extent.num_blocks() * block_size_;
- TEST_AND_RETURN_FALSE(extent.start_block() != kSparseHole);
- TEST_AND_RETURN_FALSE(utils::PWriteAll(fd,
- &buf[bytes_written],
- bytes,
- extent.start_block() * block_size_));
+ if (extent.start_block() == kSparseHole) {
+ DCHECK(buf.begin() + bytes_written ==
+ std::search_n(buf.begin() + bytes_written,
+ buf.begin() + bytes_written + bytes,
+ bytes, 0));
+ } else {
+ TEST_AND_RETURN_FALSE(
+ utils::PWriteAll(fd,
+ &buf[bytes_written],
+ bytes,
+ extent.start_block() * block_size_));
+ }
bytes_written += bytes;
}
DCHECK_EQ(bytes_written, bytes_read);
@@ -778,9 +790,13 @@
uint64_t length = 0;
for (int i = 0; i < extents.size(); i++) {
Extent extent = extents.Get(i);
- int64_t start = extent.start_block() * block_size;
+ int64_t start = extent.start_block();
uint64_t this_length = min(full_length - length,
extent.num_blocks() * block_size);
+ if (start == static_cast<int64_t>(kSparseHole))
+ start = -1;
+ else
+ start *= block_size;
ret += base::StringPrintf("%" PRIi64 ":%" PRIu64 ",", start, this_length);
length += this_length;
}
diff --git a/delta_performer_unittest.cc b/delta_performer_unittest.cc
index 2b0bc45..e0bd95d 100644
--- a/delta_performer_unittest.cc
+++ b/delta_performer_unittest.cc
@@ -1029,10 +1029,10 @@
}
TEST(DeltaPerformerTest, ExtentsToByteStringTest) {
- uint64_t test[] = {1, 1, 4, 2, 0, 1};
+ uint64_t test[] = {1, 1, 4, 2, kSparseHole, 1, 0, 1};
COMPILE_ASSERT(arraysize(test) % 2 == 0, array_size_uneven);
const uint64_t block_size = 4096;
- const uint64_t file_length = 4 * block_size - 13;
+ const uint64_t file_length = 5 * block_size - 13;
google::protobuf::RepeatedPtrField<Extent> extents;
for (size_t i = 0; i < arraysize(test); i += 2) {
@@ -1041,7 +1041,7 @@
extent->set_num_blocks(test[i + 1]);
}
- string expected_output = "4096:4096,16384:8192,0:4083";
+ string expected_output = "4096:4096,16384:8192,-1:4096,0:4083";
string actual_output;
EXPECT_TRUE(DeltaPerformer::ExtentsToBsdiffPositionsString(extents,
block_size,
diff --git a/payload_generator/delta_diff_generator.cc b/payload_generator/delta_diff_generator.cc
index 70e680e..725442a 100644
--- a/payload_generator/delta_diff_generator.cc
+++ b/payload_generator/delta_diff_generator.cc
@@ -93,10 +93,6 @@
namespace {
-bool IsSparseHole(const Extent &extent) {
- return (extent.start_block() == kSparseHole);
-}
-
// Stores all the extents of |path| into |extents|. Returns true on success.
bool GatherExtents(const string& path,
off_t chunk_offset,
@@ -104,7 +100,8 @@
vector<Extent>* extents) {
extents->clear();
TEST_AND_RETURN_FALSE(
- get_extents_with_chunk_func(path, chunk_offset, chunk_size, extents));
+ get_extents_with_chunk_func(
+ path, chunk_offset, chunk_size, extents));
return true;
}
@@ -302,8 +299,6 @@
bool DeltaDiffGenerator::DeltaReadFiles(Graph* graph,
vector<Block>* blocks,
- const string& old_part,
- const string& new_part,
const string& old_root,
const string& new_root,
off_t chunk_size,
@@ -358,8 +353,6 @@
graph,
Vertex::kInvalidIndex,
blocks,
- old_part,
- new_part,
(should_diff_from_source ? old_root : kEmptyPath),
new_root,
fs_iter.GetPartialPath(),
@@ -376,8 +369,6 @@
bool DeltaDiffGenerator::DeltaReadFile(Graph* graph,
Vertex::Index existing_vertex,
vector<Block>* blocks,
- const string& old_part,
- const string& new_part,
const string& old_root,
const string& new_root,
const string& path, // within new_root
@@ -389,6 +380,9 @@
chromeos::Blob data;
DeltaArchiveManifest_InstallOperation operation;
+ string old_path = (old_root == kEmptyPath) ? kEmptyPath :
+ old_root + path;
+
// If bsdiff breaks again, blacklist the problem file by using:
// bsdiff_allowed = (path != "/foo/bar")
//
@@ -401,19 +395,15 @@
if (!bsdiff_allowed)
LOG(INFO) << "bsdiff blacklisting: " << path;
- string old_filename = (old_root == kEmptyPath) ? kEmptyPath : old_root + path;
-
- TEST_AND_RETURN_FALSE(DeltaDiffGenerator::ReadFileToDiff(old_part,
- new_part,
+ TEST_AND_RETURN_FALSE(DeltaDiffGenerator::ReadFileToDiff(old_path,
+ new_root + path,
chunk_offset,
chunk_size,
bsdiff_allowed,
&data,
&operation,
true,
- src_ops_allowed,
- old_filename,
- new_root + path));
+ src_ops_allowed));
// Check if the operation writes nothing.
if (operation.dst_extents_size() == 0) {
@@ -460,87 +450,19 @@
}
bool DeltaDiffGenerator::ReadFileToDiff(
- const string& old_part,
- const string& new_part,
+ const string& old_filename,
+ const string& new_filename,
off_t chunk_offset,
off_t chunk_size,
bool bsdiff_allowed,
chromeos::Blob* out_data,
DeltaArchiveManifest_InstallOperation* out_op,
bool gather_extents,
- bool src_ops_allowed,
- const string& old_filename,
- const string& new_filename) {
-
- // Do we have an original file to consider?
- off_t old_size = 0;
- bool original = !old_filename.empty();
- if (original && (old_size = utils::FileSize(old_filename)) < 0) {
- // If stat-ing the old file fails, it should be because it doesn't exist.
- TEST_AND_RETURN_FALSE(!utils::FileExists(old_filename.c_str()));
- original = false;
- }
-
- DeltaArchiveManifest_InstallOperation operation;
- vector<Extent> src_extents, dst_extents;
- // Gather source extents if we have an original file.
- if (original) {
- if (gather_extents) {
- TEST_AND_RETURN_FALSE(
- GatherExtents(old_filename, chunk_offset, chunk_size, &src_extents));
- ClearSparseHoles(&src_extents);
- if (src_extents.size() == 0) {
- // Reading from sparse hole, do nothing.
- operation.set_type(DeltaArchiveManifest_InstallOperation_Type_MOVE);
- *out_op = operation;
- return true;
- }
- } else {
- // We have a kernel, so make one extent to cover it all.
- Extent* src_extent = operation.add_src_extents();
- src_extent->set_start_block(0);
- src_extent->set_num_blocks(
- (utils::FileSize(old_filename) + (kBlockSize - 1)) / kBlockSize);
- src_extents.push_back(*src_extent);
- }
- }
-
- // Gather destination extents.
- if (gather_extents) {
- TEST_AND_RETURN_FALSE(
- GatherExtents(new_filename, chunk_offset, chunk_size, &dst_extents));
- ClearSparseHoles(&dst_extents);
- if (dst_extents.size() == 0) {
- // Make an empty move operation.
- operation.set_type(DeltaArchiveManifest_InstallOperation_Type_MOVE);
- *out_op = operation;
- return true;
- }
- } else {
- Extent* dst_extent = operation.add_dst_extents();
- dst_extent->set_start_block(0);
- dst_extent->set_num_blocks(
- (utils::FileSize(new_filename) + (kBlockSize - 1)) / kBlockSize);
- dst_extents.push_back(*dst_extent);
- }
-
- // Figure out how many blocks we need to write to dst_extents.
- uint64_t blocks_to_write = 0;
- for (uint32_t i = 0; i < dst_extents.size(); i++)
- blocks_to_write += dst_extents[i].num_blocks();
-
- // Figure out how many blocks we need to read to src_extents.
- uint64_t blocks_to_read = 0;
- for (uint32_t i = 0; i < src_extents.size(); i++)
- blocks_to_read += src_extents[i].num_blocks();
-
- // Read in bytes from new data.
+ bool src_ops_allowed) {
+ // Read new data in
chromeos::Blob new_data;
- TEST_AND_RETURN_FALSE(utils::ReadExtents(new_part,
- &dst_extents,
- &new_data,
- kBlockSize * blocks_to_write,
- kBlockSize));
+ TEST_AND_RETURN_FALSE(
+ utils::ReadFileChunk(new_filename, chunk_offset, chunk_size, &new_data));
TEST_AND_RETURN_FALSE(!new_data.empty());
TEST_AND_RETURN_FALSE(chunk_size == -1 ||
@@ -549,8 +471,10 @@
chromeos::Blob new_data_bz;
TEST_AND_RETURN_FALSE(BzipCompress(new_data, &new_data_bz));
CHECK(!new_data_bz.empty());
+
chromeos::Blob data; // Data blob that will be written to delta file.
+ DeltaArchiveManifest_InstallOperation operation;
size_t current_best_size = 0;
if (new_data.size() <= new_data_bz.size()) {
operation.set_type(DeltaArchiveManifest_InstallOperation_Type_REPLACE);
@@ -561,12 +485,22 @@
current_best_size = new_data_bz.size();
data = new_data_bz;
}
+
+ // Do we have an original file to consider?
+ off_t old_size = 0;
+ bool original = !old_filename.empty();
+ if (original && (old_size = utils::FileSize(old_filename)) < 0) {
+ // If stat-ing the old file fails, it should be because it doesn't exist.
+ TEST_AND_RETURN_FALSE(!utils::FileExists(old_filename.c_str()));
+ original = false;
+ }
+
chromeos::Blob old_data;
if (original) {
- // Read old data.
+ // Read old data
TEST_AND_RETURN_FALSE(
- utils::ReadExtents(old_part, &src_extents, &old_data,
- kBlockSize * blocks_to_read, kBlockSize));
+ utils::ReadFileChunk(
+ old_filename, chunk_offset, chunk_size, &old_data));
if (old_data == new_data) {
// No change in data.
if (src_ops_allowed) {
@@ -610,12 +544,43 @@
}
}
- operation.set_src_length(utils::FileSize(old_filename));
- operation.set_dst_length(utils::FileSize(new_filename));
-
// Set parameters of the operations
CHECK_EQ(data.size(), current_best_size);
+ vector<Extent> src_extents, dst_extents;
+ if (operation.type() == DeltaArchiveManifest_InstallOperation_Type_MOVE ||
+ operation.type() == DeltaArchiveManifest_InstallOperation_Type_BSDIFF ||
+ operation.type() ==
+ DeltaArchiveManifest_InstallOperation_Type_SOURCE_COPY ||
+ operation.type() ==
+ DeltaArchiveManifest_InstallOperation_Type_SOURCE_BSDIFF) {
+ if (gather_extents) {
+ TEST_AND_RETURN_FALSE(
+ GatherExtents(old_filename,
+ chunk_offset,
+ chunk_size,
+ &src_extents));
+ } else {
+ Extent* src_extent = operation.add_src_extents();
+ src_extent->set_start_block(0);
+ src_extent->set_num_blocks((old_size + kBlockSize - 1) / kBlockSize);
+ }
+ operation.set_src_length(old_data.size());
+ }
+
+ if (gather_extents) {
+ TEST_AND_RETURN_FALSE(
+ GatherExtents(new_filename,
+ chunk_offset,
+ chunk_size,
+ &dst_extents));
+ } else {
+ Extent* dst_extent = operation.add_dst_extents();
+ dst_extent->set_start_block(0);
+ dst_extent->set_num_blocks((new_data.size() + kBlockSize - 1) / kBlockSize);
+ }
+ operation.set_dst_length(new_data.size());
+
if (gather_extents) {
// Remove identical src/dst block ranges in MOVE operations.
if (operation.type() == DeltaArchiveManifest_InstallOperation_Type_MOVE) {
@@ -634,14 +599,6 @@
StoreExtents(dst_extents, operation.mutable_dst_extents());
}
- // Replace operations should not have source extents.
- if (operation.type() == DeltaArchiveManifest_InstallOperation_Type_REPLACE ||
- operation.type() ==
- DeltaArchiveManifest_InstallOperation_Type_REPLACE_BZ) {
- operation.clear_src_extents();
- operation.clear_src_length();
- }
-
out_data->swap(data);
*out_op = operation;
@@ -668,10 +625,8 @@
true, // bsdiff_allowed
&data,
&op,
- false, // gather_extents
- src_ops_allowed,
- old_kernel_part, // Doesn't matter, kernel has no files.
- new_kernel_part));
+ false,
+ src_ops_allowed));
// Check if the operation writes nothing.
if (op.dst_extents_size() == 0) {
@@ -996,8 +951,6 @@
Graph graph;
TEST_AND_RETURN_FALSE(DeltaReadFiles(&graph,
&blocks,
- config.source.rootfs_part,
- config.target.rootfs_part,
config.source.rootfs_mountpt,
config.target.rootfs_mountpt,
config.chunk_size,
@@ -1300,9 +1253,4 @@
kBlockSize);
}
-void DeltaDiffGenerator::ClearSparseHoles(vector<Extent>* extents) {
- extents->erase(std::remove_if(extents->begin(), extents->end(), IsSparseHole),
- extents->end());
-}
-
}; // namespace chromeos_update_engine
diff --git a/payload_generator/delta_diff_generator.h b/payload_generator/delta_diff_generator.h
index 9a35a61..6004d31 100644
--- a/payload_generator/delta_diff_generator.h
+++ b/payload_generator/delta_diff_generator.h
@@ -75,8 +75,6 @@
// and writes any necessary data to the end of data_fd.
static bool DeltaReadFiles(Graph* graph,
std::vector<Block>* blocks,
- const std::string& old_part,
- const std::string& new_part,
const std::string& old_root,
const std::string& new_root,
off_t chunk_size,
@@ -95,8 +93,6 @@
static bool DeltaReadFile(Graph* graph,
Vertex::Index existing_vertex,
std::vector<Block>* blocks,
- const std::string& old_part,
- const std::string& new_part,
const std::string& old_root,
const std::string& new_root,
const std::string& path,
@@ -107,9 +103,9 @@
bool src_ops_allowed);
// Reads old_filename (if it exists) and a new_filename and determines
- // the smallest way to encode this file for the diff. It reads extents from
- // |old_part| and |new_part|. 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
+ // the smallest way to encode this file 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, or the old file doesn't exist,
// the smallest of REPLACE, REPLACE_BZ, or BSDIFF wins.
// new_filename must contain at least one byte.
@@ -118,17 +114,15 @@
// 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.
- static bool ReadFileToDiff(const std::string& old_part,
- const std::string& new_part,
+ static bool ReadFileToDiff(const std::string& old_filename,
+ const std::string& new_filename,
off_t chunk_offset,
off_t chunk_size,
bool bsdiff_allowed,
chromeos::Blob* out_data,
DeltaArchiveManifest_InstallOperation* out_op,
bool gather_extents,
- bool src_ops_allowed,
- const std::string& old_filename,
- const std::string& new_filename);
+ bool src_ops_allowed);
// Delta compresses a kernel partition |new_kernel_part| with knowledge of the
// old kernel partition |old_kernel_part|. If |old_kernel_part| is an empty
@@ -223,9 +217,6 @@
return ret;
}
- // Takes a vector of extents and removes extents that begin in a sparse hole.
- static void ClearSparseHoles(std::vector<Extent>* extents);
-
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 30ef200..22d9c12 100644
--- a/payload_generator/delta_diff_generator_unittest.cc
+++ b/payload_generator/delta_diff_generator_unittest.cc
@@ -9,7 +9,6 @@
#include <sys/types.h>
#include <unistd.h>
-#include <algorithm>
#include <map>
#include <set>
#include <sstream>
@@ -76,39 +75,6 @@
}
}
-// Inserts |data| at block |offset| in |result|. Fills in the remaining blocks
-// so that there are |num_blocks| blocks in |result|.
-bool MakePartition(uint64_t num_blocks, const string& data, off_t offset,
- chromeos::Blob* result) {
- TEST_AND_RETURN_FALSE(
- static_cast<uint64_t>((kBlockSize * offset) + data.size()) <=
- kBlockSize * num_blocks);
- chromeos::Blob out(kBlockSize * num_blocks, 0);
- chromeos::Blob::iterator start = out.begin() + (kBlockSize * offset);
- copy(data.begin(), data.end(), start);
- *result = out;
- return true;
-}
-
-// Copies from |target| to |result|. Gets the substring of |target| starting
-// at block |start_block| of length |num_blocks| blocks and inserts it at block
-// |block_offset| in |result|.
-void UpdatePartition(const string& target, uint64_t start_block,
- uint64_t num_blocks, off_t block_offset,
- chromeos::Blob* result) {
- uint64_t num_insert = num_blocks * kBlockSize;
- off_t target_offset = block_offset * kBlockSize;
-
- const string target_substr = target.substr(kBlockSize * start_block,
- num_insert);
- ASSERT_EQ(target_substr.size(), num_insert);
-
- for (uint64_t i = 0; i < num_insert; i++) {
- result->at(target_offset + i) =
- static_cast<unsigned char>(target_substr[i]);
- }
-}
-
} // namespace
class DeltaDiffGeneratorTest : public ::testing::Test {
@@ -118,13 +84,6 @@
&old_path_, nullptr));
ASSERT_TRUE(utils::MakeTempFile("DeltaDiffGeneratorTest-new_path-XXXXXX",
&new_path_, nullptr));
- ASSERT_TRUE(utils::MakeTempFile(
- "DeltaDiffGeneratorTest-old_part_path-XXXXXX",
- &old_part_path_, nullptr));
- ASSERT_TRUE(utils::MakeTempFile(
- "DeltaDiffGeneratorTest-new_part_path-XXXXXX",
- &new_part_path_, nullptr));
-
// Mock out the extent gathering function.
orig_get_extents_with_chunk_func_ = get_extents_with_chunk_func;
get_extents_with_chunk_func = FakeGetExtents;
@@ -135,8 +94,6 @@
void TearDown() override {
unlink(old_path_.c_str());
unlink(new_path_.c_str());
- unlink(old_part_path_.c_str());
- unlink(new_part_path_.c_str());
get_extents_with_chunk_func = orig_get_extents_with_chunk_func_;
}
@@ -170,10 +127,6 @@
string old_path_;
string new_path_;
- // Paths to old and new temporary filesystems used in the tests.
- string old_part_path_;
- string new_part_path_;
-
GetExtentsWithChunk orig_get_extents_with_chunk_func_;
};
@@ -186,43 +139,27 @@
}
TEST_F(DeltaDiffGeneratorTest, MoveSmallTest) {
- const string random_string(reinterpret_cast<const char*>(kRandomString),
- sizeof(kRandomString));
EXPECT_TRUE(utils::WriteFile(old_path_.c_str(),
- random_string.c_str(),
- random_string.size()));
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString)));
EXPECT_TRUE(utils::WriteFile(new_path_.c_str(),
- random_string.c_str(),
- random_string.size()));
-
- chromeos::Blob old_part;
- chromeos::Blob new_part;
- EXPECT_TRUE(MakePartition(11, random_string, 10, &old_part));
- EXPECT_TRUE(MakePartition(1, random_string, 0, &new_part));
-
- EXPECT_TRUE(utils::WriteFile(old_part_path_.c_str(),
- old_part.data(), old_part.size()));
- EXPECT_TRUE(utils::WriteFile(new_part_path_.c_str(),
- new_part.data(), new_part.size()));
-
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString)));
// Force the old file to be on a different block.
UpdateFakeFileExtents(old_path_, kBlockSize, 10);
UpdateFakeFileExtents(new_path_, kBlockSize);
chromeos::Blob data;
DeltaArchiveManifest_InstallOperation op;
-
- EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_part_path_,
- new_part_path_,
+ EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_path_,
+ new_path_,
0, // chunk_offset
-1, // chunk_size
true, // bsdiff_allowed
&data,
&op,
true, // gather_extents
- false, // src_ops_allowed
- old_path_,
- new_path_));
+ false)); // src_ops_allowed
EXPECT_TRUE(data.empty());
EXPECT_TRUE(op.has_type());
@@ -270,49 +207,24 @@
string random_data;
while (random_data.size() < file_len)
random_data += random_string;
- if (random_data.size() > file_len) {
+ if (random_data.size() > file_len)
random_data.erase(file_len);
- random_data.insert(random_data.end(), (4096 - 3333), 0);
- }
-
EXPECT_TRUE(utils::WriteFile(old_path_.c_str(),
random_data.c_str(), file_len));
EXPECT_TRUE(utils::WriteFile(new_path_.c_str(),
random_data.c_str(), file_len));
- // Make partitions that match the extents and random_data.
- chromeos::Blob old_part;
- chromeos::Blob new_part;
- EXPECT_TRUE(MakePartition(30, "", 0, &old_part));
- EXPECT_TRUE(MakePartition(30, "", 0, &new_part));
-
- UpdatePartition(random_data, 0, 6, 20, &old_part);
- UpdatePartition(random_data, 6, 2, 28, &old_part);
-
- UpdatePartition(random_data, 0, 1, 18, &new_part);
- UpdatePartition(random_data, 1, 2, 21, &new_part);
- UpdatePartition(random_data, 3, 1, 20, &new_part);
- UpdatePartition(random_data, 4, 3, 24, &new_part);
- UpdatePartition(random_data, 7, 1, 29, &new_part);
-
- EXPECT_TRUE(utils::WriteFile(old_part_path_.c_str(),
- old_part.data(), old_part.size()));
- EXPECT_TRUE(utils::WriteFile(new_part_path_.c_str(),
- new_part.data(), new_part.size()));
-
chromeos::Blob data;
DeltaArchiveManifest_InstallOperation op;
- EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_part_path_,
- new_part_path_,
+ EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_path_,
+ new_path_,
0, // chunk_offset
-1, // chunk_size
true, // bsdiff_allowed
&data,
&op,
true, // gather_extents
- false, // src_ops_allowed
- old_path_,
- new_path_));
+ false)); // src_ops_allowed
// Adjust the old/new extents to remove duplicates.
old_extents[0].set_num_blocks(1);
@@ -357,41 +269,25 @@
}
TEST_F(DeltaDiffGeneratorTest, BsdiffSmallTest) {
- const string random_string(reinterpret_cast<const char*>(kRandomString),
- sizeof(kRandomString));
EXPECT_TRUE(utils::WriteFile(old_path_.c_str(),
- random_string.c_str(),
- random_string.size() - 1));
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString) - 1));
EXPECT_TRUE(utils::WriteFile(new_path_.c_str(),
- random_string.c_str(),
- random_string.size()));
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString)));
UpdateFakeExtents();
- chromeos::Blob old_part;
- chromeos::Blob new_part;
- EXPECT_TRUE(MakePartition(
- 1, random_string.substr(0, random_string.size() - 1), 0, &old_part));
- EXPECT_TRUE(MakePartition(1, random_string, 0, &new_part));
-
- EXPECT_TRUE(utils::WriteFile(old_part_path_.c_str(),
- old_part.data(), old_part.size()));
- EXPECT_TRUE(utils::WriteFile(new_part_path_.c_str(),
- new_part.data(), new_part.size()));
-
chromeos::Blob data;
DeltaArchiveManifest_InstallOperation op;
- EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_part_path_,
- new_part_path_,
+ EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_path_,
+ new_path_,
0, // chunk_offset
-1, // chunk_size
true, // bsdiff_allowed
&data,
&op,
true, // gather_extents
- false, // src_ops_allowed
- old_path_,
- new_path_));
-
+ false)); // src_ops_allowed
EXPECT_FALSE(data.empty());
EXPECT_TRUE(op.has_type());
@@ -408,42 +304,26 @@
}
TEST_F(DeltaDiffGeneratorTest, BsdiffNotAllowedTest) {
- const string random_string(reinterpret_cast<const char*>(kRandomString),
- sizeof(kRandomString));
EXPECT_TRUE(utils::WriteFile(old_path_.c_str(),
- random_string.c_str(),
- random_string.size() - 1));
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString) - 1));
EXPECT_TRUE(utils::WriteFile(new_path_.c_str(),
- random_string.c_str(),
- random_string.size()));
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString)));
UpdateFakeExtents();
- chromeos::Blob old_part;
- chromeos::Blob new_part;
- EXPECT_TRUE(MakePartition(
- 1, random_string.substr(0, random_string.size() - 1), 0, &old_part));
- EXPECT_TRUE(MakePartition(1, random_string, 0, &new_part));
-
- EXPECT_TRUE(utils::WriteFile(old_part_path_.c_str(),
- old_part.data(), old_part.size()));
- EXPECT_TRUE(utils::WriteFile(new_part_path_.c_str(),
- new_part.data(), new_part.size()));
-
chromeos::Blob data;
DeltaArchiveManifest_InstallOperation op;
- EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_part_path_,
- new_part_path_,
+ EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_path_,
+ new_path_,
0, // chunk_offset
-1, // chunk_size
false, // bsdiff_allowed
&data,
&op,
true, // gather_extents
- false, // src_ops_allowed
- old_path_,
- new_path_));
-
+ false)); // src_ops_allowed
EXPECT_FALSE(data.empty());
// The point of this test is that we don't use BSDIFF the way the above
@@ -453,41 +333,26 @@
}
TEST_F(DeltaDiffGeneratorTest, BsdiffNotAllowedMoveTest) {
- const string random_string(reinterpret_cast<const char*>(kRandomString),
- sizeof(kRandomString));
EXPECT_TRUE(utils::WriteFile(old_path_.c_str(),
- random_string.c_str(),
- random_string.size()));
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString)));
EXPECT_TRUE(utils::WriteFile(new_path_.c_str(),
- random_string.c_str(),
- random_string.size()));
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString)));
UpdateFakeExtents();
- chromeos::Blob old_part;
- chromeos::Blob new_part;
- EXPECT_TRUE(MakePartition(1, random_string, 0, &old_part));
- EXPECT_TRUE(MakePartition(1, random_string, 0, &new_part));
-
- EXPECT_TRUE(utils::WriteFile(old_part_path_.c_str(),
- old_part.data(), old_part.size()));
- EXPECT_TRUE(utils::WriteFile(new_part_path_.c_str(),
- new_part.data(), new_part.size()));
-
chromeos::Blob data;
DeltaArchiveManifest_InstallOperation op;
- EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_part_path_,
- new_part_path_,
+ EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_path_,
+ new_path_,
0, // chunk_offset
-1, // chunk_size
false, // bsdiff_allowed
&data,
&op,
true, // gather_extents
- false, // src_ops_allowed
- old_path_,
- new_path_));
-
+ false)); // src_ops_allowed
EXPECT_TRUE(data.empty());
// The point of this test is that we can still use a MOVE for a file
@@ -497,54 +362,26 @@
}
TEST_F(DeltaDiffGeneratorTest, ReplaceSmallTest) {
- chromeos::Blob new_part;
-
- chromeos::Blob old_part(kBlockSize, 0);
- EXPECT_TRUE(utils::WriteFile(old_part_path_.c_str(),
- old_part.data(), old_part.size()));
-
- // Fill old_path with zeroes.
- chromeos::Blob old_data(kBlockSize, 0);
- EXPECT_TRUE(utils::WriteFile(old_path_.c_str(),
- old_data.data(),
- old_data.size()));
-
- chromeos::Blob random_data;
- // Make a blob with random data that won't compress well.
- std::mt19937 gen(12345);
- std::uniform_int_distribution<uint8_t> dis(0, 255);
- for (uint32_t i = 0; i < kBlockSize; i++) {
- random_data.push_back(dis(gen));
- }
-
- // Make a blob that's just 1's that will compress well.
- chromeos::Blob ones(kBlockSize, 1);
-
+ chromeos::Blob new_data;
for (int i = 0; i < 2; i++) {
- chromeos::Blob data_to_test = i == 0 ? random_data : ones;
+ new_data.insert(new_data.end(),
+ std::begin(kRandomString), std::end(kRandomString));
EXPECT_TRUE(utils::WriteFile(new_path_.c_str(),
- data_to_test.data(),
- data_to_test.size()));
+ new_data.data(),
+ new_data.size()));
UpdateFakeExtents();
- string data_str(data_to_test.begin(), data_to_test.end());
- EXPECT_TRUE(MakePartition(1, data_str, 0, &new_part));
- EXPECT_TRUE(utils::WriteFile(new_part_path_.c_str(),
- new_part.data(), new_part.size()));
-
chromeos::Blob data;
DeltaArchiveManifest_InstallOperation op;
- EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_part_path_,
- new_part_path_,
+ EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_path_,
+ new_path_,
0, // chunk_offset
-1, // chunk_size
true, // bsdiff_allowed
&data,
&op,
true, // gather_extents
- false, // src_ops_allowed
- old_path_,
- new_path_));
+ false)); // src_ops_allowed
EXPECT_FALSE(data.empty());
EXPECT_TRUE(op.has_type());
@@ -557,45 +394,29 @@
EXPECT_EQ(0, op.src_extents_size());
EXPECT_FALSE(op.has_src_length());
EXPECT_EQ(1, op.dst_extents_size());
- EXPECT_EQ(data_to_test.size(), op.dst_length());
+ EXPECT_EQ(new_data.size(), op.dst_length());
EXPECT_EQ(1, BlocksInExtents(op.dst_extents()));
}
}
TEST_F(DeltaDiffGeneratorTest, BsdiffNoGatherExtentsSmallTest) {
- const string random_string(reinterpret_cast<const char*>(kRandomString),
- sizeof(kRandomString));
EXPECT_TRUE(utils::WriteFile(old_path_.c_str(),
- random_string.data(),
- random_string.size() - 1));
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString) - 1));
EXPECT_TRUE(utils::WriteFile(new_path_.c_str(),
- random_string.c_str(),
- random_string.size()));
-
- chromeos::Blob old_part;
- chromeos::Blob new_part;
- EXPECT_TRUE(MakePartition(
- 1, random_string.substr(0, random_string.size() - 1), 0, &old_part));
- EXPECT_TRUE(MakePartition(1, random_string, 0, &new_part));
-
- EXPECT_TRUE(utils::WriteFile(old_part_path_.c_str(),
- old_part.data(), old_part.size()));
- EXPECT_TRUE(utils::WriteFile(new_part_path_.c_str(),
- new_part.data(), new_part.size()));
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString)));
chromeos::Blob data;
DeltaArchiveManifest_InstallOperation op;
-
- EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_part_path_,
- new_part_path_,
+ EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_path_,
+ new_path_,
0, // chunk_offset
-1, // chunk_size
true, // bsdiff_allowed
&data,
&op,
false, // gather_extents
- false, // src_ops_allowed
- old_path_,
- new_path_));
+ false)); // src_ops_allowed
EXPECT_FALSE(data.empty());
EXPECT_TRUE(op.has_type());
@@ -616,40 +437,25 @@
// Makes sure SOURCE_COPY operations are emitted whenever src_ops_allowed
// is true. It is the same setup as MoveSmallTest, which checks that
// the operation is well-formed.
- const string random_string(reinterpret_cast<const char*>(kRandomString),
- sizeof(kRandomString));
EXPECT_TRUE(utils::WriteFile(old_path_.c_str(),
- random_string.data(),
- random_string.size()));
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString)));
EXPECT_TRUE(utils::WriteFile(new_path_.c_str(),
- random_string.c_str(),
- random_string.size()));
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString)));
UpdateFakeExtents();
chromeos::Blob data;
DeltaArchiveManifest_InstallOperation op;
-
- chromeos::Blob old_part;
- chromeos::Blob new_part;
- EXPECT_TRUE(MakePartition(1, random_string, 0, &old_part));
- EXPECT_TRUE(MakePartition(1, random_string, 0, &new_part));
-
- EXPECT_TRUE(utils::WriteFile(old_part_path_.c_str(),
- old_part.data(), old_part.size()));
- EXPECT_TRUE(utils::WriteFile(new_part_path_.c_str(),
- new_part.data(), new_part.size()));
-
- EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_part_path_,
- new_part_path_,
+ EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_path_,
+ new_path_,
0, // chunk_offset
-1, // chunk_size
true, // bsdiff_allowed
&data,
&op,
true, // gather_extents
- true, // src_ops_allowed
- old_path_,
- new_path_));
+ true)); // src_ops_allowed
EXPECT_TRUE(data.empty());
EXPECT_TRUE(op.has_type());
@@ -660,42 +466,25 @@
// Makes sure SOURCE_BSDIFF operations are emitted whenever src_ops_allowed
// is true. It is the same setup as BsdiffSmallTest, which checks
// that the operation is well-formed.
- const string random_string(reinterpret_cast<const char*>(kRandomString),
- sizeof(kRandomString));
EXPECT_TRUE(utils::WriteFile(old_path_.c_str(),
- random_string.data(),
- random_string.size() - 1));
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString) - 1));
EXPECT_TRUE(utils::WriteFile(new_path_.c_str(),
- random_string.c_str(),
- random_string.size()));
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString)));
UpdateFakeExtents();
- chromeos::Blob old_part;
- chromeos::Blob new_part;
- EXPECT_TRUE(MakePartition(
- 1, random_string.substr(0, random_string.size() - 1), 0, &old_part));
- EXPECT_TRUE(MakePartition(1, random_string, 0, &new_part));
-
- EXPECT_TRUE(utils::WriteFile(old_part_path_.c_str(),
- old_part.data(), old_part.size()));
- EXPECT_TRUE(utils::WriteFile(new_part_path_.c_str(),
- new_part.data(), new_part.size()));
-
chromeos::Blob data;
DeltaArchiveManifest_InstallOperation op;
-
- EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_part_path_,
- new_part_path_,
+ EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_path_,
+ new_path_,
0, // chunk_offset
-1, // chunk_size
true, // bsdiff_allowed
&data,
&op,
true, // gather_extents
- true, // src_ops_allowed
- old_path_,
- new_path_));
-
+ true)); // src_ops_allowed
EXPECT_FALSE(data.empty());
EXPECT_TRUE(op.has_type());
EXPECT_EQ(DeltaArchiveManifest_InstallOperation_Type_SOURCE_BSDIFF,
@@ -757,6 +546,10 @@
*(op.add_dst_extents()) = ExtentForRange(20, 1);
*(op.add_dst_extents()) = ExtentForRange(21, 1);
EXPECT_TRUE(DeltaDiffGenerator::IsNoopOperation(op));
+ *(op.add_src_extents()) = ExtentForRange(kSparseHole, 2);
+ *(op.add_src_extents()) = ExtentForRange(kSparseHole, 1);
+ *(op.add_dst_extents()) = ExtentForRange(kSparseHole, 3);
+ EXPECT_TRUE(DeltaDiffGenerator::IsNoopOperation(op));
*(op.add_src_extents()) = ExtentForRange(24, 1);
*(op.add_dst_extents()) = ExtentForRange(25, 1);
EXPECT_FALSE(DeltaDiffGenerator::IsNoopOperation(op));
@@ -784,18 +577,4 @@
EXPECT_EQ("aop2", ops[1].name);
}
-TEST_F(DeltaDiffGeneratorTest, SparseHolesFilteredTest) {
- // Test to see that extents starting with a sparse hole are filtered out by
- // ClearSparseHoles.
- vector<Extent> extents;
- AddExtent(kSparseHole, 1, &extents);
- AddExtent(21, 2, &extents);
- AddExtent(kSparseHole, 3, &extents);
- AddExtent(29, 1, &extents);
- DeltaDiffGenerator::ClearSparseHoles(&extents);
- EXPECT_EQ(extents.size(), 2);
- EXPECT_EQ(extents[0], ExtentForRange(21, 2));
- EXPECT_EQ(extents[1], ExtentForRange(29, 1));
-}
-
} // namespace chromeos_update_engine
diff --git a/payload_generator/inplace_generator.cc b/payload_generator/inplace_generator.cc
index 23617f3..0e3284c 100644
--- a/payload_generator/inplace_generator.cc
+++ b/payload_generator/inplace_generator.cc
@@ -269,6 +269,8 @@
Extent extent = graph_utils::GetElement(extents, i);
uint64_t start = extent.start_block();
uint64_t num = extent.num_blocks();
+ if (start == kSparseHole)
+ continue;
if (start >= kTempBlockStart || (start + num) >= kTempBlockStart) {
LOG(ERROR) << "temp block!";
LOG(ERROR) << "start: " << start << ", num: " << num;
@@ -289,7 +291,6 @@
bool ConvertCutsToFull(
Graph* graph,
const string& new_root,
- const string& new_part,
int data_fd,
off_t* data_file_size,
vector<Vertex::Index>* op_indexes,
@@ -301,7 +302,6 @@
TEST_AND_RETURN_FALSE(InplaceGenerator::ConvertCutToFullOp(
graph,
cut,
- new_part,
new_root,
data_fd,
data_file_size));
@@ -331,7 +331,6 @@
bool AssignBlockForAdjoiningCuts(
Graph* graph,
const string& new_root,
- const string& new_part,
int data_fd,
off_t* data_file_size,
vector<Vertex::Index>* op_indexes,
@@ -396,7 +395,6 @@
LOG(INFO) << "Unable to find sufficient scratch";
TEST_AND_RETURN_FALSE(ConvertCutsToFull(graph,
new_root,
- new_part,
data_fd,
data_file_size,
op_indexes,
@@ -443,7 +441,6 @@
bool InplaceGenerator::AssignTempBlocks(
Graph* graph,
const string& new_root,
- const string& new_part,
int data_fd,
off_t* data_file_size,
vector<Vertex::Index>* op_indexes,
@@ -467,7 +464,6 @@
CHECK(!cuts_group.empty());
TEST_AND_RETURN_FALSE(AssignBlockForAdjoiningCuts(graph,
new_root,
- new_part,
data_fd,
data_file_size,
op_indexes,
@@ -485,7 +481,6 @@
CHECK(!cuts_group.empty());
TEST_AND_RETURN_FALSE(AssignBlockForAdjoiningCuts(graph,
new_root,
- new_part,
data_fd,
data_file_size,
op_indexes,
@@ -523,7 +518,6 @@
bool InplaceGenerator::ConvertCutToFullOp(Graph* graph,
const CutEdgeVertexes& cut,
- const string& new_part,
const string& new_root,
int data_fd,
off_t* data_file_size) {
@@ -541,8 +535,6 @@
graph,
cut.old_dst,
nullptr,
- kEmptyPath, // old_part
- new_part,
kEmptyPath,
new_root,
(*graph)[cut.old_dst].file_name,
@@ -569,7 +561,6 @@
}
bool InplaceGenerator::ConvertGraphToDag(Graph* graph,
- const string& new_part,
const string& new_root,
int fd,
off_t* data_file_size,
@@ -607,7 +598,6 @@
if (!cuts.empty())
TEST_AND_RETURN_FALSE(AssignTempBlocks(graph,
- new_part,
new_root,
fd,
data_file_size,
@@ -664,6 +654,10 @@
for (int i = 0; i < extents_size; i++) {
const Extent& extent = extents.Get(i);
+ if (extent.start_block() == kSparseHole) {
+ // Hole in sparse file. skip
+ continue;
+ }
for (uint64_t block = extent.start_block();
block < (extent.start_block() + extent.num_blocks()); block++) {
if ((*blocks)[block].*access_type != Vertex::kInvalidIndex) {
@@ -694,8 +688,6 @@
TEST_AND_RETURN_FALSE(
DeltaDiffGenerator::DeltaReadFiles(&graph,
&blocks,
- config.source.rootfs_part,
- config.target.rootfs_part,
config.source.rootfs_mountpt,
config.target.rootfs_mountpt,
config.chunk_size,
@@ -761,7 +753,6 @@
vector<Vertex::Index> final_order;
TEST_AND_RETURN_FALSE(ConvertGraphToDag(
&graph,
- config.target.rootfs_part,
config.target.rootfs_mountpt,
data_file_fd,
data_file_size,
diff --git a/payload_generator/inplace_generator.h b/payload_generator/inplace_generator.h
index 9d10548..5799c0b 100644
--- a/payload_generator/inplace_generator.h
+++ b/payload_generator/inplace_generator.h
@@ -103,7 +103,6 @@
// success.
static bool AssignTempBlocks(
Graph* graph,
- const std::string& new_part,
const std::string& new_root,
int data_fd,
off_t* data_file_size,
@@ -120,7 +119,6 @@
// A->B. Now, A is a full operation.
static bool ConvertCutToFullOp(Graph* graph,
const CutEdgeVertexes& cut,
- const std::string& new_part,
const std::string& new_root,
int data_fd,
off_t* data_file_size);
@@ -135,7 +133,6 @@
// |final_order| before returning.
// Returns true on success.
static bool ConvertGraphToDag(Graph* graph,
- const std::string& new_part,
const std::string& new_root,
int fd,
off_t* data_file_size,
diff --git a/payload_generator/inplace_generator_unittest.cc b/payload_generator/inplace_generator_unittest.cc
index fdf3aba..051902f 100644
--- a/payload_generator/inplace_generator_unittest.cc
+++ b/payload_generator/inplace_generator_unittest.cc
@@ -340,7 +340,6 @@
EXPECT_TRUE(InplaceGenerator::AssignTempBlocks(&graph,
temp_dir,
- "/dev/zero",
fd,
&data_file_size,
&op_indexes,
@@ -437,9 +436,9 @@
ScopedFdCloser fd_closer(&fd);
off_t data_file_size = 0;
+
EXPECT_TRUE(InplaceGenerator::ConvertGraphToDag(&graph,
temp_dir,
- "/dev/zero",
fd,
&data_file_size,
&final_order,
@@ -513,6 +512,189 @@
}
}
+// Test that sparse holes are not used as scratch. More specifically, test that
+// if all full operations write to sparse holes and there's no extra scratch
+// space, delta operations that need scratch are converted to full. See
+// crbug.com/238440.
+TEST_F(InplaceGeneratorTest, RunAsRootNoSparseAsTempTest) {
+ Graph graph(3);
+ const vector<Extent> kEmpty;
+ const string kFilename = "/foo";
+
+ // Make sure this sparse hole is not used as scratch.
+ GenVertex(&graph[0], kEmpty, VectOfExt(kSparseHole, 1), "", OP_REPLACE);
+
+ // Create a single-block cycle.
+ GenVertex(&graph[1], VectOfExt(0, 1), VectOfExt(1, 1), "", OP_BSDIFF);
+ graph[1].out_edges[2] = EdgeWithReadDep(VectOfExt(1, 1));
+ GenVertex(&graph[2], VectOfExt(1, 1), VectOfExt(0, 1), kFilename, OP_BSDIFF);
+ graph[2].out_edges[1] = EdgeWithReadDep(VectOfExt(0, 1));
+
+ graph_utils::DumpGraph(graph);
+
+ vector<Vertex::Index> final_order;
+
+ // Prepare the filesystem with the minimum required for this to work.
+ string temp_dir;
+ EXPECT_TRUE(utils::MakeTempDirectory("NoSparseAsTempTest.XXXXXX",
+ &temp_dir));
+ ScopedDirRemover temp_dir_remover(temp_dir);
+
+ chromeos::Blob temp_data(kBlockSize);
+ test_utils::FillWithData(&temp_data);
+ EXPECT_TRUE(test_utils::WriteFileVector(temp_dir + kFilename, temp_data));
+ ScopedPathUnlinker filename_unlinker(temp_dir + kFilename);
+
+ int fd = -1;
+ EXPECT_TRUE(utils::MakeTempFile("NoSparseAsTempTestData.XXXXXX",
+ nullptr,
+ &fd));
+ ScopedFdCloser fd_closer(&fd);
+ off_t data_file_size = 0;
+
+ EXPECT_TRUE(InplaceGenerator::ConvertGraphToDag(&graph,
+ temp_dir,
+ fd,
+ &data_file_size,
+ &final_order,
+ Vertex::kInvalidIndex));
+
+ ASSERT_EQ(4, graph.size());
+
+ // The second BSDIFF operation must have been converted to a full operation
+ // (due to insufficient scratch space).
+ EXPECT_TRUE(graph[2].op.type() == OP_REPLACE ||
+ graph[2].op.type() == OP_REPLACE_BZ);
+
+ // The temporary node created for breaking the cycle must have been marked as
+ // invalid.
+ EXPECT_FALSE(graph[3].valid);
+}
+
+// Test that sparse holes are not used as scratch. More specifically, test that
+// if scratch space comes only from full operations writing to real blocks as
+// well as sparse holes, only the real blocks are utilized. See
+// crbug.com/238440.
+TEST_F(InplaceGeneratorTest, NoSparseAsTempTest) {
+ Graph graph;
+ vector<Block> blocks(4);
+
+ // Create nodes in |graph|.
+ {
+ graph.resize(graph.size() + 1);
+ graph.back().op.set_type(
+ DeltaArchiveManifest_InstallOperation_Type_REPLACE);
+
+ // Write to a sparse hole -- basically a no-op to ensure sparse holes are
+ // not used as scratch.
+ vector<Extent> extents;
+ graph_utils::AppendBlockToExtents(&extents, kSparseHole);
+ DeltaDiffGenerator::StoreExtents(extents,
+ graph.back().op.mutable_dst_extents());
+ }
+ {
+ graph.resize(graph.size() + 1);
+ graph.back().op.set_type(OP_REPLACE);
+
+ // Scratch space: write to block 0 with sparse holes around.
+ vector<Extent> extents;
+ graph_utils::AppendBlockToExtents(&extents, kSparseHole);
+ graph_utils::AppendBlockToExtents(&extents, 0);
+ graph_utils::AppendBlockToExtents(&extents, kSparseHole);
+ DeltaDiffGenerator::StoreExtents(extents,
+ graph.back().op.mutable_dst_extents());
+ blocks[0].writer = graph.size() - 1;
+ }
+ {
+ graph.resize(graph.size() + 1);
+ graph.back().op.set_type(OP_REPLACE);
+
+ // Write to a sparse hole.
+ vector<Extent> extents;
+ graph_utils::AppendBlockToExtents(&extents, kSparseHole);
+ DeltaDiffGenerator::StoreExtents(extents,
+ graph.back().op.mutable_dst_extents());
+ }
+ // Create a two-node cycle between (2, sparse, sparse) and (1, sparse, 3).
+ {
+ graph.resize(graph.size() + 1);
+ graph.back().op.set_type(OP_MOVE);
+ // Read from (2, sparse, sparse).
+ vector<Extent> extents;
+ graph_utils::AppendBlockToExtents(&extents, 2);
+ graph_utils::AppendBlockToExtents(&extents, kSparseHole);
+ graph_utils::AppendBlockToExtents(&extents, kSparseHole);
+ DeltaDiffGenerator::StoreExtents(extents,
+ graph.back().op.mutable_src_extents());
+ blocks[2].reader = graph.size() - 1;
+
+ // Write to (1, sparse, 3).
+ extents.clear();
+ graph_utils::AppendBlockToExtents(&extents, 1);
+ graph_utils::AppendBlockToExtents(&extents, kSparseHole);
+ graph_utils::AppendBlockToExtents(&extents, 3);
+ DeltaDiffGenerator::StoreExtents(extents,
+ graph.back().op.mutable_dst_extents());
+ blocks[1].writer = graph.size() - 1;
+ blocks[3].writer = graph.size() - 1;
+ }
+ {
+ graph.resize(graph.size() + 1);
+ graph.back().op.set_type(OP_MOVE);
+ // Read from (1, sparse, 3).
+ vector<Extent> extents;
+ graph_utils::AppendBlockToExtents(&extents, 1);
+ graph_utils::AppendBlockToExtents(&extents, kSparseHole);
+ graph_utils::AppendBlockToExtents(&extents, 3);
+ DeltaDiffGenerator::StoreExtents(extents,
+ graph.back().op.mutable_src_extents());
+ blocks[1].reader = graph.size() - 1;
+ blocks[3].reader = graph.size() - 1;
+
+ // Write to (2, sparse, sparse).
+ extents.clear();
+ graph_utils::AppendBlockToExtents(&extents, 2);
+ graph_utils::AppendBlockToExtents(&extents, kSparseHole);
+ graph_utils::AppendBlockToExtents(&extents, kSparseHole);
+ DeltaDiffGenerator::StoreExtents(extents,
+ graph.back().op.mutable_dst_extents());
+ blocks[2].writer = graph.size() - 1;
+ }
+
+ graph_utils::DumpGraph(graph);
+
+ // Create edges
+ InplaceGenerator::CreateEdges(&graph, blocks);
+
+ graph_utils::DumpGraph(graph);
+
+ vector<Vertex::Index> final_order;
+ off_t data_file_size = 0;
+ EXPECT_TRUE(InplaceGenerator::ConvertGraphToDag(&graph,
+ "/non/existent/dir",
+ -1,
+ &data_file_size,
+ &final_order,
+ Vertex::kInvalidIndex));
+
+ // Check for a single temporary node writing to scratch.
+ ASSERT_EQ(6, graph.size());
+ EXPECT_EQ(DeltaArchiveManifest_InstallOperation_Type_MOVE,
+ graph[5].op.type());
+ EXPECT_EQ(1, graph[5].op.src_extents_size());
+ ASSERT_EQ(1, graph[5].op.dst_extents_size());
+ EXPECT_EQ(0, graph[5].op.dst_extents(0).start_block());
+ EXPECT_EQ(1, graph[5].op.dst_extents(0).num_blocks());
+
+ // Make sure the cycle nodes still read from and write to sparse holes.
+ for (int i = 3; i < 5; i++) {
+ ASSERT_GE(graph[i].op.src_extents_size(), 2);
+ EXPECT_EQ(kSparseHole, graph[i].op.src_extents(1).start_block());
+ ASSERT_GE(graph[i].op.dst_extents_size(), 2);
+ EXPECT_EQ(kSparseHole, graph[i].op.dst_extents(1).start_block());
+ }
+}
+
TEST_F(InplaceGeneratorTest, CreateScratchNodeTest) {
Vertex vertex;
InplaceGenerator::CreateScratchNode(12, 34, &vertex);
diff --git a/utils.cc b/utils.cc
index 31b25af..c076682 100644
--- a/utils.cc
+++ b/utils.cc
@@ -1691,32 +1691,6 @@
return false;
}
-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);
- ssize_t bytes_read = 0;
- int fd = open(path.c_str(), O_RDONLY);
- TEST_AND_RETURN_FALSE_ERRNO(fd >= 0);
- ScopedFdCloser fd_closer(&fd);
-
- 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);
- TEST_AND_RETURN_FALSE(utils::PReadAll(fd,
- &data[bytes_read],
- bytes,
- extent.start_block() * block_size,
- &bytes_read_this_iteration));
- TEST_AND_RETURN_FALSE(bytes_read_this_iteration == bytes);
- bytes_read += bytes_read_this_iteration;
- }
- TEST_AND_RETURN_FALSE(out_data_size == bytes_read);
- *out_data = data;
- return true;
-}
-
} // namespace utils
} // namespace chromeos_update_engine
diff --git a/utils.h b/utils.h
index 6d8b373..2374a94 100644
--- a/utils.h
+++ b/utils.h
@@ -28,7 +28,6 @@
#include "update_engine/constants.h"
#include "update_engine/file_descriptor.h"
#include "update_engine/metrics.h"
-#include "update_engine/update_metadata.pb.h"
namespace chromeos_update_engine {
@@ -469,14 +468,6 @@
// value and setting it, and |false| otherwise.
bool GetMinorVersion(base::FilePath path, uint32_t* minor_version);
-// This function reads the specified data in |extents| into |out_data|. The
-// 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,
- chromeos::Blob* out_data, ssize_t out_data_size,
- size_t block_size);
-
} // namespace utils