liblp: Make it easier to test UpdatePartitionTable.
This change makes the internal UpdatePartitionTable function more
testable by parameterizing its write functions. It also adds two tests,
one of which exposes a flaw in the current implementation.
Bug: 79173901
Test: liblp_test gtest
Change-Id: I3c4112794b97d577a27f035baeac2d42ac75f552
diff --git a/fs_mgr/liblp/Android.bp b/fs_mgr/liblp/Android.bp
index f59fa84..1050cf5 100644
--- a/fs_mgr/liblp/Android.bp
+++ b/fs_mgr/liblp/Android.bp
@@ -46,6 +46,9 @@
cc_test {
name: "liblp_test",
defaults: ["fs_mgr_defaults"],
+ cppflags: [
+ "-Wno-unused-parameter",
+ ],
static_libs: [
"libbase",
"liblog",
diff --git a/fs_mgr/liblp/include/liblp/writer.h b/fs_mgr/liblp/include/liblp/writer.h
index 6e3c9cb..f4d1ad7 100644
--- a/fs_mgr/liblp/include/liblp/writer.h
+++ b/fs_mgr/liblp/include/liblp/writer.h
@@ -17,6 +17,7 @@
#ifndef LIBLP_WRITER_H
#define LIBLP_WRITER_H
+#include <functional>
#include "metadata_format.h"
namespace android {
@@ -43,6 +44,9 @@
bool FlashPartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number);
bool UpdatePartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number);
+bool UpdatePartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number,
+ std::function<bool(int, const std::string&)> writer);
+
// Helper function to serialize geometry and metadata to a normal file, for
// flashing or debugging.
bool WriteToImageFile(const char* file, const LpMetadata& metadata);
diff --git a/fs_mgr/liblp/io_test.cpp b/fs_mgr/liblp/io_test.cpp
index 29d8ca5..c3f8f36 100644
--- a/fs_mgr/liblp/io_test.cpp
+++ b/fs_mgr/liblp/io_test.cpp
@@ -393,3 +393,85 @@
unique_ptr<LpMetadata> imported = ReadFromImageFile(fd);
ASSERT_NE(imported, nullptr);
}
+
+class BadWriter {
+ public:
+ // When requested, write garbage instead of the requested bytes, then
+ // return false.
+ bool operator()(int fd, const std::string& blob) {
+ if (++write_count_ == fail_on_write_) {
+ std::unique_ptr<char[]> new_data = std::make_unique<char[]>(blob.size());
+ memset(new_data.get(), 0xe5, blob.size());
+ EXPECT_TRUE(android::base::WriteFully(fd, new_data.get(), blob.size()));
+ return false;
+ } else {
+ return android::base::WriteFully(fd, blob.data(), blob.size());
+ }
+ }
+ void FailOnWrite(int number) {
+ fail_on_write_ = number;
+ write_count_ = 0;
+ }
+
+ private:
+ int fail_on_write_ = 0;
+ int write_count_ = 0;
+};
+
+// Test that an interrupted flash operation on the "primary" copy of metadata
+// is not fatal.
+TEST(liblp, FlashPrimaryMetadataFailure) {
+ // Initial state.
+ unique_fd fd = CreateFlashedDisk();
+ ASSERT_GE(fd, 0);
+
+ BadWriter writer;
+
+ // Read and write it back.
+ writer.FailOnWrite(1);
+ unique_ptr<LpMetadata> imported = ReadMetadata(fd, 0);
+ ASSERT_NE(imported, nullptr);
+ ASSERT_FALSE(UpdatePartitionTable(fd, *imported.get(), 0, writer));
+
+ // We should still be able to read the backup copy.
+ imported = ReadMetadata(fd, 0);
+ ASSERT_NE(imported, nullptr);
+
+ // Flash again, this time fail the backup copy. We should still be able
+ // to read the primary.
+ writer.FailOnWrite(2);
+ ASSERT_FALSE(UpdatePartitionTable(fd, *imported.get(), 0, writer));
+ imported = ReadMetadata(fd, 0);
+ ASSERT_NE(imported, nullptr);
+}
+
+// Test that an interrupted flash operation on the "backup" copy of metadata
+// is not fatal.
+TEST(liblp, FlashBackupMetadataFailure) {
+ // Initial state.
+ unique_fd fd = CreateFlashedDisk();
+ ASSERT_GE(fd, 0);
+
+ BadWriter writer;
+
+ // Read and write it back.
+ writer.FailOnWrite(2);
+ unique_ptr<LpMetadata> imported = ReadMetadata(fd, 0);
+ ASSERT_NE(imported, nullptr);
+ ASSERT_FALSE(UpdatePartitionTable(fd, *imported.get(), 0, writer));
+
+ // We should still be able to read the primary copy.
+ imported = ReadMetadata(fd, 0);
+ ASSERT_NE(imported, nullptr);
+
+ // Flash again, this time fail the primary copy. We should still be able
+ // to read the primary.
+ //
+ // TODO(dvander): This is currently not handled correctly. liblp does not
+ // guarantee both copies are in sync before the update. The ASSERT_EQ
+ // will change to an ASSERT_NE when this is fixed.
+ writer.FailOnWrite(1);
+ ASSERT_FALSE(UpdatePartitionTable(fd, *imported.get(), 0, writer));
+ imported = ReadMetadata(fd, 0);
+ ASSERT_EQ(imported, nullptr);
+}
diff --git a/fs_mgr/liblp/writer.cpp b/fs_mgr/liblp/writer.cpp
index e3dba47..36c7b5a 100644
--- a/fs_mgr/liblp/writer.cpp
+++ b/fs_mgr/liblp/writer.cpp
@@ -130,8 +130,13 @@
return true;
}
+static bool DefaultWriter(int fd, const std::string& blob) {
+ return android::base::WriteFully(fd, blob.data(), blob.size());
+}
+
static bool WriteMetadata(int fd, const LpMetadataGeometry& geometry, uint32_t slot_number,
- const std::string& blob) {
+ const std::string& blob,
+ std::function<bool(int, const std::string&)> writer) {
// Make sure we're writing to a valid metadata slot.
if (slot_number >= geometry.metadata_slot_count) {
LERROR << "Invalid logical partition metadata slot number.";
@@ -144,7 +149,7 @@
PERROR << __PRETTY_FUNCTION__ << "lseek failed: offset " << primary_offset;
return false;
}
- if (!android::base::WriteFully(fd, blob.data(), blob.size())) {
+ if (!writer(fd, blob)) {
PERROR << __PRETTY_FUNCTION__ << "write " << blob.size() << " bytes failed";
return false;
}
@@ -161,7 +166,7 @@
<< " is within logical partition bounds, sector " << geometry.last_logical_sector;
return false;
}
- if (!android::base::WriteFully(fd, blob.data(), blob.size())) {
+ if (!writer(fd, blob)) {
PERROR << __PRETTY_FUNCTION__ << "backup write " << blob.size() << " bytes failed";
return false;
}
@@ -197,10 +202,11 @@
}
// Write metadata to the correct slot, now that geometry is in place.
- return WriteMetadata(fd, metadata.geometry, slot_number, metadata_blob);
+ return WriteMetadata(fd, metadata.geometry, slot_number, metadata_blob, DefaultWriter);
}
-bool UpdatePartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number) {
+bool UpdatePartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number,
+ std::function<bool(int, const std::string&)> writer) {
// 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.
@@ -221,7 +227,7 @@
LERROR << "Incompatible geometry in new logical partition metadata";
return false;
}
- return WriteMetadata(fd, geometry, slot_number, blob);
+ return WriteMetadata(fd, geometry, slot_number, blob, writer);
}
bool FlashPartitionTable(const std::string& block_device, const LpMetadata& metadata,
@@ -244,6 +250,10 @@
return UpdatePartitionTable(fd, metadata, slot_number);
}
+bool UpdatePartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number) {
+ return UpdatePartitionTable(fd, metadata, slot_number, DefaultWriter);
+}
+
bool WriteToImageFile(int fd, const LpMetadata& input) {
std::string geometry = SerializeGeometry(input.geometry);
std::string padding(LP_METADATA_GEOMETRY_SIZE - geometry.size(), '\0');