Fix Short read/write issue
It was possible for "short" reads and writes to cause the pattern and
zero steps to not work correctly. This change adds logic to deal with
the short reads.
Tested: unit test and machine test
root@bmc# time busctl call xyz.openbmc_project.eStoraged.mmcblk0 \
> /xyz/openbmc_project/inventory/storage/mmcblk0 \
> xyz.openbmc_project.Inventory.Item.Volume Erase s \
> xyz.openbmc_project.Inventory.Item.Volume.EraseMethod.ZeroOverWrite \
> --timeout=1200
real 6m0.815s
user 0m0.010s
sys 0m0.010s
Change-Id: If8df9bdba159a3bcfa77104a4c17b8d352794db2
Signed-off-by: John Edward Broadbent <jebr@google.com>
diff --git a/src/test/erase/zero_test.cpp b/src/test/erase/zero_test.cpp
index 846059d..b14d546 100644
--- a/src/test/erase/zero_test.cpp
+++ b/src/test/erase/zero_test.cpp
@@ -5,6 +5,7 @@
#include <unistd.h>
#include <stdplus/fd/create.hpp>
+#include <stdplus/fd/gmock.hpp>
#include <stdplus/fd/managed.hpp>
#include <xyz/openbmc_project/Common/error.hpp>
@@ -15,12 +16,14 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>
-using estoraged::Zero;
-using sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure;
-
namespace estoraged_test
{
+using estoraged::Zero;
+using sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure;
+using testing::_;
+using testing::Return;
+
TEST(Zeros, zeroPass)
{
std::string testFileName = "testfile_pass";
@@ -31,8 +34,12 @@
testFile.close();
uint64_t size = 4096;
Zero pass(testFileName);
- EXPECT_NO_THROW(pass.writeZero(size));
- EXPECT_NO_THROW(pass.verifyZero(size));
+ stdplus::fd::Fd&& write =
+ stdplus::fd::open(testFileName, stdplus::fd::OpenAccess::ReadWrite);
+ EXPECT_NO_THROW(pass.writeZero(size, write));
+ stdplus::fd::Fd&& read =
+ stdplus::fd::open(testFileName, stdplus::fd::OpenAccess::ReadWrite);
+ EXPECT_NO_THROW(pass.verifyZero(size, read));
}
/* This test that pattern writes the correct number of bytes even if
@@ -49,9 +56,14 @@
uint64_t size = 4097;
// 4097 is not divisible by the block size, and we expect no errors
+
+ stdplus::fd::Fd&& write =
+ stdplus::fd::open(testFileName, stdplus::fd::OpenAccess::ReadWrite);
Zero pass(testFileName);
- EXPECT_NO_THROW(pass.writeZero(size));
- EXPECT_NO_THROW(pass.verifyZero(size));
+ EXPECT_NO_THROW(pass.writeZero(size, write));
+ stdplus::fd::Fd&& read =
+ stdplus::fd::open(testFileName, stdplus::fd::OpenAccess::ReadWrite);
+ EXPECT_NO_THROW(pass.verifyZero(size, read));
}
TEST(Zeros, notZeroStart)
@@ -66,15 +78,18 @@
sizeof(dummyValue));
testFile.close();
uint64_t size = 4096;
+ stdplus::fd::Fd&& readWrite =
+ stdplus::fd::open(testFileName, stdplus::fd::OpenAccess::ReadWrite);
+
Zero pass(testFileName);
- EXPECT_NO_THROW(pass.writeZero(size - sizeof(dummyValue)));
+ EXPECT_NO_THROW(pass.writeZero(size - sizeof(dummyValue), readWrite));
// force flush, needed for unit testing
std::ofstream file;
file.open(testFileName);
file.close();
- EXPECT_THROW(pass.verifyZero(size), InternalFailure);
+ EXPECT_THROW(pass.verifyZero(size, readWrite), InternalFailure);
}
TEST(Zeros, notZeroEnd)
@@ -89,14 +104,106 @@
uint64_t size = 4096;
Zero pass(testFileName);
uint32_t dummyValue = 88;
- EXPECT_NO_THROW(pass.writeZero(size - sizeof(dummyValue)));
+ stdplus::fd::Fd&& write =
+ stdplus::fd::open(testFileName, stdplus::fd::OpenAccess::ReadWrite);
+ EXPECT_NO_THROW(pass.writeZero(size - sizeof(dummyValue), write));
// open the file and write none zero to it
testFile.open(testFileName, std::ios::out);
testFile << dummyValue;
testFile.close();
- EXPECT_THROW(pass.verifyZero(size), InternalFailure);
+ stdplus::fd::Fd&& read =
+ stdplus::fd::open(testFileName, stdplus::fd::OpenAccess::ReadWrite);
+ EXPECT_THROW(pass.verifyZero(size, read), InternalFailure);
+}
+
+TEST(Zeros, shortReadWritePass)
+{
+ std::string testFileName = "testfile_shortRead";
+
+ uint64_t size = 4096;
+ size_t shortSize = 128;
+ Zero pass(testFileName);
+ auto shortData = std::vector<std::byte>(shortSize, std::byte{0});
+ auto restOfData =
+ std::vector<std::byte>(size - shortSize * 3, std::byte{0});
+ stdplus::fd::FdMock mock;
+
+ // test write zeros with short blocks
+ EXPECT_CALL(mock, write(_))
+ .WillOnce(Return(shortData))
+ .WillOnce(Return(shortData))
+ .WillOnce(Return(restOfData))
+ .WillOnce(Return(shortData));
+
+ EXPECT_NO_THROW(pass.writeZero(size, mock));
+
+ // test read zeros with short blocks
+ EXPECT_CALL(mock, read(_))
+ .WillOnce(Return(shortData))
+ .WillOnce(Return(shortData))
+ .WillOnce(Return(restOfData))
+ .WillOnce(Return(shortData));
+
+ EXPECT_NO_THROW(pass.verifyZero(size, mock));
+}
+
+TEST(Zeros, shortReadWriteFail)
+{
+ std::string testFileName = "testfile_shortRead";
+
+ uint64_t size = 4096;
+ size_t shortSize = 128;
+ Zero tryZero(testFileName);
+ auto shortData = std::vector<std::byte>(shortSize, std::byte{0});
+ auto restOfData =
+ std::vector<std::byte>(size - shortSize * 3, std::byte{0});
+ // open the file and write none zero to it
+
+ stdplus::fd::FdMock mock;
+
+ // test writes
+ EXPECT_CALL(mock, write(_))
+ .WillOnce(Return(shortData))
+ .WillOnce(Return(shortData))
+ .WillOnce(Return(restOfData))
+ .WillOnce(Return(restOfData)); // return too much data!
+
+ EXPECT_THROW(tryZero.writeZero(size, mock), InternalFailure);
+
+ // test reads
+ EXPECT_CALL(mock, read(_))
+ .WillOnce(Return(shortData))
+ .WillOnce(Return(shortData))
+ .WillOnce(Return(restOfData))
+ .WillOnce(Return(restOfData)); // return too much data!
+
+ EXPECT_THROW(tryZero.verifyZero(size, mock), InternalFailure);
+}
+
+TEST(Zeros, driveIsSmaller)
+{
+ std::string testFileName = "testfile_driveIsSmaller";
+
+ uint64_t size = 4096;
+ Zero tryZero(testFileName);
+
+ stdplus::fd::FdMock mocks;
+ testing::InSequence s;
+
+ // test writes
+ EXPECT_CALL(mocks, write(_))
+ .Times(33)
+ .WillRepeatedly(Return(std::vector<std::byte>{}))
+ .RetiresOnSaturation();
+ EXPECT_THROW(tryZero.writeZero(size, mocks), InternalFailure);
+ // test reads
+ EXPECT_CALL(mocks, read(_))
+ .Times(33)
+ .WillRepeatedly(Return(std::vector<std::byte>{}))
+ .RetiresOnSaturation();
+ EXPECT_THROW(tryZero.verifyZero(size, mocks), InternalFailure);
}
} // namespace estoraged_test