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

15 files changed, 390 insertions, 62 deletions

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 @@ namespace ipmi_flash
 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 @@ std::unique_ptr<blobs::GenericBlobInterface>
     }
 
     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 @@ bool FirmwareBlobHandler::stat(const std::string& path, blobs::BlobMeta* meta)
 ActionStatus FirmwareBlobHandler::getActionStatus()
 {
     ActionStatus value = ActionStatus::unknown;
+    auto* pack = getActionPack();
 
     switch (state)
     {
@@ -179,7 +176,13 @@ ActionStatus FirmwareBlobHandler::getActionStatus()
             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 @@ ActionStatus FirmwareBlobHandler::getActionStatus()
             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 @@ bool FirmwareBlobHandler::open(uint16_t session, uint16_t flags,
 
     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 @@ bool FirmwareBlobHandler::open(uint16_t session, uint16_t flags,
             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 @@ void FirmwareBlobHandler::changeState(UpdateState next)
         /* 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 @@ void FirmwareBlobHandler::abortProcess()
     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 @@ bool FirmwareBlobHandler::triggerVerification()
 
 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 @@ class FirmwareBlobHandler : public blobs::GenericBlobInterface
         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 @@ class FirmwareBlobHandler : public blobs::GenericBlobInterface
      * @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 @@ class FirmwareBlobHandler : public blobs::GenericBlobInterface
     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 @@ class FirmwareBlobHandler : public blobs::GenericBlobInterface
     /** 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 @@ std::unique_ptr<blobs::GenericBlobInterface> createHandler()
         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 @@ check_PROGRAMS = \
 	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_updatecompleted_unittest_LDADD = $(top_builddir)/bmc/libfirmwareb
 
 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 @@ TEST(FirmwareHandlerCanHandleTest, VerifyItemsInListAreOk)
     };
 
     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 @@ TEST_F(FirmwareHandlerCommitTest, VerifyCannotCommitOnFlashImage)
         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 @@ TEST_F(FirmwareHandlerCommitTest, VerifyCannotCommitOnHashFile)
         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 @@ TEST_F(FirmwareHandlerCommitTest, VerifyCannotCommitOnHashFile)
     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 @@ TEST(FirmwareHandlerBlobTest, VerifyFirmwareCounts)
     };
 
     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 @@ TEST(FirmwareHandlerTest, CreateEmptyListVerifyFails)
     };
 
     auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
-        {}, data, CreateTriggerMock(), CreateTriggerMock(),
-        CreateTriggerMock());
+        {}, data, std::move(CreateActionMap("abcd")));
     EXPECT_EQ(handler, nullptr);
 }
 TEST(FirmwareHandlerTest, CreateEmptyDataHandlerListFails)
@@ -37,8 +36,7 @@ TEST(FirmwareHandlerTest, CreateEmptyDataHandlerListFails)
     };
 
     auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
-        blobs, {}, CreateTriggerMock(), CreateTriggerMock(),
-        CreateTriggerMock());
+        blobs, {}, std::move(CreateActionMap("asdf")));
     EXPECT_EQ(handler, nullptr);
 }
 TEST(FirmwareHandlerTest, VerifyHashRequiredForHappiness)
@@ -54,15 +52,14 @@ TEST(FirmwareHandlerTest, VerifyHashRequiredForHappiness)
     };
 
     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 @@ TEST(FirmwareHandlerStatTest, StatOnInactiveBlobIDReturnsTransport)
     };
 
     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 @@ class FirmwareHandlerNotYetStartedUbitTest : public ::testing::Test
             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, OpenStaticImageFileVerifyStateChange)
 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, StatOnActiveImageReturnsFailure)
 
 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 @@ class IpmiOnlyFirmwareStaticTest : public ::testing::Test
             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 @@ class IpmiOnlyFirmwareStaticTest : public ::testing::Test
     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 @@ class IpmiOnlyFirmwareStaticTest : public ::testing::Test
         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 @@ class IpmiOnlyFirmwareTest : public ::testing::Test
             {"asdf", &imageMock},
         };
         handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
-            blobs, data, CreateTriggerMock(), CreateTriggerMock(),
-            CreateTriggerMock());
+            blobs, data, std::move(CreateActionMap("asdf")));
     }
 };
 
@@ -176,8 +204,7 @@ class FakeLpcFirmwareTest : public ::testing::Test
             {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 @@ std::unique_ptr<TriggerableActionInterface> CreateTriggerMock()
     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