Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b8e17119 authored by David Anderson's avatar David Anderson
Browse files

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 dccfdca1.

Bug: 125113249
Test: liblp_test gtest
Change-Id: Ibfd66734b63265e9fbb2ccf6b670c168c36190bc
parent 9a00f596

fs_mgr/liblp/builder.cpp

+2 −11
Original line number Diff line number Diff line
@@ -613,23 +613,14 @@ bool MetadataBuilder::GrowPartition(Partition* partition, uint64_t aligned_size) 
        }



        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();

fs_mgr/liblp/builder_test.cpp

+13 −37
Original line number Diff line number Diff line
@@ -209,8 +209,8 @@ TEST_F(BuilderTest, InternalPartitionAlignment) { 
        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 @@ TEST_F(BuilderTest, InternalPartitionAlignment) { 
    // 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 @@ TEST_F(BuilderTest, MultipleBlockDevices) { 
    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 @@ TEST_F(BuilderTest, ABExtents) { 
    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) {