bmc: add preparation step before data is received
Add a preparation systemd trigger event before data is received. On
systems under memory pressure, this'll trigger a service that can do
things like flush caches.
Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: I175177f4a91b58d9f163098572a9d2614002b718
diff --git a/Makefile.am b/Makefile.am
index a341233..c78a537 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -43,6 +43,7 @@
firmware_handler.cpp \
file_handler.cpp \
internal/sys.cpp \
+ prepare_systemd.cpp \
verify_systemd.cpp \
update_systemd.cpp
diff --git a/README.md b/README.md
index 1fc6027..c4e199b 100644
--- a/README.md
+++ b/README.md
@@ -108,6 +108,7 @@
`STATIC_HANDLER_STAGED_NAME` | `/run/initramfs/bmc-image` | The filename where to write the staged firmware image for static updates.
`TARBALL_STAGED_NAME` | `/tmp/image-update.tar` | The filename where to write the UBI update tarball.
`HASH_FILENAME` | `/tmp/bmc.sig` | The file to use for the hash provided.
+`PREPARATION_DBUS_SERVICE` | `prepare_update.service` | The systemd service started when the host starts to send an update.
`VERIFY_STATUS_FILENAME` | `/tmp/bmc.verify` | The file checked for the verification status.
`VERIFY_DBUS_SERVICE` | `verify_image.service` | The systemd service started for verification.
`UPDATE_DBUS_SERVICE` | `update-bmc.service` | The systemd service started for updating the BMC.
diff --git a/configure.ac b/configure.ac
index 8db7aa6..63c0bad 100644
--- a/configure.ac
+++ b/configure.ac
@@ -189,6 +189,10 @@
AS_IF([test "x$HASH_FILENAME" == "x"], [HASH_FILENAME="/tmp/bmc.sig"])
AC_DEFINE_UNQUOTED([HASH_FILENAME], ["$HASH_FILENAME"], [The file to use for the hash provided.])
+AC_ARG_VAR(PREPARATION_DBUS_SERVICE, [The systemd service started when the host starts to send an update.])
+AS_IF([test "x$PREPARATION_DBUS_SERVICE" == "x"], [PREPARATION_DBUS_SERVICE="prepare_update.service"])
+AC_DEFINE_UNQUOTED([PREPARATION_DBUS_SERVICE], ["$PREPARATION_DBUS_SERVICE"], [The systemd service started when the host starts to send an update.])
+
AC_ARG_VAR(VERIFY_STATUS_FILENAME, [The file checked for the verification status.])
AS_IF([test "x$VERIFY_STATUS_FILENAME" == "x"], [VERIFY_STATUS_FILENAME="/tmp/bmc.verify"])
AC_DEFINE_UNQUOTED([VERIFY_STATUS_FILENAME], ["$VERIFY_STATUS_FILENAME"], [The file checked for the verification status.])
diff --git a/firmware_handler.cpp b/firmware_handler.cpp
index 4967536..aaabe83 100644
--- a/firmware_handler.cpp
+++ b/firmware_handler.cpp
@@ -39,6 +39,7 @@
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)
{
@@ -72,8 +73,8 @@
}
return std::make_unique<FirmwareBlobHandler>(
- firmwares, blobs, transports, bitmask, std::move(verification),
- std::move(update));
+ firmwares, blobs, transports, bitmask, std::move(preparation),
+ std::move(verification), std::move(update));
}
/* Check if the path is in our supported list (or active list). */
@@ -712,6 +713,21 @@
void FirmwareBlobHandler::changeState(UpdateState next)
{
state = next;
+
+ if (state == UpdateState::notYetStarted)
+ {
+ /* Going back to notyetstarted, let them trigger preparation again. */
+ preparationTriggered = false;
+ }
+ else if (state == UpdateState::uploadInProgress)
+ {
+ /* Store this transition logic here instead of ::open() */
+ if (!preparationTriggered)
+ {
+ preparation->trigger();
+ preparationTriggered = true;
+ }
+ }
}
bool FirmwareBlobHandler::expire(uint16_t session)
diff --git a/firmware_handler.hpp b/firmware_handler.hpp
index de37e20..8c7cb04 100644
--- a/firmware_handler.hpp
+++ b/firmware_handler.hpp
@@ -104,6 +104,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);
@@ -121,13 +122,14 @@
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) :
handlers(firmwares),
blobIDs(blobs), transports(transports), bitmask(bitmask),
activeImage(activeImageBlobId), activeHash(activeHashBlobId),
verifyImage(verifyBlobId), updateImage(updateBlobId), lookup(),
- state(UpdateState::notYetStarted),
+ state(UpdateState::notYetStarted), preparation(std::move(preparation)),
verification(std::move(verification)), update(std::move(update))
{
}
@@ -221,6 +223,12 @@
/** The firmware update state. */
UpdateState state;
+ /* 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;
diff --git a/main.cpp b/main.cpp
index c857a6a..d28ecba 100644
--- a/main.cpp
+++ b/main.cpp
@@ -23,6 +23,7 @@
#include "lpc_handler.hpp"
#include "lpc_nuvoton.hpp"
#include "pci_handler.hpp"
+#include "prepare_systemd.hpp"
#include "status.hpp"
#include "update_systemd.hpp"
#include "util.hpp"
@@ -113,6 +114,8 @@
auto handler = ipmi_flash::FirmwareBlobHandler::CreateFirmwareBlobHandler(
ipmi_flash::supportedFirmware, ipmi_flash::supportedTransports,
+ ipmi_flash::SystemdPreparation::CreatePreparation(
+ sdbusplus::bus::new_default(), PREPARATION_DBUS_SERVICE),
ipmi_flash::SystemdVerification::CreateVerification(
sdbusplus::bus::new_default(), VERIFY_STATUS_FILENAME,
VERIFY_DBUS_SERVICE),
diff --git a/prepare_systemd.cpp b/prepare_systemd.cpp
new file mode 100644
index 0000000..b16bc99
--- /dev/null
+++ b/prepare_systemd.cpp
@@ -0,0 +1,71 @@
+/*
+ * Copyright 2019 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "prepare_systemd.hpp"
+
+#include "status.hpp"
+
+#include <memory>
+
+namespace ipmi_flash
+{
+
+std::unique_ptr<TriggerableActionInterface>
+ SystemdPreparation::CreatePreparation(sdbusplus::bus::bus&& bus,
+ const std::string& service)
+{
+ return std::make_unique<SystemdPreparation>(std::move(bus), service);
+}
+
+bool SystemdPreparation::trigger()
+{
+ static constexpr auto systemdService = "org.freedesktop.systemd1";
+ static constexpr auto systemdRoot = "/org/freedesktop/systemd1";
+ static constexpr auto systemdInterface = "org.freedesktop.systemd1.Manager";
+
+ auto method = bus.new_method_call(systemdService, systemdRoot,
+ systemdInterface, "StartUnit");
+ method.append(triggerService);
+ method.append("replace");
+
+ try
+ {
+ bus.call_noreply(method);
+ }
+ catch (const sdbusplus::exception::SdBusError& ex)
+ {
+ /* TODO: Once logging supports unit-tests, add a log message to test
+ * this failure.
+ */
+ state = ActionStatus::failed;
+ return false;
+ }
+
+ state = ActionStatus::success;
+ return true;
+}
+
+void SystemdPreparation::abort()
+{
+ return;
+}
+
+ActionStatus SystemdPreparation::status()
+{
+ return state;
+}
+
+} // namespace ipmi_flash
diff --git a/prepare_systemd.hpp b/prepare_systemd.hpp
new file mode 100644
index 0000000..af8e065
--- /dev/null
+++ b/prepare_systemd.hpp
@@ -0,0 +1,40 @@
+#pragma once
+
+#include "status.hpp"
+
+#include <memory>
+#include <sdbusplus/bus.hpp>
+#include <string>
+
+namespace ipmi_flash
+{
+
+class SystemdPreparation : public TriggerableActionInterface
+{
+ public:
+ static std::unique_ptr<TriggerableActionInterface>
+ CreatePreparation(sdbusplus::bus::bus&& bus,
+ const std::string& service);
+
+ SystemdPreparation(sdbusplus::bus::bus&& bus, const std::string& service) :
+ bus(std::move(bus)), triggerService(service)
+ {
+ }
+
+ ~SystemdPreparation() = default;
+ SystemdPreparation(const SystemdPreparation&) = delete;
+ SystemdPreparation& operator=(const SystemdPreparation&) = delete;
+ SystemdPreparation(SystemdPreparation&&) = default;
+ SystemdPreparation& operator=(SystemdPreparation&&) = default;
+
+ bool trigger() override;
+ void abort() override;
+ ActionStatus status() override;
+
+ private:
+ sdbusplus::bus::bus bus;
+ const std::string triggerService;
+ ActionStatus state = ActionStatus::unknown;
+};
+
+} // namespace ipmi_flash
diff --git a/test/firmware_canhandle_unittest.cpp b/test/firmware_canhandle_unittest.cpp
index 7fb4c7f..dabf8ef 100644
--- a/test/firmware_canhandle_unittest.cpp
+++ b/test/firmware_canhandle_unittest.cpp
@@ -33,7 +33,8 @@
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock());
+ blobs, data, CreateTriggerMock(), CreateTriggerMock(),
+ CreateTriggerMock());
for (const auto& item : items)
{
diff --git a/test/firmware_commit_unittest.cpp b/test/firmware_commit_unittest.cpp
index 11ce3fc..b8c651e 100644
--- a/test/firmware_commit_unittest.cpp
+++ b/test/firmware_commit_unittest.cpp
@@ -49,7 +49,8 @@
std::make_unique<StrictMock<TriggerMock>>();
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, std::move(verifyMock), CreateTriggerMock());
+ blobs, data, CreateTriggerMock(), std::move(verifyMock),
+ CreateTriggerMock());
EXPECT_CALL(imageMock2, open("asdf")).WillOnce(Return(true));
@@ -71,7 +72,8 @@
std::make_unique<StrictMock<TriggerMock>>();
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, std::move(verifyMock), CreateTriggerMock());
+ blobs, data, CreateTriggerMock(), std::move(verifyMock),
+ CreateTriggerMock());
EXPECT_CALL(imageMock1, open(StrEq(hashBlobId))).WillOnce(Return(true));
diff --git a/test/firmware_createhandler_unittest.cpp b/test/firmware_createhandler_unittest.cpp
index dc6d968..dd09cbe 100644
--- a/test/firmware_createhandler_unittest.cpp
+++ b/test/firmware_createhandler_unittest.cpp
@@ -31,7 +31,8 @@
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock());
+ blobs, data, CreateTriggerMock(), CreateTriggerMock(),
+ CreateTriggerMock());
// EXPECT_EQ(handler, nullptr);
EXPECT_FALSE(handler == nullptr);
diff --git a/test/firmware_handler_unittest.cpp b/test/firmware_handler_unittest.cpp
index a5d60f5..27bd517 100644
--- a/test/firmware_handler_unittest.cpp
+++ b/test/firmware_handler_unittest.cpp
@@ -22,7 +22,8 @@
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- {}, data, CreateTriggerMock(), CreateTriggerMock());
+ {}, data, CreateTriggerMock(), CreateTriggerMock(),
+ CreateTriggerMock());
EXPECT_EQ(handler, nullptr);
}
TEST(FirmwareHandlerTest, CreateEmptyDataHandlerListFails)
@@ -35,7 +36,8 @@
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, {}, CreateTriggerMock(), CreateTriggerMock());
+ blobs, {}, CreateTriggerMock(), CreateTriggerMock(),
+ CreateTriggerMock());
EXPECT_EQ(handler, nullptr);
}
TEST(FirmwareHandlerTest, VerifyHashRequiredForHappiness)
@@ -51,13 +53,15 @@
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock());
+ blobs, data, CreateTriggerMock(), CreateTriggerMock(),
+ CreateTriggerMock());
EXPECT_EQ(handler, nullptr);
blobs.push_back({hashBlobId, &imageMock});
handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock());
+ blobs, data, CreateTriggerMock(), CreateTriggerMock(),
+ CreateTriggerMock());
auto result = handler->getBlobIds();
std::vector<std::string> expectedBlobs = {"asdf", hashBlobId};
EXPECT_THAT(result, UnorderedElementsAreArray(expectedBlobs));
diff --git a/test/firmware_stat_unittest.cpp b/test/firmware_stat_unittest.cpp
index 4efae94..99ae770 100644
--- a/test/firmware_stat_unittest.cpp
+++ b/test/firmware_stat_unittest.cpp
@@ -29,7 +29,8 @@
};
auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock());
+ blobs, data, CreateTriggerMock(), CreateTriggerMock(),
+ CreateTriggerMock());
blobs::BlobMeta meta;
EXPECT_TRUE(handler->stat("asdf", &meta));
diff --git a/test/firmware_state_notyetstarted_tarball_unittest.cpp b/test/firmware_state_notyetstarted_tarball_unittest.cpp
index 5aa2baa..275763c 100644
--- a/test/firmware_state_notyetstarted_tarball_unittest.cpp
+++ b/test/firmware_state_notyetstarted_tarball_unittest.cpp
@@ -36,7 +36,8 @@
updateMockPtr = reinterpret_cast<TriggerMock*>(updateMock.get());
handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, std::move(verifyMock), std::move(updateMock));
+ blobs, data, CreateTriggerMock(), std::move(verifyMock),
+ std::move(updateMock));
}
void expectedState(FirmwareBlobHandler::UpdateState state)
diff --git a/test/firmware_state_notyetstarted_unittest.cpp b/test/firmware_state_notyetstarted_unittest.cpp
index 528e47f..54ab7f8 100644
--- a/test/firmware_state_notyetstarted_unittest.cpp
+++ b/test/firmware_state_notyetstarted_unittest.cpp
@@ -93,6 +93,7 @@
TEST_F(FirmwareHandlerNotYetStartedTest, OpenStaticImageFileVerifyStateChange)
{
EXPECT_CALL(imageMock, open(staticLayoutBlobId)).WillOnce(Return(true));
+ EXPECT_CALL(*prepareMockPtr, trigger()).WillOnce(Return(true));
EXPECT_TRUE(handler->open(session, flags, staticLayoutBlobId));
@@ -104,6 +105,7 @@
TEST_F(FirmwareHandlerNotYetStartedTest, OpenHashFileVerifyStateChange)
{
EXPECT_CALL(imageMock, open(hashBlobId)).WillOnce(Return(true));
+ EXPECT_CALL(*prepareMockPtr, trigger()).WillOnce(Return(true));
EXPECT_TRUE(handler->open(session, flags, hashBlobId));
diff --git a/test/firmware_state_verificationpending_unittest.cpp b/test/firmware_state_verificationpending_unittest.cpp
index 92ba15b..dd2df27 100644
--- a/test/firmware_state_verificationpending_unittest.cpp
+++ b/test/firmware_state_verificationpending_unittest.cpp
@@ -214,6 +214,9 @@
EXPECT_THAT(handler->getBlobIds(),
UnorderedElementsAreArray(expectedBlobs));
+ /* Verifies it isn't triggered again. */
+ EXPECT_CALL(*prepareMockPtr, trigger()).Times(0);
+
EXPECT_CALL(imageMock, open(staticLayoutBlobId)).WillOnce(Return(true));
EXPECT_TRUE(handler->open(session, flags, staticLayoutBlobId));
expectedState(FirmwareBlobHandler::UpdateState::uploadInProgress);
diff --git a/test/firmware_unittest.hpp b/test/firmware_unittest.hpp
index e36081d..8d92fe8 100644
--- a/test/firmware_unittest.hpp
+++ b/test/firmware_unittest.hpp
@@ -29,6 +29,10 @@
{staticLayoutBlobId, &imageMock},
};
+ std::unique_ptr<TriggerableActionInterface> prepareMock =
+ std::make_unique<TriggerMock>();
+ prepareMockPtr = reinterpret_cast<TriggerMock*>(prepareMock.get());
+
std::unique_ptr<TriggerableActionInterface> verifyMock =
std::make_unique<TriggerMock>();
verifyMockPtr = reinterpret_cast<TriggerMock*>(verifyMock.get());
@@ -38,7 +42,8 @@
updateMockPtr = reinterpret_cast<TriggerMock*>(updateMock.get());
handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, std::move(verifyMock), std::move(updateMock));
+ blobs, data, std::move(prepareMock), std::move(verifyMock),
+ std::move(updateMock));
}
void expectedState(FirmwareBlobHandler::UpdateState state)
@@ -50,6 +55,7 @@
void openToInProgress(const std::string& blobId)
{
EXPECT_CALL(imageMock, open(blobId)).WillOnce(Return(true));
+ EXPECT_CALL(*prepareMockPtr, trigger()).WillOnce(Return(true));
EXPECT_TRUE(handler->open(session, flags, blobId));
expectedState(FirmwareBlobHandler::UpdateState::uploadInProgress);
}
@@ -117,6 +123,7 @@
std::vector<DataHandlerPack> data = {
{FirmwareBlobHandler::UpdateFlags::ipmi, nullptr}};
std::unique_ptr<blobs::GenericBlobInterface> handler;
+ TriggerMock* prepareMockPtr;
TriggerMock* verifyMockPtr;
TriggerMock* updateMockPtr;
@@ -143,7 +150,8 @@
{"asdf", &imageMock},
};
handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock());
+ blobs, data, CreateTriggerMock(), CreateTriggerMock(),
+ CreateTriggerMock());
}
};
@@ -167,7 +175,8 @@
{FirmwareBlobHandler::UpdateFlags::lpc, &dataMock},
};
handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(
- blobs, data, CreateTriggerMock(), CreateTriggerMock());
+ blobs, data, CreateTriggerMock(), CreateTriggerMock(),
+ CreateTriggerMock());
}
};