Always write SOURCE_COPY blocks in reverse order
Test: treehugger
Bug: 174112589
Change-Id: If95893569ab41d1806f266aa269722b403a50fa4
diff --git a/Android.bp b/Android.bp
index e192a63..020fd6e 100644
--- a/Android.bp
+++ b/Android.bp
@@ -82,6 +82,21 @@
// libcow_operation_convert (type: library)
// ========================================================
+cc_library_static {
+ name: "libpayload_extent_utils",
+ defaults: [
+ "ue_defaults",
+ ],
+ host_supported: true,
+ recovery_available: true,
+ srcs: [
+ "payload_generator/extent_utils.cc",
+ ],
+ static_libs: [
+ "update_metadata-protos",
+ ],
+}
+
cc_library {
name: "libcow_operation_convert",
host_supported: true,
@@ -97,6 +112,7 @@
"libsnapshot_cow",
"update_metadata-protos",
"libpayload_extent_ranges",
+ "libpayload_extent_utils",
"libbrotli",
"libz",
],
@@ -148,6 +164,7 @@
"libbrotli",
"libz",
"libpayload_extent_ranges",
+ "libpayload_extent_utils",
"libcow_operation_convert",
],
shared_libs: [
@@ -497,6 +514,7 @@
"libpuffdiff",
"libverity_tree",
"update_metadata-protos",
+ "libpayload_extent_utils",
],
shared_libs: [
"libbase",
@@ -542,7 +560,6 @@
"payload_generator/delta_diff_utils.cc",
"payload_generator/ext2_filesystem.cc",
"payload_generator/extent_ranges.cc",
- "payload_generator/extent_utils.cc",
"payload_generator/full_update_generator.cc",
"payload_generator/mapfile_filesystem.cc",
"payload_generator/merge_sequence_generator.cc",
diff --git a/common/cow_operation_convert.cc b/common/cow_operation_convert.cc
index 6b64a9c..4dc73a7 100644
--- a/common/cow_operation_convert.cc
+++ b/common/cow_operation_convert.cc
@@ -43,29 +43,14 @@
merge_extents.AddExtent(merge_op.dst_extent());
const auto& src_extent = merge_op.src_extent();
const auto& dst_extent = merge_op.dst_extent();
- // Add blocks in reverse order to avoid merge conflicts on self-overlapping
- // Ops.
- // For example: SOURCE_COPY [20 - 30] -> [25 - 35] If blocks are added in
- // forward order, then 20->25 is performed first, destroying block 25, which
- // is neede by a later operation.
- if (src_extent.start_block() < dst_extent.start_block()) {
- for (uint64_t i = src_extent.num_blocks(); i > 0; i--) {
- auto src_block = src_extent.start_block() + i - 1;
- auto dst_block = dst_extent.start_block() + i - 1;
- CHECK(!modified_extents.ContainsBlock(src_block))
- << "block " << src_block << " is modified by previous CowCopy";
- converted.push_back({CowOperation::CowCopy, src_block, dst_block});
- modified_extents.AddBlock(dst_block);
- }
- } else {
- for (uint64_t i = 0; i < src_extent.num_blocks(); i++) {
- auto src_block = src_extent.start_block() + i;
- auto dst_block = dst_extent.start_block() + i;
- CHECK(!modified_extents.ContainsBlock(src_block))
- << "block " << src_block << " is modified by previous CowCopy";
- converted.push_back({CowOperation::CowCopy, src_block, dst_block});
- modified_extents.AddBlock(dst_block);
- }
+ // Add blocks in reverse order, because snapused specifically prefers this
+ // ordering. Since we already eliminated all self-overlapping SOURCE_COPY
+ // during delta generation, this should be safe to do.
+ for (uint64_t i = src_extent.num_blocks(); i > 0; i--) {
+ auto src_block = src_extent.start_block() + i - 1;
+ auto dst_block = dst_extent.start_block() + i - 1;
+ converted.push_back({CowOperation::CowCopy, src_block, dst_block});
+ modified_extents.AddBlock(dst_block);
}
}
// COW_REPLACE are added after COW_COPY, because replace might modify blocks
diff --git a/payload_generator/extent_utils.cc b/payload_generator/extent_utils.cc
index c0c7643..2efef12 100644
--- a/payload_generator/extent_utils.cc
+++ b/payload_generator/extent_utils.cc
@@ -155,4 +155,10 @@
return a.start_block() == b.start_block() && a.num_blocks() == b.num_blocks();
}
+std::ostream& operator<<(std::ostream& out, const Extent& extent) {
+ out << "[" << extent.start_block() << " - "
+ << extent.start_block() + extent.num_blocks() - 1 << "]";
+ return out;
+}
+
} // namespace chromeos_update_engine
diff --git a/payload_generator/extent_utils.h b/payload_generator/extent_utils.h
index f870b29..7aa614a 100644
--- a/payload_generator/extent_utils.h
+++ b/payload_generator/extent_utils.h
@@ -122,6 +122,8 @@
size_t block_offset_ = 0;
};
+std::ostream& operator<<(std::ostream& out, const Extent& extent);
+
} // namespace chromeos_update_engine
#endif // UPDATE_ENGINE_PAYLOAD_GENERATOR_EXTENT_UTILS_H_
diff --git a/payload_generator/merge_sequence_generator_unittest.cc b/payload_generator/merge_sequence_generator_unittest.cc
index 666adbe..b8507ed 100644
--- a/payload_generator/merge_sequence_generator_unittest.cc
+++ b/payload_generator/merge_sequence_generator_unittest.cc
@@ -208,12 +208,6 @@
GenerateSequence(transfers, expected);
}
-std::ostream& operator<<(std::ostream& out, const Extent& extent) {
- out << "[" << extent.start_block() << " - "
- << extent.start_block() + extent.num_blocks() - 1 << "]";
- return out;
-}
-
void ValidateSplitSequence(const Extent& src_extent, const Extent& dst_extent) {
std::vector<CowMergeOperation> sequence;
SplitSelfOverlapping(src_extent, dst_extent, &sequence);