bmc: firmware updatePending: commit(session)
Commit(updateBlobId) will trigger the update.
Now you can only trigger if you're in the correct state, therefore
delete obsolete unit-tests that didn't set up the correct starting
condition.
Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: If97d1db21269027dc61c9cd295f022499e949106
diff --git a/firmware_handler.cpp b/firmware_handler.cpp
index 15dbee8..f648d42 100644
--- a/firmware_handler.cpp
+++ b/firmware_handler.cpp
@@ -572,14 +572,21 @@
return false;
}
- /* You can only commit on the verifyBlodId */
- if (item->second->activePath != verifyBlobId)
+ /* You can only commit on the verifyBlodId or updateBlobId */
+ if (item->second->activePath != verifyBlobId &&
+ item->second->activePath != updateBlobId)
{
+ std::fprintf(stderr, "path: '%s' not expected for commit\n",
+ item->second->activePath.c_str());
return false;
}
switch (state)
{
+ case UpdateState::verificationPending:
+ /* Set state to committing. */
+ item->second->flags |= blobs::StateFlags::committing;
+ return triggerVerification();
case UpdateState::verificationStarted:
/* Calling repeatedly has no effect within an update process. */
return true;
@@ -587,10 +594,11 @@
/* Calling after the verification process has completed returns
* failure. */
return false;
- default:
- /* Set state to committing. */
+ case UpdateState::updatePending:
item->second->flags |= blobs::StateFlags::committing;
- return triggerVerification();
+ return triggerUpdate();
+ default:
+ return false;
}
}
@@ -698,4 +706,15 @@
return result;
}
+bool FirmwareBlobHandler::triggerUpdate()
+{
+ bool result = update->triggerUpdate();
+ if (result)
+ {
+ state = UpdateState::updateStarted;
+ }
+
+ return result;
+}
+
} // namespace ipmi_flash
diff --git a/firmware_handler.hpp b/firmware_handler.hpp
index bfae529..b2f5a64 100644
--- a/firmware_handler.hpp
+++ b/firmware_handler.hpp
@@ -169,6 +169,7 @@
bool expire(uint16_t session) override;
bool triggerVerification();
+ bool triggerUpdate();
/** Allow grabbing the current state. */
UpdateState getCurrentState() const
diff --git a/test/firmware_commit_unittest.cpp b/test/firmware_commit_unittest.cpp
index 87dba97..37e619e 100644
--- a/test/firmware_commit_unittest.cpp
+++ b/test/firmware_commit_unittest.cpp
@@ -83,43 +83,4 @@
EXPECT_FALSE(handler->commit(0, {}));
}
-TEST_F(FirmwareHandlerCommitTest, VerifyCommitAcceptedOnVerifyBlob)
-{
- /* Verify the verify blob lets you call this command, and it returns
- * success.
- */
- auto verifyMock = CreateVerifyMock();
- auto verifyMockPtr = reinterpret_cast<VerificationMock*>(verifyMock.get());
-
- auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, std::move(verifyMock), CreateUpdateMock());
-
- EXPECT_TRUE(handler->open(0, blobs::OpenFlags::write, verifyBlobId));
-
- EXPECT_CALL(*verifyMockPtr, triggerVerification())
- .WillRepeatedly(Return(true));
-
- EXPECT_TRUE(handler->commit(0, {}));
-}
-
-TEST_F(FirmwareHandlerCommitTest, VerifyCommitCanOnlyBeCalledOnceForEffect)
-{
- /* Verify you cannot call the commit() command once verification is
- * started, after which it will just return true.
- */
- auto verifyMock = CreateVerifyMock();
- auto verifyMockPtr = reinterpret_cast<VerificationMock*>(verifyMock.get());
-
- auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, std::move(verifyMock), CreateUpdateMock());
-
- EXPECT_TRUE(handler->open(0, blobs::OpenFlags::write, verifyBlobId));
-
- EXPECT_CALL(*verifyMockPtr, triggerVerification())
- .WillRepeatedly(Return(true));
-
- EXPECT_TRUE(handler->commit(0, {}));
- EXPECT_TRUE(handler->commit(0, {}));
-}
-
} // namespace ipmi_flash
diff --git a/test/firmware_state_updatepending_unittest.cpp b/test/firmware_state_updatepending_unittest.cpp
index eb9e437..7a1e480 100644
--- a/test/firmware_state_updatepending_unittest.cpp
+++ b/test/firmware_state_updatepending_unittest.cpp
@@ -237,11 +237,39 @@
}
/*
- * TODO: deleteBlob(blob)
+ * commit(session)
*/
+TEST_F(FirmwareHandlerUpdatePendingTest,
+ CommitOnUpdateBlobTriggersUpdateAndChangesState)
+{
+ /* Commit triggers the update mechanism (similarly for the verifyBlobId) and
+ * changes state to updateStarted.
+ */
+ getToUpdatePending();
+ EXPECT_TRUE(handler->open(session, flags, updateBlobId));
+ expectedState(FirmwareBlobHandler::UpdateState::updatePending);
+
+ EXPECT_CALL(*updateMockPtr, triggerUpdate()).WillOnce(Return(true));
+
+ EXPECT_TRUE(handler->commit(session, {}));
+ expectedState(FirmwareBlobHandler::UpdateState::updateStarted);
+}
+
+TEST_F(FirmwareHandlerUpdatePendingTest,
+ CommitOnUpdateBlobTriggersUpdateAndReturnsFailureDoesNotChangeState)
+{
+ getToUpdatePending();
+ EXPECT_TRUE(handler->open(session, flags, updateBlobId));
+ expectedState(FirmwareBlobHandler::UpdateState::updatePending);
+
+ EXPECT_CALL(*updateMockPtr, triggerUpdate()).WillOnce(Return(false));
+
+ EXPECT_FALSE(handler->commit(session, {}));
+ expectedState(FirmwareBlobHandler::UpdateState::updatePending);
+}
/*
- * commit(session)
+ * TODO: deleteBlob(blob)
*/
} // namespace
diff --git a/test/firmware_unittest.hpp b/test/firmware_unittest.hpp
index 2427b82..0aa7e12 100644
--- a/test/firmware_unittest.hpp
+++ b/test/firmware_unittest.hpp
@@ -29,8 +29,12 @@
std::make_unique<VerificationMock>();
verifyMockPtr = reinterpret_cast<VerificationMock*>(verifyMock.get());
+ std::unique_ptr<UpdateInterface> updateMock =
+ std::make_unique<UpdateMock>();
+ updateMockPtr = reinterpret_cast<UpdateMock*>(updateMock.get());
+
handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, std::move(verifyMock), CreateUpdateMock());
+ blobs, data, std::move(verifyMock), std::move(updateMock));
}
void expectedState(FirmwareBlobHandler::UpdateState state)
@@ -45,6 +49,7 @@
{FirmwareBlobHandler::UpdateFlags::ipmi, nullptr}};
std::unique_ptr<blobs::GenericBlobInterface> handler;
VerificationMock* verifyMockPtr;
+ UpdateMock* updateMockPtr;
};
class IpmiOnlyFirmwareTest : public ::testing::Test