From db29503b4d4c5fa6144eed343c062b7c20aae649 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Wed, 10 Oct 2018 18:49:36 -0700 Subject: [PATCH] liblp: Remove last_logical_sector from LpMetadataGeometry. Now that backup metadata is stored at the start of the super partition, this field is no longer needed. In actuality, it was not needed even before then: both it and first_logical_sector exist for convenience, since they can be re-derived at any time given an LpMetadataGeometry. Bug: 116802789 Test: liblp_test gtest device with dynamic partitions flashes and boots Change-Id: I259a443097e689a0a9db7f822bbf1a52d40076dc --- fs_mgr/liblp/builder.cpp | 42 ++++++-------------- fs_mgr/liblp/builder_test.cpp | 5 --- fs_mgr/liblp/include/liblp/metadata_format.h | 8 +--- fs_mgr/liblp/io_test.cpp | 11 ++--- fs_mgr/liblp/utility_test.cpp | 1 - fs_mgr/liblp/writer.cpp | 20 +++++----- 6 files changed, 25 insertions(+), 62 deletions(-) diff --git a/fs_mgr/liblp/builder.cpp b/fs_mgr/liblp/builder.cpp index 743a3fec3..3d4b8263e 100644 --- a/fs_mgr/liblp/builder.cpp +++ b/fs_mgr/liblp/builder.cpp @@ -268,39 +268,19 @@ bool MetadataBuilder::Init(const BlockDeviceInfo& device_info, uint32_t metadata } // Compute the first free sector, factoring in alignment. - uint64_t free_area = + uint64_t free_area_start = AlignTo(total_reserved, device_info.alignment, device_info.alignment_offset); - uint64_t first_sector = free_area / LP_SECTOR_SIZE; + uint64_t first_sector = free_area_start / LP_SECTOR_SIZE; - // Compute the last free sector, which is inclusive. We subtract 1 to make - // sure that logical partitions won't overlap with the same sector as the - // backup metadata, which could happen if the block device was not aligned - // to LP_SECTOR_SIZE. - uint64_t last_sector = (device_info.size / LP_SECTOR_SIZE) - 1; - - // If this check fails, it means either (1) we did not have free space to - // allocate a single sector, or (2) we did, but the alignment was high - // enough to bump the first sector out of range. Either way, we cannot - // continue. - if (first_sector > last_sector) { - LERROR << "Not enough space to allocate any partition tables."; + // There must be one logical block of free space remaining (enough for one partition). + uint64_t minimum_disk_size = (first_sector * LP_SECTOR_SIZE) + device_info.logical_block_size; + if (device_info.size < minimum_disk_size) { + LERROR << "Device must be at least " << minimum_disk_size << " bytes, only has " + << device_info.size; return false; } - // Finally, the size of the allocatable space must be a multiple of the - // logical block size. If we have no more free space after this - // computation, then we abort. Note that the last sector is inclusive, - // so we have to account for that. - uint64_t num_free_sectors = last_sector - first_sector + 1; - uint64_t sectors_per_block = device_info.logical_block_size / LP_SECTOR_SIZE; - if (num_free_sectors < sectors_per_block) { - LERROR << "Not enough space to allocate any partition tables."; - return false; - } - last_sector = first_sector + (num_free_sectors / sectors_per_block) * sectors_per_block - 1; - geometry_.first_logical_sector = first_sector; - geometry_.last_logical_sector = last_sector; geometry_.metadata_max_size = metadata_max_size; geometry_.metadata_slot_count = metadata_slot_count; geometry_.alignment = device_info.alignment; @@ -452,8 +432,10 @@ bool MetadataBuilder::GrowPartition(Partition* partition, uint64_t aligned_size) uint64_t last_free_extent_start = extents.empty() ? geometry_.first_logical_sector : extents.back().end; last_free_extent_start = AlignSector(last_free_extent_start); - if (last_free_extent_start <= geometry_.last_logical_sector) { - free_regions.emplace_back(last_free_extent_start, geometry_.last_logical_sector + 1); + + uint64_t last_sector = geometry_.block_device_size / LP_SECTOR_SIZE; + if (last_free_extent_start < last_sector) { + free_regions.emplace_back(last_free_extent_start, last_sector); } const uint64_t sectors_per_block = geometry_.logical_block_size / LP_SECTOR_SIZE; @@ -559,7 +541,7 @@ std::unique_ptr MetadataBuilder::Export() { } uint64_t MetadataBuilder::AllocatableSpace() const { - return (geometry_.last_logical_sector - geometry_.first_logical_sector + 1) * LP_SECTOR_SIZE; + return geometry_.block_device_size - (geometry_.first_logical_sector * LP_SECTOR_SIZE); } uint64_t MetadataBuilder::UsedSpace() const { diff --git a/fs_mgr/liblp/builder_test.cpp b/fs_mgr/liblp/builder_test.cpp index ffa7d3bac..eb65f89b6 100644 --- a/fs_mgr/liblp/builder_test.cpp +++ b/fs_mgr/liblp/builder_test.cpp @@ -127,7 +127,6 @@ TEST(liblp, InternalAlignment) { unique_ptr exported = builder->Export(); ASSERT_NE(exported, nullptr); EXPECT_EQ(exported->geometry.first_logical_sector, 1536); - EXPECT_EQ(exported->geometry.last_logical_sector, 2047); // Test a large alignment offset thrown in. device_info.alignment_offset = 753664; @@ -136,7 +135,6 @@ TEST(liblp, InternalAlignment) { exported = builder->Export(); ASSERT_NE(exported, nullptr); EXPECT_EQ(exported->geometry.first_logical_sector, 1472); - EXPECT_EQ(exported->geometry.last_logical_sector, 2047); // Alignment offset without alignment doesn't mean anything. device_info.alignment = 0; @@ -151,7 +149,6 @@ TEST(liblp, InternalAlignment) { exported = builder->Export(); ASSERT_NE(exported, nullptr); EXPECT_EQ(exported->geometry.first_logical_sector, 150); - EXPECT_EQ(exported->geometry.last_logical_sector, 2045); // Test a small alignment with no alignment offset. device_info.alignment = 11 * 1024; @@ -160,7 +157,6 @@ TEST(liblp, InternalAlignment) { exported = builder->Export(); ASSERT_NE(exported, nullptr); EXPECT_EQ(exported->geometry.first_logical_sector, 160); - EXPECT_EQ(exported->geometry.last_logical_sector, 2047); } TEST(liblp, InternalPartitionAlignment) { @@ -298,7 +294,6 @@ TEST(liblp, BuilderExport) { EXPECT_EQ(geometry.metadata_max_size, 1024); EXPECT_EQ(geometry.metadata_slot_count, 2); EXPECT_EQ(geometry.first_logical_sector, 24); - EXPECT_EQ(geometry.last_logical_sector, 2047); static const size_t kMetadataSpace = ((kMetadataSize * kMetadataSlots) + LP_METADATA_GEOMETRY_SIZE) * 2; diff --git a/fs_mgr/liblp/include/liblp/metadata_format.h b/fs_mgr/liblp/include/liblp/metadata_format.h index a4ff1a732..711ff95a5 100644 --- a/fs_mgr/liblp/include/liblp/metadata_format.h +++ b/fs_mgr/liblp/include/liblp/metadata_format.h @@ -38,7 +38,7 @@ extern "C" { #define LP_METADATA_HEADER_MAGIC 0x414C5030 /* Current metadata version. */ -#define LP_METADATA_MAJOR_VERSION 4 +#define LP_METADATA_MAJOR_VERSION 5 #define LP_METADATA_MINOR_VERSION 0 /* Attributes for the LpMetadataPartition::attributes field. @@ -104,12 +104,6 @@ typedef struct LpMetadataGeometry { */ uint64_t first_logical_sector; - /* 56: Last usable sector, inclusive, for allocating logical partitions. - * At the end of this sector will follow backup metadata slots and the - * backup geometry block at the very end. - */ - uint64_t last_logical_sector; - /* 64: Alignment for defining partitions or partition extents. For example, * an alignment of 1MiB will require that all partitions have a size evenly * divisible by 1MiB, and that the smallest unit the partition can grow by diff --git a/fs_mgr/liblp/io_test.cpp b/fs_mgr/liblp/io_test.cpp index 220d651d7..92696f528 100644 --- a/fs_mgr/liblp/io_test.cpp +++ b/fs_mgr/liblp/io_test.cpp @@ -161,7 +161,6 @@ TEST(liblp, FlashAndReadback) { EXPECT_EQ(exported->geometry.metadata_max_size, imported->geometry.metadata_max_size); EXPECT_EQ(exported->geometry.metadata_slot_count, imported->geometry.metadata_slot_count); EXPECT_EQ(exported->geometry.first_logical_sector, imported->geometry.first_logical_sector); - EXPECT_EQ(exported->geometry.last_logical_sector, imported->geometry.last_logical_sector); EXPECT_EQ(exported->header.major_version, imported->header.major_version); EXPECT_EQ(exported->header.minor_version, imported->header.minor_version); EXPECT_EQ(exported->header.header_size, imported->header.header_size); @@ -207,13 +206,14 @@ TEST(liblp, UpdateAnyMetadataSlot) { ASSERT_EQ(imported->partitions.size(), 1); EXPECT_EQ(GetPartitionName(imported->partitions[0]), "vendor"); + uint64_t last_sector = imported->geometry.block_device_size / LP_SECTOR_SIZE; + // Verify that we didn't overwrite anything in the logical paritition area. // We expect the disk to be filled with 0xcc on creation so we can read // this back and compare it. char expected[LP_SECTOR_SIZE]; memset(expected, 0xcc, sizeof(expected)); - for (uint64_t i = imported->geometry.first_logical_sector; - i <= imported->geometry.last_logical_sector; i++) { + for (uint64_t i = imported->geometry.first_logical_sector; i < last_sector; i++) { char buffer[LP_SECTOR_SIZE]; ASSERT_GE(lseek(fd, i * LP_SECTOR_SIZE, SEEK_SET), 0); ASSERT_TRUE(android::base::ReadFully(fd, buffer, sizeof(buffer))); @@ -261,8 +261,6 @@ TEST(liblp, NoChangingGeometry) { imported = ReadMetadata(fd, 0); ASSERT_NE(imported, nullptr); - imported->geometry.last_logical_sector--; - ASSERT_FALSE(UpdatePartitionTable(fd, *imported.get(), 1)); } // Test that changing one bit of metadata is enough to break the checksum. @@ -370,9 +368,6 @@ TEST(liblp, TooManyPartitions) { ASSERT_GE(lseek(fd, exported->geometry.first_logical_sector * LP_SECTOR_SIZE, SEEK_SET), 0); ASSERT_TRUE(android::base::ReadFully(fd, buffer, sizeof(buffer))); EXPECT_EQ(memcmp(expected, buffer, LP_SECTOR_SIZE), 0); - ASSERT_GE(lseek(fd, exported->geometry.last_logical_sector * LP_SECTOR_SIZE, SEEK_SET), 0); - ASSERT_TRUE(android::base::ReadFully(fd, buffer, sizeof(buffer))); - EXPECT_EQ(memcmp(expected, buffer, LP_SECTOR_SIZE), 0); } // Test that we can read and write image files. diff --git a/fs_mgr/liblp/utility_test.cpp b/fs_mgr/liblp/utility_test.cpp index ff50e0956..46ed2e63f 100644 --- a/fs_mgr/liblp/utility_test.cpp +++ b/fs_mgr/liblp/utility_test.cpp @@ -37,7 +37,6 @@ TEST(liblp, GetMetadataOffset) { 16384, 4, 10000, - 80000, 0, 0, 1024 * 1024, diff --git a/fs_mgr/liblp/writer.cpp b/fs_mgr/liblp/writer.cpp index 0c0cc494f..5cf1a2c1e 100644 --- a/fs_mgr/liblp/writer.cpp +++ b/fs_mgr/liblp/writer.cpp @@ -43,8 +43,9 @@ std::string SerializeGeometry(const LpMetadataGeometry& input) { static bool CompareGeometry(const LpMetadataGeometry& g1, const LpMetadataGeometry& g2) { return g1.metadata_max_size == g2.metadata_max_size && g1.metadata_slot_count == g2.metadata_slot_count && - g1.first_logical_sector == g2.first_logical_sector && - g1.last_logical_sector == g2.last_logical_sector; + g1.block_device_size == g2.block_device_size && + g1.logical_block_size == g2.logical_block_size && + g1.first_logical_sector == g2.first_logical_sector; } std::string SerializeMetadata(const LpMetadata& input) { @@ -86,15 +87,11 @@ static bool ValidateAndSerializeMetadata(int fd, const LpMetadata& metadata, std return false; } - *blob = SerializeMetadata(metadata); - const LpMetadataHeader& header = metadata.header; const LpMetadataGeometry& geometry = metadata.geometry; - // Validate the usable sector range. - if (geometry.first_logical_sector > geometry.last_logical_sector) { - LERROR << "Logical partition metadata has invalid sector range."; - return false; - } + + *blob = SerializeMetadata(metadata); + // Make sure we're writing within the space reserved. if (blob->size() > geometry.metadata_max_size) { LERROR << "Logical partition metadata is too large. " << blob->size() << " > " @@ -128,11 +125,12 @@ static bool ValidateAndSerializeMetadata(int fd, const LpMetadata& metadata, std } // Make sure all linear extents have a valid range. + uint64_t last_sector = geometry.block_device_size / LP_SECTOR_SIZE; for (const auto& extent : metadata.extents) { if (extent.target_type == LP_TARGET_TYPE_LINEAR) { uint64_t physical_sector = extent.target_data; if (physical_sector < geometry.first_logical_sector || - physical_sector + extent.num_sectors > geometry.last_logical_sector) { + physical_sector + extent.num_sectors > last_sector) { LERROR << "Extent table entry is out of bounds."; return false; } @@ -167,7 +165,7 @@ static bool WriteBackupMetadata(int fd, const LpMetadataGeometry& geometry, uint } if (abs_offset >= int64_t(geometry.first_logical_sector) * LP_SECTOR_SIZE) { PERROR << __PRETTY_FUNCTION__ << "backup offset " << abs_offset - << " is within logical partition bounds, sector " << geometry.last_logical_sector; + << " is within logical partition bounds, sector " << geometry.first_logical_sector; return false; } if (!writer(fd, blob)) {