Merge "liblp: Remove last_logical_sector from LpMetadataGeometry."

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<LpMetadata> 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 {

fs_mgr/liblp/builder_test.cpp

@@ -127,7 +127,6 @@ TEST(liblp, InternalAlignment) {
     unique_ptr<LpMetadata> 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;

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

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.

fs_mgr/liblp/utility_test.cpp

@@ -37,7 +37,6 @@ TEST(liblp, GetMetadataOffset) {
                                    16384,
                                    4,
                                    10000,
-                                   80000,
                                    0,
                                    0,
                                    1024 * 1024,

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)) {