From c720aa1893a2b37cf3937fe9fdcf142b11a8575c Mon Sep 17 00:00:00 2001 From: David Anderson Date: Tue, 14 Aug 2018 18:17:33 -0700 Subject: [PATCH] liblp: FlashPartitionTable should update all slots. Logical partition metadata has "slots" for AB purposes, but when flashing or updating the partition table via fastboot, there is no reason not to synchronize all copies of the metadata. It makes the state of the super partition much more clear. It also makes the super partition less likely to break on a slot change from the user, for example if a "_b" partition is created before changing to the "b" slot. Bug: 78793464 Test: liblp_test gtest Change-Id: I3c44f0362f21f87d0bfc3a5c3394e26dc3dd38be --- fs_mgr/liblp/include/liblp/liblp.h | 3 +-- fs_mgr/liblp/io_test.cpp | 9 ++++----- fs_mgr/liblp/writer.cpp | 14 ++++++++------ fs_mgr/liblp/writer.h | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/fs_mgr/liblp/include/liblp/liblp.h b/fs_mgr/liblp/include/liblp/liblp.h index 627aa8cad..dd9a9cdf6 100644 --- a/fs_mgr/liblp/include/liblp/liblp.h +++ b/fs_mgr/liblp/include/liblp/liblp.h @@ -42,8 +42,7 @@ struct LpMetadata { // existing geometry, and should not be used for normal partition table // updates. False can be returned if the geometry is incompatible with the // block device or an I/O error occurs. -bool FlashPartitionTable(const std::string& block_device, const LpMetadata& metadata, - uint32_t slot_number); +bool FlashPartitionTable(const std::string& block_device, const LpMetadata& metadata); // Update the partition table for a given metadata slot number. False is // returned if an error occurs, which can include: diff --git a/fs_mgr/liblp/io_test.cpp b/fs_mgr/liblp/io_test.cpp index 638f4b310..a8d6d7051 100644 --- a/fs_mgr/liblp/io_test.cpp +++ b/fs_mgr/liblp/io_test.cpp @@ -103,7 +103,7 @@ static unique_fd CreateFlashedDisk() { if (!exported) { return {}; } - if (!FlashPartitionTable(fd, *exported.get(), 0)) { + if (!FlashPartitionTable(fd, *exported.get())) { return {}; } return fd; @@ -132,7 +132,7 @@ TEST(liblp, ExportDiskTooSmall) { unique_fd fd = CreateFakeDisk(); ASSERT_GE(fd, 0); - EXPECT_FALSE(FlashPartitionTable(fd, *exported.get(), 0)); + EXPECT_FALSE(FlashPartitionTable(fd, *exported.get())); } // Test the basics of flashing a partition and reading it back. @@ -147,7 +147,7 @@ TEST(liblp, FlashAndReadback) { // Export and flash. unique_ptr exported = builder->Export(); ASSERT_NE(exported, nullptr); - ASSERT_TRUE(FlashPartitionTable(fd, *exported.get(), 0)); + ASSERT_TRUE(FlashPartitionTable(fd, *exported.get())); // Read back. Note that some fields are only filled in during // serialization, so exported and imported will not be identical. For @@ -354,8 +354,7 @@ TEST(liblp, TooManyPartitions) { ASSERT_GE(fd, 0); // Check that we are able to write our table. - ASSERT_TRUE(FlashPartitionTable(fd, *exported.get(), 0)); - ASSERT_TRUE(UpdatePartitionTable(fd, *exported.get(), 1)); + 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); diff --git a/fs_mgr/liblp/writer.cpp b/fs_mgr/liblp/writer.cpp index fc9d83fe3..ad84b2208 100644 --- a/fs_mgr/liblp/writer.cpp +++ b/fs_mgr/liblp/writer.cpp @@ -196,7 +196,7 @@ static bool DefaultWriter(int fd, const std::string& blob) { return android::base::WriteFully(fd, blob.data(), blob.size()); } -bool FlashPartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number) { +bool FlashPartitionTable(int fd, const LpMetadata& metadata) { // Before writing geometry and/or logical partition tables, perform some // basic checks that the geometry and tables are coherent, and will fit // on the given block device. @@ -224,8 +224,11 @@ bool FlashPartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_numbe return false; } - // Write metadata to the correct slot, now that geometry is in place. - return WriteMetadata(fd, metadata.geometry, slot_number, metadata_blob, DefaultWriter); + bool ok = true; + for (size_t i = 0; i < metadata.geometry.metadata_slot_count; i++) { + ok &= WriteMetadata(fd, metadata.geometry, i, metadata_blob, DefaultWriter); + } + return ok; } static bool CompareMetadata(const LpMetadata& a, const LpMetadata& b) { @@ -297,14 +300,13 @@ bool UpdatePartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_numb return WriteMetadata(fd, geometry, slot_number, blob, writer); } -bool FlashPartitionTable(const std::string& block_device, const LpMetadata& metadata, - uint32_t slot_number) { +bool FlashPartitionTable(const std::string& block_device, const LpMetadata& metadata) { android::base::unique_fd fd(open(block_device.c_str(), O_RDWR | O_SYNC)); if (fd < 0) { PERROR << __PRETTY_FUNCTION__ << "open failed: " << block_device; return false; } - if (!FlashPartitionTable(fd, metadata, slot_number)) { + if (!FlashPartitionTable(fd, metadata)) { return false; } LWARN << "Flashed new logical partition geometry to " << block_device; diff --git a/fs_mgr/liblp/writer.h b/fs_mgr/liblp/writer.h index adbbebf66..ab18d450c 100644 --- a/fs_mgr/liblp/writer.h +++ b/fs_mgr/liblp/writer.h @@ -30,7 +30,7 @@ std::string SerializeMetadata(const LpMetadata& input); // These variants are for testing only. The path-based functions should be used // for actual operation, so that open() is called with the correct flags. -bool FlashPartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number); +bool FlashPartitionTable(int fd, const LpMetadata& metadata); bool UpdatePartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number); bool UpdatePartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number,