5 files changed, 125 insertions, 35 deletions

diff --git a/fs_mgr/liblp/builder.cpp b/fs_mgr/liblp/builder.cpp
index d3c785d33..eb429b930 100644
--- a/fs_mgr/liblp/builder.cpp
+++ b/fs_mgr/liblp/builder.cpp
@@ -48,10 +48,20 @@ bool GetBlockDeviceInfo(const std::string& block_device, BlockDeviceInfo* device
         PERROR << __PRETTY_FUNCTION__ << "BLKIOMIN failed";
         return false;
     }
-    if (ioctl(fd, BLKALIGNOFF, &device_info->alignment_offset) < 0) {
+
+    int alignment_offset;
+    if (ioctl(fd, BLKALIGNOFF, &alignment_offset) < 0) {
         PERROR << __PRETTY_FUNCTION__ << "BLKIOMIN failed";
         return false;
     }
+    int logical_block_size;
+    if (ioctl(fd, BLKSSZGET, &logical_block_size) < 0) {
+        PERROR << __PRETTY_FUNCTION__ << "BLKSSZGET failed";
+        return false;
+    }
+
+    device_info->alignment_offset = static_cast<uint32_t>(alignment_offset);
+    device_info->logical_block_size = static_cast<uint32_t>(logical_block_size);
     return true;
 #else
     (void)block_device;
@@ -178,6 +188,7 @@ bool MetadataBuilder::Init(const LpMetadata& metadata) {
 
     device_info_.alignment = geometry_.alignment;
     device_info_.alignment_offset = geometry_.alignment_offset;
+    device_info_.logical_block_size = geometry_.logical_block_size;
     return true;
 }
 
@@ -201,6 +212,10 @@ bool MetadataBuilder::Init(const BlockDeviceInfo& device_info, uint32_t metadata
         LERROR << "Block device size must be a multiple of 512.";
         return false;
     }
+    if (device_info_.logical_block_size % LP_SECTOR_SIZE != 0) {
+        LERROR << "Logical block size must be a multiple of 512.";
+        return false;
+    }
     if (device_info_.alignment_offset % LP_SECTOR_SIZE != 0) {
         LERROR << "Alignment offset is not sector-aligned.";
         return false;
@@ -244,6 +259,18 @@ bool MetadataBuilder::Init(const BlockDeviceInfo& device_info, uint32_t metadata
         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;
@@ -251,6 +278,7 @@ bool MetadataBuilder::Init(const BlockDeviceInfo& device_info, uint32_t metadata
     geometry_.alignment = device_info_.alignment;
     geometry_.alignment_offset = device_info_.alignment_offset;
     geometry_.block_device_size = device_info_.size;
+    geometry_.logical_block_size = device_info.logical_block_size;
     return true;
 }
 
@@ -297,6 +325,7 @@ bool MetadataBuilder::GrowPartition(Partition* partition, uint64_t aligned_size)
         uint64_t end;
 
         Interval(uint64_t start, uint64_t end) : start(start), end(end) {}
+        uint64_t length() const { return end - start; }
         bool operator<(const Interval& other) const { return start < other.start; }
     };
 
@@ -343,31 +372,46 @@ bool MetadataBuilder::GrowPartition(Partition* partition, uint64_t aligned_size)
         free_regions.emplace_back(last_free_extent_start, geometry_.last_logical_sector + 1);
     }
 
+    const uint64_t sectors_per_block = device_info_.logical_block_size / LP_SECTOR_SIZE;
+    CHECK(sectors_needed % sectors_per_block == 0);
+
     // Find gaps that we can use for new extents. Note we store new extents in a
     // temporary vector, and only commit them if we are guaranteed enough free
     // space.
     std::vector<std::unique_ptr<LinearExtent>> new_extents;
-    for (const auto& region : free_regions) {
-        // This gap is enough to hold the remainder of the space requested, so we
-        // can allocate what we need and return.
-        if (region.end - region.start >= sectors_needed) {
-            auto extent = std::make_unique<LinearExtent>(sectors_needed, region.start);
-            sectors_needed -= extent->num_sectors();
-            new_extents.push_back(std::move(extent));
-            break;
+    for (auto& region : free_regions) {
+        if (region.length() % sectors_per_block != 0) {
+            // This should never happen, because it would imply that we
+            // once allocated an extent that was not a multiple of the
+            // block size. That extent would be rejected by DM_TABLE_LOAD.
+            LERROR << "Region " << region.start << ".." << region.end
+                   << " is not a multiple of the block size, " << sectors_per_block;
+
+            // If for some reason the final region is mis-sized we still want
+            // to be able to grow partitions. So just to be safe, round the
+            // region down to the nearest block.
+            region.end = region.start + (region.length() / sectors_per_block) * sectors_per_block;
+            if (!region.length()) {
+                continue;
+            }
         }
 
-        // This gap is not big enough to fit the remainder of the space requested,
-        // so consume the whole thing and keep looking for more.
-        auto extent = std::make_unique<LinearExtent>(region.end - region.start, region.start);
-        sectors_needed -= extent->num_sectors();
+        uint64_t sectors = std::min(sectors_needed, region.length());
+        CHECK(sectors % sectors_per_block == 0);
+
+        auto extent = std::make_unique<LinearExtent>(sectors, region.start);
         new_extents.push_back(std::move(extent));
+        sectors_needed -= sectors;
+        if (!sectors_needed) {
+            break;
+        }
     }
     if (sectors_needed) {
         LERROR << "Not enough free space to expand partition: " << partition->name();
         return false;
     }
 
+    // Everything succeeded, so commit the new extents.
     for (auto& extent : new_extents) {
         partition->AddExtent(std::move(extent));
     }
@@ -435,6 +479,12 @@ uint64_t MetadataBuilder::AlignSector(uint64_t sector) {
 void MetadataBuilder::set_block_device_info(const BlockDeviceInfo& device_info) {
     device_info_.size = device_info.size;
 
+    // Note that if the logical block size changes, we're probably in trouble:
+    // we could have already built extents that will only work on the previous
+    // size.
+    DCHECK(partitions_.empty() ||
+           device_info_.logical_block_size == device_info.logical_block_size);
+
     // The kernel does not guarantee these values are present, so we only
     // replace existing values if the new values are non-zero.
     if (device_info.alignment) {
@@ -447,7 +497,7 @@ void MetadataBuilder::set_block_device_info(const BlockDeviceInfo& device_info)
 
 bool MetadataBuilder::ResizePartition(Partition* partition, uint64_t requested_size) {
     // Align the space needed up to the nearest sector.
-    uint64_t aligned_size = AlignTo(requested_size, LP_SECTOR_SIZE);
+    uint64_t aligned_size = AlignTo(requested_size, device_info_.logical_block_size);
 
     if (aligned_size > partition->size()) {
         return GrowPartition(partition, aligned_size);
diff --git a/fs_mgr/liblp/builder_test.cpp b/fs_mgr/liblp/builder_test.cpp
index 4334d51a5..f1a91c479 100644
--- a/fs_mgr/liblp/builder_test.cpp
+++ b/fs_mgr/liblp/builder_test.cpp
@@ -92,11 +92,11 @@ TEST(liblp, PartitionAlignment) {
     Partition* system = builder->AddPartition("system", TEST_GUID, LP_PARTITION_ATTR_READONLY);
     ASSERT_NE(system, nullptr);
     EXPECT_EQ(builder->ResizePartition(system, 10000), true);
-    EXPECT_EQ(system->size(), 10240);
+    EXPECT_EQ(system->size(), 12288);
     EXPECT_EQ(system->extents().size(), 1);
 
-    builder->ResizePartition(system, 9000);
-    EXPECT_EQ(system->size(), 9216);
+    builder->ResizePartition(system, 7000);
+    EXPECT_EQ(system->size(), 8192);
     EXPECT_EQ(system->extents().size(), 1);
 }
 
@@ -120,13 +120,13 @@ TEST(liblp, MetadataAlignment) {
 
 TEST(liblp, InternalAlignment) {
     // Test the metadata fitting within alignment.
-    BlockDeviceInfo device_info(1024 * 1024, 768 * 1024, 0);
+    BlockDeviceInfo device_info(1024 * 1024, 768 * 1024, 0, 4096);
     unique_ptr<MetadataBuilder> builder = MetadataBuilder::New(device_info, 1024, 2);
     ASSERT_NE(builder, nullptr);
     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, 2035);
+    EXPECT_EQ(exported->geometry.last_logical_sector, 2031);
 
     // Test a large alignment offset thrown in.
     device_info.alignment_offset = 753664;
@@ -135,7 +135,7 @@ 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, 2035);
+    EXPECT_EQ(exported->geometry.last_logical_sector, 2031);
 
     // Alignment offset without alignment doesn't mean anything.
     device_info.alignment = 0;
@@ -150,7 +150,7 @@ TEST(liblp, InternalAlignment) {
     exported = builder->Export();
     ASSERT_NE(exported, nullptr);
     EXPECT_EQ(exported->geometry.first_logical_sector, 78);
-    EXPECT_EQ(exported->geometry.last_logical_sector, 1975);
+    EXPECT_EQ(exported->geometry.last_logical_sector, 1973);
 
     // Test a small alignment with no alignment offset.
     device_info.alignment = 11 * 1024;
@@ -163,7 +163,7 @@ TEST(liblp, InternalAlignment) {
 }
 
 TEST(liblp, InternalPartitionAlignment) {
-    BlockDeviceInfo device_info(512 * 1024 * 1024, 768 * 1024, 753664);
+    BlockDeviceInfo device_info(512 * 1024 * 1024, 768 * 1024, 753664, 4096);
     unique_ptr<MetadataBuilder> builder = MetadataBuilder::New(device_info, 32 * 1024, 2);
 
     Partition* a = builder->AddPartition("a", TEST_GUID, 0);
@@ -381,7 +381,7 @@ TEST(liblp, MetadataTooLarge) {
     static const size_t kMetadataSize = 64 * 1024;
 
     // No space to store metadata + geometry.
-    BlockDeviceInfo device_info(kDiskSize, 0, 0);
+    BlockDeviceInfo device_info(kDiskSize, 0, 0, 4096);
     unique_ptr<MetadataBuilder> builder = MetadataBuilder::New(device_info, kMetadataSize, 1);
     EXPECT_EQ(builder, nullptr);
 
@@ -390,8 +390,8 @@ TEST(liblp, MetadataTooLarge) {
     builder = MetadataBuilder::New(device_info, kMetadataSize, 1);
     EXPECT_EQ(builder, nullptr);
 
-    // Space for metadata + geometry + one free sector.
-    device_info.size += LP_SECTOR_SIZE;
+    // Space for metadata + geometry + one free block.
+    device_info.size += device_info.logical_block_size;
     builder = MetadataBuilder::New(device_info, kMetadataSize, 1);
     EXPECT_NE(builder, nullptr);
 
@@ -424,19 +424,21 @@ TEST(liblp, block_device_info) {
     ASSERT_EQ(device_info.alignment % LP_SECTOR_SIZE, 0);
     ASSERT_EQ(device_info.alignment_offset % LP_SECTOR_SIZE, 0);
     ASSERT_LE(device_info.alignment_offset, INT_MAX);
+    ASSERT_EQ(device_info.logical_block_size % LP_SECTOR_SIZE, 0);
 
     // Having an alignment offset > alignment doesn't really make sense.
     ASSERT_LT(device_info.alignment_offset, device_info.alignment);
 }
 
 TEST(liblp, UpdateBlockDeviceInfo) {
-    BlockDeviceInfo device_info(1024 * 1024, 4096, 1024);
+    BlockDeviceInfo device_info(1024 * 1024, 4096, 1024, 4096);
     unique_ptr<MetadataBuilder> builder = MetadataBuilder::New(device_info, 1024, 1);
     ASSERT_NE(builder, nullptr);
 
     EXPECT_EQ(builder->block_device_info().size, device_info.size);
     EXPECT_EQ(builder->block_device_info().alignment, device_info.alignment);
     EXPECT_EQ(builder->block_device_info().alignment_offset, device_info.alignment_offset);
+    EXPECT_EQ(builder->block_device_info().logical_block_size, device_info.logical_block_size);
 
     device_info.alignment = 0;
     device_info.alignment_offset = 2048;
@@ -450,3 +452,27 @@ TEST(liblp, UpdateBlockDeviceInfo) {
     EXPECT_EQ(builder->block_device_info().alignment, 8192);
     EXPECT_EQ(builder->block_device_info().alignment_offset, 2048);
 }
+
+TEST(liblp, InvalidBlockSize) {
+    BlockDeviceInfo device_info(1024 * 1024, 0, 0, 513);
+    unique_ptr<MetadataBuilder> builder = MetadataBuilder::New(device_info, 1024, 1);
+    EXPECT_EQ(builder, nullptr);
+}
+
+TEST(liblp, AlignedExtentSize) {
+    BlockDeviceInfo device_info(1024 * 1024, 0, 0, 4096);
+    unique_ptr<MetadataBuilder> builder = MetadataBuilder::New(device_info, 1024, 1);
+    ASSERT_NE(builder, nullptr);
+
+    Partition* partition = builder->AddPartition("system", TEST_GUID, 0);
+    ASSERT_NE(partition, nullptr);
+    ASSERT_TRUE(builder->ResizePartition(partition, 512));
+    EXPECT_EQ(partition->size(), 4096);
+}
+
+TEST(liblp, AlignedFreeSpace) {
+    // Only one sector free - at least one block is required.
+    BlockDeviceInfo device_info(10240, 0, 0, 4096);
+    unique_ptr<MetadataBuilder> builder = MetadataBuilder::New(device_info, 512, 1);
+    ASSERT_EQ(builder, nullptr);
+}
diff --git a/fs_mgr/liblp/include/liblp/builder.h b/fs_mgr/liblp/include/liblp/builder.h
index 0f96e3a60..e83b92297 100644
--- a/fs_mgr/liblp/include/liblp/builder.h
+++ b/fs_mgr/liblp/include/liblp/builder.h
@@ -32,11 +32,16 @@ class LinearExtent;
 
 // By default, partitions are aligned on a 1MiB boundary.
 static const uint32_t kDefaultPartitionAlignment = 1024 * 1024;
+static const uint32_t kDefaultBlockSize = 4096;
 
 struct BlockDeviceInfo {
-    BlockDeviceInfo() : size(0), alignment(0), alignment_offset(0) {}
-    BlockDeviceInfo(uint64_t size, uint32_t alignment, uint32_t alignment_offset)
-        : size(size), alignment(alignment), alignment_offset(alignment_offset) {}
+    BlockDeviceInfo() : size(0), alignment(0), alignment_offset(0), logical_block_size(0) {}
+    BlockDeviceInfo(uint64_t size, uint32_t alignment, uint32_t alignment_offset,
+                    uint32_t logical_block_size)
+        : size(size),
+          alignment(alignment),
+          alignment_offset(alignment_offset),
+          logical_block_size(logical_block_size) {}
     // Size of the block device, in bytes.
     uint64_t size;
     // Optimal target alignment, in bytes. Partition extents will be aligned to
@@ -46,6 +51,8 @@ struct BlockDeviceInfo {
     // |alignment_offset| on the target device is correctly aligned on its
     // parent device. This value must be 0 or a multiple of 512.
     uint32_t alignment_offset;
+    // Block size, for aligning extent sizes and partition sizes.
+    uint32_t logical_block_size;
 };
 
 // Abstraction around dm-targets that can be encoded into logical partition tables.
@@ -88,6 +95,8 @@ class ZeroExtent final : public Extent {
 };
 
 class Partition final {
+    friend class MetadataBuilder;
+
   public:
     Partition(const std::string& name, const std::string& guid, uint32_t attributes);
 
@@ -97,10 +106,6 @@ class Partition final {
     // Remove all extents from this partition.
     void RemoveExtents();
 
-    // Remove and/or shrink extents until the partition is the requested size.
-    // See MetadataBuilder::ShrinkPartition for more information.
-    void ShrinkTo(uint64_t requested_size);
-
     const std::string& name() const { return name_; }
     uint32_t attributes() const { return attributes_; }
     const std::string& guid() const { return guid_; }
@@ -108,6 +113,8 @@ class Partition final {
     uint64_t size() const { return size_; }
 
   private:
+    void ShrinkTo(uint64_t aligned_size);
+
     std::string name_;
     std::string guid_;
     std::vector<std::unique_ptr<Extent>> extents_;
@@ -144,7 +151,7 @@ class MetadataBuilder {
     // size. This is a convenience method for tests.
     static std::unique_ptr<MetadataBuilder> New(uint64_t blockdev_size, uint32_t metadata_max_size,
                                                 uint32_t metadata_slot_count) {
-        BlockDeviceInfo device_info(blockdev_size, 0, 0);
+        BlockDeviceInfo device_info(blockdev_size, 0, 0, kDefaultBlockSize);
         return New(device_info, metadata_max_size, metadata_slot_count);
     }
 
diff --git a/fs_mgr/liblp/include/liblp/metadata_format.h b/fs_mgr/liblp/include/liblp/metadata_format.h
index e1323e11e..52c80f7f4 100644
--- a/fs_mgr/liblp/include/liblp/metadata_format.h
+++ b/fs_mgr/liblp/include/liblp/metadata_format.h
@@ -136,6 +136,12 @@ typedef struct LpMetadataGeometry {
      * can be used to verify the geometry against a target device.
      */
     uint64_t block_device_size;
+
+    /* 76: Logical block size of the super partition block device. This is the
+     * minimal alignment for partition and extent sizes, and it must be a
+     * multiple of LP_SECTOR_SIZE.
+     */
+    uint32_t logical_block_size;
 } __attribute__((packed)) LpMetadataGeometry;
 
 /* The logical partition metadata has a number of tables; they are described
diff --git a/fs_mgr/liblp/utility_test.cpp b/fs_mgr/liblp/utility_test.cpp
index 092dbf11c..7bf42aed9 100644
--- a/fs_mgr/liblp/utility_test.cpp
+++ b/fs_mgr/liblp/utility_test.cpp
@@ -40,7 +40,8 @@ TEST(liblp, GetMetadataOffset) {
                                    80000,
                                    0,
                                    0,
-                                   1024 * 1024};
+                                   1024 * 1024,
+                                   4096};
     EXPECT_EQ(GetPrimaryMetadataOffset(geometry, 0), 4096);
     EXPECT_EQ(GetPrimaryMetadataOffset(geometry, 1), 4096 + 16384);
     EXPECT_EQ(GetPrimaryMetadataOffset(geometry, 2), 4096 + 16384 * 2);