bmc: add ActionPack notion to bundle actions
Each firmware type will provide its own set of action implementations
for each step, preparation, verification, and update.
Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: Id6409ac356a74e9094272b37709861e2a33d9862
diff --git a/bmc/firmware_handler.cpp b/bmc/firmware_handler.cpp
index 855c3fa..d193a4d 100644
--- a/bmc/firmware_handler.cpp
+++ b/bmc/firmware_handler.cpp
@@ -40,10 +40,7 @@
std::unique_ptr<blobs::GenericBlobInterface>
FirmwareBlobHandler::CreateFirmwareBlobHandler(
const std::vector<HandlerPack>& firmwares,
- const std::vector<DataHandlerPack>& transports,
- std::unique_ptr<TriggerableActionInterface> preparation,
- std::unique_ptr<TriggerableActionInterface> verification,
- std::unique_ptr<TriggerableActionInterface> update)
+ const std::vector<DataHandlerPack>& transports, ActionMap&& actionPacks)
{
/* There must be at least one. */
if (!firmwares.size())
@@ -75,8 +72,7 @@
}
return std::make_unique<FirmwareBlobHandler>(
- firmwares, blobs, transports, bitmask, std::move(preparation),
- std::move(verification), std::move(update));
+ firmwares, blobs, transports, bitmask, std::move(actionPacks));
}
/* Check if the path is in our supported list (or active list). */
@@ -172,6 +168,7 @@
ActionStatus FirmwareBlobHandler::getActionStatus()
{
ActionStatus value = ActionStatus::unknown;
+ auto* pack = getActionPack();
switch (state)
{
@@ -179,7 +176,13 @@
value = ActionStatus::unknown;
break;
case UpdateState::verificationStarted:
- value = verification->status();
+ /* If we got here, there must be data AND a hash, not just a hash,
+ * therefore pack will be known. */
+ if (!pack)
+ {
+ break;
+ }
+ value = pack->verification->status();
lastVerificationStatus = value;
break;
case UpdateState::verificationCompleted:
@@ -189,7 +192,11 @@
value = ActionStatus::unknown;
break;
case UpdateState::updateStarted:
- value = update->status();
+ if (!pack)
+ {
+ break;
+ }
+ value = pack->update->status();
lastUpdateStatus = value;
break;
case UpdateState::updateCompleted:
@@ -340,6 +347,9 @@
switch (state)
{
+ case UpdateState::notYetStarted:
+ /* Only hashBlobId and firmware BlobIds present. */
+ break;
case UpdateState::uploadInProgress:
/* Unreachable code because if it's started a file is open. */
break;
@@ -391,6 +401,35 @@
break;
}
+ /* To support multiple firmware options, we need to make sure they're
+ * opening the one they already opened during this update sequence, or it's
+ * the first time they're opening it.
+ */
+ if (path != hashBlobId)
+ {
+ /* If they're not opening the hashBlobId they must be opening a firmware
+ * handler.
+ */
+ if (openedFirmwareType.empty())
+ {
+ /* First time for this sequence. */
+ openedFirmwareType = path;
+ }
+ else
+ {
+ if (openedFirmwareType != path)
+ {
+ /* Previously, in this sequence they opened /flash/image, and
+ * now they're opening /flash/bios without finishing out
+ * /flash/image (for example).
+ */
+ std::fprintf(stderr, "Trying to open alternate firmware while "
+ "unfinished with other firmware.\n");
+ return false;
+ }
+ }
+ }
+
/* There are two abstractions at play, how you get the data and how you
* handle that data. such that, whether the data comes from the PCI bridge
* or LPC bridge is not connected to whether the data goes into a static
@@ -731,8 +770,12 @@
/* Store this transition logic here instead of ::open() */
if (!preparationTriggered)
{
- preparation->trigger();
- preparationTriggered = true;
+ auto* pack = getActionPack();
+ if (pack)
+ {
+ pack->preparation->trigger();
+ preparationTriggered = true;
+ }
}
}
}
@@ -763,17 +806,28 @@
removeBlobId(activeImageBlobId);
removeBlobId(activeHashBlobId);
+ openedFirmwareType = "";
changeState(UpdateState::notYetStarted);
}
void FirmwareBlobHandler::abortVerification()
{
- verification->abort();
+ auto* pack = getActionPack();
+ if (pack)
+ {
+ pack->verification->abort();
+ }
}
bool FirmwareBlobHandler::triggerVerification()
{
- bool result = verification->trigger();
+ auto* pack = getActionPack();
+ if (!pack)
+ {
+ return false;
+ }
+
+ bool result = pack->verification->trigger();
if (result)
{
changeState(UpdateState::verificationStarted);
@@ -784,12 +838,22 @@
void FirmwareBlobHandler::abortUpdate()
{
- update->abort();
+ auto* pack = getActionPack();
+ if (pack)
+ {
+ pack->update->abort();
+ }
}
bool FirmwareBlobHandler::triggerUpdate()
{
- bool result = update->trigger();
+ auto* pack = getActionPack();
+ if (!pack)
+ {
+ return false;
+ }
+
+ bool result = pack->update->trigger();
if (result)
{
changeState(UpdateState::updateStarted);
diff --git a/bmc/firmware_handler.hpp b/bmc/firmware_handler.hpp
index 3dd4978..727c68e 100644
--- a/bmc/firmware_handler.hpp
+++ b/bmc/firmware_handler.hpp
@@ -13,12 +13,31 @@
#include <map>
#include <memory>
#include <string>
+#include <unordered_map>
#include <vector>
namespace ipmi_flash
{
/**
+ * Given a firmware name, provide a set of triggerable action interfaces
+ * associated with that firmware type.
+ */
+struct ActionPack
+{
+ /** The name of the action pack, something like image, or tarball, or bios.
+ * The firmware blob id is parsed to pull the "filename" portion from the
+ * path, and matched against the key to a map of these.
+ */
+ std::unique_ptr<TriggerableActionInterface> preparation;
+ std::unique_ptr<TriggerableActionInterface> verification;
+ std::unique_ptr<TriggerableActionInterface> update;
+};
+
+using ActionMap =
+ std::unordered_map<std::string, std::unique_ptr<ipmi_flash::ActionPack>>;
+
+/**
* Representation of a session, includes how to read/write data.
*/
struct Session
@@ -90,9 +109,7 @@
CreateFirmwareBlobHandler(
const std::vector<HandlerPack>& firmwares,
const std::vector<DataHandlerPack>& transports,
- std::unique_ptr<TriggerableActionInterface> preparation,
- std::unique_ptr<TriggerableActionInterface> verification,
- std::unique_ptr<TriggerableActionInterface> update);
+ ActionMap&& actionPacks);
/**
* Create a FirmwareBlobHandler.
@@ -104,19 +121,15 @@
* @param[in] verification - pointer to object for triggering verification
* @param[in] update - point to object for triggering the update
*/
- FirmwareBlobHandler(
- const std::vector<HandlerPack>& firmwares,
- const std::vector<std::string>& blobs,
- const std::vector<DataHandlerPack>& transports, std::uint16_t bitmask,
- std::unique_ptr<TriggerableActionInterface> preparation,
- std::unique_ptr<TriggerableActionInterface> verification,
- std::unique_ptr<TriggerableActionInterface> update) :
+ FirmwareBlobHandler(const std::vector<HandlerPack>& firmwares,
+ const std::vector<std::string>& blobs,
+ const std::vector<DataHandlerPack>& transports,
+ std::uint16_t bitmask, ActionMap&& actionPacks) :
handlers(firmwares),
blobIDs(blobs), transports(transports), bitmask(bitmask),
activeImage(activeImageBlobId), activeHash(activeHashBlobId),
verifyImage(verifyBlobId), updateImage(updateBlobId), lookup(),
- state(UpdateState::notYetStarted), preparation(std::move(preparation)),
- verification(std::move(verification)), update(std::move(update))
+ state(UpdateState::notYetStarted), actionPacks(std::move(actionPacks))
{
}
~FirmwareBlobHandler() = default;
@@ -159,6 +172,27 @@
void changeState(UpdateState next);
private:
+ /**
+ * Given the current session type, grab the ActionPack (likely will be
+ * worked into the Session for lookup).
+ */
+ ActionPack* getActionPack()
+ {
+ if (openedFirmwareType.empty())
+ {
+ /* No firmware type has been opened, but we're triggering
+ * verification, or preparing. This can happen if they open the hash
+ * before the image, which is possible.
+ */
+ return nullptr;
+ }
+
+ /* TODO: Once the actionPacks and supportedFirmwares are merged this'll
+ * be less dangerous
+ */
+ return actionPacks[openedFirmwareType].get();
+ }
+
void addBlobId(const std::string& blob)
{
auto blobIdMatch = std::find_if(
@@ -209,15 +243,14 @@
/** The firmware update state. */
UpdateState state;
+ /** Track what firmware blobid they opened to start this sequence. */
+ std::string openedFirmwareType;
+
/* preparation is triggered once we go into uploadInProgress(), but only
* once per full cycle, going back to notYetStarted resets this.
*/
bool preparationTriggered = false;
- std::unique_ptr<TriggerableActionInterface> preparation;
-
- std::unique_ptr<TriggerableActionInterface> verification;
-
- std::unique_ptr<TriggerableActionInterface> update;
+ ActionMap actionPacks;
/** Temporary variable to track whether a blob is open. */
bool fileOpen = false;
diff --git a/bmc/main.cpp b/bmc/main.cpp
index ce2ca9f..d89369c 100644
--- a/bmc/main.cpp
+++ b/bmc/main.cpp
@@ -34,6 +34,7 @@
#include <memory>
#include <phosphor-logging/log.hpp>
#include <sdbusplus/bus.hpp>
+#include <unordered_map>
namespace ipmi_flash
{
@@ -119,9 +120,28 @@
sdbusplus::bus::new_default(), VERIFY_STATUS_FILENAME,
VERIFY_DBUS_SERVICE);
+ ipmi_flash::ActionMap actionPacks = {};
+
+ /* TODO: for bios should the name be, bios or /flash/bios?, these are
+ * /flash/... and it simplifies a few other things later (open/etc)
+ */
+ std::string bmcName;
+#ifdef ENABLE_STATIC_LAYOUT
+ bmcName = ipmi_flash::staticLayoutBlobId;
+#endif
+#ifdef ENABLE_TARBALL_UBI
+ bmcName = ipmi_flash::ubiTarballBlobId;
+#endif
+
+ auto bmcPack = std::make_unique<ipmi_flash::ActionPack>();
+ bmcPack->preparation = std::move(prepare);
+ bmcPack->verification = std::move(verifier);
+ bmcPack->update = std::move(updater);
+ actionPacks[bmcName] = std::move(bmcPack);
+
auto handler = ipmi_flash::FirmwareBlobHandler::CreateFirmwareBlobHandler(
ipmi_flash::supportedFirmware, ipmi_flash::supportedTransports,
- std::move(prepare), std::move(verifier), std::move(updater));
+ std::move(actionPacks));
if (!handler)
{
diff --git a/bmc/test/Makefile.am b/bmc/test/Makefile.am
index 1dcc3ae..feaee78 100644
--- a/bmc/test/Makefile.am
+++ b/bmc/test/Makefile.am
@@ -40,7 +40,8 @@
firmware_state_updatepending_unittest \
firmware_state_updatestarted_unittest \
firmware_state_updatecompleted_unittest \
- firmware_state_notyetstarted_tarball_unittest
+ firmware_state_notyetstarted_tarball_unittest \
+ firmware_multiplebundle_unittest
TESTS = $(check_PROGRAMS)
@@ -100,3 +101,6 @@
firmware_state_notyetstarted_tarball_unittest_SOURCES = firmware_state_notyetstarted_tarball_unittest.cpp
firmware_state_notyetstarted_tarball_unittest_LDADD = $(top_builddir)/bmc/libfirmwareblob_common.la
+
+firmware_multiplebundle_unittest_SOURCES = firmware_multiplebundle_unittest.cpp
+firmware_multiplebundle_unittest_LDADD = $(top_builddir)/bmc/libfirmwareblob_common.la
diff --git a/bmc/test/firmware_canhandle_unittest.cpp b/bmc/test/firmware_canhandle_unittest.cpp
index 13bf943..80bcce9 100644
--- a/bmc/test/firmware_canhandle_unittest.cpp
+++ b/bmc/test/firmware_canhandle_unittest.cpp
@@ -34,8 +34,7 @@
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock(),
- CreateTriggerMock());
+ blobs, data, std::move(CreateActionMap("asdf")));
for (const auto& item : items)
{
diff --git a/bmc/test/firmware_commit_unittest.cpp b/bmc/test/firmware_commit_unittest.cpp
index a6ab172..16d9881 100644
--- a/bmc/test/firmware_commit_unittest.cpp
+++ b/bmc/test/firmware_commit_unittest.cpp
@@ -12,6 +12,8 @@
namespace ipmi_flash
{
+namespace
+{
using ::testing::_;
using ::testing::IsNull;
using ::testing::NotNull;
@@ -50,8 +52,7 @@
std::make_unique<StrictMock<TriggerMock>>();
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), std::move(verifyMock),
- CreateTriggerMock());
+ blobs, data, std::move(CreateActionMap("asdf")));
EXPECT_CALL(imageMock2, open("asdf")).WillOnce(Return(true));
@@ -72,8 +73,7 @@
std::make_unique<StrictMock<TriggerMock>>();
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), std::move(verifyMock),
- CreateTriggerMock());
+ blobs, data, std::move(CreateActionMap("asdf")));
EXPECT_CALL(imageMock1, open(StrEq(hashBlobId))).WillOnce(Return(true));
@@ -84,4 +84,5 @@
EXPECT_FALSE(handler->commit(0, {}));
}
+} // namespace
} // namespace ipmi_flash
diff --git a/bmc/test/firmware_createhandler_unittest.cpp b/bmc/test/firmware_createhandler_unittest.cpp
index d7b99a6..2800f2e 100644
--- a/bmc/test/firmware_createhandler_unittest.cpp
+++ b/bmc/test/firmware_createhandler_unittest.cpp
@@ -9,6 +9,9 @@
namespace ipmi_flash
{
+namespace
+{
+
using ::testing::Return;
using ::testing::StrEq;
using ::testing::StrictMock;
@@ -32,11 +35,11 @@
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock(),
- CreateTriggerMock());
+ blobs, data, std::move(CreateActionMap("abcd")));
// EXPECT_EQ(handler, nullptr);
EXPECT_FALSE(handler == nullptr);
}
+} // namespace
} // namespace ipmi_flash
diff --git a/bmc/test/firmware_handler_unittest.cpp b/bmc/test/firmware_handler_unittest.cpp
index 7ab224d..29df507 100644
--- a/bmc/test/firmware_handler_unittest.cpp
+++ b/bmc/test/firmware_handler_unittest.cpp
@@ -23,8 +23,7 @@
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- {}, data, CreateTriggerMock(), CreateTriggerMock(),
- CreateTriggerMock());
+ {}, data, std::move(CreateActionMap("abcd")));
EXPECT_EQ(handler, nullptr);
}
TEST(FirmwareHandlerTest, CreateEmptyDataHandlerListFails)
@@ -37,8 +36,7 @@
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, {}, CreateTriggerMock(), CreateTriggerMock(),
- CreateTriggerMock());
+ blobs, {}, std::move(CreateActionMap("asdf")));
EXPECT_EQ(handler, nullptr);
}
TEST(FirmwareHandlerTest, VerifyHashRequiredForHappiness)
@@ -54,15 +52,14 @@
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock(),
- CreateTriggerMock());
+ blobs, data, std::move(CreateActionMap("asdf")));
EXPECT_EQ(handler, nullptr);
blobs.push_back({hashBlobId, &imageMock});
handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock(),
- CreateTriggerMock());
+ blobs, data, std::move(CreateActionMap("asdf")));
+
auto result = handler->getBlobIds();
std::vector<std::string> expectedBlobs = {"asdf", hashBlobId};
EXPECT_THAT(result, UnorderedElementsAreArray(expectedBlobs));
diff --git a/bmc/test/firmware_multiplebundle_unittest.cpp b/bmc/test/firmware_multiplebundle_unittest.cpp
new file mode 100644
index 0000000..b8d738d
--- /dev/null
+++ b/bmc/test/firmware_multiplebundle_unittest.cpp
@@ -0,0 +1,155 @@
+/* The goal of these tests is to verify that once a host-client has started the
+ * process with one blob bundle, they cannot pivot to upload data to another.
+ *
+ * This prevent someone from starting to upload a BMC firmware, and then midway
+ * through start uploading a BIOS image.
+ */
+#include "firmware_handler.hpp"
+#include "flags.hpp"
+#include "image_mock.hpp"
+#include "status.hpp"
+#include "triggerable_mock.hpp"
+#include "util.hpp"
+
+#include <memory>
+#include <string>
+#include <unordered_map>
+#include <vector>
+
+#include <gtest/gtest.h>
+
+namespace ipmi_flash
+{
+namespace
+{
+
+using ::testing::Return;
+
+class IpmiOnlyTwoFirmwaresTest : public ::testing::Test
+{
+ protected:
+ void SetUp() override
+ {
+ blobs = {
+ {hashBlobId, &hashImageMock},
+ {staticLayoutBlobId, &staticImageMock},
+ {biosId, &biosImageMock},
+ };
+
+ std::unique_ptr<TriggerableActionInterface> bmcPrepareMock =
+ std::make_unique<TriggerMock>();
+ bmcPrepareMockPtr =
+ reinterpret_cast<TriggerMock*>(bmcPrepareMock.get());
+
+ std::unique_ptr<TriggerableActionInterface> bmcVerifyMock =
+ std::make_unique<TriggerMock>();
+ bmcVerifyMockPtr = reinterpret_cast<TriggerMock*>(bmcVerifyMock.get());
+
+ std::unique_ptr<TriggerableActionInterface> bmcUpdateMock =
+ std::make_unique<TriggerMock>();
+ bmcUpdateMockPtr = reinterpret_cast<TriggerMock*>(bmcUpdateMock.get());
+
+ std::unique_ptr<TriggerableActionInterface> biosPrepareMock =
+ std::make_unique<TriggerMock>();
+ biosPrepareMockPtr =
+ reinterpret_cast<TriggerMock*>(biosPrepareMock.get());
+
+ std::unique_ptr<TriggerableActionInterface> biosVerifyMock =
+ std::make_unique<TriggerMock>();
+ biosVerifyMockPtr =
+ reinterpret_cast<TriggerMock*>(biosVerifyMock.get());
+
+ std::unique_ptr<TriggerableActionInterface> biosUpdateMock =
+ std::make_unique<TriggerMock>();
+ biosUpdateMockPtr =
+ reinterpret_cast<TriggerMock*>(biosUpdateMock.get());
+
+ ActionMap packs;
+
+ std::unique_ptr<ActionPack> bmcPack = std::make_unique<ActionPack>();
+ bmcPack->preparation = std::move(bmcPrepareMock);
+ bmcPack->verification = std::move(bmcVerifyMock);
+ bmcPack->update = std::move(bmcUpdateMock);
+
+ std::unique_ptr<ActionPack> biosPack = std::make_unique<ActionPack>();
+ biosPack->preparation = std::move(biosPrepareMock);
+ biosPack->verification = std::move(biosVerifyMock);
+ biosPack->update = std::move(biosUpdateMock);
+
+ packs[staticLayoutBlobId] = std::move(bmcPack);
+ packs[biosId] = std::move(biosPack);
+
+ handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
+ blobs, data, std::move(packs));
+ }
+
+ void expectedState(FirmwareBlobHandler::UpdateState state)
+ {
+ auto realHandler = dynamic_cast<FirmwareBlobHandler*>(handler.get());
+ EXPECT_EQ(state, realHandler->getCurrentState());
+ }
+
+ ImageHandlerMock hashImageMock, staticImageMock, biosImageMock;
+
+ std::vector<HandlerPack> blobs;
+ std::vector<DataHandlerPack> data = {
+ {FirmwareFlags::UpdateFlags::ipmi, nullptr}};
+
+ std::unique_ptr<blobs::GenericBlobInterface> handler;
+
+ TriggerMock *bmcPrepareMockPtr, *bmcVerifyMockPtr, *bmcUpdateMockPtr;
+ TriggerMock *biosPrepareMockPtr, *biosVerifyMockPtr, *biosUpdateMockPtr;
+
+ std::uint16_t session = 1;
+ std::uint16_t flags =
+ blobs::OpenFlags::write | FirmwareFlags::UpdateFlags::ipmi;
+
+ const std::string biosId = "/flash/bios";
+};
+
+TEST_F(IpmiOnlyTwoFirmwaresTest, OpeningBiosAfterBlobFails)
+{
+ /* You can only have one file open at a time, and the first firmware file
+ * you open locks it down
+ */
+ EXPECT_CALL(staticImageMock, open(staticLayoutBlobId))
+ .WillOnce(Return(true));
+ EXPECT_CALL(*bmcPrepareMockPtr, trigger()).WillOnce(Return(true));
+
+ EXPECT_TRUE(handler->open(session, flags, staticLayoutBlobId));
+ expectedState(FirmwareBlobHandler::UpdateState::uploadInProgress);
+
+ EXPECT_CALL(staticImageMock, close()).WillOnce(Return());
+ handler->close(session);
+
+ expectedState(FirmwareBlobHandler::UpdateState::verificationPending);
+
+ EXPECT_CALL(biosImageMock, open(biosId)).Times(0);
+ EXPECT_FALSE(handler->open(session, flags, biosId));
+}
+
+TEST_F(IpmiOnlyTwoFirmwaresTest, OpeningHashBeforeBiosSucceeds)
+{
+ /* Opening the hash blob does nothing special in this regard. */
+ EXPECT_CALL(hashImageMock, open(hashBlobId)).WillOnce(Return(true));
+ EXPECT_TRUE(handler->open(session, flags, hashBlobId));
+
+ expectedState(FirmwareBlobHandler::UpdateState::uploadInProgress);
+
+ EXPECT_CALL(hashImageMock, close()).WillOnce(Return());
+ handler->close(session);
+
+ expectedState(FirmwareBlobHandler::UpdateState::verificationPending);
+ ASSERT_FALSE(handler->canHandleBlob(verifyBlobId));
+
+ EXPECT_CALL(biosImageMock, open(biosId)).WillOnce(Return(true));
+ EXPECT_TRUE(handler->open(session, flags, biosId));
+
+ expectedState(FirmwareBlobHandler::UpdateState::uploadInProgress);
+
+ EXPECT_CALL(biosImageMock, close()).WillOnce(Return());
+ handler->close(session);
+}
+
+} // namespace
+} // namespace ipmi_flash
diff --git a/bmc/test/firmware_stat_unittest.cpp b/bmc/test/firmware_stat_unittest.cpp
index 021dca8..b71d5c7 100644
--- a/bmc/test/firmware_stat_unittest.cpp
+++ b/bmc/test/firmware_stat_unittest.cpp
@@ -30,8 +30,7 @@
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock(),
- CreateTriggerMock());
+ blobs, data, std::move(CreateActionMap("asdf")));
blobs::BlobMeta meta;
EXPECT_TRUE(handler->stat("asdf", &meta));
diff --git a/bmc/test/firmware_state_notyetstarted_tarball_unittest.cpp b/bmc/test/firmware_state_notyetstarted_tarball_unittest.cpp
index bdf6046..3ad118c 100644
--- a/bmc/test/firmware_state_notyetstarted_tarball_unittest.cpp
+++ b/bmc/test/firmware_state_notyetstarted_tarball_unittest.cpp
@@ -35,9 +35,16 @@
std::make_unique<TriggerMock>();
updateMockPtr = reinterpret_cast<TriggerMock*>(updateMock.get());
+ std::unique_ptr<ActionPack> actionPack = std::make_unique<ActionPack>();
+ actionPack->preparation = CreateTriggerMock();
+ actionPack->verification = std::move(verifyMock);
+ actionPack->update = std::move(updateMock);
+
+ ActionMap packs;
+ packs[ubiTarballBlobId] = std::move(actionPack);
+
handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), std::move(verifyMock),
- std::move(updateMock));
+ blobs, data, std::move(packs));
}
void expectedState(FirmwareBlobHandler::UpdateState state)
diff --git a/bmc/test/firmware_state_notyetstarted_unittest.cpp b/bmc/test/firmware_state_notyetstarted_unittest.cpp
index 8681064..3c0eb67 100644
--- a/bmc/test/firmware_state_notyetstarted_unittest.cpp
+++ b/bmc/test/firmware_state_notyetstarted_unittest.cpp
@@ -105,7 +105,10 @@
TEST_F(FirmwareHandlerNotYetStartedTest, OpenHashFileVerifyStateChange)
{
EXPECT_CALL(imageMock, open(hashBlobId)).WillOnce(Return(true));
- EXPECT_CALL(*prepareMockPtr, trigger()).WillOnce(Return(true));
+ /* Opening the hash blob id doesn't trigger a preparation, only a firmware
+ * blob.
+ */
+ EXPECT_CALL(*prepareMockPtr, trigger()).Times(0);
EXPECT_TRUE(handler->open(session, flags, hashBlobId));
diff --git a/bmc/test/firmware_state_verificationstarted_unittest.cpp b/bmc/test/firmware_state_verificationstarted_unittest.cpp
index b183db0..8c0d476 100644
--- a/bmc/test/firmware_state_verificationstarted_unittest.cpp
+++ b/bmc/test/firmware_state_verificationstarted_unittest.cpp
@@ -173,7 +173,7 @@
TEST_F(FirmwareHandlerVerificationStartedTest, StatOnActiveHashReturnsFailure)
{
- getToVerificationStarted(hashBlobId);
+ getToVerificationStartedWitHashBlob();
ASSERT_TRUE(handler->canHandleBlob(activeHashBlobId));
blobs::BlobMeta meta;
diff --git a/bmc/test/firmware_unittest.hpp b/bmc/test/firmware_unittest.hpp
index 6b97289..129b940 100644
--- a/bmc/test/firmware_unittest.hpp
+++ b/bmc/test/firmware_unittest.hpp
@@ -8,6 +8,7 @@
#include <memory>
#include <string>
+#include <unordered_map>
#include <vector>
#include <gmock/gmock.h>
@@ -42,9 +43,16 @@
std::make_unique<TriggerMock>();
updateMockPtr = reinterpret_cast<TriggerMock*>(updateMock.get());
+ std::unique_ptr<ActionPack> actionPack = std::make_unique<ActionPack>();
+ actionPack->preparation = std::move(prepareMock);
+ actionPack->verification = std::move(verifyMock);
+ actionPack->update = std::move(updateMock);
+
+ ActionMap packs;
+ packs[staticLayoutBlobId] = std::move(actionPack);
+
handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, std::move(prepareMock), std::move(verifyMock),
- std::move(updateMock));
+ blobs, data, std::move(packs));
}
void expectedState(FirmwareBlobHandler::UpdateState state)
@@ -56,7 +64,10 @@
void openToInProgress(const std::string& blobId)
{
EXPECT_CALL(imageMock, open(blobId)).WillOnce(Return(true));
- EXPECT_CALL(*prepareMockPtr, trigger()).WillOnce(Return(true));
+ if (blobId != hashBlobId)
+ {
+ EXPECT_CALL(*prepareMockPtr, trigger()).WillOnce(Return(true));
+ }
EXPECT_TRUE(handler->open(session, flags, blobId));
expectedState(FirmwareBlobHandler::UpdateState::uploadInProgress);
}
@@ -81,6 +92,24 @@
expectedState(FirmwareBlobHandler::UpdateState::verificationStarted);
}
+ void getToVerificationStartedWitHashBlob()
+ {
+ /* Open both static and hash to check for activeHashBlobId. */
+ getToVerificationPending(staticLayoutBlobId);
+
+ openToInProgress(hashBlobId);
+ EXPECT_CALL(imageMock, close()).WillRepeatedly(Return());
+ handler->close(session);
+ expectedState(FirmwareBlobHandler::UpdateState::verificationPending);
+
+ /* Now the hash is active AND the static image is active. */
+ EXPECT_TRUE(handler->open(session, flags, verifyBlobId));
+ EXPECT_CALL(*verifyMockPtr, trigger()).WillOnce(Return(true));
+
+ EXPECT_TRUE(handler->commit(session, {}));
+ expectedState(FirmwareBlobHandler::UpdateState::verificationStarted);
+ }
+
void getToVerificationCompleted(ActionStatus checkResponse)
{
getToVerificationStarted(staticLayoutBlobId);
@@ -151,8 +180,7 @@
{"asdf", &imageMock},
};
handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock(),
- CreateTriggerMock());
+ blobs, data, std::move(CreateActionMap("asdf")));
}
};
@@ -176,8 +204,7 @@
{FirmwareFlags::UpdateFlags::lpc, &dataMock},
};
handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock(),
- CreateTriggerMock());
+ blobs, data, std::move(CreateActionMap("asdf")));
}
};
diff --git a/bmc/test/triggerable_mock.hpp b/bmc/test/triggerable_mock.hpp
index c485d8e..8c83d52 100644
--- a/bmc/test/triggerable_mock.hpp
+++ b/bmc/test/triggerable_mock.hpp
@@ -1,7 +1,11 @@
#pragma once
+#include "firmware_handler.hpp"
#include "status.hpp"
+#include <memory>
+#include <string>
+
#include <gtest/gtest.h>
namespace ipmi_flash
@@ -21,4 +25,16 @@
return std::make_unique<TriggerMock>();
}
+ActionMap CreateActionMap(const std::string& blobPath)
+{
+ std::unique_ptr<ActionPack> actionPack = std::make_unique<ActionPack>();
+ actionPack->preparation = std::move(CreateTriggerMock());
+ actionPack->verification = std::move(CreateTriggerMock());
+ actionPack->update = std::move(CreateTriggerMock());
+
+ ActionMap map;
+ map[blobPath] = std::move(actionPack);
+ return map;
+}
+
} // namespace ipmi_flash