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/pattern_test.cpp b/src/test/erase/pattern_test.cpp
index e0192a0..2e2ac81 100644
--- a/src/test/erase/pattern_test.cpp
+++ b/src/test/erase/pattern_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>
@@ -20,6 +21,9 @@
using estoraged::Pattern;
using sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure;
+using testing::_;
+using testing::Invoke;
+using testing::Return;
TEST(pattern, patternPass)
{
@@ -30,14 +34,22 @@
std::ios::out | std::ios::binary | std::ios::trunc);
testFile.close();
+ stdplus::fd::Fd&& writeFd =
+ stdplus::fd::open(testFileName, stdplus::fd::OpenAccess::WriteOnly);
+
Pattern pass(testFileName);
- EXPECT_NO_THROW(pass.writePattern(size));
- EXPECT_NO_THROW(pass.verifyPattern(size));
+ EXPECT_NO_THROW(pass.writePattern(size, writeFd));
+
+ stdplus::fd::Fd&& readFd =
+ stdplus::fd::open(testFileName, stdplus::fd::OpenAccess::ReadOnly);
+
+ EXPECT_NO_THROW(pass.verifyPattern(size, readFd));
}
/* This test that pattern writes the correct number of bytes even if
* size of the drive is not divisable by the block size
*/
+
TEST(pattern, patternNotDivisible)
{
std::string testFileName = "notDivisible";
@@ -48,9 +60,14 @@
std::ios::out | std::ios::binary | std::ios::trunc);
testFile.close();
+ stdplus::fd::Fd&& writeFd =
+ stdplus::fd::open(testFileName, stdplus::fd::OpenAccess::WriteOnly);
Pattern pass(testFileName);
- EXPECT_NO_THROW(pass.writePattern(size));
- EXPECT_NO_THROW(pass.verifyPattern(size));
+ EXPECT_NO_THROW(pass.writePattern(size, writeFd));
+
+ stdplus::fd::Fd&& readFd =
+ stdplus::fd::open(testFileName, stdplus::fd::OpenAccess::ReadOnly);
+ EXPECT_NO_THROW(pass.verifyPattern(size, readFd));
}
TEST(pattern, patternsDontMatch)
@@ -67,8 +84,140 @@
sizeof(dummyValue));
testFile.close();
- EXPECT_NO_THROW(pass.writePattern(size - sizeof(dummyValue)));
- EXPECT_THROW(pass.verifyPattern(size), InternalFailure);
+ stdplus::fd::Fd&& writeFd =
+ stdplus::fd::open(testFileName, stdplus::fd::OpenAccess::WriteOnly);
+
+ EXPECT_NO_THROW(pass.writePattern(size - sizeof(dummyValue), writeFd));
+
+ stdplus::fd::Fd&& readFd =
+ stdplus::fd::open(testFileName, stdplus::fd::OpenAccess::ReadOnly);
+
+ EXPECT_THROW(pass.verifyPattern(size, readFd), InternalFailure);
+}
+
+uint64_t size = 4096;
+size_t shortSize = 128;
+static auto shortData1 = std::vector<std::byte>(shortSize);
+static auto shortData2 = std::vector<std::byte>(shortSize);
+static auto restOfData = std::vector<std::byte>(size - shortSize * 2);
+
+TEST(pattern, shortReadWritePass)
+{
+ std::string testFileName = "testfile_shortRead";
+
+ uint64_t size = 4096;
+ Pattern pass(testFileName);
+ stdplus::fd::FdMock mock;
+
+ testing::Sequence s;
+
+ // test write pattern with short blocks
+ EXPECT_CALL(mock, write(_))
+ .WillOnce(Invoke([](std::span<const std::byte> x) {
+ std::copy_n(x.begin(), shortData1.size(), shortData1.begin());
+ return shortData1;
+ }))
+ .RetiresOnSaturation();
+ EXPECT_CALL(mock, write(_))
+ .WillOnce(Invoke([](std::span<const std::byte> x) {
+ std::copy_n(x.begin(), restOfData.size(), restOfData.begin());
+ return restOfData;
+ }))
+ .RetiresOnSaturation();
+ EXPECT_CALL(mock, write(_))
+ .WillOnce(Invoke([](std::span<const std::byte> x) {
+ std::copy_n(x.begin(), shortData2.size(), shortData2.begin());
+ return shortData2;
+ }))
+ .RetiresOnSaturation();
+
+ // test read pattern
+ EXPECT_CALL(mock, read(_))
+ .WillOnce(Invoke([](nonstd::span<std::byte> x) {
+ std::copy_n(shortData1.begin(), shortData1.size(), x.data());
+ nonstd::span ret(shortData1);
+ return ret;
+ }))
+ .RetiresOnSaturation();
+
+ EXPECT_CALL(mock, read(_))
+ .WillOnce(Invoke([](nonstd::span<std::byte> x) {
+ std::copy_n(restOfData.begin(), restOfData.size(), x.data());
+ nonstd::span ret(restOfData);
+ return ret;
+ }))
+ .RetiresOnSaturation();
+
+ EXPECT_CALL(mock, read(_))
+ .WillOnce(Invoke([](nonstd::span<std::byte> x) {
+ std::copy_n(shortData2.begin(), shortData2.size(), x.data());
+ nonstd::span ret(shortData2);
+ return ret;
+ }))
+ .RetiresOnSaturation();
+
+ EXPECT_NO_THROW(pass.writePattern(size, mock));
+ EXPECT_NO_THROW(pass.verifyPattern(size, mock));
+}
+
+TEST(Zeros, shortReadWriteFail)
+{
+ std::string testFileName = "testfile_shortRead";
+
+ uint64_t size = 4096;
+ size_t shortSize = 128;
+ Pattern tryPattern(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 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(tryPattern.writePattern(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(tryPattern.verifyPattern(size, mock), InternalFailure);
+}
+
+TEST(pattern, driveIsSmaller)
+{
+ std::string testFileName = "testfile_driveIsSmaller";
+
+ uint64_t size = 4096;
+ Pattern tryPattern(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(tryPattern.writePattern(size, mocks), InternalFailure);
+
+ // test reads
+ EXPECT_CALL(mocks, read(_))
+ .Times(33)
+ .WillRepeatedly(Return(std::vector<std::byte>{}))
+ .RetiresOnSaturation();
+
+ EXPECT_THROW(tryPattern.verifyPattern(size, mocks), InternalFailure);
}
} // namespace estoraged_test