From 935420f968e969dbef5b73343b763d23c8d9328f Mon Sep 17 00:00:00 2001 From: David Anderson Date: Thu, 13 Jun 2019 16:20:39 -0700 Subject: [PATCH] liblp: Remove the slot-suffix requirement in MetadataBuilder. Normally MetadataBuilder will refuse to create non-suffixed partitions on A/B devices. There are some scenarios where this doesn't make sense, like when gsid needs to build metadata files for system_gsi/userdata_gsi. It also doesn't make sense for the "scratch" partition, so we added exceptions. It turns out that metadata created by gsid cannot be re-imported by MetadataBuilder, because there's no opportunity to set the "ignore" flag in MetadataBuilder's constructor. Rather than plumbing a flag through, I think we should just remove this error. It has too many exceptions already and it doesn't really protect against anything. The motivation was to avoid confusion in fastbootd on retrofit devices (where there are two super partitions), but it's a pretty minor concern. Bug: 134536978 Test: liblp_test gtest Change-Id: I4629a3c46070c35bcce1017096338e72aa234371 --- fs_mgr/liblp/builder.cpp | 11 +---------- fs_mgr/liblp/builder_test.cpp | 9 --------- fs_mgr/liblp/include/liblp/builder.h | 1 - 3 files changed, 1 insertion(+), 20 deletions(-) diff --git a/fs_mgr/liblp/builder.cpp b/fs_mgr/liblp/builder.cpp index 41c01da8a..25a042fb6 100644 --- a/fs_mgr/liblp/builder.cpp +++ b/fs_mgr/liblp/builder.cpp @@ -214,7 +214,7 @@ void MetadataBuilder::OverrideABForTesting(bool ab_device) { sABOverrideValue = ab_device; } -MetadataBuilder::MetadataBuilder() : auto_slot_suffixing_(false), ignore_slot_suffixing_(false) { +MetadataBuilder::MetadataBuilder() : auto_slot_suffixing_(false) { memset(&geometry_, 0, sizeof(geometry_)); geometry_.magic = LP_METADATA_GEOMETRY_MAGIC; geometry_.struct_size = sizeof(geometry_); @@ -443,11 +443,6 @@ Partition* MetadataBuilder::AddPartition(const std::string& name, const std::str LERROR << "Could not find partition group: " << group_name; return nullptr; } - if (IsABDevice() && !auto_slot_suffixing_ && name != "scratch" && !ignore_slot_suffixing_ && - GetPartitionSlotSuffix(name).empty()) { - LERROR << "Unsuffixed partition not allowed on A/B device: " << name; - return nullptr; - } partitions_.push_back(std::make_unique(name, group_name, attributes)); return partitions_.back().get(); } @@ -1049,10 +1044,6 @@ void MetadataBuilder::SetAutoSlotSuffixing() { auto_slot_suffixing_ = true; } -void MetadataBuilder::IgnoreSlotSuffixing() { - ignore_slot_suffixing_ = true; -} - bool MetadataBuilder::IsABDevice() const { if (sABOverrideSet) { return sABOverrideValue; diff --git a/fs_mgr/liblp/builder_test.cpp b/fs_mgr/liblp/builder_test.cpp index 45c3edede..34c68d4d4 100644 --- a/fs_mgr/liblp/builder_test.cpp +++ b/fs_mgr/liblp/builder_test.cpp @@ -772,15 +772,6 @@ TEST_F(BuilderTest, ImportPartitionsFail) { EXPECT_FALSE(builder->ImportPartitions(*exported.get(), {"system"})); } -TEST_F(BuilderTest, UnsuffixedPartitions) { - MetadataBuilder::OverrideABForTesting(true); - unique_ptr builder = MetadataBuilder::New(1024 * 1024, 1024, 2); - ASSERT_NE(builder, nullptr); - - ASSERT_EQ(builder->AddPartition("system", 0), nullptr); - ASSERT_NE(builder->AddPartition("system_a", 0), nullptr); -} - TEST_F(BuilderTest, ABExtents) { BlockDeviceInfo device_info("super", 10_GiB, 768 * 1024, 0, 4096); diff --git a/fs_mgr/liblp/include/liblp/builder.h b/fs_mgr/liblp/include/liblp/builder.h index c706f2a73..e70c55258 100644 --- a/fs_mgr/liblp/include/liblp/builder.h +++ b/fs_mgr/liblp/include/liblp/builder.h @@ -345,7 +345,6 @@ class MetadataBuilder { std::vector> groups_; std::vector block_devices_; bool auto_slot_suffixing_; - bool ignore_slot_suffixing_; }; // Read BlockDeviceInfo for a given block device. This always returns false