From 07c951fcf3af2d8abef2cf006cd70b1653c481a0 Mon Sep 17 00:00:00 2001 From: Tianjie Date: Mon, 29 Jun 2020 19:57:48 -0700 Subject: [PATCH] Add function to compare the partitions' extents in metadata For partial updates, the metadata for untouched dynamic partitions are just copied over to the target slot. So, verifying the extents of these partitions in the target metadata should be sufficient for correctness. And we don't need to read & hash the bytes on these partitions. Bug: 151088567 Test: unit tests pass Change-Id: I95836ee6f76d884c7a1f5537154ac7230563493a --- fs_mgr/liblp/builder.cpp | 80 +++++++++++++++++++++++++++- fs_mgr/liblp/builder_test.cpp | 52 +++++++++++++----- fs_mgr/liblp/include/liblp/builder.h | 29 +++++++++- 3 files changed, 145 insertions(+), 16 deletions(-) diff --git a/fs_mgr/liblp/builder.cpp b/fs_mgr/liblp/builder.cpp index c37d70e05..dff31b050 100644 --- a/fs_mgr/liblp/builder.cpp +++ b/fs_mgr/liblp/builder.cpp @@ -31,6 +31,22 @@ namespace android { namespace fs_mgr { +std::ostream& operator<<(std::ostream& os, const Extent& extent) { + switch (extent.GetExtentType()) { + case ExtentType::kZero: { + os << "type: Zero"; + break; + } + case ExtentType::kLinear: { + auto linear_extent = static_cast(&extent); + os << "type: Linear, physical sectors: " << linear_extent->physical_sector() + << ", end sectors: " << linear_extent->end_sector(); + break; + } + } + return os; +} + bool LinearExtent::AddTo(LpMetadata* out) const { if (device_index_ >= out->block_devices.size()) { LERROR << "Extent references unknown block device."; @@ -41,6 +57,17 @@ bool LinearExtent::AddTo(LpMetadata* out) const { return true; } +bool LinearExtent::operator==(const android::fs_mgr::Extent& other) const { + if (other.GetExtentType() != ExtentType::kLinear) { + return false; + } + + auto other_ptr = static_cast(&other); + return num_sectors_ == other_ptr->num_sectors_ && + physical_sector_ == other_ptr->physical_sector_ && + device_index_ == other_ptr->device_index_; +} + bool LinearExtent::OverlapsWith(const LinearExtent& other) const { if (device_index_ != other.device_index()) { return false; @@ -64,6 +91,10 @@ bool ZeroExtent::AddTo(LpMetadata* out) const { return true; } +bool ZeroExtent::operator==(const android::fs_mgr::Extent& other) const { + return other.GetExtentType() == ExtentType::kZero && num_sectors_ == other.num_sectors(); +} + Partition::Partition(std::string_view name, std::string_view group_name, uint32_t attributes) : name_(name), group_name_(group_name), attributes_(attributes), size_(0) {} @@ -511,7 +542,7 @@ Partition* MetadataBuilder::AddPartition(std::string_view name, std::string_view return partitions_.back().get(); } -Partition* MetadataBuilder::FindPartition(std::string_view name) { +Partition* MetadataBuilder::FindPartition(std::string_view name) const { for (const auto& partition : partitions_) { if (partition->name() == name) { return partition.get(); @@ -520,7 +551,7 @@ Partition* MetadataBuilder::FindPartition(std::string_view name) { return nullptr; } -PartitionGroup* MetadataBuilder::FindGroup(std::string_view group_name) { +PartitionGroup* MetadataBuilder::FindGroup(std::string_view group_name) const { for (const auto& group : groups_) { if (group->name() == group_name) { return group.get(); @@ -1263,5 +1294,50 @@ uint64_t MetadataBuilder::logical_block_size() const { return geometry_.logical_block_size; } +bool MetadataBuilder::VerifyExtentsAgainstSourceMetadata( + const MetadataBuilder& source_metadata, uint32_t source_slot_number, + const MetadataBuilder& target_metadata, uint32_t target_slot_number, + const std::vector& partitions) { + for (const auto& base_name : partitions) { + // Find the partition in metadata with the slot suffix. + auto target_partition_name = base_name + SlotSuffixForSlotNumber(target_slot_number); + const auto target_partition = target_metadata.FindPartition(target_partition_name); + if (!target_partition) { + LERROR << "Failed to find partition " << target_partition_name << " in metadata slot " + << target_slot_number; + return false; + } + + auto source_partition_name = base_name + SlotSuffixForSlotNumber(source_slot_number); + const auto source_partition = source_metadata.FindPartition(source_partition_name); + if (!source_partition) { + LERROR << "Failed to find partition " << source_partition << " in metadata slot " + << source_slot_number; + return false; + } + + // We expect the partitions in the target metadata to have the identical extents as the + // one in the source metadata. Because they are copied in NewForUpdate. + if (target_partition->extents().size() != source_partition->extents().size()) { + LERROR << "Extents count mismatch for partition " << base_name << " target slot has " + << target_partition->extents().size() << ", source slot has " + << source_partition->extents().size(); + return false; + } + + for (size_t i = 0; i < target_partition->extents().size(); i++) { + const auto& src_extent = *source_partition->extents()[i]; + const auto& tgt_extent = *target_partition->extents()[i]; + if (tgt_extent != src_extent) { + LERROR << "Extents " << i << " is different for partition " << base_name; + LERROR << "tgt extent " << tgt_extent << "; src extent " << src_extent; + return false; + } + } + } + + return true; +} + } // namespace fs_mgr } // namespace android diff --git a/fs_mgr/liblp/builder_test.cpp b/fs_mgr/liblp/builder_test.cpp index 1a3250ae5..a21e09e4f 100644 --- a/fs_mgr/liblp/builder_test.cpp +++ b/fs_mgr/liblp/builder_test.cpp @@ -947,9 +947,10 @@ static Interval ToInterval(const std::unique_ptr& extent) { } static void AddPartition(const std::unique_ptr& builder, - const std::string& partition_name, uint64_t num_sectors, - uint64_t start_sector, std::vector* intervals) { - Partition* p = builder->AddPartition(partition_name, "group", 0); + const std::string& partition_name, const std::string& group_name, + uint64_t num_sectors, uint64_t start_sector, + std::vector* intervals = nullptr) { + Partition* p = builder->AddPartition(partition_name, group_name, 0); ASSERT_NE(p, nullptr); ASSERT_TRUE(builder->AddLinearExtent(p, "super", num_sectors, start_sector)); ASSERT_EQ(p->extents().size(), 1); @@ -977,17 +978,17 @@ TEST_F(BuilderTest, CollidedExtents) { ASSERT_TRUE(builder->AddGroup("group", 0)); std::vector old_intervals; - AddPartition(builder, "system", 10229008, 2048, &old_intervals); - AddPartition(builder, "test_a", 648, 12709888, &old_intervals); - AddPartition(builder, "test_b", 625184, 12711936, &old_intervals); - AddPartition(builder, "test_c", 130912, 13338624, &old_intervals); - AddPartition(builder, "test_d", 888, 13469696, &old_intervals); - AddPartition(builder, "test_e", 888, 13471744, &old_intervals); - AddPartition(builder, "test_f", 888, 13475840, &old_intervals); - AddPartition(builder, "test_g", 888, 13477888, &old_intervals); + AddPartition(builder, "system", "group", 10229008, 2048, &old_intervals); + AddPartition(builder, "test_a", "group", 648, 12709888, &old_intervals); + AddPartition(builder, "test_b", "group", 625184, 12711936, &old_intervals); + AddPartition(builder, "test_c", "group", 130912, 13338624, &old_intervals); + AddPartition(builder, "test_d", "group", 888, 13469696, &old_intervals); + AddPartition(builder, "test_e", "group", 888, 13471744, &old_intervals); + AddPartition(builder, "test_f", "group", 888, 13475840, &old_intervals); + AddPartition(builder, "test_g", "group", 888, 13477888, &old_intervals); // Don't track the first vendor interval, since it will get extended. - AddPartition(builder, "vendor", 2477920, 10231808, nullptr); + AddPartition(builder, "vendor", "group", 2477920, 10231808, nullptr); std::vector new_intervals; @@ -1066,3 +1067,30 @@ TEST_F(BuilderTest, ResizeOverflow) { ASSERT_NE(p, nullptr); ASSERT_FALSE(builder->ResizePartition(p, 18446744073709551615ULL)); } + +TEST_F(BuilderTest, VerifyExtent) { + auto source_builder = MetadataBuilder::New(4096 * 50, 40960, 2); + ASSERT_NE(source_builder, nullptr); + ASSERT_TRUE(source_builder->AddGroup("test_group_a", 40960)); + ASSERT_TRUE(source_builder->AddGroup("test_group_b", 40960)); + AddPartition(source_builder, "system_a", "test_group_a", 8192, 2048); + AddPartition(source_builder, "vendor_a", "test_group_a", 10240, 10240); + AddPartition(source_builder, "system_b", "test_group_b", 8192, 20480); + + auto target_builder = MetadataBuilder::New(4096 * 50, 40960, 2); + ASSERT_NE(target_builder, nullptr); + ASSERT_TRUE(target_builder->AddGroup("test_group_b", 40960)); + AddPartition(target_builder, "system_b", "test_group_b", 8192, 2048); + AddPartition(target_builder, "vendor_b", "test_group_b", 10240, 10240); + + ASSERT_TRUE(MetadataBuilder::VerifyExtentsAgainstSourceMetadata( + *source_builder, 0, *target_builder, 1, std::vector{"system", "vendor"})); + + target_builder->RemovePartition("vendor_b"); + ASSERT_FALSE(target_builder->VerifyExtentsAgainstSourceMetadata( + *source_builder, 0, *target_builder, 1, std::vector{"vendor"})); + + AddPartition(target_builder, "vendor_b", "test_group_b", 1000, 10240); + ASSERT_FALSE(target_builder->VerifyExtentsAgainstSourceMetadata( + *source_builder, 0, *target_builder, 1, std::vector{"vendor"})); +} diff --git a/fs_mgr/liblp/include/liblp/builder.h b/fs_mgr/liblp/include/liblp/builder.h index 89a47b112..293e444bb 100644 --- a/fs_mgr/liblp/include/liblp/builder.h +++ b/fs_mgr/liblp/include/liblp/builder.h @@ -42,6 +42,11 @@ static constexpr uint32_t kDefaultBlockSize = 4096; // Name of the default group in a metadata. static constexpr std::string_view kDefaultGroup = "default"; +enum class ExtentType { + kZero, + kLinear, +}; + // Abstraction around dm-targets that can be encoded into logical partition tables. class Extent { public: @@ -50,6 +55,10 @@ class Extent { virtual bool AddTo(LpMetadata* out) const = 0; virtual LinearExtent* AsLinearExtent() { return nullptr; } + virtual ExtentType GetExtentType() const = 0; + + virtual bool operator==(const Extent& other) const = 0; + virtual bool operator!=(const Extent& other) const { return !(*this == other); } uint64_t num_sectors() const { return num_sectors_; } void set_num_sectors(uint64_t num_sectors) { num_sectors_ = num_sectors; } @@ -58,6 +67,8 @@ class Extent { uint64_t num_sectors_; }; +std::ostream& operator<<(std::ostream& os, const Extent& extent); + // This corresponds to a dm-linear target. class LinearExtent final : public Extent { public: @@ -66,6 +77,9 @@ class LinearExtent final : public Extent { bool AddTo(LpMetadata* metadata) const override; LinearExtent* AsLinearExtent() override { return this; } + ExtentType GetExtentType() const override { return ExtentType::kLinear; } + + bool operator==(const Extent& other) const override; uint64_t physical_sector() const { return physical_sector_; } uint64_t end_sector() const { return physical_sector_ + num_sectors_; } @@ -87,6 +101,9 @@ class ZeroExtent final : public Extent { explicit ZeroExtent(uint64_t num_sectors) : Extent(num_sectors) {} bool AddTo(LpMetadata* out) const override; + ExtentType GetExtentType() const override { return ExtentType::kZero; } + + bool operator==(const Extent& other) const override; }; class PartitionGroup final { @@ -241,6 +258,14 @@ class MetadataBuilder { return New(device_info, metadata_max_size, metadata_slot_count); } + // Verifies that the given partitions in the metadata have the same extents as the source + // metadata. + static bool VerifyExtentsAgainstSourceMetadata(const MetadataBuilder& source_metadata, + uint32_t source_slot_number, + const MetadataBuilder& target_metadata, + uint32_t target_slot_number, + const std::vector& partitions); + // Define a new partition group. By default there is one group called // "default", with an unrestricted size. A non-zero size will restrict the // total space used by all partitions in the group. @@ -265,10 +290,10 @@ class MetadataBuilder { void RemovePartition(std::string_view name); // Find a partition by name. If no partition is found, nullptr is returned. - Partition* FindPartition(std::string_view name); + Partition* FindPartition(std::string_view name) const; // Find a group by name. If no group is found, nullptr is returned. - PartitionGroup* FindGroup(std::string_view name); + PartitionGroup* FindGroup(std::string_view name) const; // Add a predetermined extent to a partition. bool AddLinearExtent(Partition* partition, const std::string& block_device,