Merge "liblp: Remove last_logical_sector from LpMetadataGeometry."
am: 30793ac5fa
Change-Id: I481316546b670048c24a58fb0c668b8c67f9032a
diff --git a/fs_mgr/liblp/builder.cpp b/fs_mgr/liblp/builder.cpp
index 743a3fe..3d4b826 100644
--- a/fs_mgr/liblp/builder.cpp
+++ b/fs_mgr/liblp/builder.cpp
@@ -268,39 +268,19 @@
}
// 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 @@
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 @@
}
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 ffa7d3b..eb65f89 100644
--- a/fs_mgr/liblp/builder_test.cpp
+++ b/fs_mgr/liblp/builder_test.cpp
@@ -127,7 +127,6 @@
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 @@
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 @@
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 @@
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 @@
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 a4ff1a7..711ff95 100644
--- a/fs_mgr/liblp/include/liblp/metadata_format.h
+++ b/fs_mgr/liblp/include/liblp/metadata_format.h
@@ -38,7 +38,7 @@
#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 @@
*/
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 220d651..92696f5 100644
--- a/fs_mgr/liblp/io_test.cpp
+++ b/fs_mgr/liblp/io_test.cpp
@@ -161,7 +161,6 @@
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 @@
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 @@
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 @@
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 ff50e09..46ed2e6 100644
--- a/fs_mgr/liblp/utility_test.cpp
+++ b/fs_mgr/liblp/utility_test.cpp
@@ -37,7 +37,6 @@
16384,
4,
10000,
- 80000,
0,
0,
1024 * 1024,
diff --git a/fs_mgr/liblp/writer.cpp b/fs_mgr/liblp/writer.cpp
index 0c0cc49..5cf1a2c 100644
--- a/fs_mgr/liblp/writer.cpp
+++ b/fs_mgr/liblp/writer.cpp
@@ -43,8 +43,9 @@
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 @@
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 @@
}
// 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 @@
}
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)) {