Revert "liblp: Reclaim wasted space from unaligned partitions."
When this patch landed, it was thought that over-allocating partitions
had no undesired side effects. However there are cases where it is now
causing problems. When flashing a GSI for VTS, the vbmeta is appended to
the end of the partition. If libfs_avb cannot find this vbmeta by
looking at the end of the partition, verification will fail. Similar use
cases with verifying other dynamic partitions are expected to be
problematic, so for now we revert to block-size alignment.
This reverts commit dccfdca1e19d27bac2c8f20df431fd36d0f87d53.
Bug: 125113249
Test: liblp_test gtest
Change-Id: Ibfd66734b63265e9fbb2ccf6b670c168c36190bc
diff --git a/fs_mgr/liblp/builder.cpp b/fs_mgr/liblp/builder.cpp
index ebd6997..5e1bb23 100644
--- a/fs_mgr/liblp/builder.cpp
+++ b/fs_mgr/liblp/builder.cpp
@@ -613,23 +613,14 @@
}
uint64_t sectors = std::min(sectors_needed, region.length());
- if (sectors < region.length()) {
- const auto& block_device = block_devices_[region.device_index];
- if (block_device.alignment) {
- const uint64_t alignment = block_device.alignment / LP_SECTOR_SIZE;
- sectors = AlignTo(sectors, alignment);
- sectors = std::min(sectors, region.length());
- }
- }
CHECK(sectors % sectors_per_block == 0);
auto extent = std::make_unique<LinearExtent>(sectors, region.device_index, region.start);
new_extents.push_back(std::move(extent));
- if (sectors >= sectors_needed) {
- sectors_needed = 0;
+ sectors_needed -= sectors;
+ if (!sectors_needed) {
break;
}
- sectors_needed -= sectors;
}
if (sectors_needed) {
LERROR << "Not enough free space to expand partition: " << partition->name();
diff --git a/fs_mgr/liblp/builder_test.cpp b/fs_mgr/liblp/builder_test.cpp
index 81305b3..88c3035 100644
--- a/fs_mgr/liblp/builder_test.cpp
+++ b/fs_mgr/liblp/builder_test.cpp
@@ -209,8 +209,8 @@
ASSERT_TRUE(builder->ResizePartition(a, a->size() + 4096));
ASSERT_TRUE(builder->ResizePartition(b, b->size() + 4096));
}
- EXPECT_EQ(a->size(), 7864320);
- EXPECT_EQ(b->size(), 7864320);
+ EXPECT_EQ(a->size(), 40960);
+ EXPECT_EQ(b->size(), 40960);
unique_ptr<LpMetadata> exported = builder->Export();
ASSERT_NE(exported, nullptr);
@@ -218,7 +218,7 @@
// Check that each starting sector is aligned.
for (const auto& extent : exported->extents) {
ASSERT_EQ(extent.target_type, LP_TARGET_TYPE_LINEAR);
- EXPECT_EQ(extent.num_sectors, 1536);
+ EXPECT_EQ(extent.num_sectors, 8);
uint64_t lba = extent.target_data * LP_SECTOR_SIZE;
uint64_t aligned_lba = AlignTo(lba, device_info.alignment, device_info.alignment_offset);
@@ -698,7 +698,7 @@
EXPECT_EQ(metadata->extents[1].target_type, LP_TARGET_TYPE_LINEAR);
EXPECT_EQ(metadata->extents[1].target_data, 1472);
EXPECT_EQ(metadata->extents[1].target_source, 1);
- EXPECT_EQ(metadata->extents[2].num_sectors, 129600);
+ EXPECT_EQ(metadata->extents[2].num_sectors, 129088);
EXPECT_EQ(metadata->extents[2].target_type, LP_TARGET_TYPE_LINEAR);
EXPECT_EQ(metadata->extents[2].target_data, 1472);
EXPECT_EQ(metadata->extents[2].target_source, 2);
@@ -797,43 +797,19 @@
EXPECT_EQ(system_a->extents().size(), static_cast<size_t>(1));
EXPECT_EQ(system_b->extents().size(), static_cast<size_t>(1));
ASSERT_TRUE(builder->ResizePartition(system_b, 6_GiB));
- EXPECT_EQ(system_b->extents().size(), static_cast<size_t>(2));
+ EXPECT_EQ(system_b->extents().size(), static_cast<size_t>(3));
unique_ptr<LpMetadata> exported = builder->Export();
ASSERT_NE(exported, nullptr);
- ASSERT_EQ(exported->extents.size(), static_cast<size_t>(3));
+ ASSERT_EQ(exported->extents.size(), static_cast<size_t>(4));
EXPECT_EQ(exported->extents[0].target_data, 10487808);
- EXPECT_EQ(exported->extents[0].num_sectors, 10483712);
- EXPECT_EQ(exported->extents[1].target_data, 6292992);
- EXPECT_EQ(exported->extents[1].num_sectors, 2099712);
- EXPECT_EQ(exported->extents[2].target_data, 1536);
- EXPECT_EQ(exported->extents[2].num_sectors, 6291456);
-}
-
-TEST_F(BuilderTest, PartialExtents) {
- // super has a minimum extent size of 768KiB.
- BlockDeviceInfo device_info("super", 1_GiB, 768 * 1024, 0, 4096);
- auto builder = MetadataBuilder::New(device_info, 65536, 1);
- ASSERT_NE(builder, nullptr);
- Partition* system = builder->AddPartition("system", 0);
- ASSERT_NE(system, nullptr);
- Partition* vendor = builder->AddPartition("vendor", 0);
- ASSERT_NE(vendor, nullptr);
- ASSERT_TRUE(builder->ResizePartition(system, device_info.alignment + 4096));
- ASSERT_TRUE(builder->ResizePartition(vendor, device_info.alignment));
- ASSERT_EQ(system->size(), device_info.alignment * 2);
- ASSERT_EQ(vendor->size(), device_info.alignment);
-
- ASSERT_TRUE(builder->ResizePartition(system, device_info.alignment * 2));
- ASSERT_EQ(system->extents().size(), static_cast<size_t>(1));
-
- unique_ptr<LpMetadata> exported = builder->Export();
- ASSERT_NE(exported, nullptr);
- ASSERT_EQ(exported->extents.size(), static_cast<size_t>(2));
- EXPECT_EQ(exported->extents[0].target_data, 1536);
- EXPECT_EQ(exported->extents[0].num_sectors, 3072);
- EXPECT_EQ(exported->extents[1].target_data, 4608);
- EXPECT_EQ(exported->extents[1].num_sectors, 1536);
+ EXPECT_EQ(exported->extents[0].num_sectors, 4194304);
+ EXPECT_EQ(exported->extents[1].target_data, 14682624);
+ EXPECT_EQ(exported->extents[1].num_sectors, 6288896);
+ EXPECT_EQ(exported->extents[2].target_data, 6292992);
+ EXPECT_EQ(exported->extents[2].num_sectors, 2099712);
+ EXPECT_EQ(exported->extents[3].target_data, 1536);
+ EXPECT_EQ(exported->extents[3].num_sectors, 6291456);
}
TEST_F(BuilderTest, UpdateSuper) {