1 files changed, 64 insertions, 14 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);