Always filter out all zero blocks from old_file_extents.
Even though we are allowed to read the old blocks multiple times for
non-inplace update, we should still skip those blocks that are all
zero.
Bug: 120465293
Test: no zero blocks in src_extents in delta payload
Change-Id: I48bdc06680316d2c9b373e9d929fdff1ad797a55
(cherry picked from commit ba0fc92e65d5cb7580437a5538c8c4be083503b5)
(cherry picked from commit ce59e761da8e8dd0fe07f5cb21915768ce430cce)
diff --git a/payload_generator/delta_diff_utils.cc b/payload_generator/delta_diff_utils.cc
index 16a8ad5..d3629ff 100644
--- a/payload_generator/delta_diff_utils.cc
+++ b/payload_generator/delta_diff_utils.cc
@@ -387,17 +387,18 @@
new_visited_blocks.AddExtent(new_part.verity.fec_extent);
}
- TEST_AND_RETURN_FALSE(DeltaMovedAndZeroBlocks(
- aops,
- old_part.path,
- new_part.path,
- old_part.size / kBlockSize,
- new_part.size / kBlockSize,
- soft_chunk_blocks,
- version,
- blob_file,
- &old_visited_blocks,
- &new_visited_blocks));
+ ExtentRanges old_zero_blocks;
+ TEST_AND_RETURN_FALSE(DeltaMovedAndZeroBlocks(aops,
+ old_part.path,
+ new_part.path,
+ old_part.size / kBlockSize,
+ new_part.size / kBlockSize,
+ soft_chunk_blocks,
+ version,
+ blob_file,
+ &old_visited_blocks,
+ &new_visited_blocks,
+ &old_zero_blocks));
bool puffdiff_allowed = version.OperationAllowed(InstallOperation::PUFFDIFF);
map<string, FilesystemInterface::File> old_files_map;
@@ -450,7 +451,7 @@
old_file_extents =
FilterExtentRanges(old_file.extents, old_visited_blocks);
else
- old_file_extents = old_file.extents;
+ old_file_extents = FilterExtentRanges(old_file.extents, old_zero_blocks);
old_visited_blocks.AddExtents(old_file_extents);
file_delta_processors.emplace_back(old_part.path,
@@ -527,7 +528,8 @@
const PayloadVersion& version,
BlobFileWriter* blob_file,
ExtentRanges* old_visited_blocks,
- ExtentRanges* new_visited_blocks) {
+ ExtentRanges* new_visited_blocks,
+ ExtentRanges* old_zero_blocks) {
vector<BlockMapping::BlockId> old_block_ids;
vector<BlockMapping::BlockId> new_block_ids;
TEST_AND_RETURN_FALSE(MapPartitionBlocks(old_part,
@@ -567,8 +569,9 @@
// importantly, these could sometimes be blocks discarded in the SSD which
// would read non-zero values.
if (old_block_ids[block] == 0)
- old_visited_blocks->AddBlock(block);
+ old_zero_blocks->AddBlock(block);
}
+ old_visited_blocks->AddRanges(*old_zero_blocks);
// The collection of blocks in the new partition with just zeros. This is a
// common case for free-space that's also problematic for bsdiff, so we want
diff --git a/payload_generator/delta_diff_utils.h b/payload_generator/delta_diff_utils.h
index c1f62fc..2306572 100644
--- a/payload_generator/delta_diff_utils.h
+++ b/payload_generator/delta_diff_utils.h
@@ -70,7 +70,8 @@
const PayloadVersion& version,
BlobFileWriter* blob_file,
ExtentRanges* old_visited_blocks,
- ExtentRanges* new_visited_blocks);
+ ExtentRanges* new_visited_blocks,
+ ExtentRanges* old_zero_blocks);
// For a given file |name| append operations to |aops| to produce it in the
// |new_part|. The file will be split in chunks of |chunk_blocks| blocks each
diff --git a/payload_generator/delta_diff_utils_unittest.cc b/payload_generator/delta_diff_utils_unittest.cc
index f6677f1..63e7506 100644
--- a/payload_generator/delta_diff_utils_unittest.cc
+++ b/payload_generator/delta_diff_utils_unittest.cc
@@ -131,6 +131,7 @@
uint32_t minor_version) {
BlobFileWriter blob_file(blob_fd_, &blob_size_);
PayloadVersion version(kChromeOSMajorPayloadVersion, minor_version);
+ ExtentRanges old_zero_blocks;
return diff_utils::DeltaMovedAndZeroBlocks(&aops_,
old_part_.path,
new_part_.path,
@@ -140,7 +141,8 @@
version,
&blob_file,
&old_visited_blocks_,
- &new_visited_blocks_);
+ &new_visited_blocks_,
+ &old_zero_blocks);
}
// Old and new temporary partitions used in the tests. These are initialized