From e5f2f06b00fb35eea1cdcc66af549ab061e1c5ce Mon Sep 17 00:00:00 2001 From: David Anderson Date: Wed, 3 Oct 2018 13:49:23 -0700 Subject: [PATCH] liblp: Remove the guid field from LpMetadataPartition. Bug: 117229984 Test: liblp_test gtest Change-Id: Ie42b3a8005b1cf711303966a2a117c255f0fb08c --- fastboot/device/commands.cpp | 8 +-- fastboot/device/flashing.cpp | 3 +- fs_mgr/liblp/Android.bp | 1 - fs_mgr/liblp/builder.cpp | 25 +++------ fs_mgr/liblp/builder_test.cpp | 53 +++++++------------- fs_mgr/liblp/include/liblp/builder.h | 9 ++-- fs_mgr/liblp/include/liblp/liblp.h | 1 - fs_mgr/liblp/include/liblp/metadata_format.h | 13 ++--- fs_mgr/liblp/io_test.cpp | 19 +++---- fs_mgr/liblp/utility.cpp | 10 ---- 10 files changed, 44 insertions(+), 98 deletions(-) diff --git a/fastboot/device/commands.cpp b/fastboot/device/commands.cpp index b02d968c5..8d5ba8e1c 100644 --- a/fastboot/device/commands.cpp +++ b/fastboot/device/commands.cpp @@ -353,13 +353,7 @@ bool CreatePartitionHandler(FastbootDevice* device, const std::vectorWriteFail("Partition already exists"); } - // Make a random UUID, since they're not currently used. - uuid_t uuid; - char uuid_str[37]; - uuid_generate_random(uuid); - uuid_unparse(uuid, uuid_str); - - Partition* partition = builder->AddPartition(partition_name, uuid_str, 0); + Partition* partition = builder->AddPartition(partition_name, 0); if (!partition) { return device->WriteFail("Failed to add partition"); } diff --git a/fastboot/device/flashing.cpp b/fastboot/device/flashing.cpp index a383c54b2..4fc3d1dfb 100644 --- a/fastboot/device/flashing.cpp +++ b/fastboot/device/flashing.cpp @@ -146,8 +146,7 @@ bool UpdateSuper(FastbootDevice* device, const std::string& partition_name, bool if (builder->FindPartition(name)) { continue; } - std::string guid = GetPartitionGuid(partition); - if (!builder->AddPartition(name, guid, partition.attributes)) { + if (!builder->AddPartition(name, partition.attributes)) { return device->WriteFail("Unable to add partition: " + name); } } diff --git a/fs_mgr/liblp/Android.bp b/fs_mgr/liblp/Android.bp index 89282dbc2..69dc06507 100644 --- a/fs_mgr/liblp/Android.bp +++ b/fs_mgr/liblp/Android.bp @@ -35,7 +35,6 @@ cc_library { "libcrypto", "libcrypto_utils", "libsparse", - "libext2_uuid", "libext4_utils", "libz", ], diff --git a/fs_mgr/liblp/builder.cpp b/fs_mgr/liblp/builder.cpp index d9a0e9c55..97b15bdcd 100644 --- a/fs_mgr/liblp/builder.cpp +++ b/fs_mgr/liblp/builder.cpp @@ -25,7 +25,6 @@ #include #include -#include #include "liblp/liblp.h" #include "reader.h" @@ -79,9 +78,8 @@ void ZeroExtent::AddTo(LpMetadata* out) const { out->extents.push_back(LpMetadataExtent{num_sectors_, LP_TARGET_TYPE_ZERO, 0}); } -Partition::Partition(const std::string& name, const std::string& group_name, - const std::string& guid, uint32_t attributes) - : name_(name), group_name_(group_name), guid_(guid), attributes_(attributes), size_(0) {} +Partition::Partition(const std::string& name, const std::string& group_name, uint32_t attributes) + : name_(name), group_name_(group_name), attributes_(attributes), size_(0) {} void Partition::AddExtent(std::unique_ptr&& extent) { size_ += extent->num_sectors() * LP_SECTOR_SIZE; @@ -202,8 +200,8 @@ bool MetadataBuilder::Init(const LpMetadata& metadata) { for (const auto& partition : metadata.partitions) { std::string group_name = GetPartitionGroupName(metadata.groups[partition.group_index]); - Partition* builder = AddPartition(GetPartitionName(partition), group_name, - GetPartitionGuid(partition), partition.attributes); + Partition* builder = + AddPartition(GetPartitionName(partition), group_name, partition.attributes); if (!builder) { return false; } @@ -329,13 +327,12 @@ bool MetadataBuilder::AddGroup(const std::string& group_name, uint64_t maximum_s return true; } -Partition* MetadataBuilder::AddPartition(const std::string& name, const std::string& guid, - uint32_t attributes) { - return AddPartition(name, "default", guid, attributes); +Partition* MetadataBuilder::AddPartition(const std::string& name, uint32_t attributes) { + return AddPartition(name, "default", attributes); } Partition* MetadataBuilder::AddPartition(const std::string& name, const std::string& group_name, - const std::string& guid, uint32_t attributes) { + uint32_t attributes) { if (name.empty()) { LERROR << "Partition must have a non-empty name."; return nullptr; @@ -348,7 +345,7 @@ Partition* MetadataBuilder::AddPartition(const std::string& name, const std::str LERROR << "Could not find partition group: " << group_name; return nullptr; } - partitions_.push_back(std::make_unique(name, group_name, guid, attributes)); + partitions_.push_back(std::make_unique(name, group_name, attributes)); return partitions_.back().get(); } @@ -549,12 +546,6 @@ std::unique_ptr MetadataBuilder::Export() { } strncpy(part.name, partition->name().c_str(), sizeof(part.name)); - if (uuid_parse(partition->guid().c_str(), part.guid) != 0) { - LERROR << "Could not parse guid " << partition->guid() << " for partition " - << partition->name(); - return nullptr; - } - part.first_extent_index = static_cast(metadata->extents.size()); part.num_extents = static_cast(partition->extents().size()); part.attributes = partition->attributes(); diff --git a/fs_mgr/liblp/builder_test.cpp b/fs_mgr/liblp/builder_test.cpp index 711cc64ba..c916b4426 100644 --- a/fs_mgr/liblp/builder_test.cpp +++ b/fs_mgr/liblp/builder_test.cpp @@ -22,16 +22,12 @@ using namespace std; using namespace android::fs_mgr; -static const char* TEST_GUID = "A799D1D6-669F-41D8-A3F0-EBB7572D8302"; -static const char* TEST_GUID2 = "A799D1D6-669F-41D8-A3F0-EBB7572D8303"; - TEST(liblp, BuildBasic) { unique_ptr builder = MetadataBuilder::New(1024 * 1024, 1024, 2); - Partition* partition = builder->AddPartition("system", TEST_GUID, LP_PARTITION_ATTR_READONLY); + Partition* partition = builder->AddPartition("system", LP_PARTITION_ATTR_READONLY); ASSERT_NE(partition, nullptr); EXPECT_EQ(partition->name(), "system"); - EXPECT_EQ(partition->guid(), TEST_GUID); EXPECT_EQ(partition->attributes(), LP_PARTITION_ATTR_READONLY); EXPECT_EQ(partition->size(), 0); EXPECT_EQ(builder->FindPartition("system"), partition); @@ -43,7 +39,7 @@ TEST(liblp, BuildBasic) { TEST(liblp, ResizePartition) { unique_ptr builder = MetadataBuilder::New(1024 * 1024, 1024, 2); - Partition* system = builder->AddPartition("system", TEST_GUID, LP_PARTITION_ATTR_READONLY); + Partition* system = builder->AddPartition("system", LP_PARTITION_ATTR_READONLY); ASSERT_NE(system, nullptr); EXPECT_EQ(builder->ResizePartition(system, 65536), true); EXPECT_EQ(system->size(), 65536); @@ -94,7 +90,7 @@ TEST(liblp, PartitionAlignment) { unique_ptr builder = MetadataBuilder::New(1024 * 1024, 1024, 2); // Test that we align up to one sector. - Partition* system = builder->AddPartition("system", TEST_GUID, LP_PARTITION_ATTR_READONLY); + Partition* system = builder->AddPartition("system", LP_PARTITION_ATTR_READONLY); ASSERT_NE(system, nullptr); EXPECT_EQ(builder->ResizePartition(system, 10000), true); EXPECT_EQ(system->size(), 12288); @@ -171,9 +167,9 @@ TEST(liblp, InternalPartitionAlignment) { BlockDeviceInfo device_info(512 * 1024 * 1024, 768 * 1024, 753664, 4096); unique_ptr builder = MetadataBuilder::New(device_info, 32 * 1024, 2); - Partition* a = builder->AddPartition("a", TEST_GUID, 0); + Partition* a = builder->AddPartition("a", 0); ASSERT_NE(a, nullptr); - Partition* b = builder->AddPartition("b", TEST_GUID2, 0); + Partition* b = builder->AddPartition("b", 0); ASSERT_NE(b, nullptr); // Add a bunch of small extents to each, interleaving. @@ -214,7 +210,7 @@ TEST(liblp, UseAllDiskSpace) { EXPECT_EQ(builder->AllocatableSpace(), allocatable); EXPECT_EQ(builder->UsedSpace(), 0); - Partition* system = builder->AddPartition("system", TEST_GUID, LP_PARTITION_ATTR_READONLY); + Partition* system = builder->AddPartition("system", LP_PARTITION_ATTR_READONLY); ASSERT_NE(system, nullptr); EXPECT_EQ(builder->ResizePartition(system, allocatable), true); EXPECT_EQ(system->size(), allocatable); @@ -229,8 +225,8 @@ TEST(liblp, UseAllDiskSpace) { TEST(liblp, BuildComplex) { unique_ptr builder = MetadataBuilder::New(1024 * 1024, 1024, 2); - Partition* system = builder->AddPartition("system", TEST_GUID, LP_PARTITION_ATTR_READONLY); - Partition* vendor = builder->AddPartition("vendor", TEST_GUID2, LP_PARTITION_ATTR_READONLY); + Partition* system = builder->AddPartition("system", LP_PARTITION_ATTR_READONLY); + Partition* vendor = builder->AddPartition("vendor", LP_PARTITION_ATTR_READONLY); ASSERT_NE(system, nullptr); ASSERT_NE(vendor, nullptr); EXPECT_EQ(builder->ResizePartition(system, 65536), true); @@ -263,15 +259,15 @@ TEST(liblp, BuildComplex) { TEST(liblp, AddInvalidPartition) { unique_ptr builder = MetadataBuilder::New(1024 * 1024, 1024, 2); - Partition* partition = builder->AddPartition("system", TEST_GUID, LP_PARTITION_ATTR_READONLY); + Partition* partition = builder->AddPartition("system", LP_PARTITION_ATTR_READONLY); ASSERT_NE(partition, nullptr); // Duplicate name. - partition = builder->AddPartition("system", TEST_GUID, LP_PARTITION_ATTR_READONLY); + partition = builder->AddPartition("system", LP_PARTITION_ATTR_READONLY); EXPECT_EQ(partition, nullptr); // Empty name. - partition = builder->AddPartition("", TEST_GUID, LP_PARTITION_ATTR_READONLY); + partition = builder->AddPartition("", LP_PARTITION_ATTR_READONLY); EXPECT_EQ(partition, nullptr); } @@ -282,8 +278,8 @@ TEST(liblp, BuilderExport) { unique_ptr builder = MetadataBuilder::New(kDiskSize, kMetadataSize, kMetadataSlots); - Partition* system = builder->AddPartition("system", TEST_GUID, LP_PARTITION_ATTR_READONLY); - Partition* vendor = builder->AddPartition("vendor", TEST_GUID2, LP_PARTITION_ATTR_READONLY); + Partition* system = builder->AddPartition("system", LP_PARTITION_ATTR_READONLY); + Partition* vendor = builder->AddPartition("vendor", LP_PARTITION_ATTR_READONLY); ASSERT_NE(system, nullptr); ASSERT_NE(vendor, nullptr); EXPECT_EQ(builder->ResizePartition(system, 65536), true); @@ -322,7 +318,6 @@ TEST(liblp, BuilderExport) { for (const auto& partition : exported->partitions) { Partition* original = builder->FindPartition(GetPartitionName(partition)); ASSERT_NE(original, nullptr); - EXPECT_EQ(original->guid(), GetPartitionGuid(partition)); for (size_t i = 0; i < partition.num_extents; i++) { const auto& extent = exported->extents[partition.first_extent_index + i]; LinearExtent* original_extent = original->extents()[i]->AsLinearExtent(); @@ -337,8 +332,8 @@ TEST(liblp, BuilderExport) { TEST(liblp, BuilderImport) { unique_ptr builder = MetadataBuilder::New(1024 * 1024, 1024, 2); - Partition* system = builder->AddPartition("system", TEST_GUID, LP_PARTITION_ATTR_READONLY); - Partition* vendor = builder->AddPartition("vendor", TEST_GUID2, LP_PARTITION_ATTR_READONLY); + Partition* system = builder->AddPartition("system", LP_PARTITION_ATTR_READONLY); + Partition* vendor = builder->AddPartition("vendor", LP_PARTITION_ATTR_READONLY); ASSERT_NE(system, nullptr); ASSERT_NE(vendor, nullptr); EXPECT_EQ(builder->ResizePartition(system, 65536), true); @@ -357,11 +352,9 @@ TEST(liblp, BuilderImport) { EXPECT_EQ(system->size(), 98304); ASSERT_EQ(system->extents().size(), 2); - EXPECT_EQ(system->guid(), TEST_GUID); EXPECT_EQ(system->attributes(), LP_PARTITION_ATTR_READONLY); EXPECT_EQ(vendor->size(), 32768); ASSERT_EQ(vendor->extents().size(), 1); - EXPECT_EQ(vendor->guid(), TEST_GUID2); EXPECT_EQ(vendor->attributes(), LP_PARTITION_ATTR_READONLY); LinearExtent* system1 = system->extents()[0]->AsLinearExtent(); @@ -378,17 +371,7 @@ TEST(liblp, ExportNameTooLong) { unique_ptr builder = MetadataBuilder::New(1024 * 1024, 1024, 2); std::string name = "abcdefghijklmnopqrstuvwxyz0123456789"; - Partition* system = builder->AddPartition(name + name, TEST_GUID, LP_PARTITION_ATTR_READONLY); - EXPECT_NE(system, nullptr); - - unique_ptr exported = builder->Export(); - EXPECT_EQ(exported, nullptr); -} - -TEST(liblp, ExportInvalidGuid) { - unique_ptr builder = MetadataBuilder::New(1024 * 1024, 1024, 2); - - Partition* system = builder->AddPartition("system", "bad", LP_PARTITION_ATTR_READONLY); + Partition* system = builder->AddPartition(name + name, LP_PARTITION_ATTR_READONLY); EXPECT_NE(system, nullptr); unique_ptr exported = builder->Export(); @@ -483,7 +466,7 @@ TEST(liblp, AlignedExtentSize) { unique_ptr builder = MetadataBuilder::New(device_info, 1024, 1); ASSERT_NE(builder, nullptr); - Partition* partition = builder->AddPartition("system", TEST_GUID, 0); + Partition* partition = builder->AddPartition("system", 0); ASSERT_NE(partition, nullptr); ASSERT_TRUE(builder->ResizePartition(partition, 512)); EXPECT_EQ(partition->size(), 4096); @@ -511,7 +494,7 @@ TEST(liblp, GroupSizeLimits) { ASSERT_TRUE(builder->AddGroup("google", 16384)); - Partition* partition = builder->AddPartition("system", "google", TEST_GUID, 0); + Partition* partition = builder->AddPartition("system", "google", 0); ASSERT_NE(partition, nullptr); EXPECT_TRUE(builder->ResizePartition(partition, 8192)); EXPECT_EQ(partition->size(), 8192); diff --git a/fs_mgr/liblp/include/liblp/builder.h b/fs_mgr/liblp/include/liblp/builder.h index f1edee7ea..a6044d0a2 100644 --- a/fs_mgr/liblp/include/liblp/builder.h +++ b/fs_mgr/liblp/include/liblp/builder.h @@ -112,8 +112,7 @@ class Partition final { friend class MetadataBuilder; public: - Partition(const std::string& name, const std::string& group_name, const std::string& guid, - uint32_t attributes); + Partition(const std::string& name, const std::string& group_name, uint32_t attributes); // Add a raw extent. void AddExtent(std::unique_ptr&& extent); @@ -128,7 +127,6 @@ class Partition final { const std::string& name() const { return name_; } const std::string& group_name() const { return group_name_; } uint32_t attributes() const { return attributes_; } - const std::string& guid() const { return guid_; } const std::vector>& extents() const { return extents_; } uint64_t size() const { return size_; } @@ -137,7 +135,6 @@ class Partition final { std::string name_; std::string group_name_; - std::string guid_; std::vector> extents_; uint32_t attributes_; uint64_t size_; @@ -190,11 +187,11 @@ class MetadataBuilder { // Add a partition, returning a handle so it can be sized as needed. If a // partition with the given name already exists, nullptr is returned. Partition* AddPartition(const std::string& name, const std::string& group_name, - const std::string& guid, uint32_t attributes); + uint32_t attributes); // Same as AddPartition above, but uses the default partition group which // has no size restrictions. - Partition* AddPartition(const std::string& name, const std::string& guid, uint32_t attributes); + Partition* AddPartition(const std::string& name, uint32_t attributes); // Delete a partition by name if it exists. void RemovePartition(const std::string& name); diff --git a/fs_mgr/liblp/include/liblp/liblp.h b/fs_mgr/liblp/include/liblp/liblp.h index 51d262b37..5f95dca6d 100644 --- a/fs_mgr/liblp/include/liblp/liblp.h +++ b/fs_mgr/liblp/include/liblp/liblp.h @@ -68,7 +68,6 @@ std::unique_ptr ReadFromImageBlob(const void* data, size_t bytes); // Helper to extract safe C++ strings from partition info. std::string GetPartitionName(const LpMetadataPartition& partition); -std::string GetPartitionGuid(const LpMetadataPartition& partition); std::string GetPartitionGroupName(const LpMetadataPartitionGroup& group); // Helper to return a slot number for a slot suffix. diff --git a/fs_mgr/liblp/include/liblp/metadata_format.h b/fs_mgr/liblp/include/liblp/metadata_format.h index 79ef2ab58..2172de270 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 2 +#define LP_METADATA_MAJOR_VERSION 3 #define LP_METADATA_MINOR_VERSION 0 /* Attributes for the LpMetadataPartition::attributes field. @@ -232,23 +232,20 @@ typedef struct LpMetadataPartition { */ char name[36]; - /* 36: Globally unique identifier (GUID) of this partition. */ - uint8_t guid[16]; - - /* 52: Attributes for the partition (see LP_PARTITION_ATTR_* flags above). */ + /* 36: Attributes for the partition (see LP_PARTITION_ATTR_* flags above). */ uint32_t attributes; - /* 56: Index of the first extent owned by this partition. The extent will + /* 40: Index of the first extent owned by this partition. The extent will * start at logical sector 0. Gaps between extents are not allowed. */ uint32_t first_extent_index; - /* 60: Number of extents in the partition. Every partition must have at + /* 44: Number of extents in the partition. Every partition must have at * least one extent. */ uint32_t num_extents; - /* 64: Group this partition belongs to. */ + /* 48: Group this partition belongs to. */ uint32_t group_index; } __attribute__((packed)) LpMetadataPartition; diff --git a/fs_mgr/liblp/io_test.cpp b/fs_mgr/liblp/io_test.cpp index f93852b1d..01de3aca3 100644 --- a/fs_mgr/liblp/io_test.cpp +++ b/fs_mgr/liblp/io_test.cpp @@ -37,8 +37,6 @@ using unique_fd = android::base::unique_fd; static const size_t kDiskSize = 131072; static const size_t kMetadataSize = 512; static const size_t kMetadataSlots = 2; -static const char* TEST_GUID_BASE = "A799D1D6-669F-41D8-A3F0-EBB7572D830"; -static const char* TEST_GUID = "A799D1D6-669F-41D8-A3F0-EBB7572D8302"; // Helper function for creating an in-memory file descriptor. This lets us // simulate read/writing logical partition metadata as if we had a block device @@ -81,7 +79,7 @@ static unique_ptr CreateDefaultBuilder() { } static bool AddDefaultPartitions(MetadataBuilder* builder) { - Partition* system = builder->AddPartition("system", TEST_GUID, LP_PARTITION_ATTR_NONE); + Partition* system = builder->AddPartition("system", LP_PARTITION_ATTR_NONE); if (!system) { return false; } @@ -171,7 +169,6 @@ TEST(liblp, FlashAndReadback) { // Check partition tables. ASSERT_EQ(exported->partitions.size(), imported->partitions.size()); EXPECT_EQ(GetPartitionName(exported->partitions[0]), GetPartitionName(imported->partitions[0])); - EXPECT_EQ(GetPartitionGuid(exported->partitions[0]), GetPartitionGuid(imported->partitions[0])); EXPECT_EQ(exported->partitions[0].attributes, imported->partitions[0].attributes); EXPECT_EQ(exported->partitions[0].first_extent_index, imported->partitions[0].first_extent_index); @@ -331,18 +328,18 @@ TEST(liblp, TooManyPartitions) { unique_ptr builder = CreateDefaultBuilder(); ASSERT_NE(builder, nullptr); - // Compute the maximum number of partitions we can fit in 1024 bytes of metadata. - size_t max_partitions = (kMetadataSize - sizeof(LpMetadataHeader)) / sizeof(LpMetadataPartition); - EXPECT_LT(max_partitions, 10); + // Compute the maximum number of partitions we can fit in 512 bytes of + // metadata. By default there is the header, and one partition group. + static const size_t kMaxPartitionTableSize = + kMetadataSize - sizeof(LpMetadataHeader) - sizeof(LpMetadataPartitionGroup); + size_t max_partitions = kMaxPartitionTableSize / sizeof(LpMetadataPartition); // Add this number of partitions. Partition* partition = nullptr; for (size_t i = 0; i < max_partitions; i++) { - std::string guid = std::string(TEST_GUID) + to_string(i); - partition = builder->AddPartition(to_string(i), TEST_GUID, LP_PARTITION_ATTR_NONE); + partition = builder->AddPartition(to_string(i), LP_PARTITION_ATTR_NONE); ASSERT_NE(partition, nullptr); } - ASSERT_NE(partition, nullptr); unique_ptr exported = builder->Export(); ASSERT_NE(exported, nullptr); @@ -354,7 +351,7 @@ TEST(liblp, TooManyPartitions) { ASSERT_TRUE(FlashPartitionTable(fd, *exported.get())); // Check that adding one more partition overflows the metadata allotment. - partition = builder->AddPartition("final", TEST_GUID, LP_PARTITION_ATTR_NONE); + partition = builder->AddPartition("final", LP_PARTITION_ATTR_NONE); EXPECT_NE(partition, nullptr); exported = builder->Export(); diff --git a/fs_mgr/liblp/utility.cpp b/fs_mgr/liblp/utility.cpp index a5900377a..b08f96cc2 100644 --- a/fs_mgr/liblp/utility.cpp +++ b/fs_mgr/liblp/utility.cpp @@ -23,7 +23,6 @@ #include #include #include -#include #include "utility.h" @@ -80,15 +79,6 @@ void SHA256(const void* data, size_t length, uint8_t out[32]) { SHA256_Final(out, &c); } -std::string GetPartitionGuid(const LpMetadataPartition& partition) { - // 32 hex characters, four hyphens. Unfortunately libext2_uuid provides no - // macro to assist with buffer sizing. - static const size_t kGuidLen = 36; - char buffer[kGuidLen + 1]; - uuid_unparse_upper(partition.guid, buffer); - return buffer; -} - uint32_t SlotNumberForSlotSuffix(const std::string& suffix) { if (suffix.empty()) { return 0;